Skip to content

Commit 0e215fe

Browse files
committed
fix(delegation-gate): gate council APPROVE advancement on councilActive + add missing test cases
- convene_council APPROVE now only advances state when isCouncilGateActive=true; previously the advancement fired even when plan.json was missing - isCouncilGateActive: distinguish I/O errors (EACCES/EBUSY) from missing profile (ENOENT/SQLITE_CANTOPEN) with an explicit console.warn for unexpected errors - Add 5 missing test cases: race condition (APPROVE then reviewer Task stays complete), early APPROVE when task not at pre_check_passed, plan.json missing → Stage B fallback, plan.json missing + convene_council → no state advance
1 parent 52dbe56 commit 0e215fe

4 files changed

Lines changed: 167 additions & 3 deletions

File tree

dist/index.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24628,7 +24628,7 @@ function createDelegationGateHook(config2, directory) {
2462824628
verdict: result.overallVerdict,
2462924629
roundNumber: typeof result.roundNumber === "number" ? result.roundNumber : 1
2463024630
});
24631-
if (result.overallVerdict === "APPROVE" && result.allCriteriaMet === true && (result.requiredFixesCount ?? 0) === 0) {
24631+
if (councilActive && result.overallVerdict === "APPROVE" && result.allCriteriaMet === true && (result.requiredFixesCount ?? 0) === 0) {
2463224632
try {
2463324633
advanceTaskState(session, taskId, "complete");
2463424634
} catch (err2) {
@@ -25485,7 +25485,11 @@ async function isCouncilGateActive(directory, council) {
2548525485
let profile = null;
2548625486
try {
2548725487
profile = getProfile(directory, planId);
25488-
} catch {
25488+
} catch (err2) {
25489+
const code = err2?.code;
25490+
if (code && code !== "ENOENT" && code !== "SQLITE_CANTOPEN") {
25491+
console.warn(`[isCouncilGateActive] getProfile failed for plan ${planId}: ${code}. Treating council as inactive.`);
25492+
}
2548925493
profile = null;
2549025494
}
2549125495
if (!profile) {

src/hooks/delegation-gate.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -645,6 +645,7 @@ export function createDelegationGateHook(
645645
typeof result.roundNumber === 'number' ? result.roundNumber : 1,
646646
});
647647
if (
648+
councilActive &&
648649
result.overallVerdict === 'APPROVE' &&
649650
result.allCriteriaMet === true &&
650651
(result.requiredFixesCount ?? 0) === 0

src/state.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -943,7 +943,15 @@ export async function isCouncilGateActive(
943943
let profile: ReturnType<typeof getProfile> | null = null;
944944
try {
945945
profile = getProfile(directory, planId);
946-
} catch {
946+
} catch (err) {
947+
// Distinguish a missing-profile (expected in fresh repos) from an I/O error
948+
// (EACCES, EBUSY, etc.) that may indicate a real problem.
949+
const code = (err as NodeJS.ErrnoException)?.code;
950+
if (code && code !== 'ENOENT' && code !== 'SQLITE_CANTOPEN') {
951+
console.warn(
952+
`[isCouncilGateActive] getProfile failed for plan ${planId}: ${code}. Treating council as inactive.`,
953+
);
954+
}
947955
profile = null;
948956
}
949957
if (!profile) {

tests/unit/hooks/delegation-gate-council.test.ts

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,4 +486,155 @@ describe('delegation-gate council wiring (Stage B suppression + APPROVE fast-pat
486486
).toBe(true);
487487
});
488488
});
489+
490+
describe('race condition: concurrent APPROVE + reviewer Task on same taskId', () => {
491+
it('council APPROVE then reviewer Task — state stays complete, not overwritten by reviewer_run', async () => {
492+
writePlan();
493+
enableCouncilGate();
494+
495+
const config = makeConfig(undefined, { enabled: true });
496+
const hook = createDelegationGateHook(config, tmpDir);
497+
498+
startAgentSession('sess-race', 'architect');
499+
const session = ensureAgentSession('sess-race');
500+
advanceTaskState(session, '1.1', 'coder_delegated');
501+
advanceTaskState(session, '1.1', 'pre_check_passed');
502+
503+
// Step 1: council APPROVE — should advance to complete.
504+
await hook.toolAfter(
505+
{
506+
tool: 'convene_council',
507+
sessionID: 'sess-race',
508+
callID: 'call-cc-race',
509+
args: { taskId: '1.1' },
510+
},
511+
{
512+
success: true,
513+
overallVerdict: 'APPROVE',
514+
allCriteriaMet: true,
515+
requiredFixesCount: 0,
516+
roundNumber: 1,
517+
},
518+
);
519+
520+
expect(getTaskState(session, '1.1')).toBe('complete');
521+
522+
// Step 2: a reviewer Task delegation arrives immediately after (late dispatch).
523+
// With councilActive=true the reviewer branch is suppressed — state must remain complete.
524+
await hook.toolAfter(
525+
{
526+
tool: 'Task',
527+
sessionID: 'sess-race',
528+
callID: 'call-race-rev',
529+
args: { subagent_type: 'reviewer' },
530+
},
531+
{},
532+
);
533+
534+
expect(getTaskState(session, '1.1')).toBe('complete');
535+
});
536+
});
537+
538+
describe('edge cases: task not at pre_check_passed when APPROVE arrives', () => {
539+
it('APPROVE when task is at coder_delegated (pre-check not done) does NOT advance to complete', async () => {
540+
writePlan();
541+
enableCouncilGate();
542+
543+
const config = makeConfig(undefined, { enabled: true });
544+
const hook = createDelegationGateHook(config, tmpDir);
545+
546+
startAgentSession('sess-early', 'architect');
547+
const session = ensureAgentSession('sess-early');
548+
advanceTaskState(session, '1.1', 'coder_delegated');
549+
// NOTE: do NOT advance to pre_check_passed — council arrives too early.
550+
551+
const warnings: string[] = [];
552+
const origWarn = console.warn;
553+
console.warn = (...args: unknown[]) => {
554+
warnings.push(args.map(String).join(' '));
555+
};
556+
try {
557+
await hook.toolAfter(
558+
{
559+
tool: 'convene_council',
560+
sessionID: 'sess-early',
561+
callID: 'call-cc-early',
562+
args: { taskId: '1.1' },
563+
},
564+
{
565+
success: true,
566+
overallVerdict: 'APPROVE',
567+
allCriteriaMet: true,
568+
requiredFixesCount: 0,
569+
roundNumber: 1,
570+
},
571+
);
572+
} finally {
573+
console.warn = origWarn;
574+
}
575+
576+
// Must NOT be complete; pre-check has not passed.
577+
expect(getTaskState(session, '1.1')).not.toBe('complete');
578+
// Verdict IS recorded for observability.
579+
expect(session.taskCouncilApproved?.get('1.1')?.verdict).toBe('APPROVE');
580+
});
581+
});
582+
583+
describe('isCouncilGateActive: graceful fallback when plan.json missing', () => {
584+
it('returns false (council not active) when plan.json is absent', async () => {
585+
// Deliberately do NOT call writePlan() — no plan.json exists.
586+
enableCouncilGate();
587+
588+
const config = makeConfig(undefined, { enabled: true });
589+
const hook = createDelegationGateHook(config, tmpDir);
590+
591+
startAgentSession('sess-no-plan', 'architect');
592+
const session = ensureAgentSession('sess-no-plan');
593+
session.currentTaskId = '1.1';
594+
session.taskWorkflowStates.set('1.1', 'coder_delegated');
595+
596+
// Reviewer Task: if council correctly falls back to inactive → Stage B advances.
597+
await hook.toolAfter(
598+
{
599+
tool: 'Task',
600+
sessionID: 'sess-no-plan',
601+
callID: 'call-no-plan-rev',
602+
args: { subagent_type: 'reviewer' },
603+
},
604+
{},
605+
);
606+
607+
expect(getTaskState(session, '1.1')).toBe('reviewer_run');
608+
});
609+
610+
it('convene_council with missing plan.json logs warn and does not advance', async () => {
611+
// No plan written — isCouncilGateActive returns false.
612+
const config = makeConfig(undefined, { enabled: true });
613+
const hook = createDelegationGateHook(config, tmpDir);
614+
615+
startAgentSession('sess-no-plan-cc', 'architect');
616+
const session = ensureAgentSession('sess-no-plan-cc');
617+
session.currentTaskId = '1.1';
618+
session.taskWorkflowStates.set('1.1', 'pre_check_passed');
619+
620+
await hook.toolAfter(
621+
{
622+
tool: 'convene_council',
623+
sessionID: 'sess-no-plan-cc',
624+
callID: 'call-no-plan-cc',
625+
args: { taskId: '1.1' },
626+
},
627+
{
628+
success: true,
629+
overallVerdict: 'APPROVE',
630+
allCriteriaMet: true,
631+
requiredFixesCount: 0,
632+
roundNumber: 1,
633+
},
634+
);
635+
636+
// Council not active → verdict recorded but state NOT advanced.
637+
expect(getTaskState(session, '1.1')).toBe('pre_check_passed');
638+
});
639+
});
489640
});

0 commit comments

Comments
 (0)