Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .abcd/work/DECISIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -920,4 +920,5 @@ parallel-agent merge contention bites.
- 2026-08-05 — iss-80 (v0.5.0): resolved as already-fixed, with the residual verification gap closed rather than the defect re-fixed. The branch-local id-collision class this item tracked — `itd-N`/`spc-N`/`iss-N` allocators scanning only the working tree, so two agents on branches cut from one base each mint the same id and collide at merge — was already closed by iss-115 and iss-120, which introduced `recordid.MaxAcrossRefs` (scan every git ref for the family's highest committed id) and folded it into all three minting paths: `internal/core/spec/store.go`, `internal/core/intent/create.go`, and `internal/core/capture/workflow.go`. What survived was uneven evidence, not an uneven fix: only the `spc-N` family had an end-to-end regression test standing up a real two-branch history (`internal/core/spec/refunion_test.go`), while none of capture's or intent's existing tests stood up a real two-branch history through `Capture`/`CreateFromText` and so never drove the ref scan through their public entry points at all — `MaxAcrossRefs` was exercised only as a primitive in its own package test, never as wiring. That is the shape in which a fix silently un-wires: a refactor that dropped the `MaxAcrossRefs` call from `Capture` or `CreateFromText` would have left every existing test in both packages green. So this round adds `internal/core/capture/refunion_test.go` (`TestCaptureMintsPastACommittedBranch`) and `internal/core/intent/refunion_test.go` (`TestCreateFromTextMintsPastACommittedBranch`), each mirroring the spec test's exact fixture via `internal/gittest.NewRepo` — branch A mints and commits record 1, branch B is cut from before that commit so its working tree carries no record, and B must mint record 2. Both were mutation-checked before being trusted: with `MaxAcrossRefs` stubbed to return an empty `RefScan`, each fails with precisely the collision it names (`iss-1`/`itd-1` re-minted on branch B), and all three families fail together — so the tests are pinned to the ref-union behaviour rather than passing incidentally. No production code was touched, which is the point: the minting paths were verified correct by reading before the coverage was written, and the change is test-and-record only. Impact recorded as `internal` and no CHANGELOG entry, matching the iss-34 precedent for a test-only closure with no user-facing behaviour change.
- 2026-08-05 — CORRECTION to the entry immediately above, from a merge-gate record-accuracy review on PR #195. That entry, and iss-80's own `resolution:` field, said the branch-local id-collision class "was closed" by iss-115/iss-120. It was not: `internal/core/recordid/recordid.go`'s own package doc documents an ACCEPTED, undocumented-in-that-entry residual window — two branches that BOTH mint before either commits still collide — left to the already-armed record-lint uniqueness detectors (`issue_id_unique`, `intent_lifecycle`, `spec_id_unique`) as backstop, the same trade-off iss-120's own resolution names verbatim ("the armed record-lint detectors as the residual-window backstop"). iss-80's body asked for a fully collision-free minting scheme (forge-minted / random-suffix / timestamp / reserve-registry); that was never built, that trade-off was made when iss-115/iss-120 landed rather than here, and no open item currently tracks the residual window separately. Both iss-80's `resolution:` field and this log now say "already addressed, not closed to zero" and name the trade-off explicitly, rather than implying the class was eliminated. The test-coverage work in the entry above (the two new refunion tests) is unaffected and stands as described.
- 2026-08-05 — iss-43 (v0.5.0 item D): the three-claim corpus is closed as OVERTAKEN rather than fixed — the Status section and the surface list were removed by `73428b6` (2026-07-17, "manual: Revise README with new badges and project details"), a manual revision predating the v0.5.0 plan, so the Phase 0 claim and the native review oracle / spec-task engine / autonomous run claims no longer exist to correct, and re-deriving a corpus against a rewritten document would be inventing findings to justify a ticket. That attribution is itself a correction worth recording: the round first credited the iss-143 tagline commit `48a3524`, which touches ONE README line (the strapline), because it read a SHALLOW clone whose truncation window opened well after 2026-07-17 and therefore could not see the real commit — a truncated history answers a `git log` question confidently and wrongly, and the cheap guard is `git log -S` over the file for the exact removed string, unshallowed first. Two corollaries earned the same way: the corpus's third claim was never `73428b6`'s at all (`git log -S "never shipped"` puts that line in the scaffold commit and in this branch alone), so "all three overtaken" was a second wrong attribution riding on the first; and a whole-phrase probe can miss a claim that was line-wrapped in the original, which is why `"spec/task"` reproduces where `"spec/task engine"` returns nothing. One instance survives, and it is half true rather than false, which is why it needed splitting instead of deleting: the Layout line called `.abcd/` "never shipped", which holds for the released binaries alone (release.yml uploads the four of them and `checksums.txt`, nothing else) and fails everywhere else — `.claude-plugin/marketplace.json` declares `source: "./"`, so a marketplace install takes the whole repository, and GitHub attaches auto-generated source archives to each release, which carry the directory too because `.gitattributes` declares no `export-ignore`. The line now divides on the boundary that is real: every repository checkout against the released binaries, rather than a per-channel split that would have been wrong about the release page. Doc-side deliberately: the marketplace manifest schema has no per-path exclusion, so making "never shipped" true everywhere means building a curated publish path — feature work with its own design questions, not doc repair, and writing the doc to describe a packaging mechanism that does not exist is the phantom-claim failure this item belongs to. The proposed detector (README capability and status claims resolve to wired verbs and to the roadmap) is DROPPED here and re-pointed at iss-181 as a candidate extension of the gate cross-check detector's scope, recorded as a candidate rather than a commitment because it shares that detector's two unsettled scoping questions and a second half-specified scanner would answer neither. The same claim survives across the record — CONTEXT.md's sharp-edges list, AGENTS.md, both `.abcd/` READMEs, `02-constraints/01-platform.md`, `05-internals/03-configuration.md` and `phase-1-ahoy.md`, with adr-0028, the dated plans and the planned/shipped intent bodies exempt as decision and historical records — and it is recorded as iss-183 rather than fixed here, since the disposition scoped this item to one README line. That issue also corrects a claim this round made twice: the exclusion is NOT unimplemented. `internal/core/launch/bundle.go` denies the `.abcd` namespace structurally, with tests, reachable from the wired `abcd launch ship`; it is unwired, because `Ship` stops at `WouldPublish` with no network call and `release.yml` uploads the binaries without ever invoking the verb. "No mechanism exists" and "the mechanism never runs" ask for different fixes, and only the second is true.
- 2026-08-05 — bug-hunt loop round 1: iss-184 (critical, guard tokenizer heredoc misparse) fixed at root cause, in two passes after a pre-PR adversarial security review caught the first pass narrowing the hole rather than closing it. `internal/core/guard/tokenize.go`'s `<<` handling already special-cased the literal-digit arithmetic-shift form (`$((1<<20))` is not a heredoc), but `isDelimStart` accepts any identifier-shaped word, so `$((1<<shift))` — an identifier operand — still read as a heredoc delimiter, and `skipHeredocBodies` then silently consumed every remaining line of the command looking for a "shift" terminator line that never comes, dropping later commands (a force-push, an `rm -rf`, anything) from ever reaching command position — a silent allow, not the loud fail-open the hook's own doc comment promises for anything the tokenizer cannot answer confidently. The first pass made `skipHeredocBodies` report whether it actually found each pending heredoc's terminator, turning a miss into `ErrUnparsableCommand`. The security review then constructed the exploit this missed: an attacker supplying a later line that happens to equal the misread "delimiter" (e.g. appending a bare `shift`) still finds a match, so the swallow still succeeds silently — the fix only closed the *unterminated* half of the class, not the *coincidentally-terminated* half, which is the one an attacker actually controls. The real fix is classification, not detection-of-failure: a heredoc delimiter word immediately followed by a bare `(` or `)` with no separator is never a real heredoc — its body and terminator line have to come first, so nothing legitimate places a paren directly against the delimiter word, and `$((expr<<ident))` produces exactly that shape. `<<` in that position is now read as the arithmetic operator at tokenize time, same as the literal-digit case, so the guarded line is reached and matched normally regardless of what any later line contains. The unterminated-body-to-error fix from the first pass is kept as defense in depth for genuinely malformed heredocs (`cat <<EOF` with no closing `EOF` line), a distinct, narrower gap the same swallow also covered — an existing test that pinned that silent-swallow as intended behaviour is updated to assert the corrected `ErrUnparsableCommand` contract. Detectors: `TestArithmeticShiftByIdentifierIsNotAHeredoc`, `TestArithmeticShiftCoincidentalDelimiterStillBlocks` (the adversarial payload the security review demonstrated), and `TestTokenizeRejectsUnterminatedHeredoc` — each watched failing on pre-fix code for the claimed reason and passing after. Three more bugs surfaced by the same round's multi-angle sweep and independently verified with failing tests are captured but not fixed here — iss-185 (critical, scanner leaves a second back-to-back secret token undetected and unredacted, defeating history's fail-closed residual re-scan), iss-186 (minor, a failed source-unlink in a capture ledger transition can permanently strand an issue id across two status directories), iss-187 (minor, `rules.Merge` panics on a nil-Domains base — currently unreachable through any live caller, so a latent exported-API contract defect) — left open for a future round, highest severity first.
- 2026-08-05 — iss-43, CORRECTION to the entry immediately above, from a merge-gate record-accuracy review on PR #196. That entry stands unedited; four of its claims are corrected here rather than in place, because it is merged history. (1) It opens "the three-claim corpus is closed as OVERTAKEN rather than fixed", and the corpus does not close as a unit: claims 1 and 2 — the Phase 0 status claim and the native-capability surface list — close as overtaken via `73428b6`, while claim 3's surviving half, the Layout line, is FIXED by that same PR. The entry knows this later in its own text and in the issue's `resolution:` field, so the opening verdict was a second over-claim of the same shape as the attribution it goes on to retract: a single verdict asserted over a corpus whose members had different fates. (2) "a SHALLOW clone whose truncation window opened well after 2026-07-17" over-claims. The evidence bounds the boundary to the interval after 2026-07-17 and no later than the oldest visible substantive README rewrite; "well after" asserts a distance nothing measured. Read it as "after 2026-07-17". (3) "writing the doc to describe a packaging mechanism that does not exist" is refuted by the entry's own closing paragraph. The clause scopes to the all-channel curated publish path, which genuinely does not exist and which no release has ever taken; the namespace deny itself exists and is tested. (4) The call edge is wrong in the closing paragraph: "reachable from the wired `abcd launch ship`; it is unwired, because `Ship` stops at `WouldPublish`" reads as one chain, implying the verb runs through `launch.Ship`. It does not. `launch.Ship` has NO production caller — the only calls are three in `dryrun_test.go` — while the wired verb exercises the deny through `launch.PrecheckPayload` and `launch.RenderPayload`, which resolve the bundle at `internal/core/launch/render.go:192`, reached from `internal/surface/cli/ship.go`. `release.yml` invokes neither. Both facts stand; the edge joining them did not. The lesson generalises past this entry: a summary sentence written before the detail paragraphs are finished tends to keep the verdict the round started with, and a call chain assembled from two true sentences about neighbouring functions is not evidence that either calls the other — `grep` for the callers.
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
schema_version: 1
id: "iss-185"
slug: "scanner-adjacent-secret-boundary-bypass"
severity: "critical"
category: "bug"
source: "agent-finding"
found_during: "bug-hunt loop round 1"
found_at: "internal/adapter/scanner/scanner.go:274"
---

scanner leaves a second back-to-back secret token undetected and unredacted, and the fail-closed residual re-scan reports clean anyway. Fixed-length token patterns (e.g. github_pat_ at patterns.go:93) require a leading \b; when two same-family tokens are concatenated with no separator, the first regex match consumes bytes up through the second token's first byte, leaving word/word at the junction so the second token's \b can never match. ScanText (scanner.go:274) therefore returns only one finding. Redact's fingerprintSpan keeps 2 raw alnum bytes at the tail of the masked span (scanner.go:393), so the junction stays word/word after redaction too — the second token survives completely raw in the output. Because the boundary never recovers, history.Capture's stage-two residual check (history.go:159, blockingResidual) re-scans the redacted text, finds nothing, and writes the record to disk with a live secret still in it, directly violating history's documented fail-closed guarantee ('a stored record can never contain a live secret'). Distinct from iss-65 (Finding.MarshalJSON snippet cross-leak in serialized findings, already fixed) — this is a ScanText/Redact detection gap in the scan pass itself. Reproducing test: internal/adapter/scanner/, TestConcatenatedSecretsBothDetected.
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
schema_version: 1
id: "iss-186"
slug: "capture-transition-remove-failure-strands-issue"
severity: "minor"
category: "bug"
source: "agent-finding"
found_during: "bug-hunt loop round 1"
found_at: "internal/core/capture/workflow.go:266"
---

capture.commitTransition writes the destination file before removing the source (workflow.go:255-273); a non-ENOENT os.Remove(src) failure (EPERM/EROFS/EIO — e.g. a read-only remount or restrictive ACL on the source status dir) returns an error after dst already landed, with no rollback of dst. The same issue id is now present in two status directories (e.g. both open/ and resolved/). findIssue (alloc.go:383) then rejects any further transition on that id as ErrDuplicateIssueID, so it can never again be resolved or wontfixed on a healthy filesystem — no repair verb exists; recovery requires manually deleting one copy. scanLedger-based reads (List/Status) do not dedupe either, so the id also double-counts. Confirmed with a real non-ENOENT unlink failure (ext4 immutable attribute on the source dir, which blocks unlink even as root, unlike chmod). Reproducing test: internal/core/capture/, TestTransitionRemoveFailureStrandsIssueInTwoDirs.
12 changes: 12 additions & 0 deletions .abcd/work/issues/open/iss-187-rules-merge-nil-domains-panic.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
schema_version: 1
id: "iss-187"
slug: "rules-merge-nil-domains-panic"
severity: "minor"
category: "bug"
source: "agent-finding"
found_during: "bug-hunt loop round 1"
found_at: "internal/core/rules/rules.go:193"
---

rules.Merge panics with 'assignment to entry in nil map' (rules.go:193) when the base RuleSet's Domains field is nil and the overlay has at least one domain key. cloneRuleSet (rules.go:524) only allocates out.Domains when the source map is already non-nil, so a base like RuleSet{SchemaVersion: 1} (a Validate-accepted, valid zero-ish value) crashes on merge, contradicting Merge's own doc comment ('New domain keys are added') which promises no such precondition. The sibling loader guard.Merge (internal/core/guard/config.go:77-79) gets this right, explicitly allocating out.Entries when nil before the same kind of loop — Merge is simply missing that guard. Not reachable today: RuleSet.Load's only call site passes Merge(Defaults(), over), and Defaults() always has non-nil Domains, so this is a latent exported-API contract defect rather than a live crash — it becomes live the moment any caller merges onto a non-Defaults() base (e.g. a future multi-tier overlay starting from an empty RuleSet). Reproducing test (recover-guarded): internal/core/rules/, TestMergeNilBaseDomainsAddsNewKeys.
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
---
schema_version: 1
id: "iss-184"
slug: "guard-heredoc-arithmetic-shift-bypass"
severity: "critical"
category: "bug"
source: "agent-finding"
found_during: "bug-hunt loop round 1"
found_at: "internal/core/guard/tokenize.go:152"
resolution: "Fixed at root cause, in two parts after a pre-PR security review caught the first pass narrowing rather than closing the hole. (1) Classification: a heredoc delimiter word immediately followed by a bare '(' or ')' with no separator is never a real heredoc -- the body and terminator line have to come first, so nothing legitimate places a paren directly against the delimiter word. `$((expr<<ident))` produces exactly that shape, so `<<` in that position is now correctly read as the arithmetic operator at tokenize time, same as the pre-existing literal-digit case, and the dangerous command on a later line reaches command position and blocks normally -- no error, no swallow, no reliance on whether a later line happens to coincidentally match the misread delimiter (the security review's exploit: appending a bare `shift` line let the earlier, error-only fix still swallow the guarded command silently). (2) Defense in depth: skipHeredocBodies still signals when a GENUINE pending heredoc never finds its terminator line, and tokenize turns that into ErrUnparsableCommand (guard fails open LOUDLY per the documented itd-103 contract) rather than silently consuming the rest of the input -- covers truly malformed/unterminated heredocs, a distinct residual gap the same swallow covered. Detectors: internal/core/guard/tokenize_test.go TestArithmeticShiftByIdentifierIsNotAHeredoc, TestArithmeticShiftCoincidentalDelimiterStillBlocks (the adversarial payload), and TestTokenizeRejectsUnterminatedHeredoc -- all watched failing before their respective fix and passing after."
impact: fix
---

guard tokenizer heredoc misparse: an unquoted arithmetic left-shift with an identifier operand (e.g. `$((1<<shift))`) is misparsed as a here-document start by isDelimStart (tokenize.go:206, accepts any word starting with a letter/underscore) plus readHeredocDelim (tokenize.go:249, terminates the delimiter word on ')'). skipHeredocBodies (tokenize.go:264) then scans for a line equal to the bogus delimiter, finds none, and silently swallows every subsequent line of the command before it reaches command position. A dangerous command on a later line (git push --force, rm -rf, gh repo delete, etc.) is never seen by Registry.Check and the guard returns VerdictAllow with no error — a silent fail-open, not the loud could-not-answer path the shim reserves for unparsable input. Confirmed live via both CLI front doors (internal/surface/cli/guard.go: guard check and guard hook). The literal-digit form (1<<20) is unaffected; only identifier/variable operands trip it. Reproducing test: internal/core/guard/tokenize_test.go, TestArithmeticShiftByIdentifierIsNotAHeredoc.
Loading