refactor: simplify paired review system and add reviewer container isolation
Phase 1 — Strip over-engineering (-3,665 lines): - DB tables: 7 → 3 (remove executions, approvals, artifacts, events) - Task statuses: 11 → 5 (active, review_ready, in_review, merge_ready, completed) - Remove plan governance, event sourcing, gate/verdict, freshness guards - paired-execution-context: 980 → 299 lines - Session commands: remove /risk, /plan, /approve-plan, /request-plan-changes Phase 2 — Container isolation for reviewers: - Add container-runtime.ts (Docker abstraction) - Add credential-proxy.ts (API key injection without container exposure) - Add container-runner.ts (reviewer-specific read-only mount + tmpfs) - Add container/Dockerfile + agent-runner (Chromium, Claude Code, Codex) - agent-runner.ts: auto-route reviewer execution to container mode
This commit is contained in:
@@ -37,28 +37,6 @@ describe('extractSessionCommand', () => {
|
||||
expect(extractSessionCommand('/review-ready', trigger)).toBe('/review');
|
||||
});
|
||||
|
||||
it('detects /risk with an argument payload', () => {
|
||||
expect(extractSessionCommand('/risk high', trigger)).toBe('/risk');
|
||||
});
|
||||
|
||||
it('detects /plan with structured payload', () => {
|
||||
expect(
|
||||
extractSessionCommand('/plan brief || criteria || risk', trigger),
|
||||
).toBe('/plan');
|
||||
});
|
||||
|
||||
it('detects /approve-plan', () => {
|
||||
expect(extractSessionCommand('/approve-plan', trigger)).toBe(
|
||||
'/approve-plan',
|
||||
);
|
||||
});
|
||||
|
||||
it('detects /request-plan-changes with an optional note', () => {
|
||||
expect(
|
||||
extractSessionCommand('/request-plan-changes tighten scope', trigger),
|
||||
).toBe('/request-plan-changes');
|
||||
});
|
||||
|
||||
it('rejects /compact with extra text', () => {
|
||||
expect(extractSessionCommand('/compact now please', trigger)).toBeNull();
|
||||
});
|
||||
@@ -198,11 +176,6 @@ function makeDeps(
|
||||
isAdminSender: vi.fn().mockReturnValue(false),
|
||||
canSenderInteract: vi.fn().mockReturnValue(true),
|
||||
markReviewReady: vi.fn().mockResolvedValue('Review snapshot updated.'),
|
||||
finalizeDeployment: vi.fn().mockResolvedValue('Deployment finalized.'),
|
||||
setTaskRiskLevel: vi.fn().mockResolvedValue('Task risk updated.'),
|
||||
recordPlan: vi.fn().mockResolvedValue('Plan recorded.'),
|
||||
approvePlan: vi.fn().mockResolvedValue('Plan approved.'),
|
||||
requestPlanChanges: vi.fn().mockResolvedValue('Plan changes requested.'),
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
@@ -361,127 +334,6 @@ describe('handleSessionCommand', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('handles authorized /risk without invoking the agent', async () => {
|
||||
const deps = makeDeps({
|
||||
setTaskRiskLevel: vi
|
||||
.fn()
|
||||
.mockResolvedValue('Task risk updated.\n- Task: paired-task-1'),
|
||||
});
|
||||
|
||||
const result = await handleSessionCommand({
|
||||
missedMessages: [makeMsg('/risk high')],
|
||||
isMainGroup: true,
|
||||
groupName: 'test',
|
||||
triggerPattern: trigger,
|
||||
timezone: 'UTC',
|
||||
deps,
|
||||
});
|
||||
|
||||
expect(result).toEqual({ handled: true, success: true });
|
||||
expect(deps.setTaskRiskLevel).toHaveBeenCalledWith('high', 'msg-1');
|
||||
expect(deps.runAgent).not.toHaveBeenCalled();
|
||||
expect(deps.sendMessage).toHaveBeenCalledWith(
|
||||
'Task risk updated.\n- Task: paired-task-1',
|
||||
);
|
||||
});
|
||||
|
||||
it('handles authorized /plan without invoking the agent', async () => {
|
||||
const deps = makeDeps({
|
||||
recordPlan: vi
|
||||
.fn()
|
||||
.mockResolvedValue('Plan recorded.\n- Task: paired-task-1'),
|
||||
});
|
||||
|
||||
const result = await handleSessionCommand({
|
||||
missedMessages: [
|
||||
makeMsg('/plan brief || acceptance criteria || rollout risk'),
|
||||
],
|
||||
isMainGroup: true,
|
||||
groupName: 'test',
|
||||
triggerPattern: trigger,
|
||||
timezone: 'UTC',
|
||||
deps,
|
||||
});
|
||||
|
||||
expect(result).toEqual({ handled: true, success: true });
|
||||
expect(deps.recordPlan).toHaveBeenCalledWith({
|
||||
planBrief: 'brief',
|
||||
acceptanceCriteria: 'acceptance criteria',
|
||||
riskSummary: 'rollout risk',
|
||||
}, 'msg-1');
|
||||
expect(deps.sendMessage).toHaveBeenCalledWith(
|
||||
'Plan recorded.\n- Task: paired-task-1',
|
||||
);
|
||||
});
|
||||
|
||||
it('sends /plan usage when structured payload is incomplete', async () => {
|
||||
const deps = makeDeps();
|
||||
|
||||
const result = await handleSessionCommand({
|
||||
missedMessages: [makeMsg('/plan only brief')],
|
||||
isMainGroup: true,
|
||||
groupName: 'test',
|
||||
triggerPattern: trigger,
|
||||
timezone: 'UTC',
|
||||
deps,
|
||||
});
|
||||
|
||||
expect(result).toEqual({ handled: true, success: true });
|
||||
expect(deps.recordPlan).not.toHaveBeenCalled();
|
||||
expect(deps.sendMessage).toHaveBeenCalledWith(
|
||||
'Usage: /plan <plan brief> || <acceptance criteria> || <risk summary>',
|
||||
);
|
||||
});
|
||||
|
||||
it('handles /approve-plan without invoking the agent', async () => {
|
||||
const deps = makeDeps({
|
||||
approvePlan: vi
|
||||
.fn()
|
||||
.mockResolvedValue('Plan approved.\n- Task: paired-task-1'),
|
||||
});
|
||||
|
||||
const result = await handleSessionCommand({
|
||||
missedMessages: [makeMsg('/approve-plan')],
|
||||
isMainGroup: true,
|
||||
groupName: 'test',
|
||||
triggerPattern: trigger,
|
||||
timezone: 'UTC',
|
||||
deps,
|
||||
});
|
||||
|
||||
expect(result).toEqual({ handled: true, success: true });
|
||||
expect(deps.approvePlan).toHaveBeenCalledWith('msg-1');
|
||||
expect(deps.sendMessage).toHaveBeenCalledWith(
|
||||
'Plan approved.\n- Task: paired-task-1',
|
||||
);
|
||||
});
|
||||
|
||||
it('handles /request-plan-changes without invoking the agent', async () => {
|
||||
const deps = makeDeps({
|
||||
requestPlanChanges: vi
|
||||
.fn()
|
||||
.mockResolvedValue('Plan changes requested.\n- Task: paired-task-1'),
|
||||
});
|
||||
|
||||
const result = await handleSessionCommand({
|
||||
missedMessages: [makeMsg('/request-plan-changes tighten scope')],
|
||||
isMainGroup: true,
|
||||
groupName: 'test',
|
||||
triggerPattern: trigger,
|
||||
timezone: 'UTC',
|
||||
deps,
|
||||
});
|
||||
|
||||
expect(result).toEqual({ handled: true, success: true });
|
||||
expect(deps.requestPlanChanges).toHaveBeenCalledWith(
|
||||
'tighten scope',
|
||||
'msg-1',
|
||||
);
|
||||
expect(deps.sendMessage).toHaveBeenCalledWith(
|
||||
'Plan changes requested.\n- Task: paired-task-1',
|
||||
);
|
||||
});
|
||||
|
||||
it('sends denial to interactable sender in non-main group', async () => {
|
||||
const deps = makeDeps();
|
||||
const result = await handleSessionCommand({
|
||||
|
||||
Reference in New Issue
Block a user