Merge remote-tracking branch 'origin/next' into test/3057-wave2-liveness
This commit is contained in:
5
.changeset/kind-sloths-climb.md
Normal file
5
.changeset/kind-sloths-climb.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3093
|
||||
---
|
||||
**`docs/json-errors.md` now documents the ExitError plain-text carve-out** — the page previously claimed every CLI error emits a structured JSON envelope on stderr, but usage errors (ExitError) intentionally emit plain text with their own exit code. The structured-envelope guidance is now scoped to non-usage failures, with the carve-out stated explicitly and a characterization test pinning both paths. (#2979)
|
||||
5
.changeset/lucky-tunas-leap.md
Normal file
5
.changeset/lucky-tunas-leap.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3091
|
||||
---
|
||||
**`progress.completed_plans` no longer stays pinned after a gap-closure cycle** — when plan-phase re-planned a phase and added gap-closure plans, `total_plans` corrected upward but `completed_plans` was restored to its pre-growth value, so STATE.md showed `completed_plans < total_plans` permanently even after every plan (including the gap-closure ones) was summarized. `completed_plans` and `completed_phases` now ratchet up to the disk-derived count under the plan-phase progress opt-in (never deriving downward, preserving the curated-progress ratchet for unrelated edits). (#2969)
|
||||
5
.changeset/proud-eagles-chatter.md
Normal file
5
.changeset/proud-eagles-chatter.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3097
|
||||
---
|
||||
**`/gsd` commands in Pi now display their output** — the command handler returned output as a bare string, which Pi's ExtensionAPI silently dropped. It now returns Pi's structured `{ content: [{ type: 'text', text }] }` display shape (matching the `gsd_invoke` tool's proven contract), so success output and error messages are visible. (#2991)
|
||||
5
.changeset/proud-pumas-bark.md
Normal file
5
.changeset/proud-pumas-bark.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3096
|
||||
---
|
||||
**`/gsd-code-review` no longer picks a wrong diff base from unanchored commit-message grep** — the diff-base fallback searched all commit messages for the bare phase number as a substring, matching version strings, dates, and issue refs, then took the oldest match. The grep is now anchored to the phase-mention convention (`Phase N` with a word boundary), so the fail-closed branch is reachable when no commit genuinely references the phase. (#2989)
|
||||
5
.changeset/silly-moles-rally.md
Normal file
5
.changeset/silly-moles-rally.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3092
|
||||
---
|
||||
**`roadmap validate` now performs real structural validation** — it previously returned `{"warnings":[]}` (exit 0) for every input including empty files, garbage text, and missing files, providing false assurance. It now checks file existence/readability, emptiness, frontmatter well-formedness, and the presence of at least one phase entry, exiting non-zero on any warning (per its documented contract). The existing opt-in milestone-prefix consistency check is preserved. (#2978)
|
||||
5
.changeset/witty-yaks-munch.md
Normal file
5
.changeset/witty-yaks-munch.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3095
|
||||
---
|
||||
**Local `lint:changeset` and `lint:docs-required` now diff against `next` instead of `main`** — the local fallback was the release branch (`main`), which lags far behind the integration branch (`next`), so the lint always passed by finding fragments from other already-merged PRs in the oversized diff range. The local invocation now matches the base CI uses. (#2988)
|
||||
@@ -2,11 +2,12 @@
|
||||
|
||||
## Overview
|
||||
|
||||
`gsd-tools` supports a **JSON error mode** that emits all errors as structured
|
||||
`gsd-tools` supports a **JSON error mode** that emits most errors as structured
|
||||
JSON objects on stderr instead of free-form text. This is the recommended
|
||||
surface for tests and tooling that need to assert on error types without
|
||||
grepping raw text (see `CONTRIBUTING.md` — "Prohibited: Raw Text Matching on
|
||||
Test Outputs").
|
||||
Test Outputs"). Usage errors are an intentional exception — see the
|
||||
`ExitError` carve-out below.
|
||||
|
||||
## Activating
|
||||
|
||||
@@ -37,6 +38,20 @@ Fields:
|
||||
| `reason` | string | Typed reason code from the taxonomy below. |
|
||||
| `message` | string | Human-readable description (may change; do not assert on it). |
|
||||
|
||||
### `ExitError` carve-out (plain text, not JSON)
|
||||
|
||||
Usage errors and explicit exit-code signals take a **different path**: they
|
||||
throw `ExitError` (`src/cli-exit.cts`), which `runMain` catches *before* the
|
||||
JSON-envelope branch. An `ExitError` writes its `message` as **plain text**
|
||||
to stderr (not a JSON object) and exits with the error's own `code` (which
|
||||
may differ from 1). This is intentional — usage messages are operator-facing
|
||||
prose, not structured failures.
|
||||
|
||||
If you are testing a usage/flag error, do **not** parse stderr as JSON;
|
||||
assert on the exit code and (if needed) the plain-text message. The
|
||||
"parse stderr as JSON" guidance below applies only to the structured-envelope
|
||||
branch (non-`ExitError` failures).
|
||||
|
||||
## Error code taxonomy
|
||||
|
||||
Codes are frozen constants in `gsd-core/bin/lib/core.cjs` under
|
||||
@@ -99,8 +114,9 @@ text (unstable).
|
||||
|
||||
## Writing tests
|
||||
|
||||
Always parse stderr with `JSON.parse` and assert on typed fields. Never use
|
||||
`.includes()`, `.match()`, or regex on the raw error string.
|
||||
For **non-usage** errors (the structured-envelope branch), parse stderr with
|
||||
`JSON.parse` and assert on typed fields. Never use `.includes()`, `.match()`,
|
||||
or regex on the raw error string.
|
||||
|
||||
```js
|
||||
// CORRECT: parse then assert on typed field
|
||||
|
||||
@@ -236,8 +236,13 @@ Additionally, whenever a reliable diff base is available, cross-check the SUMMAR
|
||||
against the diff and warn about (then add) any changed files the SUMMARY extractor did not
|
||||
surface — so a partial SUMMARY result can no longer silently mask the rest of the phase.
|
||||
```bash
|
||||
# Compute diff base from phase commits — fail closed if no reliable base found
|
||||
PHASE_COMMITS=$(git log --oneline --all --grep="${PADDED_PHASE}" --format="%H" 2>/dev/null)
|
||||
# Compute diff base from phase commits — fail closed if no reliable base found.
|
||||
# #2989: anchor the grep to the phase-mention convention ("Phase N" / "phase N"
|
||||
# with a word boundary) so a bare digit substring doesn't match version strings,
|
||||
# dates, issue refs, or other phases' numbers. With --extended-regexp, \b is
|
||||
# a word boundary. When no commit genuinely references the phase, this yields
|
||||
# empty and the fail-closed warning below actually fires.
|
||||
PHASE_COMMITS=$(git log --oneline --all --grep="[Pp]hase ${PADDED_PHASE}\b" --extended-regexp --format="%H" 2>/dev/null)
|
||||
DIFF_BASE=""
|
||||
if [ -n "$PHASE_COMMITS" ]; then
|
||||
DIFF_BASE=$(echo "$PHASE_COMMITS" | tail -1)^
|
||||
|
||||
11
pi/gsd.cjs
11
pi/gsd.cjs
@@ -287,11 +287,16 @@ module.exports = function gsdPiExtension(pi) {
|
||||
try {
|
||||
({ dispatchGsdCommand } = require(path.join(GSD_CORE, 'bin', 'lib', 'shell-command-projection.cjs')));
|
||||
} catch (e) {
|
||||
return `GSD engine unavailable: ${e && e.message ? e.message : String(e)}`;
|
||||
// #2991: return the structured { content } shape Pi's ExtensionAPI
|
||||
// displays, not a bare string (which Pi silently drops).
|
||||
return { content: [{ type: 'text', text: `GSD engine unavailable: ${e && e.message ? e.message : String(e)}` }] };
|
||||
}
|
||||
const result = dispatchGsdCommand({ family, subcommand, args: rest, cwd });
|
||||
if (result.ok) return result.stdout;
|
||||
return `GSD error: ${result.stderr || result.stdout || `dispatch failed (exit ${result.code})`}`;
|
||||
// #2991: match gsd_invoke's proven output shape so Pi actually displays it.
|
||||
const text = result.ok
|
||||
? result.stdout
|
||||
: `GSD error: ${result.stderr || result.stdout || `dispatch failed (exit ${result.code})`}`;
|
||||
return { content: [{ type: 'text', text }] };
|
||||
},
|
||||
});
|
||||
|
||||
|
||||
@@ -12,6 +12,10 @@
|
||||
* Tests assert on the typed verdict, never on free text.
|
||||
*/
|
||||
|
||||
// #2988: the repo's integration/default branch — the base every PR targets.
|
||||
// Used as the local fallback when GITHUB_BASE_REF is unset (CI sets it).
|
||||
const DEFAULT_BASE = 'next';
|
||||
|
||||
const LINT_REASON = Object.freeze({
|
||||
OK_FRAGMENT_PRESENT: 'ok_fragment_present',
|
||||
OK_OPT_OUT_LABEL: 'ok_opt_out_label',
|
||||
@@ -80,7 +84,10 @@ function main() {
|
||||
labels = (event.pull_request?.labels || []).map((l) => l.name);
|
||||
} catch { /* fall through */ }
|
||||
}
|
||||
const base = process.env.GITHUB_BASE_REF || 'main';
|
||||
// #2988: local fallback must match the repo's integration branch (`next`),
|
||||
// not the release branch (`main`). CI sets GITHUB_BASE_REF explicitly; the
|
||||
// fallback only fires locally, where `next` is the base every PR targets.
|
||||
const base = process.env.GITHUB_BASE_REF || DEFAULT_BASE;
|
||||
let changedFiles = [];
|
||||
try {
|
||||
// Use execFileSync with an argv array — the base ref is interpolated
|
||||
@@ -145,4 +152,4 @@ function main() {
|
||||
|
||||
if (require.main === module) runMain(main);
|
||||
|
||||
module.exports = { evaluateLint, LINT_REASON, OPT_OUT_LABEL, isUserFacing, isFragment };
|
||||
module.exports = { evaluateLint, LINT_REASON, OPT_OUT_LABEL, isUserFacing, isFragment, DEFAULT_BASE };
|
||||
|
||||
@@ -16,6 +16,10 @@
|
||||
const { parseFragment, FRAGMENT_ERROR } = require('./changeset/parse.cjs');
|
||||
const { ExitError, runMain } = require('./lib/cli-exit.cjs');
|
||||
|
||||
// #2988: the repo's integration/default branch — the base every PR targets.
|
||||
// Used as the local fallback when GITHUB_BASE_REF is unset (CI sets it).
|
||||
const DEFAULT_BASE = 'next';
|
||||
|
||||
const LINT_REASON = Object.freeze({
|
||||
OK_NO_TRIGGERING_FRAGMENTS: 'ok_no_triggering_fragments',
|
||||
OK_DOCS_UPDATED: 'ok_docs_updated',
|
||||
@@ -149,7 +153,10 @@ function main() {
|
||||
} catch { /* fall through */ }
|
||||
}
|
||||
|
||||
const base = process.env.GITHUB_BASE_REF || 'main';
|
||||
// #2988: local fallback must match the repo's integration branch (`next`),
|
||||
// not the release branch (`main`). CI sets GITHUB_BASE_REF explicitly; the
|
||||
// fallback only fires locally, where `next` is the base every PR targets.
|
||||
const base = process.env.GITHUB_BASE_REF || DEFAULT_BASE;
|
||||
let changedFiles = [];
|
||||
try {
|
||||
// execFileSync with argv — no shell, so a malicious GITHUB_BASE_REF
|
||||
@@ -216,6 +223,7 @@ module.exports = {
|
||||
OPT_OUT_LABEL,
|
||||
TRIGGERING_TYPES,
|
||||
FRAGMENT_ERROR,
|
||||
DEFAULT_BASE,
|
||||
isFragmentPath,
|
||||
isDocsFile,
|
||||
isExemptFragment,
|
||||
|
||||
@@ -21,6 +21,9 @@ const { planningDir } = planningWorkspace;
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import configLoaderMod = require('./config-loader.cjs');
|
||||
const { loadConfig } = configLoaderMod;
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import cliExitMod = require('./cli-exit.cjs');
|
||||
const { ExitError } = cliExitMod;
|
||||
|
||||
// ─── Types ────────────────────────────────────────────────────────────────────
|
||||
|
||||
@@ -143,11 +146,43 @@ function routeRoadmapCommand({ roadmap, args, cwd, raw, error }: RouteRoadmapCom
|
||||
'annotate-dependencies': () => roadmap.cmdRoadmapAnnotateDependencies(cwd, args[2], raw),
|
||||
'validate': () => {
|
||||
const roadmapPath = path.join(planningDir(cwd), 'ROADMAP.md');
|
||||
let roadmapContent = '';
|
||||
const warnings: Array<{ code: string; message: string }> = [];
|
||||
|
||||
// #2978: structural validation. A verb named "validate" that cannot
|
||||
// produce a negative result provides false assurance. Before the
|
||||
// opt-in milestone-prefix check, verify the file is structurally a
|
||||
// roadmap at all.
|
||||
let roadmapContent: string;
|
||||
try {
|
||||
roadmapContent = fs.readFileSync(roadmapPath, 'utf8');
|
||||
} catch {
|
||||
// ROADMAP.md missing — return empty warnings
|
||||
// ROADMAP.md missing — not silent success.
|
||||
warnings.push({ code: 'V001', message: 'ROADMAP.md not found or unreadable' });
|
||||
const result = { warnings };
|
||||
process.stdout.write(raw ? JSON.stringify(result) : JSON.stringify(result, null, 2));
|
||||
throw new ExitError(1);
|
||||
}
|
||||
|
||||
// Empty or whitespace-only.
|
||||
if (roadmapContent.trim() === '') {
|
||||
warnings.push({ code: 'V002', message: 'ROADMAP.md is empty' });
|
||||
}
|
||||
|
||||
// Malformed frontmatter — a `---` opener with no matching closer.
|
||||
// Tolerate a leading BOM (#3057) before the fence.
|
||||
const contentAfterBom = roadmapContent.replace(/^\uFEFF/, '');
|
||||
if (contentAfterBom.startsWith('---')) {
|
||||
const closeMatch = contentAfterBom.slice(3).match(/\r?\n---\s*(\r?\n|$)/);
|
||||
if (!closeMatch) {
|
||||
warnings.push({ code: 'V003', message: 'ROADMAP.md frontmatter is malformed (unterminated --- fence)' });
|
||||
}
|
||||
}
|
||||
|
||||
// No recognizable phase structure — at least one `### Phase N:` heading.
|
||||
// Mirrors the phase-heading pattern used across roadmap-parser.cts.
|
||||
const hasPhaseEntry = /^#{2,4}\s*Phase\s+\S/im.test(roadmapContent);
|
||||
if (!hasPhaseEntry && !warnings.some((w) => w.code === 'V002')) {
|
||||
warnings.push({ code: 'V004', message: 'ROADMAP.md contains no recognizable phase entries (no "### Phase N:" headings)' });
|
||||
}
|
||||
|
||||
// W021 only fires when phase_id_convention is explicitly 'milestone-prefixed'.
|
||||
@@ -173,13 +208,17 @@ function routeRoadmapCommand({ roadmap, args, cwd, raw, error }: RouteRoadmapCom
|
||||
}
|
||||
}
|
||||
}
|
||||
const warnings = (convention === 'milestone-prefixed')
|
||||
? checkW021(roadmapContent)
|
||||
: [];
|
||||
if (convention === 'milestone-prefixed') {
|
||||
warnings.push(...checkW021(roadmapContent));
|
||||
}
|
||||
|
||||
const result = { warnings };
|
||||
if (raw) process.stdout.write(JSON.stringify(result));
|
||||
else process.stdout.write(JSON.stringify(result, null, 2));
|
||||
process.stdout.write(raw ? JSON.stringify(result) : JSON.stringify(result, null, 2));
|
||||
// #2978: exit non-zero on any warning, per the documented contract
|
||||
// ("exits non-zero on any error or warning").
|
||||
if (warnings.length > 0) {
|
||||
throw new ExitError(1);
|
||||
}
|
||||
},
|
||||
'upgrade': () => {
|
||||
const dryRun = !args.includes('--apply');
|
||||
|
||||
@@ -195,8 +195,25 @@ export function applyStatePreservation(input: StatePreservationInput): StatePres
|
||||
const derived = (postFm['progress'] ?? {}) as Record<string, unknown>;
|
||||
const merged: Record<string, unknown> = { ...derived };
|
||||
if (curated) {
|
||||
// #2440: total_plans and total_phases always take the derived value.
|
||||
// #2969: completed_plans and completed_phases take the derived value
|
||||
// when it is GREATER than the curated value (gap-closure plans that
|
||||
// completed after the plan count grew) — ratcheting UP only, never
|
||||
// deriving downward (preserves the #3242 curated-progress protection
|
||||
// for cases unrelated to plan-count growth, e.g. a deleted SUMMARY).
|
||||
// percent also takes the derived value — the resync recomputed it from
|
||||
// disk counts, and a stale curated percent would be incoherent against
|
||||
// the ratcheted-up completed counts (e.g. 54/54 at 93%).
|
||||
const ratchetUpKeys = new Set(['completed_plans', 'completed_phases']);
|
||||
for (const [key, value] of Object.entries(curated)) {
|
||||
if (key !== 'total_plans' && key !== 'total_phases') {
|
||||
if (key === 'total_plans' || key === 'total_phases' || key === 'percent') continue;
|
||||
if (ratchetUpKeys.has(key)) {
|
||||
const derivedNum = typeof derived[key] === 'number' ? derived[key] : -Infinity;
|
||||
const curatedNum = typeof value === 'number' ? value : -Infinity;
|
||||
// Take the derived value only when it ratchets up; else keep curated.
|
||||
if (derivedNum > curatedNum) continue;
|
||||
merged[key] = value;
|
||||
} else {
|
||||
merged[key] = value;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -8,7 +8,8 @@ const fs = require('node:fs');
|
||||
const os = require('node:os');
|
||||
const cp = require('node:child_process');
|
||||
|
||||
const { evaluateLint, LINT_REASON } = require(path.join(__dirname, '..', 'scripts', 'changeset', 'lint.cjs'));
|
||||
const { evaluateLint, LINT_REASON, DEFAULT_BASE: CHANGESET_DEFAULT_BASE } = require(path.join(__dirname, '..', 'scripts', 'changeset', 'lint.cjs'));
|
||||
const { DEFAULT_BASE: DOCS_DEFAULT_BASE } = require(path.join(__dirname, '..', 'scripts', 'lint-docs-required.cjs'));
|
||||
|
||||
const ROOT = path.join(__dirname, '..');
|
||||
const LINT_SCRIPT = path.join(ROOT, 'scripts', 'changeset', 'lint.cjs');
|
||||
@@ -297,3 +298,19 @@ describe('changeset lint: main() end-to-end wiring (#1006)', () => {
|
||||
assert.ok(!deletedEntry, `deleted fragment must not appear in failures, got: ${JSON.stringify(failures)}`);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── #2988: local base fallback parity ──────────────────────────────────────
|
||||
|
||||
describe('#2988: changeset + docs lints resolve the same local base fallback', () => {
|
||||
test('both lints default to `next` (the integration branch), not `main`', () => {
|
||||
assert.strictEqual(CHANGESET_DEFAULT_BASE, 'next',
|
||||
`changeset lint DEFAULT_BASE must be 'next', got '${CHANGESET_DEFAULT_BASE}'`);
|
||||
assert.strictEqual(DOCS_DEFAULT_BASE, 'next',
|
||||
`docs lint DEFAULT_BASE must be 'next', got '${DOCS_DEFAULT_BASE}'`);
|
||||
});
|
||||
|
||||
test('both lints resolve the same base given the same environment (parity)', () => {
|
||||
assert.strictEqual(CHANGESET_DEFAULT_BASE, DOCS_DEFAULT_BASE,
|
||||
`the two lints must not diverge on base resolution: changeset='${CHANGESET_DEFAULT_BASE}' docs='${DOCS_DEFAULT_BASE}'`);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -183,11 +183,18 @@ describe('runMain', () => {
|
||||
describe('regressions', () => {
|
||||
/** Spawn a one-shot script that sets json-error mode and calls runMain with a throwing handler. */
|
||||
function spawnJsonErrorRun({ jsonMode, errorType = 'TypeError', message = 'unexpected boom' } = {}) {
|
||||
// ExitError lives in the same module as runMain; import it when the test
|
||||
// wants to exercise the ExitError carve-out path. ExitError takes (code, message).
|
||||
const isExitError = errorType === 'ExitError';
|
||||
const destructure = isExitError ? '{ runMain, ExitError }' : '{ runMain }';
|
||||
const throwExpr = isExitError
|
||||
? `new ExitError(1, ${JSON.stringify(message)})`
|
||||
: `new ${errorType}(${JSON.stringify(message)})`;
|
||||
const script = `
|
||||
const io = require(${JSON.stringify(IO_PATH)});
|
||||
const { runMain } = require(${JSON.stringify(BUILT_CLI_EXIT_PATH)});
|
||||
const ${destructure} = require(${JSON.stringify(BUILT_CLI_EXIT_PATH)});
|
||||
io.setJsonErrorMode(${jsonMode ? 'true' : 'false'});
|
||||
runMain(() => { throw new ${errorType}(${JSON.stringify(message)}); });
|
||||
runMain(() => { throw ${throwExpr}; });
|
||||
setImmediate(() => {});
|
||||
`;
|
||||
return spawnSync(process.execPath, ['-e', script], { encoding: 'utf-8' });
|
||||
@@ -245,5 +252,35 @@ describe('regressions', () => {
|
||||
`expected "unexpected boom" in stderr, got: ${stderrTrimmed.slice(0, 200)}`
|
||||
);
|
||||
});
|
||||
|
||||
// #2979: characterization test pinning the two error paths under json-errors
|
||||
// mode. The structured envelope covers non-ExitError failures; ExitError
|
||||
// (usage errors) intentionally emits plain text with its own exit code.
|
||||
// Both halves asserted together so the code cannot drift toward the doc's
|
||||
// prior overstated claim that EVERY error emits JSON.
|
||||
test('#2979: ExitError emits plain text (not JSON) even under --json-errors; non-ExitError emits the envelope', () => {
|
||||
// ExitError path: plain text, own exit code, NOT a JSON object.
|
||||
const exitResult = spawnJsonErrorRun({
|
||||
jsonMode: true,
|
||||
errorType: 'ExitError',
|
||||
message: 'Usage: gsd-tools <command> [args]',
|
||||
});
|
||||
assert.strictEqual(exitResult.status, 1, 'ExitError exits with its code');
|
||||
const exitStderr = exitResult.stderr.trim();
|
||||
let exitParsed = null;
|
||||
try { exitParsed = JSON.parse(exitStderr); } catch { /* expected — plain text */ }
|
||||
assert.strictEqual(exitParsed, null,
|
||||
`ExitError must emit plain text, not JSON; got: ${exitStderr.slice(0, 200)}`);
|
||||
assert.ok(exitStderr.includes('Usage'),
|
||||
`ExitError plain-text message must reach stderr; got: ${exitStderr.slice(0, 200)}`);
|
||||
|
||||
// Non-ExitError path: structured JSON envelope.
|
||||
const envResult = spawnJsonErrorRun({ jsonMode: true });
|
||||
assert.strictEqual(envResult.status, 1);
|
||||
const envParsed = JSON.parse(envResult.stderr.trim());
|
||||
assert.strictEqual(envParsed.ok, false);
|
||||
assert.strictEqual(envParsed.reason, 'sdk_fail_fast');
|
||||
assert.ok(envParsed.message, 'envelope must carry a message');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
{
|
||||
"version": 1,
|
||||
"paths": {
|
||||
"code-review.md": "#2989: the diff-base fallback's git log --grep was changed from an unanchored bare phase number (matching version strings, dates, issue refs) to an anchored '[Pp]hase N\\b' with --extended-regexp, plus a 5-line comment explaining the anchor. Makes the fail-closed branch reachable when no commit genuinely references the phase."
|
||||
}
|
||||
}
|
||||
@@ -89,7 +89,8 @@ describe('W021 — milestone-prefixed phase ID convention', () => {
|
||||
]);
|
||||
|
||||
const result = runGsdTools(['roadmap', 'validate'], tmpDir);
|
||||
assert.ok(result.success, `roadmap validate should exit 0 even with warnings: ${result.error}`);
|
||||
// #2978: validate now exits non-zero on any warning (per its documented contract).
|
||||
assert.strictEqual(result.success, false, `roadmap validate must exit non-zero on W021 warnings: ${result.error}`);
|
||||
|
||||
const out = JSON.parse(result.output);
|
||||
assert.ok(Array.isArray(out.warnings), 'output.warnings should be an array');
|
||||
@@ -219,7 +220,8 @@ describe('W021 — milestone-prefixed phase ID convention', () => {
|
||||
]);
|
||||
|
||||
const result = runGsdTools(['roadmap', 'validate'], tmpDir);
|
||||
assert.ok(result.success, `roadmap validate failed: ${result.error}`);
|
||||
// #2978: validate now exits non-zero on any warning (per its documented contract).
|
||||
assert.strictEqual(result.success, false, `roadmap validate must exit non-zero on W021 warnings: ${result.error}`);
|
||||
|
||||
const out = JSON.parse(result.output);
|
||||
const w021 = (out.warnings || []).filter(w => w.code === 'W021');
|
||||
|
||||
@@ -63,23 +63,28 @@ test('REACHABILITY: the /gsd handler dispatches a real family through gsd-tools.
|
||||
const dir = createTempDir();
|
||||
try {
|
||||
const result = await pi._recorded.commands['gsd'].handler('progress json', { cwd: dir });
|
||||
assert.equal(typeof result, 'string', '/gsd handler returns a string result');
|
||||
const parsed = JSON.parse(result);
|
||||
// #2991: handler returns { content: [{ type: 'text', text }] } (Pi's display shape), not a bare string.
|
||||
assert.ok(result && Array.isArray(result.content) && result.content[0].type === 'text',
|
||||
`/gsd handler must return Pi's display shape { content: [{ type: 'text', text }] }; got: ${JSON.stringify(result).slice(0, 200)}`);
|
||||
const parsed = JSON.parse(result.content[0].text);
|
||||
assert.equal(typeof parsed.percent, 'number', '/gsd dispatch reached gsd-tools.cjs for real (the engine was reached)');
|
||||
} finally {
|
||||
cleanup(dir);
|
||||
}
|
||||
});
|
||||
|
||||
test('REACHABILITY: an unknown family surfaces a clear GSD error string, not a throw', async () => {
|
||||
test('REACHABILITY: an unknown family surfaces a clear GSD error, not a throw', async () => {
|
||||
const pi = mockPi();
|
||||
gsdPiExtension(pi);
|
||||
const dir = createTempDir();
|
||||
try {
|
||||
const result = await pi._recorded.commands['gsd'].handler('no-such-family-8675309', { cwd: dir });
|
||||
assert.equal(typeof result, 'string');
|
||||
assert.match(result, /GSD error:/);
|
||||
assert.match(result, /no-such-family-8675309|Unknown command/);
|
||||
// #2991: handler returns { content: [{ type: 'text', text }] } (Pi's display shape).
|
||||
assert.ok(result && Array.isArray(result.content) && result.content[0].type === 'text',
|
||||
`error result must carry Pi's display shape; got: ${JSON.stringify(result).slice(0, 200)}`);
|
||||
const text = result.content[0].text;
|
||||
assert.match(text, /GSD error:/);
|
||||
assert.match(text, /no-such-family-8675309|Unknown command/);
|
||||
} finally {
|
||||
cleanup(dir);
|
||||
}
|
||||
|
||||
@@ -3612,3 +3612,79 @@ describe('bug #1103 — annotate-dependencies preserves newline before Plans: he
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
// bug #2978: roadmap validate returns {"warnings":[]} for every input
|
||||
// ─────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
describe('bug #2978: roadmap validate performs structural validation', () => {
|
||||
test('empty (zero-byte) ROADMAP.md → non-empty warnings, non-zero exit', () => {
|
||||
const tmpDir = createTempProject('gsd-2978-empty-');
|
||||
try {
|
||||
fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), '');
|
||||
const result = runGsdTools(['roadmap', 'validate', '--raw'], tmpDir);
|
||||
assert.strictEqual(result.success, false, 'empty file must exit non-zero');
|
||||
const payload = JSON.parse(result.output);
|
||||
assert.ok(Array.isArray(payload.warnings) && payload.warnings.length > 0,
|
||||
`empty file must produce non-empty warnings; got: ${JSON.stringify(payload)}`);
|
||||
} finally { cleanup(tmpDir); }
|
||||
});
|
||||
|
||||
test('garbage/non-roadmap text → non-empty warnings, non-zero exit', () => {
|
||||
const tmpDir = createTempProject('gsd-2978-garbage-');
|
||||
try {
|
||||
fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'), 'not a roadmap at all');
|
||||
const result = runGsdTools(['roadmap', 'validate', '--raw'], tmpDir);
|
||||
assert.strictEqual(result.success, false, 'garbage must exit non-zero');
|
||||
const payload = JSON.parse(result.output);
|
||||
assert.ok(payload.warnings.length > 0, 'garbage must produce warnings');
|
||||
} finally { cleanup(tmpDir); }
|
||||
});
|
||||
|
||||
test('missing ROADMAP.md → non-empty warnings, non-zero exit', () => {
|
||||
const tmpDir = createTempProject('gsd-2978-missing-');
|
||||
try {
|
||||
// createTempProject creates .planning/phases but no ROADMAP.md — don't write one
|
||||
const result = runGsdTools(['roadmap', 'validate', '--raw'], tmpDir);
|
||||
assert.strictEqual(result.success, false, 'missing file must exit non-zero');
|
||||
const payload = JSON.parse(result.output);
|
||||
assert.ok(payload.warnings.length > 0, 'missing file must produce warnings');
|
||||
} finally { cleanup(tmpDir); }
|
||||
});
|
||||
|
||||
test('truncated frontmatter (unterminated ---) → non-empty warnings', () => {
|
||||
const tmpDir = createTempProject('gsd-2978-trunc-');
|
||||
try {
|
||||
fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'),
|
||||
'---\nmilestone: v1.0\n### Phase 1: Setup\n');
|
||||
const result = runGsdTools(['roadmap', 'validate', '--raw'], tmpDir);
|
||||
assert.strictEqual(result.success, false, 'truncated frontmatter must exit non-zero');
|
||||
const payload = JSON.parse(result.output);
|
||||
assert.ok(payload.warnings.length > 0, 'truncated frontmatter must produce warnings');
|
||||
} finally { cleanup(tmpDir); }
|
||||
});
|
||||
|
||||
test('well-formed roadmap → warnings: [], exit 0 (no false positive)', () => {
|
||||
const tmpDir = createTempProject('gsd-2978-good-');
|
||||
try {
|
||||
fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'),
|
||||
'# Roadmap\n\n## v1.0\n\n### Phase 1: Foundation\n**Goal:** setup\n');
|
||||
const result = runGsdTools(['roadmap', 'validate', '--raw'], tmpDir);
|
||||
assert.ok(result.success, `well-formed roadmap must exit 0; got: ${result.error}`);
|
||||
const payload = JSON.parse(result.output);
|
||||
assert.deepStrictEqual(payload.warnings, [], 'well-formed roadmap must have no warnings');
|
||||
} finally { cleanup(tmpDir); }
|
||||
});
|
||||
|
||||
test('BOM-prefixed well-formed roadmap → warnings: [], exit 0 (not corruption)', () => {
|
||||
const tmpDir = createTempProject('gsd-2978-bom-');
|
||||
try {
|
||||
fs.writeFileSync(path.join(tmpDir, '.planning', 'ROADMAP.md'),
|
||||
'\uFEFF# Roadmap\n\n### Phase 1: Foundation\n**Goal:** setup\n');
|
||||
const result = runGsdTools(['roadmap', 'validate', '--raw'], tmpDir);
|
||||
assert.ok(result.success, `BOM-prefixed roadmap must exit 0; got: ${result.error}`);
|
||||
const payload = JSON.parse(result.output);
|
||||
assert.deepStrictEqual(payload.warnings, [], 'BOM is not corruption');
|
||||
} finally { cleanup(tmpDir); }
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1340,6 +1340,61 @@ describe('ADR-1769 #1796: applyStatePreservation — table-driven post-sync cons
|
||||
'total_plans equality → derived value (identity)');
|
||||
});
|
||||
|
||||
test('#2969: deriveProgressKeys=true — completed_plans ratchets UP when disk count exceeds curated (gap-closure plans completed)', () => {
|
||||
// Gap-closure scenario: a phase had 50 plans all summarized (completed_plans: 50),
|
||||
// then 4 gap-closure plans were added (total_plans -> 54) and all 4 got SUMMARYs.
|
||||
// Disk scan now counts 54 summaries. The curated completed_plans (50) must
|
||||
// ratchet UP to the derived value (54), not stay pinned at 50 — otherwise
|
||||
// completed_plans < total_plans forever even though every plan is summarized.
|
||||
const curated = { progress: { total_plans: 54, completed_plans: 50, total_phases: 2, completed_phases: 1, percent: 93 } };
|
||||
const r = applyStatePreservation({
|
||||
preFm: curated,
|
||||
preFmSnapshot: curated,
|
||||
postFm: { progress: { total_plans: 54, completed_plans: 54, total_phases: 2, completed_phases: 1, percent: 100 } },
|
||||
resync: false,
|
||||
deriveProgressKeys: true,
|
||||
...untouched,
|
||||
});
|
||||
assert.equal(r.postFm.progress.total_plans, 54, 'total_plans takes derived value');
|
||||
assert.equal(r.postFm.progress.completed_plans, 54,
|
||||
'completed_plans must ratchet UP to derived (54 > curated 50 — gap-closure plans completed) (#2969)');
|
||||
assert.equal(r.postFm.progress.percent, 100,
|
||||
'percent must reflect the true completion fraction (54/54) (#2969)');
|
||||
});
|
||||
|
||||
test('#2969 ratchet-down protection: deriveProgressKeys=true keeps curated when disk count < curated', () => {
|
||||
// The ratchet must only go UP. If the disk count is somehow LOWER than
|
||||
// curated (e.g. a SUMMARY was deleted), keep the curated value — do not
|
||||
// derive downward. (#3242 curated-progress protection, scoped to deriveProgressKeys.)
|
||||
const curated = { progress: { total_plans: 54, completed_plans: 50, percent: 93 } };
|
||||
const r = applyStatePreservation({
|
||||
preFm: curated,
|
||||
preFmSnapshot: curated,
|
||||
postFm: { progress: { total_plans: 54, completed_plans: 47, percent: 87 } },
|
||||
resync: false,
|
||||
deriveProgressKeys: true,
|
||||
...untouched,
|
||||
});
|
||||
assert.equal(r.postFm.progress.completed_plans, 50,
|
||||
'completed_plans must NOT derive downward (47 < curated 50) — ratchet-up only (#2969/#3242)');
|
||||
});
|
||||
|
||||
test('#2969 body-only write protection: deriveProgressKeys absent keeps wholesale restore', () => {
|
||||
// state.update/patch (no deriveProgressKeys flag) must keep the full #3242
|
||||
// wholesale curated restore — completed_plans never moves for a body-only edit.
|
||||
const curated = { progress: { total_plans: 54, completed_plans: 50, percent: 93 } };
|
||||
const r = applyStatePreservation({
|
||||
preFm: curated,
|
||||
preFmSnapshot: curated,
|
||||
postFm: { progress: { total_plans: 54, completed_plans: 54, percent: 100 } },
|
||||
resync: false,
|
||||
// deriveProgressKeys NOT set — body-only write path
|
||||
...untouched,
|
||||
});
|
||||
assert.equal(r.postFm.progress.completed_plans, 50,
|
||||
'body-only write must keep curated completed_plans (no deriveProgressKeys) (#2969/#3242)');
|
||||
});
|
||||
|
||||
test('progress: NOT restored when transition re-derives from disk (resync=true) — sync/advancePlan/completePhase path', () => {
|
||||
const recomputed = { progress: { total_phases: 5, completed_phases: 1, percent: 20 } };
|
||||
const r = applyStatePreservation({
|
||||
|
||||
Reference in New Issue
Block a user