Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 29 additions & 2 deletions src/resources/extensions/gsd/auto-dispatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ import { atomicWriteSync, removeProjectionFileSync } from "./atomic-write.js";
import { logWarning, logError } from "./workflow-logger.js";
import { dirname, join, sep } from "node:path";
import { hasImplementationArtifacts } from "./milestone-implementation-evidence.js";
import { isFlatPhaseMigrationInFlight } from "./flat-phase-migration.js";
import { composeToolAffordanceReminder } from "./unit-context-composer.js";
import {
buildDiscussMilestonePrompt,
buildDiscussProjectPrompt,
Expand Down Expand Up @@ -853,6 +855,15 @@ export const DISPATCH_RULES: DispatchRule[] = [
// "never discussed" and re-dispatches discuss-milestone after task
// closeout instead of continuing execution (#1317).
const artifactBasePath = resolveArtifactBasePath(basePath, mid, session);
// The layout migration moves .gsd/milestones/ aside before rendering
// .gsd/phases/, so mid-flight the slice plans exist in neither layout.
// A dispatch landing in that window reads "never discussed" and re-plans a
// milestone that is already fully planned in the DB, discarding the plan.
// Startup dispatch races the migration on every run, so decline to decide
// while the layout is converting; the next iteration sees a settled tree.
// Scoped to a migration actually in flight — a settled legacy project must
// still reach this rule (#4671).
if (isFlatPhaseMigrationInFlight(artifactBasePath)) return null;
if (hasMilestonePassedDiscuss(artifactBasePath, mid)) return null;
// Align with the plan-v2 gate's lookup semantics: whitespace-only counts
// as missing, and an auto worktree may fall back to GSD_PROJECT_ROOT.
Expand Down Expand Up @@ -2074,6 +2085,22 @@ function applyLanguageDirectiveToDispatch(
return { ...action, prompt: `${directive}\n\n${action.prompt}` };
}

/**
* Repeat the unit's allowed tool tokens at the very end of the dispatched prompt.
*
* `## Tool Surface` already carries this, but it lands ~4% into a 14K-character
* prompt (measured on validate-milestone: offset 627, 13,731 characters after it)
* and units kept reaching for `bash`, `gsd_uat_exec`, and subagent `"review"`
* regardless. Applied at the dispatch seam so every unit gets it from one place,
* rather than editing each prompt builder's tail.
*/
function appendToolAffordanceToDispatch(action: DispatchAction): DispatchAction {
if (action.action !== "dispatch" || !action.prompt || !action.unitType) return action;
const reminder = composeToolAffordanceReminder(action.unitType);
if (!reminder || action.prompt.trimEnd().endsWith(reminder)) return action;
return { ...action, prompt: `${action.prompt.trimEnd()}\n\n${reminder}` };
}

// ─── Resolver ─────────────────────────────────────────────────────────────

/**
Expand Down Expand Up @@ -2170,7 +2197,7 @@ export async function resolveDispatch(
level: "error",
};
}
return applyLanguageDirectiveToDispatch(action, ctx.prefs);
return applyLanguageDirectiveToDispatch(appendToolAffordanceToDispatch(action), ctx.prefs);
}

for (const rule of DISPATCH_RULES) {
Expand All @@ -2189,7 +2216,7 @@ export async function resolveDispatch(
matchedRule: rule.name,
};
}
return applyLanguageDirectiveToDispatch(action, ctx.prefs);
return applyLanguageDirectiveToDispatch(appendToolAffordanceToDispatch(action), ctx.prefs);
}
}

Expand Down
6 changes: 5 additions & 1 deletion src/resources/extensions/gsd/auto/loop.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
} from "./unit-phase.js";
import { STUCK_WINDOW_SIZE } from "./dispatch-history.js";
import { debugLog } from "../debug-logger.js";
import { markBlockedStopReason } from "../stop-notice.js";
import { isInfrastructureError, isTransientCooldownError, getCooldownRetryAfterMs, COOLDOWN_FALLBACK_WAIT_MS, MAX_COOLDOWN_RETRIES } from "./infra-errors.js";
import { ModelPolicyDispatchBlockedError } from "../auto-model-selection.js";
import { resolveEngine } from "../engine-resolver.js";
Expand Down Expand Up @@ -1194,7 +1195,10 @@ export async function autoLoop(
});
finishTurn("paused", "manual-attention", "orchestration-blocked");
} else {
await deps.stopAuto(ctx, pi, blockMessage);
// Carry the blocked marker: the headless host picks its exit code
// from the reason string, so an unmarked blocked stop exits 0 and
// reports success over a milestone that never closed.
await deps.stopAuto(ctx, pi, markBlockedStopReason(blockMessage));
finishTurn("stopped", "manual-attention", "orchestration-blocked");
}
finishIncompleteIteration({
Expand Down
13 changes: 12 additions & 1 deletion src/resources/extensions/gsd/browser-evidence.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@ export const BROWSER_ACTION_RE = /\b(?:open(?:ed)?|navigate(?:d)?|click(?:ed)?|t
export const BROWSER_ASSERTION_RE = /\b(?:assert(?:ed|ion)?|observed|confirmed|verified|expected|visible|text|count|label|strikethrough|localstorage|screenshot|snapshot|passed)\b/i;
const NON_REQUIREMENT_BROWSER_HEADING_RE = /^(?:not\s+proven|not\s+covered|out\s+of\s+scope|deferred|follow-?ups?|known\s+limitations|notes\s+for\s+tester)\b/i;
const NON_REQUIREMENT_BROWSER_LINE_RE = /\b(?:deferred|not\s+proven|not\s+covered|out\s+of\s+scope|future\s+slice|follow-?up|no\s+(?:live\s+)?browser|without\s+(?:a\s+)?browser|not\s+(?:a\s+)?browser)\b/i;
// The negations above only fire when the negator sits directly beside "browser".
// A negated *list* — "no runtime behavior, server, UI, or browser interaction is
// involved" — separates them, so the line fell through and matched
// `browser interaction` as a requirement. That escalated a slice whose UAT said
// browsers were irrelevant into one demanding browser verification. Bounded to a
// single clause so a genuine requirement in the next sentence still counts.
const NEGATED_BROWSER_CLAUSE_RE = /\b(?:no|without|not)\b[^.;:!?]{0,60}\bbrowser\b/i;

export function compactTextParts(parts: Array<string | string[] | null | undefined>): string {
return parts.flatMap((part) => Array.isArray(part) ? part : [part])
Expand Down Expand Up @@ -51,7 +58,11 @@ export function hasBrowserRequiredText(text: string): boolean {
if (!inNonRequirementSection && BROWSER_REQUIREMENT_RE.test(title)) return true;
continue;
}
if (inNonRequirementSection || NON_REQUIREMENT_BROWSER_LINE_RE.test(line)) continue;
if (
inNonRequirementSection ||
NON_REQUIREMENT_BROWSER_LINE_RE.test(line) ||
NEGATED_BROWSER_CLAUSE_RE.test(line)
) continue;
if (BROWSER_REQUIREMENT_RE.test(line)) return true;
}
return false;
Expand Down
12 changes: 11 additions & 1 deletion src/resources/extensions/gsd/closeout-consistency-gate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
} from "./db/milestone-closeout-readiness.js";
import { loadEffectiveGSDPreferences } from "./preferences.js";
import { captureMilestoneVerificationSourceRevision } from "./verification-source-integrity.js";
import { resolveCanonicalMilestoneRoot } from "./worktree-manager.js";
import { atomicWriteSync, removeProjectionFileSync } from "./atomic-write.js";

export const CLOSEOUT_CONSISTENCY_BLOCKED_REASON = "closeout-consistency-blocked";
Expand Down Expand Up @@ -214,7 +215,16 @@ export function checkCloseoutConsistencyGate(
}
let canonicalAuthorization = null;
if (adoptedMilestone) {
const artifactBasePath = options.artifactBasePath ?? artifactBasePathFromDb();
// Resolve the tree the milestone actually executed in. Under worktree
// isolation validation runs inside .gsd-worktrees/<id> and records that
// tree's revision; computing the current revision from the project root
// compares two different working trees, so the authorization could never
// match and the merge blocked with "validation authorization is not
// current" on a milestone the DB had already marked complete.
const artifactBasePath = resolveCanonicalMilestoneRoot(
options.artifactBasePath ?? artifactBasePathFromDb() ?? "",
milestoneId,
) || options.artifactBasePath || artifactBasePathFromDb();
if (!artifactBasePath) {
return blocked(
"validation-not-pass",
Expand Down
49 changes: 49 additions & 0 deletions src/resources/extensions/gsd/flat-phase-migration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
getMilestoneSlices,
getSliceTasks,
} from "./gsd-db.js";
import { withFileLock } from "./file-lock.js";
import { countDbHierarchy, scanMarkdownHierarchy } from "./migration-auto-check.js";
import { logWarning } from "./workflow-logger.js";
import { LAYOUT_SEGMENTS } from "./layout-policy.js";
Expand Down Expand Up @@ -306,6 +307,19 @@ export function needsFlatPhaseMigration(basePath: string): boolean {
return hasLegacyMilestoneSubdirs(legacyMigratingPath(basePath));
}

/**
* Is a flat-phase migration mid-flight right now?
*
* True only while the legacy tree sits at `.gsd/milestones.migrating` — the
* window between moving it aside and rendering `.gsd/phases/`, during which the
* slice plans exist in neither layout. Distinct from `needsFlatPhaseMigration`,
* which is also true for a settled legacy project that has not started
* migrating: callers that must not misread a transient gap want this one.
*/
export function isFlatPhaseMigrationInFlight(basePath: string): boolean {
return hasLegacyMilestoneSubdirs(legacyMigratingPath(basePath));
}

/** Retention window before flat-phase migration backups are auto-pruned. */
export const FLAT_PHASE_BACKUP_RETENTION_MS = 30 * 24 * 60 * 60 * 1000;

Expand Down Expand Up @@ -366,6 +380,41 @@ export function pruneStaleFlatPhaseBackups(basePath: string): number {
export async function migrateToFlatPhase(basePath: string): Promise<void> {
if (!needsFlatPhaseMigration(basePath)) return;

// Headless runs the gsd extension in two processes (host + RPC child) and both
// fire session_start, so two migrations race over the same tree: one moves
// milestones/ aside or clears phases/ while the other is mid-render, and the
// loser dies on ENOENT. A whole-migration cross-process lock is the only
// ordering that helps — the steps are not individually idempotent.
// Stale window is well above file-lock's 10s default: migration renders the
// entire projection and can legitimately run for minutes on a large project.
try {
await withFileLock(
join(basePath, ".gsd"),
async () => {
// Re-check under the lock: the winner may have completed the migration
// while this process was waiting, which makes the loser a clean no-op
// rather than a second destructive pass.
if (!needsFlatPhaseMigration(basePath)) return;
await migrateToFlatPhaseLocked(basePath);
},
// Migration is mostly synchronous fs work, so it blocks the event loop and
// proper-lockfile's mtime keepalive timer cannot fire while it runs. The
// stale window therefore has to exceed the whole migration, not the gap
// between keepalives, or a waiting process declares the lock dead and
// steals it mid-render.
{ retries: 30, stale: 600_000 },
);
} catch (err) {
// If the lock was stolen anyway, the holder's release() throws even though
// its migration finished. Outcome decides: a completed migration makes that
// release error noise, an incomplete one is a real failure.
const message = (err as Error)?.message ?? "";
if (!/already released|compromised/i.test(message) || needsFlatPhaseMigration(basePath)) throw err;
logWarning("migration", `flat-phase migration lock released early but migration completed: ${message}`);
}
}

async function migrateToFlatPhaseLocked(basePath: string): Promise<void> {
const milestonesPath = join(basePath, ".gsd", "milestones");
const migratingPath = legacyMigratingPath(basePath);
const phasesPath = join(basePath, ".gsd", LAYOUT_SEGMENTS.level1);
Expand Down
7 changes: 7 additions & 0 deletions src/resources/extensions/gsd/markdown-renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -425,6 +425,13 @@ function renderSlicePlanMarkdown(slice: SliceRow, tasks: TaskRow[], gates: GateR

lines.push(`# ${slice.id}: ${slice.title || slice.id}`);
lines.push("");
// State the DB identities explicitly. Under flat-phase layout this file lives
// at .gsd/phases/01-m001/01-01-PLAN.md, and an agent with only the path to go
// on infers milestoneId "01-m001"/sliceId "01-01" — which the tool contract
// rejects, since it demands the DB identities M001/S01.
lines.push(`**Milestone:** ${slice.milestone_id}`);
lines.push(`**Slice:** ${slice.id}`);
lines.push("");
lines.push(`**Goal:** ${slice.goal}`);
lines.push(`**Demo:** ${slice.demo}`);
lines.push("");
Expand Down
23 changes: 23 additions & 0 deletions src/resources/extensions/gsd/stop-notice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,17 @@ export function isBlockedStopReason(reason?: string | null): boolean {
return /^Blocked:\s*/i.test(reason ?? "");
}

/**
* Mark a reason as a blocked stop.
*
* The headless host derives its exit code from this marker alone, so a stop the
* orchestrator classified as `kind: "blocked"` must carry it — otherwise the run
* reports 0 (complete) over work that never finished. Idempotent.
*/
export function markBlockedStopReason(reason: string): string {
return isBlockedStopReason(reason) ? reason : `Blocked: ${reason}`;
}

/** Strip the "Blocked: " marker for display. */
export function stopNoticeDisplayReason(reason?: string | null): string {
return (reason ?? "").replace(/^Blocked:\s*/i, "").trim();
Expand Down Expand Up @@ -44,9 +55,16 @@ export function formatVerdictRejectedNotice(message: string): string {

export const PAUSED_NOTICE_PREFIXES = ["auto-mode paused", "step-mode paused"] as const;

/** Prefixes formatStopNoticePrefix produces for a blocked stop. */
export const BLOCKED_NOTICE_PREFIXES = ["auto-mode blocked", "step-mode blocked"] as const;

export const TERMINAL_NOTICE_PREFIXES = [
"auto-mode stopped",
"step-mode stopped",
// A blocked stop ends the run exactly like a plain stop — it just carries a
// different exit code. Omitting these left the host without a terminal signal
// for the one outcome that most needs to be reported.
...BLOCKED_NOTICE_PREFIXES,
"auto-mode complete",
"no active milestone",
"auto-mode idle",
Expand Down Expand Up @@ -95,6 +113,11 @@ export function isInteractiveMenuUnavailableNotice(message: string): boolean {
export function isBlockedNoticeMessage(message: string): boolean {
return (
message.includes("blocked:") ||
// formatStopNoticePrefix emits "Auto-mode blocked — reason" (em-dash, no
// colon), so the "blocked:" test above never matched the very notice this
// module's own formatter produces. A blocked stop then read as an ordinary
// stop and headless exited 0 over an unfinished milestone.
BLOCKED_NOTICE_PREFIXES.some((prefix) => message.startsWith(prefix)) ||
message.startsWith("verdict rejected") ||
(isPauseNotice(message) && !isNonBlockingPauseNotice(message)) ||
isManualResolutionNotice(message) ||
Expand Down
34 changes: 34 additions & 0 deletions src/resources/extensions/gsd/tests/browser-evidence.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,3 +140,37 @@ describe('hasBrowserRequiredText', () => {
);
});
});

describe('hasBrowserRequiredText — negated browser mentions', () => {
// Acceptance run 7: a slice that writes two text files declared
// "UAT mode: artifact-driven" and explained why. complete-slice rejected it with
// "UAT requires browser verification". The rationale line read
// "...no runtime behavior, server, UI, or browser interaction is involved" — the
// negator sits several list items away from "browser", so the adjacency-based
// negation guard missed it and `browser interaction` matched as a requirement.
test('a negated list mentioning browser is not a browser requirement', () => {
const text = [
'## UAT Type',
'',
'- UAT mode: artifact-driven',
'- Why this mode is sufficient: slice deliverables are static text files with exact',
' required content; no runtime behavior, server, UI, or browser interaction is involved.',
].join('\n');
assert.ok(!hasBrowserRequiredText(text), 'negated browser mention must not escalate');
});

test('adjacent negations still pass', () => {
assert.ok(!hasBrowserRequiredText('- No browser interaction is required.'));
assert.ok(!hasBrowserRequiredText('- Verified without a browser session.'));
});

test('a real requirement in a later clause still counts', () => {
// The negation guard is clause-bounded, so it must not swallow the sentence after it.
const text = [
'## Test Cases',
'',
'1. No seeded data is needed. Open the page at localhost:3000 and screenshot it.',
].join('\n');
assert.ok(hasBrowserRequiredText(text), 'a genuine browser step must still be detected');
});
});
Loading
Loading