* fix(#1776): scope prune phase resolution to ## Current Position cmdStatePrune resolved the current phase by extracting the `Phase` field over the WHOLE STATE.md body. stateExtractField's fallback chain ends in a pipe-table match (`| Phase | N |`), so a STATE.md lacking a `Current Phase` field and a prose `Phase:` line — but carrying an unrelated `Phase`-labelled table row (e.g. a historical verification table) — resolved that stale table cell as the current phase and computed a wrong cutoff (bailing "Only N phases" or pruning at a stale boundary). Resolve the phase via the same canonical chain buildStateFrontmatter uses — frontmatter `current_phase` → `Current Phase` field → prose `Phase: X of Y` — but scope ONLY the prose term to the `## Current Position` section via the fence-aware locateCurrentPosition seam (new exported sliceCurrentPositionSection). Frontmatter and the explicit `Current Phase` field stay document-wide (they are unambiguous); the shared stateExtractField is not narrowed for any other caller. Tests (folded into tests/state-prune.test.cjs): a stray `| Phase | 2 |` table with the real phase in frontmatter no longer drives the cutoff (fail-first on base); template-conformant STATE.md is unchanged; and a fast-check boundary-containment property that a `| Phase | N |` row outside Current Position never leaks into the scoped resolution. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * chore(#1776): add changeset for prune Current Position scoping Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Tom Boucher <trekkie@nomorestars.com>
This commit is contained in:
committed by
GitHub
parent
dae7f81482
commit
50ff7a8707
5
.changeset/gallant-wasps-wave.md
Normal file
5
.changeset/gallant-wasps-wave.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 1832
|
||||
---
|
||||
state prune now resolves the current phase from the canonical location — frontmatter current_phase, the Current Phase field, or the prose Phase: line scoped to the ## Current Position section — instead of extracting Phase over the whole document, where stateExtractField's pipe-table fallback could latch onto an unrelated | Phase | N | row (e.g. a historical verification table) and compute a wrong prune cutoff.
|
||||
@@ -19,6 +19,7 @@ exports.STATE_MD_SECTIONS = exports.FIELD_CLASSIFICATION = void 0;
|
||||
exports.getFieldClassification = getFieldClassification;
|
||||
exports.applyStatePreservation = applyStatePreservation;
|
||||
exports.transitionCore = transitionCore;
|
||||
exports.sliceCurrentPositionSection = sliceCurrentPositionSection;
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
const frontmatter = require("./frontmatter.cjs");
|
||||
const state_document_cjs_1 = require("./state-document.cjs");
|
||||
@@ -319,6 +320,20 @@ function locateCurrentPosition(body) {
|
||||
}
|
||||
return { start, end };
|
||||
}
|
||||
/**
|
||||
* Return the body text of the `## Current Position` section, or `null` when it
|
||||
* is absent. Reuses the fence-aware `locateCurrentPosition` locator (ADR-1372).
|
||||
*
|
||||
* Exposed so callers that must read a position field (e.g. `cmdStatePrune`,
|
||||
* #1776) can scope extraction to the canonical section instead of the whole
|
||||
* document — where `stateExtractField`'s pipe-table fallback could otherwise
|
||||
* latch onto an unrelated `| Phase | N |` row elsewhere in STATE.md. This
|
||||
* scopes the *caller*; the shared extractor is left broad for every other use.
|
||||
*/
|
||||
function sliceCurrentPositionSection(body) {
|
||||
const span = locateCurrentPosition(body);
|
||||
return span === null ? null : body.slice(span.start, span.end);
|
||||
}
|
||||
/**
|
||||
* First-time ## Current Position mutation: update Phase / Plan / Status /
|
||||
* Last activity lines. Mirrors state.cts:2261-2324 byte-for-behaviour
|
||||
|
||||
@@ -528,6 +528,21 @@ function locateCurrentPosition(body: string): { start: number; end: number } | n
|
||||
return { start, end };
|
||||
}
|
||||
|
||||
/**
|
||||
* Return the body text of the `## Current Position` section, or `null` when it
|
||||
* is absent. Reuses the fence-aware `locateCurrentPosition` locator (ADR-1372).
|
||||
*
|
||||
* Exposed so callers that must read a position field (e.g. `cmdStatePrune`,
|
||||
* #1776) can scope extraction to the canonical section instead of the whole
|
||||
* document — where `stateExtractField`'s pipe-table fallback could otherwise
|
||||
* latch onto an unrelated `| Phase | N |` row elsewhere in STATE.md. This
|
||||
* scopes the *caller*; the shared extractor is left broad for every other use.
|
||||
*/
|
||||
export function sliceCurrentPositionSection(body: string): string | null {
|
||||
const span = locateCurrentPosition(body);
|
||||
return span === null ? null : body.slice(span.start, span.end);
|
||||
}
|
||||
|
||||
/**
|
||||
* First-time ## Current Position mutation: update Phase / Plan / Status /
|
||||
* Last activity lines. Mirrors state.cts:2261-2324 byte-for-behaviour
|
||||
|
||||
@@ -32,7 +32,7 @@ const { extractFrontmatter, reconstructFrontmatter } = frontmatter;
|
||||
import scanPhasePlans = require('./plan-scan.cjs');
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import stateTransitionMod = require('./state-transition.cjs');
|
||||
const { transitionCore, applyStatePreservation } = stateTransitionMod;
|
||||
const { transitionCore, applyStatePreservation, sliceCurrentPositionSection } = stateTransitionMod;
|
||||
type StateTransitionIntent = stateTransitionMod.StateTransitionIntent;
|
||||
type StateTransitionDeps = stateTransitionMod.StateTransitionDeps;
|
||||
type PhaseInventoryRecord = stateTransitionMod.PhaseInventoryRecord;
|
||||
@@ -2501,12 +2501,31 @@ function cmdStatePrune(cwd: string, options: StatePruneOptions, raw: boolean): v
|
||||
|
||||
const keepRecent = parseInt(String(options.keepRecent), 10) || 3;
|
||||
const dryRun = !!options.dryRun;
|
||||
// #1760: the canonical STATE.md template emits `Phase: [X] of [Y]`, not
|
||||
// `Current Phase:`. Read both (mirroring buildStateFrontmatter /
|
||||
// resolvePhaseIdForCompletePhase) so prune engages on template-conformant
|
||||
// STATE.md instead of bailing with "Only 0 phases — nothing to prune".
|
||||
// Resolve the current phase via the same canonical chain buildStateFrontmatter
|
||||
// uses (frontmatter `current_phase` → `Current Phase` field → prose `Phase: X
|
||||
// of Y`), so prune engages on template-conformant STATE.md instead of bailing
|
||||
// "Only 0 phases" (#1760).
|
||||
// #1776: scope ONLY the prose `Phase:` term to the canonical `## Current
|
||||
// Position` section. Over the whole body, `stateExtractField`'s pipe-table
|
||||
// fallback matches any `| Phase | N |` row (e.g. a historical verification
|
||||
// table), resolving a stale phase and computing a wrong cutoff. Frontmatter and
|
||||
// the explicit `Current Phase` field are unambiguous, so they stay document-wide;
|
||||
// the shared extractor is not narrowed for any other caller.
|
||||
const rawState = fs.readFileSync(statePath, 'utf-8');
|
||||
const currentPhaseRaw = stateExtractField(rawState, 'Current Phase') || stateExtractField(rawState, 'Phase');
|
||||
const fm = extractFrontmatter(rawState) as Record<string, unknown>;
|
||||
const body = stripFrontmatter(rawState);
|
||||
// Mirror buildStateFrontmatter's fmScalar: only string/number/boolean
|
||||
// frontmatter scalars are usable (an object/array `current_phase` is ignored,
|
||||
// which also avoids a base-to-string on a non-primitive).
|
||||
const fmRawPhase = fm.current_phase;
|
||||
const fmCurrentPhase =
|
||||
typeof fmRawPhase === 'string' ? (fmRawPhase.trim() || null)
|
||||
: typeof fmRawPhase === 'number' || typeof fmRawPhase === 'boolean' ? String(fmRawPhase)
|
||||
: null;
|
||||
const positionSection = sliceCurrentPositionSection(body);
|
||||
const prosePhase =
|
||||
positionSection !== null ? parseProsePhaseField(stateExtractField(positionSection, 'Phase')).phase : null;
|
||||
const currentPhaseRaw = fmCurrentPhase ?? stateExtractField(body, 'Current Phase') ?? prosePhase;
|
||||
const currentPhase = parseInt(String(currentPhaseRaw), 10) || 0;
|
||||
const cutoff = currentPhase - keepRecent;
|
||||
|
||||
|
||||
@@ -8,7 +8,10 @@ const { test, describe, beforeEach, afterEach } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
const fc = require('./helpers/fast-check-setup.cjs');
|
||||
const { runGsdTools, createTempProject, cleanup } = require('./helpers.cjs');
|
||||
const { sliceCurrentPositionSection } = require('../gsd-core/bin/lib/state-transition.cjs');
|
||||
const { stateExtractField } = require('../gsd-core/bin/lib/state-document.cjs');
|
||||
|
||||
function writeStateMd(tmpDir, content) {
|
||||
fs.writeFileSync(path.join(tmpDir, '.planning', 'STATE.md'), content);
|
||||
@@ -275,3 +278,101 @@ describe('state prune (#1970)', () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
// #1776 — `cmdStatePrune` resolved the current phase by extracting the `Phase`
|
||||
// field over the WHOLE STATE.md; `stateExtractField`'s pipe-table fallback then
|
||||
// matched any `| Phase | N |` row anywhere (e.g. a historical verification
|
||||
// table), so a stale table cell drove the cutoff. The fix scopes the prose
|
||||
// `Phase:` lookup to the canonical `## Current Position` section.
|
||||
describe('#1776: prune reads the current phase only from ## Current Position', () => {
|
||||
let tmpDir;
|
||||
beforeEach(() => { tmpDir = createTempProject(); });
|
||||
afterEach(() => { cleanup(tmpDir); });
|
||||
|
||||
// hiSandog's fixture shape: an unrelated `| Phase | 2 |` verification table,
|
||||
// with the real position only in frontmatter (`current_phase: 12`) and NO
|
||||
// `Current Phase` body field / prose `Phase:` line. Pre-fix, prune ignored
|
||||
// frontmatter and `stateExtractField(body, 'Phase')` fell to the table cell →
|
||||
// currentPhase=2 → cutoff -1 → "Only 2 phases" bail. Post-fix it reads the
|
||||
// frontmatter phase and the table cell is never consulted.
|
||||
test('a stray | Phase | N | table does not override the canonical (frontmatter) phase', () => {
|
||||
writeStateMd(tmpDir, [
|
||||
'---',
|
||||
'gsd_state_version: 1.0',
|
||||
'current_phase: 12',
|
||||
'status: executing',
|
||||
'---',
|
||||
'',
|
||||
'# GSD State',
|
||||
'',
|
||||
'## Verification History',
|
||||
'',
|
||||
'| Field | Value |',
|
||||
'| --- | --- |',
|
||||
'| Phase | 2 |',
|
||||
'| Result | passed |',
|
||||
'',
|
||||
'## Decisions',
|
||||
'',
|
||||
'- [Phase 1]: Old decision',
|
||||
'- [Phase 4]: Mid decision',
|
||||
'- [Phase 11]: Recent decision',
|
||||
'',
|
||||
].join('\n'));
|
||||
|
||||
const result = runGsdTools('state prune --keep-recent 3', tmpDir);
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
const out = JSON.parse(result.output);
|
||||
|
||||
// Phase 12, keep-recent 3 → cutoff 9. If the table cell (2) had leaked, the
|
||||
// cutoff would be -1 and prune would bail "Only 2 phases — nothing to prune".
|
||||
assert.strictEqual(out.pruned, true, `expected prune to engage, got: ${JSON.stringify(out)}`);
|
||||
assert.strictEqual(out.cutoff_phase, 9);
|
||||
});
|
||||
|
||||
// AC2 — template-conformant STATE.md (prose Phase under Current Position, no
|
||||
// stray table) is unchanged: prune still engages off the real phase.
|
||||
test('template-conformant Current Position (no stray table) still prunes', () => {
|
||||
writeStateMd(tmpDir, [
|
||||
'# GSD State',
|
||||
'',
|
||||
'## Current Position',
|
||||
'',
|
||||
'Phase: 10 of 15',
|
||||
'',
|
||||
'## Decisions',
|
||||
'',
|
||||
'- [Phase 1]: Old',
|
||||
'- [Phase 9]: Recent',
|
||||
'',
|
||||
].join('\n'));
|
||||
|
||||
const result = runGsdTools('state prune --keep-recent 3', tmpDir);
|
||||
assert.ok(result.success, `Command failed: ${result.error}`);
|
||||
const out = JSON.parse(result.output);
|
||||
assert.strictEqual(out.cutoff_phase, 7);
|
||||
});
|
||||
|
||||
// Boundary-containment property — the core scoping invariant: a `| Phase | N |`
|
||||
// row OUTSIDE the `## Current Position` section is never visible to extraction
|
||||
// scoped to that section. The section here carries NO `Phase:` line, so a
|
||||
// correct slice yields `null`; a whole-document leak would instead surface the
|
||||
// stray table cell (`stateExtractField`'s pipe-table fallback). The table is
|
||||
// placed both before and after the section to exercise both span boundaries.
|
||||
test('property: a | Phase | N | table outside Current Position is excluded from the scoped slice', () => {
|
||||
fc.assert(
|
||||
fc.property(
|
||||
fc.integer({ min: 0, max: 998 }), // stray table phase
|
||||
fc.boolean(), // stray table before (true) or after (false) the section
|
||||
(stray, before) => {
|
||||
const table = ['## History', '', '| Field | Value |', '| --- | --- |', `| Phase | ${stray} |`, ''];
|
||||
const position = ['## Current Position', '', '**Status:** Executing', '']; // deliberately no `Phase:` line
|
||||
const body = ['# GSD State', '', ...(before ? [...table, ...position] : [...position, ...table])].join('\n');
|
||||
const section = sliceCurrentPositionSection(body);
|
||||
// Slice must exist and must NOT see the out-of-section table cell.
|
||||
return section !== null && stateExtractField(section, 'Phase') === null;
|
||||
}
|
||||
)
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user