* chore(#3484): adr-0174 behavior carry-forward amendment and merge gate * chore(#3484): regen example context index for new ruleset predicates * chore(#3484): review fixes - amendment heading per contributor-standards, helper-based fixtures --------- Co-authored-by: sim <sim@local>
This commit is contained in:
@@ -593,6 +593,8 @@ The prompt-level data/instruction isolation seam for untrusted web/document ingr
|
|||||||
`RULESET.CONTRIB.CLASSIFY.fix=requires confirmed-bug before implementation (legacy 'confirmed' label is back-compat only for duplicate-sweep exemption, not a valid implementation gate)`
|
`RULESET.CONTRIB.CLASSIFY.fix=requires confirmed-bug before implementation (legacy 'confirmed' label is back-compat only for duplicate-sweep exemption, not a valid implementation gate)`
|
||||||
`RULESET.CONTRIB.CLASSIFY.enhancement=requires approved-enhancement before implementation`
|
`RULESET.CONTRIB.CLASSIFY.enhancement=requires approved-enhancement before implementation`
|
||||||
`RULESET.CONTRIB.CLASSIFY.feature=requires approved-feature before implementation`
|
`RULESET.CONTRIB.CLASSIFY.feature=requires approved-feature before implementation`
|
||||||
|
`RULESET.CONTRIB.GATE.DELETED-MODULE-DISPOSITION=a PR deleting a module/package/lineage with a surviving counterpart carries a symbol-level disposition table for the deleted side — every declared name (exported or local) marked migrated (location named) | renamed → X (conventions normalized: verifyX → cmdVerifyX) | dropped because Y; SCREAMING_CASE constants ranked first (policy lives there; both confirmed ADR-0174 losses were bounds); the surviving tests passing is NOT evidence — the surviving tests belong to the surviving side (#3484, ADR-0174 behavior-carry-forward amendment; losses: #3427 shortFormToId tier + unresolved-deps warning, #3477 regexForKeyLinkPattern 512-char cap + nested-quantifier screen, epic #3473 B5 MAX_JSON_SEARCH_DEPTH=48). Method with mandatory positive control: docs/how-to/audit-a-retiring-lineage.md`
|
||||||
|
`RULESET.CONTRIB.GAP-TRACKING=a known gap is tracked only by an issue — a code comment, TODO, or changeset note is attached to code and dies with the refactor that deletes it, while the gap stays real (worked example: archived changeset clever-yaks-cheer.md recorded the shortFormToId backfill as 'tracked as a follow-up parity gap' + a // KNOWN GAP: comment marked the call site; the SDK retirement deleted both, no issue was opened, and it surfaced a year later as #3427). If you write 'tracked as a follow-up' anywhere, open the issue first and cite its number. Prose twin of #3473 criterion B6 (a guard needs a retirement condition)`
|
||||||
|
|
||||||
## Workspace seams (machine-oriented predicates)
|
## Workspace seams (machine-oriented predicates)
|
||||||
|
|
||||||
|
|||||||
@@ -198,6 +198,52 @@ Contributor requirements (summary):
|
|||||||
- **CI must pass** — all configured matrix jobs must be green. Node 24 is the compatibility floor and primary target; Node 26 compatibility must be preserved for code and tests even when a Node 26 CI lane is not yet available.
|
- **CI must pass** — all configured matrix jobs must be green. Node 24 is the compatibility floor and primary target; Node 26 compatibility must be preserved for code and tests even when a Node 26 CI lane is not yet available.
|
||||||
- **Scope matches the approved issue** — if your PR does more than what the issue describes, the extra changes will be asked to be removed or moved to a new issue
|
- **Scope matches the approved issue** — if your PR does more than what the issue describes, the extra changes will be asked to be removed or moved to a new issue
|
||||||
|
|
||||||
|
### Deleting a module that has a surviving counterpart — disposition required
|
||||||
|
|
||||||
|
A consolidation PR that deletes a module, package, or whole lineage — where any part of
|
||||||
|
the deleted side has a surviving counterpart in the tree — must carry a **symbol-level
|
||||||
|
disposition table for the deleted side**: for every declared name (exported *or* local),
|
||||||
|
one of:
|
||||||
|
|
||||||
|
- `migrated` — the behavior exists at a named location in the surviving tree,
|
||||||
|
- `renamed → <new name>` — including convention drift (`verifyX` → `cmdVerifyX`,
|
||||||
|
`preserveExistingProgress` → `shouldPreserveExistingProgress`),
|
||||||
|
- `dropped because <reason>` — a deliberate deletion, with the reason stated.
|
||||||
|
|
||||||
|
Rank **SCREAMING_CASE constants first** — that is where policy lives; both confirmed
|
||||||
|
losses from the one retirement this repo has performed were bounds
|
||||||
|
([#3484](https://github.com/open-gsd/gsd-core/issues/3484)). Local-variable renames are
|
||||||
|
noise and may be summarized.
|
||||||
|
|
||||||
|
**"The surviving tests pass" is not evidence.** The surviving tests belong to the
|
||||||
|
surviving side and cannot observe behavior only the deleted side had. ADR-0174 retired
|
||||||
|
the SDK package and deleted ADR-3524's parity apparatus in the same migration; three
|
||||||
|
invariants went into the bin with the package and nobody noticed for months (#3427,
|
||||||
|
#3477, epic #3473 B5) — see ADR-0174's behavior-carry-forward amendment (#3484) for the record.
|
||||||
|
|
||||||
|
**The audit is a method, not a guess.** The calibrated procedure — pairing by basename,
|
||||||
|
comment-stripped declared-name diffing, rename normalization, SCREAMING_CASE-first
|
||||||
|
ranking, and a **mandatory positive control** (the audit must re-detect a known loss or
|
||||||
|
it is not calibrated; an export-only scan misses `shortFormToId`, which was a *local*
|
||||||
|
`const` in a surviving function) — is written up step by step in
|
||||||
|
[`docs/how-to/audit-a-retiring-lineage.md`](docs/how-to/audit-a-retiring-lineage.md).
|
||||||
|
|
||||||
|
### A known gap gets an issue — a comment is not a tracking mechanism
|
||||||
|
|
||||||
|
Recording a known-but-unfixed gap in a code comment, a changeset note, or a TODO, and
|
||||||
|
treating that as *tracking* it, is an anti-pattern: all three are attached to code, and
|
||||||
|
a refactor deletes the code they annotate while the gap stays real. The worked example
|
||||||
|
is `.changeset/archived/clever-yaks-cheer.md` (PR #3798): it accurately recorded the
|
||||||
|
`shortFormToId` backfill as "tracked as a follow-up parity gap", a `// KNOWN GAP:`
|
||||||
|
comment marked the call site, then the SDK retirement deleted both — and no issue was
|
||||||
|
ever opened. The gap surfaced a year later as production bug #3427.
|
||||||
|
|
||||||
|
The rule: **a gap that matters gets an issue. A gap with no issue is not tracked.** If
|
||||||
|
you find yourself writing "tracked as a follow-up" in a comment or a changeset body,
|
||||||
|
open the issue first, then cite its number. This is the prose-level twin of epic #3473's
|
||||||
|
criterion B6 (a guard needs a retirement condition): the tracking mechanism must
|
||||||
|
survive the artifact it is attached to.
|
||||||
|
|
||||||
## CHANGELOG Entries — Drop a Fragment
|
## CHANGELOG Entries — Drop a Fragment
|
||||||
|
|
||||||
**Do not edit `CHANGELOG.md` directly.** Two PRs that both append to a `### Fixed` block always conflict on merge — git can't pick a serialization order without a human. Instead, every PR with user-facing changes drops a fragment file in `.changeset/`.
|
**Do not edit `CHANGELOG.md` directly.** Two PRs that both append to a `### Fixed` block always conflict on merge — git can't pick a serialization order without a human. Instead, every PR with user-facing changes drops a fragment file in `.changeset/`.
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
{
|
{
|
||||||
"schemaVersion": 1,
|
"schemaVersion": 1,
|
||||||
"count": 261,
|
"count": 263,
|
||||||
"classes": {
|
"classes": {
|
||||||
"ARCH": 1,
|
"ARCH": 1,
|
||||||
"CI": 2,
|
"CI": 2,
|
||||||
@@ -17,7 +17,7 @@
|
|||||||
"PROC": 14,
|
"PROC": 14,
|
||||||
"PROHIB": 10,
|
"PROHIB": 10,
|
||||||
"RELEASE-NOTES": 31,
|
"RELEASE-NOTES": 31,
|
||||||
"RULESET": 58,
|
"RULESET": 60,
|
||||||
"SESSION": 9,
|
"SESSION": 9,
|
||||||
"WAVE": 5,
|
"WAVE": 5,
|
||||||
"WORKSTREAM": 5,
|
"WORKSTREAM": 5,
|
||||||
@@ -979,6 +979,16 @@
|
|||||||
"klass": "RULESET",
|
"klass": "RULESET",
|
||||||
"value": "requires confirmed-bug before implementation (legacy 'confirmed' label is back-compat only for duplicate-sweep exemption, not a valid implementation gate)"
|
"value": "requires confirmed-bug before implementation (legacy 'confirmed' label is back-compat only for duplicate-sweep exemption, not a valid implementation gate)"
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
"id": "RULESET.CONTRIB.GAP-TRACKING",
|
||||||
|
"klass": "RULESET",
|
||||||
|
"value": "a known gap is tracked only by an issue — a code comment, TODO, or changeset note is attached to code and dies with the refactor that deletes it, while the gap stays real (worked example: archived changeset clever-yaks-cheer.md recorded the shortFormToId backfill as 'tracked as a follow-up parity gap' + a // KNOWN GAP: comment marked the call site; the SDK retirement deleted both, no issue was opened, and it surfaced a year later as #3427). If you write 'tracked as a follow-up' anywhere, open the issue first and cite its number. Prose twin of #3473 criterion B6 (a guard needs a retirement condition)"
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"id": "RULESET.CONTRIB.GATE.DELETED-MODULE-DISPOSITION",
|
||||||
|
"klass": "RULESET",
|
||||||
|
"value": "a PR deleting a module/package/lineage with a surviving counterpart carries a symbol-level disposition table for the deleted side — every declared name (exported or local) marked migrated (location named) | renamed → X (conventions normalized: verifyX → cmdVerifyX) | dropped because Y; SCREAMING_CASE constants ranked first (policy lives there; both confirmed ADR-0174 losses were bounds); the surviving tests passing is NOT evidence — the surviving tests belong to the surviving side (#3484, ADR-0174 behavior-carry-forward amendment; losses: #3427 shortFormToId tier + unresolved-deps warning, #3477 regexForKeyLinkPattern 512-char cap + nested-quantifier screen, epic #3473 B5 MAX_JSON_SEARCH_DEPTH=48). Method with mandatory positive control: docs/how-to/audit-a-retiring-lineage.md"
|
||||||
|
},
|
||||||
{
|
{
|
||||||
"id": "RULESET.CONTRIB.GATE.ORDER",
|
"id": "RULESET.CONTRIB.GATE.ORDER",
|
||||||
"klass": "RULESET",
|
"klass": "RULESET",
|
||||||
|
|||||||
@@ -24,6 +24,7 @@ Language versions: [English](README.md) · [Português (pt-BR)](pt-BR/README.md)
|
|||||||
- [Resolve edge-coverage findings](how-to/resolve-edge-coverage-findings.md) — turn the spec phase's surfaced domain-boundary edges into covered, dismissed, or backstopped spec decisions
|
- [Resolve edge-coverage findings](how-to/resolve-edge-coverage-findings.md) — turn the spec phase's surfaced domain-boundary edges into covered, dismissed, or backstopped spec decisions
|
||||||
- [Resolve prohibition findings](how-to/resolve-prohibition-findings.md) — turn the spec phase's surfaced must-NOT constraints into resolved, dismissed, or deferred spec decisions
|
- [Resolve prohibition findings](how-to/resolve-prohibition-findings.md) — turn the spec phase's surfaced must-NOT constraints into resolved, dismissed, or deferred spec decisions
|
||||||
- [Resolve an ESLint glob-coverage finding](how-to/resolve-eslint-coverage-findings.md) — bring a source file that matches no lint rule under coverage, or record a reasoned exemption
|
- [Resolve an ESLint glob-coverage finding](how-to/resolve-eslint-coverage-findings.md) — bring a source file that matches no lint rule under coverage, or record a reasoned exemption
|
||||||
|
- [Audit a retiring lineage](how-to/audit-a-retiring-lineage.md) — produce the symbol-level disposition table a module-deleting consolidation PR must carry, with a mandatory positive control
|
||||||
- [Plan a phase](how-to/plan-a-phase.md) — run research, decompose work, and verify plan quality
|
- [Plan a phase](how-to/plan-a-phase.md) — run research, decompose work, and verify plan quality
|
||||||
- [Execute a phase](how-to/execute-a-phase.md) — run plans in parallel waves with fresh-context subagents
|
- [Execute a phase](how-to/execute-a-phase.md) — run plans in parallel waves with fresh-context subagents
|
||||||
- [Verify and ship](how-to/verify-and-ship.md) — walk through completed work, diagnose failures, and create the PR
|
- [Verify and ship](how-to/verify-and-ship.md) — walk through completed work, diagnose failures, and create the PR
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
# ADR-0174: Retire @opengsd/gsd-sdk package boundary — single-runtime collapse
|
# ADR-0174: Retire @opengsd/gsd-sdk package boundary — single-runtime collapse
|
||||||
|
|
||||||
- **Status:** Accepted (2026-05-23); amended #1642 (2026-06-23) — §5 reconciled to as-built Result type + `exitReason?` field added on `InvalidArgs`
|
- **Status:** Accepted (2026-05-23); amended #1642 (2026-06-23) — §5 reconciled to as-built Result type + `exitReason?` field added on `InvalidArgs`; amended #3484 (2026-08-14) — behavior-carry-forward amendment appended
|
||||||
- **Date:** 2026-05-23
|
- **Date:** 2026-05-23
|
||||||
- **Tracking issue:** [#174](https://github.com/open-gsd/get-shit-done-redux/issues/174) — sub-issues #175–#197
|
- **Tracking issue:** [#174](https://github.com/open-gsd/get-shit-done-redux/issues/174) — sub-issues #175–#197
|
||||||
|
|
||||||
@@ -161,3 +161,46 @@ Seven phases, ~15–18 PRs total. Each phase is a coherent slice that leaves the
|
|||||||
| 7 — Land this ADR's PR | The PR for this ADR closes the umbrella tracking issue. | 1 (this PR) |
|
| 7 — Land this ADR's PR | The PR for this ADR closes the umbrella tracking issue. | 1 (this PR) |
|
||||||
|
|
||||||
Implementation is tracked in [#174 — sub-issues #175–#197](https://github.com/open-gsd/get-shit-done-redux/issues/174).
|
Implementation is tracked in [#174 — sub-issues #175–#197](https://github.com/open-gsd/get-shit-done-redux/issues/174).
|
||||||
|
|
||||||
|
## Amendment (2026-08-14): Behavior carry-forward for lineage-retiring consolidations (#3484)
|
||||||
|
|
||||||
|
This amendment was appended after the migration completed. It changes nothing about the
|
||||||
|
collapse decision above — one runtime still cannot drift from itself, and ADR-3524's
|
||||||
|
parity apparatus is still correctly gone. What this ADR lacked was a clause about the
|
||||||
|
*instant of collapse*: the two lineages were not equivalent at the moment `sdk/` was
|
||||||
|
deleted (drift had been accumulating for months — ADR-3524 itself names nine prior
|
||||||
|
one-sided fixes), and the collapse took the CJS lineage as canonical. Everything the SDK
|
||||||
|
side had and the CJS side did not was deleted with the package — and the parity tests
|
||||||
|
that would have caught exactly that were deleted by Migration Plan Phase 3 in the same
|
||||||
|
motion. The surviving tests were the CJS side's tests, and they passed throughout.
|
||||||
|
|
||||||
|
**The requirement.** A consolidation that retires a lineage must enumerate the retiring
|
||||||
|
side's behavior and record, per item, whether it was **`migrated`**, **`renamed → X`**,
|
||||||
|
or **`dropped because Y`** — before the deletion merges. "The tests pass" is explicitly
|
||||||
|
not sufficient evidence, because the surviving tests belong to the surviving side and
|
||||||
|
cannot observe behavior only the deleted side had.
|
||||||
|
|
||||||
|
**The evidence that produced this amendment — three confirmed losses through the gap:**
|
||||||
|
|
||||||
|
| Lost in the collapse | Surviving site | Consequence |
|
||||||
|
|---|---|---|
|
||||||
|
| `shortFormToId` resolution tier + the `unresolved depends_on reference` warning | `src/phase.cts` | [#3427](https://github.com/open-gsd/gsd-core/issues/3427) — short-form `depends_on` edges silently dropped; dependency-ordered plans execute in parallel |
|
||||||
|
| `regexForKeyLinkPattern`'s 512-char cap + nested-quantifier screen | `src/verify.cts` | [#3477](https://github.com/open-gsd/gsd-core/issues/3477) — ReDoS; `verify-phase` hangs on an untrusted plan pattern |
|
||||||
|
| `MAX_JSON_SEARCH_DEPTH = 48` | `src/intel.cts` | unbounded recursion in `matchesInValue`; stack overflow on deeply nested intel JSON (carried by epic #3473, B5) |
|
||||||
|
|
||||||
|
None was noticed for months; #3427 surfaced only when a user hit it in production. The
|
||||||
|
`shortFormToId` loss is the sharpest: the gap was *known* — recorded as "tracked as a
|
||||||
|
follow-up parity gap" in an archived changeset (`.changeset/archived/clever-yaks-cheer.md`)
|
||||||
|
and marked with a `// KNOWN GAP:` comment — then the SDK was retired, the comment died
|
||||||
|
with the lineage it annotated, and no issue was ever opened. A comment is attached to
|
||||||
|
code; it is not a tracking mechanism. A gap that matters gets an issue.
|
||||||
|
|
||||||
|
A fourth-loss audit run on 2026-08-14 (method recorded in [#3484](https://github.com/open-gsd/gsd-core/issues/3484)
|
||||||
|
and in the contributor how-to) found no further losses — which calibrates the audit
|
||||||
|
method, not a claim that the class is closed.
|
||||||
|
|
||||||
|
**Where the rule lives now.** The general form of this requirement — not scoped to this
|
||||||
|
ADR — is a merge gate in `CONTRIBUTING.md` ("Deleting a module that has a surviving
|
||||||
|
counterpart"): the deleting PR carries a symbol-level disposition table, SCREAMING_CASE
|
||||||
|
constants ranked first, with a mandatory positive control. Future lineage-retiring
|
||||||
|
consolidations cite this amendment and satisfy that gate.
|
||||||
|
|||||||
96
docs/how-to/audit-a-retiring-lineage.md
Normal file
96
docs/how-to/audit-a-retiring-lineage.md
Normal file
@@ -0,0 +1,96 @@
|
|||||||
|
# Audit a retiring lineage (behavior carry-forward)
|
||||||
|
|
||||||
|
**You need this if** your PR deletes a module, package, or runtime lineage — and any part
|
||||||
|
of the deleted side has a surviving counterpart in the tree. The merge gate
|
||||||
|
([CONTRIBUTING.md → "Deleting a module that has a surviving counterpart"](../../CONTRIBUTING.md))
|
||||||
|
requires the PR to carry a symbol-level disposition table. This page is the calibrated
|
||||||
|
method for producing that table. It was first run against the ADR-0174 SDK retirement
|
||||||
|
and found three confirmed losses after the fact (#3484); run it *before* the deletion
|
||||||
|
merges and those losses become review findings instead of production bugs.
|
||||||
|
|
||||||
|
## Why "the tests pass" proves nothing here
|
||||||
|
|
||||||
|
The surviving tests belong to the surviving side. They were written against the
|
||||||
|
surviving implementation and pass before, during, and after the deletion — including
|
||||||
|
when the deleted side carried an invariant the survivor never had. ADR-3524 existed
|
||||||
|
precisely because the two sides had already drifted in nine known places; deleting one
|
||||||
|
side without enumerating it is choosing the survivor's omissions by default.
|
||||||
|
|
||||||
|
## The audit, step by step
|
||||||
|
|
||||||
|
All commands run against `<parent>` — the last commit **before** the retirement (for the
|
||||||
|
SDK retirement this was `04b3be683`, the parent of the retiring commit).
|
||||||
|
|
||||||
|
### 1. List the deleted source files
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git ls-tree -r --name-only <parent> -- sdk/src
|
||||||
|
```
|
||||||
|
|
||||||
|
172 files for the SDK run. Everything under the deleted root is a candidate; test files
|
||||||
|
are excluded from pairing (they are the deleted side's claims, not its behavior).
|
||||||
|
|
||||||
|
### 2. Pair each file with a surviving counterpart
|
||||||
|
|
||||||
|
Pair by **basename**: `sdk/src/query/phase.ts` ↔ `src/phase.cts`. For the SDK run this
|
||||||
|
yielded 22 paired modules. Files with **no** counterpart are whole surfaces deleted on
|
||||||
|
purpose — list them in the disposition as one `dropped because …` line each (or grouped
|
||||||
|
by surface), but they are out of scope for symbol diffing.
|
||||||
|
|
||||||
|
### 3. Diff *declared* names, not raw tokens
|
||||||
|
|
||||||
|
Strip comments from both sides, then diff the declared names (functions, constants,
|
||||||
|
types — exported or local). **Do not tokenize the raw text**: raw token diffing matches
|
||||||
|
prose in comments and produced ~500 false positives on the first SDK run.
|
||||||
|
|
||||||
|
### 4. Normalize rename conventions before reporting
|
||||||
|
|
||||||
|
The two lineages used different naming conventions. Normalize before diffing:
|
||||||
|
|
||||||
|
| Deleted-side name | Surviving-side convention |
|
||||||
|
|---|---|
|
||||||
|
| `verifyX` | `cmdVerifyX` |
|
||||||
|
| `preserveExistingProgress` | `shouldPreserveExistingProgress` |
|
||||||
|
|
||||||
|
Skipping this step is what makes the raw diff unreadable — every convention-renamed
|
||||||
|
symbol reads as a loss.
|
||||||
|
|
||||||
|
### 5. Rank what remains by shape
|
||||||
|
|
||||||
|
**SCREAMING_CASE constants are the high-signal class.** That is where policy lives
|
||||||
|
(bounds, limits, tiers, thresholds), and both real losses the SDK audit confirmed were
|
||||||
|
bounds (`regexForKeyLinkPattern`'s 512-char cap; `MAX_JSON_SEARCH_DEPTH = 48`). Local
|
||||||
|
variable renames are noise — summarize them.
|
||||||
|
|
||||||
|
### 6. Positive control — mandatory
|
||||||
|
|
||||||
|
Before trusting the audit, verify it **re-detects a known loss**. For the SDK lineage the
|
||||||
|
control is `shortFormToId` — a *local* `const` inside a surviving function at
|
||||||
|
`sdk/src/query/phase.ts:609`, not an export. An export-only scan misses it entirely and
|
||||||
|
reports a clean bill of health; a calibrated run finds both halves of #3427 (the tier
|
||||||
|
`shortFormToId` and the diagnostic `unresolvedDeps`). If your audit method cannot find
|
||||||
|
the control, it is not calibrated — fix the method, do not ship the table.
|
||||||
|
|
||||||
|
### 7. Write the disposition table into the PR
|
||||||
|
|
||||||
|
One row per name that survived steps 3–5: `migrated` (name the location), `renamed → X`,
|
||||||
|
or `dropped because Y`. Unpaired whole surfaces get their own grouped `dropped because`
|
||||||
|
rows. The table goes in the PR body where the reviewer of the deletion will see it.
|
||||||
|
|
||||||
|
## Reading the result honestly
|
||||||
|
|
||||||
|
- **No losses found** means the audit found none — with the method calibrated by the
|
||||||
|
positive control. It is evidence, not proof; say which control you ran.
|
||||||
|
- **A "migrated" claim you cannot point at** is a loss wearing a disposition. Every
|
||||||
|
`migrated` row names a file or symbol in the surviving tree.
|
||||||
|
- The audit is run **once per retirement**, against that retirement's parent commit —
|
||||||
|
it is not a standing CI job. Its value is the moment before the deletion merges.
|
||||||
|
|
||||||
|
## Reason codes at a glance
|
||||||
|
|
||||||
|
| Code | Meaning |
|
||||||
|
|---|---|
|
||||||
|
| `migrated` | Behavior exists at a named location in the surviving tree |
|
||||||
|
| `renamed → X` | Same behavior under the surviving convention's name |
|
||||||
|
| `dropped because Y` | Deliberate deletion, reason stated |
|
||||||
|
| *(nothing — no issue)* | Not tracked. A gap that matters gets an issue; a comment or changeset note dies with the code it annotates (`clever-yaks-cheer.md` → #3427) |
|
||||||
File diff suppressed because one or more lines are too long
@@ -378,6 +378,11 @@ function isInsideRoot(candidatePath: string, rootDir: string): boolean {
|
|||||||
return target === root || target.startsWith(`${root}${path.sep}`);
|
return target === root || target.startsWith(`${root}${path.sep}`);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// #3484: restored from the retired SDK lineage (dropped as a bare 256 * 1024 literal
|
||||||
|
// in the ADR-0174 collapse). Counts String#length (UTF-16 code units), not bytes —
|
||||||
|
// semantics preserved from the deleted source.
|
||||||
|
const MAX_MODIFIED_FILE_BYTES = 256 * 1024;
|
||||||
|
|
||||||
function readModifiedFilesContent(projectDir: string, summaries: string[]): string {
|
function readModifiedFilesContent(projectDir: string, summaries: string[]): string {
|
||||||
const out: string[] = [];
|
const out: string[] = [];
|
||||||
let total = 0;
|
let total = 0;
|
||||||
@@ -390,7 +395,7 @@ function readModifiedFilesContent(projectDir: string, summaries: string[]): stri
|
|||||||
if (total >= 50) break;
|
if (total >= 50) break;
|
||||||
if (!file || !isInsideRoot(file, projectDir)) continue;
|
if (!file || !isInsideRoot(file, projectDir)) continue;
|
||||||
const raw = readIfExists(resolvePath(file, projectDir));
|
const raw = readIfExists(resolvePath(file, projectDir));
|
||||||
out.push(raw.length > 256 * 1024 ? raw.slice(0, 256 * 1024) : raw);
|
out.push(raw.length > MAX_MODIFIED_FILE_BYTES ? raw.slice(0, MAX_MODIFIED_FILE_BYTES) : raw);
|
||||||
total++;
|
total++;
|
||||||
}
|
}
|
||||||
if (total >= 50) break;
|
if (total >= 50) break;
|
||||||
|
|||||||
@@ -29,7 +29,7 @@ const fs = require('fs');
|
|||||||
const path = require('path');
|
const path = require('path');
|
||||||
|
|
||||||
const { parseDecisions, extractDecisions } = require('../gsd-core/bin/lib/decisions.cjs');
|
const { parseDecisions, extractDecisions } = require('../gsd-core/bin/lib/decisions.cjs');
|
||||||
const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs');
|
const { runGsdTools, createTempProject, createTempDir, cleanup } = require('./helpers.cjs');
|
||||||
|
|
||||||
// ─── Regression #1364: markdown-header fallback ───────────────────────────────
|
// ─── Regression #1364: markdown-header fallback ───────────────────────────────
|
||||||
|
|
||||||
@@ -1300,3 +1300,119 @@ describe('check.decision-coverage-plan — empty contextPath argument fails clos
|
|||||||
`Missing contextPath argument must fail closed. Got: ${JSON.stringify(parsed)}`);
|
`Missing contextPath argument must fail closed. Got: ${JSON.stringify(parsed)}`);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// ─── #3484: modified-file truncation bound (MAX_MODIFIED_FILE_BYTES) ─────────
|
||||||
|
//
|
||||||
|
// readModifiedFilesContent harvests files_modified entries from phase summaries into
|
||||||
|
// the decision-coverage-verify haystack, capping each file's content. The bound
|
||||||
|
// survived the ADR-0174 SDK collapse as a bare `256 * 1024` literal duplicated in one
|
||||||
|
// expression; #3484 restored it as MAX_MODIFIED_FILE_BYTES. These rows pin the bound
|
||||||
|
// behaviorally — honored/not_honored flips at exactly 262,144 chars — so the value
|
||||||
|
// cannot drift silently again. The constant counts String#length (UTF-16 code units),
|
||||||
|
// not bytes; semantics preserved from the retired lineage.
|
||||||
|
//
|
||||||
|
// Fixture facts these rows depend on (verified against the compiled CLI):
|
||||||
|
// - files_modified entries resolve against the PROJECT root (cwd), not phaseDir;
|
||||||
|
// - decisionMentioned matches \bD-99\b, so the id needs a non-word char before it.
|
||||||
|
|
||||||
|
describe('check.decision-coverage-verify — modified-file truncation bound (#3484)', () => {
|
||||||
|
const CAP = 256 * 1024;
|
||||||
|
let tmpDir;
|
||||||
|
let phaseDir;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
tmpDir = createTempProject('gsd-3484-');
|
||||||
|
phaseDir = path.join(tmpDir, '.planning', 'phases', '01-init');
|
||||||
|
fs.mkdirSync(phaseDir, { recursive: true });
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => cleanup(tmpDir));
|
||||||
|
|
||||||
|
function writeVerifyFixture(modifiedFileContent) {
|
||||||
|
writeGateFiles(['# Summary', '', 'files_modified:', '- big-modified.txt', '', 'Wrapped up the widget work.']);
|
||||||
|
fs.writeFileSync(path.join(tmpDir, 'big-modified.txt'), modifiedFileContent);
|
||||||
|
return path.join(phaseDir, 'CONTEXT.md');
|
||||||
|
}
|
||||||
|
|
||||||
|
function writeGateFiles(summaryLines) {
|
||||||
|
writeContextFile(phaseDir, [
|
||||||
|
'# Phase Context',
|
||||||
|
'',
|
||||||
|
'<decisions>',
|
||||||
|
'- **D-99:** keep the marker token unique to this fixture',
|
||||||
|
'</decisions>',
|
||||||
|
].join('\n'));
|
||||||
|
writePlanFile(phaseDir, '01', '# Plan\n\n## Must Haves\n\n- deliver the widget\n');
|
||||||
|
fs.writeFileSync(path.join(phaseDir, '01-SUMMARY.md'), summaryLines.join('\n') + '\n');
|
||||||
|
}
|
||||||
|
|
||||||
|
function runVerify(contextPath) {
|
||||||
|
const result = runGsdTools(
|
||||||
|
['query', 'check.decision-coverage-verify', phaseDir, contextPath],
|
||||||
|
tmpDir
|
||||||
|
);
|
||||||
|
return JSON.parse(result.output || '{}');
|
||||||
|
}
|
||||||
|
|
||||||
|
test('id ending at the last included char is kept (limit)', () => {
|
||||||
|
// Length exactly CAP: `raw.length > CAP` is false → content passes through whole,
|
||||||
|
// so a D-99 whose last char sits at index CAP-1 is honored. Pins > vs >=.
|
||||||
|
const contextPath = writeVerifyFixture('x'.repeat(CAP - 5) + ' D-99');
|
||||||
|
const parsed = runVerify(contextPath);
|
||||||
|
assert.strictEqual(parsed.honored, 1,
|
||||||
|
`D-99 ending at index CAP-1 must be honored. Got: ${JSON.stringify(parsed)}`);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('id beyond the cap is truncated away (limit+1)', () => {
|
||||||
|
// Length CAP+4 with D-99 starting AT index CAP: slice(0, CAP) drops it entirely.
|
||||||
|
const contextPath = writeVerifyFixture('x'.repeat(CAP - 1) + ' D-99');
|
||||||
|
const parsed = runVerify(contextPath);
|
||||||
|
assert.strictEqual(parsed.honored, 0,
|
||||||
|
`D-99 starting at index CAP must be truncated away. Got: ${JSON.stringify(parsed)}`);
|
||||||
|
assert.ok(
|
||||||
|
(parsed.not_honored || []).some((item) => item.id === 'D-99'),
|
||||||
|
`D-99 must land in not_honored. Got: ${JSON.stringify(parsed)}`
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('id within a small modified file is honored (happy)', () => {
|
||||||
|
const contextPath = writeVerifyFixture('D-99 ' + 'y'.repeat(64));
|
||||||
|
const parsed = runVerify(contextPath);
|
||||||
|
assert.strictEqual(parsed.honored, 1,
|
||||||
|
`D-99 in a small modified file must be honored. Got: ${JSON.stringify(parsed)}`);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('files_modified entry outside the root is skipped', (t) => {
|
||||||
|
// A sibling dir (outside the project root) holding a file that WOULD satisfy the
|
||||||
|
// decision — proving isInsideRoot skipped it, as distinct from a missing file.
|
||||||
|
const sibling = createTempDir('gsd-3484-escape-');
|
||||||
|
t.after(() => cleanup(sibling));
|
||||||
|
fs.writeFileSync(path.join(sibling, 'escape.txt'), 'D-99 '.repeat(10));
|
||||||
|
|
||||||
|
writeGateFiles(['# Summary', '', 'files_modified:', `- ../${path.basename(sibling)}/escape.txt`, '']);
|
||||||
|
|
||||||
|
const parsed = runVerify(path.join(phaseDir, 'CONTEXT.md'));
|
||||||
|
assert.strictEqual(parsed.honored, 0,
|
||||||
|
`An out-of-root files_modified entry must be skipped, not harvested. Got: ${JSON.stringify(parsed)}`);
|
||||||
|
assert.ok(
|
||||||
|
(parsed.not_honored || []).some((item) => item.id === 'D-99'),
|
||||||
|
'D-99 must be reported not honored — the only D-99 lives outside the root'
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('unreadable files_modified entry does not crash the gate', () => {
|
||||||
|
writeGateFiles(['# Summary', '', 'files_modified:', '- does-not-exist.txt', '']);
|
||||||
|
|
||||||
|
const parsed = runVerify(path.join(phaseDir, 'CONTEXT.md'));
|
||||||
|
assert.strictEqual(parsed.honored, 0,
|
||||||
|
`A missing file must harvest empty content, not crash. Got: ${JSON.stringify(parsed)}`);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('summary without files_modified yields no haystack content', () => {
|
||||||
|
writeGateFiles(['# Summary', '', 'No file list here.']);
|
||||||
|
|
||||||
|
const parsed = runVerify(path.join(phaseDir, 'CONTEXT.md'));
|
||||||
|
assert.strictEqual(parsed.honored, 0,
|
||||||
|
`No files_modified block means D-99 cannot be honored. Got: ${JSON.stringify(parsed)}`);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user