diff --git a/.abcd/config/reading-presets.json b/.abcd/config/reading-presets.json index 10aa9d299..7047d76da 100644 --- a/.abcd/config/reading-presets.json +++ b/.abcd/config/reading-presets.json @@ -60,10 +60,10 @@ "test" ], "window": { - "tokens_est": 1350000, - "measured_tokens_est": 1332620, - "measured_bytes": 5130589, - "measured_at": "c72b6658f76425e3a5265054190527f5a411c17d" + "tokens_est": 1360000, + "measured_tokens_est": 1344909, + "measured_bytes": 5177902, + "measured_at": "db30f1a10992df69d1253260a88065884a21937d" } }, "entailment": { @@ -133,9 +133,9 @@ ], "window": { "tokens_est": 400000, - "measured_tokens_est": 387345, - "measured_bytes": 1491280, - "measured_at": "c72b6658f76425e3a5265054190527f5a411c17d" + "measured_tokens_est": 387939, + "measured_bytes": 1493566, + "measured_at": "db30f1a10992df69d1253260a88065884a21937d" } }, "comparative": { @@ -216,10 +216,10 @@ "test" ], "window": { - "tokens_est": 1360000, - "measured_tokens_est": 1341656, - "measured_bytes": 5165377, - "measured_at": "c72b6658f76425e3a5265054190527f5a411c17d" + "tokens_est": 1370000, + "measured_tokens_est": 1353945, + "measured_bytes": 5212690, + "measured_at": "db30f1a10992df69d1253260a88065884a21937d" } } } diff --git a/.abcd/development/brief/04-surfaces/01-ahoy.md b/.abcd/development/brief/04-surfaces/01-ahoy.md index 9844259c1..b12de289d 100644 --- a/.abcd/development/brief/04-surfaces/01-ahoy.md +++ b/.abcd/development/brief/04-surfaces/01-ahoy.md @@ -524,8 +524,9 @@ way to apply it (iss-166). `git_identity.committer` for a committer that diverges on its own (required where the repo pins an identity, advisory where it does not), and `git_identity.tool` wherever the author or the committer is a machine identity -(the harness's own default, a `[bot]` account, a vendor's address), pinned or -not, because the human is the author of record either way. The machine +(the harness's own default, a `[bot]` account, a vendor's address, a configured +automation's name such as `semantic-release-bot`), pinned or not, because the +human is the author of record either way. The machine identities are one list, `internal/core/identity/tool-identities.txt`, which the CI attribution gate reads too, role asymmetry included: a `noreply@` mailbox is a machine as the author, and the forge's own committer stamp passes. Once the diff --git a/.abcd/development/brief/04-surfaces/06-capture.md b/.abcd/development/brief/04-surfaces/06-capture.md index 904c41ed6..3e2cbad1f 100644 --- a/.abcd/development/brief/04-surfaces/06-capture.md +++ b/.abcd/development/brief/04-surfaces/06-capture.md @@ -194,7 +194,13 @@ second** (itd-2609020625400194, spc-2609020626040342). No disposition in any state, and no admission, is written for a widening item until a committed comparative run names the item's run; a comparative run committed with an empty item set, the position not exercised, satisfies this as a characterising run -does. The refusal names the run it is waiting on. It is one gate in the one +does. A committed run is the pair the channel's ingest leaves in the run's +directory, its manifest and its run record, agreeing on the run id, the position +and the candidate join; a run record naming the item's run without that +agreement, such as a marker written by hand, is refused by name as a record that +contradicts itself (iss-2609251842111593). Whether git tracks the pair is not +asked, because the gate answers between an ingest and the commit that carries +it. The refusal names the run it is waiting on. It is one gate in the one disposition writer every verb routes through, so the disposition verb, the admission verb and a scribe's ingest all refuse the same way. The other positions are answered with no comparative run anywhere. @@ -231,8 +237,12 @@ any surface. The verb reads the surfaces at `HEAD`, in the working tree and along their history, so the operator supplies no hash. Written after the rewrite's commit it is one write, against the previous distinct state along first parents, so a rewrite a merge brought in is recorded as a squash of the -same branch would record it, whatever the commits' timestamps; written before -it, a first half records the +same branch would record it, whatever the commits' timestamps. A rewrite that +lands as several commits on the first-parent line, a rebased branch among them, +is recorded by that one write as its last step alone, because the previous +distinct state is the one before that step (iss-2609261325441711); the open and +complete pair below records such a rewrite whole. Written before +the rewrite's commit, a first half records the before fingerprints and a second write finishes it once the rewrite is committed, walking back across as many commits as the rewrite took, merges included. Every render names the half it wrote. The occasion @@ -288,7 +298,10 @@ tag that is not the checkout's newest release tag, an empty reason, a record tha is not open, and a record whose grade is neither `major` nor `critical`, which the guard never blocks on. The grade is judged before the tag. A record deferred past an earlier anchor is deferred again: the pair is replaced and a new section -appended, so each cycle's deferral stays readable in the record. +appended, so each cycle's deferral stays readable in the record. A record +deferred again past the SAME anchor has the pair and that cycle's section +rewritten in place, so the body keeps one section per cycle +(iss-2609251823555125). **Marking an issue wontfix** records an explicit non-action decision and moves the issue to `wontfix/`. Grounds are optional here and override the recorded @@ -330,7 +343,8 @@ Every verb also says which checkout's ledger it addressed, and the record dispatcher says it for an issue id (iss-2609202053570475): one stderr line naming the checkout and its branch in the plain render, and a `ledger` member with `checkout` and `branch` in the machine-readable one. The checkout is written home-relative where -it can be. A record filed in another worktree is invisible here, and a refusal +it can be, and by its directory name where it cannot, so neither render carries an +absolute local path (iss-2609251823560369). A record filed in another worktree is invisible here, and a refusal that says "not found" without naming where it looked sends the reader to the wrong conclusion. diff --git a/.abcd/development/brief/04-surfaces/08-abcd.md b/.abcd/development/brief/04-surfaces/08-abcd.md index cab851983..7a144d59d 100644 --- a/.abcd/development/brief/04-surfaces/08-abcd.md +++ b/.abcd/development/brief/04-surfaces/08-abcd.md @@ -37,8 +37,14 @@ Two read-only forms, and no third. **Bare `abcd`** renders a four-field snapshot of the current directory: the directory itself, whether it is a git repo, whether an abcd record is present, -and which of the `.abcd/` work tiers exist. The plugin command invokes its JSON -form. +and which of the `.abcd/` work tiers exist. The directory is named home-relative +(`~/…`), or by its directory name outside HOME, in the text form's first line +and in the JSON form's `dir` alike, never by an absolute path +(iss-2609281613094952): the board is the output most often pasted, and no +consumer acts on `dir`. The text line masks a control character or bidi control +in that name, as every other board line does (iss-2609281736483740); `dir` +carries the name as it is, escaped by the JSON encoder where it is a control +byte. The plugin command invokes its JSON form. **`abcd `** takes a single positional matching `iss-N`, `itd-N`, `spc-N`, `adr-N`, `adm-N`, `srp-N` or `rfm-N` and reports, read-only, what that @@ -142,8 +148,9 @@ and one with no records at the committed layout is named with the reason and not read; when the worktree is gone or git refuses it, its branch is read from the object store instead, so a dead worktree never hides an unmerged commit. The board carries one `peers:` line (JSON `peers`: `live`, `ids`) only when some peer holds a record that differs here; `abcd peers` prints the -whole picture, as text or in its JSON form, with every home path redacted to -`~`. The same reader answers the not-found paths of `abcd `, of +whole picture, as text or in its JSON form, with every worktree named +home-relative, or by its directory name outside HOME, never by an absolute +path (iss-2609281329007423). The same reader answers the not-found paths of `abcd `, of resolving a capture and of the intent audit, consulted only after the local lookup fails. It writes nothing, takes no lock and fetches nothing. diff --git a/.abcd/development/brief/04-surfaces/11-history.md b/.abcd/development/brief/04-surfaces/11-history.md index f8b9fe72a..6de91a776 100644 --- a/.abcd/development/brief/04-surfaces/11-history.md +++ b/.abcd/development/brief/04-surfaces/11-history.md @@ -90,7 +90,8 @@ ahoy's registry stays under `~/.abcd/history/` and holds no transcripts. path. A transcript owned elsewhere is skipped and its owner named by root SHA; one recorded in two repositories is skipped rather than split. A transcript whose repository is not on this machine is an **orphan: ignored, reported, - never guessed**, and adopted only when this repository claims its project name + never guessed** (its recorded directory, like the destination, is shown + home-relative or by its directory name outside HOME, iss-2609281329007423), and adopted only when this repository claims its project name in `adopt_projects` or on the command line; an adopted record carries `adopted_project`. Setting `on_orphan` to `prompt` makes the CLI ask — core never prompts. Ingesting the same material twice adds nothing. diff --git a/.abcd/development/brief/04-surfaces/34-build.md b/.abcd/development/brief/04-surfaces/34-build.md index 0ae1cc703..41cf1d172 100644 --- a/.abcd/development/brief/04-surfaces/34-build.md +++ b/.abcd/development/brief/04-surfaces/34-build.md @@ -42,10 +42,13 @@ No run is created until every check passes, and each is a read (criteria 1 and 2 is settled whole, and an item explicitly marked resolved or deferred — a bold span opening with the word (`**Resolved — …**`, `**Deferred**`, `**explicitly deferred**`, `**explicit deferral**`) or the word as a label (`resolved:`, - `Deferred:`) — is not a question. Every other list item is a question - whatever it says: one led `**Open`, one that only points to another record, - and one that merely mentions deferral all count (the 2026-09-25 entry in - `.abcd/work/DECISIONS.md`). + `Deferred:`) opening a line of the item (a nested sub-bullet included), + after a closing bold (with or without a colon after it) or after a dash — is + not a question; the same word and colon mid-sentence are prose. + Every other list item is a question whatever it says: one led `**Open`, one + that only points to another record, and one that merely mentions deferral + all count (the 2026-09-25 entry in `.abcd/work/DECISIONS.md`, and its + 2026-09-28 correction). - **claim sections** — the mechanism prompt is answered or the section absent, and the scope conditions are recorded. The readiness gate reports both as advisory; a run is where they bind, because an autonomous lane has nobody to @@ -109,7 +112,10 @@ default); the window clock the pacing intent writes (`window_started_at`, the run record, one line per completed step. A lane carries its spec step and title, its next step, what it awaits when a step has handed work to an agent, and the footprint its steps fill in: branch, base and head, worktree, brief, -receipt and pull request. +receipt and pull request. The status render names the worktree home-relative, +or by its directory name outside HOME (iss-2609281329007423); the brief and the +receipt stay whole paths, home-redacted, because the agent reads the one and +writes the other. Starting creates one lane, for the first unlanded spec step, and records the rest as pending. Starting again while that run is in progress creates nothing diff --git a/.abcd/record-lint.json b/.abcd/record-lint.json index f969f4e45..4ea533b36 100644 --- a/.abcd/record-lint.json +++ b/.abcd/record-lint.json @@ -235,7 +235,10 @@ }, "harness_leak": { "enabled": true, - "severity": "blocker" + "severity": "blocker", + "extra_roots": [ + ".abcd/work" + ] }, "directory_coverage": { "enabled": true, diff --git a/.abcd/work/DECISIONS.md b/.abcd/work/DECISIONS.md index 47bd6986e..638c70558 100644 --- a/.abcd/work/DECISIONS.md +++ b/.abcd/work/DECISIONS.md @@ -2579,3 +2579,4 @@ together (the script's header says why there is no escape hatch). - 2026-09-27 — A pipe into a group and a redirect into a shell string reach every command there, and the reading keeps two over-blocks, recorded so they are not mistaken for defects (lane fix3-drainG, autonomous run A, on iss-2609270028388291 from review-drainG2). A pipe into a `{ … }` or `( … )` group is the standard input of every command in it, so each command emitted inside the group reads it, not only those before the group's first separator; a shell in the group reads it as a stream too. A command in a group is read as handed its group's input even when a pipe inside the group hands it another, because the command before that pipe may pass the group's input on (`cat`), and which commands do is not modelled; so `pgrep make | { true; echo 4242 | { xargs kill; }; }` blocks. The runs of the open groups are held as one covering run, which keeps each command's reading constant however deep groups nest, at the cost of also counting a search that sits inside an outer piped group before an inner one opens. A shell passes its standard input to the commands of its string, and a here-string or a process substitution redirected into the shell is that input, as a pipe is; the `<` that redirects a process substitution is not kept by the tokenizer, so a process substitution handed to the shell as an operand is read as its input as well, which is what `sh -c 'xargs kill < "$1"' _ <(pgrep make)` does with it. - 2026-09-28 — A parameter expansion is an unknown word, read for what its value can spell and not for what an earlier command carried into it, and the reading keeps its over-reads, recorded so they are not mistaken for defects (lane drainG3, autonomous run A, on iss-2609251824244354; this supersedes allow (4) of the first 2026-09-25 entry and ruling (c) of the third, which parked the plain-variable half). `$X`, `$1`, `$@`, `$*`, `$-` and a `${…}` read whole to its own `}` leave the unknown word's mark where the value goes, so `--$X` is every flag it could become and `$GIT` in command position is every program its known text allows; `$$`, `$!`, `$?` and `$#` print numbers and stay text, as an arithmetic expansion's output is a number. Allow (1) of the first 2026-09-25 entry extends to a word that is wholly a variable: it is one operand, so `git push origin "$branch"` stays allowed and `git push $X origin main` is not seen. A string handed to a shell is read with each variable-only word written back out (`sh -c "… $X …"` reads `$X` as that shell does), so a bare variable in a string raises no new warn, which keeps the gap `shellRawUninspectable` names: a value holding shell syntax is not read. The 4,315-input false-positive sweep (Makefile recipes, script lines and whole scripts, the repo-mined and adversarial corpora, and 81 everyday variable lines) moved from 4,000 allow / 47 block / 19 warn to 3,991 / 49 / 26 with 249 unparsable lines unchanged, after three readings of a carried value were left out because together they added 37 blocks on ordinary work: a variable handed to a shell or `source` as its script is not a stream (`bash "$script"`), and a variable standing as the program fires no entry that names only its program and an operand (pkill-by-pattern, killall-by-name) and is not a bare interpreter inside a string (`"$GO" build`, `$EDITOR notes.md`). Those, a pid list carried through a variable, and `eval "$X"` (which allows, as it did) are iss-2609281134544802, deferred past v0.11.0 for a ruling. Over-reads kept, each the variable twin of a ruled substitution over-read: a variable program name with a variable first operand can be `git clean` and warns (`exec "$BIN" "$@"`, `"$BASH" "$GATE"`; five lines of the sweep), `git -c core.quotePath=false "$@"` warns under git-clean, `git grep` whose pattern holds a variable and a `(` warns under the fail-safe as its substitution twin does, two printf continuation lines of a script read on their own (`"$n" "$n" "$spec" "$n"`) block as gh-repo-delete, `git -c "$KV" commit` blocks as a commit that may move core.hooksPath, and a stream piped into a shell whose script is a variable (`curl … | bash "$f"`) blocks, because a word wholly an expansion may be no word. The kill-by-search reading gains three feeds in the same lane (iss-2609270036253187): an unquoted here-document's substitutions are the standard input of the command that opened it, a substitution runs with the pipe into its own command as its input, and a shell string is handed the output of the substitutions in its command's words as its positional parameters or text, so every command of such a string is read as handed it (`sh -c 'kill 4242' _ "$(pgrep make)"` blocks), the over-block the 2026-09-27 entry accepts for a string xargs runs. - 2026-09-28 — v0.11.1 is published: autonomous run A approved the `release` environment at 22:44:40Z under ruling A2 and the releases ruling of 2026-09-25T08:04:52Z, after the merge queue, the verify job, the tag job and main's own CI on the tagged commit 2bc519f7 reported green (every check run succeeded apart from those skipped by design; the macOS leg of the push CI was the last to report). The release published at 22:47:00Z with four binaries, the plugin archive, checksums.txt and the site archive; the run verified the darwin-arm64 binary and the plugin archive against checksums.txt, `abcd --version` reports v0.11.1, the plugin archive's SHA-256 equals the digest the catalog pins and its address answers, and the build-provenance attestation verifies as signed by release.yml on main. Unlike v0.11.0, the site rendered and deployed inside the release run, so no redeploy was needed; abcdev.app shows v0.11.1. The version is v0.11.1 rather than the v0.12.0 the run had expected, because the cut derives the version from the records and nothing since v0.11.0 is breaking. +- 2026-09-28 — Correction to the 2026-09-25 entry on the build's open-question check (lane implementer, autonomous run A, lane drainInt, on iss-2609260932374727). A settled LABEL (`resolved:`, `RESOLVED:`, `Deferred:`) is no longer read anywhere in the item: it counts opening a line of the item (its first line or a continuation line), after a closing bold (`**Which surface scaffolds it?** RESOLVED:`), or after a dash (`**Refusal breadth** — resolved:`, `**Relationship to itd-73** (derived versioning) — RESOLVED:`). The same word and colon mid-sentence are prose, so "Which id wins once the split is resolved: the old or the new?" is a question, where the entry's "anywhere in the item" read it as settled and let build start past it. The bold-span marker keeps its reach anywhere in the item. Every intent in the tree reads the same open-question count under the tightened rule as under the old one, so no record changes verdict. diff --git a/.abcd/work/issues/open/iss-2608270655499478-record-lint-vs-capture-parser-divergence-remainder-block-seq.md b/.abcd/work/issues/open/iss-2608270655499478-record-lint-vs-capture-parser-divergence-remainder-block-seq.md deleted file mode 100644 index 72fb58b87..000000000 --- a/.abcd/work/issues/open/iss-2608270655499478-record-lint-vs-capture-parser-divergence-remainder-block-seq.md +++ /dev/null @@ -1,12 +0,0 @@ ---- -schema_version: 1 -id: "iss-2608270655499478" -slug: "record-lint-vs-capture-parser-divergence-remainder-block-seq" -severity: "minor" -category: "tech-debt" -source: "agent-finding" -found_during: "security-cut-agent-flagged-siblings-2026-08-27" -found_at: "internal/core/frontmatter" ---- - -record-lint vs capture parser divergence remainder: block-sequence frontmatter fields are legitimate and used in 21+ intent/adr records but only capture's strict ledger parser rejects them, so a correct fix is store-scoped (share capture's typed strict parser through the canonical frontmatter package) rather than a universal rejection. The duplicate-key and space-before-colon halves of #357 are fixed; this block-sequence remainder is the follow-up. Flagged by the lint-integrity fix agent. \ No newline at end of file diff --git a/.abcd/work/issues/open/iss-2609221820487644-the-attribution-gate-should-let-a-dependabot-dependency-bump-through-without-a-human-re-authoring-it.md b/.abcd/work/issues/open/iss-2609221820487644-the-attribution-gate-should-let-a-dependabot-dependency-bump-through-without-a-human-re-authoring-it.md index 02d7f3b67..e8a089036 100644 --- a/.abcd/work/issues/open/iss-2609221820487644-the-attribution-gate-should-let-a-dependabot-dependency-bump-through-without-a-human-re-authoring-it.md +++ b/.abcd/work/issues/open/iss-2609221820487644-the-attribution-gate-should-let-a-dependabot-dependency-bump-through-without-a-human-re-authoring-it.md @@ -9,6 +9,8 @@ found_during: "PR 655 blocked on 2026-09-22; the bump landed by hand as PR 659" origin: researcher-authored production_mode: hand-written found_at: "scripts/check-attribution.sh (check_ident); AGENTS.md, Attribution and acknowledgements" +deferred_after: "v0.11.0" +deferral_reason: "ruling owed to the product thinker (deferred by lane drainScr of autonomous run A, 2026-09-28): the standing ruling in AGENTS.md is that a dependabot pull request is not mergeable as authored and a dependency bump is landed by a human, and the 2026-09-22 ruling below keeps the gate unchanged and routes the bump through a re-authoring workflow (itd-2609221842494980). That workflow is blocked on the rulings-owed list, section L: a push made with the built-in CI token starts no checks, so the re-authored commit would never be checked, and the token choice (a personal access token or an app token kept as a secret) and whether automated dependency updates are switched on at all are the product thinker's. Nothing in scripts/check-attribution.sh changes for this record." --- The attribution gate should let a dependabot dependency bump through without a human re-authoring it. The product thinker asked for this on 2026-09-22 after PR 655 sat blocked with auto-merge armed and every other check green. Two things must be said precisely, because the request names the trailer and the trailer is not what fails. The gate refuses a bot on IDENTITY: check_ident reads dependabot[bot] as a machine in the author role on two independent signals, the forge's [bot] name suffix and the bot mailbox, and it deliberately stays silent about the missing Assisted-by trailer so the remedy is not misread, its own comment saying that adding a trailer is not the fix for a dependency bump and landing it as a human is. No review clears an identity refusal, which is why an armed auto-merge looks like it is waiting for a reviewer when it is waiting for something no reviewer can give. The rule is deliberate and stated in AGENTS.md: the contributor graph is built from the author and committer fields, a machine there asserts an authorship it does not hold, and a squash merge re-appends a mis-identified branch author as a co-author, so the consequence, that a dependabot pull request is not mergeable as authored, is written down as intended. Wanted: a way for a dependency bump to land without a person re-authoring it every time, without letting a machine into the contributor graph on any other change. Candidate shapes for the decision to weigh: the gate exempts a commit whose diff touches only go.mod and go.sum on a branch the forge marks as dependabot's, with the exemption named in the refusal it would otherwise raise; or the repository takes dependabot's bumps through a workflow that re-authors them as the human owner before the gate runs, so nothing about the gate changes; or dependabot is turned off for this repository and bumps are made by hand on a schedule. This reverses a stated rule, so it is an ADR plus a brief invariant before any code moves, not a quiet edit to the script. diff --git a/.abcd/work/issues/open/iss-263-dispatch-unparseable-issue-reads-as-not-found.md b/.abcd/work/issues/open/iss-263-dispatch-unparseable-issue-reads-as-not-found.md deleted file mode 100644 index 4eb7f5096..000000000 --- a/.abcd/work/issues/open/iss-263-dispatch-unparseable-issue-reads-as-not-found.md +++ /dev/null @@ -1,12 +0,0 @@ ---- -schema_version: 1 -id: "iss-263" -slug: "dispatch-unparseable-issue-reads-as-not-found" -severity: "nitpick" -category: "ux" -source: "impl-review" -found_during: "spc-26 build, ruthless-reviewer note" -found_at: "internal/core/record/record.go" ---- - -describeIssue discards ListResult.Skipped, so an issue file that exists but is unparseable (broken frontmatter) makes abcd iss-N report 'not found in the issue ledger' — a diagnostic that misleads about a record physically present in a status dir. Surface the skip roster in the fault: 'iss-N present but unreadable at : '. \ No newline at end of file diff --git a/.abcd/work/issues/open/iss-2608221126066379-frontmatter-bom-tolerance-sibling-parsers-diverge.md b/.abcd/work/issues/resolved/iss-2608221126066379-frontmatter-bom-tolerance-sibling-parsers-diverge.md similarity index 55% rename from .abcd/work/issues/open/iss-2608221126066379-frontmatter-bom-tolerance-sibling-parsers-diverge.md rename to .abcd/work/issues/resolved/iss-2608221126066379-frontmatter-bom-tolerance-sibling-parsers-diverge.md index c69f66b7d..5d4aa3d78 100644 --- a/.abcd/work/issues/open/iss-2608221126066379-frontmatter-bom-tolerance-sibling-parsers-diverge.md +++ b/.abcd/work/issues/resolved/iss-2608221126066379-frontmatter-bom-tolerance-sibling-parsers-diverge.md @@ -7,6 +7,14 @@ category: "tech-debt" source: "agent-finding" found_during: "bughunt round 7 merge-gate dual review" found_at: "internal/core/intent/intent.go" +resolution: "frontmatter.Close is the reader's own block walk (BOM trimmed at line 0 only, delimiters by IsDelimiter); intent's three writers, the changelog body, the record page body and peers' title route through it. record-lint's title and agent scope ask frontmatterOpen, and the disposition parser, the launch prose gate and the site stripper trim the BOM at line 0. Each sibling has a BOM-led test watched fail first." +impact: fix +resolved_by: + commit: "7d8d1b71ca70d93f676bc8aac94aabecf2b3dcf3" --- -frontmatter.Fields trims a leading UTF-8 BOM (iss-2608220134344680) but the sibling parsers that promise byte-exact parity with it do not: intent's setFrontmatterFields (internal/core/intent/intent.go:129, whose comment claims it matches frontmatter.Fields's delimiter tolerance exactly) refuses a BOM-led record the reader accepts, so intent mutations on such a record fail-closed with 'no leading frontmatter block'; changelog's bodyStart (internal/core/changelog/source.go:60, 'so the two never disagree') returns 0 and leaks the whole frontmatter block into the derived changelog body; lint's recordTitle (internal/core/lint/schema.go:554) hand-rolls the comment skip with neither TrimBOM nor the multi-line-comment state. Sweep the siblings onto the shared tolerance or narrow the parity comments to the truth. \ No newline at end of file +frontmatter.Fields trims a leading UTF-8 BOM (iss-2608220134344680) but the sibling parsers that promise byte-exact parity with it do not: intent's setFrontmatterFields (internal/core/intent/intent.go:129, whose comment claims it matches frontmatter.Fields's delimiter tolerance exactly) refuses a BOM-led record the reader accepts, so intent mutations on such a record fail-closed with 'no leading frontmatter block'; changelog's bodyStart (internal/core/changelog/source.go:60, 'so the two never disagree') returns 0 and leaks the whole frontmatter block into the derived changelog body; lint's recordTitle (internal/core/lint/schema.go:554) hand-rolls the comment skip with neither TrimBOM nor the multi-line-comment state. Sweep the siblings onto the shared tolerance or narrow the parity comments to the truth. + +## Grounds + +- pursued: we expect one exported walk plus line-0 BOM trims to make every frontmatter reader agree with frontmatter.Fields on a BOM-led record; it is shown wrong if any reader of a record's bytes still refuses, mis-titles or leaks the block of a BOM-led record Fields reads diff --git a/.abcd/work/issues/resolved/iss-2608270655499478-record-lint-vs-capture-parser-divergence-remainder-block-seq.md b/.abcd/work/issues/resolved/iss-2608270655499478-record-lint-vs-capture-parser-divergence-remainder-block-seq.md new file mode 100644 index 000000000..a0e85c16b --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2608270655499478-record-lint-vs-capture-parser-divergence-remainder-block-seq.md @@ -0,0 +1,20 @@ +--- +schema_version: 1 +id: "iss-2608270655499478" +slug: "record-lint-vs-capture-parser-divergence-remainder-block-seq" +severity: "minor" +category: "tech-debt" +source: "agent-finding" +found_during: "security-cut-agent-flagged-siblings-2026-08-27" +found_at: "internal/core/frontmatter" +resolution: "Overtaken by 35600e968: record_schema's issue-store reader-parity leg asks capture's strict ledger reader itself (capture.ReadRefusal, registered by the front doors), so a block-sequence field in an issue record is refused by the gate exactly as the reader refuses it, while the intent and ADR stores, whose readers take block sequences, are untouched: the store-scoped fix the record asks for. Pinned by TestRecordSchemaRefusesAnIssueBlockSequenceAsTheReaderDoes in 8f0cf5f42." +impact: internal +resolved_by: + commit: "35600e968" +--- + +record-lint vs capture parser divergence remainder: block-sequence frontmatter fields are legitimate and used in 21+ intent/adr records but only capture's strict ledger parser rejects them, so a correct fix is store-scoped (share capture's typed strict parser through the canonical frontmatter package) rather than a universal rejection. The duplicate-key and space-before-colon halves of #357 are fixed; this block-sequence remainder is the follow-up. Flagged by the lint-integrity fix agent. + +## Grounds + +- pursued: record-lint refuses an issue record with a block-sequence field because capture's reader does, and no other store; an issue record carrying a block sequence that lints green, or an intent or ADR block sequence the gate starts refusing, would show it wrong diff --git a/.abcd/work/issues/open/iss-2608270908348042-five-delimiter-compare-variants-remain-beside-the-canonical.md b/.abcd/work/issues/resolved/iss-2608270908348042-five-delimiter-compare-variants-remain-beside-the-canonical.md similarity index 50% rename from .abcd/work/issues/open/iss-2608270908348042-five-delimiter-compare-variants-remain-beside-the-canonical.md rename to .abcd/work/issues/resolved/iss-2608270908348042-five-delimiter-compare-variants-remain-beside-the-canonical.md index 0f61b9176..434783ab7 100644 --- a/.abcd/work/issues/open/iss-2608270908348042-five-delimiter-compare-variants-remain-beside-the-canonical.md +++ b/.abcd/work/issues/resolved/iss-2608270908348042-five-delimiter-compare-variants-remain-beside-the-canonical.md @@ -7,7 +7,15 @@ category: "tech-debt" source: "agent-finding" found_during: "issue-sweep-2026-08-27" found_at: "internal/core/frontmatter/frontmatter.go" +resolution: "The private delimiter walks route through frontmatter.Close, the new CloseAfter or IsDelimiter; the deliberate differences (memory's indented opener, the transcript store's own byte-exact format, the reading exclusion floor) are commented and allowlisted, and TestNoPrivateDelimiterCompare fails on a new private three-dash literal outside that allowlist. The mid-file ZWNBSP close in the re-verification note is refused by record-lint through the issue store's reader-parity leg, pinned by a case in TestRecordSchemaAgreesWithTheLedgerReader." +impact: internal +resolved_by: + commit: "2b6e0bf8e" --- five delimiter-compare variants remain beside the canonical frontmatter.IsDelimiter: gate-side TrimSpace compares in lint and glossary accept an indented delimiter the canonical rule refuses, intent and changelog carry tolerant local copies, memory keeps its own close predicate, and site tests a bare HasPrefix — one consolidation pass onto the canonical predicate closes the family Re-verification note: a record whose block is closed only by a mid-file ZWNBSP delimiter is capture-refused but frontmatter.Fields-green, and no lint rule runs the strict ledger parser — record-lint passes what capture refuses until the consolidation lands. + +## Grounds + +- pursued: every reader of a record's frontmatter judges its delimiters by one rule, so a gate and its reader read the same block; a committed record whose block closes differently to a gate than to Fields, or a new private three-dash compare that the detector does not name, would show it wrong diff --git a/.abcd/work/issues/open/iss-2608301306580014-the-privacy-backstop-has-two-blind-spots-over-the-issue-ledg.md b/.abcd/work/issues/resolved/iss-2608301306580014-the-privacy-backstop-has-two-blind-spots-over-the-issue-ledg.md similarity index 78% rename from .abcd/work/issues/open/iss-2608301306580014-the-privacy-backstop-has-two-blind-spots-over-the-issue-ledg.md rename to .abcd/work/issues/resolved/iss-2608301306580014-the-privacy-backstop-has-two-blind-spots-over-the-issue-ledg.md index d309effa0..6ca2a7104 100644 --- a/.abcd/work/issues/open/iss-2608301306580014-the-privacy-backstop-has-two-blind-spots-over-the-issue-ledg.md +++ b/.abcd/work/issues/resolved/iss-2608301306580014-the-privacy-backstop-has-two-blind-spots-over-the-issue-ledg.md @@ -7,6 +7,10 @@ category: "security" source: "user-observation" found_during: "itd-179-round-3-security" found_at: "internal/core/lint" +resolution: "Both blind spots closed: privacy-hygiene's Windows arm reads a separator as a backslash run at any escaping depth (the escaped C:\\\\Users\\\\ the serialiser writes), and record-lint's harness_leak reads .abcd/work through its own extra_roots. The folded DEL/C1/bidi half was closed at the record-write boundary by iss-2608301206073609; the write-time redactor's Windows arm is iss-2609251639261103's own record." +impact: fix +resolved_by: + commit: "922a2a6a3" --- the privacy backstop has two blind spots over the issue ledger: the escaped Windows home path yamlScalar writes and a harness_leak root that excludes the ledger @@ -46,3 +50,7 @@ Trojan-Source-shaped display concern on committed prose. Pre-existing serialiser contract, unchanged by this branch. Refusal messages use %q, which Go escapes, so an ANSI escape in grounds can never reach a terminal raw through a refusal. + +## Grounds + +- pursued: a committed ledger record carrying an escaped Windows home or a session URL now fails abcd lint or record-lint; either shape passing both gates in a ledger file would show it wrong diff --git a/.abcd/work/issues/open/iss-2608301744300631-an-empty-collection-in-superseded-by-is-an-absence-to-the-ga.md b/.abcd/work/issues/resolved/iss-2608301744300631-an-empty-collection-in-superseded-by-is-an-absence-to-the-ga.md similarity index 74% rename from .abcd/work/issues/open/iss-2608301744300631-an-empty-collection-in-superseded-by-is-an-absence-to-the-ga.md rename to .abcd/work/issues/resolved/iss-2608301744300631-an-empty-collection-in-superseded-by-is-an-absence-to-the-ga.md index de1623f44..5ebfd2f78 100644 --- a/.abcd/work/issues/open/iss-2608301744300631-an-empty-collection-in-superseded-by-is-an-absence-to-the-ga.md +++ b/.abcd/work/issues/resolved/iss-2608301744300631-an-empty-collection-in-superseded-by-is-an-absence-to-the-ga.md @@ -7,6 +7,10 @@ category: "bug" source: "impl-review" found_during: "itd-189-round-5-build" found_at: "internal/core/record/record.go (describeADR)" +resolution: "Describe asks frontmatter.IsEmptyValue, the emptiness question record-lint's supersession gate asks through isAbsentValue, for the ADR and the intent page alike, so an empty collection or an empty node in superseded_by is no successor to both readers." +impact: fix +resolved_by: + commit: "63c8a613e" --- an empty collection in superseded_by is an absence to the gate and a rendered link to record Describe so the two readers disagree about whether the record names a successor @@ -33,3 +37,7 @@ disagreement is between the two predicates and not between the two spellings. Remedy: give the dispatcher the same emptiness question the gate asks, so one value gets one answer — not a second special case in `isAbsentValue`, which would leave the dispatcher rendering `[]` as a link. + +## Grounds + +- pursued: the dispatcher and the gate agree on whether a record names a successor; an ADR or intent whose superseded_by the gate reads as absent but abcd renders as a link, as TestDescribeReadsAnEmptySupersededByAsNoSuccessor asserts for [], {}, !!null, [ ] and ~, would show it wrong diff --git a/.abcd/work/issues/open/iss-2609012047551175-ids-entering-closed-in-scripts-check-issue-resolution-sh-acc.md b/.abcd/work/issues/resolved/iss-2609012047551175-ids-entering-closed-in-scripts-check-issue-resolution-sh-acc.md similarity index 53% rename from .abcd/work/issues/open/iss-2609012047551175-ids-entering-closed-in-scripts-check-issue-resolution-sh-acc.md rename to .abcd/work/issues/resolved/iss-2609012047551175-ids-entering-closed-in-scripts-check-issue-resolution-sh-acc.md index e25062065..b10d76122 100644 --- a/.abcd/work/issues/open/iss-2609012047551175-ids-entering-closed-in-scripts-check-issue-resolution-sh-acc.md +++ b/.abcd/work/issues/resolved/iss-2609012047551175-ids-entering-closed-in-scripts-check-issue-resolution-sh-acc.md @@ -9,6 +9,14 @@ found_during: "autonomous-run-2026-09-01" origin: researcher-authored production_mode: hand-written found_at: "scripts/check-issue-resolution.sh" +resolution: "ids_entering_closed now counts an id as entering resolved/ or wontfix/ only when the merge base did not hold it in the very folder it lands in (the merge base's listing, so a move rewritten past rename detection is caught too, and a branch's own wontfix/ -> resolved/ move still enters: a24c792e8); ids_entering_shipped filters on the base's shipped/ listing. Proved by four cases in scripts/check-issue-resolution-cases.sh (base-side resolved->wontfix move, base-side reslug, the move rewritten past rename detection, the shipped/ reslug twin), each passing the old gate and refused by the new one, while a branch's own wontfix/ -> resolved/ move passes." +impact: internal +resolved_by: + commit: "46ae88404" --- ids_entering_closed in scripts/check-issue-resolution.sh accepts a rename whose SOURCE is already a terminal folder: it keys on the destination alone, so a record that moves resolved/ to wontfix/, or is reslugged within resolved/, counts as ENTERING a terminal folder. Two topologies let a stale trailer pass silently: main has since moved the record from resolved/ to wontfix/ (the branch's Resolves trailer is satisfied by a move it did not make), and main has reslugged the record inside resolved/ (the rename's destination is terminal, the id is extracted from the new basename, and the trailer is satisfied by a rename). Pre-existing, untouched by the hygiene branch; found by the ruthless review of it. The honest test is that the rename's source is NOT a terminal folder, or that the record was open at the base. + +## Grounds + +- pursued: a stale Resolves: or Delivers: trailer is no longer satisfied by a base-side move between or within terminal folders; a same-change capture-and-resolve or an honest open->resolved move still passes, and a clean case in the suite failing would show it wrong diff --git a/.abcd/work/issues/open/iss-2609012047566360-the-placer-probe-in-rs001-s-stale-branch-diagnosis-scripts-c.md b/.abcd/work/issues/resolved/iss-2609012047566360-the-placer-probe-in-rs001-s-stale-branch-diagnosis-scripts-c.md similarity index 53% rename from .abcd/work/issues/open/iss-2609012047566360-the-placer-probe-in-rs001-s-stale-branch-diagnosis-scripts-c.md rename to .abcd/work/issues/resolved/iss-2609012047566360-the-placer-probe-in-rs001-s-stale-branch-diagnosis-scripts-c.md index 310d2de6c..ea7a972e3 100644 --- a/.abcd/work/issues/open/iss-2609012047566360-the-placer-probe-in-rs001-s-stale-branch-diagnosis-scripts-c.md +++ b/.abcd/work/issues/resolved/iss-2609012047566360-the-placer-probe-in-rs001-s-stale-branch-diagnosis-scripts-c.md @@ -9,6 +9,14 @@ found_during: "autonomous-run-2026-09-01" origin: researcher-authored production_mode: hand-written found_at: "scripts/check-issue-resolution.sh" +resolution: "RS001's stale-branch split (and RS005's twin for shipped/) now asks the merge base's tree whether the record was already terminal there, and names as placer the base-side commit that added or renamed the path into place (--diff-filter=AR), never a later edit; fail() deletes control bytes from every refusal, so a placer subject carrying an escape sequence no longer reaches the terminal. Proved by cases in scripts/check-issue-resolution-cases.sh: a base-side body edit of a record terminal at the merge base (and the shipped/ twin) prescribes no rebase, the placer named is the move and not a later edit, and an escape sequence in the placer subject is named without its control bytes; each failed against the previous gate." +impact: internal +resolved_by: + commit: "7a96f7b0f" --- The placer probe in RS001's stale-branch diagnosis (scripts/check-issue-resolution.sh) names the last base-side commit that touched the record's terminal path after divergence, as evidence that the base placed the record there after the branch forked. Any touch qualifies, so a body edit of a record that was already terminal at the merge base reports 'placed there on main's side by … rebase' when the honest verdict is 'terminal before this branch diverged; drop the trailer'. The probe should key on the commit that ADDED or renamed the path into the terminal folder (--diff-filter=AR) or compare the merge base's tree, not on any touch. Separately, the base-side commit subject is interpolated into the stderr message unsanitised; a subject carrying terminal control sequences would reach the terminal through the gate's output. Found by the ruthless review of the hygiene branch; left open as a follow-up. + +## Grounds + +- pursued: a trailer naming a record terminal before the branch diverged is told to drop the trailer and a truly stale branch is still told to rebase naming the placing commit; the existing stale-branch and terminal-before-divergence cases going red would show it wrong diff --git a/.abcd/work/issues/open/iss-2609021815563506-intent-setpromotedfrom-returns-a-populated-intent-beside-a-n.md b/.abcd/work/issues/resolved/iss-2609021815563506-intent-setpromotedfrom-returns-a-populated-intent-beside-a-n.md similarity index 53% rename from .abcd/work/issues/open/iss-2609021815563506-intent-setpromotedfrom-returns-a-populated-intent-beside-a-n.md rename to .abcd/work/issues/resolved/iss-2609021815563506-intent-setpromotedfrom-returns-a-populated-intent-beside-a-n.md index 246e5748b..f161147f8 100644 --- a/.abcd/work/issues/open/iss-2609021815563506-intent-setpromotedfrom-returns-a-populated-intent-beside-a-n.md +++ b/.abcd/work/issues/resolved/iss-2609021815563506-intent-setpromotedfrom-returns-a-populated-intent-beside-a-n.md @@ -9,6 +9,14 @@ found_during: "itd-2609020625400169 fidelity audit" origin: researcher-authored production_mode: hand-written found_at: "internal/core/intent/lifecycle.go" +resolution: "SetPromotedFrom and ErrBackEdgeTaken were replaced by AddRelatedIssue (48c61088c), which never returns a populated intent beside an error. Its return contract is now asserted at the primitive: the record's list on the append and the idempotent no-op paths, and the zero Intent beside every refusal." +impact: internal +resolved_by: + commit: "c530b76645ebe2ed07bfe89c0cce02528e73c3f7" --- intent.SetPromotedFrom returns a populated Intent beside a non-nil ErrBackEdgeTaken, deliberately and documented, and the promote route depends on that value to report the kept back-edge, but the primitive's own test discards the return, so the stated contract is asserted only indirectly through the capture result's BackEdgeKept field. The fidelity verdict for itd-2609020625400169 records this as its one missing item; a primitive-level assertion closes it. + +## Grounds + +- pursued: we expect primitive-level assertions on AddRelatedIssue's return to catch a regression the promote route's BackEdgeKept would otherwise hide; it is shown wrong if a mutation returning a populated intent beside an error, or an empty list on the no-op, passes the intent tests diff --git a/.abcd/work/issues/open/iss-2609090951276167-attribution-machine-signal-is-bot-suffix-shaped-only.md b/.abcd/work/issues/resolved/iss-2609090951276167-attribution-machine-signal-is-bot-suffix-shaped-only.md similarity index 64% rename from .abcd/work/issues/open/iss-2609090951276167-attribution-machine-signal-is-bot-suffix-shaped-only.md rename to .abcd/work/issues/resolved/iss-2609090951276167-attribution-machine-signal-is-bot-suffix-shaped-only.md index 5425fb5e6..69faabd1f 100644 --- a/.abcd/work/issues/open/iss-2609090951276167-attribution-machine-signal-is-bot-suffix-shaped-only.md +++ b/.abcd/work/issues/resolved/iss-2609090951276167-attribution-machine-signal-is-bot-suffix-shaped-only.md @@ -9,6 +9,14 @@ found_during: "adversarial-review" origin: researcher-authored production_mode: hand-written found_at: "scripts/check-attribution.sh" +resolution: "The attribution gate also refuses, in both roles, a trailing bot, robot or automation word ending the display name or the mailbox's local part, standing alone or joined by - or _ (and + in the local part), so a configured automation such as semantic-release-bot at a forge no-reply address is a machine; whitespace does not separate the word in a display name, so a person named Jan Bot passes (cec6047b5), and the comment names the shapes still out of reach. Proved by cases in scripts/check-attribution-cases.sh: semantic-release-bot at a forge no-reply address and by name alone, ci_bot, Renovate Bot at bot@renovateapp.com, release_automation@ and a ci-robot committer are refused, while Ada Talbot at a forge privacy address, jean.bot@ and Jan Bot pass." +impact: internal +resolved_by: + commit: "bd3f55e68" --- The attribution gate states its rule as refuse machines and allow humans, and implements the machine half structurally rather than nominally: a name ending in the forge-stamped bot suffix, a mailbox carrying that suffix, or one vendor domain. Its comment claims a second automation lands in the right place with no edit here, which holds only for automations the forge itself stamps. An identity such as semantic-release-bot with a forge no-reply address matches neither the AI name list nor the AI mail list, neither machine pattern, and not the author-only no-reply rule, so it is judged a human and the gate exits clean; verified by reading the four patterns against that identity. Self-hosted release automation, CI bots committing under a plain configured name, and any forge whose suffix is not the one hard-coded here all land the same way, and this gate is the only thing standing between them and the contributor graph the rule exists to protect. It matters because the rule was written after a bot walked past the nominal list, and the structural replacement inherits the same enumeration in a different alphabet. Fix direction: widen the structural signal beyond one forge suffix, whether by treating a trailing bot or automation token in the name or local part as machine-shaped, by keeping an automation mailbox list beside it, or by requiring a positive human signal, and say in the comment which shapes remain out of reach. Detector: a commit authored as semantic-release-bot with a forge no-reply address must be refused as a machine, while an outside human contributor with a forge privacy address still passes. + +## Grounds + +- pursued: a self-configured automation identity is refused while outside humans at forge privacy addresses still pass; a full-history run refusing a human commit the previous gate passed would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609100509537730-a-debt-nothing-lists-owed-fidelity-reviews.md b/.abcd/work/issues/resolved/iss-2609100509537730-a-debt-nothing-lists-owed-fidelity-reviews.md index 22a2fd40e..76ea3ad32 100644 --- a/.abcd/work/issues/resolved/iss-2609100509537730-a-debt-nothing-lists-owed-fidelity-reviews.md +++ b/.abcd/work/issues/resolved/iss-2609100509537730-a-debt-nothing-lists-owed-fidelity-reviews.md @@ -30,10 +30,6 @@ This is a designed obligation that silently accumulates, which is a sharper fail Wanted: a row in the status render or in `abcd lint` that names the number of owed fidelity reviews and the intents they belong to, and a listing verb that enumerates them. Related and filed separately: two defects that make an owed review expensive to discharge once found — the request carries no provenance hashes that `ingest` nevertheless requires, and it asks the host for a delivered diff range it has no mechanism to supply. -## Grounds - -- pursued: we expect a count of owed fidelity reviews on the bare status surfaces to be enough to make the debt get paid, because the debt was invisible rather than resisted, and a session that was asked what was outstanding discharged three in one sitting; it is shown wrong if the count is rendered and the debt still accumulates, which would mean visibility was not the constraint - **Corroboration (2026-09-18, Gropius managed-repo session gropiusllm-56, relayed to abcd-17).** Second managed repository, at v0.9.0, after twelve fidelity audits in one day: `abcd spec close` prints "fidelity review OWED, receipt @@ -43,4 +39,8 @@ ask is a read-only listing, `abcd intent audit --owed`, so a session can find what is outstanding without enumerating shipped intents by hand. Same shape as the filing; the number this time was twelve, all paid, found by grep. +## Grounds + +- pursued: we expect a count of owed fidelity reviews on the bare status surfaces to be enough to make the debt get paid, because the debt was invisible rather than resisted, and a session that was asked what was outstanding discharged three in one sitting; it is shown wrong if the count is rendered and the debt still accumulates, which would mean visibility was not the constraint + - pursued: we expect a count of owed fidelity reviews on the bare status surfaces to be enough to make the debt get paid, because the debt was invisible rather than resisted; it is shown wrong if the count is rendered and the debt still accumulates, which would mean visibility was not the constraint diff --git a/.abcd/work/issues/open/iss-2609240646522330-remainder-close-refusal-omits-intent-plan-impact.md b/.abcd/work/issues/resolved/iss-2609240646522330-remainder-close-refusal-omits-intent-plan-impact.md similarity index 63% rename from .abcd/work/issues/open/iss-2609240646522330-remainder-close-refusal-omits-intent-plan-impact.md rename to .abcd/work/issues/resolved/iss-2609240646522330-remainder-close-refusal-omits-intent-plan-impact.md index 93411d25a..17598bffb 100644 --- a/.abcd/work/issues/open/iss-2609240646522330-remainder-close-refusal-omits-intent-plan-impact.md +++ b/.abcd/work/issues/resolved/iss-2609240646522330-remainder-close-refusal-omits-intent-plan-impact.md @@ -9,6 +9,14 @@ found_during: "autonomous run 2026-09-23" origin: researcher-authored production_mode: hand-written found_at: "internal/core/intent/lifecycle.go" +resolution: "Both early-close impact refusals (another spec still open; a remainder minted) name abcd intent plan --impact as the way to record the judgement now, after which the close that ships needs no flag." +impact: fix +resolved_by: + commit: "fe9c705c2d9f239b5d8b5afee46877072e949fd0" --- `abcd spec close --remainder --impact ` is refused by design, because a remainder close ships nothing, and the refusal says to supply the impact at the close that ships. It does not say that the judgement can be kept now: `abcd intent plan --impact ` stamps the impact on an intent that is already planned, and the later close then needs no flag. In autonomous run A a lane that knew the impact at a remainder close dropped it, which left it to be remembered by whichever session makes the final close. Wanted: both remainder refusals in internal/core/intent/lifecycle.go name `abcd intent plan --impact ` as the way to record the judgement at once. + +## Grounds + +- pursued: we expect naming the plan route in the refusal to keep an impact a lane already knows from being dropped at a remainder close; it is shown wrong if a lane refused there still re-runs without recording the impact and the judgement is lost to the final close diff --git a/.abcd/work/issues/open/iss-2609240646533487-revert-cannot-withdraw-a-delivers-trailer-from-rs005.md b/.abcd/work/issues/resolved/iss-2609240646533487-revert-cannot-withdraw-a-delivers-trailer-from-rs005.md similarity index 51% rename from .abcd/work/issues/open/iss-2609240646533487-revert-cannot-withdraw-a-delivers-trailer-from-rs005.md rename to .abcd/work/issues/resolved/iss-2609240646533487-revert-cannot-withdraw-a-delivers-trailer-from-rs005.md index 2003d1554..72de23bed 100644 --- a/.abcd/work/issues/open/iss-2609240646533487-revert-cannot-withdraw-a-delivers-trailer-from-rs005.md +++ b/.abcd/work/issues/resolved/iss-2609240646533487-revert-cannot-withdraw-a-delivers-trailer-from-rs005.md @@ -9,6 +9,14 @@ found_during: "autonomous run 2026-09-23" origin: researcher-authored production_mode: hand-written found_at: "scripts/check-issue-resolution.sh" +resolution: "A commit that a later commit of the same range reverts, in git revert's own words, has its Resolves: and Delivers: declarations withdrawn (RS001 and RS005 alike), but only for the ids whose record the revert's own diff takes back out of resolved/, wontfix/ or shipped/ (d68712713), so a hand-written reverts line over a commit that moves no record withdraws nothing; a revert of that revert reinstates them, and a reverts line naming a commit outside the range, or not an ancestor of the revert, withdraws nothing. Proved by cases in scripts/check-issue-resolution-cases.sh: a reverted delivery and a reverted resolution pass (both refused by the previous gate), a revert of the revert is refused again, a revert naming a commit outside the range withdraws nothing, and a hand-written reverts line over a commit that reverts nothing (RS001 and RS005) or that takes a different record out is refused." +impact: internal +resolved_by: + commit: "aa9a1c241" --- RS005 judges every `Delivers: itd-N` trailer in the range, so once a commit carrying the trailer is on a branch, a later `git revert` of that commit cannot withdraw the declaration: the revert takes the intent back out of `shipped/`, the trailer stays in the range, and `lint-issues` refuses the branch. In autonomous run A the load-check lane (itd-2609231434459890) shipped its intent, review found that shipping it over-claimed, and the fix round reverted the ship commit and closed the spec with a remainder instead, which RS005 refused. The branch had not been pushed, so the orchestrator rebuilt it from the commits before the ship plus cherry-picks, with no ship-and-revert pair in the history; a pushed branch under an open pull request has no such exit short of a new branch and a new pull request. Wanted: RS005 treats a delivery whose commit is reverted within the same range as withdrawn (git names the reverted commit in the revert's message), or its refusal names the rebuild as the remedy. + +## Grounds + +- pursued: a pushed branch can take a delivery or resolution back with git revert instead of being rebuilt; a declaration left in force after a range-internal revert, or one withdrawn by a hand-written line naming a commit outside the range, would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609251543293588-the-generic-account-floor-misses-the-json-serialised-windows.md b/.abcd/work/issues/resolved/iss-2609251543293588-the-generic-account-floor-misses-the-json-serialised-windows.md index 57c554e95..c0ec9f216 100644 --- a/.abcd/work/issues/resolved/iss-2609251543293588-the-generic-account-floor-misses-the-json-serialised-windows.md +++ b/.abcd/work/issues/resolved/iss-2609251543293588-the-generic-account-floor-misses-the-json-serialised-windows.md @@ -15,8 +15,8 @@ resolved_by: commit: "0d213f8f" --- -The generic-account floor misses the JSON-serialised Windows home root. accountRootPrefixes (internal/adapter/scanner/identity.go) carries the single-backslash spelling (\users\) but not the doubled one every JSON encoder writes, so C:\\Users\\LOGIN\\Desktop in a transcript line raises no finding at all while C:\Users\LOGIN\Desktop hard-fails; the home-literal clause of standsAsAccountName compares the configured home verbatim and misses its doubled spelling the same way. Before the generic floor the bare word was flagged, so the floor narrowed a hard_fail rule in the redactor's own input shape: a caller whose login is on the generic list leaks the home path of every Windows path a JSON transcript quotes through capture and history. +The generic-account floor misses the JSON-serialised Windows home root. accountRootPrefixes (internal/adapter/scanner/identity.go) carries the single-backslash spelling (\users\) but not the doubled one every JSON encoder writes, so C:\\Users\\carol\\Desktop in a transcript line raises no finding at all while C:\Users\carol\Desktop hard-fails; the home-literal clause of standsAsAccountName compares the configured home verbatim and misses its doubled spelling the same way. Before the generic floor the bare word was flagged, so the floor narrowed a hard_fail rule in the redactor's own input shape: a caller whose login is on the generic list leaks the home path of every Windows path a JSON transcript quotes through capture and history. ## Grounds -- pursued: a generic login in a JSON-escaped Windows home path is reported as local_username; a transcript line quoting C:\\Users\\LOGIN that yields no local_username finding would show it wrong. +- pursued: a generic login in a JSON-escaped Windows home path is reported as local_username; a transcript line quoting C:\\Users\\carol that yields no local_username finding would show it wrong. diff --git a/.abcd/work/issues/open/iss-2609251600029607-testnosecondfencerule-s-delimiter-only-detector-misses-a.md b/.abcd/work/issues/resolved/iss-2609251600029607-testnosecondfencerule-s-delimiter-only-detector-misses-a.md similarity index 62% rename from .abcd/work/issues/open/iss-2609251600029607-testnosecondfencerule-s-delimiter-only-detector-misses-a.md rename to .abcd/work/issues/resolved/iss-2609251600029607-testnosecondfencerule-s-delimiter-only-detector-misses-a.md index e4a8b1fc2..f00b81414 100644 --- a/.abcd/work/issues/open/iss-2609251600029607-testnosecondfencerule-s-delimiter-only-detector-misses-a.md +++ b/.abcd/work/issues/resolved/iss-2609251600029607-testnosecondfencerule-s-delimiter-only-detector-misses-a.md @@ -9,6 +9,14 @@ found_during: "autonomous run A resumed 2026-09-25" origin: researcher-authored production_mode: hand-written found_at: "internal/core/mdrecord" +resolution: "TestNoFenceRunReaderOutsideMdrecord reads the parsed source for regexp literals matching a fence run and first-byte comparisons against a backtick or tilde, allowlisted per file with a count and reason; TestNoSecondFenceRule's doc names what each detector reaches" +impact: internal +resolved_by: + commit: "38958d6d4" --- TestNoSecondFenceRule's delimiter-only detector misses a private toggle written as a literal regexp that matches a run (for example a pattern of 0-3 spaces then a backtick or tilde run, with a length check) or as byte comparisons on the first character; its doc says a pattern 'assembled at run time' escapes, which understates a literal regexp. Extend the detector to regexp literals whose pattern can match a fence run, and to first-byte comparisons against a backtick or tilde, or narrow the doc to the truth. + +## Grounds + +- pursued: a private toggle in either shape now fails a detector; a planted regexp run toggle or ln[0] == '~' toggle passing both detectors would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609251638574543-the-generic-account-floor-misses-a-windows-home-root-escaped.md b/.abcd/work/issues/resolved/iss-2609251638574543-the-generic-account-floor-misses-a-windows-home-root-escaped.md index 12f8097d5..4082e4e60 100644 --- a/.abcd/work/issues/resolved/iss-2609251638574543-the-generic-account-floor-misses-a-windows-home-root-escaped.md +++ b/.abcd/work/issues/resolved/iss-2609251638574543-the-generic-account-floor-misses-a-windows-home-root-escaped.md @@ -15,8 +15,8 @@ resolved_by: commit: "a0c126b1" --- -The generic-account floor misses a Windows home root escaped more than once. accountRootPrefixes (internal/adapter/scanner/identity.go) lists the single and the doubled backslash spellings of \users\ and the home-literal clause of standsAsAccountName lists the home and its doubled spelling, so C:\\\\Users\\\\LOGIN\\\\Desktop, the shape a transcript line carries when a tool result is itself JSON text (go env -json, npm config ls --json, any --json output), raises no finding for a login on the generic list while the doubled spelling hard-fails. Before the generic floor the bare word was flagged, so the floor narrowed a hard_fail rule in the redactor input: capture and history redact nothing on such a line. +The generic-account floor misses a Windows home root escaped more than once. accountRootPrefixes (internal/adapter/scanner/identity.go) lists the single and the doubled backslash spellings of \users\ and the home-literal clause of standsAsAccountName lists the home and its doubled spelling, so C:\\\\Users\\\\carol\\\\Desktop, the shape a transcript line carries when a tool result is itself JSON text (go env -json, npm config ls --json, any --json output), raises no finding for a login on the generic list while the doubled spelling hard-fails. Before the generic floor the bare word was flagged, so the floor narrowed a hard_fail rule in the redactor input: capture and history redact nothing on such a line. ## Grounds -- pursued: a generic login under a Windows home root escaped at any depth up to maxSeparatorRun is reported as local_username; a line carrying C:\Users\LOGIN with its separators quadrupled or more that yields no local_username finding, or a meter fixture of escaped roots whose charge grows faster than the line, would show it wrong. +- pursued: a generic login under a Windows home root escaped at any depth up to maxSeparatorRun is reported as local_username; a line carrying C:\Users\carol with its separators quadrupled or more that yields no local_username finding, or a meter fixture of escaped roots whose charge grows faster than the line, would show it wrong. diff --git a/.abcd/work/issues/open/iss-2609251823551349-capture-disposition-writes-disposition-grounds-and-exit.md b/.abcd/work/issues/resolved/iss-2609251823551349-capture-disposition-writes-disposition-grounds-and-exit.md similarity index 54% rename from .abcd/work/issues/open/iss-2609251823551349-capture-disposition-writes-disposition-grounds-and-exit.md rename to .abcd/work/issues/resolved/iss-2609251823551349-capture-disposition-writes-disposition-grounds-and-exit.md index 54eb45b5b..82d108b87 100644 --- a/.abcd/work/issues/open/iss-2609251823551349-capture-disposition-writes-disposition-grounds-and-exit.md +++ b/.abcd/work/issues/resolved/iss-2609251823551349-capture-disposition-writes-disposition-grounds-and-exit.md @@ -8,6 +8,14 @@ source: "user-observation" found_during: "autonomous run A resumed 2026-09-25" origin: researcher-authored production_mode: hand-written +resolution: "capture disposition now encodes hidden runes in --grounds and --exit-condition through termsafe.EncodeHiddenRunes after redaction; the sweep across every capture verb found no other unencoded free-text write" +impact: fix +resolved_by: + commit: "084a8adb0" --- capture disposition writes disposition_grounds and exit_condition redacted but never through termsafe.EncodeHiddenRunes (internal/core/capture/reading.go:471-479), so a bidi or zero-width rune in --exit-condition lands in the committed record verbatim, the class iss-2608301206073609 closed for other free-text writes (review-capture 2). + +## Grounds + +- pursued: a bidi or zero-width rune in a disposition's grounds or exit condition is percent-encoded in the committed dsp-N record; TestDispositionGroundsAndExitConditionEncodeHiddenRunes failing, or a raw hidden rune in a dsp record, would show it wrong diff --git a/.abcd/work/issues/open/iss-2609251823555125-capture-defer-past-the-same-anchor-appends-a-second-deferral.md b/.abcd/work/issues/resolved/iss-2609251823555125-capture-defer-past-the-same-anchor-appends-a-second-deferral.md similarity index 51% rename from .abcd/work/issues/open/iss-2609251823555125-capture-defer-past-the-same-anchor-appends-a-second-deferral.md rename to .abcd/work/issues/resolved/iss-2609251823555125-capture-defer-past-the-same-anchor-appends-a-second-deferral.md index fd8a2040d..4927c7653 100644 --- a/.abcd/work/issues/open/iss-2609251823555125-capture-defer-past-the-same-anchor-appends-a-second-deferral.md +++ b/.abcd/work/issues/resolved/iss-2609251823555125-capture-defer-past-the-same-anchor-appends-a-second-deferral.md @@ -8,6 +8,14 @@ source: "user-observation" found_during: "autonomous run A resumed 2026-09-25" origin: researcher-authored production_mode: hand-written +resolution: "capture defer past the same anchor rewrites that cycle's Deferral section in place instead of appending a second; a later anchor still appends" +impact: fix +resolved_by: + commit: "543100249" --- capture defer past the SAME anchor appends a second ## Deferral section (internal/core/capture/deferral.go:114), where the doc says one per cycle (review-capture 3). + +## Grounds + +- pursued: a record deferred twice past one anchor carries one Deferral section naming the latest reason, and a later anchor adds a second; TestReDeferringPastTheSameAnchorKeepsOneSection failing would show it wrong diff --git a/.abcd/work/issues/open/iss-2609251823559111-the-capture-ledger-s-os-root-escape-is-classified-by.md b/.abcd/work/issues/resolved/iss-2609251823559111-the-capture-ledger-s-os-root-escape-is-classified-by.md similarity index 59% rename from .abcd/work/issues/open/iss-2609251823559111-the-capture-ledger-s-os-root-escape-is-classified-by.md rename to .abcd/work/issues/resolved/iss-2609251823559111-the-capture-ledger-s-os-root-escape-is-classified-by.md index ef239c7b6..9166d57da 100644 --- a/.abcd/work/issues/open/iss-2609251823559111-the-capture-ledger-s-os-root-escape-is-classified-by.md +++ b/.abcd/work/issues/resolved/iss-2609251823559111-the-capture-ledger-s-os-root-escape-is-classified-by.md @@ -8,6 +8,14 @@ source: "user-observation" found_during: "autonomous run A resumed 2026-09-25" origin: researcher-authored production_mode: hand-written +resolution: "both ledger race tests assert errors.Is(err, ErrPathUnsafe) and a direct test feeds mapEscape a real os.Root escape, so a reworded Go message fails the capture tests" +impact: internal +resolved_by: + commit: "20da9778c" --- The capture ledger's os.Root escape is classified by matching the string 'path escapes from parent' (internal/core/capture/ledgerroot.go:59-64), and nothing pins the match: ledgerroot_test.go:62 and :97 assert only err != nil, so a Go release that rewords the message would silently degrade ErrPathUnsafe to a generic error (review-capture 1). Assert errors.Is(err, ErrPathUnsafe) in both race tests. + +## Grounds + +- pursued: a Go release that rewords the os.Root escape text turns TestMapEscapeClassifiesTheRealOSRootEscape and the write-race test RED; a mutant of the match string leaving the capture tests green would show it wrong diff --git a/.abcd/work/issues/open/iss-2609251823560369-the-capture-verbs-json-and-stderr-print-the-checkout-path.md b/.abcd/work/issues/resolved/iss-2609251823560369-the-capture-verbs-json-and-stderr-print-the-checkout-path.md similarity index 55% rename from .abcd/work/issues/open/iss-2609251823560369-the-capture-verbs-json-and-stderr-print-the-checkout-path.md rename to .abcd/work/issues/resolved/iss-2609251823560369-the-capture-verbs-json-and-stderr-print-the-checkout-path.md index c82162697..ff5cde8d9 100644 --- a/.abcd/work/issues/open/iss-2609251823560369-the-capture-verbs-json-and-stderr-print-the-checkout-path.md +++ b/.abcd/work/issues/resolved/iss-2609251823560369-the-capture-verbs-json-and-stderr-print-the-checkout-path.md @@ -8,6 +8,14 @@ source: "user-observation" found_during: "autonomous run A resumed 2026-09-25" origin: researcher-authored production_mode: hand-written +resolution: "the capture verbs' ledger identity names a checkout outside HOME by its directory name, never its absolute path, and renderLedger refuses a non-object result instead of silently dropping the ledger member" +impact: fix +resolved_by: + commit: "a6c630c39" --- The capture verbs' --json and stderr print the checkout path through RedactHome only, so a checkout outside HOME is printed in full (internal/surface/cli/cli.go renderLedger), an absolute local path in output that may be pasted elsewhere; renderLedger also splices the ledger member by byte surgery on the trailing brace (review-capture 4). + +## Grounds + +- pursued: a capture verb run in a checkout outside HOME prints no absolute path in --json or on stderr; TestTheLedgerIdentityNeverPrintsAnAbsoluteCheckout failing, or an absolute checkout in either render, would show it wrong diff --git a/.abcd/work/issues/open/iss-2609251842111593-the-admission-ordering-gate-reads-committed-as-a-run-json.md b/.abcd/work/issues/resolved/iss-2609251842111593-the-admission-ordering-gate-reads-committed-as-a-run-json.md similarity index 54% rename from .abcd/work/issues/open/iss-2609251842111593-the-admission-ordering-gate-reads-committed-as-a-run-json.md rename to .abcd/work/issues/resolved/iss-2609251842111593-the-admission-ordering-gate-reads-committed-as-a-run-json.md index 85fa07d64..de12e40ec 100644 --- a/.abcd/work/issues/open/iss-2609251842111593-the-admission-ordering-gate-reads-committed-as-a-run-json.md +++ b/.abcd/work/issues/resolved/iss-2609251842111593-the-admission-ordering-gate-reads-committed-as-a-run-json.md @@ -9,6 +9,14 @@ found_during: "autonomous run A resumed 2026-09-25" origin: researcher-authored production_mode: hand-written found_at: "internal/core/capture/itemfate.go" +resolution: "the ordering gate holds a comparative run record naming the widening run to the channel's pair: run_id names its directory and the manifest beside it agrees on run, position and candidate_run; a hand-placed marker without that is refused by name" +impact: fix +resolved_by: + commit: "819709856" --- The admission ordering gate reads 'committed' as a run.json decoding to position comparative with a candidate_run, with no run_id/directory, manifest, item or git check (internal/core/capture/itemfate.go:145-186), so an untracked, id-less marker opens the gate; this matches the spec's Approach but not its line that no mutable file anywhere records the outcome (review-admission 2). + +## Grounds + +- pursued: an id-less or manifest-less comparative marker no longer opens the widening gate while the channel's own ingest still does; TestComparativeRunForRefusesAMarkerTheChannelDidNotWrite or TestTheChannelsCommittedComparativeRunSatisfiesTheGate failing would show it wrong diff --git a/.abcd/work/issues/open/iss-2609251842112266-the-widening-run-summary-s-stand-down-is-bypassed-for-an.md b/.abcd/work/issues/resolved/iss-2609251842112266-the-widening-run-summary-s-stand-down-is-bypassed-for-an.md similarity index 60% rename from .abcd/work/issues/open/iss-2609251842112266-the-widening-run-summary-s-stand-down-is-bypassed-for-an.md rename to .abcd/work/issues/resolved/iss-2609251842112266-the-widening-run-summary-s-stand-down-is-bypassed-for-an.md index 83e7e29a1..4edb01cef 100644 --- a/.abcd/work/issues/open/iss-2609251842112266-the-widening-run-summary-s-stand-down-is-bypassed-for-an.md +++ b/.abcd/work/issues/resolved/iss-2609251842112266-the-widening-run-summary-s-stand-down-is-bypassed-for-an.md @@ -9,6 +9,14 @@ found_during: "autonomous run A resumed 2026-09-25" origin: researcher-authored production_mode: hand-written found_at: "internal/core/lint/readingoutstanding.go" +resolution: "the widening-run summary checks the stand-down before counting an admission, so an admitted proposal with a contested, cyclic, unsafe or illegible disposition stands the run's summary down" +impact: fix +resolved_by: + commit: "698f83df9" --- The widening-run summary's stand-down is bypassed for an admitted item: internal/core/lint/readingoutstanding.go:421-425 takes the admissions.admits case before the unsafe/contested/cyclic check, so a run whose only acceptance is unreadable or contested still reports 'admitted 1, outstanding []', contradicting the type's own doc comment (lines 131-135; review-admission 1). + +## Grounds + +- pursued: a run whose admitted proposal carries an answer the walk cannot read reports no summary; a summary line reporting admitted 1 over a contested or illegible disposition would show it wrong diff --git a/.abcd/work/issues/open/iss-2609252038344132-two-low-notes-from-the-owed-review-1-intent-audit-json.md b/.abcd/work/issues/resolved/iss-2609252038344132-two-low-notes-from-the-owed-review-1-intent-audit-json.md similarity index 55% rename from .abcd/work/issues/open/iss-2609252038344132-two-low-notes-from-the-owed-review-1-intent-audit-json.md rename to .abcd/work/issues/resolved/iss-2609252038344132-two-low-notes-from-the-owed-review-1-intent-audit-json.md index eadb93b73..3404c2e05 100644 --- a/.abcd/work/issues/open/iss-2609252038344132-two-low-notes-from-the-owed-review-1-intent-audit-json.md +++ b/.abcd/work/issues/resolved/iss-2609252038344132-two-low-notes-from-the-owed-review-1-intent-audit-json.md @@ -9,6 +9,14 @@ found_during: "autonomous run A resumed 2026-09-25" origin: researcher-authored production_mode: hand-written found_at: "internal/core/intent/owed.go" +resolution: "The owed listing withholds every token of a dead-letter reason that names the local tier, so a forged retention clause in the host-supplied reason never reaches intent audit --json; the property test forges one. The corroboration prose of iss-2609100509537730 moves out of its Grounds section, whose two pursued entries stay because the section is append-only." +impact: fix +resolved_by: + commit: "d39f08a0117557f7223eb154136039cbe9b7ff57" --- Two low notes from the owed review: (1) intent audit --json echoes a dead-letter reason's attacker-supplied text verbatim, so a quoted-back token shaped like 'Raw payload retained at .../.work.local/...' puts a local-tier-looking string in the output (the real retention path is cut correctly; internal/core/intent/owed.go:120-147), and the test's .work.local substring check proves the fixture, not the property; (2) the resolution of iss-2609100509537730 appends a near-duplicate pursued grounds bullet after its corroboration prose. + +## Grounds + +- pursued: we expect withholding local-tier tokens at the reader to keep the listing's no-local-path promise against any reason text; it is shown wrong if a reason spelled to name the local tier still reaches the JSON or text listing diff --git a/.abcd/work/issues/open/iss-2609260932374727-settled-marker-admitted-mid-sentence.md b/.abcd/work/issues/resolved/iss-2609260932374727-settled-marker-admitted-mid-sentence.md similarity index 52% rename from .abcd/work/issues/open/iss-2609260932374727-settled-marker-admitted-mid-sentence.md rename to .abcd/work/issues/resolved/iss-2609260932374727-settled-marker-admitted-mid-sentence.md index f570b8259..3293bbaaa 100644 --- a/.abcd/work/issues/open/iss-2609260932374727-settled-marker-admitted-mid-sentence.md +++ b/.abcd/work/issues/resolved/iss-2609260932374727-settled-marker-admitted-mid-sentence.md @@ -9,6 +9,14 @@ found_during: "autonomous run A resumed 2026-09-25: review2-loop1 item 3" origin: researcher-authored production_mode: hand-written found_at: "internal/core/intent/questions.go" +resolution: "A settled label (resolved:, deferred:) counts only opening a line of an Open Questions item, after a closing bold, or after a dash; mid-sentence it is prose and the item stays a question. The bold-span marker keeps its reach. No committed intent changes verdict." +impact: fix +resolved_by: + commit: "ef9ef3819c6401f4806b6c5353ce8ca0a29d818f" --- settledMarkRe (internal/core/intent/questions.go:30) admits `resolved:` or `deferred:` anywhere in an open-question item, so an item such as 'Which id wins once the split is resolved: the old or the new?' reads as settled and build starts past a real question. No record trips it today (every intent scanned); the tightening is a label at the item's start or after a closing bold plus dash, not mid-sentence. + +## Grounds + +- pursued: we expect anchoring the label to its written positions to stop a question that uses the word mid-sentence from reading as settled without refusing any record the tree settles; it is shown wrong if a record written in the settled convention starts reading as open, or a mid-sentence label still admits an item diff --git a/.abcd/work/issues/open/iss-2609261140284421-setext-principle-title-not-carried.md b/.abcd/work/issues/resolved/iss-2609261140284421-setext-principle-title-not-carried.md similarity index 62% rename from .abcd/work/issues/open/iss-2609261140284421-setext-principle-title-not-carried.md rename to .abcd/work/issues/resolved/iss-2609261140284421-setext-principle-title-not-carried.md index a1b40c1a0..ecb392976 100644 --- a/.abcd/work/issues/open/iss-2609261140284421-setext-principle-title-not-carried.md +++ b/.abcd/work/issues/resolved/iss-2609261140284421-setext-principle-title-not-carried.md @@ -9,6 +9,14 @@ found_during: "autonomous run A resumed 2026-09-25: review2-principles LOW" origin: researcher-authored production_mode: hand-written found_at: "internal/core/lint/principles.go" +resolution: "FindPrincipleStatement reads a setext H1 (the paragraph above a === underline) as well as an ATX one, so the projection carries the title and principle_claims judges it" +impact: fix +resolved_by: + commit: "0e9a7c5b9" --- A principle whose H1 title is written in setext form (the title line underlined with ===) travels to a reading without its title: principleTitleRe in internal/core/lint/principles.go:127 matches ATX headings only, so the projection sends the statement paragraph bare, and a cold reader gets a sentence with no name. Both readers agree, so no gate disagrees; the promise that a principle is readable cold does not hold for that file shape. + +## Grounds + +- pursued: a principle titled in setext form now travels to a reading with its title above the statement; a setext-titled principle projected bare, or a citation in a setext title passing principle_claims, would show it wrong diff --git a/.abcd/work/issues/open/iss-2609261208193041-record-schema-md-named-link-at-store-root.md b/.abcd/work/issues/resolved/iss-2609261208193041-record-schema-md-named-link-at-store-root.md similarity index 66% rename from .abcd/work/issues/open/iss-2609261208193041-record-schema-md-named-link-at-store-root.md rename to .abcd/work/issues/resolved/iss-2609261208193041-record-schema-md-named-link-at-store-root.md index 9cc643470..8a9c5c931 100644 --- a/.abcd/work/issues/open/iss-2609261208193041-record-schema-md-named-link-at-store-root.md +++ b/.abcd/work/issues/resolved/iss-2609261208193041-record-schema-md-named-link-at-store-root.md @@ -9,6 +9,14 @@ found_during: "autonomous run A resumed 2026-09-25: fix4-lintA sweep" origin: researcher-authored production_mode: hand-written found_at: "internal/core/lint/schema.go" +resolution: "record_schema names every markdown-named link at a bucketed store root that is not README.md or a record filename, without following it" +impact: fix +resolved_by: + commit: "f407dc13f" --- record_schema still passes silently over one link shape at a record store root: a link whose name ends in .md but does not match the record filename pattern (for example notes.md pointing at a directory). A real directory with that name is reported; the link is not, because telling whether it points at a directory would mean following it, which the walk never does. Reporting every link at a store root other than README.md, without following it, closes the shape. + +## Grounds + +- pursued: a notes.md link at a store root draws one finding whatever it points at, and nothing behind it is read; such a link drawing no finding, or its target's content surfacing, would show it wrong diff --git a/.abcd/work/issues/open/iss-2609261325441711-capture-reframe-s-whole-write-reads-a-multi-commit-rebase.md b/.abcd/work/issues/resolved/iss-2609261325441711-capture-reframe-s-whole-write-reads-a-multi-commit-rebase.md similarity index 64% rename from .abcd/work/issues/open/iss-2609261325441711-capture-reframe-s-whole-write-reads-a-multi-commit-rebase.md rename to .abcd/work/issues/resolved/iss-2609261325441711-capture-reframe-s-whole-write-reads-a-multi-commit-rebase.md index f18c6c0e3..07bfdb7ba 100644 --- a/.abcd/work/issues/open/iss-2609261325441711-capture-reframe-s-whole-write-reads-a-multi-commit-rebase.md +++ b/.abcd/work/issues/resolved/iss-2609261325441711-capture-reframe-s-whole-write-reads-a-multi-commit-rebase.md @@ -9,6 +9,14 @@ found_during: "autonomous run A resumed 2026-09-25: integ3, review2-reframe OBS- origin: researcher-authored production_mode: hand-written found_at: "internal/core/capture/reframe.go" +resolution: "commands/capture.md and brief 06-capture.md name the rebased-series case (the whole write records the last step alone) and the open/complete route that records it whole; TestReframeWholeWriteOfALinearSeriesRecordsItsLastStep pins the behaviour" +impact: fix +resolved_by: + commit: "91c12634e" --- capture reframe's whole write reads a multi-commit rebase differently from a --no-ff merge or a squash of the same rewrite: two rebased commits that move the construal and then the glossary give changed=[glossary] with before at the post-construal state, where --no-ff and squash give changed=[construal glossary]. The first-differing-triple rule of spc-2609020626048705 produces it, and commands/capture.md and brief 06-capture.md claim only squash equivalence. Either a line on the page naming the rebase case or a spec ruling on what previous distinct state means across a rebased series; not decided here (review2-reframe OBS-A). + +## Grounds + +- pursued: the page now says what the whole write records for a multi-commit first-parent rewrite, matching the code; the pin test failing, or a spec ruling that redefines the previous distinct state across a rebased series, would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609281314564762-the-two-ci-gate-scripts-test-membership-and-matches-by.md b/.abcd/work/issues/resolved/iss-2609281314564762-the-two-ci-gate-scripts-test-membership-and-matches-by.md new file mode 100644 index 000000000..62570a3b9 --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609281314564762-the-two-ci-gate-scripts-test-membership-and-matches-by.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609281314564762" +slug: "the-two-ci-gate-scripts-test-membership-and-matches-by" +severity: "major" +category: "bug" +source: "user-observation" +found_during: "autonomous run A resumed 2026-09-25 (lane drainScr, full-history before/after runs)" +origin: researcher-authored +production_mode: hand-written +found_at: "scripts/check-issue-resolution.sh" +resolution: "Every membership and match test in scripts/check-issue-resolution.sh and scripts/check-attribution.sh reads a here-string instead of a printf pipe, and RS006's frontmatter reader reads to the end instead of exiting early; the cases suites' own helpers take the same change. Proved by two cases, each failing against the previous scripts: a message declaring 1,100 long ids (about 70 KiB) is read whole by RS004 (the previous gate refused 39 of its declared ids), and a Co-authored-by line at the top of a 200 KiB pull-request body is refused (the previous gate accepted it). The RS006 reader half landed in eff2eefda, proved by a resolved record of about 200 KiB passing (the previous gate ended at exit 141)." +impact: internal +resolved_by: + commit: "5b7c79426" +--- + +The two CI gate scripts test membership and matches by piping printf into grep -q under set -o pipefail, and that pipeline is a race whenever printf needs more than one write: grep -q exits at its first match, printf's next write takes SIGPIPE, the pipeline returns 141, and the test reads as no match. Under load the race loses at a few KiB; past one pipe buffer (64 KiB) it loses every time. Measured: a 50 KiB id list missed 0 of 30 runs and a 70 KiB one 30 of 30; a here-string missed none. In scripts/check-issue-resolution.sh the RS001 membership test against the ids entering a terminal folder, RS004's declared-id test, and RS005's shipped test all take this shape, so two full-history runs of the gate over the same range at the same commits differed by 47 violations (39 RS001 refusals of records that did enter resolved/, seven RS004 refusals of ids a 190-line Refs: block declared, one RS005), and the base-listing membership test the lane adds for iss-2609012047551175 would read a record terminal at the base as absent on a real ledger, failing open. In scripts/check-attribution.sh check_text pipes a whole commit message or pull-request body into grep -q for the tool-footer, Co-authored-by and trailer checks, so a body longer than the buffer whose banned line is matched before the last write can pass. Fix: feed grep from a here-string, which bash writes in full before grep reads, so no writer is left to take SIGPIPE. + +## Grounds + +- pursued: the gates give the same verdict on the same range whatever the machine load or input size; two full-history runs at the same commits disagreeing again would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609281329007423-a-checkout-worktree-or-store-path-outside-home-is-printed.md b/.abcd/work/issues/resolved/iss-2609281329007423-a-checkout-worktree-or-store-path-outside-home-is-printed.md new file mode 100644 index 000000000..90df6a19c --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609281329007423-a-checkout-worktree-or-store-path-outside-home-is-printed.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609281329007423" +slug: "a-checkout-worktree-or-store-path-outside-home-is-printed" +severity: "minor" +category: "security" +source: "review-followup" +found_during: "autonomous run A resumed 2026-09-25: review-drainCap" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/fsutil/paths.go" +resolution: "fsutil.DisplayPath states the display rule once (home-relative under HOME, the directory's base name outside it) and DisplayPathsIn applies it inside a message; scrubPaths, ledgerIdentityOf and every confirmed checkout or worktree site route through them, and paths the reader acts on (an await's brief and receipt, the sources corpus, the history store notes) keep RedactHome" +impact: fix +resolved_by: + commit: "1d334d124" +--- + +A checkout, worktree or store path outside HOME is printed whole by the surfaces that redact through fsutil.RedactHome alone: RedactHome turns a path under HOME into ~/rel and leaves every other absolute path untouched, so a sibling worktree in /private/tmp or on another volume reaches abcd peers (--json path, not_read, skipped, the peer-held refusal), the implement check's contention detail, history ingest's destination and orphan cwd, and implement status's lane worktree as a full local path in output a person pastes elsewhere. The display rule that fixes it (under HOME the home-relative form, outside HOME the directory's base name) is stated inline twice, in scrubPaths and in the capture verbs' ledgerIdentityOf, and in no primitive a new surface can reach, which is why each new surface re-derives RedactHome alone. + +## Grounds + +- pursued: a checkout or worktree outside HOME is printed by its directory name by peers, the implement peers check, history ingest and implement status; TestPeersNamesAWorktreeOutsideHomeByItsDirectoryName or its siblings failing, or TestNoInlineBaseNameDisplayRule finding an inline copy, would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609281613094952-the-status-board-prints-the-checkout-s-absolute-path.md b/.abcd/work/issues/resolved/iss-2609281613094952-the-status-board-prints-the-checkout-s-absolute-path.md new file mode 100644 index 000000000..e49410b40 --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609281613094952-the-status-board-prints-the-checkout-s-absolute-path.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609281613094952" +slug: "the-status-board-prints-the-checkout-s-absolute-path" +severity: "minor" +category: "security" +source: "review-followup" +found_during: "autonomous run A resumed 2026-09-25: review-drainRedact" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/core.go" +resolution: "The status board routes the checkout through fsutil.DisplayPath in the text form's first line and in --json's dir (a display field: the plugin page relays it and no consumer acts on it), and the board's peers notice names the checkout through DisplayPathsIn against its root, so a checkout under HOME is shown as ~/rel and one outside HOME by its directory name" +impact: fix +resolved_by: + commit: "a03e1a975" +--- + +The status board prints the checkout's absolute path with no redaction at all: core.Status sets Dir to filepath.Abs(cwd) (internal/core/core.go:46), bare abcd renders it raw as its first line ('abcd — ', internal/surface/cli/cli.go:285) and abcd --json carries it whole as dir. Under HOME it prints /Users//..., and outside HOME the full path, in the output a person pastes most often. The board should name the checkout by the display rule fsutil.DisplayPath states (home-relative under HOME, the directory's base name outside it), the sibling of iss-2609281329007423, which routed every other checkout and worktree display but not this one. + +## Grounds + +- pursued: bare abcd names a checkout under HOME as ~/rel and one outside HOME by its directory name, in text, JSON and the peers notice; TestBoardNamesACheckoutUnderHomeHomeRelative, TestBoardNamesACheckoutOutsideHomeByItsDirectoryName or TestBoardPeersNoticeNamesACheckoutOutsideHomeByItsDirectoryName failing would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609281627055603-exclusion-floor-closes-its-frontmatter-block-before-the-canonical-reader.md b/.abcd/work/issues/resolved/iss-2609281627055603-exclusion-floor-closes-its-frontmatter-block-before-the-canonical-reader.md new file mode 100644 index 000000000..7a4ce6e4f --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609281627055603-exclusion-floor-closes-its-frontmatter-block-before-the-canonical-reader.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609281627055603" +slug: "exclusion-floor-closes-its-frontmatter-block-before-the-canonical-reader" +severity: "minor" +category: "security" +source: "review-followup" +found_during: "autonomous run A resumed 2026-09-25: review-drainFm" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/reading/project.go" +resolution: "The floor's key and shape scans run to blockScanEnd, the later of its own prefix close and frontmatter.Close, so a four-dash line or a delimiter carrying text no longer ends them early; the body scans keep the earlier close." +impact: fix +resolved_by: + commit: "3b6a81ad5" +--- + +The reading corpus's exclusion floor closes its frontmatter block earlier than the canonical reader: blockCloser (internal/core/reading/project.go) closes on any column-0 line opening with three dashes, so a column-0 `----` or `--- x` ends the floor's block while frontmatter.IsDelimiter, frontmatter.Fields and a YAML reader read on to the real `---`. The keys between the two closes are frontmatter to the canonical reader and invisible to excludedKeyInFirstBlock and unresolvableFrontmatterShape, so an excluded key there reaches the corpus under a manifest asserting its refusal. Probe: a block opened by `---`, holding `foo: 1`, then `----`, then `secret: x`, then `---` — floor close is line 2, canonical close line 4, Fields reports secret, and excludedKeyInFirstBlock does not refuse it. The allowlist reason in delimiter_canonical_test.go (the floor 'refuses more, never less') is false for this shape. + +## Grounds + +- pursued: an excluded key between a `----` (or `--- x`, BOM-led or CRLF) and the canonical close is refused, and every committed markdown file reads identically through the floor; a document Fields reads a key from that the floor admits would show it wrong diff --git a/.abcd/work/issues/resolved/iss-2609281736483740-the-status-board-s-first-line-prints-the-checkout-s-display.md b/.abcd/work/issues/resolved/iss-2609281736483740-the-status-board-s-first-line-prints-the-checkout-s-display.md new file mode 100644 index 000000000..e0aa887f9 --- /dev/null +++ b/.abcd/work/issues/resolved/iss-2609281736483740-the-status-board-s-first-line-prints-the-checkout-s-display.md @@ -0,0 +1,22 @@ +--- +schema_version: 1 +id: "iss-2609281736483740" +slug: "the-status-board-s-first-line-prints-the-checkout-s-display" +severity: "minor" +category: "security" +source: "review-followup" +found_during: "autonomous run A resumed 2026-09-25: fix2-drainRedact" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/surface/cli/cli.go" +resolution: "The board's text first line goes through termsafe.Sanitize, so an ESC sequence or a bidi control in the checkout's directory name is masked; --json dir keeps the true name, which encoding/json escapes where it is a control byte. TestBoardFirstLineMasksControlsInTheCheckoutName was watched fail with both names printed raw, and passes after." +impact: fix +resolved_by: + commit: "7004cc395" +--- + +The status board's first line prints the checkout's display name without termsafe.Sanitize, while every other board value is sanitised. Bare abcd writes 'abcd — ' from fsutil.DisplayPath(st.Dir) straight to the terminal, so a checkout whose directory name carries an ESC sequence or a bidi control (U+202E) reaches the terminal raw: a crafted clone directory can recolour, retitle or reorder the board a person reads and pastes. Named FOR THE REVIEWER by fix2-drainRedact after it routed the line through DisplayPath. + +## Grounds + +- pursued: a checkout directory named with ESC or U+202E prints masked on the board's first line; the raw control reaching the text output, or --json dir no longer carrying the true name, would show it wrong. diff --git a/.abcd/work/issues/resolved/iss-263-dispatch-unparseable-issue-reads-as-not-found.md b/.abcd/work/issues/resolved/iss-263-dispatch-unparseable-issue-reads-as-not-found.md new file mode 100644 index 000000000..1a4da49fa --- /dev/null +++ b/.abcd/work/issues/resolved/iss-263-dispatch-unparseable-issue-reads-as-not-found.md @@ -0,0 +1,20 @@ +--- +schema_version: 1 +id: "iss-263" +slug: "dispatch-unparseable-issue-reads-as-not-found" +severity: "nitpick" +category: "ux" +source: "impl-review" +found_during: "spc-26 build, ruthless-reviewer note" +found_at: "internal/core/record/record.go" +resolution: "Overtaken by 4323948fc: describeIssue consults the skipped roster capture.List returns and faults with ErrSkippedRecord naming the file and the reader's own parse error, never 'not found'. The record's own shape, a never-closed block, is pinned by TestDescribeUnparseableIssueIsNotNotFound in 1ebed1fa0." +impact: internal +resolved_by: + commit: "4323948fc" +--- + +describeIssue discards ListResult.Skipped, so an issue file that exists but is unparseable (broken frontmatter) makes abcd iss-N report 'not found in the issue ledger' — a diagnostic that misleads about a record physically present in a status dir. Surface the skip roster in the fault: 'iss-N present but unreadable at : '. + +## Grounds + +- pursued: abcd iss-N on a present but unparseable issue file names the file and the parse error; a broken-frontmatter issue that answers not found in the issue ledger would show it wrong diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 8ba1c0781..48247f99a 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -110,18 +110,23 @@ disclosure, and never an authorship assertion for a tool. The rules: responsibility. - **Commit as yourself.** The gate reads the git author AND committer of every commit in a pull request, merge commits included, and refuses a machine - identity on any of five signals. Four apply to both roles: an assistant + identity on any of six signals. Five apply to both roles: an assistant vendor's name standing alone (`Claude`, `Copilot`, `Gemini` and their kin, matched whole, so a human named Claudette passes); an assistant vendor's mail domain (`@anthropic.com`, `@openai.com`); the forge's own `[bot]` name suffix; - and a bot mailbox (`NNNN+name[bot]@users.noreply.github.com`, or - `@dependabot.com`). The fifth applies to the AUTHOR role only: **any** address + a bot mailbox (`NNNN+name[bot]@users.noreply.github.com`, or + `@dependabot.com`); and a trailing `bot`, `robot` or `automation` word ending + the name or the mailbox's local part, standing alone or joined by `-` or `_` + (`semantic-release-bot`, `ci_bot@`, and `Renovate Bot` at its default + `bot@renovateapp.com`), so Talbot, `jean.bot@` and a person named `Jan Bot` + pass. + The sixth applies to the AUTHOR role only: **any** address whose mailbox begins `noreply@` or `donotreply@`, with or without hyphens and whatever the host, not just a vendor's. Your forge privacy address (`1234+you@users.noreply.github.com`) is yours and passes — the `[bot]` marker in the mailbox is what marks a machine, not the `users.noreply.github.com` host — and the forge's own `GitHub ` committer stamp on a - web-UI merge passes too, which is why the fifth signal is author-only. Set + web-UI merge passes too, which is why the sixth signal is author-only. Set `user.name` and `user.email` to a human before you commit; the assistant belongs in the trailer, never in the identity fields the contributor graph reads. An automated dependency bump is therefore landed by a human rather than diff --git a/AGENTS.md b/AGENTS.md index 4af5abed7..36793cecf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -354,7 +354,11 @@ irreversible; guessing downward costs nothing.** record satisfies nothing. Resolution is deliberately not a post-merge step: a step that happens after the merge is the one that gets forgotten, and a fixed-but-open issue leaves no marker to find it by. Resolving without a trailer stays legal — a - stale issue closed on its own merits has no fixing commit to name. + stale issue closed on its own merits has no fixing commit to name. A `git + revert` later in the same range withdraws a `Resolves:` only for a record its + own diff takes back out of `resolved/` or `wontfix/`, a record the reverted + commit itself moved in; a "This reverts commit" line over a commit that moves + no record, or naming a commit that never moved that record, withdraws nothing. - **A change that delivers a planned intent closes its spec in the same change**: `go run ./cmd/abcd spec close ` moves the spec to `closed/` and, as its close-hook, the intent from `planned/` to `shipped/`. Nothing @@ -373,7 +377,9 @@ irreversible; guessing downward costs nothing.** and there is no default: a record that does not already declare it takes `--impact additive|breaking|fix` on the close, and a close with neither is refused before anything moves. Same shape as the issue rule above: the step - that happens after the merge is the one that gets forgotten. + that happens after the merge is the one that gets forgotten. A revert + withdraws a `Delivers:` on the same terms, only for an intent its own diff + takes back out of `shipped/`, an intent the reverted commit itself moved in. - **A `resolved_by.commit` stamp names a commit that is actually reachable.** `abcd capture resolve --commit` is shape-checked only, so a wrong sha reads exactly like a right one; RS002/RS003 check reachability instead. Note the @@ -405,15 +411,22 @@ irreversible; guessing downward costs nothing.** there asserts an authorship it does not hold — and a squash merge re-appends a mis-identified branch author as a co-author, inflating the graph again on every squash. `scripts/check-attribution.sh commits` reads the identity of every - commit in a range, merge commits included, and refuses one on any of five - signals. Four are checked in both roles: an assistant vendor's name standing + commit in a range, merge commits included, and refuses one on any of six + signals. Five are checked in both roles: an assistant vendor's name standing alone as the identity name (`Claude`, `Copilot`, `Gemini` and their kin, matched whole so a human named Claudette passes); an assistant vendor's mail domain (`@anthropic.com`, `@openai.com`); the forge's own `[bot]` name suffix; - and a bot mailbox (`NNNN+name[bot]@users.noreply.github.com`, or - `@dependabot.com`). The last two are structural rather than nominal, which is - why a second automation lands in the right place with no edit to the list. The - fifth signal is checked in the AUTHOR role only: **any** address whose mailbox + a bot mailbox (`NNNN+name[bot]@users.noreply.github.com`, or + `@dependabot.com`); and a trailing `bot`, `robot` or `automation` word ending + the name or the mailbox's local part, standing alone or joined by `-` or `_` + (`semantic-release-bot`, `ci_bot@`, and `Renovate Bot` at its default + `bot@renovateapp.com`), so Talbot, `jean.bot@` and a person named `Jan Bot` + pass. The `[bot]` suffix and the bot mailbox are structural rather than + nominal, which is why a second automation the forge stamps lands in the right + place with no edit to the list; the trailing word is a name shape, drawn + narrowly, and a machine configured with a person-shaped name and mailbox stays + out of reach, and the reviewer is the check on it. The + sixth signal is checked in the AUTHOR role only: **any** address whose mailbox begins `noreply@` or `donotreply@` (with or without hyphens), whatever the host — it is not scoped to a vendor, because an address named for not being read names no person in the role that claims authorship. It is refuse-machines, not an diff --git a/README.md b/README.md index 0f86caf2c..2c4dccbed 100644 --- a/README.md +++ b/README.md @@ -118,7 +118,7 @@ In a plugin session, inside a repository you own, `/abcd:prepare-this-repo` audi ```text $ abcd -abcd — /path/to/your-repo +abcd — ~/code/your-repo git repo: true record: true work tiers: [development work work.local] diff --git a/commands/abcd.md b/commands/abcd.md index bb92d7297..baeeae7d5 100644 --- a/commands/abcd.md +++ b/commands/abcd.md @@ -15,10 +15,10 @@ Run: "${CLAUDE_PLUGIN_ROOT}/abcd" --json ``` -Then summarise the JSON for the user: the directory (`dir`, with the home -directory written as `~`), whether it is a git repo, -whether the abcd development record is present, and which `.abcd/` work tiers -exist. +Then summarise the JSON for the user: the directory (`dir`, named +home-relative as `~/…`, or by its directory name outside HOME, never by an +absolute path), whether it is a git repo, whether the abcd development record is +present, and which `.abcd/` work tiers exist. In a repository abcd manages the board also carries one line of presence — the `statusline` object in the JSON (`state`, `plain`, `elements`), rendered as a diff --git a/commands/build.md b/commands/build.md index 3ab886177..50a5587e7 100644 --- a/commands/build.md +++ b/commands/build.md @@ -88,7 +88,9 @@ returns hand the receipt back: The lane advances only on a receipt that verifies. Asking for a step while the lane awaits a receipt re-tells what it awaits and moves nothing. `"${CLAUDE_PLUGIN_ROOT}/abcd" implement status --json` renders every run, its -lanes and its record, and writes nothing. +lanes and its record, and writes nothing. A lane's `worktree` is home-relative, +or its directory name when it sits outside HOME; its `brief` and `receipt` keep +their full home-relative paths, because the agent acts on them. A lane's steps run in order: diff --git a/commands/capture.md b/commands/capture.md index 6852f6d51..7b684dde6 100644 --- a/commands/capture.md +++ b/commands/capture.md @@ -434,6 +434,8 @@ since the last release and still open, and one sanctioned way past it is a deferral stated out loud. `defer` writes it: `deferred_after` (the anchor tag) and `deferral_reason` in the record's frontmatter, and a dated `## Deferral ` section appended to its body. The record stays in `open/`. +A second deferral past the same anchor replaces that cycle's pair and section +rather than adding another, so the body carries one section per cycle. Report the `id`, `deferred_after` and `deferral_reason` from the JSON, and tell the user that the waiver lapses when the next release re-anchors, so it must be renewed then or the finding fixed. Report `redacted` whenever it is non-zero. @@ -500,7 +502,8 @@ run. The refusal names the run and says what it is waiting for: the comparative reading over that run, ingested through `/abcd:reading`. A comparative run committed with an empty item set, the position not exercised, satisfies it too. Every other position is answered with no comparative run anywhere. Relay the -refusal; do not write the record by hand to get past it. +refusal; do not write the record by hand to get past it: a run record without the +manifest the ingest writes beside it, or disagreeing with it, is refused by name. ## Admit a widening proposal @@ -594,7 +597,12 @@ A reframe is written in one of three halves, and every render names which: not. It walks the surfaces' history along first parents to the previous distinct committed state and writes both halves at once, so a rewrite a merge brought in is recorded against the state the merge's first parent held, as a - squash of the same branch would be, whatever the commits' timestamps. + squash of the same branch would be, whatever the commits' timestamps. A + rewrite that lands as several commits on the first-parent line, a rebased + branch among them, is different: the previous distinct state is the one + before its last commit, so the whole write records that step alone. To record + such a rewrite whole, open the record before its first commit and complete it + after its last. - **Open**, before the rewrite is committed (`--open`). The before fingerprints are `HEAD`'s and the after half is absent; the render names the completion. Only one record may be open at a time. diff --git a/commands/history.md b/commands/history.md index a2688c9dd..1c62c36bd 100644 --- a/commands/history.md +++ b/commands/history.md @@ -263,7 +263,9 @@ gone is placed by the session that spawned it. Report all four populations: root SHA), `orphans`, and `failed`. An **orphan** — a transcript whose repository is not on this machine — is -ignored and reported, never guessed at. It is stored only when this repository +ignored and reported, never guessed at; its recorded `cwd` is shown +home-relative, or by its directory name when it sits outside HOME, as is the +destination the report leads with. It is stored only when this repository claims its project name, through `adopt_projects` in the configuration or `--adopt` for one run, and an adopted record carries `adopted_project` so the adoption is on the artefact. When the configuration sets `on_orphan` to diff --git a/commands/peers.md b/commands/peers.md index 4bf5bd080..8187f4171 100644 --- a/commands/peers.md +++ b/commands/peers.md @@ -26,8 +26,9 @@ Run: The payload carries `live` (the live-peer count), `ids` (the distinct records across every row), `default_ref` (the branch a peer is judged merged into), `sources` (the sources read: `worktree` and `branch`), `peers` and `skipped`. -Each peer names its `source`, `branch`, `path` (home-redacted to `~`) and its -`rows`; each row is an `id`, a `kind`, the `folder` that holds it in the peer +Each peer names its `source`, `branch`, `path` (home-relative, `~/…`, or the +worktree's directory name when it sits outside HOME, never an absolute path) +and its `rows`; each row is an `id`, a `kind`, the `folder` that holds it in the peer and, when the file could be read, a `title`: - `open-there` — an issue open in the peer and absent from every status folder diff --git a/docs/reference/cli/commands.md b/docs/reference/cli/commands.md index d504fc3d3..198b320c1 100644 --- a/docs/reference/cli/commands.md +++ b/docs/reference/cli/commands.md @@ -2268,8 +2268,9 @@ two status folders is named with the reason and not read; a gone or refused worktree's branch is then read from the object store instead. Strictly read-only: it writes nothing, takes no lock, and fetches nothing. -Home paths are redacted to ~ on every stream. Exit 0 whatever the peers -hold; exit 2 outside a git checkout. +A worktree is named home-relative (~/...), or by its directory name when it +sits outside HOME, on every stream. Exit 0 whatever the peers hold; exit 2 +outside a git checkout. ### `abcd reading` diff --git a/evals/coldreading_rehearsal_test.go b/evals/coldreading_rehearsal_test.go index 693c7af95..1d7c461d1 100644 --- a/evals/coldreading_rehearsal_test.go +++ b/evals/coldreading_rehearsal_test.go @@ -1391,9 +1391,9 @@ func disposition(t *testing.T, f fixture, item string, args ...string) dispositi // which is right and makes an item a single-use subject. Assembling another run // is what the operator would do, and it is cheap on this corpus. // placeComparativeRunRecord writes the committed run record of a comparative run -// over wideningRun into the durable run directory: the commit marker -// capture's ordering gate reads (ComparativeRunFor), carrying the -// candidate-join subset it decodes. +// over wideningRun into the durable run directory, with the manifest beside it: +// the commit marker capture's ordering gate reads (ComparativeRunFor), carrying +// the candidate-join subset it decodes. func placeComparativeRunRecord(t *testing.T, f fixture, wideningRun string) { t.Helper() const compRun = "rdg-2609259999999999" @@ -1401,9 +1401,14 @@ func placeComparativeRunRecord(t *testing.T, f fixture, wideningRun string) { if err := os.MkdirAll(dir, 0o755); err != nil { t.Fatal(err) } - if err := os.WriteFile(filepath.Join(dir, "run.json"), []byte(`{"run_id":"`+compRun+ - `","position":"comparative","candidate_run":"`+wideningRun+`"}`), 0o644); err != nil { - t.Fatal(err) + // The channel's pair: the manifest it promotes and the run record after it, + // agreeing on the run and the candidate join, which is what the gate holds a + // committed run to (iss-2609251842111593). + head := []byte(`{"run_id":"` + compRun + `","position":"comparative","candidate_run":"` + wideningRun + `"}`) + for _, name := range []string{"manifest.json", "run.json"} { + if err := os.WriteFile(filepath.Join(dir, name), head, 0o644); err != nil { + t.Fatal(err) + } } } diff --git a/internal/core/capture/admit_test.go b/internal/core/capture/admit_test.go index e1c52fa1d..59a140023 100644 --- a/internal/core/capture/admit_test.go +++ b/internal/core/capture/admit_test.go @@ -207,8 +207,10 @@ func TestAdmitRefusesBeforeTheComparativeRun(t *testing.T) { // gate exactly as a characterising run does. func TestAdmitProceedsOnAnEmptyComparativeRun(t *testing.T) { repo, ir, item := readingFixture(t, issueschema.PositionWidening) - writeFile(t, filepath.Join(repo, filepath.FromSlash(issueschema.ReadingsRecordDir), "rdg-2608300000000003", issueschema.RunRecordFileName), - `{"run_id":"rdg-2608300000000003","position":"comparative","candidate_run":"`+fixtureRun+`","candidates":1,"exercised":false,"records":[]}`) + dir := filepath.Join(repo, filepath.FromSlash(issueschema.ReadingsRecordDir), "rdg-2608300000000003") + empty := `{"run_id":"rdg-2608300000000003","position":"comparative","candidate_run":"` + fixtureRun + `","candidates":1,"exercised":false,"records":[]}` + writeFile(t, filepath.Join(dir, issueschema.RunManifestFileName), empty) + writeFile(t, filepath.Join(dir, issueschema.RunRecordFileName), empty) if _, err := Admit(AdmitRequest{RepoRoot: repo, IssuesRoot: ir, Item: item, Grounds: admitGround}); err != nil { t.Fatalf("Admit after an empty comparative run: %v", err) } diff --git a/internal/core/capture/deferral.go b/internal/core/capture/deferral.go index 66b6873ff..fdb2b653e 100644 --- a/internal/core/capture/deferral.go +++ b/internal/core/capture/deferral.go @@ -55,7 +55,8 @@ var deferrableSeverities = map[Severity]bool{SeverityMajor: true, SeverityCritic // record that is not open, and a grade the guard never blocks on. A record // already deferred past an earlier anchor is re-deferred: the pair is replaced // and a new body section is appended, so the history of each deferral stays in -// the record. +// the record. A record already deferred past the SAME anchor has the pair and +// that cycle's section replaced, so the body keeps one section per cycle. func Defer(req DeferRequest) (DeferResult, error) { repoRoot, issuesRoot, err := resolveRoots(req.RepoRoot, req.IssuesRoot) if err != nil { @@ -142,14 +143,43 @@ func Defer(req DeferRequest) (DeferResult, error) { return result, nil } -// appendDeferralSection appends one dated `## Deferral` section to the record, +// appendDeferralSection writes one dated `## Deferral` section into the record, // the body half of a deferral's shape: the frontmatter pair is what the cut // reads, and the section is what a reader of the record sees, one per cycle. +// +// One per cycle is what the section is, so a second deferral past the SAME +// anchor (a corrected reason, or the verb run twice) rewrites that cycle's +// section in place rather than appending a second one (iss-2609251823555125); +// the superseded wording stays in git's history, where every earlier revision of +// a record lives. A deferral past a different anchor appends, so each cycle's +// section stays in the record. func appendDeferralSection(content, date, after, reason string) string { + heading, line := "## Deferral "+date, "Deferred past "+after+": "+reason + if i := sameCycleDeferral(content, after); i >= 0 { + lines := strings.Split(content, "\n") + lines[i], lines[i+2] = heading, line + return strings.Join(lines, "\n") + } if !strings.HasSuffix(content, "\n") { content += "\n" } - return content + "\n## Deferral " + date + "\n\nDeferred past " + after + ": " + reason + "\n" + return content + "\n" + heading + "\n\n" + line + "\n" +} + +// sameCycleDeferral returns the line index of the last `## Deferral` heading +// whose section is the one this verb writes for anchor — the heading, a blank +// line, then `Deferred past : ` — or -1 when the record carries none. A +// section of any other shape is a hand edit the verb does not own, so it is left +// alone and the new section appended. +func sameCycleDeferral(content, anchor string) int { + lines := strings.Split(content, "\n") + for i := len(lines) - 3; i >= 0; i-- { + if strings.HasPrefix(lines[i], "## Deferral ") && lines[i+1] == "" && + strings.HasPrefix(lines[i+2], "Deferred past "+anchor+": ") { + return i + } + } + return -1 } // requireCurrentAnchor refuses a tag that is not the checkout's newest release diff --git a/internal/core/capture/deferral_test.go b/internal/core/capture/deferral_test.go index d837add41..b5e9ded65 100644 --- a/internal/core/capture/deferral_test.go +++ b/internal/core/capture/deferral_test.go @@ -15,7 +15,15 @@ import ( // name — holding one open major record and one open minor record. func deferralLedger(t *testing.T) (repo, ir, major, minor string) { t.Helper() - r := gittest.NewRepo(t) + _, repo, ir, major, minor = deferralLedgerRepo(t) + return repo, ir, major, minor +} + +// deferralLedgerRepo is deferralLedger with the repository handle, for a test +// that cuts a later tag. +func deferralLedgerRepo(t *testing.T) (r *gittest.Repo, repo, ir, major, minor string) { + t.Helper() + r = gittest.NewRepo(t) r.Commit("root") r.Git("tag", "v0.1.0") repo = r.Root() @@ -28,7 +36,7 @@ func deferralLedger(t *testing.T) (repo, ir, major, minor string) { } return res.ID } - return repo, ir, mk(SeverityMajor, "big"), mk(SeverityMinor, "small") + return r, repo, ir, mk(SeverityMajor, "big"), mk(SeverityMinor, "small") } // TestDeferWritesTheWaiverPairAndABodySection is iss-2609181223260994: the @@ -127,3 +135,50 @@ func TestADeferralTheVerbWritesIsOneTheCutHonours(t *testing.T) { t.Fatalf("the cut did not honour the verb's deferral: %+v", g) } } + +// TestReDeferringPastTheSameAnchorKeepsOneSection is iss-2609251823555125: the +// body carries one `## Deferral` section per cycle, so a second deferral past +// the SAME anchor — a corrected reason, or the verb run twice — replaces that +// cycle's section rather than appending a second one. A deferral past a later +// anchor still appends, so each cycle's history stays in the record. +func TestReDeferringPastTheSameAnchorKeepsOneSection(t *testing.T) { + r, repo, ir, major, _ := deferralLedgerRepo(t) + deferralNow = func() time.Time { return time.Date(2026, 9, 25, 10, 0, 0, 0, time.UTC) } + t.Cleanup(func() { deferralNow = time.Now }) + if _, err := Defer(DeferRequest{RepoRoot: repo, IssuesRoot: ir, ID: major, After: "v0.1.0", Reason: "the first reason"}); err != nil { + t.Fatal(err) + } + deferralNow = func() time.Time { return time.Date(2026, 9, 26, 10, 0, 0, 0, time.UTC) } + if _, err := Defer(DeferRequest{RepoRoot: repo, IssuesRoot: ir, ID: major, After: "v0.1.0", Reason: "the corrected reason"}); err != nil { + t.Fatal(err) + } + raw := readRaw(t, ir, major) + if n := strings.Count(raw, "\n## Deferral "); n != 1 { + t.Fatalf("two deferrals past one anchor left %d `## Deferral` sections, want 1:\n%s", n, raw) + } + for _, want := range []string{ + "\ndeferral_reason: \"the corrected reason\"\n", + "\n## Deferral 2026-09-26\n\nDeferred past v0.1.0: the corrected reason\n", + } { + if !strings.Contains(raw, want) { + t.Errorf("the re-deferred record lacks %q:\n%s", want, raw) + } + } + if strings.Contains(raw, "the first reason") { + t.Errorf("the superseded reason for the same cycle survived:\n%s", raw) + } + + // The next cycle appends: its section is new history, not a correction. + r.Commit("next") + r.Git("tag", "v0.2.0") + if _, err := Defer(DeferRequest{RepoRoot: repo, IssuesRoot: ir, ID: major, After: "v0.2.0", Reason: "the next cycle's reason"}); err != nil { + t.Fatal(err) + } + raw = readRaw(t, ir, major) + if n := strings.Count(raw, "\n## Deferral "); n != 2 { + t.Fatalf("a deferral past a later anchor left %d sections, want 2 (one per cycle):\n%s", n, raw) + } + if !strings.Contains(raw, "Deferred past v0.1.0: the corrected reason\n") || !strings.Contains(raw, "Deferred past v0.2.0: the next cycle's reason\n") { + t.Errorf("each cycle's section must survive:\n%s", raw) + } +} diff --git a/internal/core/capture/hiddenrunes_test.go b/internal/core/capture/hiddenrunes_test.go index f03387832..bb9918dea 100644 --- a/internal/core/capture/hiddenrunes_test.go +++ b/internal/core/capture/hiddenrunes_test.go @@ -2,8 +2,12 @@ package capture import ( "errors" + "os" + "path/filepath" "strings" "testing" + + "github.com/intentdriven/abcd/internal/core/issueschema" ) // hiddenRunes is one of each class the record-write boundary must encode: a @@ -108,3 +112,32 @@ func TestWontfixDerivedGroundsRefuseAControlCharacter(t *testing.T) { t.Fatalf("a terse wontfix reason must still be accepted: %v", err) } } + +// TestDispositionGroundsAndExitConditionEncodeHiddenRunes: `capture disposition` +// writes two free-text values into a committed record, and both are held to the +// same boundary every other capture write is: a bidi override or a zero-width +// rune in --grounds or --exit-condition is encoded, never written verbatim +// (iss-2609251823551349). +func TestDispositionGroundsAndExitConditionEncodeHiddenRunes(t *testing.T) { + for _, tc := range []struct { + name string + req DispositionRequest + }{ + {"grounds", DispositionRequest{State: issueschema.DispositionAccepted, Grounds: hiddenText("the grounds")}}, + {"exit condition", DispositionRequest{State: issueschema.DispositionHeld, ExitCondition: hiddenText("the exit")}}, + } { + t.Run(tc.name, func(t *testing.T) { + repo, ir, item := readingFixture(t, "detection") + tc.req.RepoRoot, tc.req.IssuesRoot, tc.req.Item = repo, ir, item + res, err := Disposition(tc.req) + if err != nil { + t.Fatal(err) + } + raw, err := os.ReadFile(filepath.Join(repo, filepath.FromSlash(res.Path))) + if err != nil { + t.Fatal(err) + } + assertNoHiddenRune(t, "disposition "+tc.name, string(raw)) + }) + } +} diff --git a/internal/core/capture/itemfate.go b/internal/core/capture/itemfate.go index 587b34748..d6d67609d 100644 --- a/internal/core/capture/itemfate.go +++ b/internal/core/capture/itemfate.go @@ -138,6 +138,18 @@ func admissionsNaming(issuesRoot, run, item string) ([]string, error) { // run's commit marker lives: a directory holding a parked manifest and no run // record is a run that never happened, and it answers nothing here. // +// A committed run is the channel's PAIR, not a run record alone +// (iss-2609251842111593). The ingest promotes the run's manifest into its +// durable directory before it writes the run record, and both carry the run id +// and the candidate join, so a run record that names this widening run is held +// to them: its run_id names its own directory, and the manifest beside it names +// the same run, the comparative position and the same candidate_run. A record +// that names the widening run and fails either is refused by name, as a record +// contradicting itself, never read as "no run yet" — the id-less, manifest-less +// two-key marker a session could write by hand is exactly that record. Whether +// the pair is tracked by git is not asked: the gate answers between an ingest +// and the commit that carries it, which is the order the channel is used in. +// // The LOWEST match rather than any match, so two comparative runs over one // widening run — a legitimate state, since a second comparative run before any // disposition is a second run — resolve to one answer that does not depend on @@ -171,7 +183,7 @@ func ComparativeRunFor(repoRoot, run string) (string, error) { if err := refuseSymlinkedDir(dir); err != nil { return "", err } - head, ok, err := readRunHead(filepath.Join(dir, issueschema.RunRecordFileName)) + head, ok, err := readRunHead(filepath.Join(dir, issueschema.RunRecordFileName), issueschema.RecordReadLimit) if err != nil { return "", err } @@ -179,12 +191,42 @@ func ComparativeRunFor(repoRoot, run string) (string, error) { continue } if head.Position == PositionComparative && head.CandidateRun == run { + if err := requireChannelPair(dir, name, head); err != nil { + return "", err + } return name, nil } } return "", nil } +// requireChannelPair holds a comparative run record that names a widening run +// to the shape the channel's ingest leaves: its run_id is its directory's name, +// and the manifest beside it agrees on the run, the position and the candidate +// join. A missing or disagreeing half is a fault named with the directory, so a +// hand-placed marker is refused out loud rather than opening the gate. +func requireChannelPair(dir, name string, head issueschema.RunHead) error { + record := filepath.Join(dir, issueschema.RunRecordFileName) + if head.RunID != name { + return fmt.Errorf("%w: the comparative run record %s declares run_id %q but is filed under %s; the channel writes a run's record into its own directory, so this record contradicts itself and does not characterise %s", + ErrInvariantViolation, record, head.RunID, name, head.CandidateRun) + } + manifest := filepath.Join(dir, issueschema.RunManifestFileName) + m, ok, err := readRunHead(manifest, issueschema.RunArtefactReadLimit) + if err != nil { + return err + } + if !ok { + return fmt.Errorf("%w: the comparative run %s has a run record naming %s but no %s beside it; the channel's ingest writes the manifest before the record, so this run was not committed by it and does not characterise %s", + ErrInvariantViolation, name, head.CandidateRun, issueschema.RunManifestFileName, head.CandidateRun) + } + if m.RunID != head.RunID || m.Position != head.Position || m.CandidateRun != head.CandidateRun { + return fmt.Errorf("%w: the comparative run %s's manifest (run_id %q, position %q, candidate_run %q) disagrees with its run record (run_id %q, position %q, candidate_run %q), so it does not characterise %s", + ErrInvariantViolation, name, m.RunID, m.Position, m.CandidateRun, head.RunID, head.Position, head.CandidateRun, head.CandidateRun) + } + return nil +} + // PositionComparative is the comparative position's token, as the run record // spells it. It is stated here rather than imported because core/reading imports // this package and not the other way round; issueschema.ReadingPositions is the @@ -193,8 +235,8 @@ const PositionComparative = "comparative" // readRunHead decodes one run record's candidate-join subset. A missing file is // "not a committed run", not a fault: the marker's absence is the state. -func readRunHead(path string) (issueschema.RunHead, bool, error) { - raw, err := fsutil.ReadGuarded(path, issueschema.RecordReadLimit) +func readRunHead(path string, limit int64) (issueschema.RunHead, bool, error) { + raw, err := fsutil.ReadGuarded(path, limit) if err != nil { if os.IsNotExist(err) { return issueschema.RunHead{}, false, nil diff --git a/internal/core/capture/itemfate_test.go b/internal/core/capture/itemfate_test.go index 289b030a2..f2f55957d 100644 --- a/internal/core/capture/itemfate_test.go +++ b/internal/core/capture/itemfate_test.go @@ -1,6 +1,7 @@ package capture import ( + "errors" "os" "path/filepath" "slices" @@ -141,10 +142,8 @@ func TestComparativeRunForNamesTheLowestMatch(t *testing.T) { // Two comparative runs over the same widening run, plus a comparative run // over another and a widening run of its own. The lowest match is what comes // back, so the answer does not depend on directory order. - writeFile(t, filepath.Join(runs, "rdg-2608300000000009", issueschema.RunRecordFileName), - `{"run_id":"rdg-2608300000000009","position":"comparative","candidate_run":"`+widening+`"}`) - writeFile(t, filepath.Join(runs, "rdg-2608300000000005", issueschema.RunRecordFileName), - `{"run_id":"rdg-2608300000000005","position":"comparative","candidate_run":"`+widening+`"}`) + writeComparativeRun(t, repo, "rdg-2608300000000009", widening) + writeComparativeRun(t, repo, "rdg-2608300000000005", widening) writeFile(t, filepath.Join(runs, "rdg-2608300000000007", issueschema.RunRecordFileName), `{"run_id":"rdg-2608300000000007","position":"comparative","candidate_run":"rdg-2608300000000002"}`) writeFile(t, filepath.Join(runs, widening, issueschema.RunRecordFileName), @@ -208,3 +207,51 @@ func TestIngestReadingCommitsARunWithNoItems(t *testing.T) { t.Errorf("the result names run %q", res.Run) } } + +// TestComparativeRunForRefusesAMarkerTheChannelDidNotWrite is +// iss-2609251842111593: the gate read "committed comparative run" as any +// run.json decoding to position comparative with a candidate_run, so an id-less, +// manifest-less two-key marker written by hand opened it. The channel's ingest +// writes the run's manifest into the run directory before the run record, and +// both carry the run id and the candidate join, so a run that satisfies the gate +// is one whose record names its own directory and whose manifest agrees with it. +// A marker that names the widening run and fails either check is a record +// contradicting itself, refused by name rather than read as "no run yet". +func TestComparativeRunForRefusesAMarkerTheChannelDidNotWrite(t *testing.T) { + const widening, comp = "rdg-2608300000000001", "rdg-2608300000000005" + record := `{"run_id":"` + comp + `","position":"comparative","candidate_run":"` + widening + `"}` + manifest := `{"run_id":"` + comp + `","position":"comparative","candidate_run":"` + widening + `"}` + for _, tc := range []struct { + name, record, manifest string + }{ + {"an id-less two-key marker", `{"position":"comparative","candidate_run":"` + widening + `"}`, manifest}, + {"a run id naming another directory", `{"run_id":"rdg-2608300000000006","position":"comparative","candidate_run":"` + widening + `"}`, manifest}, + {"no manifest beside the record", record, ""}, + {"a manifest naming another candidate", record, `{"run_id":"` + comp + `","position":"comparative","candidate_run":"rdg-2608300000000002"}`}, + {"a manifest at another position", record, `{"run_id":"` + comp + `","position":"widening","candidate_run":"` + widening + `"}`}, + {"a manifest naming another run", record, `{"run_id":"rdg-2608300000000006","position":"comparative","candidate_run":"` + widening + `"}`}, + } { + t.Run(tc.name, func(t *testing.T) { + repo, _ := ledger(t) + dir := filepath.Join(repo, filepath.FromSlash(issueschema.ReadingsRecordDir), comp) + writeFile(t, filepath.Join(dir, issueschema.RunRecordFileName), tc.record) + if tc.manifest != "" { + writeFile(t, filepath.Join(dir, issueschema.RunManifestFileName), tc.manifest) + } + got, err := ComparativeRunFor(repo, widening) + if got != "" { + t.Fatalf("a marker the channel did not write satisfied the gate as %q", got) + } + if !errors.Is(err, ErrInvariantViolation) || !strings.Contains(err.Error(), comp) { + t.Fatalf("err = %v, want ErrInvariantViolation naming %s", err, comp) + } + }) + } + + // The channel's own pair satisfies it. + repo, _ := ledger(t) + writeComparativeRun(t, repo, comp, widening) + if got, err := ComparativeRunFor(repo, widening); err != nil || got != comp { + t.Fatalf("ComparativeRunFor over the channel's pair = %q, %v; want %s", got, err, comp) + } +} diff --git a/internal/core/capture/ledgerroot_test.go b/internal/core/capture/ledgerroot_test.go index 2c9d01f31..b24eadb4b 100644 --- a/internal/core/capture/ledgerroot_test.go +++ b/internal/core/capture/ledgerroot_test.go @@ -1,6 +1,7 @@ package capture import ( + "errors" "os" "path/filepath" "strings" @@ -62,6 +63,12 @@ func TestLedgerMkdirCannotBeRedirectedOutsideTheCheckout(t *testing.T) { if err == nil { t.Fatal("a capture whose ledger ancestor was swapped for an outside symlink succeeded") } + // The refusal is the ledger's own path-unsafe sentinel, not merely an error: + // mapEscape classifies os.Root's escape by its message, and this pins the + // match against the Go release in use (iss-2609251823559111). + if !errors.Is(err, ErrPathUnsafe) { + t.Fatalf("the swapped-ancestor mkdir was refused with %v, want ErrPathUnsafe", err) + } if entries, _ := os.ReadDir(outside); len(entries) != 0 { t.Fatalf("the ledger walk created %d entr(ies) outside the checkout: %v", len(entries), entries) } @@ -97,9 +104,33 @@ func TestLedgerRecordWriteCannotBeRedirectedOutsideTheCheckout(t *testing.T) { if err == nil { t.Fatal("a capture whose ledger ancestor was swapped at the write succeeded") } + if !errors.Is(err, ErrPathUnsafe) { + t.Fatalf("the swapped-ancestor write was refused with %v, want ErrPathUnsafe", err) + } for _, f := range walkFiles(t, outside) { if strings.HasSuffix(f, ".md") || strings.Contains(filepath.Base(f), "abcd-tmp") { t.Fatalf("the record write landed outside the checkout: %s", f) } } } + +// TestMapEscapeClassifiesTheRealOSRootEscape pins mapEscape's message match to +// the error os.Root actually returns on the Go release in use. The os package +// does not export its escape error, so the match is by text; a release that +// rewords it would still refuse the write but degrade ErrPathUnsafe to a +// generic error, and this test is what notices (iss-2609251823559111). +func TestMapEscapeClassifiesTheRealOSRootEscape(t *testing.T) { + base := t.TempDir() + root, err := os.OpenRoot(base) + if err != nil { + t.Fatal(err) + } + defer root.Close() + _, escErr := root.Open("../outside") + if escErr == nil { + t.Fatal("os.Root opened a path outside its root") + } + if got := mapEscape(escErr, filepath.Join(base, "..", "outside")); !errors.Is(got, ErrPathUnsafe) { + t.Fatalf("mapEscape(%v) = %v, want ErrPathUnsafe", escErr, got) + } +} diff --git a/internal/core/capture/reading.go b/internal/core/capture/reading.go index 481b6af4c..c965f6a87 100644 --- a/internal/core/capture/reading.go +++ b/internal/core/capture/reading.go @@ -32,6 +32,7 @@ import ( "github.com/intentdriven/abcd/internal/core/readingitem" "github.com/intentdriven/abcd/internal/core/recordid" "github.com/intentdriven/abcd/internal/fsutil" + "github.com/intentdriven/abcd/internal/termsafe" ) // The disposition family's id grammar. It is checked BEFORE the value is used to @@ -383,7 +384,10 @@ func readItemHead(issuesRoot, item string) (itemHead, error) { } // redactDispositionRequest redacts every free-text value a disposition carries, -// with one scanner, before the lock is taken. +// with one scanner, before the lock is taken, and encodes the hidden runes in it +// the way every other free-text record write does: a bidi override or a +// zero-width rune in --grounds or --exit-condition would otherwise reach the +// committed record verbatim (iss-2609251823551349). func redactDispositionRequest(repoRoot string, req DispositionRequest) (DispositionRequest, int, string) { r := newLedgerRedactor(repoRoot) total := 0 @@ -393,7 +397,7 @@ func redactDispositionRequest(repoRoot string, req DispositionRequest) (Disposit } out, n := r.redact(s) total += n - return out + return termsafe.EncodeHiddenRunes(out) } req.Grounds = scrub(req.Grounds) req.ExitCondition = scrub(req.ExitCondition) diff --git a/internal/core/capture/reading_test.go b/internal/core/capture/reading_test.go index 43ae82720..3ff8e324b 100644 --- a/internal/core/capture/reading_test.go +++ b/internal/core/capture/reading_test.go @@ -555,8 +555,19 @@ const fixtureRun = "rdg-2608300000000001" // because the gate only READS the channel's output (spc-2609020626040342, Out). func commitComparativeRun(t *testing.T, repo, compRun, candidateRun string) { t.Helper() - writeFile(t, filepath.Join(repo, filepath.FromSlash(issueschema.ReadingsRecordDir), compRun, issueschema.RunRecordFileName), - `{"run_id":"`+compRun+`","position":"comparative","candidate_run":"`+candidateRun+`"}`) + writeComparativeRun(t, repo, compRun, candidateRun) +} + +// writeComparativeRun writes the pair the comparative channel's ingest leaves in +// a committed run's directory — the manifest it promotes and the run record +// after it — agreeing on the run id, the position and the candidate join, which +// is what ComparativeRunFor holds a committed run to (iss-2609251842111593). +func writeComparativeRun(t *testing.T, repo, compRun, candidateRun string) { + t.Helper() + dir := filepath.Join(repo, filepath.FromSlash(issueschema.ReadingsRecordDir), compRun) + head := `{"run_id":"` + compRun + `","position":"comparative","candidate_run":"` + candidateRun + `"}` + writeFile(t, filepath.Join(dir, issueschema.RunManifestFileName), head) + writeFile(t, filepath.Join(dir, issueschema.RunRecordFileName), head) } // dispositionFiles lists every file under the dispositions tree, for a test that diff --git a/internal/core/capture/reframe_test.go b/internal/core/capture/reframe_test.go index 59325986e..4b3c836a9 100644 --- a/internal/core/capture/reframe_test.go +++ b/internal/core/capture/reframe_test.go @@ -441,6 +441,33 @@ func TestReframeWholeWriteAcrossAMergeIsTopological(t *testing.T) { } } +// TestReframeWholeWriteOfALinearSeriesRecordsItsLastStep pins what the command +// page states about a rewrite that lands as a series of commits on the first +// parent line, a rebased branch among them (iss-2609261325441711): the whole +// write reads back to the previous DISTINCT state, which is the state before the +// series' last step, so it records that step alone. Two commits that move the +// construal and then the glossary give changed=[glossary] with before at the +// post-construal state, where a squash or a --no-ff merge of the same rewrite +// gives [construal glossary]. The route that records the whole series is --open +// before its first commit and --complete after its last +// (TestCompleteCrossesATwoCommitRewrite). +func TestReframeWholeWriteOfALinearSeriesRecordsItsLastStep(t *testing.T) { + r := reframeFixture(t) + rewriteConstrual(r, "First step of a rebased rewrite.") + mid := frameAt(t, r) + r.Write(fxGlossary+"/core/term.md", "# Term\n\nSecond step of a rebased rewrite.\n") + r.Commit("rewrite a term") + after := frameAt(t, r) + + res, err := Reframe(reframeReq(r, fxItem)) + if err != nil { + t.Fatalf("Reframe: %v", err) + } + if res.Before != mid || res.After != after || res.Commits != 1 || !slices.Equal(res.Changed, []string{"glossary"}) { + t.Fatalf("result = %+v\nwant before at the post-construal state, [glossary] across 1 commit", res) + } +} + func TestReframeRefusesUncommittedChangesWithoutOpen(t *testing.T) { r := reframeFixture(t) rewriteConstrual(r, "A committed rewrite.") diff --git a/internal/core/capture/serialize.go b/internal/core/capture/serialize.go index 0876edbdf..4ae89a1c6 100644 --- a/internal/core/capture/serialize.go +++ b/internal/core/capture/serialize.go @@ -344,16 +344,9 @@ func frontmatterBounds(lines []string) (openIdx, closeIdx int, err error) { if openIdx == -1 { return -1, -1, fmt.Errorf("%w: content has no frontmatter block", ErrMalformedFrontmatter) } - for j := openIdx + 1; j < len(lines); j++ { - ln := lines[j] - if strings.HasPrefix(ln, " ") || strings.HasPrefix(ln, "\t") { - continue - } - if frontmatter.IsDelimiter(ln) { - closeIdx = j - break - } - } + // The close is frontmatter.CloseAfter's, the one closing walk + // (iss-2608270908348042). + closeIdx = frontmatter.CloseAfter(lines, openIdx) if closeIdx == -1 { return -1, -1, fmt.Errorf("%w: frontmatter not terminated", ErrMalformedFrontmatter) } diff --git a/internal/core/changelog/bom_test.go b/internal/core/changelog/bom_test.go new file mode 100644 index 000000000..df697bb4d --- /dev/null +++ b/internal/core/changelog/bom_test.go @@ -0,0 +1,14 @@ +package changelog + +import "testing" + +// TestSummariseReadsPastABOM: the changelog's body is the body frontmatter.Fields +// leaves, so a BOM-led record's frontmatter never reaches the changelog as its +// summary (iss-2608221126066379). +func TestSummariseReadsPastABOM(t *testing.T) { + t.Parallel() + title, summary := summarise("\ufeff---\nid: iss-1\nslug: the-slug\n---\n\nThe body paragraph.\n", "iss-1") + if title != "the-slug" || summary != "The body paragraph." { + t.Fatalf("summarise = (%q, %q), want (\"the-slug\", \"The body paragraph.\")", title, summary) + } +} diff --git a/internal/core/changelog/source.go b/internal/core/changelog/source.go index 05ba9dace..dcbec20a5 100644 --- a/internal/core/changelog/source.go +++ b/internal/core/changelog/source.go @@ -87,19 +87,13 @@ func pressReleaseSection(blob string) string { } // bodyStart returns the index of the first line after the frontmatter block, or -// 0 when the document has none. The block is delimited by the first TWO `---` -// lines, exactly as internal/core/frontmatter reads it, so the two never -// disagree about where the body begins. +// 0 when the document has none. The block is frontmatter.Close's, the one walk +// frontmatter.Fields makes, so the two never disagree about where the body +// begins — a private copy skipped the BOM Fields trims and handed a BOM-led +// record's whole frontmatter to the changelog as its body +// (iss-2608221126066379). func bodyStart(lines []string) int { - if len(lines) == 0 || strings.TrimRight(lines[0], " \t\r") != "---" { - return 0 - } - for i := 1; i < len(lines); i++ { - if strings.TrimRight(lines[i], " \t\r") == "---" { - return i + 1 - } - } - return 0 + return frontmatter.Close(lines) + 1 } // firstHeading returns the text of the first level-one heading in body, or "". diff --git a/internal/core/frontmatter/close_test.go b/internal/core/frontmatter/close_test.go new file mode 100644 index 000000000..11d839c70 --- /dev/null +++ b/internal/core/frontmatter/close_test.go @@ -0,0 +1,78 @@ +package frontmatter + +import ( + "strings" + "testing" +) + +// TestCloseFindsTheBlockFieldsReads: Close is the one answer to "where does the +// leading block close", on Fields' terms exactly — a BOM tolerated at line 0 and +// nowhere else, a delimiter by IsDelimiter, an unclosed block no block at all — +// so a sibling reader that asks it cannot disagree with the reader about where +// the body begins (iss-2608221126066379). +func TestCloseFindsTheBlockFieldsReads(t *testing.T) { + t.Parallel() + cases := []struct { + name string + doc string + want int + }{ + {"plain", "---\nid: a\n---\nbody\n", 2}, + {"BOM ahead of the opening delimiter", "\ufeff---\nid: a\n---\nbody\n", 2}, + {"trailing whitespace and CRLF on the delimiters", "--- \r\nid: a\r\n---\t\r\nbody\r\n", 2}, + {"no opening delimiter", "id: a\n---\nbody\n", -1}, + {"unclosed", "---\nid: a\nbody\n", -1}, + {"a mid-file ZWNBSP line is not a close", "---\nid: a\n\ufeff---\nb: c\n---\n", 4}, + {"an indented rule is not a close", "---\nid: a\n ---\n---\n", 3}, + {"empty", "", -1}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + lines := strings.Split(tc.doc, "\n") + got := Close(lines) + if got != tc.want { + t.Fatalf("Close = %d, want %d", got, tc.want) + } + // Agreement with Fields: a block Close finds is one Fields reads. + if fields := Fields(lines); (got >= 0) != (len(fields) > 0) { + t.Fatalf("Close = %d but Fields read %d field(s)", got, len(fields)) + } + }) + } +} + +// TestCloseAfterJudgesTheCloseAsCloseDoes: a reader that located the opening +// delimiter itself (past an attribution comment) asks CloseAfter for the close, +// and gets Close's answer about which lines close a block — IsDelimiter's, so an +// indented rule and a mid-file ZWNBSP rule are body lines +// (iss-2608270908348042). +func TestCloseAfterJudgesTheCloseAsCloseDoes(t *testing.T) { + t.Parallel() + cases := []struct { + name string + doc string + open int + want int + }{ + {"opening past a comment", "\n---\nid: a\n---\nbody\n", 1, 3}, + {"an indented rule is not a close", "\n---\nid: a\n ---\n---\n", 1, 4}, + {"a mid-file ZWNBSP rule is not a close", "\n---\nid: a\n\ufeff---\n---\n", 1, 4}, + {"trailing whitespace and CRLF", "\r\n---\r\nid: a\r\n--- \r\n", 1, 3}, + {"unclosed", "\n---\nid: a\n", 1, -1}, + {"no opening", "body\n", -1, -1}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + lines := strings.Split(tc.doc, "\n") + if got := CloseAfter(lines, tc.open); got != tc.want { + t.Fatalf("CloseAfter = %d, want %d", got, tc.want) + } + // With line ends kept, the answer is the same. + if got := CloseAfter(strings.SplitAfter(tc.doc, "\n"), tc.open); got != tc.want { + t.Fatalf("CloseAfter (ends kept) = %d, want %d", got, tc.want) + } + }) + } +} diff --git a/internal/core/frontmatter/delimiter_canonical_test.go b/internal/core/frontmatter/delimiter_canonical_test.go new file mode 100644 index 000000000..10f82a761 --- /dev/null +++ b/internal/core/frontmatter/delimiter_canonical_test.go @@ -0,0 +1,162 @@ +package frontmatter + +import ( + "fmt" + "go/scanner" + "go/token" + "os" + "path/filepath" + "sort" + "strconv" + "strings" + "testing" +) + +// delimiterSite is one allowlisted file: how many string literals spelling a +// `---` delimiter it holds, and why none of them is a second delimiter rule. +type delimiterSite struct { + count int + reason string +} + +// delimiterSites names every non-test Go file outside this package whose string +// literals spell a `---` delimiter at a line start, with the number of such +// literals and the reason each is not a private compare. The count is pinned so +// a compare added to a file already on the list is a new claim too. The default +// for a file this test names is to route it through IsDelimiter, Close or +// CloseAfter. +var delimiterSites = map[string]delimiterSite{ + // Writers: they emit a delimiter and judge none. + "internal/core/capture/serialize.go": {2, "a WRITER: buildIssueText emits the block's two delimiters; the reader side is frontmatterBounds, which asks IsDelimiter and CloseAfter"}, + "internal/core/decide/decide.go": {2, "a WRITER: the ADR skeleton's two delimiters"}, + "internal/core/intent/create.go": {2, "a WRITER: the minted intent's two delimiters"}, + "internal/core/intent/consistency.go": {2, "a WRITER: the review record's two delimiters"}, + "internal/core/spec/spec.go": {2, "a WRITER: the minted spec's two delimiters"}, + "internal/core/report/report.go": {4, "a WRITER: two report templates' delimiters; the report reader judges by IsDelimiter"}, + "internal/core/source/add.go": {2, "a WRITER: a source entry's two delimiters"}, + "internal/core/lab/record.go": {2, "a WRITER: a probe record's two delimiters"}, + "internal/core/lab/mint.go": {1, "a WRITER: the lab entry's block, both delimiters in one format string"}, + "internal/core/memory/schema.go": {2, "a WRITER: rebuilds a region as a block to hand parseFrontmatter; it judges no delimiter"}, + "internal/gittest/repo.go": {2, "a WRITER: a test-fixture record's block's two delimiters"}, + "internal/surface/cli/history.go": {1, "a WRITER: a separator line between rendered transcripts; not frontmatter"}, + "internal/core/positioning/render.go": {1, "a WRITER: a unified diff's `--- a/` header; not frontmatter"}, + "internal/core/lint/subverbs.go": {1, "a markdown TABLE's separator row (`|---|`), not a frontmatter delimiter"}, + "internal/core/implement/loop/brief.go": {1, "a WRITER: the lane brief's thematic break above the intent section, in the body; not frontmatter"}, + // Deliberate, documented differences. + "internal/core/memory/yaml.go": {3, "the memory store's opener tolerates an indented delimiter (documented at frontmatterOpenIndex and textOpensFrontmatter); joinFileFrontmatter WRITES the block's two delimiters; every close is IsDelimiter"}, + "internal/core/memory/writer.go": {3, "a WRITER rebuilding a region for parseFrontmatter, and a byte-0 test that leaves a page with a tolerated preamble alone because rebuilding it would drop the preamble"}, + "internal/core/history/store.go": {6, "the transcript store's own record format, written by marshalRecord and read back byte-exact: a record this store did not write is refused, which is the point"}, + "internal/core/reading/project.go": {3, "the reading exclusion floor: it OPENS a block on any line beginning with three dashes, more broadly than IsDelimiter, and scans its keys and shapes to the LATER of its own prefix close (or `...`) and frontmatter.Close (blockScanEnd), so every line the canonical reader reads as frontmatter is scanned; its body scans start at the earlier close, so every line a renderer shows is scanned too"}, +} + +// TestNoPrivateDelimiterCompare is the one-canonical-primitive detector for the +// frontmatter delimiter rule, the counterpart of mdrecord's fence detector. +// +// IsDelimiter is the one delimiter rule and Close/CloseAfter the one closing +// walk. Private compares disagreed with them — a TrimSpace compare in the gates +// closed a block on an indented rule the reader reads as body, a bare prefix test +// in the site opened one on `----` — and a gate that disagrees with its reader +// about where the block ends passes the record the reader refuses +// (iss-2608270908348042). +// +// The check reads string LITERALS through go/scanner, so a comment quoting a +// delimiter is not a claim; a literal counts when its value opens with `---` or +// carries one at the start of a later line, a byte-order mark ahead of it or +// not. A delimiter assembled at run time is outside its reach, and is left to +// review. +func TestNoPrivateDelimiterCompare(t *testing.T) { + root := filepath.Join("..", "..", "..") // internal/core/frontmatter -> repository root + var offenders []string + seen := map[string]bool{} + for _, dir := range []string{"internal", "cmd"} { + err := filepath.WalkDir(filepath.Join(root, dir), func(path string, d os.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() { + if path == filepath.Join(root, "internal", "core", "frontmatter") { + return filepath.SkipDir + } + return nil + } + if !strings.HasSuffix(d.Name(), ".go") || strings.HasSuffix(d.Name(), "_test.go") { + return nil + } + data, err := os.ReadFile(path) + if err != nil { + return err + } + n := delimiterLiterals(data) + if n == 0 { + return nil + } + rel, _ := filepath.Rel(root, path) + rel = filepath.ToSlash(rel) + seen[rel] = true + s, ok := delimiterSites[rel] + switch { + case !ok: + offenders = append(offenders, fmt.Sprintf("%s (spells %d `---` delimiter literal(s) and is not allowlisted)", rel, n)) + case s.count != n: + offenders = append(offenders, fmt.Sprintf("%s (spells %d `---` delimiter literal(s); the allowlist names %d)", rel, n, s.count)) + } + return nil + }) + if err != nil { + t.Fatalf("walk %s: %v", dir, err) + } + } + if len(offenders) > 0 { + sort.Strings(offenders) + t.Errorf("`---` delimiter literals outside frontmatter (route through IsDelimiter, Close or CloseAfter, or allowlist with a reason and the count):\n %s", + strings.Join(offenders, "\n ")) + } + for rel := range delimiterSites { + if !seen[rel] { + t.Errorf("%s is allowlisted but no longer spells a `---` delimiter literal; remove the entry", rel) + } + } +} + +// delimiterLiterals counts the Go string literals in src whose value opens with +// `---` or carries `---` at the start of a later line, either one optionally led +// by a byte-order mark. +func delimiterLiterals(src []byte) int { + fset := token.NewFileSet() + file := fset.AddFile("", fset.Base(), len(src)) + var s scanner.Scanner + s.Init(file, src, nil, 0) + n := 0 + for { + _, tok, lit := s.Scan() + if tok == token.EOF { + return n + } + if tok != token.STRING { + continue + } + v, err := strconv.Unquote(lit) + if err != nil { + continue + } + if strings.HasPrefix(v, "---") || strings.Contains(v, "\n---") || + strings.HasPrefix(v, "\ufeff---") || strings.Contains(v, "\n\ufeff---") { + n++ + } + } +} + +// TestDelimiterLiteralsReadsLiteralsNotComments pins the counter the detector +// stands on: a compare in code counts, a delimiter quoted in a comment does not, +// a raw string spelling a block counts once, and a delimiter led by a byte-order +// mark counts — a private compare against "\ufeff---" is a second opener rule +// that TrimBOM exists to make unnecessary, and a counter blind to it let one be +// substituted for an allowlisted literal with the pinned count unchanged. +func TestDelimiterLiteralsReadsLiteralsNotComments(t *testing.T) { + t.Parallel() + src := "package p\n// a comment quoting \"---\" is not a claim\nvar a = x == \"---\"\nvar b = `id: a\n---\n`\nvar c = \"-- \"\n" + + "var d = x == \"\\ufeff---\"\nvar e = \"id: a\\n\\ufeff---\"\n" + if got := delimiterLiterals([]byte(src)); got != 4 { + t.Fatalf("delimiterLiterals = %d, want 4", got) + } +} diff --git a/internal/core/frontmatter/frontmatter.go b/internal/core/frontmatter/frontmatter.go index 13e41cbdc..1a0ca5074 100644 --- a/internal/core/frontmatter/frontmatter.go +++ b/internal/core/frontmatter/frontmatter.go @@ -115,6 +115,44 @@ func Fields(lines []string) map[string]Field { return fields } +// Close returns the index in lines of the leading frontmatter block's closing +// delimiter, or -1 when there is no block: no opening delimiter on line 0, or +// nothing closing it. It reads the block exactly as Fields does — the BOM +// trimmed at line 0 and nowhere else, every delimiter judged by IsDelimiter — +// so a reader that needs the block's extent rather than its keys (a writer +// splicing a key in, a reader taking the body after it) asks here instead of +// re-deriving the walk. Private copies of this walk skipped the BOM Fields +// trims, so a BOM-led record the reader accepted was refused by intent's +// writers and had its whole frontmatter taken for body by the changelog +// (iss-2608221126066379). +func Close(lines []string) int { + if len(lines) == 0 || !IsDelimiter(TrimBOM(lines[0])) { + return -1 + } + return CloseAfter(lines, 0) +} + +// CloseAfter returns the index of the first delimiter after the opening one at +// lines[open], or -1 when nothing closes the block. It is Close's walk for a +// reader that has located the opening delimiter itself — record-lint and the +// glossary admit an attribution comment above it, so their block need not open +// at line 0 — and it judges every closing line by IsDelimiter exactly as Close +// does: an indented ` ---` and a mid-file "\ufeff---" are body lines, never a +// close. Whether lines[open] opens a block is the caller's question. +// +// The lines may carry their end-of-line bytes or not; IsDelimiter trims both. +func CloseAfter(lines []string, open int) int { + if open < 0 { + return -1 + } + for i := open + 1; i < len(lines); i++ { + if IsDelimiter(lines[i]) { + return i + } + } + return -1 +} + // StripComment removes a trailing YAML comment from the text that follows a // key's colon, returning the value alone. // diff --git a/internal/core/glossary/delimiter_rule_test.go b/internal/core/glossary/delimiter_rule_test.go new file mode 100644 index 000000000..5e210eb82 --- /dev/null +++ b/internal/core/glossary/delimiter_rule_test.go @@ -0,0 +1,20 @@ +package glossary + +import ( + "strings" + "testing" +) + +// TestGlossaryOpensTheBlockOnTheOneDelimiterRule: the glossary judges the +// opening delimiter by frontmatter.IsDelimiter, so an indented rule opens no +// block here as it opens none to Fields; the comment preamble stays tolerated +// (iss-2608270908348042). +func TestGlossaryOpensTheBlockOnTheOneDelimiterRule(t *testing.T) { + t.Parallel() + if got := frontmatterOpen(strings.Split(" ---\nterm: a\n---\n", "\n")); got != -1 { + t.Errorf("frontmatterOpen = %d, want -1 (an indented rule opens nothing)", got) + } + if got := frontmatterOpen(strings.Split("\n---\nterm: a\n---\n", "\n")); got != 1 { + t.Errorf("frontmatterOpen past a comment = %d, want 1", got) + } +} diff --git a/internal/core/glossary/index.go b/internal/core/glossary/index.go index 6990accb7..129c5b238 100644 --- a/internal/core/glossary/index.go +++ b/internal/core/glossary/index.go @@ -237,7 +237,10 @@ func frontmatterOpen(lines []string) int { if i >= len(lines) { return -1 } - if strings.TrimSpace(lines[i][col:]) == "---" && strings.TrimSpace(frontmatter.TrimBOM(lines[i][:col])) == "" { + // The comment preamble is tolerated; the delimiter line is judged by + // frontmatter.IsDelimiter, the one rule, so an indented ` ---` opens + // nothing here as it opens nothing to Fields (iss-2608270908348042). + if frontmatter.TrimBOM(lines[i][:col]) == "" && frontmatter.IsDelimiter(lines[i][col:]) { return i } return -1 diff --git a/internal/core/history/ingest.go b/internal/core/history/ingest.go index 539fb0a59..a4225b103 100644 --- a/internal/core/history/ingest.go +++ b/internal/core/history/ingest.go @@ -119,11 +119,11 @@ func (d Destination) verify() error { sha, ok := resolveRootSHA(d.RepoRoot) if !ok || sha == "" { return fmt.Errorf("history: ingest cannot resolve the root commit of the destination repository at %s, so it cannot prove that root owns the store key %s; name a git repository with commits as the destination", - fsutil.RedactHome(d.RepoRoot), d.RootSHA) + fsutil.DisplayPath(d.RepoRoot), d.RootSHA) } if sha != d.RootSHA { return fmt.Errorf("history: ingest destination is inconsistent — the repository at %s has root commit %s, not the store key %s; the pair must name ONE repository, because the scanner is built from the root and the records are filed under the key, and a mismatch redacts under one repository's configuration while filing into another's corpus", - fsutil.RedactHome(d.RepoRoot), sha, d.RootSHA) + fsutil.DisplayPath(d.RepoRoot), sha, d.RootSHA) } return nil } @@ -185,7 +185,8 @@ type Orphan struct { Project string `json:"project"` SessionID string `json:"session_id,omitempty"` AgentID string `json:"agent_id,omitempty"` - // Cwd is the working directory the transcript recorded, home-redacted. + // Cwd is the working directory the transcript recorded, as fsutil.DisplayPath + // shows it: home-relative, or its directory name outside HOME. Cwd string `json:"cwd,omitempty"` } @@ -390,7 +391,7 @@ func ingestOne(dest Destination, opts IngestOptions, p transcriptProbe, placed s if _, claimed := adopt[p.project]; !claimed { res.Orphans = append(res.Orphans, Orphan{ Path: p.path, Project: p.project, SessionID: p.sessionID, - AgentID: p.agentID, Cwd: fsutil.RedactHome(p.firstCwd()), + AgentID: p.agentID, Cwd: fsutil.DisplayPath(p.firstCwd()), }) return } diff --git a/internal/core/history/ingest_test.go b/internal/core/history/ingest_test.go index d5188f02a..ce5f09918 100644 --- a/internal/core/history/ingest_test.go +++ b/internal/core/history/ingest_test.go @@ -443,3 +443,37 @@ func TestIngestRefusesADestinationRootThatIsNoRepository(t *testing.T) { t.Errorf("the refusal must say what it could not resolve: %v", err) } } + +// TestIngestNamesADirectoryOutsideHomeByItsBaseName: the destination refusals +// and an orphan's recorded directory named a path through the home redaction +// alone, so a repository or working directory outside HOME reached them as an +// absolute local path (iss-2609281329007423). Each names it by its directory +// name instead. +func TestIngestNamesADirectoryOutsideHomeByItsBaseName(t *testing.T) { + repoRoot, _ := setupStore(t) + outside := t.TempDir() + unresolved := filepath.Join(outside, "dest-unresolved") + mismatched := filepath.Join(outside, "dest-mismatched") + orphanCwd := filepath.Join(outside, "orphan-cwd") + fakeRepos(t, map[string]string{repoRoot: testRootSHA, mismatched: otherRootSHA}) + + for root, name := range map[string]string{unresolved: "dest-unresolved", mismatched: "dest-mismatched"} { + _, err := Ingest(Destination{RepoRoot: root, RootSHA: testRootSHA}, []string{t.TempDir()}, IngestOptions{}) + if err == nil { + t.Fatalf("Ingest accepted the destination %s", name) + } + if !strings.Contains(err.Error(), "repository at "+name) || strings.Contains(err.Error(), outside) { + t.Errorf("the refusal must name the destination by its directory name %q, got %q", name, err) + } + } + + src := t.TempDir() + transcriptFile(t, filepath.Join(src, "some-project"), "s1.jsonl", "sess-orphan", "", orphanCwd) + res, err := Ingest(Destination{RepoRoot: repoRoot, RootSHA: testRootSHA}, []string{src}, IngestOptions{}) + if err != nil { + t.Fatalf("Ingest: %v", err) + } + if len(res.Orphans) != 1 || res.Orphans[0].Cwd != "orphan-cwd" { + t.Fatalf("want one orphan whose directory is named orphan-cwd, got %+v", res.Orphans) + } +} diff --git a/internal/core/history/location.go b/internal/core/history/location.go index 00812b827..a6592184f 100644 --- a/internal/core/history/location.go +++ b/internal/core/history/location.go @@ -330,6 +330,8 @@ func migrateLegacy(home, rootSHA string, dst Resolution) string { if movedRecords == 0 && movedStaged == 0 && complete { return "" // the legacy dirs existed but were empty: nothing worth saying. } + // Store paths, not checkouts: the note is the one notice of where the corpus + // went, so both keep RedactHome rather than fsutil.DisplayPath's base name. note := fmt.Sprintf("history: moved %d transcript(s) and %d staged file(s) out of %s into %s", movedRecords, movedStaged, fsutil.RedactHome(legacyRepo), fsutil.RedactHome(dst.Base)) if !complete { diff --git a/internal/core/history/store.go b/internal/core/history/store.go index fdb6b2116..bfac1bb1a 100644 --- a/internal/core/history/store.go +++ b/internal/core/history/store.go @@ -262,6 +262,11 @@ func marshalBody(body string) string { // parseRecord splits a record file into its metadata and redacted body. The // Path field is set by the caller. Returns an error when the frontmatter fence // is missing or a required field is malformed. +// +// The delimiters are matched byte-exact, deliberately NOT by +// frontmatter.IsDelimiter: this is the store's own format, written only by this +// file, so a record whose fence is not the one the writer emits was not written +// here and is refused rather than read leniently (iss-2608270908348042). func parseRecord(data []byte) (Record, string, error) { text := string(data) if !strings.HasPrefix(text, "---\n") { diff --git a/internal/core/identity/committer_test.go b/internal/core/identity/committer_test.go index c1ae25fc8..a05b79c5d 100644 --- a/internal/core/identity/committer_test.go +++ b/internal/core/identity/committer_test.go @@ -197,6 +197,16 @@ func TestIsToolIdentity(t *testing.T) { {RoleCommitter, "GitHub", "noreply@github.com", false}, {RoleAuthor, "Some Tool", "do-not-reply@example.com", true}, {RoleAuthor, "Alex Reppel", "alex@example.com", false}, + // A configured automation's name shape: the machine_name_word and + // machine_local_word keys the attribution gate reads are refused here + // too, since the list has two readers (iss-2609090951276167). + {RoleAuthor, "semantic-release-bot", "12345+semantic-release-bot@users.noreply.github.com", true}, + {RoleCommitter, "ci_bot", "carol@example.com", true}, + {RoleAuthor, "Renovate Bot", "bot@renovateapp.com", true}, + {RoleAuthor, "release-automation", "carol@example.com", true}, + {RoleAuthor, "Jan Bot", "jan@example.com", false}, + {RoleAuthor, "Carol Talbot", "carol.talbot@example.com", false}, + {RoleAuthor, "Jean", "jean.bot@example.com", false}, } for _, c := range cases { if got := IsToolIdentity(c.role, c.name, c.email); got != c.want { @@ -205,6 +215,18 @@ func TestIsToolIdentity(t *testing.T) { } } +// TestStructuralSignalsLeaveTheNameShapeOut: the contributors page reads the +// structural signals alone, so the name-shape keys never reach it — refusing a +// configured name is the gates' job, not the published history's. +func TestStructuralSignalsLeaveTheNameShapeOut(t *testing.T) { + if IsMachineName("semantic-release-bot") { + t.Error("IsMachineName reads the name-shape word; it is the structural `[bot]` suffix alone") + } + if IsMachineAddress(RoleAuthor, "ci-bot@example.com") { + t.Error("IsMachineAddress reads the local-part word; it is the structural bot mailbox alone") + } +} + // TestToolIdentityListIsTheGatesOwn holds the one-list property: the CI // attribution gate reads its identity patterns from the same file this package // embeds, and defines none of its own. A literal pattern reappearing in the @@ -233,7 +255,7 @@ func TestToolIdentityListIsTheGatesOwn(t *testing.T) { // TestToolIdentityPatternsSpeakBothDialects: every pattern is read by POSIX ERE // (grep -Ei, in the gate) and by RE2 (here), so each must parse in both. func TestToolIdentityPatternsSpeakBothDialects(t *testing.T) { - want := []string{"ai_name", "ai_mail", "machine_name", "machine_mail", "author_only_mail"} + want := []string{"ai_name", "ai_mail", "machine_name", "machine_mail", "machine_name_word", "machine_local_word", "author_only_mail"} if len(toolPatternSource) != len(want) { t.Fatalf("the list carries %d keys, want exactly %v", len(toolPatternSource), want) } diff --git a/internal/core/identity/tool-identities.txt b/internal/core/identity/tool-identities.txt index 191af8102..08a24f5b4 100644 --- a/internal/core/identity/tool-identities.txt +++ b/internal/core/identity/tool-identities.txt @@ -14,13 +14,15 @@ # their intersection: bracket expressions, `[[:space:]]`, alternation, groups, # `?`, `*`, anchors. No `\s`, no `\d`, no lookaround, no back-references. # -# The five keys, and which role each is refused in: +# The seven keys, and which role each is refused in: # -# ai_name both roles an AI vendor's name standing alone as the name -# ai_mail both roles an AI vendor's mail domain -# machine_name both roles the forge's own `[bot]` account-name suffix -# machine_mail both roles a bot mailbox -# author_only_mail AUTHOR only a mailbox named for not being read +# ai_name both roles an AI vendor's name standing alone as the name +# ai_mail both roles an AI vendor's mail domain +# machine_name both roles the forge's own `[bot]` account-name suffix +# machine_mail both roles a bot mailbox +# machine_name_word both roles a configured automation name's trailing word +# machine_local_word both roles the same word ending the mailbox's local part +# author_only_mail AUTHOR only a mailbox named for not being read # The git IDENTITY itself, not only the message. A commit authored AND committed # as `Claude ` carried a fully compliant message and @@ -48,7 +50,8 @@ ai_mail=@anthropic\.com$|@openai\.com$ # So these two are STRUCTURAL rather than nominal: the `[bot]` suffix the forge # itself stamps on an app's account name, and the mailbox shape it stamps on the # address — `49699333+dependabot[bot]@users.noreply.github.com`, or the older -# `name[bot]@…`. A second automation lands in the right place with no edit here. +# `name[bot]@…`. A second automation THE FORGE STAMPS lands in the right place +# with no edit here; one it does not stamp is the next rule's. # # THE MAILBOX IS THE DISCRIMINATOR, NEVER THE HOST, and this is the line to read # twice before touching it. `1234+name@users.noreply.github.com` is a PERSON'S @@ -60,6 +63,38 @@ ai_mail=@anthropic\.com$|@openai\.com$ machine_name=\[bot\][[:space:]]*$ machine_mail=\[bot\]@|@dependabot\.com$ +# The forge's suffix is only the machines THE FORGE stamps. An automation that +# commits under a name it was configured with — semantic-release-bot at a forge +# no-reply address, a self-hosted CI account, a forge whose app suffix is not +# `[bot]` — matched none of the lists above and was judged a human +# (iss-2609090951276167). So a second signal reads the SHAPE such configured +# names take: a trailing `bot`, `robot` or `automation` word, ending the display +# name or the mailbox's local part. It is a name shape, not a forge stamp, so +# unlike the two above it is nominal, and it is drawn narrowly for that reason. +# +# The word must stand alone: at the start of the field, or after a `-` or `_` +# joining it to the rest — the separators a configured account name uses and a +# person's name does not. Talbot and Abbott pass; semantic-release-bot, ci_bot +# and `12345+semantic-release-bot@users.noreply.github.com` do not. In the local +# part the forge's `+` separates too, and deliberately not `.`: `jean.bot@` is +# the ordinary shape of a person's address. In the display name WHITESPACE DOES +# NOT SEPARATE: `Jan Bot` is a person whose surname is Bot, and refusing a +# person's name is the thing refuse-machines-not-an-allowlist rules out. +# `Renovate Bot` is still refused, at its default address `bot@renovateapp.com`, +# by the local-part half. +# +# OUT OF REACH, said plainly: a machine whose configured name and mailbox look +# like a person's (`Release Manager `, or `Renovate Bot` +# reconfigured to `renovate@example.com`), a trailing word other than these +# three (`-ci`, `-agent`), and `name.bot@` addresses. Nothing in the identity +# separates those from a human; the reviewer reading the identity is the check +# on them. The contributors page (internal/core/site/contributors.go) does not +# read these two keys: it reads the published history through IsMachineName and +# IsMachineAddress, the structural signals alone, and refusing a name shape is +# the gates' job, not the page's. +machine_name_word=(^|[-_])(bot|robot|automation)[[:space:]]*$ +machine_local_word=(^|[-_+])(bot|robot|automation)@ + # A mailbox literally named for not being read. Refused in the AUTHOR role only, # and the asymmetry is load-bearing rather than a hedge: `GitHub # ` is the COMMITTER of every merge and squash made through diff --git a/internal/core/identity/toolidentity.go b/internal/core/identity/toolidentity.go index 6c9b123c6..42a615722 100644 --- a/internal/core/identity/toolidentity.go +++ b/internal/core/identity/toolidentity.go @@ -25,13 +25,17 @@ var ( aiMailRe = toolPattern("ai_mail") machineNameRe = toolPattern("machine_name") machineMailRe = toolPattern("machine_mail") + machineNameWord = toolPattern("machine_name_word") + machineLocalWord = toolPattern("machine_local_word") authorOnlyMailRe = toolPattern("author_only_mail") ) // IsToolIdentity reports whether name is a machine identity in the // given role of a commit: an AI vendor's name or mail domain, the forge's own -// `[bot]` account, or a bot mailbox, in either role; and, in the AUTHOR role -// only, a mailbox named for not being read (`noreply@`). The asymmetry is the +// `[bot]` account, a bot mailbox, or a configured automation's name shape (a +// trailing `bot`, `robot` or `automation` word ending the name or the mailbox's +// local part), in either role; and, in the AUTHOR role only, a mailbox named for +// not being read (`noreply@`). The asymmetry is the // attribution gate's own — the forge stamps `GitHub ` as the // committer of every web-UI merge made on a human's click, so it passes as a // committer and nowhere else. @@ -41,7 +45,8 @@ var ( // gate runs, before the attribution gate refuses the pull request in CI. func IsToolIdentity(role Role, name, email string) bool { return aiNameRe.MatchString(name) || aiMailRe.MatchString(email) || - IsMachineName(name) || IsMachineAddress(role, email) + IsMachineName(name) || IsMachineAddress(role, email) || + machineNameWord.MatchString(name) || machineLocalWord.MatchString(email) } // IsMachineName reports the STRUCTURAL name signal alone: the `[bot]` suffix the diff --git a/internal/core/implement/loop/check.go b/internal/core/implement/loop/check.go index deaf39c0a..e0c5c511c 100644 --- a/internal/core/implement/loop/check.go +++ b/internal/core/implement/loop/check.go @@ -313,7 +313,7 @@ func peersCheck(repoRoot string, r intent.ReadyResult, session string) (CheckRow // A peer the listing names and cannot read may hold the record; the check // fails closed on it, as it does on an unreadable claim below. for _, p := range rep.Unjudged() { - holders = append(holders, peerName(p.Source, p.Branch, p.Path)+" could not be read, so what it holds is unknown ("+fsutil.RedactHome(p.NotRead)+")") + holders = append(holders, peerName(p.Source, p.Branch, p.Path)+" could not be read, so what it holds is unknown ("+fsutil.DisplayPathsIn(p.NotRead, p.Path)+")") } if sha := gitutil.RootCommit(repoRoot); gitutil.IsFullSHA(sha) { run, err := implement.Peek(sha) @@ -348,12 +348,14 @@ func peersCheck(repoRoot string, r intent.ReadyResult, session string) (CheckRow return row, nil } -// peerName names a peer for a refusal. +// peerName names a peer for a refusal, a worktree by fsutil.DisplayPath so one +// outside HOME is its directory name, not an absolute local path +// (iss-2609281329007423). func peerName(src peers.Source, branch, path string) string { if src != peers.SourceWorktree { return "branch " + branch } - who := "the worktree at " + fsutil.RedactHome(path) + who := "the worktree at " + fsutil.DisplayPath(path) if branch != "" { who += " (branch " + branch + ")" } diff --git a/internal/core/implement/loop/loop_test.go b/internal/core/implement/loop/loop_test.go index b283fe067..978cda0ca 100644 --- a/internal/core/implement/loop/loop_test.go +++ b/internal/core/implement/loop/loop_test.go @@ -178,6 +178,60 @@ func TestStartRefusesAPeerHoldingTheRecord(t *testing.T) { }) } +// TestThePeerRefusalNamesAWorktreeOutsideHomeByItsDirectoryName: the refusal +// named a peer worktree through the home redaction alone, so one outside HOME +// reached the refusal as an absolute local path, in its name and inside its +// not-read reason (iss-2609281329007423). Both name it by its directory name. +func TestThePeerRefusalNamesAWorktreeOutsideHomeByItsDirectoryName(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("chmod 0 on the shut worktree's planned folder does not deny root, so its could-not-be-read holder never forms") + } + repo := loopRepo(t, readyIntent("", settledQuestions), specWithSteps("")) + outside := t.TempDir() + held := filepath.Join(outside, "wt-held") + repo.Git("worktree", "add", "-q", "-b", "lane-alpha", held) + shipped := ".abcd/development/intents/shipped/itd-10-alpha.md" + if err := os.MkdirAll(filepath.Dir(filepath.Join(held, shipped)), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Rename(filepath.Join(held, plannedRel), filepath.Join(held, shipped)); err != nil { + t.Fatal(err) + } + shut := filepath.Join(outside, "wt-shut") + repo.Git("worktree", "add", "-q", "-b", "lane-shut", shut) + if err := os.WriteFile(filepath.Join(shut, "note"), []byte("ahead\n"), 0o644); err != nil { + t.Fatal(err) + } + repo.Git("-C", shut, "add", "note") + repo.Git("-C", shut, "commit", "-q", "-m", "ahead of main") + locked := filepath.Join(shut, ".abcd", "development", "intents", "planned") + if err := os.Chmod(locked, 0); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(locked, 0o755) }) + + _, err := Start(repo.Root(), "itd-10", Options{}) + r := mustRefusal(t, err) + if r.Check != CheckPeers || !r.Contention { + t.Fatalf("want the peers check as contention: %+v", r) + } + for _, want := range []string{"the worktree at wt-held (branch lane-alpha)", "the worktree at wt-shut (branch lane-shut) could not be read", "wt-shut/"} { + if !strings.Contains(r.Reason, want) { + t.Errorf("the refusal lacks %q: %q", want, r.Reason) + } + } + abs := []string{outside} + if real, err := filepath.EvalSymlinks(outside); err == nil && real != outside { + abs = append(abs, real) + } + for _, a := range abs { + if strings.Contains(r.Reason, a) { + t.Errorf("the refusal prints the absolute worktree path under %s: %q", a, r.Reason) + } + } + runTierAbsent(t, repo.Root()) +} + // TestStartAgainResumesTheRunItsOwnLaneChanged is criterion 7's resume once // the run has changed the tree it was judged on: its lane's worktree (in the // machine-scoped store, piece 6's shape) delivers the intent to shipped/, or diff --git a/internal/core/intent/bom_test.go b/internal/core/intent/bom_test.go new file mode 100644 index 000000000..d5aba60f3 --- /dev/null +++ b/internal/core/intent/bom_test.go @@ -0,0 +1,63 @@ +package intent + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/intentdriven/abcd/internal/core/frontmatter" +) + +// TestIntentWritersReadTheBlockTheReaderReads: a BOM ahead of the opening +// delimiter is one frontmatter.Fields reads past, so every intent writer must +// too. Its private walks refused such a record with "no leading frontmatter +// block" while Load read it fine (iss-2608221126066379). The BOM survives the +// write: a writer edits the keys, never the file's first bytes. +func TestIntentWritersReadTheBlockTheReaderReads(t *testing.T) { + t.Parallel() + const bom = "\ufeff" + doc := bom + "---\nid: itd-10\nslug: alpha\nheld: yes\n---\n# alpha\n" + + set, err := setFrontmatterFields(doc, map[string]string{"spec_id": "spc-1", "slug": "beta"}) + if err != nil { + t.Fatalf("setFrontmatterFields on a BOM-led record: %v", err) + } + if !strings.HasPrefix(set, bom+"---\n") { + t.Fatalf("the write must keep the file's BOM and opening delimiter:\n%q", set) + } + fields := frontmatter.Fields(strings.Split(set, "\n")) + if fields["spec_id"].Value != "spc-1" || fields["slug"].Value != "beta" { + t.Fatalf("the reader must see both writes inside the block: %+v\n%q", fields, set) + } + + removed, err := removeFrontmatterField(doc, "held") + if err != nil { + t.Fatalf("removeFrontmatterField on a BOM-led record: %v", err) + } + if _, ok := frontmatter.Fields(strings.Split(removed, "\n"))["held"]; ok || !strings.HasPrefix(removed, bom) { + t.Fatalf("the key must be gone and the BOM kept:\n%q", removed) + } + + if _, err := frontmatterClose(strings.Split(doc, "\n")); err != nil { + t.Fatalf("frontmatterClose on a BOM-led record: %v", err) + } +} + +// TestAddRelatedIssueWritesABOMLedRecord: the verb a user runs, end to end, on +// a record the reader loads. +func TestAddRelatedIssueWritesABOMLedRecord(t *testing.T) { + root := t.TempDir() + writeFile(t, root, draftsDir+"/itd-10-alpha.md", + "\ufeff---\nid: itd-10\nslug: alpha\nspec_id: null\nkind: null\n---\n# alpha\n") + if _, err := AddRelatedIssue(root, "itd-10", "iss-4"); err != nil { + t.Fatalf("AddRelatedIssue on a BOM-led record the reader loads: %v", err) + } + after, err := os.ReadFile(filepath.Join(root, draftsDir, "itd-10-alpha.md")) + if err != nil { + t.Fatal(err) + } + if got := frontmatter.Fields(strings.Split(string(after), "\n"))["related_issues"].Value; got != "[iss-4]" { + t.Fatalf("related_issues = %q, want [iss-4]:\n%q", got, after) + } +} diff --git a/internal/core/intent/hold.go b/internal/core/intent/hold.go index 8d0a931bc..ef9b561c1 100644 --- a/internal/core/intent/hold.go +++ b/internal/core/intent/hold.go @@ -315,18 +315,9 @@ func validateHoldReason(reason string) error { // frontmatter block is an error (fail closed rather than corrupt a file). func removeFrontmatterField(content, key string) (string, error) { lines := strings.Split(content, "\n") - if len(lines) == 0 || strings.TrimRight(lines[0], " \t\r") != "---" { - return "", fmt.Errorf("intent: file has no leading frontmatter block") - } - closing := -1 - for i := 1; i < len(lines); i++ { - if strings.TrimRight(lines[i], " \t\r") == "---" { - closing = i - break - } - } - if closing < 0 { - return "", fmt.Errorf("intent: frontmatter block is not closed") + closing, err := frontmatterClose(lines) + if err != nil { + return "", err } out := make([]string, 0, len(lines)) for i, line := range lines { diff --git a/internal/core/intent/intent.go b/internal/core/intent/intent.go index c9746ddf5..5fa1fd20b 100644 --- a/internal/core/intent/intent.go +++ b/internal/core/intent/intent.go @@ -159,22 +159,13 @@ func hasAcceptanceCriteria(content string) bool { // leading frontmatter block is an error (fail closed rather than corrupt a file). func setFrontmatterFields(content string, updates map[string]string) (string, error) { lines := strings.Split(content, "\n") - // Match frontmatter.Fields's delimiter tolerance exactly: a `---` line may - // carry trailing whitespace ("--- "). Trimming only "\r" here (stricter than - // the reader) makes the writer skip a delimiter the reader accepts and insert - // keys into the body instead of the frontmatter — corrupting the record. - if len(lines) == 0 || strings.TrimRight(lines[0], " \t\r") != "---" { - return "", fmt.Errorf("intent: file has no leading frontmatter block") - } - closing := -1 - for i := 1; i < len(lines); i++ { - if strings.TrimRight(lines[i], " \t\r") == "---" { - closing = i - break - } - } - if closing < 0 { - return "", fmt.Errorf("intent: frontmatter block is not closed") + // The block is frontmatter.Fields's block, found by the same walk: a writer + // stricter than the reader about a delimiter (a trailing space, a BOM ahead + // of the opening `---`) skips one the reader accepts and inserts keys into + // the body, or refuses a record the reader reads (iss-2608221126066379). + closing, err := frontmatterClose(lines) + if err != nil { + return "", err } remaining := make(map[string]string, len(updates)) diff --git a/internal/core/intent/intent_test.go b/internal/core/intent/intent_test.go index 32ae9b313..de5387995 100644 --- a/internal/core/intent/intent_test.go +++ b/internal/core/intent/intent_test.go @@ -3,6 +3,7 @@ package intent import ( "os" "path/filepath" + "reflect" "strings" "testing" @@ -723,11 +724,15 @@ func TestAddRelatedIssueWritesOnlyTheBackEdge(t *testing.T) { // An unknown intent and a source outside the two graduating families are // refused, and nothing is written. - if _, err := AddRelatedIssue(root, "itd-99", "rdi-17"); err == nil { + if got, err := AddRelatedIssue(root, "itd-99", "rdi-17"); err == nil { t.Error("AddRelatedIssue on an intent in no bucket must be refused") + } else if !reflect.DeepEqual(got, Intent{}) { + t.Errorf("a refused AddRelatedIssue must return the zero Intent, got %+v", got) } - if _, err := AddRelatedIssue(root, "itd-10", "adr-4"); err == nil { + if got, err := AddRelatedIssue(root, "itd-10", "adr-4"); err == nil { t.Error("AddRelatedIssue with a source outside ^(iss|rdi)-[0-9]+$ must be refused") + } else if !reflect.DeepEqual(got, Intent{}) { + t.Errorf("a refused AddRelatedIssue must return the zero Intent, got %+v", got) } } @@ -757,10 +762,16 @@ func TestAddRelatedIssueKeepsAnExistingEdgeAndIsIdempotentOnTheSame(t *testing.T t.Fatalf("the record must carry both edges, the first kept first:\n%s", after) } - // The SAME source is a no-op that leaves the record byte-identical. - if _, err := AddRelatedIssue(root, "itd-11", "rdi-17"); err != nil { + // The SAME source is a no-op that leaves the record byte-identical, and it + // still returns the record's list: the promote route reads the kept edge + // off this return on a re-run too (iss-2609021815563506). + same, err := AddRelatedIssue(root, "itd-11", "rdi-17") + if err != nil { t.Fatalf("AddRelatedIssue with the source already there must be a no-op: %v", err) } + if got := strings.Join(same.RelatedIssues, ","); same.ID != "itd-11" || got != "rdi-17,rdi-18" { + t.Fatalf("the no-op must return the record as it stands: id %q, RelatedIssues %q, want itd-11 and rdi-17,rdi-18", same.ID, got) + } again, err := os.ReadFile(filepath.Join(root, draftsDir, "itd-11-beta.md")) if err != nil { t.Fatal(err) @@ -778,10 +789,15 @@ func TestAddRelatedIssueRefusesARecordCarryingTheRetiredField(t *testing.T) { root := t.TempDir() body := "---\nid: itd-12\nslug: gamma\nspec_id: null\nkind: null\npromoted_from: iss-3\n---\n# gamma\n" writeFile(t, root, draftsDir+"/itd-12-gamma.md", body) - _, err := AddRelatedIssue(root, "itd-12", "iss-4") + got, err := AddRelatedIssue(root, "itd-12", "iss-4") if err == nil { t.Fatal("AddRelatedIssue on a record carrying promoted_from must be refused") } + // A refusal returns no record beside its error: a caller reading the + // return on an error path reads nothing (iss-2609021815563506). + if !reflect.DeepEqual(got, Intent{}) { + t.Errorf("a refused AddRelatedIssue must return the zero Intent, got %+v", got) + } if !strings.Contains(err.Error(), "capture migrate") { t.Errorf("the refusal must name the migration; got %v", err) } diff --git a/internal/core/intent/lifecycle.go b/internal/core/intent/lifecycle.go index b74698795..f440ae100 100644 --- a/internal/core/intent/lifecycle.go +++ b/internal/core/intent/lifecycle.go @@ -907,14 +907,20 @@ func Reconcile(repoRoot, specID, impact string, remainder RemainderRequest) (Rec // accepting it here would report a write that never happened — and stamping it // early would pre-decide the derived version of a release this close does not // reach. The judgement belongs at the close that ships (adr-2609151513118583). + // + // Both refusals name the way to keep a judgement already made: `intent plan + // --impact` stamps it on the planned record now, and the close that ships + // then needs no flag. Without that a lane that knew the impact dropped it, + // leaving it to be remembered at the last close (iss-2609240646522330). if strings.TrimSpace(impact) != "" { + keep := keepImpactNow(intentID, impact) if len(held) > 0 { - return ReconcileResult{}, fmt.Errorf("intent: --impact is the judgement %s carries into shipped/, and this close ships nothing — %s is still open on %s; re-run without --impact, and supply it at the close that ships", - intentID, strings.Join(specIDs(held), ", "), intentID) + return ReconcileResult{}, fmt.Errorf("intent: --impact is the judgement %s carries into shipped/, and this close ships nothing — %s is still open on %s; re-run without --impact, and %s", + intentID, strings.Join(specIDs(held), ", "), intentID, keep) } if remainder.Slug != "" { - return ReconcileResult{}, fmt.Errorf("intent: --impact is the judgement %s carries into shipped/, and a remainder spec leaves it planned; re-run without --impact, and supply it at the close that ships", - intentID) + return ReconcileResult{}, fmt.Errorf("intent: --impact is the judgement %s carries into shipped/, and a remainder spec leaves it planned; re-run without --impact, and %s", + intentID, keep) } } @@ -1307,6 +1313,18 @@ func recordedImpact(content string) string { return recorded } +// keepImpactNow is the remedy clause an early close's impact refusal ends on: +// the judgement is recorded now with `abcd intent plan`, or supplied at the +// close that ships. The value is echoed only when it is one shipped/ accepts, +// so a refusal never hands back a command that would itself be refused. +func keepImpactNow(intentID, impact string) string { + value := "<" + shipImpactValues + ">" + if validShipImpact(impact) == nil { + value = impact + } + return fmt.Sprintf("either record it now with `abcd intent plan %s --impact %s`, after which the close that ships needs no flag, or supply it at the close that ships", intentID, value) +} + // validShipImpact applies the shipped/ bar to one impact value: a legal member // of the changelog vocabulary, and not `internal`. It is the same pair of checks // CreateFromText makes at the seed, so a value either boundary accepts survives diff --git a/internal/core/intent/multispec_test.go b/internal/core/intent/multispec_test.go index 853587f5d..96e56f84b 100644 --- a/internal/core/intent/multispec_test.go +++ b/internal/core/intent/multispec_test.go @@ -70,6 +70,11 @@ func TestReconcileRefusesImpactWhenAnotherSpecStaysOpen(t *testing.T) { if !strings.Contains(err.Error(), "spc-2") || !strings.Contains(err.Error(), "still open") { t.Fatalf("refusal must name the open spec that keeps the intent planned: %v", err) } + // The judgement need not wait for the close that ships: the refusal names + // the verb that records it now (iss-2609240646522330). + if !strings.Contains(err.Error(), "abcd intent plan itd-10 --impact fix") { + t.Fatalf("refusal must name `abcd intent plan itd-10 --impact fix` as the way to record the impact now: %v", err) + } if _, err := os.Stat(filepath.Join(root, specsOpen, "spc-1-alpha.md")); err != nil { t.Fatalf("nothing may move on the refusal: %v", err) } @@ -142,14 +147,22 @@ func TestReconcileMintsTheRemainderSpec(t *testing.T) { // An --impact at a close that mints a remainder is refused: that close ships // nothing, so the judgement would be written against a record staying planned. +// The refusal names `abcd intent plan --impact `, which keeps +// the judgement now, and the close that ships then needs no flag +// (iss-2609240646522330). func TestReconcileRefusesImpactWithARemainder(t *testing.T) { root := t.TempDir() - writeFile(t, root, plannedDir+"/itd-10-alpha.md", plannedLinked("itd-10", "alpha", "spc-1")) + unjudged := strings.Replace(plannedLinked("itd-10", "alpha", "spc-1"), "impact: fix\n", "", 1) + writeFile(t, root, plannedDir+"/itd-10-alpha.md", unjudged) writeFile(t, root, specsOpen+"/spc-1-alpha.md", specNaming("spc-1", "alpha", "itd-10")) - if _, err := Reconcile(root, "spc-1", "fix", RemainderRequest{Slug: "the-rest"}); err == nil { + _, err := Reconcile(root, "spc-1", "fix", RemainderRequest{Slug: "the-rest"}) + if err == nil { t.Fatal("--impact with a remainder must be refused") } + if !strings.Contains(err.Error(), "abcd intent plan itd-10 --impact fix") { + t.Fatalf("refusal must name `abcd intent plan itd-10 --impact fix` as the way to record the impact now: %v", err) + } // Nothing was minted and nothing moved. entries, err := os.ReadDir(filepath.Join(root, specsOpen)) if err != nil { @@ -158,6 +171,24 @@ func TestReconcileRefusesImpactWithARemainder(t *testing.T) { if len(entries) != 1 { t.Fatalf("the refusal must mint nothing: %d specs in open/", len(entries)) } + + // The way the refusal names works: the plan stamps the impact on the + // planned record, the remainder close goes through without the flag, and + // the close that ships needs none. + if _, err := Plan(root, "itd-10", PlanOptions{Impact: "fix"}); err != nil { + t.Fatalf("intent plan --impact on the planned intent: %v", err) + } + res, err := Reconcile(root, "spc-1", "", RemainderRequest{Slug: "the-rest"}) + if err != nil { + t.Fatalf("the remainder close without --impact: %v", err) + } + last, err := Reconcile(root, res.Remainder.ID, "", RemainderRequest{}) + if err != nil { + t.Fatalf("the close that ships must need no --impact once plan recorded it: %v", err) + } + if !last.IntentMoved || last.To != BucketShipped { + t.Fatalf("the last close must ship the intent: %+v", last) + } } // TestPartialDeliveryResidualPassesRecordLint proves the state a partial close diff --git a/internal/core/intent/owed.go b/internal/core/intent/owed.go index 78168bd25..c2fc46c44 100644 --- a/internal/core/intent/owed.go +++ b/internal/core/intent/owed.go @@ -2,6 +2,7 @@ package intent import ( "path/filepath" + "regexp" "strings" ) @@ -116,11 +117,25 @@ func ReviewOf(repoRoot string, it Intent) (ReviewEntry, error) { case ReviewOwed, ReviewNone: e.ReEmit = ReEmitCommand(it.ID) case ReviewDeadLetter: - e.Reason = deadLetterReason(content, e.ReceiptID) + e.Reason = withholdLocalTier(deadLetterReason(content, e.ReceiptID)) } return e, nil } +// localTierTokenRe is one whitespace-delimited token that names the local tier. +var localTierTokenRe = regexp.MustCompile(`\S*\.work\.local\S*`) + +// withholdLocalTier replaces every token of a dead-letter reason that names the +// local tier. The reader cuts OUR retention clause off the reason, but the +// reason itself is free text a host's payload supplied (an out-of-enum token +// quoted back), so it can carry a clause of the same shape, and the listing +// promises never to hand out a path into the gitignored tier whoever wrote it +// (iss-2609252038344132). Only the path is withheld; the rest of the reason is +// what the reader is for. +func withholdLocalTier(reason string) string { + return localTierTokenRe.ReplaceAllString(reason, "[local-tier path withheld]") +} + // deadLetterReason recovers the reason deadLetterBlock wrote on the line after // the marker: "Fidelity review DEAD_LETTER (receipt R): . Raw payload // retained at . ...". The reason is cut at the LAST retention clause, diff --git a/internal/core/intent/owed_test.go b/internal/core/intent/owed_test.go index 501859fb2..9aff6c319 100644 --- a/internal/core/intent/owed_test.go +++ b/internal/core/intent/owed_test.go @@ -111,9 +111,17 @@ func TestReviewsReadsEveryShippedMarker(t *testing.T) { // TestReviewsCarriesNoLocalTierPath: the dead-letter block names where its raw // payload is retained, under the gitignored local tier; the listing reports the // reason and never that path. +// +// The reason is free text a host's payload supplied, so the fixture forges one +// that quotes a retention clause of its own: the property is that no local-tier +// path reaches the listing whoever wrote it, not that the block the reader cuts +// happens to hold only ours (iss-2609252038344132). func TestReviewsCarriesNoLocalTierPath(t *testing.T) { root := t.TempDir() seedReviewStates(t, root) + const forged = "verdict quoted back: Raw payload retained at .abcd/.work.local/reviews/rcp-0000000000f7.deadletter.json (see it)" + writeFile(t, root, shippedDir+"/itd-17-forged.md", shippedWithNotes("itd-17", "forged", + deadLetterBlock("rcp-0000000000f7", forged, reviewsRelDir+"/rcp-0000000000f7.deadletter.json", nil, func(s string) string { return oneLine(s) }))) l, err := Reviews(root) if err != nil { t.Fatal(err) @@ -125,6 +133,11 @@ func TestReviewsCarriesNoLocalTierPath(t *testing.T) { if strings.Contains(string(b), ".work.local") || strings.Contains(string(b), "request.md") { t.Fatalf("listing carries a local-tier path:\n%s", b) } + // The rest of the reason is still reported: only the path is withheld. + got := reviewsByID(t, l)["itd-17"].Reason + if !strings.HasPrefix(got, "verdict quoted back: Raw payload retained at ") || !strings.HasSuffix(got, " (see it)") { + t.Fatalf("the forged reason must be reported with only its path withheld; got %q", got) + } } // TestReviewsNeverWrites: the reader reads; it never re-emits a request or diff --git a/internal/core/intent/questions.go b/internal/core/intent/questions.go index 461260e30..20c8b710f 100644 --- a/internal/core/intent/questions.go +++ b/internal/core/intent/questions.go @@ -24,11 +24,20 @@ var ( // openLeadRe is an item led by a bold "Open": a question, whatever else // the item says. openLeadRe = regexp.MustCompile(`(?i)^\*\*open\b`) - // settledMarkRe is an item's explicit disposition: a bold span that opens + // settledBoldRe is an item's explicit disposition as a bold span that opens // with it (`**Resolved — …**`, `**Deferred**`, `**explicitly deferred**`, - // `**explicit deferral**`), or the word as a label (`resolved:`, - // `RESOLVED:`, `Deferred:`). - settledMarkRe = regexp.MustCompile(`(?i)\*\*(resolved|deferred|explicitly deferred|explicit deferral)\b|\b(resolved|deferred)\s*:`) + // `**explicit deferral**`), wherever in the item the span sits. + settledBoldRe = regexp.MustCompile(`(?i)\*\*(resolved|deferred|explicitly deferred|explicit deferral)\b`) + // settledLabelRe is the disposition as a LABEL (`resolved:`, `RESOLVED:`, + // `Deferred:`), and a label only where a label is written: opening a line of + // the item — a nested sub-bullet continuation (` - Resolved: …`) opens one + // too — after a closing bold, with or without a colon after it + // (`**Which surface?** RESOLVED:`, `**Which id?**: Resolved:`), or after a + // dash (`**Refusal breadth** — resolved:`). Mid-sentence the same word and + // colon are prose — "once the split is resolved: the old or the new?" is a + // question — and reading them as a marker let build start past it + // (iss-2609260932374727). + settledLabelRe = regexp.MustCompile(`(?i)(^|^[-*][ \t]+|\*\*:?[ \t]*|[—–][ \t]*|[ \t]-[ \t]+)(resolved|deferred)[ \t]*:`) ) // OpenQuestions returns the questions an intent's `## Open Questions` section @@ -46,9 +55,11 @@ var ( // kept for the reader; // - an item explicitly marked resolved or deferred — a bold span opening // with the word (`**Resolved — …**`, `**Deferred**`, `**explicitly -// deferred**`, `**explicit deferral**`) or the word as a label -// (`resolved:`, `Deferred:`) anywhere in the item, continuation lines -// included — is not a question. +// deferred**`, `**explicit deferral**`) anywhere in the item, continuation +// lines included, or the word as a label (`resolved:`, `Deferred:`) +// opening a line of the item (a nested sub-bullet included), after a +// closing bold (and an optional colon), or after a dash — is not a +// question. The same word and colon mid-sentence are prose. // // Everything else under the heading that is a list item is a question // whatever it says: an item led `**Open`, an item that only points elsewhere, @@ -62,7 +73,7 @@ func OpenQuestions(content string) []string { if item == nil { return } - if !settledItem(strings.Join(item, " ")) { + if !settledItem(item) { out = append(out, item[0]) } item = nil @@ -97,8 +108,21 @@ func OpenQuestions(content string) []string { return out } -// settledItem reports whether an item's text, its continuation lines joined, -// carries an explicit resolved or deferred marker and is not led "Open". -func settledItem(text string) bool { - return !openLeadRe.MatchString(text) && settledMarkRe.MatchString(text) +// settledItem reports whether an item — its first line's text, then its +// continuation lines, each trimmed — carries an explicit resolved or deferred +// marker and is not led "Open". The lines are judged apart for the label, whose +// place is the start of a line, and joined for the bold span, which may wrap. +func settledItem(lines []string) bool { + if openLeadRe.MatchString(lines[0]) { + return false + } + if settledBoldRe.MatchString(strings.Join(lines, " ")) { + return true + } + for _, ln := range lines { + if settledLabelRe.MatchString(ln) { + return true + } + } + return false } diff --git a/internal/core/intent/questions_test.go b/internal/core/intent/questions_test.go index c57de3805..368534fc0 100644 --- a/internal/core/intent/questions_test.go +++ b/internal/core/intent/questions_test.go @@ -80,6 +80,26 @@ func TestOpenQuestionsReadsTheSettledConvention(t *testing.T) { {"a question that mentions deferral", head + "- Should the check be deferred until the runner ships?\n- **Out of scope, recorded for clarity**: a workspace layer.\n", []string{"Should the check be deferred until the runner ships?", "**Out of scope, recorded for clarity**: a workspace layer."}}, + {"a label mid-sentence is not a marker (iss-2609260932374727)", head + + "- Which id wins once the split is resolved: the old or the new?\n" + + "- Once the flag is deferred: who picks it up?\n" + + "- Which runner?\n It stays a question until it is resolved: see below.\n", + []string{"Which id wins once the split is resolved: the old or the new?", + "Once the flag is deferred: who picks it up?", "Which runner?"}}, + {"a label opening the item or a continuation line (iss-2609260932374727)", head + + "- Resolved: the local runner.\n" + + "- Which runner?\n Deferred: to the runner intent.\n", nil}, + {"a label after a closing bold and a parenthetical dash (itd-93)", head + + "- **Relationship to itd-73** (derived versioning) — RESOLVED: the CHANGELOG.\n", nil}, + {"a label opening a nested sub-bullet continuation", head + + "- Which id wins?\n - Resolved: the new one.\n" + + "- Which runner?\n * Deferred: to the runner intent.\n", nil}, + {"a label after a colon that follows the closing bold", head + + "- **Which id?**: Resolved: the new one.\n" + + "- **Which runner?**:Deferred: to the runner intent.\n", nil}, + {"a sub-bullet that only mentions resolution mid-sentence is still a question", head + + "- Which id wins?\n - once the split is resolved: the old or the new?\n", + []string{"Which id wins?"}}, {"an opener below the first item does not open the section", head + "- Which runner?\n\n_All resolved at planning._\n", []string{"Which runner?"}}, {"an opener that settles only some", head + diff --git a/internal/core/intent/reclassify.go b/internal/core/intent/reclassify.go index 576a56cd3..3e5e9570b 100644 --- a/internal/core/intent/reclassify.go +++ b/internal/core/intent/reclassify.go @@ -473,17 +473,21 @@ func historyEntry(date, from, to, reason string) string { } // frontmatterClose returns the index of the frontmatter block's closing -// delimiter in lines, with setFrontmatterFields's delimiter tolerance. +// delimiter in lines. It is this package's one form of frontmatter.Close, the +// walk frontmatter.Fields makes, so every intent writer agrees with the reader +// about where the block ends — a BOM ahead of the opening delimiter included, +// which a private walk here refused while the reader accepted the record +// (iss-2608221126066379). The two refusals stay apart, because they name +// different repairs. func frontmatterClose(lines []string) (int, error) { - if len(lines) == 0 || strings.TrimRight(lines[0], " \t\r") != "---" { + if len(lines) == 0 || !frontmatter.IsDelimiter(frontmatter.TrimBOM(lines[0])) { return 0, fmt.Errorf("intent: file has no leading frontmatter block") } - for i := 1; i < len(lines); i++ { - if strings.TrimRight(lines[i], " \t\r") == "---" { - return i, nil - } + closing := frontmatter.Close(lines) + if closing < 0 { + return 0, fmt.Errorf("intent: frontmatter block is not closed") } - return 0, fmt.Errorf("intent: frontmatter block is not closed") + return closing, nil } // frontmatterKeyLine returns the index of key's top-level line, or -1. diff --git a/internal/core/issuerecord/parse.go b/internal/core/issuerecord/parse.go index ae154cbdf..79a64e3b0 100644 --- a/internal/core/issuerecord/parse.go +++ b/internal/core/issuerecord/parse.go @@ -35,17 +35,10 @@ func Parse(text string) (map[string]any, string, error) { if len(lines) == 0 || !frontmatter.IsDelimiter(frontmatter.TrimBOM(lines[0])) { return nil, "", fmt.Errorf("%w: frontmatter must start with '---' on the first line", ErrMalformedFrontmatter) } - closeIdx := -1 - for i := 1; i < len(lines); i++ { - ln := lines[i] - if strings.HasPrefix(ln, " ") || strings.HasPrefix(ln, "\t") { - continue - } - if frontmatter.IsDelimiter(ln) { - closeIdx = i - break - } - } + // The close is frontmatter.CloseAfter's, the one closing walk: an indented + // line is never a close there, which is the refusal this parser needs + // (iss-2608270908348042). + closeIdx := frontmatter.CloseAfter(lines, 0) if closeIdx == -1 { return nil, "", fmt.Errorf("%w: frontmatter not terminated: missing closing '---'", ErrMalformedFrontmatter) } diff --git a/internal/core/issueschema/bom_test.go b/internal/core/issueschema/bom_test.go new file mode 100644 index 000000000..36fc7fe65 --- /dev/null +++ b/internal/core/issueschema/bom_test.go @@ -0,0 +1,14 @@ +package issueschema + +import "testing" + +// TestParseDispositionReadsPastABOM: a BOM is the file's encoding mark, not +// preamble, so a BOM-led disposition reads as the record it is +// (iss-2608221126066379). +func TestParseDispositionReadsPastABOM(t *testing.T) { + t.Parallel() + rec := ParseDisposition("dsp-1", "\ufeff---\nstate: held\nexit_condition: x\n---\nbody\n") + if rec.State != "held" || !rec.WellFormed { + t.Fatalf("ParseDisposition = %+v, want state held and well-formed", rec) + } +} diff --git a/internal/core/issueschema/delimiter_rule_test.go b/internal/core/issueschema/delimiter_rule_test.go new file mode 100644 index 000000000..ed0fd0412 --- /dev/null +++ b/internal/core/issueschema/delimiter_rule_test.go @@ -0,0 +1,19 @@ +package issueschema + +import "testing" + +// TestParseDispositionOpensOnTheOneDelimiterRule: a disposition's block is read +// on frontmatter.Close's terms, the strict ledger parser's, so an indented +// opening rule is no block at all — the record is not well-formed, and its +// fields are not trusted (iss-2608270908348042). +func TestParseDispositionOpensOnTheOneDelimiterRule(t *testing.T) { + t.Parallel() + rec := ParseDisposition("dsp-1", " ---\nstate: held\n---\n") + if rec.WellFormed || rec.State != "" { + t.Fatalf("ParseDisposition read an indented opener as a block: %+v", rec) + } + rec = ParseDisposition("dsp-1", "---\nstate: held\n---\n") + if rec.State != "held" { + t.Fatalf("ParseDisposition = %+v, want state held", rec) + } +} diff --git a/internal/core/issueschema/disposition.go b/internal/core/issueschema/disposition.go index 7938baa08..778f258fb 100644 --- a/internal/core/issueschema/disposition.go +++ b/internal/core/issueschema/disposition.go @@ -72,19 +72,13 @@ func ParseDisposition(id, content string) DispositionRecord { // The block must OPEN on the first line. A comment, a blank line, or any // other preamble means the file is not the shape a record is written in, and // tolerating it is precisely where the two readers parted company. - if len(lines) == 0 || strings.TrimSpace(lines[0]) != "---" { - return rec - } - closeAt := -1 - for i := 1; i < len(lines); i++ { - if strings.HasPrefix(lines[i], " ") || strings.HasPrefix(lines[i], "\t") { - continue - } - if strings.TrimSpace(lines[i]) == "---" { - closeAt = i - break - } - } + // A BOM is not preamble: it is the file's encoding mark, which the strict + // ledger parser and frontmatter.Fields both trim at line 0 and only there + // (iss-2608221126066379). + // The block's extent is frontmatter.Close's, the walk the strict ledger + // parser takes, so an indented opening rule opens nothing here either + // (iss-2608270908348042). + closeAt := frontmatter.Close(lines) if closeAt == -1 { return rec } diff --git a/internal/core/issueschema/ledgerdirs.go b/internal/core/issueschema/ledgerdirs.go index 1d32a7190..833914589 100644 --- a/internal/core/issueschema/ledgerdirs.go +++ b/internal/core/issueschema/ledgerdirs.go @@ -45,6 +45,20 @@ const ReadingsRecordDir = ".abcd/development/readings" // records are the next ingest sweep's to roll back. const RunRecordFileName = "run.json" +// RunManifestFileName is the manifest a run was assembled from. The channel's +// ingest promotes it into the durable run directory BEFORE it writes the run +// record, so a committed run holds both, and the two agree on the run id, the +// position and the candidate join. The ordering gate reads that agreement +// (capture.ComparativeRunFor), which is why the name lives here rather than in +// core/reading alone. +const RunManifestFileName = "manifest.json" + +// RunArtefactReadLimit is the ceiling a run's own artefacts (its manifest, its +// bundle) are read under, by the reading channel and by the gate that reads a +// committed run's manifest back. A manifest carries the run's whole item list, +// so it is bounded by the channel's ceiling rather than a single record's. +const RunArtefactReadLimit = 4 << 20 + // RunHead is the strictly decoded subset of a committed run record that answers // one question: which widening run did this comparative run characterise? // diff --git a/internal/core/launch/archive.go b/internal/core/launch/archive.go index 91578db6f..16bdd4067 100644 --- a/internal/core/launch/archive.go +++ b/internal/core/launch/archive.go @@ -88,7 +88,7 @@ type PluginArchive struct { // caller removes or reads the archive through. It never reaches machine // output (iss-81); DisplayPath is what a report names. Path string `json:"-"` - // DisplayPath is Path as a report names it (fsutil.DisplayPath): relative + // DisplayPath is Path as a report names it (fsutil.RepoRelativePath): relative // to the repository when --out is inside it, the home redacted to "~" // otherwise (iss-2609261950077063). DisplayPath string `json:"path"` @@ -140,7 +140,7 @@ func RenderPluginArchive(req PayloadRenderRequest, outDir string) (PluginArchive a.Name = PluginArchiveName(name, req.Version) a.Version = req.Version a.Path = filepath.Join(outDir, a.Name) - a.DisplayPath = fsutil.DisplayPath(req.RepoRoot, a.Path) + a.DisplayPath = fsutil.RepoRelativePath(req.RepoRoot, a.Path) if _, err := os.Lstat(a.Path); err == nil { return a, res, fmt.Errorf("%s already exists in the output directory — refusing to replace a release archive", a.Name) } diff --git a/internal/core/launch/bom_test.go b/internal/core/launch/bom_test.go new file mode 100644 index 000000000..b9c82e41b --- /dev/null +++ b/internal/core/launch/bom_test.go @@ -0,0 +1,14 @@ +package launch + +import "testing" + +// TestProseLinesDropsABOMLedFrontmatter: a BOM ahead of the opening rule does +// not turn a document's frontmatter into prose (iss-2608221126066379). +func TestProseLinesDropsABOMLedFrontmatter(t *testing.T) { + t.Parallel() + for _, pl := range proseLines([]byte("\ufeff---\ntitle: x\n---\nProse.\n")) { + if pl.text != "Prose." && pl.text != "" { + t.Fatalf("line %d read as prose: %q", pl.n, pl.text) + } + } +} diff --git a/internal/core/launch/bundle.go b/internal/core/launch/bundle.go index 2efa165bf..e38837c54 100644 --- a/internal/core/launch/bundle.go +++ b/internal/core/launch/bundle.go @@ -330,7 +330,7 @@ func (r *resolver) included(c candidate) IncludedFile { return IncludedFile{ LogicalPath: c.logical, ResolvedPath: c.resolved, - DisplayResolvedPath: fsutil.DisplayPath(r.root, c.resolved), + DisplayResolvedPath: fsutil.RepoRelativePath(r.root, c.resolved), GitMode: c.gitMode, } } diff --git a/internal/core/launch/delimiter_rule_test.go b/internal/core/launch/delimiter_rule_test.go new file mode 100644 index 000000000..f429f1cc3 --- /dev/null +++ b/internal/core/launch/delimiter_rule_test.go @@ -0,0 +1,20 @@ +package launch + +import "testing" + +// TestProseLinesReadsTheBlockOnTheOneDelimiterRule: the prose gate takes the +// frontmatter block's extent from frontmatter.Close, so an indented rule does +// not close it and the line after it is metadata, not prose — as Fields reads +// it (iss-2608270908348042). +func TestProseLinesReadsTheBlockOnTheOneDelimiterRule(t *testing.T) { + t.Parallel() + got := proseLines([]byte("---\ntitle: x\n ---\nkind: y\n---\nbody line\n")) + for _, l := range got { + if l.text == "kind: y" || l.text == " ---" { + t.Fatalf("proseLines read frontmatter line %d (%q) as prose: the indented rule closed the block", l.n, l.text) + } + } + if len(got) == 0 || got[0].text != "body line" { + t.Fatalf("proseLines = %+v, want the body to open at \"body line\"", got) + } +} diff --git a/internal/core/launch/gates.go b/internal/core/launch/gates.go index e543797f2..ad2a6ab47 100644 --- a/internal/core/launch/gates.go +++ b/internal/core/launch/gates.go @@ -15,6 +15,7 @@ import ( "encoding/json" "errors" "fmt" + "github.com/intentdriven/abcd/internal/core/frontmatter" "github.com/intentdriven/abcd/internal/core/mdrecord" "os" "path" @@ -257,15 +258,12 @@ func proseLines(data []byte) []proseLine { lines := strings.Split(strings.ReplaceAll(string(data), "\r\n", "\n"), "\n") var out []proseLine frontEnd := -1 // index of the closing "---"; -1 when there is no frontmatter - if len(lines) > 0 && strings.TrimSpace(lines[0]) == "---" { - for i := 1; i < len(lines); i++ { - if strings.TrimSpace(lines[i]) == "---" { - if isFrontmatter(lines[1:i]) { - frontEnd = i - } - break - } - } + // The block's extent is frontmatter.Close's: a BOM ahead of the opening + // rule trimmed at line 0 (iss-2608221126066379), and every delimiter judged + // by the one rule, so an indented rule neither opens nor closes it + // (iss-2608270908348042). + if end := frontmatter.Close(lines); end > 0 && isFrontmatter(lines[1:end]) { + frontEnd = end } // Fenced code is read through the tree's one fence rule (mdrecord), so a // longer run, a mismatched closer or an unclosed fence reads here exactly diff --git a/internal/core/launch/render.go b/internal/core/launch/render.go index 569825fb8..fe350c953 100644 --- a/internal/core/launch/render.go +++ b/internal/core/launch/render.go @@ -87,7 +87,7 @@ type PayloadRenderResult struct { // absolute working value the archive step packs from. It never reaches // machine output (iss-81); DisplayDest is what a report names. Dest string `json:"-"` - // DisplayDest is Dest as a report names it (fsutil.DisplayPath): relative + // DisplayDest is Dest as a report names it (fsutil.RepoRelativePath): relative // to the repository inside it, the home redacted to "~" outside it — and a // destination is always outside it (iss-2609261848338673). DisplayDest string `json:"dest"` @@ -438,7 +438,7 @@ func RenderPayload(req PayloadRenderRequest) (PayloadRenderResult, error) { primaryPath, primaryPtr := pre.PrimaryPath, pre.PrimaryPointer bundle := pre.Bundle res.Dest = dest - res.DisplayDest = fsutil.DisplayPath(req.RepoRoot, dest) + res.DisplayDest = fsutil.RepoRelativePath(req.RepoRoot, dest) res.Bundle = bundle if err := os.MkdirAll(dest, 0o755); err != nil { diff --git a/internal/core/lint/agentcontract.go b/internal/core/lint/agentcontract.go index 907ad1944..5e19052f0 100644 --- a/internal/core/lint/agentcontract.go +++ b/internal/core/lint/agentcontract.go @@ -31,6 +31,7 @@ import ( "regexp" "strings" + "github.com/intentdriven/abcd/internal/core/frontmatter" "github.com/intentdriven/abcd/internal/fsutil" "github.com/intentdriven/abcd/internal/gitutil" ) @@ -437,19 +438,25 @@ func changedPaths(repoRoot, rangeSpec string) (map[string]bool, error) { // value, so it reads as present either way; the inline-list convention // (agents/README.md) is a style rule this parser does not adjudicate. func agentCapabilityScope(lines []string) map[string]string { - if start := frontmatterOpen(lines); start > 0 { - lines = lines[start:] - } - if len(lines) == 0 || strings.TrimSpace(lines[0]) != "---" { + start := frontmatterOpen(lines) + if start < 0 { return nil } + // frontmatterOpen has judged the opening line, a BOM ahead of it included; + // re-judging it here without the trim refused a BOM-led prompt's block + // (iss-2608221126066379). + lines = lines[start:] + // The block ends at frontmatter.CloseAfter's close, the one closing walk + // (iss-2608270908348042); an unclosed block is read to the end of the file, + // as this reader always has. + end := frontmatter.CloseAfter(lines, 0) + if end < 0 { + end = len(lines) + } scope := map[string]string{} inScope, member := false, "" - for i := 1; i < len(lines); i++ { + for i := 1; i < end; i++ { line := strings.TrimRight(lines[i], "\r") - if strings.TrimSpace(line) == "---" { - break - } indented := line != "" && (line[0] == ' ' || line[0] == '\t') if !indented { inScope, member = strings.HasPrefix(line, "capability_scope:"), "" diff --git a/internal/core/lint/bom_title_test.go b/internal/core/lint/bom_title_test.go new file mode 100644 index 000000000..c38f809bf --- /dev/null +++ b/internal/core/lint/bom_title_test.go @@ -0,0 +1,26 @@ +package lint + +import ( + "strings" + "testing" +) + +// TestRecordTitleReadsPastABOM: a BOM-led issue's title is its first body line, +// not its opening delimiter (iss-2608221126066379). +func TestRecordTitleReadsPastABOM(t *testing.T) { + t.Parallel() + lines := strings.Split("\ufeff---\nid: iss-1\n---\nThe first body line.\n", "\n") + if got := recordTitle(lines); got != "The first body line." { + t.Fatalf("recordTitle = %q, want the first body line", got) + } +} + +// TestAgentCapabilityScopeReadsPastABOM: a BOM-led prompt's capability scope +// is read from its block (iss-2608221126066379). +func TestAgentCapabilityScopeReadsPastABOM(t *testing.T) { + t.Parallel() + lines := strings.Split("\ufeff---\nname: x\ncapability_scope:\n designed_for: [review]\n---\nbody\n", "\n") + if got := agentCapabilityScope(lines); got["designed_for"] != "[review]" { + t.Fatalf("agentCapabilityScope = %v, want designed_for [review]", got) + } +} diff --git a/internal/core/lint/config.go b/internal/core/lint/config.go index 3f705b8f8..0e4f25219 100644 --- a/internal/core/lint/config.go +++ b/internal/core/lint/config.go @@ -108,7 +108,9 @@ type RuleConfig struct { // ExtraRoots are repo-relative trees links_resolve walks for links ALONE, // beyond Roots: the working tier (.abcd/work) holds relative links in the issue // ledger, DECISIONS.md and CONTEXT.md, and adding it to Roots would arm every - // content rule there too (iss-2608230752354927). + // content rule there too (iss-2608230752354927). harness_leak reads its own + // ExtraRoots the same way, for the leak class alone: the ledger is committed + // free text outside the record's Roots (iss-2608301306580014). ExtraRoots []string `json:"extra_roots"` // IntentsDir is the intents subdirectory (relative to a root) read by the // intent-tree rules, intent_lifecycle and intent_impact_valid. Rules that name diff --git a/internal/core/lint/delimiter_rule_test.go b/internal/core/lint/delimiter_rule_test.go new file mode 100644 index 000000000..24aa64025 --- /dev/null +++ b/internal/core/lint/delimiter_rule_test.go @@ -0,0 +1,38 @@ +package lint + +import ( + "strings" + "testing" +) + +// TestLintReadsTheBlockOnTheOneDelimiterRule: record-lint's frontmatter readers +// judge the delimiter by frontmatter.IsDelimiter and close the block by +// CloseAfter, so an indented rule neither opens nor closes a block — exactly +// as Fields, the reader, reads it. A private TrimSpace compare closed the block +// on the indented rule, and the gate read a different block from the reader's +// (iss-2608270908348042). +func TestLintReadsTheBlockOnTheOneDelimiterRule(t *testing.T) { + t.Parallel() + indentedClose := strings.Split("---\nid: a\n ---\nb: c\n---\n# Title\n", "\n") + if got := frontmatterBodyStart(indentedClose); got != 5 { + t.Errorf("frontmatterBodyStart = %d, want 5 (the indented rule is not a close)", got) + } + if got := recordBodyStart(indentedClose); got != 5 { + t.Errorf("recordBodyStart = %d, want 5 (the indented rule is not a close)", got) + } + if title, line := recordH1(indentedClose); title != "Title" || line != 6 { + t.Errorf("recordH1 = %q at %d, want \"Title\" at 6", title, line) + } + indentedOpen := strings.Split(" ---\nid: a\n---\n# Title\n", "\n") + if got := frontmatterOpen(indentedOpen); got != -1 { + t.Errorf("frontmatterOpen = %d, want -1 (an indented rule opens nothing)", got) + } + // The comment preamble stays this reader's deliberate tolerance. + if got := frontmatterOpen(strings.Split("\n---\nid: a\n---\n", "\n")); got != 1 { + t.Errorf("frontmatterOpen past a comment = %d, want 1", got) + } + scope := agentCapabilityScope(strings.Split("---\ncapability_scope:\n designed_for: [a]\n ---\n task_classes: [b]\n---\n", "\n")) + if scope["task_classes"] != "[b]" { + t.Errorf("agentCapabilityScope = %v, want task_classes read past the indented rule", scope) + } +} diff --git a/internal/core/lint/guardedread_rules_test.go b/internal/core/lint/guardedread_rules_test.go index 306ce04fe..16f23adda 100644 --- a/internal/core/lint/guardedread_rules_test.go +++ b/internal/core/lint/guardedread_rules_test.go @@ -3,6 +3,7 @@ package lint import ( "os" "path/filepath" + "reflect" "strings" "syscall" "testing" @@ -269,6 +270,46 @@ func TestRecordSchemaNamesEveryUndeclaredLink(t *testing.T) { } } +// A markdown-named link at a bucketed store's root that is no record filename +// is named too, without being followed (iss-2609261208193041): telling whether +// `notes.md` points at a directory or a file would mean following it, so every +// such link is reported, whatever it points at. The store's README.md is the +// one link the root may carry, and a dot-named link is tooling state. +func TestRecordSchemaNamesAMarkdownNamedLinkAtAStoreRoot(t *testing.T) { + cfg := Config{Rules: map[string]RuleConfig{ + ruleRecordSchema: {Enabled: true, Severity: "blocker", RecordStores: map[string]string{"iss": "work/issues"}}, + }} + forged := map[string]string{"iss-9-x.md": "---\nid: \"iss-9\"\nseverity: \"SECRET-TARGET\"\n---\n"} + root := t.TempDir() + writeFile(t, root, "work/issues/resolved/iss-5-a.md", "---\nid: \"iss-5\"\n---\n") + symlinkDirOut(t, root, "work/issues/notes.md", forged) + writeFile(t, root, "elsewhere/target.md", "---\nid: \"iss-9\"\nseverity: \"SECRET-TARGET\"\n---\n") + for _, name := range []string{"filed.md", "README.md", ".scratch.md"} { + if err := os.Symlink(filepath.Join(root, "elsewhere", "target.md"), filepath.Join(root, "work", "issues", name)); err != nil { + t.Fatal(err) + } + } + fs, err := lintWithin(t, cfg, root) + if err != nil { + t.Fatal(err) + } + named := map[string]int{} + for _, f := range fs { + if strings.Contains(f.Message, "SECRET-TARGET") || strings.Contains(f.File, "iss-9") { + t.Fatalf("something behind a link was read: %+v", f) + } + if f.RuleID == ruleRecordSchema && filepath.Dir(f.File) == filepath.Join("work", "issues") { + named[filepath.Base(f.File)]++ + if !strings.Contains(f.Message, "is a link at the issue store root") { + t.Errorf("finding on %s: %q", f.File, f.Message) + } + } + } + if want := map[string]int{"notes.md": 1, "filed.md": 1}; !reflect.DeepEqual(named, want) { + t.Fatalf("links named at the store root = %v, want %v; findings %+v", named, want, fs) + } +} + // A link at a store root that is itself a CONFIGURED store root is scanned by // that store, exactly as a real nested root is, so its parent does not call it an // undeclared bucket. diff --git a/internal/core/lint/harnessleak_test.go b/internal/core/lint/harnessleak_test.go index 70d5f171b..878f323b2 100644 --- a/internal/core/lint/harnessleak_test.go +++ b/internal/core/lint/harnessleak_test.go @@ -127,3 +127,61 @@ func TestHarnessLeakSecondMatchOnALine(t *testing.T) { t.Fatalf("a skipped leftmost candidate hid a real session URL; got %d: %+v", n, fs) } } + +// The issue ledger is committed free text a verb writes from operator input, +// and it sits outside the record's Roots, so a harness_leak rooted at the +// durable record alone never read it (iss-2608301306580014). The rule's own +// extra_roots arm it over the ledger for the leak class alone, and a tree the +// Roots walk already read is not read twice. +func TestHarnessLeakReadsItsExtraRoots(t *testing.T) { + root := t.TempDir() + writeFile(t, root, filepath.Join("docs", "run.md"), "# Run\n\nRecorded at "+synthSessionURL(t, 31)+"\n") + writeFile(t, root, filepath.Join("work", "issues", "resolved", "iss-1-a.md"), + "---\nid: \"iss-1\"\nresolution: \"fixed in the run at "+synthSessionURL(t, 37)+"\"\n---\n\nBody.\n") + writeFile(t, root, filepath.Join("work", "reviews", "r.md"), "# Review\n\nAt "+synthSessionURL(t, 41)+"\n") + + cfg := harnessLeakCfg() + rc := cfg.Rules[ruleHarnessLeak] + rc.ExtraRoots = []string{"work", "docs"} + rc.Exempt = []string{"work/reviews/*"} + cfg.Rules[ruleHarnessLeak] = rc + fs, err := Lint(cfg, root) + if err != nil { + t.Fatal(err) + } + if !hasFinding(fs, filepath.Join("work", "issues", "resolved", "iss-1-a.md"), ruleHarnessLeak, 3) { + t.Errorf("a session URL in a ledger record is not flagged: %+v", fs) + } + if n := countRule(fs, ruleHarnessLeak); n != 2 { + t.Fatalf("want one finding in the ledger and one in docs, the exempt review and the doubly-declared docs "+ + "tree drawing no second one; got %d: %+v", n, fs) + } + + // Without the extra root the ledger is outside the rule, as it was. + fs, err = Lint(harnessLeakCfg(), root) + if err != nil { + t.Fatal(err) + } + if n := countRule(fs, ruleHarnessLeak); n != 1 { + t.Fatalf("want the docs finding alone without extra_roots; got %d: %+v", n, fs) + } +} + +// The repository's own record-lint configuration arms harness_leak over the +// working tier, where the issue ledger lives (iss-2608301306580014). +func TestRecordLintArmsHarnessLeakOverTheLedger(t *testing.T) { + cfg, err := LoadConfig(filepath.Join("..", "..", "..", ".abcd", "record-lint.json")) + if err != nil { + t.Fatal(err) + } + rc, ok := cfg.Rules[ruleHarnessLeak] + if !ok || !rc.Enabled { + t.Fatal("record-lint.json must enable harness_leak") + } + for _, r := range rc.ExtraRoots { + if r == ".abcd/work" { + return + } + } + t.Fatalf("record-lint.json harness_leak extra_roots = %v, want .abcd/work, the tree the issue ledger lives in", rc.ExtraRoots) +} diff --git a/internal/core/lint/linksextra.go b/internal/core/lint/linksextra.go index a67cb4be5..a404b80e6 100644 --- a/internal/core/lint/linksextra.go +++ b/internal/core/lint/linksextra.go @@ -13,20 +13,39 @@ import ( // not exist is misconfiguration, for the reason a missing root is: it would // silently disarm the rule for that tree. func checkLinksExtraRoots(repoRoot string, cfg RuleConfig) ([]Finding, error) { + return walkExtraRoots(repoRoot, "links_resolve", cfg, nil, func(rel, fileAbs string, lines []string) []Finding { + return checkLinks(rel, fileAbs, repoRoot, lines, fenceMask(lines), cfg) + }) +} + +// checkHarnessLeakExtraRoots runs harness_leak over the rule's ExtraRoots, the +// same way: the working tier's issue ledger is committed free text a verb +// writes from operator input, and a rule rooted at the durable record alone +// never read it (iss-2608301306580014). A file the Roots walk already read is +// not read twice, so a tree both declare draws one finding per leak. +func checkHarnessLeakExtraRoots(repoRoot string, cfg RuleConfig, scanned map[string]bool) ([]Finding, error) { + return walkExtraRoots(repoRoot, ruleHarnessLeak, cfg, scanned, func(rel, _ string, lines []string) []Finding { + return checkHarnessLeak(rel, lines, fenceMask(lines), cfg) + }) +} + +// walkExtraRoots reads every markdown file under a rule's ExtraRoots, except +// one skip names or an Exempt glob matches, and hands each to check. +func walkExtraRoots(repoRoot, ruleID string, cfg RuleConfig, skip map[string]bool, check func(rel, fileAbs string, lines []string) []Finding) ([]Finding, error) { var out []Finding for _, root := range cfg.ExtraRoots { if err := containedRepoPath(root); err != nil { - return nil, &configError{"links_resolve extra_roots entry " + quote(root) + " " + err.Error() + + return nil, &configError{ruleID + " extra_roots entry " + quote(root) + " " + err.Error() + "; the lint reads only inside the repository"} } rootAbs := filepath.Join(repoRoot, filepath.FromSlash(root)) if err := resolvedInsideRoot(repoRoot, rootAbs); err != nil { - return nil, &configError{"links_resolve extra_roots entry " + quote(root) + " " + err.Error() + + return nil, &configError{ruleID + " extra_roots entry " + quote(root) + " " + err.Error() + "; the lint reads only inside the repository"} } if _, err := os.Stat(rootAbs); err != nil { if os.IsNotExist(err) { - return nil, &configError{"links_resolve extra_roots entry " + quote(root) + + return nil, &configError{ruleID + " extra_roots entry " + quote(root) + " does not exist; a configured tree that does not resolve silently disarms the rule for it"} } return nil, err @@ -37,15 +56,14 @@ func checkLinksExtraRoots(repoRoot string, cfg RuleConfig) ([]Finding, error) { } for _, fileAbs := range files { rel := repoRel(repoRoot, fileAbs) - if matchesGlob(cfg.Exempt, filepath.ToSlash(rel)) { + if skip[fileAbs] || matchesGlob(cfg.Exempt, filepath.ToSlash(rel)) { continue } content, err := readRepoAbs(repoRoot, fileAbs, maxRepoFileBytes) if err != nil { return nil, err } - lines := strings.Split(string(content), "\n") - out = append(out, checkLinks(rel, fileAbs, repoRoot, lines, fenceMask(lines), cfg)...) + out = append(out, check(rel, fileAbs, strings.Split(string(content), "\n"))...) } } return out, nil diff --git a/internal/core/lint/lint.go b/internal/core/lint/lint.go index bdb40ad92..d8aa178f4 100644 --- a/internal/core/lint/lint.go +++ b/internal/core/lint/lint.go @@ -425,6 +425,13 @@ func LintAt(cfg Config, repoRoot string, now time.Time) ([]Finding, error) { } findings = append(findings, lx...) } + if leakOn && len(leakCfg.ExtraRoots) > 0 { + hx, err := checkHarnessLeakExtraRoots(repoRoot, leakCfg, scanned) + if err != nil { + return nil, err + } + findings = append(findings, hx...) + } if len(cfg.NameRoots) > 0 { nf, err := lintNameRoots(cfg, repoRoot, scanned) @@ -2733,6 +2740,11 @@ func parseYAMLStringList(v string) []string { return frontmatter.StringList(v) } // untrimmed BOM ahead of the `---` (or ahead of a leading comment) would make a // well-formed record read as having no frontmatter and slip every // frontmatter-keyed blocker. +// +// The comment preamble is this reader's deliberate tolerance; the delimiter +// line itself is judged by frontmatter.IsDelimiter, the one rule, so an +// indented ` ---` opens nothing here exactly as it opens nothing to Fields +// (iss-2608270908348042). func frontmatterOpen(lines []string) int { // The comments are mdrecord's to locate (iss-2609251518418878); a line // holding prose after a comment's closer is content, not a comment. @@ -2740,7 +2752,7 @@ func frontmatterOpen(lines []string) int { if i >= len(lines) { return -1 } - if strings.TrimSpace(lines[i][col:]) == "---" && strings.TrimSpace(frontmatter.TrimBOM(lines[i][:col])) == "" { + if frontmatter.TrimBOM(lines[i][:col]) == "" && frontmatter.IsDelimiter(lines[i][col:]) { return i } return -1 @@ -2752,16 +2764,11 @@ func frontmatterOpen(lines []string) int { // file whose frontmatter carries a `core/epic` term reference is never scanned as // prose just because a comment precedes its `---`. func frontmatterBodyStart(lines []string) int { - open := frontmatterOpen(lines) - if open < 0 { - return 0 - } - for j := open + 1; j < len(lines); j++ { - if strings.TrimSpace(lines[j]) == "---" { - return j + 1 - } + // The close is frontmatter.CloseAfter's (iss-2608270908348042). + if end := frontmatter.CloseAfter(lines, frontmatterOpen(lines)); end >= 0 { + return end + 1 } - return 0 // unterminated frontmatter: treat all as body rather than swallow the file + return 0 // no frontmatter, or unterminated: treat all as body rather than swallow the file } // stripInlineCode blanks every inline code span on a line, its delimiters and diff --git a/internal/core/lint/principles.go b/internal/core/lint/principles.go index d47c83d1e..374d65824 100644 --- a/internal/core/lint/principles.go +++ b/internal/core/lint/principles.go @@ -124,6 +124,9 @@ var ( // closing sequence, which is not part of the title. principleTitleRe = regexp.MustCompile(`^#[ \t]+(.*)$`) principleTitleCloseRe = regexp.MustCompile(`[ \t]+#+[ \t]*$`) + // principleSetextRuleRe is the `===` underline that makes the paragraph + // above it a setext H1 (iss-2609261140284421); the `---` form is an H2. + principleSetextRuleRe = regexp.MustCompile(`^ {0,3}=+[ \t]*$`) ) // PrincipleStatement is where a principle's statement sits in its document: @@ -137,8 +140,9 @@ var ( // are how a gate came to judge a paragraph while the projection sent a title // above it (iss-2609261039134673). type PrincipleStatement struct { - // Title is the H1 title with any closing hashes removed, and TitleLine its - // 0-based line; TitleLine is -1 when the document carries no H1. + // Title is the H1 title, ATX with any closing hashes removed or setext with + // its lines joined, and TitleLine its first 0-based line; TitleLine is -1 + // when the document carries no H1. Title string TitleLine int // Start and End bound the paragraph's lines, [Start, End), 0-based, the @@ -157,10 +161,15 @@ func FindPrincipleStatement(lines []string) (PrincipleStatement, bool) { } st := PrincipleStatement{TitleLine: -1, Start: body + start, End: body + end} mask := mdrecord.Mask(lines[body:]) - for i, ln := range lines[body:] { + rest := lines[body:] + for i, ln := range rest { if i < len(mask) && mask[i] != 0 { continue } + if first, title := setextPrincipleTitle(rest, mask, i); title != "" { + st.Title, st.TitleLine = title, body+first + break + } m := principleTitleRe.FindStringSubmatch(strings.TrimRight(ln, "\r")) if m == nil { continue @@ -173,16 +182,39 @@ func FindPrincipleStatement(lines []string) (PrincipleStatement, bool) { return st, true } +// setextPrincipleTitle reads the setext H1 whose underline sits directly under +// line i: the paragraph ending at i, its lines joined by one space, and its +// first line. The title is empty when line i+1 is no `===` underline or line i +// opens a block other than a paragraph, under which the run is no underline. +// A setext heading's content is the whole paragraph above the underline, so +// the walk runs up to the blank line, a masked line, or a line opening another +// block that ends it. +func setextPrincipleTitle(lines []string, mask []uint8, i int) (int, string) { + masked := func(j int) bool { return j < len(mask) && mask[j] != 0 } + isPara := func(j int) bool { + ln := strings.TrimRight(lines[j], "\r") + return strings.TrimSpace(ln) != "" && !masked(j) && !notParagraphRe.MatchString(ln) + } + if i+1 >= len(lines) || masked(i+1) || !principleSetextRuleRe.MatchString(strings.TrimRight(lines[i+1], "\r")) || !isPara(i) { + return 0, "" + } + first := i + for first > 0 && isPara(first-1) { + first-- + } + words := make([]string, 0, i-first+1) + for _, ln := range lines[first : i+1] { + words = append(words, strings.TrimSpace(ln)) + } + return first, strings.Join(words, " ") +} + // principleBodyStart is the first line after the leading frontmatter block, 0 // when there is none: a YAML comment is a `#` line, and it is not a title. func principleBodyStart(lines []string) int { - if len(lines) == 0 || !frontmatter.IsDelimiter(frontmatter.TrimBOM(lines[0])) { - return 0 - } - for i := 1; i < len(lines); i++ { - if !strings.HasPrefix(lines[i], " ") && !strings.HasPrefix(lines[i], "\t") && frontmatter.IsDelimiter(lines[i]) { - return i + 1 - } + // frontmatter.Close is the one walk (iss-2608270908348042). + if end := frontmatter.Close(lines); end >= 0 { + return end + 1 } return 0 } diff --git a/internal/core/lint/principles_test.go b/internal/core/lint/principles_test.go index efa070f8a..4ef155736 100644 --- a/internal/core/lint/principles_test.go +++ b/internal/core/lint/principles_test.go @@ -352,6 +352,58 @@ func TestPrincipleTitleMayNotCite(t *testing.T) { } } +// TestSetextPrincipleTitleIsCarried: an H1 written in setext form, the title +// underlined with `===`, is the principle's title as much as an ATX one, so the +// one derivation carries it (iss-2609261140284421) and principle_claims judges +// it like any other title. +func TestSetextPrincipleTitleIsCarried(t *testing.T) { + for name, tc := range map[string]struct { + title, want string + line int + }{ + "one line": {"Fix the detector\n================", "Fix the detector", 4}, + "indented rule": {"Fix the detector\n = ", "Fix the detector", 4}, + "two-line heading": {"Fix the\ndetector\n===", "Fix the detector", 4}, + "after a comment": {"\n\nFix the detector\n=", "Fix the detector", 6}, + } { + t.Run(name, func(t *testing.T) { + doc := "---\nid: prn-p\n---\n\n" + tc.title + "\n\n**The rule.** Fix the class.\n" + st, ok := FindPrincipleStatement(strings.Split(doc, "\n")) + if !ok { + t.Fatalf("no statement found in %q", doc) + } + if st.Title != tc.want || st.TitleLine != tc.line { + t.Errorf("title = %q at line %d, want %q at line %d", st.Title, st.TitleLine, tc.want, tc.line) + } + }) + } + // A `===` under a line that opens another block, or inside a fence, is no + // setext heading, and neither is the `---` form, which is an H2. + for name, body := range map[string]string{ + "H2 underline": "Fix the detector\n---\n", + "under a list": "- Fix the detector\n===\n", + "fenced": "```\nFix the detector\n===\n```\n", + "indented code": " Fix the detector\n===\n", + } { + t.Run(name, func(t *testing.T) { + doc := "---\nid: prn-p\n---\n\n" + body + "\n**The rule.** Fix the class.\n" + st, _ := FindPrincipleStatement(strings.Split(doc, "\n")) + if st.Title != "" { + t.Errorf("read the title %q from %q", st.Title, body) + } + }) + } + root := t.TempDir() + doc := strings.Replace(typedPrinciple("p", "causal", `"abcd lint"`, `"One against another."`, "[adr-1]", + "Fix the class, not the instance."), "# A principle\n", "A principle citing itd-79\n===\n", 1) + writeFile(t, root, prnDir+"/p.md", doc) + writeFile(t, root, "rec/decisions/adrs/0001-a.md", "---\nid: adr-1\n---\n# ADR-1\n") + fs := lintPrinciples(t, root) + if !findingWith(fs, filepath.Join(prnDir, "p.md"), rulePrincipleClaims, "title carries the record handle 'itd-79'") { + t.Errorf("a citation in a setext title is not refused: %v", rulesOf(fs, filepath.Join(prnDir, "p.md"))) + } +} + // TestHeadingShapedPrincipleHasNoStatement: the statement is the labelled // paragraph and nothing else, in both readers (iss-2609261039132350). A typed // principle that writes it as a `## The rule` heading carries no statement the diff --git a/internal/core/lint/readerparity_test.go b/internal/core/lint/readerparity_test.go index 13e18aa52..2d30156bc 100644 --- a/internal/core/lint/readerparity_test.go +++ b/internal/core/lint/readerparity_test.go @@ -32,6 +32,10 @@ func TestRecordSchemaAgreesWithTheLedgerReader(t *testing.T) { {"quoted schema_version", "schema_version: 1\n", "schema_version: \"1\"\n", true}, {"schema_version with a trailing comment", "schema_version: 1\n", "schema_version: 1 # v1\n", false}, {"found_during as a list", "found_during: t\n", "found_during: [a]\n", true}, + // A block closed only by a mid-file ZWNBSP rule is never closed to the + // reader, and the gate reads the block on the same delimiter rule + // (iss-2608270908348042). + {"closed only by a mid-file ZWNBSP rule", "found_during: t\n---\n", "found_during: t\n\ufeff---\n", true}, {"the valid record itself", good, good, false}, } for _, c := range cases { @@ -66,3 +70,33 @@ func TestRecordSchemaAgreesWithTheLedgerReader(t *testing.T) { }) } } + +// TestRecordSchemaRefusesAnIssueBlockSequenceAsTheReaderDoes is the +// block-sequence remainder of #357 (iss-2608270655499478). A list written as an +// indented block sequence is legitimate in the intent and ADR stores, whose +// readers take it, and refused by the issue ledger's reader, so the refusal is +// store-scoped: the gate asks capture's reader itself about an issue record and +// names the refusal, rather than rejecting the spelling everywhere. The +// referenced record exists, so no other leg speaks for the reader here. +func TestRecordSchemaRefusesAnIssueBlockSequenceAsTheReaderDoes(t *testing.T) { + for _, add := range []string{"related_issues:\n - iss-6\n", "blocked_by:\n - iss-6\n"} { + t.Run(strings.SplitN(add, ":", 2)[0], func(t *testing.T) { + root := t.TempDir() + seedRecRoot(t, root) + writeFile(t, root, filepath.Join("work", "issues", "open", "iss-6-b-slug.md"), validIssue("iss-6", "b-slug")) + rel := filepath.Join("work", "issues", "open", "iss-5-a-slug.md") + content := strings.Replace(validIssue("iss-5", "a-slug"), "severity: minor\n", "severity: minor\n"+add, 1) + if issueReadRefusal == nil || issueReadRefusal(content, "open", rel) == nil { + t.Fatal("fixture expectation is wrong: the ledger reader must refuse a block sequence") + } + writeFile(t, root, rel, content) + fs, err := Lint(schemaConfig(), root) + if err != nil { + t.Fatal(err) + } + if !findingWith(fs, rel, ruleRecordSchema, "ledger reader refuses") { + t.Fatalf("the gate does not name the reader's refusal of a block sequence: %+v", fs) + } + }) + } +} diff --git a/internal/core/lint/reading_outstanding_test.go b/internal/core/lint/reading_outstanding_test.go index 9e9dd2f04..65a957bec 100644 --- a/internal/core/lint/reading_outstanding_test.go +++ b/internal/core/lint/reading_outstanding_test.go @@ -1174,6 +1174,40 @@ func TestWideningRunSummaryStandsDownOnAnUnreadableRun(t *testing.T) { } } +// An admission does not buy a count past an answer the walk could not read +// (iss-2609251842112266): a run whose only admitted proposal carries a +// contested or illegible disposition supports no count, so its summary stands +// down as the WideningRun doc promises, rather than reporting "admitted 1, +// outstanding []" over a disposition nobody could weigh. +func TestWideningRunSummaryStandsDownOnAnAdmittedButContestedItem(t *testing.T) { + const run, item = "rdg-2608300000000001", "rdi-2608300000000011" + for name, write := range map[string]func(root string){ + "contested": func(root string) { + dispositionRecord(t, root, item, "dsp-2608300000000021", issueschema.DispositionAccepted) + dispositionRecord(t, root, item, "dsp-2608300000000022", issueschema.DispositionAccepted) + }, + "illegible": func(root string) { + // A duplicated top-level key: malformed to every reader of this ledger. + writeFile(t, root, ".abcd/work/issues/dispositions/"+item+"/dsp-2608300000000021.md", + "---\nschema_version: 1\nid: \"dsp-2608300000000021\"\nid: \"dsp-2608300000000021\"\n"+ + "item: \""+item+"\"\nstate: \"accepted\"\ndisposition_grounds: \"a\"\n---\n\n") + }, + } { + t.Run(name, func(t *testing.T) { + root := readingLedger(t, run, item, "widening") + admissionRecord(t, root, run, "adm-2608300000000031", item) + write(root) + report, err := ReadReadingOutstanding(root, ".abcd/work/issues") + if err != nil { + t.Fatal(err) + } + if len(report.WideningRuns) != 0 { + t.Fatalf("WideningRuns = %+v, want the summary stood down", report.WideningRuns) + } + }) + } +} + // The summary is a report line, pinned at info whatever the configuration asks. func TestWideningRunSummaryIsInfoNotBlocker(t *testing.T) { root := readingLedger(t, "rdg-2608300000000001", "rdi-2608300000000011", "widening") diff --git a/internal/core/lint/readingoutstanding.go b/internal/core/lint/readingoutstanding.go index cda9a67b1..d1e2fd6b5 100644 --- a/internal/core/lint/readingoutstanding.go +++ b/internal/core/lint/readingoutstanding.go @@ -444,12 +444,17 @@ func ReadReadingOutstanding(repoRoot, issuesDir string) (OutstandingReadings, er report.OpenHolds = append(report.OpenHolds, answer.holds...) if widening { + // The stand-down comes first: an admission does not buy a count + // past an answer the walk could not read, so an admitted proposal + // carrying an unreadable, contested, cyclic or illegible + // disposition stands the run's summary down like any other + // (iss-2609251842112266). switch { - case admissions.admits(run.Name(), item): - summary.Admitted++ case len(answer.unsafe) > 0 || answer.cyclic || len(answer.contested) > 1 || (answer.standing != nil && !answer.standing.wellFormed): standDown = true + case admissions.admits(run.Name(), item): + summary.Admitted++ case answer.standing != nil && answer.standing.state == issueschema.DispositionDeclined: summary.Declined++ case answer.standing != nil && answer.standing.state == issueschema.DispositionHeld: diff --git a/internal/core/lint/schema.go b/internal/core/lint/schema.go index f81c6db5a..e093dba1b 100644 --- a/internal/core/lint/schema.go +++ b/internal/core/lint/schema.go @@ -1852,6 +1852,21 @@ func scanRecordStores(repoRoot string, cfg RuleConfig) ([]schemaRecord, []Findin } continue } + // A markdown-named link that is no record filename is named as well + // (iss-2609261208193041): whether `notes.md` points at a directory or + // a file could be told only by following it, which the walk never + // does, so every such link is reported whatever it points at. The + // store's README.md is the one link the root may carry, and a link + // with a record filename falls to the store-root record leg below. + if e.Type()&fs.ModeSymlink != 0 && !strings.EqualFold(e.Name(), "README.md") && + !store.fileNumRe.MatchString(e.Name()) { + if !strings.HasPrefix(e.Name(), ".") { + add(rel, "'"+e.Name()+"' is a link at the "+store.noun+" store root; the gate never follows a link, "+ + "so whether it points at a record or at a bucket nobody declared ("+store.bucketDesc()+ + "), nothing behind it is checked") + } + continue + } if e.IsDir() { // A dot-directory is tooling state (an editor's, a scanner's), never // a lifecycle the record authored — the record's own buckets are all @@ -2199,15 +2214,19 @@ func recordBodyStart(lines []string) int { // The leading comments are mdrecord's to locate: a private walk on a // `