mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 14:38:59 +02:00
fix: pre-landing review fixes
Testing-specialist findings, all mechanical: (1) pin the E2E fixture's git branch (-b main / init.defaultBranch=main) and assert every setup command's exit status so operator git config can't silently corrupt a paid run; (2) tighten the dispatch matcher to Step 18-prompt-specific markers (document-release/SKILL.md | executing the /document-release workflow) so a subagent merely quoting section text can't false-pass the regression assert (verified against recorded burn-in transcripts); (3) replace the subsumed carve-guards anchor with three non-overlapping per-touchpoint anchors (gerund/imperative/3rd-person) so each touchpoint is independently enforced. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
62cdb0811c
commit
14ee421fb1
@@ -118,17 +118,20 @@ export const CARVE_GUARDS: Record<string, CarveGuard> = {
|
||||
// (CI backstop: test/pr-title-sync-workflow-safety.test.ts).
|
||||
// Same carve also stranded the Step 18 /document-release dispatch out of
|
||||
// sight — the skeleton never named it and the handoff "got lost" (#2666
|
||||
// follow-up). The two subagent anchors pin the restored visibility:
|
||||
// 'the /document-release subagent' matches all three touchpoints (trigger
|
||||
// via section-index + STOP pointer, Step 17 handoff, hoisted invariant);
|
||||
// 'dispatches the /document-release subagent' pins the invariant itself.
|
||||
// follow-up). Three NON-OVERLAPPING anchors pin the restored visibility,
|
||||
// one per touchpoint (no anchor is a substring of another, so each is
|
||||
// independently enforced — a subsumed anchor adds zero enforcement):
|
||||
// gerund form → manifest trigger (renders 2x: section index + STOP)
|
||||
// imperative → Step 17 handoff line
|
||||
// 3rd person → hoisted doc-sync invariant
|
||||
// Matching is case-sensitive String.includes — "dispatching the" does NOT
|
||||
// contain "dispatch the" — so update anchors in lockstep with any
|
||||
// touchpoint rewording.
|
||||
mustStayInSkeleton: [
|
||||
'v$NEW_VERSION',
|
||||
'gstack-pr-title-rewrite',
|
||||
'the /document-release subagent',
|
||||
'dispatching the /document-release subagent to sync docs',
|
||||
'dispatch the /document-release subagent to sync docs',
|
||||
'dispatches the /document-release subagent',
|
||||
],
|
||||
// ...while the full create/update procedure stays carved into pr-body.md
|
||||
|
||||
@@ -91,10 +91,32 @@ describeE2E('Ship doc-sync dispatch E2E (gate)', () => {
|
||||
// Bare remote + clone; Steps 0-16 "already done": feature branch with a
|
||||
// committed change, VERSION bumped, CHANGELOG entry written. Not pushed —
|
||||
// Step 17 (the slice's first step) does that.
|
||||
spawnSync('git', ['init', '--bare'], { cwd: remoteDir, stdio: 'pipe' });
|
||||
spawnSync('git', ['clone', remoteDir, repoDir], { stdio: 'pipe' });
|
||||
// Branch pinned with -b main / -c init.defaultBranch=main so operator git
|
||||
// config never leaks into the fixture (default-config machines would
|
||||
// otherwise create master and the later `push -u origin main` would fail).
|
||||
// Every setup command asserts status — a broken fixture must fail loud
|
||||
// and free here, never burn a paid run downstream.
|
||||
const assertOk = (r: ReturnType<typeof spawnSync>, what: string) => {
|
||||
if (r.status !== 0) {
|
||||
throw new Error(
|
||||
`ship-docsync fixture setup failed: ${what} → exit ${r.status}\n${r.stderr?.toString() ?? ''}`
|
||||
);
|
||||
}
|
||||
return r;
|
||||
};
|
||||
assertOk(
|
||||
spawnSync('git', ['init', '--bare', '-b', 'main'], { cwd: remoteDir, stdio: 'pipe', timeout: 15000 }),
|
||||
'git init --bare -b main'
|
||||
);
|
||||
assertOk(
|
||||
spawnSync('git', ['-c', 'init.defaultBranch=main', 'clone', remoteDir, repoDir], { stdio: 'pipe', timeout: 15000 }),
|
||||
'git clone'
|
||||
);
|
||||
const run = (cmd: string, args: string[]) =>
|
||||
spawnSync(cmd, args, { cwd: repoDir, stdio: 'pipe', timeout: 10000 });
|
||||
assertOk(
|
||||
spawnSync(cmd, args, { cwd: repoDir, stdio: 'pipe', timeout: 10000 }),
|
||||
`${cmd} ${args.join(' ')}`
|
||||
);
|
||||
run('git', ['config', 'user.email', 'test@test.com']);
|
||||
run('git', ['config', 'user.name', 'Test']);
|
||||
run('git', ['config', 'commit.gpgsign', 'false']);
|
||||
@@ -180,10 +202,18 @@ describeE2E('Ship doc-sync dispatch E2E (gate)', () => {
|
||||
// the transcript and would false-positive any transcript-wide match
|
||||
// (trap documented in skill-e2e-autoplan-dual-voice.test.ts).
|
||||
const calls = Array.isArray(result.toolCalls) ? result.toolCalls : [];
|
||||
// Matcher is dispatch-SPECIFIC, not mention-specific: both markers come
|
||||
// verbatim from the Step 18 subagent prompt dictated by pr-body.md. A
|
||||
// subagent that merely quotes section text mentioning "document-release"
|
||||
// (e.g. a PR-body drafter) must NOT count — that false-pass would mask
|
||||
// the exact regression this test exists to catch. Verified against
|
||||
// recorded burn-in transcripts: real dispatch inputs carry both markers.
|
||||
const dispatchIdx = calls.findIndex(
|
||||
(tc) =>
|
||||
(tc.tool === 'Agent' || tc.tool === 'Task') &&
|
||||
/document-release/i.test(JSON.stringify(tc.input ?? {}))
|
||||
/document-release\/SKILL\.md|executing the \/document-release workflow/i.test(
|
||||
JSON.stringify(tc.input ?? {})
|
||||
)
|
||||
);
|
||||
const prCreateIdx = calls.findIndex(
|
||||
(tc) =>
|
||||
|
||||
Reference in New Issue
Block a user