Merge branch 'next' into feat/1173-wire-agent-converters-descriptor
This commit is contained in:
Tom Boucher
@@ -72,7 +72,6 @@ const CANONICAL_HEADERS: Record<CanonicalHeader, string[]> = {
|
||||
'candidates',
|
||||
'approaches considered',
|
||||
'variants',
|
||||
'trade-offs',
|
||||
'pros and cons of the options',
|
||||
'discussion',
|
||||
],
|
||||
@@ -208,13 +207,30 @@ function normalizeAdrHeader(raw: unknown): string {
|
||||
.trim();
|
||||
}
|
||||
|
||||
function classifyHeader(normalizedHeader: string): CanonicalHeader | null {
|
||||
// Normalized synonym index (audit M7). classifyHeader receives an ALREADY-normalized
|
||||
// header (via normalizeAdrHeader), but historically compared it against the RAW synonym
|
||||
// strings. Because normalizeAdrHeader collapses [\s:._-]+ to a space and strips [^\w\s],
|
||||
// any synonym carrying a hyphen/apostrophe/etc. ('trade-offs', "won't do", 'post-grilling')
|
||||
// could never match a normalized header — it was silently dead, and its ADR section went
|
||||
// unmapped. Normalizing BOTH sides closes that abstraction asymmetry once, so every synonym
|
||||
// (current and future) is reachable regardless of punctuation. Precomputed at module load to
|
||||
// avoid re-normalizing the whole table per call; insertion order is preserved so first-match-
|
||||
// wins and the exact-then-prefix precedence stay identical to the prior raw-compare loop.
|
||||
const _NORMALIZED_SYNONYM_INDEX: Array<[string, CanonicalHeader]> = (() => {
|
||||
const index: Array<[string, CanonicalHeader]> = [];
|
||||
for (const [canonical, synonyms] of Object.entries(CANONICAL_HEADERS) as Array<[CanonicalHeader, string[]]>) {
|
||||
for (const synonym of synonyms) {
|
||||
if (normalizedHeader === synonym) return canonical;
|
||||
if (normalizedHeader.startsWith(`${synonym} `)) return canonical;
|
||||
index.push([normalizeAdrHeader(synonym), canonical]);
|
||||
}
|
||||
}
|
||||
return index;
|
||||
})();
|
||||
|
||||
function classifyHeader(normalizedHeader: string): CanonicalHeader | null {
|
||||
for (const [synonym, canonical] of _NORMALIZED_SYNONYM_INDEX) {
|
||||
if (normalizedHeader === synonym) return canonical;
|
||||
if (normalizedHeader.startsWith(`${synonym} `)) return canonical;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
|
||||
283
src/commands.cts
283
src/commands.cts
@@ -9,6 +9,7 @@
|
||||
import fs from 'node:fs';
|
||||
import path from 'node:path';
|
||||
import { execGit, platformWriteSync, platformReadSync, platformEnsureDir } from './shell-command-projection.cjs';
|
||||
import { requireSafePath, sanitizeForDisplay } from './security.cjs';
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports
|
||||
import ioMod = require('./io.cjs');
|
||||
const { output, error } = ioMod;
|
||||
@@ -195,6 +196,120 @@ function cmdListTodos(cwd: string, area: string | undefined, raw: boolean): void
|
||||
output(result, raw, count.toString());
|
||||
}
|
||||
|
||||
/**
|
||||
* List captured seeds from .planning/seeds/SEED-*.md for browsing/audit (#441).
|
||||
*
|
||||
* Unlike audit.scanSeeds (which returns only *unimplemented* seeds for the
|
||||
* milestone surface), this lists seeds of every status with the richer fields a
|
||||
* human audit needs (scope, trigger, planted date). An optional case-insensitive
|
||||
* status filter narrows the set. Seed content is user-controlled, so every
|
||||
* displayed field is passed through sanitizeForDisplay and each file path is
|
||||
* validated with requireSafePath before reading. Read-only — never mutates.
|
||||
*/
|
||||
/**
|
||||
* Derive the canonical `{ seed_id, slug }` from a seed filename stem and the
|
||||
* frontmatter `id:` value. Pure (no I/O) so it can be property-tested directly.
|
||||
*
|
||||
* seed_id: frontmatter `id:` when it matches `SEED-NNN`, else the numeric prefix
|
||||
* of the filename (`SEED-NNN-…`), else the whole stem. slug: the descriptive
|
||||
* remainder after `SEED-NNN-`, else the stem with a leading `SEED-` stripped.
|
||||
* `rawFmId` is `unknown` because frontmatter values are not guaranteed strings.
|
||||
*/
|
||||
function deriveSeedIdentity(stem: string, rawFmId: unknown): { seed_id: string; slug: string } {
|
||||
const fmId = typeof rawFmId === 'string' ? rawFmId.trim() : '';
|
||||
let seedId: string;
|
||||
if (/^SEED-\d+$/i.test(fmId)) {
|
||||
seedId = fmId;
|
||||
} else {
|
||||
const numMatch = stem.match(/^(SEED-\d+)/i);
|
||||
seedId = numMatch ? numMatch[1] : stem;
|
||||
}
|
||||
const slugMatch = stem.match(/^SEED-\d+-(.+)$/i);
|
||||
const slug = slugMatch ? slugMatch[1] : stem.replace(/^SEED-/i, '');
|
||||
return { seed_id: seedId, slug };
|
||||
}
|
||||
|
||||
function cmdListSeeds(cwd: string, statusFilter: string | undefined, raw: boolean): void {
|
||||
const planDir = planningDir(cwd);
|
||||
const seedsDir = path.join(planDir, 'seeds');
|
||||
const wantStatus = statusFilter ? statusFilter.trim().toLowerCase() : null;
|
||||
|
||||
const seeds: Array<{
|
||||
seed_id: string; slug: string; status: string; scope: string;
|
||||
trigger_when: string; planted: string; title: string; path: string;
|
||||
}> = [];
|
||||
const summary: Record<string, number> = {};
|
||||
|
||||
// Frontmatter values are not guaranteed to be scalars: extractFrontmatter
|
||||
// yields {} for a bare `key:` line and an array for `key: [a, b]`. Coerce every
|
||||
// read to a string so one malformed seed cannot crash the whole audit list
|
||||
// (`.toLowerCase()` on a non-string throws) or leak a raw object/array into the
|
||||
// JSON contract. Mirrors the existing `typeof fm.id === 'string'` guard below.
|
||||
const fmStr = (v: unknown): string => (typeof v === 'string' ? v : '');
|
||||
|
||||
let files: fs.Dirent[];
|
||||
try {
|
||||
files = fs.readdirSync(seedsDir, { withFileTypes: true });
|
||||
} catch {
|
||||
// No seeds dir (or unreadable) — an empty, non-error result. The seed dir is
|
||||
// created lazily by the first plant-seed, so absence is the normal zero case.
|
||||
output({ count: 0, seeds: [], summary: {} }, raw, '0');
|
||||
return;
|
||||
}
|
||||
|
||||
for (const entry of files) {
|
||||
if (!entry.isFile()) continue;
|
||||
if (!entry.name.startsWith('SEED-') || !entry.name.endsWith('.md')) continue;
|
||||
|
||||
let safeFilePath: string;
|
||||
try {
|
||||
safeFilePath = requireSafePath(path.join(seedsDir, entry.name), planDir, 'seed file', { allowAbsolute: true });
|
||||
} catch {
|
||||
continue;
|
||||
}
|
||||
const content = platformReadSync(safeFilePath);
|
||||
if (content === null) continue;
|
||||
|
||||
const fm = extractFrontmatter(content) as Record<string, unknown>;
|
||||
const status = (fmStr(fm.status) || 'dormant').toLowerCase().trim() || 'dormant';
|
||||
|
||||
// Match on the raw lowercased status (both sides already normalized);
|
||||
// sanitizeForDisplay is for output, not comparison.
|
||||
if (wantStatus && status !== wantStatus) continue;
|
||||
|
||||
// Canonical seed id is `SEED-NNN` (frontmatter `id:`, e.g. SEED-001). Fall
|
||||
// back to the numeric prefix of the filename, then to the whole stem. The
|
||||
// descriptive remainder of the filename (`SEED-NNN-<slug>.md`) is the slug.
|
||||
const stem = path.basename(entry.name, '.md');
|
||||
const { seed_id: seedId, slug } = deriveSeedIdentity(stem, fm.id);
|
||||
|
||||
let title = sanitizeForDisplay(fmStr(fm.title).slice(0, 100));
|
||||
if (!title) {
|
||||
const headingMatch = content.match(/^#\s*(.+)$/m);
|
||||
if (headingMatch) title = sanitizeForDisplay(headingMatch[1].trim().slice(0, 100));
|
||||
}
|
||||
|
||||
const safeStatus = sanitizeForDisplay(status);
|
||||
summary[safeStatus] = (summary[safeStatus] || 0) + 1;
|
||||
|
||||
seeds.push({
|
||||
seed_id: sanitizeForDisplay(seedId),
|
||||
slug: sanitizeForDisplay(slug),
|
||||
status: safeStatus,
|
||||
scope: sanitizeForDisplay(fmStr(fm.scope) || 'unknown'),
|
||||
trigger_when: sanitizeForDisplay(fmStr(fm.trigger_when)),
|
||||
planted: sanitizeForDisplay(fmStr(fm.planted)),
|
||||
title,
|
||||
path: toPosixPath(path.relative(cwd, safeFilePath)),
|
||||
});
|
||||
}
|
||||
|
||||
// Stable order: by seed_id so output is deterministic across filesystems.
|
||||
seeds.sort((a, b) => a.seed_id.localeCompare(b.seed_id));
|
||||
|
||||
output({ count: seeds.length, seeds, summary }, raw, seeds.length.toString());
|
||||
}
|
||||
|
||||
function cmdVerifyPathExists(cwd: string, targetPath: string | undefined, raw: boolean): void {
|
||||
if (!targetPath) {
|
||||
error('path required for verification');
|
||||
@@ -729,6 +844,171 @@ function cmdCommitToSubrepo(cwd: string, message: string | undefined, files: str
|
||||
output(result, raw, Object.entries(repos).map(([r, v]) => `${r}:${v.hash || 'skip'}`).join(' '));
|
||||
}
|
||||
|
||||
/**
|
||||
* Prepare a sub-repo for a companion PR branch.
|
||||
*
|
||||
* Detects uncommitted changes, creates a new branch, stages every changed
|
||||
* file explicitly (never git add -A per universal-anti-patterns.md:44), commits,
|
||||
* and pushes with --set-upstream. Returns a structured result the workflow uses
|
||||
* to call `gh pr create`.
|
||||
*
|
||||
* On a stage/commit failure (nothing committed yet), the branch is deleted and
|
||||
* the caller is returned to the original HEAD so the repo is left clean. On a
|
||||
* push failure, the commit already exists — the branch is left in place instead
|
||||
* so the user's work is not lost; the error includes a retry instruction.
|
||||
*/
|
||||
function cmdPrSubrepo(
|
||||
cwd: string,
|
||||
repo: string | undefined,
|
||||
branch: string | undefined,
|
||||
commitMessage: string | undefined,
|
||||
raw: boolean,
|
||||
): void {
|
||||
if (!repo) {
|
||||
error('--repo required');
|
||||
}
|
||||
if (!branch) {
|
||||
error('--branch required');
|
||||
}
|
||||
if (!commitMessage || commitMessage.startsWith('--')) {
|
||||
error('commit message required');
|
||||
}
|
||||
if ((branch as string).startsWith('-')) {
|
||||
error(`Branch name must not start with '-': ${branch}`);
|
||||
}
|
||||
|
||||
// 0. Security: validate repo path is contained within the workspace root.
|
||||
// Uses security.cjs validatePath (symlink-safe realpathSync + startsWith guard)
|
||||
// to reject ../escape, absolute paths, and symlink traversal.
|
||||
// eslint-disable-next-line @typescript-eslint/no-require-imports, @typescript-eslint/unbound-method
|
||||
const { validatePath } = require('./security.cjs') as {
|
||||
validatePath(filePath: string, baseDir: string): { safe: boolean; resolved: string; error?: string };
|
||||
};
|
||||
const pathCheck = validatePath(repo as string, cwd);
|
||||
if (!pathCheck.safe) {
|
||||
error(`Sub-repo path is unsafe: ${pathCheck.error}`);
|
||||
}
|
||||
const repoCwd = pathCheck.resolved;
|
||||
if (!fs.existsSync(repoCwd)) {
|
||||
error(`Sub-repo not found: ${repoCwd}`);
|
||||
}
|
||||
|
||||
// 1. Collect changed files via porcelain status — explicit, never git add -A.
|
||||
// ?? (untracked) lines are excluded — only stage tracked modifications.
|
||||
const statusResult = execGit(['-c', 'core.quotePath=false', 'status', '--porcelain'], { cwd: repoCwd });
|
||||
if (statusResult.exitCode !== 0) {
|
||||
error(`git status failed in ${repo}: ${statusResult.stderr}`);
|
||||
}
|
||||
|
||||
// Parse porcelain output into two lists:
|
||||
// changedFiles — all affected paths (old + new for renames) → goes into result.files
|
||||
// filesToStage — paths to pass to git add (rename old-paths are already staged by
|
||||
// the rename op and no longer exist in the worktree; only add new paths)
|
||||
const changedFiles: string[] = [];
|
||||
const filesToStage: string[] = [];
|
||||
for (const line of statusResult.stdout.split('\n').filter(Boolean).filter(l => !l.startsWith('??'))) {
|
||||
// execGit trims the entire stdout string, which may strip the leading X-status
|
||||
// space from the first output line. Normalize before slicing.
|
||||
const normalized = line.trimStart();
|
||||
const file = normalized.slice(2).trim();
|
||||
const arrowIdx = file.indexOf(' -> ');
|
||||
if (arrowIdx !== -1) {
|
||||
const oldPath = file.slice(0, arrowIdx).trim();
|
||||
const newPath = file.slice(arrowIdx + 4).trim();
|
||||
changedFiles.push(oldPath, newPath);
|
||||
filesToStage.push(newPath); // old path already staged; worktree no longer has it
|
||||
} else {
|
||||
changedFiles.push(file);
|
||||
filesToStage.push(file);
|
||||
}
|
||||
}
|
||||
|
||||
if (changedFiles.length === 0) {
|
||||
output(
|
||||
{ ok: true, repo, branch, committed: false, reason: 'nothing_to_commit', files: [] },
|
||||
raw,
|
||||
'nothing_to_commit',
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
// 2. Guard: refuse if branch already exists — checkout -b is non-idempotent
|
||||
const branchCheck = execGit(['rev-parse', '--verify', branch as string], { cwd: repoCwd });
|
||||
if (branchCheck.exitCode === 0) {
|
||||
error(`Branch already exists in ${repo}: ${branch}. Delete it first or choose a unique name.`);
|
||||
}
|
||||
|
||||
// Capture current HEAD before switching so rollback can return explicitly.
|
||||
// git checkout - fails on a fresh single-branch repo with no prior HEAD.
|
||||
const prevBranchResult = execGit(['rev-parse', '--abbrev-ref', 'HEAD'], { cwd: repoCwd });
|
||||
const prevBranchName = prevBranchResult.exitCode === 0 ? prevBranchResult.stdout.trim() : null;
|
||||
|
||||
// 3. Create branch
|
||||
const checkoutResult = execGit(['checkout', '-b', branch as string], { cwd: repoCwd });
|
||||
if (checkoutResult.exitCode !== 0) {
|
||||
error(`Failed to create branch ${branch} in ${repo}: ${checkoutResult.stderr}`);
|
||||
}
|
||||
|
||||
// Helper: rollback the created branch and return to the previous HEAD.
|
||||
const rollback = (): void => {
|
||||
if (prevBranchName) {
|
||||
execGit(['checkout', prevBranchName], { cwd: repoCwd });
|
||||
}
|
||||
execGit(['branch', '-D', branch as string], { cwd: repoCwd });
|
||||
};
|
||||
|
||||
// 4. Stage explicit files (never git add -A per universal-anti-patterns.md:44)
|
||||
for (const file of filesToStage) {
|
||||
const addResult = execGit(['add', '--', file], { cwd: repoCwd });
|
||||
if (addResult.exitCode !== 0) {
|
||||
rollback();
|
||||
error(`Failed to stage ${file} in ${repo}: ${addResult.stderr}`);
|
||||
}
|
||||
}
|
||||
|
||||
// 5. Commit
|
||||
const commitResult = execGit(['commit', '-m', commitMessage as string], { cwd: repoCwd });
|
||||
if (commitResult.exitCode !== 0) {
|
||||
rollback();
|
||||
error(`Failed to commit in ${repo}: ${commitResult.stderr}`);
|
||||
}
|
||||
|
||||
// 6. Capture commit hash
|
||||
const hashResult = execGit(['rev-parse', '--short', 'HEAD'], { cwd: repoCwd });
|
||||
const commitHash = hashResult.exitCode === 0 ? hashResult.stdout.trim() : null;
|
||||
|
||||
// 7. Capture remote URL and derive GitHub owner/repo slug for gh pr create
|
||||
const remoteResult = execGit(['remote', 'get-url', 'origin'], { cwd: repoCwd });
|
||||
const remoteUrl = remoteResult.exitCode === 0 ? remoteResult.stdout.trim() : null;
|
||||
let remoteSlug: string | null = null;
|
||||
if (remoteUrl) {
|
||||
const m = remoteUrl.match(/github\.com[:/](.+?)(?:\.git)?$/);
|
||||
remoteSlug = m ? m[1] : null;
|
||||
}
|
||||
|
||||
// 8. Push with --set-upstream so gh pr create can find the branch.
|
||||
// Network operation — use a longer timeout than the default 10 s.
|
||||
// Do NOT rollback on push failure — the commit already exists on the local branch.
|
||||
// Deleting the branch here would destroy the only ref holding the user's work.
|
||||
// Leave the branch in place so the user can retry the push.
|
||||
const pushResult = execGit(['push', '--set-upstream', 'origin', branch as string], { cwd: repoCwd, timeout: 60_000 });
|
||||
if (pushResult.exitCode !== 0) {
|
||||
error(`Failed to push ${branch} in ${repo}: ${pushResult.stderr}\nBranch ${branch} was created locally — retry with: git -C ${repo} push --set-upstream origin ${branch}`);
|
||||
}
|
||||
|
||||
const result = {
|
||||
ok: true,
|
||||
repo,
|
||||
branch,
|
||||
committed: true,
|
||||
files: changedFiles,
|
||||
commit_hash: commitHash,
|
||||
remote_url: remoteUrl,
|
||||
remote_slug: remoteSlug,
|
||||
};
|
||||
output(result, raw, `${repo}@${commitHash ?? 'unknown'}`);
|
||||
}
|
||||
|
||||
function cmdSummaryExtract(cwd: string, summaryPath: string | undefined, fields: string[] | undefined, raw: boolean): void {
|
||||
if (!summaryPath) {
|
||||
error('summary-path required for summary-extract');
|
||||
@@ -1413,6 +1693,8 @@ export = {
|
||||
cmdGenerateSlug,
|
||||
cmdCurrentTimestamp,
|
||||
cmdListTodos,
|
||||
cmdListSeeds,
|
||||
deriveSeedIdentity,
|
||||
cmdVerifyPathExists,
|
||||
cmdHistoryDigest,
|
||||
cmdResolveModel,
|
||||
@@ -1421,6 +1703,7 @@ export = {
|
||||
cmdEffortSync,
|
||||
cmdCommit,
|
||||
cmdCommitToSubrepo,
|
||||
cmdPrSubrepo,
|
||||
cmdSummaryExtract,
|
||||
cmdWebsearch,
|
||||
cmdProgressRender,
|
||||
|
||||
@@ -138,6 +138,11 @@ function _deepMergeConfig(base: Record<string, unknown>, overlay: Record<string,
|
||||
if (typeof base !== 'object' || typeof overlay !== 'object') return overlay;
|
||||
const result: Record<string, unknown> = { ...base };
|
||||
for (const key of Object.keys(overlay)) {
|
||||
// Prototype-pollution guard — mirrors the four sibling guards in this file
|
||||
// (lines ~315/319/331/341/549). Without it a workstream/root config.json with
|
||||
// {"__proto__": {...}} pollutes this merged object's prototype chain and can
|
||||
// spoof unset config flags. (Per-object pollution, not global Object.prototype.)
|
||||
if (key === '__proto__' || key === 'constructor' || key === 'prototype') continue;
|
||||
if (overlay[key] !== null && typeof overlay[key] === 'object' && !Array.isArray(overlay[key])) {
|
||||
result[key] = _deepMergeConfig((base[key] ?? {}) as Record<string, unknown>, overlay[key] as Record<string, unknown>);
|
||||
} else {
|
||||
|
||||
@@ -56,10 +56,14 @@ function extractFrontmatter(content: string): Frontmatter {
|
||||
const frontmatter: Frontmatter = {};
|
||||
// Match frontmatter only at byte 0 — a `---` block later in the document
|
||||
// body (YAML examples, horizontal rules) must never be treated as frontmatter.
|
||||
const match = content.match(/^---\r?\n([\s\S]+?)\r?\n---/);
|
||||
if (!match) return frontmatter;
|
||||
const headerEnd = content.startsWith('---\r\n') ? 5 : content.startsWith('---\n') ? 4 : -1;
|
||||
if (headerEnd === -1) return frontmatter;
|
||||
|
||||
const yaml = match[1];
|
||||
const closingLineStart = content.indexOf('\n---', headerEnd);
|
||||
if (closingLineStart === -1) return frontmatter;
|
||||
|
||||
const yamlEnd = content[closingLineStart - 1] === '\r' ? closingLineStart - 1 : closingLineStart;
|
||||
const yaml = content.slice(headerEnd, yamlEnd);
|
||||
const lines = yaml.split(/\r?\n/);
|
||||
|
||||
// Stack to track nested objects: [{obj, key, indent}]
|
||||
|
||||
12
src/init.cts
12
src/init.cts
@@ -2104,10 +2104,14 @@ function cmdAgentSkills(
|
||||
return;
|
||||
}
|
||||
|
||||
if (block) {
|
||||
process.stdout.write(block);
|
||||
}
|
||||
process.exit(0);
|
||||
// #1400: emit the raw block via the synchronous-flush output() helper (the same
|
||||
// one the --json branch uses) rather than process.stdout.write + process.exit(0).
|
||||
// When stdout is a pipe/file (how workflows consume this via command
|
||||
// substitution) the async stdout buffer is torn down by process.exit() before
|
||||
// it drains — on Windows this reliably truncates the write to 0 bytes, so every
|
||||
// ${AGENT_SKILLS_*} substitution expands empty. output() writes every byte with
|
||||
// writeAllSync and returns, letting the event loop drain naturally.
|
||||
output(block || '', true, block || '');
|
||||
}
|
||||
|
||||
interface SkillEntry {
|
||||
|
||||
@@ -37,6 +37,65 @@ process.on('exit', () => {
|
||||
}
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Lock liveness probe (test seam) — audit M1
|
||||
//
|
||||
// mtime is a leaky proxy for "the holder is alive". The prior withPlanningLock
|
||||
// timeout fallback unconditionally unlinked WHATEVER lock existed — even a fresh,
|
||||
// live holder's — and re-acquired it, force-stealing a live writer's critical
|
||||
// section. We backport capability-lock.cts's pid-liveness gate: a dead holder is
|
||||
// stolen promptly inside the polite loop; a live holder is waited on. The
|
||||
// indirection lets unit tests inject a deterministic isPidAlive without real pids.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/** Is `pid` a live process? process.kill(pid, 0) succeeds for a live (signalable) process. */
|
||||
function _realIsPidAlive(pid: number): boolean {
|
||||
try {
|
||||
process.kill(pid, 0);
|
||||
return true; // signalable → alive
|
||||
} catch (err) {
|
||||
// EPERM = process exists but we cannot signal it (still ALIVE). ESRCH = gone.
|
||||
return (err as NodeJS.ErrnoException).code === 'EPERM';
|
||||
}
|
||||
}
|
||||
|
||||
const _planningLockProbes: { isPidAlive: (pid: number) => boolean } = { isPidAlive: _realIsPidAlive };
|
||||
|
||||
function _planningLockIsPidAlive(pid: number): boolean {
|
||||
return _planningLockProbes.isPidAlive(pid);
|
||||
}
|
||||
|
||||
// Test seam (PR #1532 review): beforeSteal fires AFTER the steal decision but BEFORE
|
||||
// the identity re-confirm + atomic rename-steal, so a test can recreate a fresh lock
|
||||
// in the decision→steal gap and prove the identity re-confirm aborts a double-steal.
|
||||
// Defaults to a no-op; real callers are byte-for-behaviour unchanged.
|
||||
interface PlanningLockTestHooks {
|
||||
beforeSteal?: (ctx: { lockPath: string }) => void;
|
||||
}
|
||||
const _planningLockTestHooks: PlanningLockTestHooks = {};
|
||||
|
||||
// Monotonic sequence for unique stale-steal rename targets (no crypto dependency).
|
||||
let _planningStealSeq = 0;
|
||||
|
||||
/**
|
||||
* Is the holder recorded in the .lock body VERIFIED-LIVE? The body is JSON
|
||||
* { pid, cwd, acquired }. Returns true ONLY when the body parses AND the recorded
|
||||
* pid signals alive. A garbage / pid-less / unreadable body (or a dead pid) is NOT
|
||||
* verified-live, so the lock stays stealable — corrupt locks never block forever,
|
||||
* and a live holder is never force-stolen.
|
||||
*/
|
||||
function _planningHolderVerifiedLive(lockPath: string): boolean {
|
||||
let parsed: unknown;
|
||||
try {
|
||||
parsed = JSON.parse(fs.readFileSync(lockPath, 'utf-8'));
|
||||
} catch {
|
||||
return false; // unreadable / unparseable body → cannot verify → not verified-live
|
||||
}
|
||||
const pid = (parsed as { pid?: unknown } | null)?.pid;
|
||||
if (typeof pid !== 'number' || !Number.isInteger(pid) || pid <= 0) return false;
|
||||
return _planningLockIsPidAlive(pid);
|
||||
}
|
||||
|
||||
// Transient errno codes that indicate a temporary filesystem condition under
|
||||
// concurrent O_EXCL races — Docker overlay-fs (ENOENT/EINVAL/EIO), NFS
|
||||
// (ESTALE), and OS-level interrupt/retry signals (EAGAIN/EINTR). These are
|
||||
@@ -118,6 +177,12 @@ function withPlanningLock<T>(cwd: string, fn: () => T, clock?: Clock): T {
|
||||
if (clock === undefined) clock = realClock;
|
||||
const lockPath = path.join(planningDir(cwd), '.lock');
|
||||
const lockTimeout = 10000; // 10 seconds
|
||||
// Deadman ceiling (audit M1 / R4-FIX) — set ABOVE lockTimeout so a holder that reads
|
||||
// as alive but is actually a pid-reuse alias (the .lock body has no startTime, so
|
||||
// liveness alone cannot detect reuse) is still recovered once its lock ages past this
|
||||
// absolute ceiling. Without it, a false-alive holder would make withPlanningLock throw
|
||||
// on every call with no self-heal. Mirrors acquireStateLock's deadmanCeilingMs.
|
||||
const deadmanCeilingMs = 60000;
|
||||
const start = clock.now();
|
||||
|
||||
// Ensure .planning/ exists
|
||||
@@ -160,16 +225,68 @@ function withPlanningLock<T>(cwd: string, fn: () => T, clock?: Clock): T {
|
||||
continue;
|
||||
}
|
||||
if (nodeErr.code === 'EEXIST') {
|
||||
// Lock exists — check if stale (>30s old)
|
||||
// Liveness-gated steal (audit M1). Steal the lock PROMPTLY only when its
|
||||
// recorded holder is NOT verified-live (crashed/dead pid or garbage body).
|
||||
// A verified-live holder is waited on — never force-stolen — because nuking
|
||||
// a slow-but-live writer's lock corrupts the .planning/ critical section.
|
||||
// The steal is an ATOMIC rename-then-recreate guarded by an identity re-confirm
|
||||
// so a racer that recreates a fresh lock in the decision→steal gap never has
|
||||
// its replacement deleted (audit M2 / PR #1532 review, window b). The body is
|
||||
// written atomically (writeFileSync …{flag:'wx'}) so there is no empty-body
|
||||
// create window here — only the double-steal needs hardening.
|
||||
try {
|
||||
const stat = fs.statSync(lockPath);
|
||||
if (clock.now() - stat.mtimeMs > 30000) {
|
||||
fs.unlinkSync(lockPath);
|
||||
continue; // retry
|
||||
const decisionStat = fs.statSync(lockPath);
|
||||
// Snapshot the decision-time body too: (dev, ino) alone is defeated by inode
|
||||
// REUSE (a racer's unlink+recreate can land on the same inode), so the body
|
||||
// content binds the identity as well — mirrors capability-lock.cts's (dev,
|
||||
// ino, ts) re-confirm.
|
||||
let decisionBody: string | null;
|
||||
try { decisionBody = fs.readFileSync(lockPath, 'utf-8'); } catch { decisionBody = null; }
|
||||
let stealable = !_planningHolderVerifiedLive(lockPath);
|
||||
if (!stealable) {
|
||||
// Verified-live, but recover anyway once the lock crosses the absolute
|
||||
// deadman ceiling — defeats a pid-reuse false-alive that would otherwise
|
||||
// block forever (R4-FIX; mtime age is from lock creation, not this call).
|
||||
const age = clock.now() - decisionStat.mtimeMs;
|
||||
stealable = age > deadmanCeilingMs;
|
||||
}
|
||||
if (stealable) {
|
||||
if (_planningLockTestHooks.beforeSteal) _planningLockTestHooks.beforeSteal({ lockPath });
|
||||
// Identity re-confirm immediately before the steal: a racer that stole +
|
||||
// recreated a fresh lock in the decision→steal gap changes (dev, ino) → do
|
||||
// NOT delete the replacement; back off and re-evaluate.
|
||||
let confirmStat: fs.Stats;
|
||||
try {
|
||||
confirmStat = fs.statSync(lockPath);
|
||||
} catch {
|
||||
continue; // vanished between decision and steal — retry the create.
|
||||
}
|
||||
let confirmBody: string | null;
|
||||
try { confirmBody = fs.readFileSync(lockPath, 'utf-8'); } catch { confirmBody = null; }
|
||||
const sameInstance =
|
||||
typeof decisionStat.dev === 'number' && typeof decisionStat.ino === 'number' &&
|
||||
confirmStat.dev === decisionStat.dev && confirmStat.ino === decisionStat.ino &&
|
||||
decisionBody !== null && confirmBody === decisionBody;
|
||||
if (!sameInstance) {
|
||||
clock.sleep(100); // a racer won the steal + recreated — re-evaluate, don't delete it.
|
||||
continue;
|
||||
}
|
||||
// Atomic steal: rename the inode aside, then remove it. Only ONE racer can
|
||||
// win the rename; a failed rename means another process already stole it, so
|
||||
// we must NOT fall through to a delete — back off and retry the create.
|
||||
const stolen = lockPath + '.stale-' + process.pid + '-' + clock.now() + '-' + (_planningStealSeq++);
|
||||
let renamed = false;
|
||||
try { fs.renameSync(lockPath, stolen); renamed = true; } catch { /* another racer won */ }
|
||||
if (renamed) {
|
||||
try { fs.rmSync(stolen, { force: true }); } catch { /* best-effort */ }
|
||||
continue; // dead/garbage/expired holder freed — retry immediately to grab it.
|
||||
}
|
||||
clock.sleep(100); // lost the steal race — back off and retry.
|
||||
continue;
|
||||
}
|
||||
} catch { continue; }
|
||||
|
||||
// Wait and retry (cross-platform, no shell dependency)
|
||||
// Live holder — wait and retry (cross-platform, no shell dependency).
|
||||
clock.sleep(100);
|
||||
continue;
|
||||
}
|
||||
@@ -177,10 +294,18 @@ function withPlanningLock<T>(cwd: string, fn: () => T, clock?: Clock): T {
|
||||
}
|
||||
}
|
||||
|
||||
// Timeout — stale-lock recovery, then re-acquire atomically before entering critical section.
|
||||
try { fs.unlinkSync(lockPath); } catch { /* ok */ }
|
||||
acquireLock();
|
||||
return runWithHeldLock();
|
||||
// Timeout against a holder still present at budget exhaustion. The polite loop
|
||||
// already stole any DEAD holder; reaching here means the holder is verified-live
|
||||
// (or a pid-reuse alias we must not corrupt). Do NOT force-steal — the prior
|
||||
// unconditional `unlinkSync(lockPath); acquireLock()` here (audit M1) robbed live
|
||||
// writers, and its re-acquire sat OUTSIDE any try so a concurrent re-create raced
|
||||
// a raw EEXIST out of the helper (audit M2). Surface a clear timeout error instead.
|
||||
const timeoutErr = new Error(
|
||||
'withPlanningLock: ' + lockPath + ' held by a live process for ' +
|
||||
(clock.now() - start) + 'ms (exceeded ' + lockTimeout + 'ms budget)'
|
||||
);
|
||||
(timeoutErr as unknown as Record<string, unknown>).lockTimeout = true;
|
||||
throw timeoutErr;
|
||||
}
|
||||
|
||||
function createPlanningWorkspace(cwd: string, opts: WorkstreamAdapterOpts = {}): {
|
||||
@@ -269,4 +394,19 @@ export = {
|
||||
getActiveWorkstream,
|
||||
setActiveWorkstream,
|
||||
findContextMdIn,
|
||||
// Test seam (audit M1): inject a deterministic isPidAlive so the liveness-gated
|
||||
// steal decision is exercised without real pids. Mirrors capability-lock.cts.
|
||||
_setLockProbes(probes: Partial<{ isPidAlive: (pid: number) => boolean }>): void {
|
||||
if (typeof probes.isPidAlive === 'function') _planningLockProbes.isPidAlive = probes.isPidAlive;
|
||||
},
|
||||
_resetLockProbes(): void {
|
||||
_planningLockProbes.isPidAlive = _realIsPidAlive;
|
||||
},
|
||||
// Test seam (PR #1532 review): script the steal decision→steal gap (window b).
|
||||
_setPlanningLockTestHooks(hooks: PlanningLockTestHooks): void {
|
||||
if ('beforeSteal' in hooks) _planningLockTestHooks.beforeSteal = hooks.beforeSteal;
|
||||
},
|
||||
_resetPlanningLockTestHooks(): void {
|
||||
delete _planningLockTestHooks.beforeSteal;
|
||||
},
|
||||
};
|
||||
|
||||
@@ -303,6 +303,11 @@ export interface Prohibition {
|
||||
// against to MACHINE-PROVE fail-first. Projected only alongside a well-formed descriptor; absent ->
|
||||
// the producer hard-gates (green requires a fixture). Mirrors `CheckDescriptor.violationFixture`.
|
||||
check_violation_fixture?: string;
|
||||
// Optional 5th flat scalar (#1346): the path to a KNOWN-CLEAN control subject the prover ALSO runs
|
||||
// the check against, requiring it to stay GREEN — proving the violation RED is caused by the
|
||||
// subject's CONTENT, not merely by GSD_PROHIB_SUBJECT being set. Projected only alongside a
|
||||
// well-formed descriptor; absent -> no control (documented residual). Mirrors `CheckDescriptor.cleanFixture`.
|
||||
check_clean_fixture?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -387,6 +392,13 @@ export function projectProhibitions(
|
||||
if (typeof p.check_violation_fixture === 'string' && p.check_violation_fixture.trim() !== '') {
|
||||
entry.check_violation_fixture = String(p.check_violation_fixture);
|
||||
}
|
||||
// `check_clean_fixture` (#1346) rides BOTH kinds — the KNOWN-CLEAN control subject the prover
|
||||
// requires to stay GREEN (content-dependence proof). Emit ONLY a non-empty fixture (blank ->
|
||||
// absent so no control runs; the documented residual remains). Like the violation fixture it is
|
||||
// meaningless without the descriptor, so it lives inside this well-formed-descriptor branch.
|
||||
if (typeof p.check_clean_fixture === 'string' && p.check_clean_fixture.trim() !== '') {
|
||||
entry.check_clean_fixture = String(p.check_clean_fixture);
|
||||
}
|
||||
}
|
||||
out.push(entry);
|
||||
}
|
||||
|
||||
@@ -25,7 +25,7 @@ const { loadConfig } = configLoader;
|
||||
import { platformReadSync as safeReadFile, platformWriteSync, platformEnsureDir } from './shell-command-projection.cjs';
|
||||
import { getGlobalSkillDir, getGlobalConfigDir } from './runtime-homes.cjs';
|
||||
import { formatGsdSlash, resolveRuntime } from './runtime-slash.cjs';
|
||||
import { resolveRuntimeNameFromCandidates } from './runtime-name-policy.cjs';
|
||||
import { resolveRuntimeNameFromCandidates, getProjectInstructionFile } from './runtime-name-policy.cjs';
|
||||
|
||||
// ─── Types ────────────────────────────────────────────────────────────────────
|
||||
|
||||
@@ -1120,20 +1120,32 @@ function cmdGenerateClaudeMd(cwd: string, options: CmdGenerateClaudeMdOptions, r
|
||||
// repo-root `CLAUDE.md`, so generated GSD content does not land next to — or
|
||||
// pollute — a hand-crafted repo-root CLAUDE.md. An explicit `claude_md_path`
|
||||
// config value or `--output` still wins.
|
||||
let configClaudeMdPath = './.claude/CLAUDE.md';
|
||||
let configClaudeMdPath = '.claude/CLAUDE.md';
|
||||
try {
|
||||
const config = loadConfig(cwd);
|
||||
if (config['claude_md_path']) configClaudeMdPath = config['claude_md_path'] as string;
|
||||
if (config['claude_md_assembly']) assemblyConfig = config['claude_md_assembly'] as Record<string, unknown>;
|
||||
// #3163: When runtime is codex, override the output target to AGENTS.md
|
||||
// regardless of claude_md_path, so Codex projects never write to CLAUDE.md.
|
||||
// GSD_RUNTIME env var takes precedence over config.runtime, mirroring detectRuntime().
|
||||
// #1529: When no explicit --output is provided, derive the instruction
|
||||
// file from the runtime via the shared `getProjectInstructionFile` policy
|
||||
// (single source of truth in runtime-name-policy.cjs, shared with the
|
||||
// new-project.md bash workflow via `gsd-tools query
|
||||
// project-instruction-file`). Previously this was a codex-only override
|
||||
// (#3163) that left AGENTS-native runtimes (opencode/kilo/kimi) emitting
|
||||
// CLAUDE.md; copilot now resolves to .github/copilot-instructions.md, and
|
||||
// antigravity/gemini to GEMINI.md. GSD_RUNTIME env var takes precedence
|
||||
// over config.runtime, mirroring detectRuntime().
|
||||
//
|
||||
// Non-claude runtimes always win over a stale `claude_md_path` (the #3163
|
||||
// rationale: a Codex/AGENTS-native project must never write to CLAUDE.md
|
||||
// even if a prior Claude setup left a `claude_md_path` behind). For the
|
||||
// claude runtime, `claude_md_path` config is honored — it IS the
|
||||
// Claude-specific output setting (per #1098 and the #3163 non-codex test).
|
||||
const effectiveRuntime = resolveRuntimeNameFromCandidates(
|
||||
process.env['GSD_RUNTIME'],
|
||||
config['runtime']
|
||||
);
|
||||
if (!options.output && effectiveRuntime === 'codex') {
|
||||
configClaudeMdPath = './AGENTS.md';
|
||||
if (!options.output && effectiveRuntime && effectiveRuntime !== 'claude') {
|
||||
configClaudeMdPath = getProjectInstructionFile(effectiveRuntime);
|
||||
}
|
||||
} catch { /* use default */ }
|
||||
|
||||
|
||||
@@ -76,6 +76,15 @@ export interface CheckDescriptor {
|
||||
* prove fail-first; ABSENT for node-test → the default prover fails closed (never attestation).
|
||||
*/
|
||||
violationFixture?: string;
|
||||
/**
|
||||
* OPTIONAL author-supplied path to a KNOWN-CLEAN control subject (#1346). When present, the prover
|
||||
* runs the check against it as a CAUSATION CONTROL and requires it to stay GREEN — proof that the
|
||||
* RED on `violationFixture` was caused by the subject's CONTENT, not merely by `GSD_PROHIB_SUBJECT`
|
||||
* being set. A deceptive content-independent check reds on the clean subject too → control fails →
|
||||
* not proven. ABSENT → no control runs (the documented residual remains; backward-compatible with
|
||||
* the #1314 zero-authoring compose path). A supplied-but-missing path fails closed.
|
||||
*/
|
||||
cleanFixture?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -90,8 +99,9 @@ export interface CheckDescriptor {
|
||||
* - `null`/`undefined`/non-object input -> `null`.
|
||||
* - `check_kind` ABSENT -> `null` (no descriptor -> producer locates nothing -> fail-closed).
|
||||
* - `check_kind` present -> `{ kind: check_kind, target: check_target }`, adding `rule: check_rule`
|
||||
* ONLY when `check_rule` is a non-empty string, and `violationFixture: check_violation_fixture`
|
||||
* ONLY when that scalar is a non-empty string (#1346 — composes #1278 locate with #1279 proof).
|
||||
* ONLY when `check_rule` is a non-empty string, `violationFixture: check_violation_fixture`
|
||||
* ONLY when that scalar is a non-empty string (composes #1278 locate with #1279 proof), and
|
||||
* `cleanFixture: check_clean_fixture` ONLY when that scalar is non-empty (#1346 causation control).
|
||||
* - `failFirst` is NEVER sourced from the projection — it stays a verify-time caller attestation
|
||||
* (#1279 machine-proves it; out of scope here). The returned descriptor carries no `failFirst`.
|
||||
* - The adapter does NOT strictly validate kind/target/rule: it faithfully reconstructs whatever
|
||||
@@ -128,6 +138,12 @@ export function descriptorFromProjection(
|
||||
// hard-gates (fail-closed; green requires a fixture), never fabricated.
|
||||
const fixture = scalar(projected.check_violation_fixture);
|
||||
if (fixture.trim().length > 0) descriptor.violationFixture = fixture;
|
||||
// `cleanFixture` (#1346) rides BOTH kinds — reconstruct it from `check_clean_fixture` so the
|
||||
// causation control runs end-to-end: when present the prover also requires the check to stay GREEN
|
||||
// against this known-clean subject (proving the violation RED is content-dependent). Absent/blank ->
|
||||
// no control (the documented residual remains; backward-compatible with the #1314 compose path).
|
||||
const clean = scalar(projected.check_clean_fixture);
|
||||
if (clean.trim().length > 0) descriptor.cleanFixture = clean;
|
||||
return descriptor;
|
||||
}
|
||||
|
||||
@@ -438,6 +454,31 @@ function posTimeout(timeoutMs: number | undefined, def: number): number {
|
||||
return typeof timeoutMs === 'number' && timeoutMs > 0 ? timeoutMs : def;
|
||||
}
|
||||
|
||||
/**
|
||||
* Spawn the negative `node --test` against a single subject (set via the `GSD_PROHIB_SUBJECT`
|
||||
* convention, #1279) and return its TAP output. Reuses the bounded-subprocess machinery
|
||||
* (`process.execPath`, arg arrays → no shell, `childEnv`, bounded `timeout`/`maxBuffer`) and NEVER
|
||||
* throws — a RED run exits non-zero, so the partial TAP (with the `# fail` summary) is recovered from
|
||||
* the thrown error's `stdout`. The prover calls this once per subject: the KNOWN-BAD violation fixture
|
||||
* (expect RED) and, for the #1346 causation control, the KNOWN-CLEAN control subject (expect GREEN).
|
||||
*/
|
||||
function runNodeTestWithSubject(check: CheckDescriptor, cwd: string, subject: string, timeoutMs?: number): string {
|
||||
try {
|
||||
return execFileSync(process.execPath, buildNodeTestArgs(check), {
|
||||
cwd,
|
||||
encoding: 'utf-8',
|
||||
stdio: ['ignore', 'pipe', 'pipe'],
|
||||
windowsHide: true,
|
||||
env: { ...childEnv(), GSD_PROHIB_SUBJECT: subject },
|
||||
timeout: posTimeout(timeoutMs, NODE_TEST_TIMEOUT_MS),
|
||||
maxBuffer: CHECK_MAX_BUFFER,
|
||||
});
|
||||
} catch (e) {
|
||||
const stdout = e && typeof e === 'object' && 'stdout' in e ? (e as { stdout?: unknown }).stdout : '';
|
||||
return typeof stdout === 'string' ? stdout : '';
|
||||
}
|
||||
}
|
||||
|
||||
function defaultRunCheck(check: CheckDescriptor, cwd: string, timeoutMs?: number): CheckRunResult {
|
||||
try {
|
||||
if (check.kind === 'node-test') {
|
||||
@@ -554,34 +595,32 @@ function defaultProveFailFirst(check: CheckDescriptor, cwd: string, timeoutMs?:
|
||||
// a setup crash, not from the prohibition firing. Requiring the fixture to exist before spawning
|
||||
// closes the realistic typo/stale-path case (#1279 review, Major 1).
|
||||
//
|
||||
// KNOWN RESIDUAL (documented, fail-open direction, tracked follow-up #1346): existence is
|
||||
// necessary but not sufficient — a deliberately deceptive negative test that reds merely BECAUSE
|
||||
// `GSD_PROHIB_SUBJECT` is set (rather than because the subject's CONTENT violates the must-NOT)
|
||||
// is still accepted. Proving "the red was CAUSED BY the violation" cannot be done generically for
|
||||
// an arbitrary author-supplied test, so it is recorded as a constraint, not silently implied-solved.
|
||||
// CAUSATION (#1346): existence + a non-vacuous red is necessary but not sufficient — a deceptive
|
||||
// negative test that reds merely BECAUSE `GSD_PROHIB_SUBJECT` is set (rather than because the
|
||||
// subject's CONTENT violates the must-NOT) would otherwise be accepted. The OPTIONAL `cleanFixture`
|
||||
// control below proves content-dependence when supplied (red on bad AND green on clean). When NO
|
||||
// clean fixture is authored the control cannot run, so the residual remains a documented constraint
|
||||
// for that case (an author opts into the stronger proof by supplying a known-clean control subject).
|
||||
// Resolve the fixture against `cwd` (NOT the verify process's cwd): the spawned test reads
|
||||
// `GSD_PROHIB_SUBJECT` and resolves a relative subject against `cwd`, so the existence check must
|
||||
// use the SAME base or it could pass here yet ENOENT in the child (re-opening the fail-open hole).
|
||||
if (!fixture || !fs.existsSync(path.resolve(cwd, fixture))) return { provenFailFirst: false };
|
||||
let out = '';
|
||||
try {
|
||||
out = execFileSync(process.execPath, buildNodeTestArgs(check), {
|
||||
cwd,
|
||||
encoding: 'utf-8',
|
||||
stdio: ['ignore', 'pipe', 'pipe'],
|
||||
windowsHide: true,
|
||||
// CONVENTION (#1279): the negative test reads its subject-under-test from this env var.
|
||||
env: { ...childEnv(), GSD_PROHIB_SUBJECT: fixture },
|
||||
timeout: posTimeout(timeoutMs, NODE_TEST_TIMEOUT_MS),
|
||||
maxBuffer: CHECK_MAX_BUFFER,
|
||||
});
|
||||
} catch (e) {
|
||||
// A negative test that goes RED exits non-zero; the partial TAP (with the `# fail` summary)
|
||||
// is on stdout. Parse what we have: a real failure here is the PROOF the test is fail-first.
|
||||
const stdout = e && typeof e === 'object' && 'stdout' in e ? (e as { stdout?: unknown }).stdout : '';
|
||||
out = typeof stdout === 'string' ? stdout : '';
|
||||
// Run the negative test against the KNOWN-BAD subject and require a NON-VACUOUS red.
|
||||
const redOut = runNodeTestWithSubject(check, cwd, fixture, timeoutMs);
|
||||
if (!isNonVacuousNodeTestRed(redOut, check.target)) return { provenFailFirst: false, method: 'violation-fixture' };
|
||||
// #1346 CAUSATION CONTROL (optional): if a clean control subject is supplied, run the SAME test
|
||||
// against it and require it to stay GREEN. This proves the red above was caused by the subject's
|
||||
// CONTENT — a deceptive test that reds merely because GSD_PROHIB_SUBJECT is SET reds here too →
|
||||
// not content-dependent → not proven. Absent → no control (documented residual; backward-compat).
|
||||
const clean = check.cleanFixture;
|
||||
if (clean) {
|
||||
// A supplied-but-missing/typo'd control path can't run the control → fail-closed, symmetric
|
||||
// with the violation-fixture existence guard (resolve against the SAME `cwd` as the child).
|
||||
if (!fs.existsSync(path.resolve(cwd, clean))) return { provenFailFirst: false, method: 'violation-fixture' };
|
||||
const cleanOut = runNodeTestWithSubject(check, cwd, clean, timeoutMs);
|
||||
if (!isNonVacuousNodeTestPass(cleanOut, check.target)) return { provenFailFirst: false, method: 'violation-fixture' };
|
||||
}
|
||||
return { provenFailFirst: isNonVacuousNodeTestRed(out, check.target), method: 'violation-fixture' };
|
||||
return { provenFailFirst: true, method: 'violation-fixture' };
|
||||
}
|
||||
// Unknown kind — defensive; the LOCATE guard already rejects it.
|
||||
return { provenFailFirst: false };
|
||||
|
||||
@@ -181,10 +181,25 @@ function routeRoadmapCommand({ roadmap, args, cwd, raw, error }: RouteRoadmapCom
|
||||
},
|
||||
'upgrade': () => {
|
||||
const dryRun = !args.includes('--apply');
|
||||
const convention = args.find((_a, i) => args[i - 1] === '--convention') || 'milestone-prefixed';
|
||||
// Parse `--convention <value>` and `--convention=<value>`. When the flag is
|
||||
// absent entirely, default to the only supported convention; when present
|
||||
// with a missing/unsupported value, fall through to the rejection below
|
||||
// (fail-closed — never silently run a migration the user did not request).
|
||||
let convention = 'milestone-prefixed';
|
||||
const conventionFlagIdx = args.findIndex(
|
||||
(a) => a === '--convention' || a.startsWith('--convention='),
|
||||
);
|
||||
if (conventionFlagIdx !== -1) {
|
||||
const token = args[conventionFlagIdx];
|
||||
convention = token.includes('=')
|
||||
? token.slice(token.indexOf('=') + 1)
|
||||
: (args[conventionFlagIdx + 1] ?? '');
|
||||
}
|
||||
if (convention !== 'milestone-prefixed') {
|
||||
process.stderr.write('Only --convention milestone-prefixed is supported\n');
|
||||
process.exit(1);
|
||||
// No-throw hub contract (ADR-0012): a hub-dispatched handler must not call
|
||||
// process.exit. Throw instead — the hub converts this to HandlerFailure and
|
||||
// the adapter routes it through the injected error() boundary.
|
||||
throw new Error('Only --convention milestone-prefixed is supported');
|
||||
}
|
||||
const plan = roadmapUpgrade.computeMigrationPlan(cwd);
|
||||
roadmapUpgrade.applyMigration(cwd, plan, { dryRun });
|
||||
|
||||
@@ -492,14 +492,6 @@ function applyMigration(cwd: string, plan: MigrationPlan, options: { dryRun?: bo
|
||||
throw new Error('Working tree is dirty. Commit or stash changes before migrating.');
|
||||
}
|
||||
|
||||
// Capture HEAD sha for rollback
|
||||
let headSha: string;
|
||||
try {
|
||||
headSha = execSync('git rev-parse HEAD', { cwd, encoding: 'utf8', windowsHide: true }).trim();
|
||||
} catch (err) {
|
||||
throw new Error(`git rev-parse HEAD failed: ${(err as Error).message}`);
|
||||
}
|
||||
|
||||
const pDir = planningDir(cwd);
|
||||
const phasesDir = path.join(pDir, 'phases');
|
||||
const roadmapPath = path.join(pDir, 'ROADMAP.md');
|
||||
@@ -508,6 +500,23 @@ function applyMigration(cwd: string, plan: MigrationPlan, options: { dryRun?: bo
|
||||
const renamedDirs: string[] = [];
|
||||
const editedFiles: string[] = [];
|
||||
|
||||
// Surgical, git-independent rollback state (#1542). A `git reset --hard` +
|
||||
// `git clean` rollback restores NOTHING for a gitignored `.planning/`
|
||||
// (commit_docs:false — the default) and is a whole-repo operation besides.
|
||||
// Instead, record the exact renames performed and snapshot each file before
|
||||
// rewriting it, then undo precisely those on failure — correct whether
|
||||
// `.planning/` is git-tracked or ignored.
|
||||
const performedRenames: Array<{ oldPath: string; newPath: string }> = [];
|
||||
const fileBackups = new Map<string, { existed: boolean; content: string }>();
|
||||
const snapshotFile = (filePath: string): void => {
|
||||
if (fileBackups.has(filePath)) return;
|
||||
try {
|
||||
fileBackups.set(filePath, { existed: true, content: fs.readFileSync(filePath, 'utf8') });
|
||||
} catch {
|
||||
fileBackups.set(filePath, { existed: false, content: '' });
|
||||
}
|
||||
};
|
||||
|
||||
try {
|
||||
// 1. Rename phase directories
|
||||
for (const phaseEntry of plan.phases) {
|
||||
@@ -515,6 +524,7 @@ function applyMigration(cwd: string, plan: MigrationPlan, options: { dryRun?: bo
|
||||
const newPath = path.join(phasesDir, phaseEntry.newDir);
|
||||
if (fs.existsSync(oldPath)) {
|
||||
fs.renameSync(oldPath, newPath);
|
||||
performedRenames.push({ oldPath, newPath });
|
||||
renamedDirs.push(`${phaseEntry.oldDir} → ${phaseEntry.newDir}`);
|
||||
}
|
||||
}
|
||||
@@ -532,6 +542,7 @@ function applyMigration(cwd: string, plan: MigrationPlan, options: { dryRun?: bo
|
||||
}
|
||||
}
|
||||
|
||||
snapshotFile(roadmapPath);
|
||||
fs.writeFileSync(roadmapPath, lines.join('\n'), 'utf8');
|
||||
editedFiles.push('ROADMAP.md');
|
||||
}
|
||||
@@ -561,6 +572,7 @@ function applyMigration(cwd: string, plan: MigrationPlan, options: { dryRun?: bo
|
||||
}
|
||||
|
||||
if (changed) {
|
||||
snapshotFile(filePath);
|
||||
fs.writeFileSync(filePath, content, 'utf8');
|
||||
editedFiles.push(fileName);
|
||||
}
|
||||
@@ -573,18 +585,28 @@ function applyMigration(cwd: string, plan: MigrationPlan, options: { dryRun?: bo
|
||||
} catch { /* config may not exist yet */ }
|
||||
|
||||
configData['phase_id_convention'] = 'milestone-prefixed';
|
||||
snapshotFile(configPath);
|
||||
fs.writeFileSync(configPath, JSON.stringify(configData, null, 2) + '\n', 'utf8');
|
||||
editedFiles.push('config.json');
|
||||
|
||||
} catch (err) {
|
||||
// Rollback via git reset --hard + git clean
|
||||
try {
|
||||
execSync(`git reset --hard ${headSha}`, { cwd, stdio: 'pipe', windowsHide: true });
|
||||
execSync('git clean -fd .planning/phases/', { cwd, stdio: 'pipe', windowsHide: true });
|
||||
} catch {
|
||||
// Swallow rollback errors — surface original error
|
||||
// Surgical rollback: reverse the renames (newest first) and restore every
|
||||
// file we snapshotted (deleting files that did not previously exist). This
|
||||
// actually restores `.planning/` regardless of git tracking — so the
|
||||
// "rolled back" claim is truthful — and never touches anything else.
|
||||
for (let i = performedRenames.length - 1; i >= 0; i--) {
|
||||
const { oldPath, newPath } = performedRenames[i];
|
||||
try {
|
||||
if (fs.existsSync(newPath)) fs.renameSync(newPath, oldPath);
|
||||
} catch { /* best-effort */ }
|
||||
}
|
||||
throw new Error(`Migration failed (rolled back to ${headSha}): ${(err as Error).message}`);
|
||||
for (const [filePath, backup] of fileBackups) {
|
||||
try {
|
||||
if (backup.existed) fs.writeFileSync(filePath, backup.content, 'utf8');
|
||||
else if (fs.existsSync(filePath)) fs.unlinkSync(filePath);
|
||||
} catch { /* best-effort */ }
|
||||
}
|
||||
throw new Error(`Migration failed and rolled back: ${(err as Error).message}`);
|
||||
}
|
||||
|
||||
return { applied: true, renamedDirs, editedFiles };
|
||||
|
||||
@@ -426,8 +426,11 @@ function cmdRoadmapAnalyze(cwd: string, raw: boolean): void {
|
||||
const totalSummaries = phases.reduce((sum, p) => sum + p.summary_count, 0);
|
||||
const completedPhases = phases.filter(p => p.disk_status === 'complete').length;
|
||||
|
||||
// Detect phases in summary list without detail sections (malformed ROADMAP)
|
||||
const checklistPattern = /-\s*\[[ x]\]\s*\*\*Phase\s+(\d+[A-Z]?(?:\.\d+)*)/gi;
|
||||
// Detect phases in summary list without detail sections (malformed ROADMAP).
|
||||
// The char class must allow `-` (not just `.`) so dash-separated milestone-prefixed
|
||||
// IDs (e.g. `1-01`) match the detail-heading scanner above; otherwise they truncate
|
||||
// at the dash (`1-01` -> `1`) and every such phase reports a phantom missing detail.
|
||||
const checklistPattern = /-\s*\[[ x]\]\s*\*\*Phase\s+(\d+[A-Z]?(?:[.-]\d+)*)/gi;
|
||||
const checklistPhases = new Set<string>();
|
||||
let checklistMatch: RegExpExecArray | null;
|
||||
while ((checklistMatch = checklistPattern.exec(content)) !== null) {
|
||||
|
||||
@@ -21,10 +21,46 @@ import os from 'node:os';
|
||||
import fs from 'node:fs';
|
||||
import commandRoster = require('./command-roster.cjs');
|
||||
const { readGsdCommandNames, transformContentToHyphen } = commandRoster;
|
||||
const pkg = require('../../../package.json');
|
||||
import runtimeNamePolicy = require('./runtime-name-policy.cjs');
|
||||
const { getDirName } = runtimeNamePolicy;
|
||||
|
||||
// #1383: resolve GSD's version WITHOUT a top-level
|
||||
// `require('../../../package.json')`. That require ran at module load on every
|
||||
// gsd-tools invocation (this module sits in the gsd-tools loader chain) and
|
||||
// threw `Cannot find module '../../../package.json'` on runtimes whose root has
|
||||
// no package.json — notably Codex, where the installer omits the synthetic root
|
||||
// package.json — taking the entire CLI down before it did anything. And even
|
||||
// where it resolved (Claude's synthetic `{"type":"commonjs"}`), there is no
|
||||
// `version` field, so the single consumer below already emitted
|
||||
// `version: undefined`. Resolve lazily and defensively instead:
|
||||
// 1. Installed trees carry <root>/gsd-core/VERSION (written by the installer);
|
||||
// this module lives at <root>/gsd-core/bin/lib, so VERSION is two dirs up.
|
||||
// 2. The source / npm-package tree has no gsd-core/VERSION but carries a real
|
||||
// package.json three dirs up — read it lazily, never at module-load time.
|
||||
// A failed/invalid lookup degrades to '' (the caller omits the field) rather
|
||||
// than crashing or emitting `version: undefined`. Both sources are validated
|
||||
// against the same semver shape the repo's other VERSION reader enforces
|
||||
// (src/update-context.cts) so a garbled VERSION file is never emitted verbatim.
|
||||
// Exported for the #1383 regression.
|
||||
const SEMVER_PREFIX = /^\d+\.\d+\.\d+/; // mirrors src/update-context.cts SEMVER_PREFIX
|
||||
function resolveVersionFrom(libDir: string): string {
|
||||
try {
|
||||
const v = fs.readFileSync(path.join(libDir, '..', '..', 'VERSION'), 'utf8').trim();
|
||||
if (SEMVER_PREFIX.test(v)) return v;
|
||||
} catch { /* not an installed tree (no gsd-core/VERSION) */ }
|
||||
try {
|
||||
const pkg = require(path.join(libDir, '..', '..', '..', 'package.json'));
|
||||
if (pkg && typeof pkg.version === 'string' && SEMVER_PREFIX.test(pkg.version)) return pkg.version;
|
||||
} catch { /* runtime root has no package.json (e.g. Codex) */ }
|
||||
return '';
|
||||
}
|
||||
|
||||
let cachedVersion: string | undefined;
|
||||
function gsdVersion(): string {
|
||||
if (cachedVersion === undefined) cachedVersion = resolveVersionFrom(__dirname);
|
||||
return cachedVersion;
|
||||
}
|
||||
|
||||
|
||||
const colorNameToHex = {
|
||||
cyan: '#00FFFF',
|
||||
@@ -393,7 +429,10 @@ function convertClaudeCommandToClaudeSkill(content, skillName, runtime = null, c
|
||||
// Hermes' SKILL.md spec lists `version` as a required frontmatter field.
|
||||
// Track GSD's package version so Hermes' skill_view() reports a stable
|
||||
// identifier per install.
|
||||
if (runtime === 'hermes') fm += `version: ${yamlQuote(pkg.version)}\n`;
|
||||
if (runtime === 'hermes') {
|
||||
const version = gsdVersion();
|
||||
if (version) fm += `version: ${yamlQuote(version)}\n`;
|
||||
}
|
||||
// #778 (b) — Qwen-only numeric priority for /skills ordering. Scoped to qwen
|
||||
// so Claude/Hermes skill frontmatter is unchanged (they ignore the field, but
|
||||
// we keep their output byte-stable). skillName is the `gsd-<stem>` dir name.
|
||||
@@ -1816,11 +1855,17 @@ function convertGeminiToolName(claudeTool) {
|
||||
// Task/Agent: exclude — agents are auto-registered as callable tools.
|
||||
// AskUserQuestion: exclude — Gemini CLI does not expose an ask_user tool;
|
||||
// emitting it causes frontmatter validation errors (#3362).
|
||||
// Skill/SlashCommand: exclude — Gemini CLI has no 'skill' built-in tool;
|
||||
// the lowercase fallback would emit an invalid 'skill'/'slashcommand' name
|
||||
// that fails frontmatter validation (tools.N: Invalid tool name) and aborts
|
||||
// the entire agent load (#1394).
|
||||
if (
|
||||
claudeTool === 'Task' ||
|
||||
claudeTool === 'Agent' ||
|
||||
claudeTool === 'AskUserQuestion' ||
|
||||
claudeTool === 'ask_user'
|
||||
claudeTool === 'ask_user' ||
|
||||
claudeTool === 'Skill' ||
|
||||
claudeTool === 'SlashCommand'
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
@@ -2444,6 +2489,11 @@ function rewriteStagedSkillBodies(stagedDir, opts) {
|
||||
* attribution from opts, then delegates to applyRuntimeContentRewritesForCommandsInPlace
|
||||
* (single copy+rewrite owner).
|
||||
*
|
||||
* @internal — symmetric companion to rewriteStagedSkillBodies; retained as the deep-seam
|
||||
* API for command bodies. No production caller today (install rewrites commands via
|
||||
* copyWithPathReplacement → applyRuntimeContentRewritesForCommandsInPlace). Kept for
|
||||
* API symmetry + test coverage.
|
||||
*
|
||||
* @returns {string} path to the temp dir (caller is responsible for cleanup)
|
||||
*/
|
||||
function rewriteStagedCommandBodies(stagedDir, opts) {
|
||||
@@ -2536,6 +2586,9 @@ export = {
|
||||
convertClaudeCommandToKiloSkill,
|
||||
readGsdCommandNames,
|
||||
transformContentToHyphen,
|
||||
// #1383: version resolver (exported for regression test of the Codex
|
||||
// missing-package.json crash + the VERSION-file source of truth).
|
||||
resolveVersionFrom,
|
||||
// #1182: agent converters + tool-name table dependency closure
|
||||
claudeToCopilotTools,
|
||||
convertCopilotToolName,
|
||||
|
||||
165
src/runtime-artifact-install-plan.cts
Normal file
165
src/runtime-artifact-install-plan.cts
Normal file
@@ -0,0 +1,165 @@
|
||||
'use strict';
|
||||
|
||||
/**
|
||||
* Runtime Artifact Install Plan Module.
|
||||
*
|
||||
* Turns a pre-resolved runtime artifact layout into staged copy inputs. The
|
||||
* installer adapter still owns pruning, copying, migrations, output, and final
|
||||
* cleanup execution.
|
||||
*/
|
||||
|
||||
// In .cts (CommonJS output) files, `require` is available as a global.
|
||||
const _require: NodeRequire = require;
|
||||
const path = _require('node:path') as typeof import('node:path');
|
||||
|
||||
type ArtifactKindName = 'commands' | 'agents' | 'skills' | 'kimi-agents';
|
||||
type InstallScope = 'local' | 'global';
|
||||
|
||||
interface ResolvedProfile {
|
||||
name?: string;
|
||||
skills?: Set<string> | '*';
|
||||
agents?: Set<string>;
|
||||
}
|
||||
|
||||
interface ArtifactKind {
|
||||
kind: ArtifactKindName;
|
||||
destSubpath: string;
|
||||
prefix?: string;
|
||||
stage: (resolvedProfile: ResolvedProfile) => string;
|
||||
}
|
||||
|
||||
interface Layout {
|
||||
runtime: string;
|
||||
configDir: string;
|
||||
scope?: InstallScope;
|
||||
kinds: ArtifactKind[];
|
||||
}
|
||||
|
||||
interface RewriteOpts {
|
||||
runtime: string;
|
||||
configDir: string;
|
||||
scope: InstallScope;
|
||||
homedir?: () => string;
|
||||
platform?: NodeJS.Platform;
|
||||
resolveAttribution?: (runtime: string) => string | null | undefined;
|
||||
}
|
||||
|
||||
interface Dependencies {
|
||||
rewriteStagedSkillBodies?: (stagedDir: string, opts: RewriteOpts) => string | void;
|
||||
rewriteStagedCommandBodies?: (stagedDir: string, opts: RewriteOpts) => string | void;
|
||||
}
|
||||
|
||||
interface RuntimeArtifactConversionExports {
|
||||
rewriteStagedSkillBodies: (stagedDir: string, opts: RewriteOpts) => string | void;
|
||||
rewriteStagedCommandBodies: (stagedDir: string, opts: RewriteOpts) => string | void;
|
||||
}
|
||||
|
||||
interface PlanItem {
|
||||
kind: ArtifactKindName;
|
||||
sourceDir: string;
|
||||
destDir: string;
|
||||
}
|
||||
|
||||
interface InstallPlan {
|
||||
items: PlanItem[];
|
||||
cleanupDirs: string[];
|
||||
}
|
||||
|
||||
interface UninstallPlanItem {
|
||||
kind: ArtifactKindName;
|
||||
destDir: string;
|
||||
}
|
||||
|
||||
interface UninstallPlan {
|
||||
items: UninstallPlanItem[];
|
||||
}
|
||||
|
||||
type InstallPlanResult =
|
||||
| { ok: true; plan: InstallPlan }
|
||||
| { ok: false; kind: 'stage_failed' | 'rewrite_failed'; message: string; cleanupDirs: string[]; failedKind?: ArtifactKindName };
|
||||
|
||||
interface CreateRuntimeArtifactInstallPlanArgs {
|
||||
layout: Layout;
|
||||
resolvedProfile: ResolvedProfile;
|
||||
homedir?: () => string;
|
||||
platform?: NodeJS.Platform;
|
||||
resolveAttribution?: (runtime: string) => string | null | undefined;
|
||||
deps?: Dependencies;
|
||||
}
|
||||
|
||||
function errorMessage(err: unknown): string {
|
||||
if (err instanceof Error) return err.message;
|
||||
return String(err);
|
||||
}
|
||||
|
||||
function addCleanupDir(cleanupDirs: string[], stagedDir: string, rewrittenDir: string | void): string {
|
||||
const sourceDir = rewrittenDir ?? stagedDir;
|
||||
if (sourceDir !== stagedDir) cleanupDirs.push(sourceDir);
|
||||
return sourceDir;
|
||||
}
|
||||
|
||||
function createRuntimeArtifactInstallPlan(args: CreateRuntimeArtifactInstallPlanArgs): InstallPlanResult {
|
||||
const {
|
||||
layout,
|
||||
resolvedProfile,
|
||||
homedir,
|
||||
platform,
|
||||
resolveAttribution,
|
||||
deps = {},
|
||||
} = args;
|
||||
const conversionExports = _require('./runtime-artifact-conversion.cjs') as RuntimeArtifactConversionExports;
|
||||
const rewriteStagedSkillBodies = deps.rewriteStagedSkillBodies ?? conversionExports.rewriteStagedSkillBodies;
|
||||
const rewriteStagedCommandBodies = deps.rewriteStagedCommandBodies ?? conversionExports.rewriteStagedCommandBodies;
|
||||
const cleanupDirs: string[] = [];
|
||||
const items: PlanItem[] = [];
|
||||
const scope = layout.scope ?? 'global';
|
||||
const rewriteOpts: RewriteOpts = {
|
||||
runtime: layout.runtime,
|
||||
configDir: layout.configDir,
|
||||
scope,
|
||||
homedir,
|
||||
platform,
|
||||
resolveAttribution,
|
||||
};
|
||||
|
||||
for (const kind of layout.kinds) {
|
||||
let stagedDir: string;
|
||||
try {
|
||||
stagedDir = kind.stage(resolvedProfile);
|
||||
} catch (err) {
|
||||
return { ok: false, kind: 'stage_failed', message: errorMessage(err), cleanupDirs, failedKind: kind.kind };
|
||||
}
|
||||
|
||||
let sourceDir = stagedDir;
|
||||
try {
|
||||
if (kind.kind === 'commands') {
|
||||
const rewrittenDir = rewriteStagedCommandBodies(stagedDir, rewriteOpts);
|
||||
sourceDir = addCleanupDir(cleanupDirs, stagedDir, rewrittenDir);
|
||||
} else if (kind.kind === 'skills' || kind.kind === 'kimi-agents') {
|
||||
const rewrittenDir = rewriteStagedSkillBodies(stagedDir, rewriteOpts);
|
||||
sourceDir = addCleanupDir(cleanupDirs, stagedDir, rewrittenDir);
|
||||
}
|
||||
} catch (err) {
|
||||
return { ok: false, kind: 'rewrite_failed', message: errorMessage(err), cleanupDirs, failedKind: kind.kind };
|
||||
}
|
||||
|
||||
items.push({
|
||||
kind: kind.kind,
|
||||
sourceDir,
|
||||
destDir: path.join(layout.configDir, kind.destSubpath),
|
||||
});
|
||||
}
|
||||
|
||||
return { ok: true, plan: { items, cleanupDirs } };
|
||||
}
|
||||
|
||||
function createRuntimeArtifactUninstallPlan(layout: Layout): UninstallPlan {
|
||||
return {
|
||||
items: layout.kinds.map((kind) => ({
|
||||
kind: kind.kind,
|
||||
destDir: path.join(layout.configDir, kind.destSubpath),
|
||||
})),
|
||||
};
|
||||
}
|
||||
|
||||
export = { createRuntimeArtifactInstallPlan, createRuntimeArtifactUninstallPlan };
|
||||
@@ -89,6 +89,48 @@ export function resolveRuntimeNameFromCandidates(...candidates: unknown[]): stri
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Map a runtime id to its project instruction file path (relative to project
|
||||
* root). Bug #1529: this is the SINGLE source of truth shared by both
|
||||
* consumption surfaces —
|
||||
* (A) the Node surface: profile-output.cjs (generate-claude-md handler)
|
||||
* (B) the bash surface: `gsd-tools query project-instruction-file --runtime <r>`,
|
||||
* consumed by gsd-core/workflows/new-project.md to set $INSTRUCTION_FILE
|
||||
*
|
||||
* Mapping table (per the #1529 issue contract):
|
||||
*
|
||||
* claude → .claude/CLAUDE.md
|
||||
* codex, opencode, kilo, kimi → AGENTS.md
|
||||
* copilot → .github/copilot-instructions.md
|
||||
* antigravity, gemini → GEMINI.md
|
||||
* unknown / future runtimes → AGENTS.md (safe cross-agent default)
|
||||
*
|
||||
* Source-of-truth references for each runtime's read path:
|
||||
* - copilot: GitHub Docs — repository-wide custom instructions are read ONLY
|
||||
* from `.github/copilot-instructions.md`; a root `copilot-instructions.md`
|
||||
* is not a read path. `AGENTS.md` is also read (agent instructions).
|
||||
* https://docs.github.com/en/copilot/how-tos/configure-custom-instructions/add-repository-instructions
|
||||
* (Installer parity: runtime-config-adapter-registry.cts installSurface
|
||||
* 'copilot-instructions' writes the same `.github/copilot-instructions.md`.)
|
||||
* - codex/opencode/kilo/kimi: AGENTS.md is the documented cross-agent
|
||||
* instruction file (agentsmd/agents.md convention).
|
||||
* - antigravity/gemini: GEMINI.md is Gemini CLI's contextFileName.
|
||||
*
|
||||
* Aliases are normalized via `canonicalizeRuntimeName` first, so inputs like
|
||||
* `codex-cli` resolve to `codex` → `AGENTS.md`. Replaces the prior codex-only
|
||||
* override in profile-output.cjs (#3163) which left AGENTS-native runtimes
|
||||
* (opencode/kilo/kimi) incorrectly emitting `.claude/CLAUDE.md`. Pure: no I/O.
|
||||
*/
|
||||
export function getProjectInstructionFile(runtime: unknown): string {
|
||||
const canonical = canonicalizeRuntimeName(runtime);
|
||||
if (canonical === 'claude') return '.claude/CLAUDE.md';
|
||||
if (canonical === 'copilot') return '.github/copilot-instructions.md';
|
||||
if (canonical === 'antigravity' || canonical === 'gemini') return 'GEMINI.md';
|
||||
// codex, opencode, kilo, kimi, AND unknown/future runtimes all default to
|
||||
// root AGENTS.md (the safe cross-agent instruction file).
|
||||
return 'AGENTS.md';
|
||||
}
|
||||
|
||||
/**
|
||||
* Map a canonical runtime id to its on-disk local config directory name
|
||||
* (e.g. `cursor` -> `.cursor`, `windsurf` -> `.devin`). Unknown/empty inputs
|
||||
|
||||
@@ -550,17 +550,71 @@ export function normalizeContent(filePath: string, content: string, opts: { enco
|
||||
return { content: normalized, encoding };
|
||||
}
|
||||
|
||||
// Rename errnos that are transient on Windows: a concurrent reader (or an AV
|
||||
// scanner / indexer) holding the target open makes renameSync fail briefly.
|
||||
// Same idiom as capability-ledger.cts / capability-consent.cts.
|
||||
const RENAME_RETRY_ERRNOS = new Set(['EPERM', 'EBUSY', 'EACCES']);
|
||||
const RENAME_MAX_ATTEMPTS = 3;
|
||||
const RENAME_RETRY_BACKOFF_MS = 50;
|
||||
|
||||
/** Synchronous best-effort backoff sleep (Atomics.wait — same idiom as io.cts). */
|
||||
let _renameSleepBuf: Int32Array | null = null;
|
||||
function renameBackoff(): void {
|
||||
if (_renameSleepBuf === null) _renameSleepBuf = new Int32Array(new SharedArrayBuffer(4));
|
||||
Atomics.wait(_renameSleepBuf, 0, 0, RENAME_RETRY_BACKOFF_MS);
|
||||
}
|
||||
|
||||
/**
|
||||
* Atomic publish with bounded retry on transient Windows lock errnos.
|
||||
* Returns null on success, or the final error if every attempt failed.
|
||||
*/
|
||||
function atomicRenameWithRetry(tmpPath: string, filePath: string): NodeJS.ErrnoException | null {
|
||||
let renameErr: NodeJS.ErrnoException | null = null;
|
||||
for (let attempt = 1; attempt <= RENAME_MAX_ATTEMPTS; attempt++) {
|
||||
try {
|
||||
fs.renameSync(tmpPath, filePath);
|
||||
return null;
|
||||
} catch (err) {
|
||||
renameErr = err as NodeJS.ErrnoException;
|
||||
if (attempt < RENAME_MAX_ATTEMPTS && RENAME_RETRY_ERRNOS.has(renameErr.code ?? '')) {
|
||||
renameBackoff();
|
||||
continue;
|
||||
}
|
||||
break;
|
||||
}
|
||||
}
|
||||
return renameErr;
|
||||
}
|
||||
|
||||
export function platformWriteSync(filePath: string, content: string, opts: { encoding?: BufferEncoding } = {}): void {
|
||||
const { content: normalized, encoding } = normalizeContent(filePath, content, opts);
|
||||
fs.mkdirSync(path.dirname(filePath), { recursive: true });
|
||||
const tmpPath = filePath + '.tmp.' + process.pid;
|
||||
|
||||
// Step 1: write the sibling tmp file. If THIS fails, nothing was published, so a
|
||||
// direct fallback write cannot truncate a concurrent reader of an existing file.
|
||||
try {
|
||||
fs.writeFileSync(tmpPath, normalized, encoding);
|
||||
fs.renameSync(tmpPath, filePath);
|
||||
} catch {
|
||||
try { fs.unlinkSync(tmpPath); } catch { /* already gone */ }
|
||||
fs.writeFileSync(filePath, normalized, encoding);
|
||||
return;
|
||||
}
|
||||
|
||||
// Step 2: atomic publish, retrying transient Windows locks.
|
||||
const renameErr = atomicRenameWithRetry(tmpPath, filePath);
|
||||
if (renameErr === null) return;
|
||||
|
||||
try { fs.unlinkSync(tmpPath); } catch { /* already gone */ }
|
||||
if (RENAME_RETRY_ERRNOS.has(renameErr.code ?? '')) {
|
||||
// A live reader still holds the target open after every retry. A non-atomic
|
||||
// direct write here would truncate that reader (the exact corruption this seam
|
||||
// exists to prevent), so surface the error instead of falling back.
|
||||
throw renameErr;
|
||||
}
|
||||
// Atomic publish is genuinely impossible here (e.g. EXDEV cross-device move):
|
||||
// fall back to a direct write to preserve write availability.
|
||||
fs.writeFileSync(filePath, normalized, encoding);
|
||||
}
|
||||
|
||||
export function platformReadSync(filePath: string, opts: { encoding?: BufferEncoding; required?: boolean } = {}): string | null {
|
||||
|
||||
282
src/state.cts
282
src/state.cts
@@ -152,6 +152,116 @@ process.on('exit', () => {
|
||||
}
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Lock liveness probe (test seam) — audit M1
|
||||
//
|
||||
// mtime is a LEAKY proxy for "the holder is still alive": a live-but-slow writer
|
||||
// whose critical section runs past staleThresholdMs ages out and a waiter would
|
||||
// steal its lock → two writers in STATE.md's read-modify-write window → lost
|
||||
// update / corruption (the recurring #500/#905/#1230 family). The real signal —
|
||||
// process.kill(pid, 0) — is already used by capability-lock.cts. We backport it
|
||||
// here. The indirection lets unit tests inject a deterministic isPidAlive without
|
||||
// real pids (mirrors capability-lock's _lockProbes / _setLockProbes seam).
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/** Is `pid` a live process? process.kill(pid, 0) succeeds for a live (signalable) process. */
|
||||
function _realIsPidAlive(pid: number): boolean {
|
||||
try {
|
||||
process.kill(pid, 0);
|
||||
return true; // signalable → alive
|
||||
} catch (err) {
|
||||
// EPERM = process exists but we cannot signal it (still ALIVE). ESRCH = gone.
|
||||
return (err as NodeJS.ErrnoException).code === 'EPERM';
|
||||
}
|
||||
}
|
||||
|
||||
const _stateLockProbes: { isPidAlive: (pid: number) => boolean } = { isPidAlive: _realIsPidAlive };
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// State-lock test hooks (test seam) — audit M8 / M9
|
||||
//
|
||||
// Both M8 (scan-before-lock TOCTOU in writeStateMd) and M9 (orphan empty lock +
|
||||
// fd leak on a recoverable writeSync/closeSync error in acquireStateLock) are
|
||||
// concurrency / resource-safety issues a single-threaded test cannot otherwise
|
||||
// observe. These purpose-built hooks make the failure windows deterministic
|
||||
// (mirrors the M1 _setLockProbes seam above):
|
||||
//
|
||||
// afterAcquire(lockPath) — fired inside writeStateMd immediately AFTER the lock
|
||||
// is acquired. A test can mutate the disk here (simulate a concurrent writer
|
||||
// landing in the scan→lock window) to prove the disk scan runs INSIDE the lock.
|
||||
// simulateWriteError — a ONE-SHOT errno string. When set, the next writeSync
|
||||
// inside acquireStateLock throws it (and the hook self-clears), forcing the
|
||||
// openSync-succeeds-then-write-fails cleanup path without an OS-level fault.
|
||||
// onLoopIteration(ctx) — fired at the TOP of each acquireStateLock retry
|
||||
// iteration so a test can snapshot whether an orphan lock is stranded.
|
||||
// beforeSteal(ctx) — fired AFTER the steal decision but BEFORE the identity
|
||||
// re-confirm + atomic rename-steal. A test can recreate a fresh lock here to
|
||||
// simulate a racer winning the steal in the decision→steal gap, proving the
|
||||
// identity re-confirm aborts a double-steal (PR #1532 review window b).
|
||||
//
|
||||
// All hooks default to no-ops; real callers are byte-for-behaviour unchanged.
|
||||
// ---------------------------------------------------------------------------
|
||||
interface StateLockTestHooks {
|
||||
afterAcquire?: (lockPath: string) => void;
|
||||
simulateWriteError?: string | null;
|
||||
onLoopIteration?: (ctx: { iteration: number }) => void;
|
||||
beforeSteal?: (ctx: { lockPath: string }) => void;
|
||||
}
|
||||
const _stateLockTestHooks: StateLockTestHooks = {};
|
||||
|
||||
/**
|
||||
* Consume the one-shot simulateWriteError errno, if set. Returns an Error with the
|
||||
* configured `.code` and self-clears so only the NEXT writeSync throws (the retry
|
||||
* then succeeds). Returns null when no injection is pending.
|
||||
*/
|
||||
function _consumeSimulatedWriteError(): NodeJS.ErrnoException | null {
|
||||
const code = _stateLockTestHooks.simulateWriteError;
|
||||
if (!code) return null;
|
||||
_stateLockTestHooks.simulateWriteError = null; // one-shot
|
||||
const e = new Error('simulated writeSync failure (' + code + ')') as NodeJS.ErrnoException;
|
||||
e.code = code;
|
||||
return e;
|
||||
}
|
||||
|
||||
function _stateLockIsPidAlive(pid: number): boolean {
|
||||
return _stateLockProbes.isPidAlive(pid);
|
||||
}
|
||||
|
||||
/**
|
||||
* Is the holder recorded in the lock body VERIFIED-LIVE? The STATE.md lock body is
|
||||
* a bare pid (written at acquire time). Returns true ONLY when the body parses to a
|
||||
* positive integer pid AND that pid signals alive. A garbage / non-numeric / legacy
|
||||
* body (or a dead pid) is NOT verified-live, so the lock stays stealable — corrupt
|
||||
* locks never block forever, and a live holder is never stolen.
|
||||
*/
|
||||
function _stateHolderVerifiedLive(lockPath: string): boolean {
|
||||
const pid = _stateLockBodyPid(lockPath);
|
||||
return pid !== null && _stateLockIsPidAlive(pid);
|
||||
}
|
||||
|
||||
/**
|
||||
* Parse the lock body to its recorded pid, or null when the body is empty / non-numeric
|
||||
* / unreadable (legacy or mid-creation). Distinguishing a COMPLETE dead-pid body (steal
|
||||
* promptly) from an EMPTY/unparseable one (the create→write window — do not steal while
|
||||
* fresh) is what `_stateHolderVerifiedLive` alone cannot express, so the steal decision
|
||||
* in acquireStateLock reads the pid directly (PR #1532 review, window a).
|
||||
*/
|
||||
function _stateLockBodyPid(lockPath: string): number | null {
|
||||
let body: string;
|
||||
try {
|
||||
body = fs.readFileSync(lockPath, 'utf-8');
|
||||
} catch {
|
||||
return null; // unreadable body → cannot verify
|
||||
}
|
||||
const trimmed = body.trim();
|
||||
const pid = parseInt(trimmed, 10);
|
||||
if (!Number.isInteger(pid) || pid <= 0 || String(pid) !== trimmed) return null;
|
||||
return pid;
|
||||
}
|
||||
|
||||
// Monotonic sequence for unique stale-steal rename targets (no crypto dependency).
|
||||
let _stateStealSeq = 0;
|
||||
|
||||
// Hoisted to module scope — compiled once, not per call (#320). Stateless (/i, used with .match).
|
||||
const byPhaseTablePattern = /(\|\s*Phase\s*\|\s*Plans\s*\|\s*Total\s*\|\s*Avg\/Plan\s*\|[ \t]*\n\|(?:[- :\t]+\|)+[ \t]*\n)((?:[ \t]*\|[^\n]*\n)*)(?=\n|$)/i;
|
||||
|
||||
@@ -1587,8 +1697,23 @@ function acquireStateLock(statePath: string, clock?: StateLockClock): string {
|
||||
if (clock === undefined) clock = realClock;
|
||||
const lockPath = statePath + '.lock';
|
||||
const retryDelay = 200; // ms
|
||||
const staleThresholdMs = 10000;
|
||||
const maxWaitMs = 30000;
|
||||
// Deadman ceiling (audit M1) — set ABOVE maxWaitMs so a holder that reads as
|
||||
// VERIFIED-LIVE is NEVER stolen within the wait budget; only a crashed (dead
|
||||
// pid) or unparseable-body lock is stolen, and a pid-reuse holder (reads alive
|
||||
// but is unrelated) is recovered once age crosses this absolute ceiling rather
|
||||
// than blocking forever. The prior mtime-only `staleThresholdMs = 10000` gate
|
||||
// was BELOW maxWaitMs, so a live-but-slow holder >10 s was robbed mid-write.
|
||||
const deadmanCeilingMs = 60000;
|
||||
// Fresh-create floor (PR #1532 review, window a) — a lock with an EMPTY/unparseable
|
||||
// body is either mid-creation (O_EXCL create done, pid not yet written by the holder)
|
||||
// or a genuine orphan. While such a body is younger than this floor it is treated as
|
||||
// mid-creation and is NEVER stolen — stealing it at age ≈ 0 robs a holder still
|
||||
// writing its pid (the lost-update window capability-lock.cts's `age <= LOCK_STALE_MS`
|
||||
// floor closes). The create→write gap is sub-millisecond; this floor is orders of
|
||||
// magnitude larger yet well under maxWaitMs so a real orphan still clears within budget.
|
||||
// A COMPLETE dead-pid body is NOT subject to this floor — it is stolen promptly.
|
||||
const freshCreateFloorMs = 1000;
|
||||
const startedAt = clock.now();
|
||||
|
||||
// Shared helper: check the time budget then back off with jitter before the
|
||||
@@ -1607,11 +1732,33 @@ function acquireStateLock(statePath: string, clock?: StateLockClock): string {
|
||||
clock.sleep(retryDelay + jitter);
|
||||
};
|
||||
|
||||
let _loopIteration = 0;
|
||||
while (true) {
|
||||
if (_stateLockTestHooks.onLoopIteration) _stateLockTestHooks.onLoopIteration({ iteration: _loopIteration++ });
|
||||
try {
|
||||
const fd = fs.openSync(lockPath, fs.constants.O_CREAT | fs.constants.O_EXCL | fs.constants.O_WRONLY);
|
||||
fs.writeSync(fd, String(process.pid));
|
||||
fs.closeSync(fd);
|
||||
// Audit M9 (resource-safety): once the exclusive create SUCCEEDS, a
|
||||
// writeSync/closeSync failure must NOT leak the fd or strand the just-created
|
||||
// (now empty) lock — an orphan body self-blocks every later acquirer until a
|
||||
// liveness steal or the deadman. On any write/close error, guardedly close the
|
||||
// fd and unlink the file we created, then re-throw to the existing outer catch
|
||||
// (which keeps classifying recoverable vs fatal errnos — DRY). A FATAL errno
|
||||
// still propagates after cleanup; a RECOVERABLE one retries from a clean slate.
|
||||
// Mirrors capability-lock.cts:415-425.
|
||||
try {
|
||||
const injected = _consumeSimulatedWriteError();
|
||||
if (injected) throw injected; // test seam: one-shot writeSync failure (M9)
|
||||
fs.writeSync(fd, String(process.pid));
|
||||
fs.closeSync(fd);
|
||||
} catch (writeErr) {
|
||||
try { fs.closeSync(fd); } catch { /* best-effort — fd may already be closed */ }
|
||||
// Best-effort unlink of the lock WE just created. Guarded so we never throw
|
||||
// here; if another acquirer already stole the empty lock the unlink is a
|
||||
// harmless ENOENT no-op (we do not double-unlink someone else's lock — the
|
||||
// open(O_EXCL) above guarantees we created this path this iteration).
|
||||
try { fs.unlinkSync(lockPath); } catch { /* best-effort — no orphan */ }
|
||||
throw writeErr; // re-throw to the outer catch for recoverable/fatal classification
|
||||
}
|
||||
// Exit-time cleanup keeps a crashed locked region from leaving a stale file (#1916).
|
||||
_heldStateLocks.add(lockPath);
|
||||
return lockPath;
|
||||
@@ -1625,31 +1772,80 @@ function acquireStateLock(statePath: string, clock?: StateLockClock): string {
|
||||
continue;
|
||||
}
|
||||
if ((err as NodeJS.ErrnoException).code !== 'EEXIST') throw err; // propagate — silent bypass causes lost updates
|
||||
// Only unlink a lock we did not place when it has crossed the staleness
|
||||
// threshold (crashed holder). Nuking a fresh lock held by a slow-but-live
|
||||
// writer causes lost updates (#3711 regression).
|
||||
// Liveness-gated steal (audit M1) + steal-safety (PR #1532 review). The steal
|
||||
// decision is three-way on the lock body:
|
||||
// - VERIFIED-LIVE holder (parseable pid that signals alive): NEVER stolen until
|
||||
// its age crosses the absolute deadman ceiling (the pid-reuse backstop) —
|
||||
// nuking a slow-but-live writer's lock causes lost updates (#3711 / #500/#905/
|
||||
// #1230 family).
|
||||
// - COMPLETE DEAD pid (parseable pid, not alive): stolen PROMPTLY regardless of
|
||||
// age — a crashed holder left a full body.
|
||||
// - EMPTY / unparseable body: liveness is unknowable. While FRESH (age <=
|
||||
// freshCreateFloorMs) it is a lock still mid-creation (O_EXCL done, pid not yet
|
||||
// written) and is NOT stolen (window a); only once aged past the floor is it a
|
||||
// genuine orphan and stealable.
|
||||
// The steal itself is an ATOMIC rename-then-recreate (only one racer can rename the
|
||||
// inode) guarded by an identity re-confirm, so a racer that recreates a fresh lock
|
||||
// in the decision→steal gap never has its replacement deleted (window b). Mirrors
|
||||
// capability-lock.cts:455-499.
|
||||
try {
|
||||
const stat = fs.statSync(lockPath);
|
||||
if ((clock).now() - stat.mtimeMs > staleThresholdMs) {
|
||||
let removed = false;
|
||||
try { fs.unlinkSync(lockPath); removed = true; } catch { /* swallow: bounded below */ }
|
||||
if (removed) {
|
||||
// Successful steal — retry immediately to grab the just-freed lock.
|
||||
// Must NOT call checkBudgetAndSleep here: a throw-after-delete would
|
||||
// corrupt the filesystem state, and the budget is already bounded on
|
||||
// the next iteration's EEXIST or open attempt (#1217 regression fix).
|
||||
const ageMs = clock.now() - stat.mtimeMs;
|
||||
const bodyPid = _stateLockBodyPid(lockPath);
|
||||
const holderLive = bodyPid !== null && _stateLockIsPidAlive(bodyPid);
|
||||
let steal: boolean;
|
||||
if (holderLive) {
|
||||
steal = ageMs > deadmanCeilingMs; // pid-reuse backstop only
|
||||
} else if (bodyPid !== null) {
|
||||
steal = true; // complete dead pid → prompt steal
|
||||
} else {
|
||||
steal = ageMs > freshCreateFloorMs; // empty/garbage → protect the create window
|
||||
}
|
||||
if (steal) {
|
||||
if (_stateLockTestHooks.beforeSteal) _stateLockTestHooks.beforeSteal({ lockPath });
|
||||
// Identity re-confirm immediately before the steal: a racer that stole +
|
||||
// recreated a fresh lock in the decision→steal gap changes (dev, ino) and/or
|
||||
// the body pid → do NOT delete the replacement; re-evaluate from scratch.
|
||||
let confirmStat: fs.Stats;
|
||||
try {
|
||||
confirmStat = fs.statSync(lockPath);
|
||||
} catch {
|
||||
continue; // lock vanished between decision and steal — retry the create.
|
||||
}
|
||||
const sameInstance =
|
||||
typeof stat.dev === 'number' && typeof stat.ino === 'number' &&
|
||||
confirmStat.dev === stat.dev && confirmStat.ino === stat.ino &&
|
||||
_stateLockBodyPid(lockPath) === bodyPid;
|
||||
if (!sameInstance) {
|
||||
// The lock changed under us (a racer won the steal + recreated). Back off
|
||||
// and re-evaluate rather than deleting the racer's fresh replacement.
|
||||
checkBudgetAndSleep('lock changed before steal');
|
||||
continue;
|
||||
}
|
||||
// Persistent unlinkSync failure — apply budget + backoff so it cannot
|
||||
// busy-spin (#1217).
|
||||
checkBudgetAndSleep('stale lock removal failed');
|
||||
// Atomic steal: rename the inode aside, then remove it. Only ONE racer can
|
||||
// win the rename; a failed rename means another process already stole it, so
|
||||
// we must NOT fall through to a delete — back off and retry the create.
|
||||
const stolen = lockPath + '.stale-' + process.pid + '-' + clock.now() + '-' + (_stateStealSeq++);
|
||||
let renamed = false;
|
||||
try { fs.renameSync(lockPath, stolen); renamed = true; } catch { /* another racer won */ }
|
||||
if (renamed) {
|
||||
try { fs.rmSync(stolen, { force: true }); } catch { /* best-effort */ }
|
||||
// Successful steal — retry immediately to grab the just-freed lock.
|
||||
// Must NOT call checkBudgetAndSleep here: a throw-after-rename would
|
||||
// corrupt filesystem state, and the budget is already bounded on the next
|
||||
// iteration's EEXIST or open attempt (#1217 regression fix).
|
||||
continue;
|
||||
}
|
||||
// Lost the steal race (or a transient rename failure) — apply budget + backoff
|
||||
// so it cannot busy-spin (#1217).
|
||||
checkBudgetAndSleep('stale lock steal lost to racer');
|
||||
continue;
|
||||
}
|
||||
} catch (err) {
|
||||
// Re-throw a budget-exceeded error from the unlinkSync failure path above
|
||||
// unchanged — its message already names the real cause ("stale lock removal
|
||||
// failed") and double-wrapping it would replace that with the misleading
|
||||
// "statSync failed after EEXIST" context string (#1217 diagnostic fix).
|
||||
// Re-throw a budget-exceeded error from the steal path above unchanged — its
|
||||
// message already names the real cause ("lock changed before steal" / "stale
|
||||
// lock steal lost to racer") and double-wrapping it would replace that with the
|
||||
// misleading "statSync failed after EEXIST" context string (#1217 diagnostic fix).
|
||||
if ((err as Record<string, unknown>)?.lockBudgetExceeded) throw err;
|
||||
// statSync failed — lock was likely released between our EEXIST and this
|
||||
// stat call. Apply budget + backoff so a persistent statSync failure
|
||||
@@ -1689,13 +1885,24 @@ function withStateLock<T>(statePath: string, fn: () => T): T {
|
||||
* Optional clock seam; defaults to realClock. Passed through to acquireStateLock.
|
||||
*/
|
||||
function writeStateMd(statePath: string, content: string, cwd?: string, clock?: StateLockClock): void {
|
||||
// Invalidate disk scan cache before computing new frontmatter — the write
|
||||
// may create new PLAN/SUMMARY files that buildStateFrontmatter must see.
|
||||
// Safe for any calling pattern, not just short-lived CLI processes (#1967).
|
||||
if (cwd) _diskScanCache.delete(cwd);
|
||||
const synced = syncStateFrontmatter(content, cwd);
|
||||
const lockPath = acquireStateLock(statePath, clock);
|
||||
// Test seam (audit M8): fire AFTER the lock is taken so a test can simulate a
|
||||
// concurrent writer landing in the (now-closed) scan→lock window.
|
||||
if (_stateLockTestHooks.afterAcquire) _stateLockTestHooks.afterAcquire(lockPath);
|
||||
try {
|
||||
// Audit M8 (leaky-abstractions): the disk scan that counts PLAN/SUMMARY files
|
||||
// to build the frontmatter is the READ half of this read-modify-write — it must
|
||||
// run INSIDE the lock (mirroring readModifyWriteStateMd), not before it. Scanning
|
||||
// before acquireStateLock left a TOCTOU window where a concurrent writer that
|
||||
// committed a new PLAN/SUMMARY between our scan and our lock made writeStateMd
|
||||
// stamp STALE progress counts (lost update — the #500/#905/#1230 family). The
|
||||
// scan order is otherwise byte-for-behaviour identical for single-threaded
|
||||
// callers — only the concurrent-writer window closes.
|
||||
//
|
||||
// Invalidate the disk scan cache first — the write may create new PLAN/SUMMARY
|
||||
// files that buildStateFrontmatter must see (#1967).
|
||||
if (cwd) _diskScanCache.delete(cwd);
|
||||
const synced = syncStateFrontmatter(content, cwd);
|
||||
platformWriteSync(statePath, synced);
|
||||
} finally {
|
||||
releaseStateLock(lockPath);
|
||||
@@ -2891,4 +3098,27 @@ export = {
|
||||
cmdStateMilestoneSwitch,
|
||||
cmdSignalWaiting,
|
||||
cmdSignalResume,
|
||||
// Test seam (audit M1): inject a deterministic isPidAlive so the liveness-gated
|
||||
// steal decision is exercised without real pids. Mirrors capability-lock.cts.
|
||||
_setLockProbes(probes: Partial<{ isPidAlive: (pid: number) => boolean }>): void {
|
||||
if (typeof probes.isPidAlive === 'function') _stateLockProbes.isPidAlive = probes.isPidAlive;
|
||||
},
|
||||
_resetLockProbes(): void {
|
||||
_stateLockProbes.isPidAlive = _realIsPidAlive;
|
||||
},
|
||||
// Test seam (audit M8/M9): inject deterministic hooks for the scan-in-lock window
|
||||
// (afterAcquire), the one-shot recoverable writeSync failure (simulateWriteError),
|
||||
// and per-iteration orphan-lock snapshots (onLoopIteration). See _stateLockTestHooks.
|
||||
_setStateLockTestHooks(hooks: StateLockTestHooks): void {
|
||||
if ('afterAcquire' in hooks) _stateLockTestHooks.afterAcquire = hooks.afterAcquire;
|
||||
if ('simulateWriteError' in hooks) _stateLockTestHooks.simulateWriteError = hooks.simulateWriteError;
|
||||
if ('onLoopIteration' in hooks) _stateLockTestHooks.onLoopIteration = hooks.onLoopIteration;
|
||||
if ('beforeSteal' in hooks) _stateLockTestHooks.beforeSteal = hooks.beforeSteal;
|
||||
},
|
||||
_resetStateLockTestHooks(): void {
|
||||
delete _stateLockTestHooks.afterAcquire;
|
||||
delete _stateLockTestHooks.simulateWriteError;
|
||||
delete _stateLockTestHooks.onLoopIteration;
|
||||
delete _stateLockTestHooks.beforeSteal;
|
||||
},
|
||||
};
|
||||
|
||||
@@ -868,6 +868,241 @@ function cmdWorktreeCleanupWave(cwd: string, args: string[] = []): void {
|
||||
}
|
||||
}
|
||||
|
||||
interface RecordAgentFields {
|
||||
agentId: string;
|
||||
worktreePath: string;
|
||||
branch: string;
|
||||
base: string;
|
||||
}
|
||||
|
||||
interface RecordAgentPlan {
|
||||
ok: boolean;
|
||||
reason: string;
|
||||
hint?: string;
|
||||
entry: CleanupManifestEntry | null;
|
||||
/** Serialized manifest to write back (with trailing newline); null when ok === false. */
|
||||
manifest: string | null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Pure planner for the per-agent wave-manifest append.
|
||||
*
|
||||
* Validates the candidate entry at write time using the SAME rules the
|
||||
* cleanup-wave reader enforces (via `normalizeCleanupManifestEntry`), so an
|
||||
* entry that `record-agent` accepts is guaranteed to survive
|
||||
* `normalizeCleanupManifest` on read — a field that would be silently dropped
|
||||
* at cleanup time fails loudly here instead.
|
||||
*
|
||||
* `agent_id` is treated write-strict (required) even though the reader is
|
||||
* lenient (nullable): the whole point of this verb is to catch an
|
||||
* under-populated entry at write time, and an entry whose author cannot be
|
||||
* identified defeats that. A duplicate `(worktree_path, branch)` is also
|
||||
* rejected loudly — the reader dedups on that key, so a re-record would be
|
||||
* silently dropped (the failure mode this verb exists to eliminate). The
|
||||
* on-disk shape stays the existing 4-field entry (`agent_id`, `worktree_path`,
|
||||
* `branch`, `expected_base`) — no schema change; the reader re-derives
|
||||
* `allowed_bases`.
|
||||
*/
|
||||
function planWorktreeRecordAgent(manifestRaw: string, fields: RecordAgentFields): RecordAgentPlan {
|
||||
// 1. Write-strict required-field check (loud, with which flag is missing).
|
||||
// Trim first so a whitespace-only value (" ") is rejected here rather
|
||||
// than deferred to a guaranteed `git worktree remove` failure at cleanup.
|
||||
const agentId = (fields.agentId || '').trim();
|
||||
const worktreePath = (fields.worktreePath || '').trim();
|
||||
const branch = (fields.branch || '').trim();
|
||||
const base = (fields.base || '').trim();
|
||||
const missing: string[] = [];
|
||||
if (!agentId) missing.push('--agent-id');
|
||||
if (!worktreePath) missing.push('--path');
|
||||
if (!branch) missing.push('--branch');
|
||||
if (!base) missing.push('--base');
|
||||
if (missing.length > 0) {
|
||||
return {
|
||||
ok: false,
|
||||
reason: 'missing_field',
|
||||
hint: `record-agent requires ${missing.join(', ')}. Re-run with all of --agent-id, --path, --branch, --base set to non-empty (non-whitespace) values.`,
|
||||
entry: null,
|
||||
manifest: null,
|
||||
};
|
||||
}
|
||||
|
||||
// 2. Shared validation: run the candidate through the reader's normalizer.
|
||||
// If it returns null the reader would drop this entry on read — reject now.
|
||||
const candidate = {
|
||||
agent_id: agentId,
|
||||
worktree_path: worktreePath,
|
||||
branch,
|
||||
expected_base: base,
|
||||
};
|
||||
const entry = normalizeCleanupManifestEntry(candidate);
|
||||
if (!entry) {
|
||||
return {
|
||||
ok: false,
|
||||
reason: 'invalid_entry',
|
||||
hint: `Entry failed cleanup-manifest validation: --path/--branch/--base must be non-empty and --branch must match ^worktree-agent-[A-Za-z0-9._/-]+$ (got branch="${branch}"). Fix the field and re-run.`,
|
||||
entry: null,
|
||||
manifest: null,
|
||||
};
|
||||
}
|
||||
|
||||
// 3. Parse the existing manifest. The init shell ({orchestrator_root, worktrees: []})
|
||||
// is written inline by the orchestrator before any agent spawns; a missing or
|
||||
// malformed manifest is a loud failure here, not a silent under-populated write.
|
||||
let parsed: unknown;
|
||||
try {
|
||||
parsed = JSON.parse(manifestRaw);
|
||||
} catch {
|
||||
return {
|
||||
ok: false,
|
||||
reason: 'invalid_manifest_json',
|
||||
hint: 'Manifest is not valid JSON. The orchestrator must initialize it as {"orchestrator_root": "...", "worktrees": []} before recording agents.',
|
||||
entry: null,
|
||||
manifest: null,
|
||||
};
|
||||
}
|
||||
|
||||
// Accept the canonical {worktrees: []} shell or a bare top-level array (both
|
||||
// are read by normalizeCleanupManifest); preserve any other top-level keys.
|
||||
let worktrees: unknown[];
|
||||
let writeBack: unknown;
|
||||
if (Array.isArray(parsed)) {
|
||||
worktrees = parsed;
|
||||
writeBack = worktrees;
|
||||
} else if (parsed && typeof parsed === 'object') {
|
||||
const container = parsed as Record<string, unknown>;
|
||||
if (container.worktrees === undefined) container.worktrees = [];
|
||||
if (!Array.isArray(container.worktrees)) {
|
||||
return {
|
||||
ok: false,
|
||||
reason: 'manifest_shape_invalid',
|
||||
hint: 'Manifest "worktrees" must be an array. Re-initialize as {"orchestrator_root": "...", "worktrees": []}.',
|
||||
entry: null,
|
||||
manifest: null,
|
||||
};
|
||||
}
|
||||
worktrees = container.worktrees;
|
||||
writeBack = container;
|
||||
} else {
|
||||
return {
|
||||
ok: false,
|
||||
reason: 'manifest_shape_invalid',
|
||||
hint: 'Manifest must be a JSON object {"worktrees": []} or a top-level array.',
|
||||
entry: null,
|
||||
manifest: null,
|
||||
};
|
||||
}
|
||||
|
||||
// 4. Reject a duplicate (worktree_path, branch). The reader dedups on this
|
||||
// exact key, but only over entries that NORMALIZE successfully — so an
|
||||
// existing malformed same-key entry (which the reader would drop) must NOT
|
||||
// block recording a valid one. Run each existing entry through the reader's
|
||||
// own normalizer and compare only the entries the reader would keep; this
|
||||
// matches its dedup behavior exactly. A real duplicate signals an upstream
|
||||
// double-spawn — surface it loudly instead of silently dropping it.
|
||||
const dupKey = `${entry.worktree_path}\0${entry.branch}`;
|
||||
const isDuplicate = worktrees.some((existing) => {
|
||||
const normalized = normalizeCleanupManifestEntry(existing);
|
||||
return normalized !== null && `${normalized.worktree_path}\0${normalized.branch}` === dupKey;
|
||||
});
|
||||
if (isDuplicate) {
|
||||
return {
|
||||
ok: false,
|
||||
reason: 'duplicate_entry',
|
||||
hint: `The manifest already records worktree_path="${entry.worktree_path}" branch="${entry.branch}". The cleanup reader dedups on (worktree_path, branch), so re-recording would be silently dropped — this usually signals an upstream double-spawn. Investigate rather than re-record.`,
|
||||
entry: null,
|
||||
manifest: null,
|
||||
};
|
||||
}
|
||||
|
||||
// 5. Append the minimal 4-field entry, matching the existing on-disk format.
|
||||
const recorded: CleanupManifestEntry = {
|
||||
agent_id: entry.agent_id,
|
||||
worktree_path: entry.worktree_path,
|
||||
branch: entry.branch,
|
||||
expected_base: entry.expected_base,
|
||||
};
|
||||
worktrees.push(recorded);
|
||||
|
||||
return {
|
||||
ok: true,
|
||||
reason: 'ok',
|
||||
entry: recorded,
|
||||
manifest: `${JSON.stringify(writeBack, null, 2)}\n`,
|
||||
};
|
||||
}
|
||||
|
||||
interface RecordAgentCmdDeps {
|
||||
readFile?: (p: string) => string;
|
||||
writeFile?: (p: string, content: string) => void;
|
||||
write?: (s: string) => void;
|
||||
writeErr?: (s: string) => void;
|
||||
}
|
||||
|
||||
interface RecordAgentCmdResult {
|
||||
ok: boolean;
|
||||
reason: string;
|
||||
hint?: string;
|
||||
entry: CleanupManifestEntry | null;
|
||||
manifest_path?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* CLI command: append a validated per-agent entry to a wave cleanup manifest.
|
||||
*
|
||||
* Usage: worktree record-agent --manifest <path> --agent-id <id> --path <worktree> --branch <branch> --base <sha>
|
||||
*
|
||||
* Fails loudly (non-zero exit + recovery hint on stderr) when a field is
|
||||
* missing/garbled or the manifest is absent/malformed, rather than appending an
|
||||
* under-populated entry that the cleanup reader would silently drop.
|
||||
*/
|
||||
function cmdWorktreeRecordAgent(cwd: string, args: string[] = [], deps: RecordAgentCmdDeps = {}): RecordAgentCmdResult {
|
||||
const flag = (name: string): string => {
|
||||
const i = args.indexOf(name);
|
||||
return i >= 0 && i + 1 < args.length ? args[i + 1] : '';
|
||||
};
|
||||
const write = deps.write || ((s: string) => process.stdout.write(s));
|
||||
const writeErr = deps.writeErr || ((s: string) => process.stderr.write(s));
|
||||
|
||||
const manifestPath = flag('--manifest');
|
||||
if (!manifestPath) {
|
||||
writeErr('Usage: worktree record-agent --manifest <path> --agent-id <id> --path <worktree> --branch <branch> --base <sha>\n');
|
||||
process.exitCode = 2;
|
||||
return { ok: false, reason: 'usage', entry: null };
|
||||
}
|
||||
|
||||
const resolved = path.resolve(cwd, manifestPath);
|
||||
const readFile = deps.readFile || ((p: string) => fs.readFileSync(p, 'utf8'));
|
||||
let manifestRaw: string;
|
||||
try {
|
||||
manifestRaw = readFile(resolved);
|
||||
} catch (err) {
|
||||
const hint = `Manifest not found or unreadable at ${manifestPath}. The orchestrator must initialize it ({"orchestrator_root": "...", "worktrees": []}) before recording agents.`;
|
||||
writeErr(`[gsd] worktree.record-agent: manifest_read_failed — ${hint}\n`);
|
||||
write(`${JSON.stringify({ ok: false, reason: 'manifest_read_failed', hint, error: (err as Error).message }, null, 2)}\n`);
|
||||
process.exitCode = 1;
|
||||
return { ok: false, reason: 'manifest_read_failed', hint, entry: null };
|
||||
}
|
||||
|
||||
const plan = planWorktreeRecordAgent(manifestRaw, {
|
||||
agentId: flag('--agent-id'),
|
||||
worktreePath: flag('--path'),
|
||||
branch: flag('--branch'),
|
||||
base: flag('--base'),
|
||||
});
|
||||
|
||||
if (!plan.ok || plan.manifest === null) {
|
||||
writeErr(`[gsd] worktree.record-agent: ${plan.reason} — ${plan.hint || ''}\n`);
|
||||
write(`${JSON.stringify({ ok: false, reason: plan.reason, hint: plan.hint }, null, 2)}\n`);
|
||||
process.exitCode = 1;
|
||||
return { ok: false, reason: plan.reason, hint: plan.hint, entry: null };
|
||||
}
|
||||
|
||||
const writeFile = deps.writeFile || ((p: string, content: string) => fs.writeFileSync(p, content, 'utf8'));
|
||||
writeFile(resolved, plan.manifest);
|
||||
write(`${JSON.stringify({ ok: true, reason: 'ok', entry: plan.entry, manifest_path: resolved }, null, 2)}\n`);
|
||||
return { ok: true, reason: 'ok', entry: plan.entry, manifest_path: resolved };
|
||||
}
|
||||
|
||||
/**
|
||||
* Reap orphaned linked worktrees whose lock owner process is dead, whose
|
||||
* branch tip is fully merged into the default branch, and whose lock file
|
||||
@@ -1167,6 +1402,8 @@ export = {
|
||||
planWorktreeWaveCleanup,
|
||||
executeWorktreeWaveCleanupPlan,
|
||||
cmdWorktreeCleanupWave,
|
||||
planWorktreeRecordAgent,
|
||||
cmdWorktreeRecordAgent,
|
||||
reapOrphanWorktrees,
|
||||
cmdWorktreeReapOrphans,
|
||||
resolveWorktreeRoot,
|
||||
|
||||
Reference in New Issue
Block a user