diff --git a/.abcd/development/intents/planned/itd-95-disembark-grounds-a-lifeboat-s-open-questions-on-a-repo-s-to.md b/.abcd/development/intents/planned/itd-95-disembark-grounds-a-lifeboat-s-open-questions-on-a-repo-s-to.md index 8d43d93..721946a 100644 --- a/.abcd/development/intents/planned/itd-95-disembark-grounds-a-lifeboat-s-open-questions-on-a-repo-s-to.md +++ b/.abcd/development/intents/planned/itd-95-disembark-grounds-a-lifeboat-s-open-questions-on-a-repo-s-to.md @@ -7,6 +7,7 @@ suggested_kind: null reclassification_history: [] builds_on: [] severity: minor +impact: additive --- # Disembark Grounds A Lifeboat's Open Questions On A Repo's TODO And FIXME Markers diff --git a/.abcd/development/intents/planned/itd-96-disembark-reads-a-repo-s-naming-and-internals-conventions-in.md b/.abcd/development/intents/planned/itd-96-disembark-reads-a-repo-s-naming-and-internals-conventions-in.md index 6f68068..b12c611 100644 --- a/.abcd/development/intents/planned/itd-96-disembark-reads-a-repo-s-naming-and-internals-conventions-in.md +++ b/.abcd/development/intents/planned/itd-96-disembark-reads-a-repo-s-naming-and-internals-conventions-in.md @@ -7,6 +7,7 @@ suggested_kind: null reclassification_history: [] builds_on: [] severity: minor +impact: additive --- # Disembark Reads A Repo's Naming And Internals Conventions Into Its Lifeboat diff --git a/.abcd/development/specs/open/spc-10-disembark-grounds-a-lifeboat-s-open-questions-on-a-repo-s-to.md b/.abcd/development/specs/open/spc-10-disembark-grounds-a-lifeboat-s-open-questions-on-a-repo-s-to.md index 8a4d8b7..b67cf8c 100644 --- a/.abcd/development/specs/open/spc-10-disembark-grounds-a-lifeboat-s-open-questions-on-a-repo-s-to.md +++ b/.abcd/development/specs/open/spc-10-disembark-grounds-a-lifeboat-s-open-questions-on-a-repo-s-to.md @@ -7,4 +7,158 @@ intent: itd-95 ## Summary -_Draft: describe what spc-10 delivers for itd-95 — scope, approach, and how it satisfies the intent's Acceptance Criteria. This spec is the design record the fidelity review audits against._ +spc-10 delivers itd-95: a conventions-tier `Source` adapter that grounds +`evidence/open-questions` on the in-code work markers a team left in its source +tree, plus the bounded recursive-walk primitive the adapter needs. A repository +with no `.abcd/` record but a codebase full of `TODO:` and `FIXME:` now reports +that section non-blank, citing `file:line` for every marker it found, instead of +falling through to a blank a rescuer has to fill from nothing. + +## Scope + +- **Core primitive** (`internal/core/lifeboat/probe.go`): + `(*SourceContext).WalkFiles(rel string) (paths []string, truncated bool)` — a + bounded, contained recursive walk of regular files under a repo-relative root. + It is the missing primitive both itd-95 and itd-96 raise, landed once here + (one-canonical-primitive) and consumed by itd-96's internals adapter. +- **Adapter** (`internal/core/lifeboat/sources_conventions.go`): + `convOpenQuestionsSource`, a sibling of `convGlossarySource`, grounding + `evidence/open-questions` at `TierConventions`. +- **Registration** (`conventionSources()` in `probe.go`): one line. +- **No mapping change.** The `evidence/open-questions` row already names "TODO + and FIXME markers" as a read and predicts a *partial* conventions status; this + spec delivers the adapter that makes the prediction true, and honours the + prediction as a ceiling rather than editing the contract. +- **No new dependency.** `regexp`, `io/fs`, and the existing `SourceContext` + surface only. + +## Approach + +### The `WalkFiles` primitive + +`SourceContext` today exposes a non-recursive `ListDir` and a bounded +`ReadFile`; there is no recursive walk. `WalkFiles` adds one, mirroring +`embark.go`'s `walkLifeboatFiles` — the repository's existing canonical +`fs.WalkDir(root.FS(), …)` walk — rather than inventing a second traversal +idiom. + +- **Containment.** The walk runs over `c.root.FS()`, so `os.Root` refuses any + component that escapes the repository root, symlinked intermediates included. + A `nil` root (unopenable repository) returns `(nil, false)`. +- **Symlinks are skipped, never followed.** Unlike embark — where a symlink in a + packed lifeboat is a trust violation and therefore fatal — a probe reads an + arbitrary foreign tree where symlinks are ordinary. The walk skips symlinked + entries (files and directories) and continues; it never errors on one. +- **Skip set.** Directory names never descended into: `.git`, `node_modules`, + `vendor`, `generated`. These are dependency, VCS-internal, and generated + trees — never a team's own open questions, and the dominant cost of an + unfiltered walk. +- **Caps, in three dimensions.** `maxWalkFiles` mirrors `maxDirEntries` + (50 000) and bounds regular files *and* directories visited: a tree of + directories holding nothing regular yields no path, so a file cap alone never + fires there and the walk would run to exhaustion over a foreign tree. + `maxWalkDepth` (32) bounds descent, because `os.Root` resolves every + directory from the containment root one component at a time — a chain of + directories costs the square of its depth, and a few thousand of them are + trivial to create and take minutes to traverse. Real trees are shallow, so + the depth cap prunes only pathological chains, and prunes the chain rather + than abandoning the tree. Any bound firing returns `truncated = true`. + Truncation is *reported*, never silent (loud-staging): an adapter that hits a + cap says so in its cited evidence, and a blank drawn from a truncated walk + says the walk was truncated. +- **Non-regular files** (FIFOs, devices, sockets) are skipped, so the walk + cannot hand an adapter a path whose read would block. +- **Output** is repo-relative POSIX paths, sorted, so every consumer is + deterministic. + +`WalkFiles` returns paths only; content is read through the existing +`ReadFile`, which already enforces the containment root, the +`maxProbeReadBytes` per-file cap, regular-file-only, and the non-blocking open. +The primitive therefore adds no second read path to audit. + +### The marker adapter + +`convOpenQuestionsSource.Probe` walks the tree from `.`, reads each file through +`ReadFile`, and records every recognised marker as a `file:line` citation. + +- **Recognised markers:** `TODO`, `FIXME`, `XXX`, `HACK`, `BUG` — uppercase + only, matched by + `(^|[^A-Za-z0-9_-])(TODO|FIXME|XXX|HACK|BUG)(:|\(|\s|$)`. The leading class + is the word boundary that stops `TODO` matching inside `TODOS` or + `todo_list`; the trailing class admits the two conventional spellings + (`TODO:` and `TODO(alice):`) plus a bare word. The hyphen is excluded from + the leading class so the common redaction placeholder shape (`XXX-XXX-XXX`) + is rejected at every one of its triples — with the hyphen admitted, the last + triple matches on its leading `-` and a support phone number becomes a + fabricated open question. `NOTE` and `OPTIMIZE` are *not* recognised: `NOTE` + marks explanation rather than unfinished work, and `OPTIMIZE` is rare enough + that its false-positive cost exceeds its value. +- **Binary files are skipped** by a NUL byte in the first 8 KiB — the + conventional heuristic, and dependency-free. No extension allow-list is + maintained. +- **Caps.** Files walked: `maxWalkFiles` (inherited). Bytes per file: + `maxProbeReadBytes` (inherited — an oversized file is skipped by `ReadFile`). + Bytes across the whole scan: `maxMarkerScanBytes`, reusing `maxPlanTotalBytes` + (512 MiB). The file cap and the per-file cap bound one walk and one read but + their product does not bound the scan, so the aggregate budget is the third + dimension `maxEmbarkTotalBytes` already caps on the embark side; every byte + `ReadFile` returns is charged against it, before the binary test, so a tree of + blobs spends it exactly as a tree of source does. Citations reported: + `maxMarkerCitations` (200); beyond it the scan keeps counting but stops + citing, and says so. +- **Output shape** mirrors `convADRsSource`: a headline count + (`"N work marker(s) across M file(s)"`) followed by up to + `maxMarkerCitations` `path:line (TODO)` entries, all passed through + `dedupeSorted` so repeated identical citations collapse and the order is + stable. +- **Status ceiling: `StatusPartial`.** Markers are a thread, not a framed set of + open questions — a `TODO` says something is unfinished, not what the question + is. This also honours the mapping row's conventions prediction. Confidence + carries the strength instead: `ConfidenceMedium` at ten or more markers, + `ConfidenceLow` below that. +- **No markers → a blank** carrying `Searched` (the marker set and the skip + set) and the human `Question`, exactly as every other adapter's blank + contract requires. When a bound cut the scan short, the blank says so: + `Searched` names the cap that stopped it and the `Question` claims only that + the part of the tree the scan reached carries no markers. A blank is a + first-class result (adr-35) only while it is trustworthy, so it never + generalises from a partial read. +- **Read-only by construction.** The adapter opens nothing for writing and + touches nothing outside the containment root; a byte-for-byte tree-invariance + test proves it. + +### Resolved open questions (itd-95 § Open Questions) + +| Question | Decision | +|---|---| +| Which markers? | `TODO`, `FIXME`, `XXX`, `HACK`, `BUG`; uppercase only; word-boundary anchored, trailing `:`/`(`/space/EOL. `NOTE`, `OPTIMIZE` excluded. | +| Which tier — conventions or git? | **Conventions.** A working-tree file scan through the `SourceContext` file surface. The adapter never touches git, so it grounds a bare snapshot as readily as a working tree. | +| Scan scope and the missing primitive | Option (a): add a bounded recursive-walk primitive, `WalkFiles`, to `SourceContext`. It is shared with itd-96, so the walk lands once. | +| Which files to scan | Every regular file the walk yields, minus the skip set (`.git`, `node_modules`, `vendor`, `generated`), minus symlinks, minus binaries (NUL-byte heuristic), minus oversized files (`ReadFile`'s cap). | +| Per-repo caps | `maxWalkFiles` = 50 000 files **and** 50 000 directories (mirrors `maxDirEntries`), `maxWalkDepth` = 32 levels of descent, `maxProbeReadBytes` per file, `maxMarkerScanBytes` = 512 MiB across the scan (reuses `maxPlanTotalBytes`), `maxMarkerCitations` = 200 citations. Every bound that fires is reported in the cited evidence. | +| Output shape and framing | Headline count + up to 200 `path:line (MARKER)` citations, `dedupeSorted`. | +| Dedup | `dedupeSorted` on the rendered citation string, so an identical `path:line (MARKER)` appears once. Multiple distinct markers in one file each keep their own line. | +| Status and confidence thresholds | Ceiling `StatusPartial`. `ConfidenceMedium` at ≥ 10 markers, else `ConfidenceLow`. Never `StatusGrounded`. | +| Interaction with the native adapter | Unchanged: richest-tier-wins. `nativeOpenQuestionsSource` displaces the marker evidence on a repository carrying both, deterministically, and the coverage report names the winning tier. Merging across tiers stays out of scope. | + +## Acceptance-criteria satisfaction + +- **Record-less repo with markers → non-blank, cites the files** — + `convOpenQuestionsSource` over a fixture repo carrying `TODO`/`FIXME`; + asserted non-blank, `Tier == TierConventions`, evidence carrying `file:line`. +- **No markers → an honest blank** — a marker-free fixture asserts + `StatusBlank` with a populated `Searched` and a non-empty `Question`. +- **Read-only** — a byte-for-byte tree-invariance test (walk the fixture, + hash every file, probe, re-hash) proves the probe writes nothing. +- **Pathological tree stays inside the caps** — a fixture exceeding + `maxWalkFiles` asserts the walk returns `truncated = true` and terminates; a + fixture exceeding an injected scan budget asserts the scan stops partway and + names the budget in its own evidence; a fixture with a symlink escaping the + root asserts nothing outside the containment root is ever read. +- **No fabricated evidence** — a fixture whose only uppercase triples are + `XXX-XXX-XXX` redaction placeholders asserts `StatusBlank`, and a table test + pins the marker pattern against the spellings it accepts and the near-misses + it rejects. +- **Both tiers present → one deterministic result** — the existing + `beats`/`tierRank` reduction is unchanged and already tested; the new + adapter adds no second winner. diff --git a/.abcd/development/specs/open/spc-11-disembark-reads-a-repo-s-naming-and-internals-conventions-in.md b/.abcd/development/specs/open/spc-11-disembark-reads-a-repo-s-naming-and-internals-conventions-in.md index 6bfbdff..391c83a 100644 --- a/.abcd/development/specs/open/spc-11-disembark-reads-a-repo-s-naming-and-internals-conventions-in.md +++ b/.abcd/development/specs/open/spc-11-disembark-reads-a-repo-s-naming-and-internals-conventions-in.md @@ -7,4 +7,125 @@ intent: itd-96 ## Summary -_Draft: describe what spc-11 delivers for itd-96 — scope, approach, and how it satisfies the intent's Acceptance Criteria. This spec is the design record the fidelity review audits against._ +spc-11 delivers itd-96: two conventions-tier `Source` adapters that ground +`constraints/naming` and `internals` on the files a team naturally writes — a +`NAMING.md` or naming page under `docs/` for naming, an `ARCHITECTURE.md` or +architecture/explanation tree plus the package layout for internals. A +repository with conventional docs but no `.abcd/` record now carries both +sections in its lifeboat instead of two blanks. `glossary` is untouched: +`convGlossarySource` already owns it, and this spec adds no second adapter for +it. + +## Scope + +- **Adapters** (`internal/core/lifeboat/sources_conventions.go`): + `convNamingSource` (`constraints/naming`) and `convInternalsSource` + (`internals`), both `TierConventions`, both siblings of `convGlossarySource`. +- **Registration** (`conventionSources()` in `probe.go`): one line each. With + spc-10's `convOpenQuestionsSource` this brings the conventions tier to + fourteen adapters. +- **Tier gate** (`hasConventions` in `probe.go`): the new adapters' own name + lists are appended to the candidate union, per the comment that already + forbids narrowing the gate below what the adapters read — otherwise a repo + carrying *only* a `NAMING.md` or `ARCHITECTURE.md` would have the whole Tier-1 + set skipped and blank falsely. +- **Depends on spc-10** for `(*SourceContext).WalkFiles`, which + `convInternalsSource` uses for its layout scan. spc-11 stacks on spc-10; it + does not re-land the primitive. +- **No mapping change, no new dependency.** + +## Approach + +### `convNamingSource` — `constraints/naming` + +Preference order, first match wins: + +1. A dedicated naming document — `ctx.FindFirst("NAMING.md", "NAMING", + "naming.md")`, then the first `docs/` entry whose lower-cased name starts + with `naming` (the same `ListDir`-prefix idiom `convGlossarySource` uses for + `docs/glossary*`). Cited alone, `ConfidenceMedium`. +2. **Fallback: the glossary document.** A project that never wrote a naming + registry usually encodes its reserved vocabulary in its glossary, so the + glossary is real evidence for naming — but weaker, because a glossary + defines terms rather than ruling on what may be renamed. Cited as + `" (glossary fallback — no dedicated naming document)"`, + `ConfidenceLow`. +3. Neither → a blank naming both search sets in `Searched` and asking the human + question. + +**Status is `StatusPartial` in both non-blank cases** — the ceiling the mapping +row predicts for this section, and the honest one: neither a naming page nor a +glossary enumerates a project's full reserved vocabulary. The strength of the +signal is carried by `Confidence` (medium for a dedicated document, low for the +glossary fallback), not by inflating the status. + +**Distinctness from `glossary` is structural, not incidental.** The two +adapters declare different `Section()` values, so the orchestrator indexes them +into different coverage rows and neither can displace the other. On a +glossary-only repository the report shows `glossary` partial/medium cited to +`GLOSSARY.md`, and `constraints/naming` partial/low cited to the same file +*with the fallback qualifier* — visibly a weaker, derived reading rather than a +duplicate row. A test asserts the two rows differ in confidence and in cited +string on that fixture. + +### `convInternalsSource` — `internals` + +Two independent signals, combined: + +- **Architecture prose.** `ctx.FindFirst("ARCHITECTURE.md", "ARCHITECTURE", + "architecture.md", "docs/architecture.md")`, then the directories + `docs/architecture`, `docs/design`, `docs/explanation` (Diátaxis), in that + preference order. A file is read and measured with the existing + `convProseBytes` / `convGroundedProseBytes` threshold — the same measure + `convContextSource` uses for a README, reused rather than re-invented. A + directory counts as prose evidence when it holds at least one Markdown file. +- **Package layout.** `WalkFiles` from `.`, keeping the top-two path segments + of every file under a recognised source root (`internal`, `pkg`, `src`, + `lib`, `cmd`, `app`) — the packages a rescuer must navigate. Cited as + `"//"` entries, capped at `maxLayoutCitations` (50) with the + overflow reported as a count. `WalkFiles`'s `truncated` flag, when set, is + reported in the citation so a capped scan never reads as a complete one. + +Outcomes: + +| Signals present | Status | Confidence | +|---|---|---| +| Architecture prose above the threshold (± layout) | `StatusPartial` | `ConfidenceHigh` | +| Architecture doc present but thin, or a doc directory with no prose measured | `StatusPartial` | `ConfidenceMedium` | +| Layout only, no architecture doc | `StatusPartial` | `ConfidenceLow` | +| Neither | `StatusBlank` with `Searched` + `Question` | — | + +As with naming, `StatusPartial` is the ceiling: an `ARCHITECTURE.md` plus a +package listing describes the shape of a system, not its internals chapters. +This matches the mapping row's conventions prediction, which stays unedited. + +### Resolved open questions (itd-96 § Open Questions) + +| Question | Decision | +|---|---| +| Naming vs glossary — what is naming's distinct source? | A **dedicated** naming document (`NAMING.md`, `docs/naming*`) is naming's primary source; the glossary is an explicitly-qualified **fallback** at lower confidence. The two sections are distinct rows with distinct evidence strings; no second `glossary` adapter is added. | +| Which paths map to `internals`? | `ARCHITECTURE.md` (and spellings) → `docs/architecture.md` → `docs/architecture/` → `docs/design/` → `docs/explanation/`, in that preference order, plus the package layout as an independent second signal. | +| What counts as "package layout", and the missing primitive? | Top-two path segments under `internal`, `pkg`, `src`, `lib`, `cmd`, `app`, gathered with spc-10's `WalkFiles`. A bounded recursive walk is needed (a top-level `ListDir` cannot see `internal/core/lifeboat`), and it is the shared primitive both intents asked for — landed once, in spc-10. | +| Extraction heuristics and status thresholds | The existing `convProseBytes` ≥ `convGroundedProseBytes` threshold separates real architecture prose from a stub. Status ceilings at `StatusPartial` for both sections (the mapping contract's conventions prediction); the distinction lives in `Confidence`. | +| Confidence levels | Naming: medium (dedicated doc) / low (glossary fallback). Internals: high (real prose) / medium (thin doc) / low (layout only). | +| Reserved-vocabulary spellings | `NAMING.md`, `NAMING`, `naming.md`, and any `docs/` entry whose name starts with `naming` (case-insensitive). Heading-shape sniffing inside a glossary is *not* attempted — it would guess at a convention that has no standard. | + +## Acceptance-criteria satisfaction + +- **Naming docs → non-blank, cites the file** — a fixture with `NAMING.md` + asserts `constraints/naming` partial, `TierConventions`, citing `NAMING.md`. +- **ARCHITECTURE.md + layout → non-blank, cites both** — a fixture with + `ARCHITECTURE.md` and an `internal//` tree asserts `internals` non-blank + citing the doc and the layout entries. +- **Neither → honest blanks** — a bare fixture asserts both sections blank with + populated `Searched` and a non-empty `Question`. +- **Read-only** — the byte-for-byte tree-invariance test from spc-10 covers the + whole probe, both new adapters included. +- **No duplicate `glossary` adapter** — a glossary-only fixture asserts + `glossary` is still grounded by `convGlossarySource`, that + `constraints/naming` is partial with the fallback qualifier and lower + confidence, and that `conventionSources()` contains exactly one adapter whose + `Section()` is `glossary`. +- **Both tiers present → one deterministic result** — the unchanged + `beats`/`tierRank` reduction; the native adapters keep winning where a record + exists, and the report names the winning tier. diff --git a/.abcd/work/DECISIONS.md b/.abcd/work/DECISIONS.md index 5a4aace..47c45cb 100644 --- a/.abcd/work/DECISIONS.md +++ b/.abcd/work/DECISIONS.md @@ -623,3 +623,27 @@ parallel-agent merge contention bites. `/abcd:intent`'s interview script: `plan` is never run without the human's explicit in-session confirmation). Escalation path if violations appear: an `ac_confirmed_by:` field is lint-legal today and slots in as a fifth check. + +- 2026-07-21 — The mapping table's per-tier status columns are a **ceiling**, + not merely a prediction. Every conventions adapter already honours its row + (`convGlossarySource` returns partial where the row says partial; + `convPlatformSource` returns grounded where it says grounded), so itd-95's + and itd-96's three new adapters cap at `StatusPartial` — the value all three + rows predict — and carry signal strength in `Confidence` instead. Rejected: + returning `StatusGrounded` for a dedicated `NAMING.md` or a prose-bearing + `ARCHITECTURE.md`, which would have made the rendered brief table wrong and + required editing `mapping.go`, the brief-to-lifeboat contract both intents + put out of scope. Every acceptance bar asks only for "non-blank", so the + ceiling satisfies them. Revisit only by amending the mapping row first. + +- 2026-07-21 — A probe walk of a foreign tree must be bounded in **three** + dimensions, not one. itd-95 shipped `WalkFiles` with a regular-file cap; an + independent security review of the itd-96 branch showed both remaining + dimensions were exploitable — a tree of directories holding no regular file + never reaches a file cap, and `os.Root` re-resolves each directory from the + containment root one component at a time, making a directory chain quadratic + in its depth (depth-1500 did not finish in two minutes). Directories are now + counted against the same cap and descent is capped at `maxWalkDepth`. The + general rule: any new whole-tree traversal states which of {entries, depth, + aggregate bytes} bounds it, because a per-item cap and a count cap multiply + and their product is not a bound. diff --git a/.abcd/work/issues/open/iss-110-the-walkfiles-skip-set-covers-only-node-and-go-dependency-tr.md b/.abcd/work/issues/open/iss-110-the-walkfiles-skip-set-covers-only-node-and-go-dependency-tr.md new file mode 100644 index 0000000..8d10ced --- /dev/null +++ b/.abcd/work/issues/open/iss-110-the-walkfiles-skip-set-covers-only-node-and-go-dependency-tr.md @@ -0,0 +1,12 @@ +--- +schema_version: 1 +id: "iss-110" +slug: "the-walkfiles-skip-set-covers-only-node-and-go-dependency-tr" +severity: "minor" +category: "tech-debt" +source: "impl-review" +found_during: "itd-95 P1 review" +found_at: "internal/core/lifeboat/probe.go" +--- + +The WalkFiles skip set covers only Node and Go dependency trees, so a Python .venv, a Rust target/, __pycache__, dist/, build/ and Pods/ are walked as if they were the team's own source. Two consequences on a record-less repo: the open-questions adapter cites a vendored dependency's TODO as this project's open question, and because fs.WalkDir reads directories in lexical order a large dot-prefixed dependency tree can consume the 50000-file walk cap before the project's own src/ is reached. Found by an independent review of the itd-95 diff; spc-10 names the current skip set, so widening it is a design revisit rather than an implementation fix. \ No newline at end of file diff --git a/.abcd/work/issues/open/iss-111-the-open-questions-marker-pattern-admits-a-bare-uppercase-wo.md b/.abcd/work/issues/open/iss-111-the-open-questions-marker-pattern-admits-a-bare-uppercase-wo.md new file mode 100644 index 0000000..d362ec7 --- /dev/null +++ b/.abcd/work/issues/open/iss-111-the-open-questions-marker-pattern-admits-a-bare-uppercase-wo.md @@ -0,0 +1,12 @@ +--- +schema_version: 1 +id: "iss-111" +slug: "the-open-questions-marker-pattern-admits-a-bare-uppercase-wo" +severity: "minor" +category: "tech-debt" +source: "impl-review" +found_during: "itd-95 P1 review" +found_at: "internal/core/lifeboat/sources_conventions.go" +--- + +The open-questions marker pattern admits a bare uppercase word followed by whitespace, so prose that merely mentions a marker matches. Measured against this repository the adapter reports 42 markers across 11 files at medium confidence, and every one is documentation about markers rather than a work marker. The precision cost lands on repos that document their own conventions. Options: require the trailing colon or parenthesis, or drop the bare-word alternative for the ambiguous markers only. spc-10 fixes the current pattern, so this is a design revisit. \ No newline at end of file diff --git a/.abcd/work/issues/open/iss-112-walkfiles-caps-the-number-of-regular-files-it-returns-but-fs.md b/.abcd/work/issues/open/iss-112-walkfiles-caps-the-number-of-regular-files-it-returns-but-fs.md new file mode 100644 index 0000000..8c46ab4 --- /dev/null +++ b/.abcd/work/issues/open/iss-112-walkfiles-caps-the-number-of-regular-files-it-returns-but-fs.md @@ -0,0 +1,12 @@ +--- +schema_version: 1 +id: "iss-112" +slug: "walkfiles-caps-the-number-of-regular-files-it-returns-but-fs" +severity: "minor" +category: "tech-debt" +source: "impl-review" +found_during: "itd-95 P1 review" +found_at: "internal/core/lifeboat/probe.go" +--- + +WalkFiles caps the number of regular files it returns, but fs.WalkDir calls ReadDir(-1) on every directory it enters, so a single directory holding millions of entries balloons memory before the file cap can apply. The maxWalkFiles doc comment claims the walk stays bounded in memory, which is only true of the returned slice. ListDir already bounds this with ReadDir(maxDirEntries); the walk does not share that guard. Found by an independent security review of the itd-95 diff. \ No newline at end of file diff --git a/.abcd/work/issues/open/iss-114-walkfiles-opens-every-directory-through-os-root-fs-which-re.md b/.abcd/work/issues/open/iss-114-walkfiles-opens-every-directory-through-os-root-fs-which-re.md new file mode 100644 index 0000000..f8e356f --- /dev/null +++ b/.abcd/work/issues/open/iss-114-walkfiles-opens-every-directory-through-os-root-fs-which-re.md @@ -0,0 +1,12 @@ +--- +schema_version: 1 +id: "iss-114" +slug: "walkfiles-opens-every-directory-through-os-root-fs-which-re" +severity: "minor" +category: "tech-debt" +source: "impl-review" +found_during: "itd-96 P1 review" +found_at: "internal/core/lifeboat/probe.go" +--- + +WalkFiles opens every directory through os.Root.FS(), which re-resolves the whole path from the containment root one component at a time, so a walk costs O(entries x depth) component opens. Measured post-fix: a 60000-directory tree at depth 30 takes 10.5s for one probe (two adapters walk it concurrently). The depth and directory caps bound this, but the constant is high. os.Root.OpenRoot would let the walk hold a sub-root per directory and open each child in O(1). Found while fixing the unbounded-walk finding in the itd-96 review. \ No newline at end of file diff --git a/.abcd/work/issues/resolved/iss-113-convopenquestionssource-s-truncation-evidence-names-only-the.md b/.abcd/work/issues/resolved/iss-113-convopenquestionssource-s-truncation-evidence-names-only-the.md new file mode 100644 index 0000000..8e1ef44 --- /dev/null +++ b/.abcd/work/issues/resolved/iss-113-convopenquestionssource-s-truncation-evidence-names-only-the.md @@ -0,0 +1,13 @@ +--- +schema_version: 1 +id: "iss-113" +slug: "convopenquestionssource-s-truncation-evidence-names-only-the" +severity: "nitpick" +category: "tech-debt" +source: "impl-review" +found_during: "itd-96 P1 review" +found_at: "internal/core/lifeboat/sources_conventions.go" +resolution: "Fixed on the itd-95 branch alongside the walk-bounding change that caused it: both truncation strings in convOpenQuestionsSource now name the walk cap as '(N entries, M levels deep)' rather than as a file cap alone, so the evidence matches the three dimensions the walk actually bounds." +--- + +convOpenQuestionsSource's truncation evidence names only the "%d-file walk cap", but WalkFiles now also truncates on its directory cap and its depth cap, so a walk stopped by either reports a cause it did not hit. The three affected strings are the searched note, the blank's qualified question, and the non-blank scan note. convInternalsSource words the same note as "walk cap (N entries, M levels deep)"; the two should say the same thing. Found by an independent security review of the itd-96 diff. \ No newline at end of file diff --git a/internal/core/lifeboat/probe.go b/internal/core/lifeboat/probe.go index dd009f4..70aa4ec 100644 --- a/internal/core/lifeboat/probe.go +++ b/internal/core/lifeboat/probe.go @@ -2,7 +2,9 @@ package lifeboat import ( "io" + "io/fs" "os" + "path" "path/filepath" "sort" "strconv" @@ -33,6 +35,29 @@ const maxGitOutputBytes = 16 << 20 // 16 MiB // it. const maxDirEntries = 50000 +// maxWalkFiles caps how many regular files WalkFiles returns from one walk, and +// equally how many directories it descends into, mirroring maxDirEntries: a +// whole-tree walk of a vast monorepo must terminate and stay bounded in memory. +// Both halves are load-bearing — a tree of directories holding no regular file +// never reaches a file cap. Reaching either is reported, never silent, so an +// adapter can say in its evidence that it saw only part of the tree. +const maxWalkFiles = maxDirEntries + +// maxWalkDepth caps how many levels below its start WalkFiles descends. Every +// directory is opened by resolving its path from the containment root one +// component at a time, so a chain of directories costs the square of its depth +// to walk: a few thousand nested directories are trivial to create and take +// minutes to traverse. Real trees are shallow — the deepest path in this +// repository is six levels — so the cap prunes only the pathological ones, and +// says so when it does. +const maxWalkDepth = 32 + +// walkSkipDirs are the directory names WalkFiles never descends into, matched by +// name at any depth: VCS internals, dependency trees, and generated code. None +// of them holds a team's own material, and together they are the dominant cost +// of an unfiltered walk. +var walkSkipDirs = []string{".git", "generated", "node_modules", "vendor"} + // Confidence qualifies a non-blank status: how sure the adapter is that the // evidence it cites actually grounds the section. It is meaningless for a blank. type Confidence string @@ -268,6 +293,99 @@ func (c *SourceContext) ListDir(rel string) []string { return names } +// WalkFiles returns the repo-relative POSIX paths of every regular file beneath +// a repo-relative directory, sorted, and reports whether the walk stopped at any +// of its bounds — the file cap, the directory cap, or the depth cap. It is the +// recursive counterpart of ListDir, for adapters whose evidence is the shape of +// the tree rather than a known filename. Content is still read through ReadFile, +// so the walk adds no second read path. +// +// It is contained by the same os.Root as every other read, skips the +// walkSkipDirs trees, and skips non-regular files (FIFOs, devices, sockets) so +// no path it yields can block on open. Symlinks — file or directory — are +// SKIPPED and the walk continues. This deliberately differs from embark's +// walkLifeboatFiles, where a symlink is a trust violation in a packed lifeboat +// and therefore fatal: a probe reads an arbitrary foreign tree in which a +// symlink is ordinary, so refusing to follow one is enough, and erroring on one +// would blank a whole section over a repository's normal furniture. +func (c *SourceContext) WalkFiles(rel string) (paths []string, truncated bool) { + return c.walkFilesLimited(rel, maxWalkFiles) +} + +// walkFilesLimited is WalkFiles with the file and directory cap injected, so the +// truncation branches are exercisable by a test at an affordable scale. The +// shipped cap stays a const: adapters run concurrently, and a mutable +// package-level cap would be shared state between them. +func (c *SourceContext) walkFilesLimited(rel string, limit int) (paths []string, truncated bool) { + if c.root == nil { + return nil, false + } + start := path.Clean(filepath.ToSlash(rel)) + if !fs.ValidPath(start) { + return nil, false + } + startDepth := pathDepth(start) + dirs := 0 + err := fs.WalkDir(c.root.FS(), start, func(p string, d fs.DirEntry, err error) error { + if err != nil { + // An unreadable entry in a foreign tree is skipped, not fatal: the + // probe reports what it could read. + return nil + } + if d.Type()&fs.ModeSymlink != 0 { + // Never followed. A symlinked directory arrives here as a symlink + // entry, not a directory, so returning nil skips it alone — fs.SkipDir + // would skip the rest of its parent. + return nil + } + if d.IsDir() { + for _, skip := range walkSkipDirs { + if path.Base(p) == skip { + return fs.SkipDir + } + } + // Directories are capped alongside files: a tree of directories + // holding nothing regular yields no path, so a file cap alone never + // fires and the walk runs to exhaustion over a foreign tree. + if dirs >= limit { + truncated = true + return fs.SkipAll + } + dirs++ + if pathDepth(p)-startDepth >= maxWalkDepth { + // Prune the chain, not the tree: everything above the cap is + // still walked, and the truncation is reported either way. + truncated = true + return fs.SkipDir + } + return nil + } + if !d.Type().IsRegular() { + return nil + } + if len(paths) >= limit { + truncated = true + return fs.SkipAll + } + paths = append(paths, p) + return nil + }) + if err != nil { + return nil, false + } + sort.Strings(paths) + return paths, truncated +} + +// pathDepth counts the segments in a cleaned slash path, with "." the zero +// depth. It is how the walk measures how far below its start it has descended. +func pathDepth(p string) int { + if p == "." { + return 0 + } + return strings.Count(p, "/") + 1 +} + // firstRootSHA returns the canonical (first) root-commit SHA, or "". func firstRootSHA(repoRoot string) string { out, err := gitutil.Run(repoRoot, "rev-list", "--max-parents=0", "HEAD") @@ -452,7 +570,13 @@ func hasConventions(c *SourceContext) bool { candidates = append(candidates, convGlossaryDocNames...) // convGlossarySource candidates = append(candidates, convPlatformFiles...) // convPlatformSource (Dockerfile, Makefile, go.mod, package.json) candidates = append(candidates, convADRDirs...) // convADRsSource - for _, ml := range convManifestLocks { // convDependenciesSource + candidates = append(candidates, convNamingDocNames...) // convNamingSource + // convInternalsSource: an architecture document, an architecture tree, or the + // package layout on its own is grounding evidence for internals. + candidates = append(candidates, convArchitectureDocNames...) + candidates = append(candidates, convArchitectureDirs...) + candidates = append(candidates, convLayoutRoots...) + for _, ml := range convManifestLocks { // convDependenciesSource candidates = append(candidates, ml.manifest) } return c.FindFirst(candidates...) != "" diff --git a/internal/core/lifeboat/probe_test.go b/internal/core/lifeboat/probe_test.go index be47c79..07a391b 100644 --- a/internal/core/lifeboat/probe_test.go +++ b/internal/core/lifeboat/probe_test.go @@ -1,10 +1,14 @@ package lifeboat import ( + "crypto/sha256" "encoding/json" + "fmt" "os" "os/exec" + "path" "path/filepath" + "strings" "testing" ) @@ -54,6 +58,263 @@ type fixtureCommit struct { message string } +// writeTree writes each repo-relative path in files under dir, creating parent +// directories. It is the plain-directory counterpart of gitFixture: a tree with +// no git and no record, carrying exactly the files a test names. +func writeTree(t *testing.T, dir string, files map[string]string) { + t.Helper() + for rel, content := range files { + full := filepath.Join(dir, filepath.FromSlash(rel)) + if err := os.MkdirAll(filepath.Dir(full), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(full, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } +} + +// TestWalkFilesCannotEscapeTheContainmentRoot is the containment property of the +// recursive walk: a probed tree is foreign and may point anywhere, so a +// symlinked file and a symlinked directory must both be skipped rather than +// followed, and no path the walk yields may resolve outside the repository root. +func TestWalkFilesCannotEscapeTheContainmentRoot(t *testing.T) { + base := t.TempDir() + repo := filepath.Join(base, "repo") + outside := filepath.Join(base, "outside") + writeTree(t, repo, map[string]string{"src/inside.go": "package src\n\n// TODO: inside\n"}) + writeTree(t, outside, map[string]string{ + "secret.txt": "// TODO: outside the root\n", + "nested/out.go": "// FIXME: outside the root\n", + }) + if err := os.Symlink(filepath.Join(outside, "secret.txt"), filepath.Join(repo, "link-file.txt")); err != nil { + t.Fatal(err) + } + if err := os.Symlink(outside, filepath.Join(repo, "link-dir")); err != nil { + t.Fatal(err) + } + + ctx, err := newSourceContext(repo) + if err != nil { + t.Fatal(err) + } + defer ctx.Close() + + paths, truncated := ctx.WalkFiles(".") + if truncated { + t.Error("WalkFiles reported truncation on a two-file tree") + } + if got := strings.Join(paths, ","); got != "src/inside.go" { + t.Fatalf("WalkFiles = %v, want only [src/inside.go]; a symlink was followed", paths) + } + realRoot, err := filepath.EvalSymlinks(repo) + if err != nil { + t.Fatal(err) + } + for _, p := range paths { + real, err := filepath.EvalSymlinks(filepath.Join(repo, filepath.FromSlash(p))) + if err != nil { + t.Fatal(err) + } + rel, err := filepath.Rel(realRoot, real) + if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + t.Errorf("WalkFiles yielded %q resolving to %q, outside the containment root %q", p, real, realRoot) + } + } +} + +// TestWalkFilesSkipsDependencyAndGeneratedTrees pins the skip set: VCS +// internals, dependency trees, and generated code are never a team's own +// material and are the dominant cost of an unfiltered walk, so the walk never +// descends into them — at any depth, matched by directory name. +func TestWalkFilesSkipsDependencyAndGeneratedTrees(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + "src/keep.go": "package src\n\n// TODO: keep me\n", + ".git/hooks/pre-commit": "# TODO: git internals\n", + "node_modules/pkg/index.js": "// TODO: a dependency's marker\n", + "vendor/dep/dep.go": "// TODO: vendored\n", + "generated/api.pb.go": "// TODO: generated\n", + "deep/vendor/nested.go": "// TODO: vendored below the root\n", + }) + ctx, err := newSourceContext(dir) + if err != nil { + t.Fatal(err) + } + defer ctx.Close() + + paths, _ := ctx.WalkFiles(".") + if got := strings.Join(paths, ","); got != "src/keep.go" { + t.Errorf("WalkFiles = %v, want only [src/keep.go]; skip set is %v", paths, walkSkipDirs) + } +} + +// TestWalkFilesStopsAtTheFileCap proves the walk is bounded: it stops at its +// file cap and says so, rather than running to exhaustion on a vast tree. The +// cap branch is exercised through the same code path at an affordable scale — +// the shipped cap stays a const, so concurrent adapters share no mutable state. +func TestWalkFilesStopsAtTheFileCap(t *testing.T) { + dir := t.TempDir() + files := map[string]string{} + for i := 0; i < 10; i++ { + files[fmt.Sprintf("f%02d.txt", i)] = "x\n" + } + writeTree(t, dir, files) + ctx, err := newSourceContext(dir) + if err != nil { + t.Fatal(err) + } + defer ctx.Close() + + paths, truncated := ctx.WalkFiles(".") + if truncated || len(paths) != len(files) { + t.Errorf("WalkFiles = %d files, truncated=%v; want %d files untruncated under the %d cap", + len(paths), truncated, len(files), maxWalkFiles) + } + + capped, truncated := ctx.walkFilesLimited(".", 3) + if !truncated { + t.Error("walk did not report truncation with 10 files under a 3-file cap") + } + if len(capped) != 3 { + t.Errorf("walk returned %d files under a 3-file cap, want 3", len(capped)) + } +} + +// TestWalkFilesStopsAtTheDirectoryCap holds the bound a file cap alone cannot: +// a tree of directories holding no regular file at all never reaches the file +// cap, so the walk would run to exhaustion over a foreign tree that costs two +// syscalls per directory and yields nothing. Directories are counted against the +// same cap, and reaching it is reported like any other truncation. +func TestWalkFilesStopsAtTheDirectoryCap(t *testing.T) { + dir := t.TempDir() + for i := 0; i < 10; i++ { + if err := os.MkdirAll(filepath.Join(dir, fmt.Sprintf("d%02d", i)), 0o755); err != nil { + t.Fatal(err) + } + } + ctx, err := newSourceContext(dir) + if err != nil { + t.Fatal(err) + } + defer ctx.Close() + + paths, truncated := ctx.walkFilesLimited(".", 3) + if !truncated { + t.Errorf("walk of 10 empty directories under a 3-entry cap reported no truncation (paths %v)", paths) + } +} + +// TestWalkFilesStopsAtTheDepthCap holds the other unbounded dimension: every +// directory the walk opens is resolved from the containment root one component +// at a time, so the cost of a chain of directories grows with the square of its +// depth — a few thousand nested directories, cheap to create, cost minutes to +// walk. The walk descends maxWalkDepth levels and says it stopped there. +func TestWalkFilesStopsAtTheDepthCap(t *testing.T) { + dir := t.TempDir() + deep := make([]string, 0, maxWalkDepth+2) + for i := 0; i < maxWalkDepth+2; i++ { + deep = append(deep, "d") + } + buried := path.Join(append(deep, "buried.txt")...) + writeTree(t, dir, map[string]string{ + "shallow.txt": "x\n", + buried: "x\n", + }) + ctx, err := newSourceContext(dir) + if err != nil { + t.Fatal(err) + } + defer ctx.Close() + + paths, truncated := ctx.WalkFiles(".") + if !truncated { + t.Errorf("walk of a %d-deep chain under a depth cap of %d reported no truncation", len(deep), maxWalkDepth) + } + for _, p := range paths { + if p == buried { + t.Errorf("walk returned %q, %d levels below its depth cap of %d", p, len(deep)-maxWalkDepth, maxWalkDepth) + } + } + // Pruning a chain is not abandoning the tree: what sits above the cap is + // still walked and still returned. + if len(paths) == 0 || paths[len(paths)-1] != "shallow.txt" { + t.Errorf("walk = %v, want the shallow file still returned", paths) + } +} + +// TestProbeLeavesEveryFileByteIdentical is the read-only invariance property at +// full strength: a probe of a marker-bearing tree must leave every file's +// contents unchanged and add or remove nothing. The marker adapter reads every +// file in the tree, so byte-level proof — not a path/size fingerprint — is what +// makes "point it at an archived project, touch nothing" true. The fixture +// carries every signal the whole-tree adapters consult — work markers, a naming +// document, an architecture document, and a package tree — so one fixture proves +// the property for all of them. +func TestProbeLeavesEveryFileByteIdentical(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + "README.md": "# demo\n\nA demo project kept for the probe.\n", + "go.mod": "module example.com/demo\n\ngo 1.22\n", + "src/a.go": "package a\n\n// TODO: handle the retry case\n", + "src/b.go": "package a\n\n// FIXME: this leaks a connection\n", + "docs/notes.md": "Notes about the demo.\n", + "NAMING.md": "# Naming\n\nA voyage is never called a run.\n", + "ARCHITECTURE.md": "# Architecture\n\nA core, and a shell that formats it.\n", + "internal/core/core.go": "package core\n", + "internal/store/store.go": "package store\n", + }) + before := fileHashes(t, dir) + if _, err := Probe(dir); err != nil { + t.Fatal(err) + } + after := fileHashes(t, dir) + + for p, want := range before { + got, ok := after[p] + if !ok { + t.Errorf("probe removed %s", p) + continue + } + if got != want { + t.Errorf("probe rewrote %s (sha256 %s -> %s)", p, want, got) + } + } + for p := range after { + if _, ok := before[p]; !ok { + t.Errorf("probe created %s", p) + } + } +} + +// fileHashes maps every regular file under root to the SHA-256 of its contents. +func fileHashes(t *testing.T, root string) map[string]string { + t.Helper() + out := map[string]string{} + err := filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() { + return nil + } + data, err := os.ReadFile(path) + if err != nil { + return err + } + rel, err := filepath.Rel(root, path) + if err != nil { + return err + } + out[filepath.ToSlash(rel)] = fmt.Sprintf("%x", sha256.Sum256(data)) + return nil + }) + if err != nil { + t.Fatal(err) + } + return out +} + // TestProbeReportsEverySection is the report's completeness invariant: a probe // names every brief section in the mapping, exactly once, and its summary // accounts for all of them. A section silently dropped from the report would be diff --git a/internal/core/lifeboat/sources_conventions.go b/internal/core/lifeboat/sources_conventions.go index 31e4986..cd33a5d 100644 --- a/internal/core/lifeboat/sources_conventions.go +++ b/internal/core/lifeboat/sources_conventions.go @@ -1,7 +1,9 @@ package lifeboat import ( + "bytes" "fmt" + "regexp" "sort" "strings" ) @@ -23,6 +25,9 @@ func conventionSources() []Source { convSurfacesSource{}, convOutOfScopeSource{}, convGlossarySource{}, + convNamingSource{}, + convInternalsSource{}, + convOpenQuestionsSource{}, convIssuesSource{}, convWhatWorkedSource{}, } @@ -438,6 +443,30 @@ func (convOutOfScopeSource) Probe(ctx *SourceContext) Evidence { // convGlossaryDocNames are the glossary spellings checked outside docs/. var convGlossaryDocNames = []string{"GLOSSARY.md", "GLOSSARY", "glossary.md"} +// convDocsEntryWithPrefix returns the repo-relative path of the first entry +// directly under docs/ whose lower-cased name starts with prefix, or "". It is +// the docs/ convention several adapters share: a project that keeps its glossary +// or its naming rules under docs/ names the page after the thing it documents. +func convDocsEntryWithPrefix(ctx *SourceContext, prefix string) string { + for _, name := range ctx.ListDir("docs") { + if strings.HasPrefix(strings.ToLower(name), prefix) { + return "docs/" + name + } + } + return "" +} + +// convGlossaryDoc returns the glossary document a repository carries — a +// conventional spelling at the root, else a docs/glossary* page — or "". It is +// the single definition of "where the glossary lives", read both by the glossary +// adapter that owns the section and by the naming adapter that falls back to it. +func convGlossaryDoc(ctx *SourceContext) string { + if p := ctx.FindFirst(convGlossaryDocNames...); p != "" { + return p + } + return convDocsEntryWithPrefix(ctx, "glossary") +} + // convGlossarySource partially grounds "glossary" from a glossary document under // docs/ or at the repo root. Blank when none exists. type convGlossarySource struct{} @@ -446,28 +475,391 @@ func (convGlossarySource) Section() Section { return "glossary" } func (convGlossarySource) Tier() Tier { return TierConventions } func (convGlossarySource) Probe(ctx *SourceContext) Evidence { - if p := ctx.FindFirst(convGlossaryDocNames...); p != "" { + if p := convGlossaryDoc(ctx); p != "" { return Evidence{ Status: StatusPartial, Confidence: ConfidenceMedium, Sources: []string{p}, } } - for _, name := range ctx.ListDir("docs") { - if strings.HasPrefix(strings.ToLower(name), "glossary") { - return Evidence{ - Status: StatusPartial, - Confidence: ConfidenceMedium, - Sources: []string{"docs/" + name}, - } - } - } return blank( []string{"GLOSSARY.md", "docs/glossary*"}, "What terms does this project define? No glossary document found.", ) } +// convNamingDocNames are the dedicated naming-document spellings checked outside +// docs/. The list is short because there is no strong standard: a project that +// rules on names usually writes one file and calls it NAMING. +var convNamingDocNames = []string{"NAMING.md", "NAMING", "naming.md"} + +// convNamingSource partially grounds "constraints/naming" from a dedicated +// naming document, falling back to the glossary. A project that never wrote a +// naming registry usually encodes its reserved vocabulary in its glossary, so +// the glossary is real evidence for naming — weaker, because a glossary defines +// terms rather than ruling on what may be renamed, and cited with that +// qualifier so the section is never mistaken for a copy of the glossary row. +// +// The ceiling is StatusPartial in both cases: neither a naming page nor a +// glossary enumerates a project's full reserved vocabulary. The strength of the +// signal is carried by Confidence, not by inflating the status. +type convNamingSource struct{} + +func (convNamingSource) Section() Section { return "constraints/naming" } +func (convNamingSource) Tier() Tier { return TierConventions } + +func (convNamingSource) Probe(ctx *SourceContext) Evidence { + p := ctx.FindFirst(convNamingDocNames...) + if p == "" { + p = convDocsEntryWithPrefix(ctx, "naming") + } + if p != "" { + return Evidence{ + Status: StatusPartial, + Confidence: ConfidenceMedium, + Sources: []string{p}, + } + } + if g := convGlossaryDoc(ctx); g != "" { + return Evidence{ + Status: StatusPartial, + Confidence: ConfidenceLow, + Sources: []string{g + " (glossary fallback — no dedicated naming document)"}, + } + } + return blank( + []string{ + "naming document (" + strings.Join(convNamingDocNames, ", ") + ")", + "docs/naming*", + "GLOSSARY.md", "docs/glossary*", + }, + "What names and reserved vocabulary are fixed? No naming document and no glossary found.", + ) +} + +// convArchitectureDocNames are the architecture-document spellings checked as a +// single file, in preference order. +var convArchitectureDocNames = []string{ + "ARCHITECTURE.md", "ARCHITECTURE", "architecture.md", "docs/architecture.md", +} + +// convArchitectureDirs are the conventional homes for architecture prose spread +// across several files, in preference order. The last is the Diátaxis +// explanation quadrant, which is where a repo following that scheme puts it. +var convArchitectureDirs = []string{ + "docs/architecture", "docs/design", "docs/explanation", +} + +// convLayoutRoots are the directory names that hold a project's own packages — +// the second, independent internals signal, and the one every codebase has +// whether or not it wrote its architecture down. +var convLayoutRoots = []string{"internal", "pkg", "src", "lib", "cmd", "app"} + +// maxLayoutCitations caps how many package entries the layout scan cites. Beyond +// it the scan keeps counting — the headline stays truthful — but stops citing, +// so a monorepo with thousands of packages cannot dump them all into a brief +// section. +const maxLayoutCitations = 50 + +// convDirHasMarkdown reports whether a repo-relative directory holds at least one +// Markdown file — the test that separates a real documentation tree from an +// empty or placeholder one. +func convDirHasMarkdown(ctx *SourceContext, dir string) bool { + if !ctx.IsDir(dir) { + return false + } + for _, name := range ctx.ListDir(dir) { + low := strings.ToLower(name) + if strings.HasSuffix(low, ".md") || strings.HasSuffix(low, ".markdown") { + return true + } + } + return false +} + +// convLayoutPackages returns the "//" entries implied by walked file +// paths: the top two segments of every file beneath a recognised source root, +// unique and sorted. A file sitting directly in a root names no package, so it +// contributes nothing. +func convLayoutPackages(paths []string) []string { + var out []string + for _, p := range paths { + seg := strings.Split(p, "/") + if len(seg) < 3 { + continue + } + for _, root := range convLayoutRoots { + if seg[0] == root { + out = append(out, seg[0]+"/"+seg[1]+"/") + break + } + } + } + return dedupeSorted(out) +} + +// convInternalsSource partially grounds "internals" from what a team wrote about +// its own architecture and from the package layout a rescuer must navigate. The +// two signals are independent: either alone is evidence, and together they are +// the strongest reading available without a record. +// +// The ceiling is StatusPartial by construction: an architecture document plus a +// package listing describes the shape of a system, not its internals chapters. +// Which signals were found moves the confidence instead. +type convInternalsSource struct{} + +func (convInternalsSource) Section() Section { return "internals" } +func (convInternalsSource) Tier() Tier { return TierConventions } + +func (s convInternalsSource) Probe(ctx *SourceContext) Evidence { + return s.probeLimited(ctx, maxWalkFiles) +} + +// probeLimited is Probe with the walk cap injected, so the truncation branch is +// exercisable by a test at an affordable scale. The shipped cap stays a const: +// adapters run concurrently, and a mutable package-level cap would be shared +// state between them. +func (convInternalsSource) probeLimited(ctx *SourceContext, walkLimit int) Evidence { + docPath := ctx.FindFirst(convArchitectureDocNames...) + docDir := "" + if docPath == "" { + for _, dir := range convArchitectureDirs { + if convDirHasMarkdown(ctx, dir) { + docDir = dir + break + } + } + } + paths, truncated := ctx.walkFilesLimited(".", walkLimit) + pkgs := convLayoutPackages(paths) + + // Loud staging: a walk the bounds cut short saw only part of the tree, so a + // rescuer never mistakes a partial layout for the whole one — and the note is + // owed whether or not the truncated walk happened to reach a package first. + truncatedNote := "" + if truncated { + truncatedNote = fmt.Sprintf("walk cap (%d entries, %d levels deep) cut the layout scan short", walkLimit, maxWalkDepth) + } + + if docPath == "" && docDir == "" && len(pkgs) == 0 { + searched := []string{ + "architecture document (" + strings.Join(convArchitectureDocNames, ", ") + ")", + "architecture trees (" + strings.Join(convArchitectureDirs, ", ") + ")", + "package layout under " + strings.Join(convLayoutRoots, ", "), + } + // A blank is a first-class result only while it is trustworthy (adr-35), + // so a scan that never reached the source roots says so rather than + // claiming there are none. + question := "How is this system built internally? No architecture document and no recognisable package layout." + if truncated { + searched = append(searched, truncatedNote+"; the rest of the tree was not walked") + question = "How is this system built internally? No architecture document, and no package layout in the part of the tree the scan reached — it did not reach all of it." + } + return blank(searched, question) + } + + var sources []string + // Layout alone is the weakest reading: it says where the code is, never why. + confidence := ConfidenceLow + switch { + case docPath != "": + // The same prose measure convContextSource applies to a README, so a + // scaffolded ARCHITECTURE.md is not mistaken for a written one. + if data, ok := ctx.ReadFile(docPath); ok && convProseBytes(data) >= convGroundedProseBytes { + sources = append(sources, docPath) + confidence = ConfidenceHigh + } else { + sources = append(sources, docPath+" (near-empty)") + confidence = ConfidenceMedium + } + case docDir != "": + sources = append(sources, docDir+"/") + confidence = ConfidenceMedium + } + + if truncated { + sources = append(sources, truncatedNote+"; packages beyond it were not seen") + } + if len(pkgs) > 0 { + if len(pkgs) > maxLayoutCitations { + sources = append(sources, fmt.Sprintf("%d further package(s) counted but not cited (citation cap %d)", len(pkgs)-maxLayoutCitations, maxLayoutCitations)) + pkgs = pkgs[:maxLayoutCitations] + } + sources = append(sources, pkgs...) + } + return Evidence{ + Status: StatusPartial, + Confidence: confidence, + Sources: dedupeSorted(sources), + } +} + +// convMarkerNames are the in-code work markers recognised as open questions, +// uppercase only. NOTE and OPTIMIZE are deliberately absent: NOTE marks +// explanation rather than unfinished work, and OPTIMIZE is rare enough that its +// false-positive cost exceeds its value. +var convMarkerNames = []string{"TODO", "FIXME", "XXX", "HACK", "BUG"} + +// convMarkerRe matches one recognised marker on a line. The leading class is the +// word boundary that stops TODO matching inside TODOS or todo_list; the trailing +// class admits the two conventional spellings (TODO: and TODO(alice):) plus a +// bare word. The hyphen is excluded from the leading class so the redaction +// placeholder shape (XXX-XXX-XXX) is rejected at every one of its triples — +// without it the last triple matches on its leading hyphen and a support phone +// number becomes a fabricated open question. Built from convMarkerNames so the +// set and the pattern cannot drift, and compiled once. +var convMarkerRe = regexp.MustCompile(`(^|[^A-Za-z0-9_-])(` + strings.Join(convMarkerNames, "|") + `)(:|\(|\s|$)`) + +// maxMarkerCitations caps how many path:line citations the marker scan reports. +// Beyond it the scan keeps counting — the headline stays truthful — but stops +// citing, so a repo with ten thousand markers cannot dump ten thousand lines +// into a brief section. +const maxMarkerCitations = 200 + +// maxMarkerScanBytes caps how much file content the marker scan reads across the +// whole tree. The per-file cap (maxProbeReadBytes) bounds one read and +// maxWalkFiles bounds one walk, but their product does not bound the scan: a +// hostile tree of many large files would hold the probe for hours reading and +// matching them. It reuses maxPlanTotalBytes, the ceiling the pack planner +// already puts on the same multiplication, rather than adding a third spelling +// of it. Spending it is reported in the cited evidence, exactly as reaching the +// walk cap is. +const maxMarkerScanBytes = maxPlanTotalBytes // 512 MiB + +// convBinarySniffBytes is how far into a file the NUL-byte binary heuristic +// looks. It is the conventional prefix test, and needs no extension allow-list. +const convBinarySniffBytes = 8 << 10 + +// convMarkerMediumConfidence is the marker count at which the scan is confident +// the markers are a real seam of open questions rather than a stray note. +const convMarkerMediumConfidence = 10 + +// convIsBinary reports whether data looks like a binary blob: a NUL byte within +// the sniffed prefix. A blob carries no readable markers, so it is skipped. +func convIsBinary(data []byte) bool { + head := data + if len(head) > convBinarySniffBytes { + head = head[:convBinarySniffBytes] + } + return bytes.IndexByte(head, 0) >= 0 +} + +// convOpenQuestionsSource partially grounds "evidence/open-questions" from the +// work markers a team left in its source — the one place every codebase records +// what it knows is unfinished, and the only surviving trace of it in a project +// that never kept a record. Blank when the tree carries none. +// +// The ceiling is StatusPartial by construction: a marker says something is +// unfinished, not what the question is, so markers are a thread to pull rather +// than a framed set of open questions. Volume moves the confidence instead. +type convOpenQuestionsSource struct{} + +func (convOpenQuestionsSource) Section() Section { return "evidence/open-questions" } +func (convOpenQuestionsSource) Tier() Tier { return TierConventions } + +func (s convOpenQuestionsSource) Probe(ctx *SourceContext) Evidence { + return s.probeLimited(ctx, maxMarkerScanBytes) +} + +// probeLimited is Probe with the scan budget injected, so the exhaustion branch +// is exercisable by a test at an affordable scale. The shipped budget stays a +// const: adapters run concurrently, and a mutable package-level budget would be +// shared state between them. +func (convOpenQuestionsSource) probeLimited(ctx *SourceContext, budget int) Evidence { + paths, truncated := ctx.WalkFiles(".") + var ( + citations []string + markers int + files int + scanned int + unread int + ) + for i, p := range paths { + if scanned >= budget { + // Loud staging: the walk found more of the tree than the scan could + // afford to read, so the count below is partial and says so. + unread = len(paths) - i + break + } + // ReadFile carries the whole read contract already — containment, the + // per-file byte cap, regular-file-only, non-blocking open — so an + // oversized or unreadable file is simply skipped. + data, ok := ctx.ReadFile(p) + if !ok { + continue + } + // Charged before the binary test: the bytes are already read and copied, + // so a tree of blobs spends the budget exactly as a tree of source does. + scanned += len(data) + if convIsBinary(data) { + continue + } + hits := 0 + for i, line := range strings.Split(string(data), "\n") { + // One citation per line: the first recognised marker on it. A line + // carrying two markers is still one place to look. + m := convMarkerRe.FindStringSubmatch(line) + if m == nil { + continue + } + hits++ + markers++ + if len(citations) < maxMarkerCitations { + citations = append(citations, fmt.Sprintf("%s:%d (%s)", p, i+1, m[2])) + } + } + if hits > 0 { + files++ + } + } + + if markers == 0 { + searched := []string{ + "in-code work markers (" + strings.Join(convMarkerNames, ", ") + ")", + "every regular file in the tree except " + strings.Join(walkSkipDirs, ", "), + } + // Loud staging on the blank too: a scan the bounds cut short read only + // part of the tree, so "no markers" would be a claim about files that + // were never opened. A blank is a first-class result only while it is + // trustworthy (adr-35), so it says what it did not read. + question := "What did this project know was unfinished? Its source carries no work markers." + if truncated || unread > 0 { + if truncated { + searched = append(searched, fmt.Sprintf("stopped at the walk cap (%d entries, %d levels deep); the rest of the tree was not walked", maxWalkFiles, maxWalkDepth)) + } + if unread > 0 { + searched = append(searched, fmt.Sprintf("stopped at the %d-byte read budget; %d further file(s) were not read", budget, unread)) + } + question = "What did this project know was unfinished? No work markers in the part of the tree the scan reached, and the scan did not reach all of it." + } + return blank(searched, question) + } + + sources := []string{fmt.Sprintf("%d work marker(s) across %d file(s)", markers, files)} + // Loud staging: a partial scan says so in its own evidence, so a rescuer + // never mistakes a truncated count for the whole tree. + if truncated { + sources = append(sources, fmt.Sprintf("scan truncated at the walk cap (%d entries, %d levels deep); markers beyond it were not read", maxWalkFiles, maxWalkDepth)) + } + if unread > 0 { + sources = append(sources, fmt.Sprintf("scan stopped at the %d-byte read budget; %d further file(s) were not read", budget, unread)) + } + if markers > len(citations) { + sources = append(sources, fmt.Sprintf("%d further marker(s) counted but not cited (citation cap %d)", markers-len(citations), maxMarkerCitations)) + } + sources = append(sources, citations...) + + confidence := ConfidenceLow + if markers >= convMarkerMediumConfidence { + confidence = ConfidenceMedium + } + return Evidence{ + Status: StatusPartial, + Confidence: confidence, + Sources: dedupeSorted(sources), + } +} + // convIssuesSource partially grounds "activity/issues" from a checked-in issues // ledger, an issues directory, or issue templates. Blank when none is present. type convIssuesSource struct{} diff --git a/internal/core/lifeboat/sources_conventions_test.go b/internal/core/lifeboat/sources_conventions_test.go index 2307af9..47e2a86 100644 --- a/internal/core/lifeboat/sources_conventions_test.go +++ b/internal/core/lifeboat/sources_conventions_test.go @@ -1,8 +1,10 @@ package lifeboat import ( + "fmt" "os" "path/filepath" + "strings" "testing" ) @@ -307,6 +309,737 @@ func TestConvProbeIsDeterministic(t *testing.T) { } } +// convMarkerFixture builds the intent's motivating repository: no .abcd record, +// no git, but source files carrying the work markers the last team left behind. +// go.mod is deliberate — it is the conventions tier gate's sentinel, without +// which the whole Tier-1 set is skipped and every conventions section blanks. +func convMarkerFixture(t *testing.T) string { + t.Helper() + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + "go.mod": "module example.com/wreck\n\ngo 1.22\n", + "retry.go": "package wreck\n\nfunc retry() {\n\t// TODO: handle the retry case\n}\n", + "store.go": "package wreck\n\n// FIXME(alice): this leaks a connection\nfunc store() {}\n", + }) + return dir +} + +// TestConvOpenQuestionsPartialFromWorkMarkers is the intent's headline +// behaviour: a record-less repository whose source is dense with TODO/FIXME no +// longer reports "no open questions on record" — the section comes back +// non-blank at the conventions tier, citing the file and line of each marker. +func TestConvOpenQuestionsPartialFromWorkMarkers(t *testing.T) { + cov, err := Probe(convMarkerFixture(t)) + if err != nil { + t.Fatal(err) + } + sc := findSection(t, cov, "evidence/open-questions") + if sc.Status != StatusPartial { + t.Fatalf("evidence/open-questions status = %s, want partial", sc.Status) + } + if sc.Tier != TierConventions { + t.Errorf("evidence/open-questions tier = %s, want %s", sc.Tier, TierConventions) + } + if !containsSource(sc.Evidence, "retry.go:4 (TODO)") { + t.Errorf("evidence = %v, want the TODO cited at retry.go:4", sc.Evidence) + } + if !containsSource(sc.Evidence, "store.go:3 (FIXME)") { + t.Errorf("evidence = %v, want the FIXME cited at store.go:3", sc.Evidence) + } + if sc.Confidence == "" { + t.Error("non-blank evidence/open-questions carries no confidence") + } +} + +// TestConvOpenQuestionsCeilingIsPartial holds the status ceiling: markers are a +// thread, not a framed set of open questions, so no quantity of them ever +// grounds the section. Volume moves the confidence instead. +func TestConvOpenQuestionsCeilingIsPartial(t *testing.T) { + dir := t.TempDir() + var body strings.Builder + body.WriteString("package wreck\n") + for i := 0; i < 20; i++ { + fmt.Fprintf(&body, "// TODO: unfinished thing %d\n", i) + } + writeTree(t, dir, map[string]string{ + "go.mod": "module example.com/wreck\n\ngo 1.22\n", + "many.go": body.String(), + }) + ctx, err := newSourceContext(dir) + if err != nil { + t.Fatal(err) + } + defer ctx.Close() + + ev := convSourceForSection(t, "evidence/open-questions").Probe(ctx) + if ev.Status != StatusPartial { + t.Fatalf("20 markers give status %s, want partial (grounded is not reachable)", ev.Status) + } + if ev.Confidence != ConfidenceMedium { + t.Errorf("confidence = %s at 20 markers, want %s", ev.Confidence, ConfidenceMedium) + } +} + +// TestConvOpenQuestionsBlankWithoutMarkers holds the blank contract for the +// marker scan: a tree with no work markers is an honest blank naming the markers +// and the trees it searched, never a fabricated result. +func TestConvOpenQuestionsBlankWithoutMarkers(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + "go.mod": "module example.com/tidy\n\ngo 1.22\n", + "README.md": "# Tidy\n\nA project that left nothing unfinished.\n", + "tidy.go": "package tidy\n\n// NOTE: this explains, it does not ask.\nfunc tidy() {}\n", + }) + ctx, err := newSourceContext(dir) + if err != nil { + t.Fatal(err) + } + defer ctx.Close() + + ev := convSourceForSection(t, "evidence/open-questions").Probe(ctx) + if ev.Status != StatusBlank { + t.Fatalf("marker-free tree gives status %s, want blank (evidence %v)", ev.Status, ev.Sources) + } + if len(ev.Searched) == 0 { + t.Error("blank evidence/open-questions names nothing it searched") + } + if ev.Question == "" { + t.Error("blank evidence/open-questions carries no question for a human") + } +} + +// TestConvOpenQuestionsIgnoresRedactionPlaceholders holds the honest-blank +// contract against the one shape that reads like a marker and is not one: the +// XXX-XXX-XXX redaction placeholder. Fabricated evidence is the failure a tool +// built around honest blanks cannot afford, so a tree whose only uppercase +// triples are redactions must still come back blank. +func TestConvOpenQuestionsIgnoresRedactionPlaceholders(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + "go.mod": "module example.com/support\n\ngo 1.22\n", + "docs/support.md": "# Support\n\nCall us on XXX-XXX-XXX for details.\n", + "docs/redacted.md": "account: XXX-XXX-XXX\n", + "docs/trailing.md": "XXX-XXX-XXX\n", + "docs/customers.md": "Reach the team on XXX-XXX-XXX\n", + }) + ctx, err := newSourceContext(dir) + if err != nil { + t.Fatal(err) + } + defer ctx.Close() + + ev := convSourceForSection(t, "evidence/open-questions").Probe(ctx) + if ev.Status != StatusBlank { + t.Fatalf("redaction placeholders give status %s, want blank; fabricated evidence %v", ev.Status, ev.Sources) + } +} + +// TestConvMarkerRePinsTheRecognisedSpellings pins the marker pattern against the +// spellings it must accept and the near-misses it must reject, so the word +// boundary cannot be loosened or tightened without a test saying so. +func TestConvMarkerRePinsTheRecognisedSpellings(t *testing.T) { + match := []string{ + "// TODO: handle the retry case", + "//TODO: no space after the slashes", + "# TODO", + "- TODO: a list item", + "* FIXME check this", + "FIXME(alice): leaks a connection", + "-- BUG --", + "// HACK around the driver", + "XXX", + } + reject := []string{ + "redacted: XXX-XXX-XXX", + "call XXX-XXX-XXX for details", + "XXX-XXX-XXX", + "TODOS are not markers", + "todo_list := nil", + } + for _, line := range match { + if convMarkerRe.FindStringSubmatch(line) == nil { + t.Errorf("convMarkerRe rejects the marker line %q", line) + } + } + for _, line := range reject { + if m := convMarkerRe.FindStringSubmatch(line); m != nil { + t.Errorf("convMarkerRe matches %q as a %s marker", line, m[2]) + } + } +} + +// TestConvOpenQuestionsIgnoresSkippedTrees proves the adapter inherits the +// walk's skip set: a dependency's or a generator's TODO is not this team's open +// question, so it is never cited. +func TestConvOpenQuestionsIgnoresSkippedTrees(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + "go.mod": "module example.com/wreck\n\ngo 1.22\n", + "keep.go": "package wreck\n\n// TODO: our own question\n", + ".git/hooks/pre-commit": "# TODO: git internals\n", + "node_modules/pkg/index.js": "// TODO: a dependency's marker\n", + "vendor/dep/dep.go": "// TODO: vendored\n", + "generated/api.pb.go": "// TODO: generated\n", + }) + ctx, err := newSourceContext(dir) + if err != nil { + t.Fatal(err) + } + defer ctx.Close() + + ev := convSourceForSection(t, "evidence/open-questions").Probe(ctx) + if ev.Status != StatusPartial { + t.Fatalf("status = %s, want partial", ev.Status) + } + if !containsSource(ev.Sources, "keep.go:3 (TODO)") { + t.Errorf("evidence = %v, want keep.go:3 cited", ev.Sources) + } + for _, s := range ev.Sources { + for _, skipped := range walkSkipDirs { + if strings.Contains(s, skipped+"/") { + t.Errorf("evidence cites %q from the skipped %s/ tree", s, skipped) + } + } + } +} + +// TestConvOpenQuestionsStopsAtTheScanBudget proves the scan is bounded in bytes, +// not only in files: the per-file cap and the walk cap multiply, so without an +// aggregate budget a hostile tree of large files holds the probe for hours. The +// scan must stop when the budget is spent and say so in its own evidence, so a +// rescuer never mistakes a partial count for the whole tree. The branch is +// exercised through the same code path at an affordable scale — the shipped +// budget stays a const, so concurrent adapters share no mutable state. +func TestConvOpenQuestionsStopsAtTheScanBudget(t *testing.T) { + dir := t.TempDir() + files := map[string]string{"go.mod": "module example.com/vast\n\ngo 1.22\n"} + for i := 0; i < 6; i++ { + files[fmt.Sprintf("f%d.go", i)] = strings.Repeat("x", 1000) + "\n// TODO: unfinished\n" + } + writeTree(t, dir, files) + ctx, err := newSourceContext(dir) + if err != nil { + t.Fatal(err) + } + defer ctx.Close() + + src := convSourceForSection(t, "evidence/open-questions").(convOpenQuestionsSource) + full := src.probeLimited(ctx, maxMarkerScanBytes) + if !containsSource(full.Sources, "6 work marker(s) across 6 file(s)") { + t.Fatalf("unbudgeted scan evidence = %v, want all 6 markers counted", full.Sources) + } + + // A budget of two files' worth stops the scan partway through the walk. + capped := src.probeLimited(ctx, 2100) + if containsSource(capped.Sources, "6 work marker(s) across 6 file(s)") { + t.Errorf("scan ran past its 2100-byte budget: %v", capped.Sources) + } + said := false + for _, s := range capped.Sources { + if strings.Contains(s, "read budget") { + said = true + } + } + if !said { + t.Errorf("budget-exhausted scan does not report it in its evidence: %v", capped.Sources) + } +} + +// TestConvOpenQuestionsBlankAdmitsAPartialScan closes the honesty gap the scan +// bounds open up: when the budget or the walk cap stops the scan before the end +// of the tree and nothing was found in the part that was read, "its source +// carries no work markers" is a claim about files the adapter never opened. A +// blank is a first-class result (adr-35) precisely because it is trustworthy, so +// a blank drawn from a partial read must say the read was partial. +func TestConvOpenQuestionsBlankAdmitsAPartialScan(t *testing.T) { + dir := t.TempDir() + files := map[string]string{} + // Marker-free files sort first, so a small budget is spent on them and the + // one marker-bearing file is never reached. + for i := 0; i < 4; i++ { + files[fmt.Sprintf("a%d.go", i)] = strings.Repeat("x", 1000) + "\n" + } + files["z.go"] = "// TODO: the marker the scan never reaches\n" + writeTree(t, dir, files) + ctx, err := newSourceContext(dir) + if err != nil { + t.Fatal(err) + } + defer ctx.Close() + + src := convSourceForSection(t, "evidence/open-questions").(convOpenQuestionsSource) + capped := src.probeLimited(ctx, 2100) + if capped.Status != StatusBlank { + t.Fatalf("budget-exhausted marker-free prefix = %s, want blank", capped.Status) + } + if strings.Contains(capped.Question, "carries no work markers") { + t.Errorf("blank claims the whole tree is marker-free after a partial scan: %q", capped.Question) + } + said := false + for _, s := range capped.Searched { + if strings.Contains(s, "read budget") || strings.Contains(s, "walk cap") { + said = true + } + } + if !said { + t.Errorf("blank from a partial scan does not say the scan was partial: searched = %v", capped.Searched) + } + + // The unbudgeted scan over the same tree still finds the marker, so the + // fixture proves truncation and not an unrelated miss. + if full := src.probeLimited(ctx, maxMarkerScanBytes); full.Status != StatusPartial { + t.Fatalf("unbudgeted scan = %s, want partial (z.go carries a TODO)", full.Status) + } +} + +// convArchitectureProse is an ARCHITECTURE.md body comfortably above the +// convGroundedProseBytes threshold — real architecture prose rather than a stub. +const convArchitectureProse = "# Architecture\n\n" + + "The wreck is split into a transport-agnostic core and a thin CLI shell. " + + "The core owns every decision; the shell only formats what the core returns, " + + "so a second front door costs nothing but its formatter.\n" + +// TestConvNamingPartialFromNamingDoc is the intent's headline naming behaviour: a +// record-less repository carrying a dedicated NAMING.md no longer blanks +// "constraints/naming" — the section comes back non-blank at the conventions +// tier, citing the file it read. +func TestConvNamingPartialFromNamingDoc(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + "go.mod": "module example.com/wreck\n\ngo 1.22\n", + "NAMING.md": "# Naming\n\nA voyage is never called a run.\n", + }) + cov, err := Probe(dir) + if err != nil { + t.Fatal(err) + } + sc := findSection(t, cov, "constraints/naming") + if sc.Status != StatusPartial { + t.Fatalf("constraints/naming status = %s, want partial", sc.Status) + } + if sc.Tier != TierConventions { + t.Errorf("constraints/naming tier = %s, want %s", sc.Tier, TierConventions) + } + if !containsSource(sc.Evidence, "NAMING.md") { + t.Errorf("evidence = %v, want NAMING.md cited", sc.Evidence) + } + if sc.Confidence != ConfidenceMedium { + t.Errorf("confidence = %s for a dedicated naming document, want %s", sc.Confidence, ConfidenceMedium) + } +} + +// TestConvNamingPartialFromDocsNamingPage confirms the docs/ prefix idiom: a +// naming page under docs/ grounds the section when no root NAMING.md exists. +func TestConvNamingPartialFromDocsNamingPage(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + "go.mod": "module example.com/wreck\n\ngo 1.22\n", + "docs/naming-conventions.md": "# Naming conventions\n\nPackages are singular nouns.\n", + "docs/unrelated-appendix.md": "# Appendix\n\nNothing about naming.\n", + }) + ctx, err := newSourceContext(dir) + if err != nil { + t.Fatal(err) + } + defer ctx.Close() + + ev := convSourceForSection(t, "constraints/naming").Probe(ctx) + if ev.Status != StatusPartial { + t.Fatalf("constraints/naming status = %s, want partial", ev.Status) + } + if !containsSource(ev.Sources, "docs/naming-conventions.md") { + t.Errorf("evidence = %v, want the docs/ naming page cited", ev.Sources) + } + if ev.Confidence != ConfidenceMedium { + t.Errorf("confidence = %s for a docs/ naming page, want %s", ev.Confidence, ConfidenceMedium) + } +} + +// TestConvNamingFallsBackToGlossaryWithoutDisplacingIt holds the distinctness +// contract on the fixture that could collapse the two sections into one: a +// repository whose only vocabulary signal is a GLOSSARY.md. The glossary section +// stays exactly what convGlossarySource already made it, and naming is a visibly +// weaker derived reading — lower confidence, and a citation qualified as the +// fallback it is — never a duplicate row. +func TestConvNamingFallsBackToGlossaryWithoutDisplacingIt(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + "GLOSSARY.md": "# Glossary\n\n**Voyage** — one packed lifeboat.\n", + }) + cov, err := Probe(dir) + if err != nil { + t.Fatal(err) + } + + naming := findSection(t, cov, "constraints/naming") + if naming.Status != StatusPartial { + t.Fatalf("constraints/naming status = %s, want partial", naming.Status) + } + if naming.Confidence != ConfidenceLow { + t.Errorf("naming confidence = %s on a glossary fallback, want %s", naming.Confidence, ConfidenceLow) + } + if len(naming.Evidence) != 1 || !strings.Contains(naming.Evidence[0], "GLOSSARY.md") { + t.Fatalf("naming evidence = %v, want the glossary cited", naming.Evidence) + } + if !strings.Contains(naming.Evidence[0], "glossary fallback") { + t.Errorf("naming cites %q without the glossary-fallback qualifier", naming.Evidence[0]) + } + + glossary := findSection(t, cov, "glossary") + if glossary.Status != StatusPartial || glossary.Confidence != ConfidenceMedium { + t.Fatalf("glossary = %s/%s, want partial/medium (convGlossarySource unchanged)", + glossary.Status, glossary.Confidence) + } + if !containsSource(glossary.Evidence, "GLOSSARY.md") { + t.Errorf("glossary evidence = %v, want GLOSSARY.md cited bare", glossary.Evidence) + } + + // The two rows must differ in both dimensions: same file, different reading. + if naming.Confidence == glossary.Confidence { + t.Errorf("naming and glossary share confidence %s; naming must be the weaker reading", naming.Confidence) + } + if naming.Evidence[0] == glossary.Evidence[0] { + t.Errorf("naming and glossary cite the identical string %q; naming must be visibly derived", naming.Evidence[0]) + } +} + +// TestConventionSourcesHaveOneAdapterPerSection guards the registry against a +// duplicate adapter: two adapters for one section race for the same coverage row +// and the loser's evidence silently vanishes. It pins the sections this intent +// touches — including glossary, which keeps its single pre-existing adapter. +func TestConventionSourcesHaveOneAdapterPerSection(t *testing.T) { + count := map[Section]int{} + for _, s := range conventionSources() { + count[s.Section()]++ + } + for _, section := range []Section{ + "glossary", "constraints/naming", "internals", "evidence/open-questions", + } { + if count[section] != 1 { + t.Errorf("conventionSources has %d adapters for %s, want exactly 1", count[section], section) + } + } + for section, n := range count { + if n > 1 { + t.Errorf("conventionSources has %d adapters for %s, want at most 1", n, section) + } + } +} + +// TestConvInternalsCitesArchitectureAndLayout is the intent's headline internals +// behaviour: a record-less repository with an ARCHITECTURE.md and a package tree +// grounds "internals" from both signals at once, citing the document and the +// packages a rescuer must navigate. +func TestConvInternalsCitesArchitectureAndLayout(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + "go.mod": "module example.com/wreck\n\ngo 1.22\n", + "ARCHITECTURE.md": convArchitectureProse, + "internal/core/voyage.go": "package core\n", + "internal/surface/cli/main.go": "package cli\n", + "cmd/wreck/main.go": "package main\n", + }) + cov, err := Probe(dir) + if err != nil { + t.Fatal(err) + } + sc := findSection(t, cov, "internals") + if sc.Status != StatusPartial { + t.Fatalf("internals status = %s, want partial", sc.Status) + } + if sc.Tier != TierConventions { + t.Errorf("internals tier = %s, want %s", sc.Tier, TierConventions) + } + if sc.Confidence != ConfidenceHigh { + t.Errorf("internals confidence = %s with real architecture prose, want %s", sc.Confidence, ConfidenceHigh) + } + if !containsSource(sc.Evidence, "ARCHITECTURE.md") { + t.Errorf("evidence = %v, want ARCHITECTURE.md cited", sc.Evidence) + } + for _, want := range []string{"internal/core/", "internal/surface/", "cmd/wreck/"} { + if !containsSource(sc.Evidence, want) { + t.Errorf("evidence = %v, want the layout entry %s cited", sc.Evidence, want) + } + } +} + +// TestConvInternalsLayoutOnlyIsLowConfidence holds the weakest internals signal: +// a package tree with no architecture document still says something about the +// shape of the system, at low confidence and citing the layout alone. The fixture +// carries no other conventional sentinel, so it also proves the tier gate admits +// a repository whose only Tier-1 signal is its layout. +func TestConvInternalsLayoutOnlyIsLowConfidence(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + "internal/store/store.go": "package store\n", + "pkg/api/api.go": "package api\n", + }) + cov, err := Probe(dir) + if err != nil { + t.Fatal(err) + } + sc := findSection(t, cov, "internals") + if sc.Status != StatusPartial { + t.Fatalf("internals status = %s, want partial (evidence %v)", sc.Status, sc.Evidence) + } + if sc.Confidence != ConfidenceLow { + t.Errorf("internals confidence = %s with layout only, want %s", sc.Confidence, ConfidenceLow) + } + for _, want := range []string{"internal/store/", "pkg/api/"} { + if !containsSource(sc.Evidence, want) { + t.Errorf("evidence = %v, want the layout entry %s cited", sc.Evidence, want) + } + } + for _, got := range sc.Evidence { + if strings.Contains(strings.ToLower(got), "architecture") { + t.Errorf("layout-only internals cites %q; no architecture document exists", got) + } + } +} + +// TestConvInternalsThinArchitectureIsMedium separates a written architecture +// chapter from a placeholder: a document below the prose threshold is still +// evidence, but only at medium confidence. +func TestConvInternalsThinArchitectureIsMedium(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + "go.mod": "module example.com/wreck\n\ngo 1.22\n", + "ARCHITECTURE.md": "# Architecture\n\nTBD.\n", + }) + ctx, err := newSourceContext(dir) + if err != nil { + t.Fatal(err) + } + defer ctx.Close() + + ev := convSourceForSection(t, "internals").Probe(ctx) + if ev.Status != StatusPartial { + t.Fatalf("internals status = %s, want partial", ev.Status) + } + if ev.Confidence != ConfidenceMedium { + t.Errorf("internals confidence = %s for a thin architecture doc, want %s", ev.Confidence, ConfidenceMedium) + } + if len(ev.Sources) == 0 { + t.Fatal("partial internals cites no evidence") + } +} + +// TestConvInternalsMediumFromArchitectureDirectory confirms the Diataxis +// fallback: a docs/explanation tree holding Markdown is architecture prose spread +// across files, and counts at medium confidence. +func TestConvInternalsMediumFromArchitectureDirectory(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + "go.mod": "module example.com/wreck\n\ngo 1.22\n", + "docs/explanation/model.md": "# The model\n\nHow the pieces fit.\n", + "docs/explanation/notes.txt": "not markdown\n", + }) + ctx, err := newSourceContext(dir) + if err != nil { + t.Fatal(err) + } + defer ctx.Close() + + ev := convSourceForSection(t, "internals").Probe(ctx) + if ev.Status != StatusPartial { + t.Fatalf("internals status = %s, want partial", ev.Status) + } + if ev.Confidence != ConfidenceMedium { + t.Errorf("internals confidence = %s for a doc directory, want %s", ev.Confidence, ConfidenceMedium) + } + if !containsSource(ev.Sources, "docs/explanation/") { + t.Errorf("evidence = %v, want the docs/explanation/ tree cited", ev.Sources) + } +} + +// TestConvInternalsBoundsItsLayoutCitations holds both layout bounds at once: the +// citation cap keeps a vast monorepo from dumping every package into a section +// while the count stays truthful, and a walk cut short by the file cap says so in +// its own evidence rather than reading as a complete survey. +func TestConvInternalsBoundsItsLayoutCitations(t *testing.T) { + dir := t.TempDir() + files := map[string]string{"go.mod": "module example.com/vast\n\ngo 1.22\n"} + for i := 0; i < maxLayoutCitations+5; i++ { + files[fmt.Sprintf("internal/p%03d/p.go", i)] = "package p\n" + } + writeTree(t, dir, files) + ctx, err := newSourceContext(dir) + if err != nil { + t.Fatal(err) + } + defer ctx.Close() + + src := convSourceForSection(t, "internals").(convInternalsSource) + full := src.probeLimited(ctx, maxWalkFiles) + cited := 0 + for _, s := range full.Sources { + if strings.HasPrefix(s, "internal/p") { + cited++ + } + } + if cited != maxLayoutCitations { + t.Errorf("cited %d layout entries, want the cap of %d", cited, maxLayoutCitations) + } + if !containsSource(full.Sources, fmt.Sprintf("5 further package(s) counted but not cited (citation cap %d)", maxLayoutCitations)) { + t.Errorf("evidence = %v, want the 5 uncited packages reported as a count", full.Sources) + } + + // A walk cut short must be visible in the evidence, exactly as the marker + // scan's truncation is. + capped := src.probeLimited(ctx, 3) + said := false + for _, s := range capped.Sources { + if strings.Contains(s, "walk cap") { + said = true + } + } + if !said { + t.Errorf("truncated layout scan does not report it in its evidence: %v", capped.Sources) + } +} + +// TestConvInternalsStagesAScanThatReachedNoPackage holds the loud staging where +// it is easiest to lose: a walk the cap cut short before it ever reached a +// source root found no packages at all, so both the blank and the +// architecture-only reading would otherwise describe a tree the scan never +// finished reading. A blank is a first-class result only while it is trustworthy +// (adr-35), so it must say the layout scan stopped early — and so must the +// non-blank reading beside it. +func TestConvInternalsStagesAScanThatReachedNoPackage(t *testing.T) { + // assets/ sorts before every source root, so a cap of ten files is spent + // before the walk reaches src/ — exactly the shape of a repository whose + // code sits behind a large data or frontend tree. + bulk := func(extra map[string]string) map[string]string { + files := map[string]string{"src/pkg/code.go": "package pkg\n"} + for i := 0; i < 40; i++ { + files[fmt.Sprintf("assets/a%02d.bin", i)] = "x\n" + } + for rel, content := range extra { + files[rel] = content + } + return files + } + + t.Run("blank", func(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, bulk(nil)) + ctx, err := newSourceContext(dir) + if err != nil { + t.Fatal(err) + } + defer ctx.Close() + + ev := convSourceForSection(t, "internals").(convInternalsSource).probeLimited(ctx, 10) + if ev.Status != StatusBlank { + t.Fatalf("internals status = %s, want blank (evidence %v)", ev.Status, ev.Sources) + } + said := false + for _, s := range ev.Searched { + if strings.Contains(s, "walk cap") { + said = true + } + } + if !said { + t.Errorf("blank internals searched %v without admitting the walk cap cut the layout scan short", ev.Searched) + } + if !strings.Contains(ev.Question, "reached") { + t.Errorf("blank internals asks %q, claiming no layout in a tree it did not finish reading", ev.Question) + } + }) + + t.Run("architecture only", func(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, bulk(map[string]string{"ARCHITECTURE.md": convArchitectureProse})) + ctx, err := newSourceContext(dir) + if err != nil { + t.Fatal(err) + } + defer ctx.Close() + + ev := convSourceForSection(t, "internals").(convInternalsSource).probeLimited(ctx, 10) + if ev.Status != StatusPartial { + t.Fatalf("internals status = %s, want partial", ev.Status) + } + said := false + for _, s := range ev.Sources { + if strings.Contains(s, "walk cap") { + said = true + } + } + if !said { + t.Errorf("evidence = %v, want the truncated layout scan reported even though it found no package", ev.Sources) + } + }) +} + +// TestConvNamingAndInternalsBlankWithoutSignals holds the "a blank is a result" +// contract for both new adapters: a repository with neither naming nor +// architecture documentation and no recognisable layout returns honest blanks +// naming what was searched and the question a human must answer — never a +// fabricated section. +func TestConvNamingAndInternalsBlankWithoutSignals(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + "README.md": "# Bare\n\nA project that documented nothing else.\n", + "main.go": "package main\n\nfunc main() {}\n", + }) + ctx, err := newSourceContext(dir) + if err != nil { + t.Fatal(err) + } + defer ctx.Close() + + for _, section := range []Section{"constraints/naming", "internals"} { + ev := convSourceForSection(t, section).Probe(ctx) + if ev.Status != StatusBlank { + t.Fatalf("%s status = %s, want blank (evidence %v)", section, ev.Status, ev.Sources) + } + if len(ev.Searched) == 0 { + t.Errorf("blank %s names nothing it searched", section) + } + if ev.Question == "" { + t.Errorf("blank %s carries no question for a human", section) + } + } +} + +// TestHasConventionsCoversNamingAndArchitecture is the tier-gate regression the +// new adapters open up: the gate skips every adapter of an absent tier, so a +// repository whose only conventional signal is a NAMING.md or an ARCHITECTURE.md +// would have the whole Tier-1 set skipped and its section blanked falsely. +func TestHasConventionsCoversNamingAndArchitecture(t *testing.T) { + for _, tc := range []struct { + file string + content string + section Section + }{ + {"NAMING.md", "# Naming\n\nA voyage is never called a run.\n", "constraints/naming"}, + {"ARCHITECTURE.md", convArchitectureProse, "internals"}, + } { + t.Run(tc.file, func(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{tc.file: tc.content}) + cov, err := Probe(dir) + if err != nil { + t.Fatal(err) + } + found := false + for _, tr := range cov.TiersPresent { + if tr == TierConventions { + found = true + } + } + if !found { + t.Fatalf("tiers_present = %v omits TierConventions for a %s-only repo", cov.TiersPresent, tc.file) + } + sc := findSection(t, cov, tc.section) + if sc.Status == StatusBlank { + t.Errorf("%s blanked in a %s-only repo; the tier gate skipped its adapter", tc.section, tc.file) + } + }) + } +} + // containsSource reports whether want appears in sources. func containsSource(sources []string, want string) bool { for _, s := range sources {