Conversation
A multi-round review with adversarial verification confirmed 13 defects beyond those fixed in bursts 2-4. Each fix carries a regression test that was verified failing against the pre-fix code. Correctness and data integrity: - report: attach events by utterance position, not by the transcript's unvalidated id, so an id-less or duplicate-id transcript no longer renders every event under every utterance. - timeline: reject an interaction record missing t or kind, which previously became a phantom event at roughly -1.78e9 seconds. - analyze: label validation errors by the finding's position in the answer, not in the post-decode slice, so messages name the right one. Injection and unsafe writes: - analyze: sanitise the manifest's app, participant, and task text in the emitted request; it was the one consumer of those fields that did not. - record: refuse a non-regular ffmpeg output path, so a planted symlink cannot redirect a recording outside the session directory. - session: refuse a non-regular file in OpenFileNoFollow, so a planted FIFO can no longer hold a write open for ever. - session: strip U+061C, completing the Unicode Bidi_Control set. Error handling and resource lifetime: - review: propagate a verdict persistence failure instead of printing it as a retry hint and exiting 0 with the verdict never on disk. - review: start the appended verdict on a fresh line when the existing findings file lacks its final newline. - record: sample the start-up window when the exit is observed, not after the stop path has already burned the grace period. - demo: bound the capture server's read and idle timeouts and shut it down under a deadline, so a stalled connection cannot hang the session. - demo: reject a record larger than the readers' line limit rather than persisting one no reader can take back. - demo: reject an addr that does not parse, so an empty -addr cannot bind the unauthenticated capture endpoints on every interface. Assisted-by: Claude:claude-opus-4-8
Reviewing the architecture note against the intent set surfaced four user-facing capabilities the note describes but no intent recorded. - itd-6 terminal-capture: asciinema .cast capture for CLI targets. This closes a dependency hole — itd-3's acceptance criteria already assume a cast stream exists, while itd-1 fences asciinema wrappers out of scope. - itd-7 native-app-capture: macOS and iOS capture, keyframe-anchored, kept separate from itd-6 so the two evidence models are not conflated. - itd-8 local-analysis: a flag on the analysis surface selecting a local model backend, same rubric and schema, backend recorded for provenance. - itd-9 regression-test-drafting: a drafted test case from a confirmed finding, for human accept or reject. All four are drafts; each carries acceptance criteria and the open questions that need answers before it can be planned. Extension-injected capture of third-party websites is deliberately not among them: itd-4 already lists it in scope for reference capture. Assisted-by: Claude:claude-opus-4-8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two independent commits, reviewable separately.
1. Thirteen bug fixes
A multi-round review with three-lens adversarial verification ran over the tree at
bughunt-4. It confirmed 21 findings (13 distinct after dedup) from 161 candidates — 140 were refuted by the verification panel and dropped. Each fix carries a regression test verified failing against the pre-fix code.Major
report.go— events attached by utterance ID. The attachment map was keyed on the transcript'sid, whichmergecopies verbatim and never validates. An id-less transcript gives every speech entry the ID"", collapsing all utterances into one bucket, so every event renders under every utterance. Reproduced against the real binary: three id-less utterances plus three clicks produced nine event lines for three events.report.mdis the human evidence artefact, so this silently fabricated the record of what the participant was doing while speaking. Now keyed by position in thespeechslice, removing the dependence on ID uniqueness entirely.emit.go— unsanitised manifest text.App,ParticipantandTaskswere written into the emitted analysis request withoutsession.SafeText— the one consumer of those fields that did not sanitise them. A session directory is an exchange unit, so attacker-authored manifest text could inject ANSI escapes into the operator's terminal and forge Markdown structure inside the request an agent is asked to obey.Minor
timeline.go— an interaction record missingtbecame a phantom event at roughly −1.78e9 seconds, counted by merge and rendered at 00:00. Now rejected, with a pointer-typed decode so an absenttis distinguishable from a legitimatet == t0.ingest.go/validate.go— validation errors were labelled by position in the post-decode slice, so a decode failure earlier in the answer made every subsequent message name the wrong finding.record.go— the start-up window was sampled afterstopAllhad already burned the grace period, misclassifying a genuine TCC permission denial as an unexpected mid-session stop.record.go— ffmpeg output paths bypassed the symlink guard every other write in the codebase uses; a planted symlink ataudio.wavredirected the recording outside the session directory.session.go—OpenFileNoFollowrefused symlinks but not other non-regular files, so a planted FIFO held a write open for ever. NowO_NONBLOCKplus an fstat regular-file check.session.go—SafeTextomitted U+061C ARABIC LETTER MARK, leaving one member of the Unicode Bidi_Control set able to do the reordering the rest are stripped to prevent.review.go— a verdict persistence failure was returned through the same channel as an invalid keystroke, printed as a retry hint and swallowed;testimony reviewexited 0 with the human's verdict never on disk.review.go—AppendVerdictdid not check for a trailing newline, so a findings file whose last line was unterminated was concatenated into one malformed record, breaking the whole file for every reader.demo.go— noReadHeaderTimeout/ReadTimeout/IdleTimeoutand an unboundedShutdown, so one stalled connection hungtestimony recordfor ever after Ctrl+C.demo.go— the capture endpoint accepted 8 MiB bodies while every reader caps a line at 4 MiB, durably persisting records no reader could take back. Now bounded by the exportedsession.MaxJSONLLine.demo.go—listenAddrfell through to the raw string on aSplitHostPortfailure, so an empty-addrbound the unauthenticated capture endpoints on every interface — the exact outcome its doc comment claims to prevent.assets/index.html— the slider's synthesised click was re-captured by the document listener, recording one user toggle as two interactions.2. Four intent records
Reviewing the architecture note against the intent set surfaced four user-facing capabilities the note describes but no intent recorded. All filed as drafts with acceptance criteria and open questions.
.castcapture for CLI targets. Closes a dependency hole: itd-3's acceptance criteria already assume a cast stream exists, while itd-1 fences asciinema wrappers out of scope.Extension-injected capture of third-party websites is deliberately absent: itd-4 already lists it in scope.
Verification
gofmt -l,go vet ./...,go test ./...andabcd auditall green. +1631/−78 across 27 files, roughly half of it regression tests.Note on the prior hunt's conclusion
PR #19 reported the hunt had reached its noise floor and recommended stopping. That conclusion did not hold — this round found 13 further defects on the same tree, two of them major. A follow-up hunt against this tree is running now, including two passes dedicated to reviewing these fixes, and the
data-integrityand completeness-sweep lenses that died on network errors last time.Assisted-by: Claude:claude-opus-4-8