* test(#1995): regression test for agent-<id> branch namespace Add failing-first tests proving that normalizeCleanupManifestEntry and planWorktreeRecordAgent reject Claude Code's current agent-<id> isolation branches (only worktree-agent-<id> is accepted). Boundary tests cover both namespaces plus rejection cases. * fix(#1995): widen worktree branch regex to accept agent-<id> namespace Claude Code's isolation="worktree" branch naming changed from worktree-agent-<id> to agent-<id>. Widen the regex in all 7 locations from ^worktree-agent-[A-Za-z0-9._/-]+$ to ^(worktree-)?agent-[A-Za-z0-9._/-]+$ so both namespaces are accepted. Introduce a shared WORKTREE_AGENT_BRANCH_RE constant in src/worktree-safety.cts to prevent future drift. Closes #1995 * fix(#1995): update workflow guards, test assertions, and baselines Widen the branch-check regex in execute-phase.md and execute-plan.md. Update all test assertions that checked for ^worktree-agent- to expect the widened ^(worktree-)?agent- pattern. Regenerate golden-install-parity fixtures, agent-size-baseline, and workflow-size-baseline. Closes #1995 * fix(#1995): update extractCwdGuardBash sanity check for widened regex The e2e test's sanity check verified the extracted bash block contained 'worktree-agent-'. After widening to '(worktree-)?agent-', update the check to match the new pattern. * fix(#1995): widen missed workflow-guard branch check + changeset + lint fixes - hooks/gsd-workflow-guard.js: widen startsWith('worktree-agent-') to /^(worktree-)?agent-/ regex — same defect class, was missed in prior commit - tests/worktree.test.cjs: fix indentation regression from prior edit - Add .changeset/1995-worktree-agent-branch-namespace.md (pr:0 placeholder) Found by orthogonal code review (Step 4). * fix(#1995): regenerate golden + size baselines for workflow-guard change * docs(#1995): backfill changeset PR number (2548)
This commit is contained in:
Tom Boucher
@@ -662,6 +662,16 @@ describe('planWorktreeRecordAgent', () => {
|
||||
assert.equal(readBack.reason, 'empty_manifest');
|
||||
});
|
||||
|
||||
test('accepts the current agent-<id> namespace (#1995)', () => {
|
||||
const plan = planWorktreeRecordAgent('{"worktrees":[]}', { ...VALID, branch: 'agent-a1' });
|
||||
assert.equal(plan.ok, true);
|
||||
assert.equal(plan.entry.branch, 'agent-a1');
|
||||
const readBack = planWorktreeWaveCleanup('/repo/main', JSON.parse(plan.manifest));
|
||||
assert.equal(readBack.ok, true);
|
||||
assert.equal(readBack.entries.length, 1);
|
||||
assert.equal(readBack.entries[0].branch, 'agent-a1');
|
||||
});
|
||||
|
||||
test('fails loudly on malformed manifest JSON instead of clobbering it', () => {
|
||||
const plan = planWorktreeRecordAgent('{not valid json', VALID);
|
||||
assert.equal(plan.ok, false);
|
||||
@@ -749,7 +759,9 @@ describe('planWorktreeRecordAgent', () => {
|
||||
|
||||
describe('planWorktreeRecordAgent — fast-check parity invariant (#1298)', () => {
|
||||
const seg = fc.stringMatching(/^[A-Za-z0-9._/-]+$/); // include '/' — the namespace allows it
|
||||
const agentBranch = seg.map((s) => `worktree-agent-${s}`);
|
||||
const legacyAgentBranch = seg.map((s) => `worktree-agent-${s}`);
|
||||
const currentAgentBranch = seg.map((s) => `agent-${s}`);
|
||||
const agentBranch = fc.oneof(legacyAgentBranch, currentAgentBranch);
|
||||
const nonEmpty = fc.stringMatching(/^\S[\S ]*$/); // no leading whitespace, not blank
|
||||
|
||||
test('any writer-accepted entry round-trips through the cleanup reader unchanged', () => {
|
||||
@@ -776,7 +788,7 @@ describe('planWorktreeRecordAgent — fast-check parity invariant (#1298)', () =
|
||||
agentId: nonEmpty,
|
||||
worktreePath: nonEmpty,
|
||||
// Any branch that does NOT match the disposable namespace.
|
||||
branch: fc.string({ minLength: 1 }).filter((b) => !/^worktree-agent-[A-Za-z0-9._/-]+$/.test(b.trim())),
|
||||
branch: fc.string({ minLength: 1 }).filter((b) => !/^(worktree-)?agent-[A-Za-z0-9._/-]+$/.test(b.trim())),
|
||||
base: nonEmpty,
|
||||
}),
|
||||
(fields) => {
|
||||
@@ -788,6 +800,79 @@ describe('planWorktreeRecordAgent — fast-check parity invariant (#1298)', () =
|
||||
});
|
||||
});
|
||||
|
||||
// ─── branch namespace boundary tests (#1995) ─────────────────────────────────
|
||||
// Claude Code's isolation="worktree" branch naming changed from worktree-agent-<id>
|
||||
// to agent-<id>. Both namespaces must be accepted; non-agent branches rejected.
|
||||
|
||||
describe('worktree branch namespace boundaries (#1995)', () => {
|
||||
const ACCEPTED_BRANCHES = [
|
||||
'agent-a1',
|
||||
'agent-abc123',
|
||||
'agent-session.42',
|
||||
'worktree-agent-a1',
|
||||
'worktree-agent-abc123',
|
||||
'worktree-agent-session.42',
|
||||
];
|
||||
const REJECTED_BRANCHES = [
|
||||
'feature-x',
|
||||
'main',
|
||||
'agent',
|
||||
'worktree-agent',
|
||||
'xagent-y',
|
||||
'worktree-foo',
|
||||
'worktree-agent-',
|
||||
'agent-',
|
||||
'',
|
||||
];
|
||||
|
||||
for (const branch of ACCEPTED_BRANCHES) {
|
||||
test(`planWorktreeRecordAgent accepts branch "${branch}"`, () => {
|
||||
const plan = planWorktreeRecordAgent('{"worktrees":[]}', {
|
||||
agentId: 'a1', worktreePath: '/repo/wt', branch, base: 'abc123',
|
||||
});
|
||||
assert.equal(plan.ok, true, `expected branch "${branch}" to be accepted`);
|
||||
assert.equal(plan.entry.branch, branch);
|
||||
});
|
||||
}
|
||||
|
||||
for (const branch of REJECTED_BRANCHES) {
|
||||
test(`planWorktreeRecordAgent rejects branch "${branch}"`, () => {
|
||||
const plan = planWorktreeRecordAgent('{"worktrees":[]}', {
|
||||
agentId: 'a1', worktreePath: '/repo/wt', branch, base: 'abc123',
|
||||
});
|
||||
if (branch === '') {
|
||||
assert.equal(plan.reason, 'missing_field');
|
||||
} else {
|
||||
assert.equal(plan.ok, false, `expected branch "${branch}" to be rejected`);
|
||||
assert.equal(plan.reason, 'invalid_entry');
|
||||
assert.match(plan.hint, /\(worktree-\)\?agent-/);
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
test('cleanup-wave retains agent-<id> entries alongside worktree-agent-<id> entries', () => {
|
||||
const manifest = {
|
||||
orchestrator_root: '/repo/main',
|
||||
worktrees: [
|
||||
{ agent_id: 'a1', worktree_path: '/repo/wt-a1', branch: 'agent-a1', expected_base: 'abc' },
|
||||
{ agent_id: 'a2', worktree_path: '/repo/wt-a2', branch: 'worktree-agent-a2', expected_base: 'def' },
|
||||
],
|
||||
};
|
||||
const result = planWorktreeWaveCleanup('/repo/main', manifest);
|
||||
assert.equal(result.ok, true);
|
||||
assert.equal(result.entries.length, 2);
|
||||
assert.deepEqual(result.entries.map((e) => e.branch), ['agent-a1', 'worktree-agent-a2']);
|
||||
});
|
||||
|
||||
test('hint string names the widened accepted pattern', () => {
|
||||
const plan = planWorktreeRecordAgent('{"worktrees":[]}', {
|
||||
agentId: 'a1', worktreePath: '/repo/wt', branch: 'feature-x', base: 'abc123',
|
||||
});
|
||||
assert.equal(plan.ok, false);
|
||||
assert.match(plan.hint, /\(worktree-\)\?agent-\[A-Za-z0-9\._\/-\]\+/);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── cmdWorktreeRecordAgent (#1298 CLI wrapper) ───────────────────────────────
|
||||
|
||||
describe('cmdWorktreeRecordAgent', () => {
|
||||
@@ -824,6 +909,26 @@ describe('cmdWorktreeRecordAgent', () => {
|
||||
assert.match(out.join(''), /"ok": true/);
|
||||
});
|
||||
|
||||
test('accepts the current agent-<id> namespace via CLI (#1995)', () => {
|
||||
let writtenContent = null;
|
||||
const agentArgs = [
|
||||
'--manifest', 'manifest.json',
|
||||
'--agent-id', 'a1',
|
||||
'--path', '/repo/.claude/worktrees/agent-a1',
|
||||
'--branch', 'agent-a1',
|
||||
'--base', 'abc123',
|
||||
];
|
||||
const result = cmdWorktreeRecordAgent('/repo/main', agentArgs, {
|
||||
readFile: () => '{"orchestrator_root":"/repo/main","worktrees":[]}',
|
||||
writeFile: (_p, c) => { writtenContent = c; },
|
||||
write: () => {},
|
||||
writeErr: () => {},
|
||||
});
|
||||
assert.equal(result.ok, true);
|
||||
const written = JSON.parse(writtenContent);
|
||||
assert.equal(written.worktrees[0].branch, 'agent-a1');
|
||||
});
|
||||
|
||||
test('exits 2 with usage when --manifest is missing', () => {
|
||||
withExitCode(() => {
|
||||
const errs = [];
|
||||
@@ -2620,10 +2725,10 @@ describe('bug #3384: adjacent worktree data-loss guards', () => {
|
||||
|
||||
const fragmentSource = fs.readFileSync(WORKTREE_BRANCH_CHECK_FRAGMENT, 'utf8');
|
||||
const branchCheck = fragmentSource.indexOf('HEAD_REF=$(git symbolic-ref --quiet HEAD || echo');
|
||||
const namespaceCheck = fragmentSource.indexOf('^worktree-agent-');
|
||||
const namespaceCheck = fragmentSource.indexOf('(worktree-)?agent-');
|
||||
|
||||
assert.ok(branchCheck > 0, 'canonical fragment must assert HEAD before any work');
|
||||
assert.ok(namespaceCheck > branchCheck, 'canonical fragment must require disposable worktree-agent branch');
|
||||
assert.ok(namespaceCheck > branchCheck, 'canonical fragment must require disposable agent/worktree-agent branch');
|
||||
// #48: verify-only — the destructive self-recovery is gone; the fragment fails closed instead.
|
||||
assert.ok(!fragmentSource.includes('git reset --hard {EXPECTED_BASE}'), 'canonical fragment must not self-recover via reset --hard — orchestrator owns recovery (#48)');
|
||||
assert.ok(fragmentSource.includes('exit 42'), 'canonical fragment must fail closed with exit 42 on base mismatch (#48)');
|
||||
|
||||
Reference in New Issue
Block a user