[LUOaini1213]

Fixes from the 2026-11-03 portfolio review. Every finding was re-checked on the current main (09b4cdb, after PR #3) before any change.

Findings

id	outcome	what
track1-1 (timing-flaky tests on Windows)	fixed	ReplayRunner.run() registered its cancel hook only after await this.load(), so a Stop during the first fixture load was dropped. The hook is now registered before the first await, and a cancel seen after loading throws RunCancelledError. The cancel test waits on the first replayed event, not a fixed 20 ms sleep, and a new test cancels before the load finishes. Outbox tests used a 100 ms client timeout, and a client timeout means a retry, so on a loaded host "exactly 1 request" became 2. They now use 2 s; the timeout test keeps 100 ms. The Retry-After assertion measures from a point before the attempt. apps/server/vitest.config.ts raises the per-test ceiling (30 s) and the expect.poll ceiling (10 s). These are ceilings, not sleeps. The OTLP smoke helper now waits up to 60 s for a cold server start. CI adds windows-latest (Node 24).
track1-2 (README buries own work)	already-fixed (refuted on 09b4cdb), leftover fixed	The screenshots were already moved by d2133d7. Two things remained: no measured result on the README, and the description sat after the demo commands. The first screen now gives what was built, what was measured (recovery PASS for 503/timeout/SIGKILL; 100-span persist p95 2.1 vs 12.1 ms, each with an evidence link) and the limits.
track1-3 (starter vs own work)	fixed (team attribution deferred)	New Provenance section: the starter import commit 8d0bd4f, the compare link 8d0bd4f...main, and the list of added modules (checked against git diff --name-status 8d0bd4f HEAD). "This fork" is gone because GitHub reports fork=false. package.json now describes this project. LICENSE is unchanged (team repo). Naming team members, if any, is for the owner to decide.
track1-4 (policy false positives / glob bypass)	fixed	Reproduced: cat .secr*/demo* was allowed. A glob that expands to .secrets is now denied (.secr*, .s?crets, .[s]ecrets, .{secrets,x}, also after a / as in ./.secr*, added in review commit 2d581e2). Regex-like .*foo (but not .*ts, which also matches .secrets as a glob and is denied), .cache/*, .git* and .github/... stay allowed, and both sets are tested. "Write a README section that explains how to set the OPENAI_API_KEY ..." was denied because the to in "how to" was read as a destination. It is now allowed and pinned. Two routine prompts stay denied on purpose, and a test with a comment explains the trade-off: "send it in the Authorization header" (credential-egress) and "paste their API key and display it masked" (print-ark-secret).
track1-5 (benchmark predates HEAD)	fixed	Reproduced: 3 of the 8 source hashes no longer matched. npm run benchmark:otlp was re-run at ff2da0d on this machine (Node 24.19.0, Windows, i7-11370H). The old record is kept as otlp-benchmark-5552e1e.json and labelled historical. New npm run check:evidence (first step of npm run check and of CI) fails on: doc/README numbers that differ from the JSON, a hashed source that changed since the measurement, a fault mode recorded in otlp-recovery.json that the README does not name or the CI recovery smoke test does not exercise (the reverse direction is not checked), and broken relative Markdown links.
track1-6 (stale docs)	fixed	CI comments translated to English and the stale "28 files / 271 cases" count dropped. The clone step uses the real URL. terraform fmt and docker compose config are marked as manual, not CI. The H1 now says "Glass Box Trace Plane", matching the repo description.
track1-7 (organizer XML / Devpost steps)	XML already-fixed (b2d5bda); rest fixed	docs/SUBMISSION.md keeps the write-up and drops the click-through steps and the bit.ly registration link. git grep bit.ly and ls docs/*.xml both return nothing.

Each new check fails once

check	how it was broken (temp copy or restored file)	result
cancel-during-load test	restored the old replay-runner.ts	failed
glob test	removed the globReachesProtectedDir call	failed
"how to" test	removed the (?<!\bhow ) lookbehind	failed
Retry-After assertion	made the outbox ignore Retry-After	failed (133 ms < 2000)
check:evidence	changed a README number, a doc table cell, the pending count, otlp-outbox.ts, a README fault mode, a link target, the platform word (7 mutations)	7/7 exit 1; unmodified copy exits 0
timeouts / vitest config	10 concurrent full server suites on Windows	before: 10/10 runs red (2-9 failures each, the cancel test in 8); after: 10/10 green (191/191)

Commands run (Windows 11, Node 24.19.0)

• npm ci
• npm run check on 09b4cdb before any change: exit 0
• npm run check after: exit 0. check:evidence OK, tests 5 + 191 + 80, all smokes PASS
• npm run build && npm run benchmark:otlp (new evidence)
• 10x concurrent npx vitest run in apps/server, before and after

Suggested repo metadata (not changed)

The description is accurate. If you want the result in it: "TikTok TechJam 2026 Track 1 (Glass Box): span waterfall, redaction, secret-exfiltration gate and durable OTLP export on the Volc Agent Launchpad starter". Possible extra topics: typescript, fastify, codex.

Left for the owner

• Team members and roles, if this was a team entry. The README says every later commit is yours, which matches the history.
• LICENSE copyright line (team repo; unchanged).

🤖 Generated with Claude Code

[LUOaini1213]

Independent review of 13518e4

Recommendation: merge after you read the README first screen and Provenance section. The code and test changes hold up. The README now makes claims about attribution and quotes measured numbers, so you should confirm those yourself. I did not merge anything or push to this branch.

What I re-ran (Windows 11, Node 24.19.0, i7-11370H, same machine as the new benchmark)

• npm run check: exit 0 in 56 s. check:evidence OK (8/8 source hashes match ff2da0d), tests 5 + 191 + 80, smoke / smoke:replay / smoke:otlp all PASS.
• 6 full server suites run at the same time: 6/6 green (191/191 each). The verified review saw red runs under similar load on 09b4cdb.
• CI on 13518e4: push and pull_request runs both green, all 6 jobs (ubuntu 22/24, windows 24).
• docs/evidence/otlp-benchmark-5552e1e.json has the same blob as the old otlp-benchmark.json on main (4e1da22), so the old evidence was renamed, not deleted.
• The Provenance module list matches git diff --name-status 8d0bd4f HEAD. Every commit after 8d0bd4f is authored by you under one of your names.
• The diff adds no personal paths, emails or phone numbers. Both commits use the noreply identity and carry the Co-Authored-By line.

Mutation checks (each guarded thing broken once, then restored)

broken	result
replay-runner.ts reverted to main	cancel-during-load test fails
globReachesProtectedDir call removed	glob test fails
(?<!\bhow ) lookbehind removed	"how to set OPENAI_API_KEY" test fails
outbox ignores Retry-After	Retry-After test fails
check:evidence, temp copy: README 12.1 to 12.2, 40 to 41 samples, "HTTP timeout" renamed, "Windows" to "Linux", broken link, OTLP_DELIVERY table cell, completed-Run count, span-store.ts edited, verifier fault-mode string changed	9/9 exit 1
check:evidence, temp copy: otlp.ts converted to CRLF only	exit 0, as intended (hashes are LF-normalized)

Small things to know (none blocks the merge)

1. The PR body overstates one check:evidence rule. It says the check fails on "a README fault mode that the CI recovery smoke test does not exercise". What it actually checks is the reverse: each fault mode in otlp-recovery.json must be named in the README and in verify-otlp-recovery.mjs. I added "a DNS failure" to the README fault list in a temp copy, and check:evidence still exited 0.
2. The glob rule adds a new false positive. Regex text such as .*ts or .*s is denied as protected-env-file, because .*ts also matches .secrets as a glob. The test pins only .*foo as allowed. This is the usual trade-off of a pattern-based gate, and the README already says it has false positives, but the trade-off is not pinned by a test.
3. The new post-load if (cancelled) check in replay-runner.ts is redundant. If you remove it, all tests still pass, because the per-event check in the loop catches the same case. It is harmless.
4. The benchmark numbers come from a different machine. The README now quotes the re-run on an i7-11370H laptop (2.1 vs 12.1 ms). The earlier record came from an i5-14500. The docs label both, but the two sets of numbers cannot be compared.
5. Expect CI to go red after edits to measured files. Any later change to otlp-outbox.ts, otlp.ts, config.ts, agent-service.ts, span-store.ts, the benchmark scripts or package-lock.json fails CI until you re-run npm run benchmark:otlp or retire the record. This is deliberate.

For you to confirm before merging

• "Every later commit is mine" and the absence of team names. The README and the history agree, but the TechJam rules imply teams (the old SUBMISSION.md said "Every teammate").
• The LICENSE copyright line and the repo description/topics are unchanged. The suggested wording is in the PR body.

🤖 Generated with Claude Code

[LUOaini1213]

Second independent review (head 2d581e2)

Recommendation: merge after you read the README first screen and the Provenance section. The code, tests and evidence check hold up. I pushed one small fix commit (2d581e2) and corrected two lines of the PR body. Nothing was merged.

Bypass found and fixed in 2d581e2

cat ./.secr*/demo* and tar czf out.tgz workspace/.s?crets were still allowed on 13518e4. The glob rule only looked for a dot-word after whitespace, a quote, =, ( or :, not after /. A / now counts too, and both commands are in the glob test. Removing the / again makes that test fail (cat ./.secr*/demo*: expected true to be false). The same commit corrects the check-evidence.mjs header comment, which described the fault-mode check backwards.

Re-run on Windows 11, Node 24.19.0, i7-11370H

• npm run check exits 0 on both 13518e4 and 2d581e2. check:evidence OK (8/8 hashes), tests 5 + 191 + 80, smoke, smoke:replay and smoke:otlp all PASS (recovery: HTTP 503, HTTP timeout, SIGKILL restart).
• CI on 2d581e2: all 6 jobs green (ubuntu Node 22/24, windows Node 24, push and pull_request).
• The already-fixed items are confirmed on main: d2133d7 and b2d5bda are ancestors of main, and docs/ on main has no XML.
• I probed the policy with 24 prompts. Besides the / gap, quote splitting such as ".secr"*/demo* still gets through. A text-pattern gate can't close that, and the README's Limits paragraph already says so.

check:evidence mutations (temp copy, not covered by the first review)

broken	result
OTLP_DELIVERY Node 24.19.0 changed to 24.18.0	exit 1
pending count 44 changed to 45	exit 1
one byte appended to package-lock.json	exit 1
"a DNS failure" added to the README fault list	exit 0. Known: only the evidence-to-README direction is checked. The PR body and the script comment now say so

What I could not verify

• The benchmark JSON stores only summary statistics (median, p95, mean), not raw samples. I cannot recompute 2.1 / 12.1 ms from the file. They are consistent with the record and with the source hashes, but only a re-run reproduces them.
• The review's top suggestion was to put the waterfall screenshot on the first screen. It is at README line 45, below the no-key demo block, so at 1366x768 it is probably just below the fold. Your call.

For you to decide

• "Every later commit is mine" and naming team members, if any (TechJam implies teams).
• The LICENSE copyright line and the repo description/topics are unchanged. The suggested wording is in the PR body.
• Any later edit to the 8 hashed files will turn CI red until the benchmark is re-run or the record is retired. This is deliberate.

🤖 Generated with Claude Code

[LUOaini1213]

Third independent review (head 3148ce0)

Recommendation: merge after you read the README first screen and the Provenance section. The code, tests and evidence check hold up. I pushed one tests-only commit (3148ce0). Nothing was merged.

What I re-ran (Windows 11, Node 24.19.0, i7-11370H)

• npm run check on 2d581e2: exit 0 in 56 s. check:evidence OK (8/8 source hashes), tests 5 + 191 + 80, all three smoke runs PASS.
• Load test with a control. 8 full server suites at the same time on the PR head: 8/8 green (191/191 each). The same 8 with main's replay-runner.ts, main's three test files and no vitest.config.ts: 8/8 red, 3 or more failures each. The cancel test, the outbox tests and the delivery-http tests all failed, as the finding said.
• CI on 3148ce0: all 6 jobs green (Ubuntu Node 22 and 24, Windows Node 24, push and pull_request).
• The old benchmark record was renamed, not deleted: otlp-benchmark-5552e1e.json has the same blob as otlp-benchmark.json on main.
• The Provenance module list matches git diff --name-status 8d0bd4f HEAD. Every commit after 8d0bd4f is under one of your names. LICENSE is untouched.
• The diff adds no local paths, emails or phone numbers. git grep bit.ly, docs/*.xml and <repository-url> all come back empty.

The benchmark numbers reproduce only roughly

I ran npm run benchmark:otlp 4 more times on the same machine, writing to a scratch file, with other work running:

run	100-span persist p95, no outbox	with outbox
committed record	2.1 ms	12.1 ms
1	11.0 ms	56.8 ms
2	2.0 ms	20.0 ms
3	6.3 ms	21.5 ms
4	2.0 ms	9.5 ms

The committed numbers are real and sit at the quiet end of this range. The conclusion holds in every run: the durable arm costs several times more. But "12.1 ms" is one run of 40 samples on a shared laptop, so do not quote it as a stable figure. The docs already say the numbers are host-specific, so I left the wording alone. Whether to add a range is your call.

Commit 3148ce0 (tests only, policy.ts unchanged)

The README says the gate has false positives and negatives "pinned in policy.test.ts". Only false positives were pinned. The commit adds:

• Known misses, pinned as allowed: cat .???????/d*, cat .*/d*, cat ".secr"*/demo* and the Windows form type .\.secr*\demo*. All four reach .secrets in a real shell. A text-pattern gate cannot close these without denying ordinary regex text.
• Known over-blocking, pinned as denied: ls .s* and the regex text .*ts.
• Two egress prompts that must stay denied next to the "how to" exclusion: "write the ARK_API_KEY to notes.txt" and "Explain how to write the API key to notes.txt".

Each was broken once in policy.ts and restored: widening the "how to" exclusion, removing it, dropping the letter requirement and removing the glob rule each fail a test. The server suite is now 193 tests; the PR body still says 191.

check:evidence mutations (temp copy, new ones)

Evidence p95 changed, README 2.1 to 2.0, historical evidence file deleted, recovery result set to FAIL, config.ts edited, allRunsCompleted false, a table cell changed, an unexercised fault mode added to the recovery record, source SHA changed in the doc: 9/9 exit 1. The unmodified copy exits 0.

For you to decide

• "Every later commit is mine." The history supports it, but TechJam entries are usually team entries. Name teammates if there were any.
• The first screen now quotes 2.1 ms vs 12.1 ms. See the table above.
• The recovery evidence file is still the 2026-10-09 run at c6ff89c. It was not re-recorded, but the same scenario passes in CI on every build.
• Any later edit to the 8 hashed files turns CI red until the benchmark is re-run or the record is retired. This is deliberate.
• LICENSE copyright line and repo description/topics: unchanged.

🤖 Generated with Claude Code