feat: add multi-reviewer human gate (review-policy routing)
Implements the §14 Phase 3 review gate. requestReview() routes a building job into the review stage (fencing any worker), carrying a normalized policy (requiredApprovals + reviewer allowlist) and clearing prior decisions. submitReview() records one decision per reviewer (last-write-wins, identity- normalized), advances the job to testing once distinct approvals reach the quorum, and treats any reject as a veto that returns the job to queued for rework. Adds POST /fleet/jobs/:id/review/request and POST /fleet/jobs/:id/review, a typed client, and a review-gate card on the job-detail page. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Saravanakumar D
parent
c9c2c174db
commit
4ac499f301
@ -26,6 +26,14 @@ type Job = {
|
||||
leaseEpoch: number;
|
||||
createdAt: string;
|
||||
updatedAt: string;
|
||||
reviewPolicy?: { requiredApprovals: number; reviewers: string[] };
|
||||
reviewDecisions?: {
|
||||
reviewer: string;
|
||||
decision: 'approve' | 'reject';
|
||||
at: string;
|
||||
note?: string;
|
||||
}[];
|
||||
gate?: string;
|
||||
};
|
||||
|
||||
function makeJob(partial: Partial<Job> & { id: string; idempotencyKey: string }): Job {
|
||||
@ -83,8 +91,22 @@ async function mockFleet(
|
||||
const state = {
|
||||
jobStage: opts?.jobStage ?? 'building',
|
||||
budgetStatus: opts?.budgetStatus ?? ('active' as 'active' | 'paused'),
|
||||
reviewDecisions: [] as { reviewer: string; decision: 'approve' | 'reject'; at: string }[],
|
||||
};
|
||||
|
||||
const jobView = () =>
|
||||
makeJob({
|
||||
id: 'job-1',
|
||||
idempotencyKey: 'feat-x',
|
||||
stage: state.jobStage,
|
||||
...(state.jobStage === 'review'
|
||||
? {
|
||||
reviewPolicy: { requiredApprovals: 2, reviewers: ['admin@example.com', 'bob@x.com'] },
|
||||
reviewDecisions: state.reviewDecisions,
|
||||
}
|
||||
: {}),
|
||||
});
|
||||
|
||||
const budget = () => ({
|
||||
id: PRODUCT,
|
||||
productId: PRODUCT,
|
||||
@ -145,6 +167,32 @@ async function mockFleet(
|
||||
});
|
||||
}
|
||||
|
||||
// ── Review gate (multi-reviewer) ──
|
||||
if (path.endsWith('/review/request') && method === 'POST') {
|
||||
state.jobStage = 'review';
|
||||
state.reviewDecisions = [];
|
||||
return route.fulfill({ json: jobView() });
|
||||
}
|
||||
if (path.endsWith('/review') && method === 'POST') {
|
||||
const payload = req.postDataJSON() as {
|
||||
reviewer: string;
|
||||
decision: 'approve' | 'reject';
|
||||
};
|
||||
if (payload.decision === 'reject') {
|
||||
state.jobStage = 'queued';
|
||||
return route.fulfill({ json: { ...jobView(), gate: 'rejected' } });
|
||||
}
|
||||
state.reviewDecisions = state.reviewDecisions
|
||||
.filter(d => d.reviewer !== payload.reviewer)
|
||||
.concat({ reviewer: payload.reviewer, decision: 'approve', at: ISO });
|
||||
const approvals = new Set(
|
||||
state.reviewDecisions.filter(d => d.decision === 'approve').map(d => d.reviewer)
|
||||
).size;
|
||||
const gate = approvals >= 2 ? 'approved' : 'pending';
|
||||
if (gate === 'approved') state.jobStage = 'testing';
|
||||
return route.fulfill({ json: { ...jobView(), gate } });
|
||||
}
|
||||
|
||||
// ── Budgets ──
|
||||
if (path.endsWith('/burndown')) return route.fulfill({ status: 404, json: {} });
|
||||
if (path.endsWith('/pause') && method === 'POST') {
|
||||
@ -159,14 +207,10 @@ async function mockFleet(
|
||||
|
||||
// ── Jobs ──
|
||||
if (path.match(/\/jobs\/[^/]+$/) && method === 'GET') {
|
||||
return route.fulfill({
|
||||
json: makeJob({ id: 'job-1', idempotencyKey: 'feat-x', stage: state.jobStage }),
|
||||
});
|
||||
return route.fulfill({ json: jobView() });
|
||||
}
|
||||
if (path.endsWith('/jobs') && method === 'GET') {
|
||||
return route.fulfill({
|
||||
json: { jobs: [makeJob({ id: 'job-1', idempotencyKey: 'feat-x', stage: state.jobStage })] },
|
||||
});
|
||||
return route.fulfill({ json: { jobs: [jobView()] } });
|
||||
}
|
||||
if (path.endsWith('/factories')) return route.fulfill({ json: { factories: [FACTORY] } });
|
||||
|
||||
@ -252,6 +296,24 @@ test.describe('Fleet — Job detail', () => {
|
||||
});
|
||||
});
|
||||
|
||||
test.describe('Fleet — Review gate', () => {
|
||||
test('routes a building job to review and approves through the gate', async ({ page }) => {
|
||||
await authenticate(page);
|
||||
await mockFleet(page, { jobStage: 'building' });
|
||||
await page.goto('/dashboard/fleet/jobs/job-1');
|
||||
|
||||
await expect(page.getByRole('heading', { name: 'feat-x' })).toBeVisible();
|
||||
// Send the building job to review.
|
||||
await page.getByRole('button', { name: 'Send this job to review' }).click();
|
||||
await expect(page.getByTestId('review-gate')).toBeVisible();
|
||||
await expect(page.getByTestId('review-progress')).toHaveText('0 / 2 approvals');
|
||||
|
||||
// First approval (admin@example.com) keeps the gate pending at 1/2.
|
||||
await page.getByRole('button', { name: 'Approve this job' }).click();
|
||||
await expect(page.getByTestId('review-progress')).toHaveText('1 / 2 approvals');
|
||||
});
|
||||
});
|
||||
|
||||
test.describe('Fleet — Budget', () => {
|
||||
test('pauses and resumes the budget', async ({ page }) => {
|
||||
await authenticate(page);
|
||||
|
||||
@ -16,6 +16,8 @@ import {
|
||||
getJob,
|
||||
patchJob,
|
||||
operatorAction,
|
||||
requestReview,
|
||||
submitReview,
|
||||
getJobRuns,
|
||||
getJobEvents,
|
||||
getJobArtifacts,
|
||||
@ -103,6 +105,43 @@ describe('fleet-client', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('review gate', () => {
|
||||
it('requestReview POSTs the policy to /jobs/:id/review/request', async () => {
|
||||
fetchSpy.mockResolvedValue({ id: 'j1', stage: 'review' });
|
||||
const res = await requestReview('j1', { requiredApprovals: 2, reviewers: ['a@x.com'] });
|
||||
expect(res.stage).toBe('review');
|
||||
expect(fetchSpy).toHaveBeenCalledWith(
|
||||
'/jobs/j1/review/request',
|
||||
expect.objectContaining({
|
||||
method: 'POST',
|
||||
body: JSON.stringify({ requiredApprovals: 2, reviewers: ['a@x.com'] }),
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
it('requestReview sends an empty object when no policy given', async () => {
|
||||
fetchSpy.mockResolvedValue({ id: 'j1', stage: 'review' });
|
||||
await requestReview('j1');
|
||||
expect(fetchSpy).toHaveBeenCalledWith(
|
||||
'/jobs/j1/review/request',
|
||||
expect.objectContaining({ method: 'POST', body: '{}' })
|
||||
);
|
||||
});
|
||||
|
||||
it('submitReview POSTs the decision to /jobs/:id/review', async () => {
|
||||
fetchSpy.mockResolvedValue({ id: 'j1', stage: 'testing', gate: 'approved' });
|
||||
const res = await submitReview('j1', { reviewer: 'a@x.com', decision: 'approve' });
|
||||
expect(res.gate).toBe('approved');
|
||||
expect(fetchSpy).toHaveBeenCalledWith(
|
||||
'/jobs/j1/review',
|
||||
expect.objectContaining({
|
||||
method: 'POST',
|
||||
body: JSON.stringify({ reviewer: 'a@x.com', decision: 'approve' }),
|
||||
})
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getJobRuns', () => {
|
||||
it('returns runs array', async () => {
|
||||
fetchSpy.mockResolvedValue({ runs: [{ id: 'r1', attempt: 1 }] });
|
||||
|
||||
@ -15,6 +15,8 @@ import {
|
||||
getJobExplain,
|
||||
patchJob,
|
||||
operatorAction,
|
||||
requestReview,
|
||||
submitReview,
|
||||
subscribeJobEvents,
|
||||
type OperatorAction,
|
||||
type FleetJob,
|
||||
@ -26,7 +28,7 @@ import {
|
||||
} from '@/lib/fleet-client';
|
||||
|
||||
export default function FleetJobDetailPage() {
|
||||
const { token } = useAuth();
|
||||
const { token, user } = useAuth();
|
||||
const params = useParams();
|
||||
const jobId = params.id as string;
|
||||
|
||||
@ -39,6 +41,7 @@ export default function FleetJobDetailPage() {
|
||||
const [loading, setLoading] = useState(true);
|
||||
const [shipping, setShipping] = useState(false);
|
||||
const [acting, setActing] = useState<OperatorAction | null>(null);
|
||||
const [reviewing, setReviewing] = useState(false);
|
||||
const [live, setLive] = useState(false);
|
||||
|
||||
const refresh = useCallback(async () => {
|
||||
@ -135,6 +138,34 @@ export default function FleetJobDetailPage() {
|
||||
}
|
||||
};
|
||||
|
||||
const handleRequestReview = async () => {
|
||||
if (!job) return;
|
||||
setReviewing(true);
|
||||
try {
|
||||
const updated = await requestReview(jobId);
|
||||
setJob(updated);
|
||||
await refresh();
|
||||
} catch {
|
||||
/* show error in production */
|
||||
} finally {
|
||||
setReviewing(false);
|
||||
}
|
||||
};
|
||||
|
||||
const handleReview = async (decision: 'approve' | 'reject') => {
|
||||
if (!job || !user) return;
|
||||
setReviewing(true);
|
||||
try {
|
||||
const updated = await submitReview(jobId, { reviewer: user.email, decision });
|
||||
setJob(updated);
|
||||
await refresh();
|
||||
} catch {
|
||||
/* show error in production */
|
||||
} finally {
|
||||
setReviewing(false);
|
||||
}
|
||||
};
|
||||
|
||||
if (loading) {
|
||||
return (
|
||||
<div className="p-6">
|
||||
@ -166,6 +197,16 @@ export default function FleetJobDetailPage() {
|
||||
{shipping ? 'Shipping...' : 'Ship ✓'}
|
||||
</Button>
|
||||
)}
|
||||
{job.stage === 'building' && (
|
||||
<Button
|
||||
variant="secondary"
|
||||
onClick={handleRequestReview}
|
||||
disabled={reviewing}
|
||||
aria-label="Send this job to review"
|
||||
>
|
||||
{reviewing ? 'Sending...' : 'Send to review'}
|
||||
</Button>
|
||||
)}
|
||||
{job.stage !== 'shipped' && (
|
||||
<Button
|
||||
variant="secondary"
|
||||
@ -207,6 +248,16 @@ export default function FleetJobDetailPage() {
|
||||
<MetaCard label="Attempts" value={String(job.attempts)} />
|
||||
</section>
|
||||
|
||||
{/* Review gate (multi-reviewer human gate) */}
|
||||
{job.stage === 'review' && (
|
||||
<ReviewGateCard
|
||||
job={job}
|
||||
reviewing={reviewing}
|
||||
onApprove={() => handleReview('approve')}
|
||||
onReject={() => handleReview('reject')}
|
||||
/>
|
||||
)}
|
||||
|
||||
{/* DAG subtree (if present) */}
|
||||
{dag && dag.children.length > 0 && (
|
||||
<section>
|
||||
@ -321,6 +372,74 @@ function MetaCard({ label, value }: { label: string; value: string }) {
|
||||
);
|
||||
}
|
||||
|
||||
function ReviewGateCard({
|
||||
job,
|
||||
reviewing,
|
||||
onApprove,
|
||||
onReject,
|
||||
}: {
|
||||
job: FleetJob;
|
||||
reviewing: boolean;
|
||||
onApprove: () => void;
|
||||
onReject: () => void;
|
||||
}) {
|
||||
const decisions = job.reviewDecisions ?? [];
|
||||
const required = job.reviewPolicy?.requiredApprovals ?? 1;
|
||||
const reviewers = job.reviewPolicy?.reviewers ?? [];
|
||||
const approvals = new Set(decisions.filter(d => d.decision === 'approve').map(d => d.reviewer))
|
||||
.size;
|
||||
|
||||
return (
|
||||
<section
|
||||
className="rounded-lg border border-purple-500/40 bg-purple-500/5 p-4 space-y-3"
|
||||
aria-label="Review gate"
|
||||
data-testid="review-gate"
|
||||
>
|
||||
<div className="flex items-center justify-between">
|
||||
<h2 className="text-lg font-semibold">Review gate</h2>
|
||||
<span className="text-sm tabular-nums" data-testid="review-progress">
|
||||
{approvals} / {required} approvals
|
||||
</span>
|
||||
</div>
|
||||
{reviewers.length > 0 && (
|
||||
<p className="text-xs text-muted-foreground">Reviewers: {reviewers.join(', ')}</p>
|
||||
)}
|
||||
{decisions.length > 0 && (
|
||||
<ul className="space-y-1 text-sm" aria-label="Review decisions">
|
||||
{decisions.map(d => (
|
||||
<li key={d.reviewer} className="flex items-center gap-2">
|
||||
<span
|
||||
className={`inline-flex items-center rounded-full px-2 py-0.5 text-xs font-medium ${
|
||||
d.decision === 'approve'
|
||||
? 'bg-green-500/20 text-green-700 dark:text-green-400'
|
||||
: 'bg-red-500/20 text-red-700 dark:text-red-400'
|
||||
}`}
|
||||
>
|
||||
{d.decision}
|
||||
</span>
|
||||
<span className="font-mono text-xs">{d.reviewer}</span>
|
||||
{d.note && <span className="text-xs text-muted-foreground">— {d.note}</span>}
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
)}
|
||||
<div className="flex gap-2">
|
||||
<Button onClick={onApprove} disabled={reviewing} aria-label="Approve this job">
|
||||
{reviewing ? 'Submitting...' : 'Approve'}
|
||||
</Button>
|
||||
<Button
|
||||
variant="destructive"
|
||||
onClick={onReject}
|
||||
disabled={reviewing}
|
||||
aria-label="Reject this job in review"
|
||||
>
|
||||
{reviewing ? 'Submitting...' : 'Request changes'}
|
||||
</Button>
|
||||
</div>
|
||||
</section>
|
||||
);
|
||||
}
|
||||
|
||||
function DagTree({ node, depth = 0 }: { node: DagNode; depth?: number }) {
|
||||
return (
|
||||
<div className={`${depth > 0 ? 'ml-4 border-l pl-3' : ''}`}>
|
||||
|
||||
@ -23,6 +23,8 @@ export interface FleetJob {
|
||||
leaseEpoch: number;
|
||||
createdAt: string;
|
||||
updatedAt: string;
|
||||
reviewPolicy?: ReviewPolicy;
|
||||
reviewDecisions?: ReviewDecision[];
|
||||
}
|
||||
|
||||
export interface FleetFactory {
|
||||
@ -192,6 +194,44 @@ export async function operatorAction(id: string, action: OperatorAction): Promis
|
||||
return apiFetch(`/jobs/${id}/actions/${action}`, { method: 'POST' });
|
||||
}
|
||||
|
||||
// ── Multi-reviewer human gate ─────────────────────────────────────────────────
|
||||
|
||||
export interface ReviewPolicy {
|
||||
requiredApprovals: number;
|
||||
reviewers: string[];
|
||||
}
|
||||
|
||||
export interface ReviewDecision {
|
||||
reviewer: string;
|
||||
decision: 'approve' | 'reject';
|
||||
at: string;
|
||||
note?: string;
|
||||
}
|
||||
|
||||
export type ReviewGate = 'pending' | 'approved' | 'rejected';
|
||||
|
||||
/** Route a building job into the review gate with an optional policy. */
|
||||
export async function requestReview(
|
||||
id: string,
|
||||
policy?: { requiredApprovals?: number; reviewers?: string[] }
|
||||
): Promise<FleetJob> {
|
||||
return apiFetch(`/jobs/${id}/review/request`, {
|
||||
method: 'POST',
|
||||
body: JSON.stringify(policy ?? {}),
|
||||
});
|
||||
}
|
||||
|
||||
/** Submit a single reviewer's approve/reject decision. */
|
||||
export async function submitReview(
|
||||
id: string,
|
||||
input: { reviewer: string; decision: 'approve' | 'reject'; note?: string }
|
||||
): Promise<FleetJob & { gate: ReviewGate }> {
|
||||
return apiFetch(`/jobs/${id}/review`, {
|
||||
method: 'POST',
|
||||
body: JSON.stringify(input),
|
||||
});
|
||||
}
|
||||
|
||||
export async function getJobRuns(jobId: string): Promise<{ runs: FleetRun[] }> {
|
||||
return apiFetch(`/jobs/${jobId}/runs`);
|
||||
}
|
||||
|
||||
@ -68,7 +68,13 @@
|
||||
derived alerts (`no_live_capacity`, `all_factories_down`, `queue_starvation`, `saturated`,
|
||||
`stale_factories`). Surfaced as a metrics+alerts panel on the fleet overview (`getFleetMetrics`).
|
||||
Files: `coordinator.ts`, `routes.ts`, `fleet-client.ts`, `dashboard/fleet/page.tsx` + tests + e2e.
|
||||
- [ ] **Multi-reviewer routing** — P3 — Phase-3 §14.
|
||||
- [x] **Multi-reviewer routing** — P3 — ✅ DONE — review-policy human gate (§14). `requestReview`
|
||||
routes a building job into `review` (fences worker); `submitReview` records per-reviewer
|
||||
approve/reject (last-write-wins, identity-normalized), advances to `testing` once distinct
|
||||
approvals reach the quorum, or vetoes any reject back to `queued` for rework. Routes:
|
||||
`POST /fleet/jobs/:id/review/request`, `POST /fleet/jobs/:id/review`. UI: review-gate card on
|
||||
job detail (`requestReview`/`submitReview`). Files: `types.ts`, `coordinator.ts`, `routes.ts`,
|
||||
`fleet-client.ts`, `dashboard/fleet/jobs/[id]/page.tsx` + coordinator/route/client tests + e2e.
|
||||
- [ ] **TUI re-point at `/fleet`** — P3 — Phase-3 §14.
|
||||
|
||||
### Phase 4 / 5 (post-MVP, tracked only)
|
||||
|
||||
@ -1034,4 +1034,120 @@ describe('fleet coordinator — Phase 3 per-product budgets', () => {
|
||||
expect(m.alerts.some(a => a.code === 'queue_starvation')).toBe(true);
|
||||
expect(m.alerts.some(a => a.code === 'saturated')).toBe(true);
|
||||
});
|
||||
|
||||
// ── MULTI-REVIEWER HUMAN GATE (§14 Phase 3) ──
|
||||
async function toBuilding(idempotencyKey = 'rev-1') {
|
||||
const { job } = await coord.submitJob(PID, input({ idempotencyKey }));
|
||||
const claim = await coord.claimNextJob(factory());
|
||||
await coord.patchJobFenced(job.id, PID, {
|
||||
leaseEpoch: claim!.job.leaseEpoch,
|
||||
stage: 'building',
|
||||
});
|
||||
return job;
|
||||
}
|
||||
|
||||
it('requestReview: only a building job can enter review; fences the worker', async () => {
|
||||
const job = await toBuilding();
|
||||
const before = await repo.getJob(job.id, PID);
|
||||
const res = await coord.requestReview(job.id, PID, { requiredApprovals: 1 });
|
||||
expect(res.ok).toBe(true);
|
||||
const after = await repo.getJob(job.id, PID);
|
||||
expect(after?.stage).toBe('review');
|
||||
expect(after?.leaseEpoch).toBe(before!.leaseEpoch + 1);
|
||||
expect((await repo.getLease(job.id))?.status).toBe('released');
|
||||
});
|
||||
|
||||
it('requestReview: a queued job cannot be routed to review', async () => {
|
||||
const { job } = await coord.submitJob(PID, input());
|
||||
const res = await coord.requestReview(job.id, PID);
|
||||
expect(res.ok).toBe(false);
|
||||
if (!res.ok) expect(res.reason).toBe('invalid_state');
|
||||
});
|
||||
|
||||
it('requestReview: rejects an unsatisfiable policy (quorum > allowlist size)', async () => {
|
||||
const job = await toBuilding();
|
||||
const res = await coord.requestReview(job.id, PID, {
|
||||
requiredApprovals: 2,
|
||||
reviewers: ['alice@x.com'],
|
||||
});
|
||||
expect(res.ok).toBe(false);
|
||||
if (!res.ok) expect(res.reason).toBe('invalid_policy');
|
||||
});
|
||||
|
||||
it('submitReview: single approval advances the job to testing', async () => {
|
||||
const job = await toBuilding();
|
||||
await coord.requestReview(job.id, PID, { requiredApprovals: 1 });
|
||||
const res = await coord.submitReview(job.id, PID, {
|
||||
reviewer: 'Alice@X.com',
|
||||
decision: 'approve',
|
||||
});
|
||||
expect(res.ok).toBe(true);
|
||||
if (res.ok) expect(res.gate).toBe('approved');
|
||||
const after = await repo.getJob(job.id, PID);
|
||||
expect(after?.stage).toBe('testing');
|
||||
expect(after?.reviewDecisions?.[0].reviewer).toBe('alice@x.com'); // normalized
|
||||
});
|
||||
|
||||
it('submitReview: quorum of 2 needs two distinct approvers', async () => {
|
||||
const job = await toBuilding();
|
||||
await coord.requestReview(job.id, PID, {
|
||||
requiredApprovals: 2,
|
||||
reviewers: ['alice@x.com', 'bob@x.com'],
|
||||
});
|
||||
const r1 = await coord.submitReview(job.id, PID, {
|
||||
reviewer: 'alice@x.com',
|
||||
decision: 'approve',
|
||||
});
|
||||
expect(r1.ok && r1.gate).toBe('pending');
|
||||
expect((await repo.getJob(job.id, PID))?.stage).toBe('review');
|
||||
// Same reviewer approving again does not satisfy the quorum.
|
||||
const dup = await coord.submitReview(job.id, PID, {
|
||||
reviewer: 'alice@x.com',
|
||||
decision: 'approve',
|
||||
});
|
||||
expect(dup.ok && dup.gate).toBe('pending');
|
||||
expect((await repo.getJob(job.id, PID))?.stage).toBe('review');
|
||||
const r2 = await coord.submitReview(job.id, PID, {
|
||||
reviewer: 'bob@x.com',
|
||||
decision: 'approve',
|
||||
});
|
||||
expect(r2.ok && r2.gate).toBe('approved');
|
||||
expect((await repo.getJob(job.id, PID))?.stage).toBe('testing');
|
||||
});
|
||||
|
||||
it('submitReview: any reject vetoes and returns the job to queued for rework', async () => {
|
||||
const job = await toBuilding();
|
||||
await coord.requestReview(job.id, PID, { requiredApprovals: 2 });
|
||||
const res = await coord.submitReview(job.id, PID, {
|
||||
reviewer: 'carol@x.com',
|
||||
decision: 'reject',
|
||||
note: 'needs tests',
|
||||
});
|
||||
expect(res.ok).toBe(true);
|
||||
if (res.ok) expect(res.gate).toBe('rejected');
|
||||
const after = await repo.getJob(job.id, PID);
|
||||
expect(after?.stage).toBe('queued');
|
||||
expect(after?.blockedReason).toContain('needs tests');
|
||||
});
|
||||
|
||||
it('submitReview: a non-allowlisted reviewer is rejected', async () => {
|
||||
const job = await toBuilding();
|
||||
await coord.requestReview(job.id, PID, { requiredApprovals: 1, reviewers: ['alice@x.com'] });
|
||||
const res = await coord.submitReview(job.id, PID, {
|
||||
reviewer: 'mallory@x.com',
|
||||
decision: 'approve',
|
||||
});
|
||||
expect(res.ok).toBe(false);
|
||||
if (!res.ok) expect(res.reason).toBe('not_authorized');
|
||||
});
|
||||
|
||||
it('submitReview: a job not in review cannot be reviewed', async () => {
|
||||
const { job } = await coord.submitJob(PID, input());
|
||||
const res = await coord.submitReview(job.id, PID, {
|
||||
reviewer: 'alice@x.com',
|
||||
decision: 'approve',
|
||||
});
|
||||
expect(res.ok).toBe(false);
|
||||
if (!res.ok) expect(res.reason).toBe('invalid_state');
|
||||
});
|
||||
});
|
||||
|
||||
@ -46,6 +46,9 @@ import {
|
||||
type FleetBudgetDoc,
|
||||
type BudgetWindow,
|
||||
type SubmitJobInput,
|
||||
type ReviewPolicy,
|
||||
type ReviewDecision,
|
||||
type ReviewDecisionKind,
|
||||
} from './types.js';
|
||||
|
||||
const CLAIM_MAX_RETRIES = 8;
|
||||
@ -913,7 +916,179 @@ export async function operatorAction(
|
||||
return { ok: true, doc: res.doc };
|
||||
}
|
||||
|
||||
// ── Scoring explainability (§7 / Phase 3 — "why does this job route here?") ────
|
||||
// ── Multi-reviewer human gate (§14 Phase 3 — review-policy routing) ────────────
|
||||
|
||||
/** Canonicalize a reviewer identity so allowlist + dedupe are case/space stable. */
|
||||
function normalizeReviewer(id: string): string {
|
||||
return id.trim().toLowerCase();
|
||||
}
|
||||
|
||||
export type RequestReviewResult =
|
||||
| { ok: true; doc: FleetJobDoc }
|
||||
| { ok: false; reason: 'not_found' | 'conflict' | 'invalid_state' | 'invalid_policy' };
|
||||
|
||||
export type SubmitReviewResult =
|
||||
| { ok: true; doc: FleetJobDoc; gate: 'pending' | 'approved' | 'rejected' }
|
||||
| {
|
||||
ok: false;
|
||||
reason: 'not_found' | 'conflict' | 'invalid_state' | 'not_authorized';
|
||||
};
|
||||
|
||||
/**
|
||||
* Route a job into the `review` human gate (§14). Only a `building` job can enter
|
||||
* review (its work product is ready to inspect). Like `operatorAction`, the
|
||||
* control plane holds no lease, so this bumps `leaseEpoch` to fence the building
|
||||
* worker and releases its lease. The policy is normalized + validated, and any
|
||||
* prior decisions are cleared so each review round starts fresh.
|
||||
*/
|
||||
export async function requestReview(
|
||||
jobId: string,
|
||||
productId: string,
|
||||
policy?: { requiredApprovals?: number; reviewers?: string[] }
|
||||
): Promise<RequestReviewResult> {
|
||||
const job = await repo.getJob(jobId, productId);
|
||||
if (!job) return { ok: false, reason: 'not_found' };
|
||||
if (job.stage !== 'building') return { ok: false, reason: 'invalid_state' };
|
||||
|
||||
const requiredApprovals = Math.max(1, Math.floor(policy?.requiredApprovals ?? 1));
|
||||
const reviewers = Array.from(
|
||||
new Set((policy?.reviewers ?? []).map(normalizeReviewer).filter(Boolean))
|
||||
);
|
||||
// An allowlist smaller than the approval quorum can never be satisfied.
|
||||
if (reviewers.length > 0 && requiredApprovals > reviewers.length) {
|
||||
return { ok: false, reason: 'invalid_policy' };
|
||||
}
|
||||
const reviewPolicy: ReviewPolicy = { requiredApprovals, reviewers };
|
||||
|
||||
const newEpoch = job.leaseEpoch + 1; // fence the building worker
|
||||
const res = await repo.revUpdateJob(jobId, productId, job.rev, {
|
||||
stage: 'review',
|
||||
leaseEpoch: newEpoch,
|
||||
reviewPolicy,
|
||||
reviewDecisions: [],
|
||||
blockedReason: undefined,
|
||||
});
|
||||
if (!res.ok) {
|
||||
return { ok: false, reason: res.reason === 'not_found' ? 'not_found' : 'conflict' };
|
||||
}
|
||||
|
||||
const lease = await repo.getLease(jobId);
|
||||
if (lease && lease.status === 'held') {
|
||||
await repo.revUpdateLease(jobId, lease.rev, {
|
||||
status: 'released',
|
||||
leaseEpoch: newEpoch,
|
||||
holderFactoryId: undefined,
|
||||
});
|
||||
}
|
||||
|
||||
await repo.appendEvent({
|
||||
jobId,
|
||||
productId,
|
||||
type: 'review_requested',
|
||||
actor: 'operator',
|
||||
data: { requiredApprovals, reviewers, leaseEpoch: newEpoch },
|
||||
});
|
||||
|
||||
return { ok: true, doc: res.doc };
|
||||
}
|
||||
|
||||
const REVIEW_MAX_RETRIES = 4;
|
||||
|
||||
/**
|
||||
* Record one reviewer's decision against a job in the `review` gate (§14):
|
||||
* - any `reject` is a veto → the job returns to `queued` for rework (fenced),
|
||||
* - once distinct approvals reach `requiredApprovals` → the job advances to
|
||||
* `testing`,
|
||||
* - otherwise the decision is recorded and the job stays in `review`.
|
||||
*
|
||||
* Decisions are upserted per reviewer (last-write-wins). The read-modify-write is
|
||||
* guarded by the job `rev` CAS; on a concurrent-writer conflict we re-read and
|
||||
* retry a bounded number of times before surfacing `conflict`.
|
||||
*/
|
||||
export async function submitReview(
|
||||
jobId: string,
|
||||
productId: string,
|
||||
input: { reviewer: string; decision: ReviewDecisionKind; note?: string }
|
||||
): Promise<SubmitReviewResult> {
|
||||
const reviewer = normalizeReviewer(input.reviewer);
|
||||
|
||||
for (let attempt = 0; attempt < REVIEW_MAX_RETRIES; attempt++) {
|
||||
const job = await repo.getJob(jobId, productId);
|
||||
if (!job) return { ok: false, reason: 'not_found' };
|
||||
if (job.stage !== 'review') return { ok: false, reason: 'invalid_state' };
|
||||
|
||||
const allowlist = job.reviewPolicy?.reviewers ?? [];
|
||||
if (allowlist.length > 0 && !allowlist.includes(reviewer)) {
|
||||
return { ok: false, reason: 'not_authorized' };
|
||||
}
|
||||
|
||||
const decision: ReviewDecision = {
|
||||
reviewer,
|
||||
decision: input.decision,
|
||||
at: new Date().toISOString(),
|
||||
...(input.note ? { note: input.note } : {}),
|
||||
};
|
||||
// Upsert: one current decision per reviewer (last-write-wins).
|
||||
const decisions = (job.reviewDecisions ?? [])
|
||||
.filter(d => d.reviewer !== reviewer)
|
||||
.concat(decision);
|
||||
|
||||
if (input.decision === 'reject') {
|
||||
const newEpoch = job.leaseEpoch + 1; // fence any holder before rework
|
||||
const res = await repo.revUpdateJob(jobId, productId, job.rev, {
|
||||
stage: 'queued',
|
||||
leaseEpoch: newEpoch,
|
||||
reviewDecisions: decisions,
|
||||
blockedReason: `review rejected by ${reviewer}${input.note ? `: ${input.note}` : ''}`,
|
||||
});
|
||||
if (!res.ok) {
|
||||
if (res.reason === 'not_found') return { ok: false, reason: 'not_found' };
|
||||
continue; // conflict → re-read + retry
|
||||
}
|
||||
const lease = await repo.getLease(jobId);
|
||||
if (lease && lease.status === 'held') {
|
||||
await repo.revUpdateLease(jobId, lease.rev, {
|
||||
status: 'released',
|
||||
leaseEpoch: newEpoch,
|
||||
holderFactoryId: undefined,
|
||||
});
|
||||
}
|
||||
await repo.appendEvent({
|
||||
jobId,
|
||||
productId,
|
||||
type: 'review_rejected',
|
||||
actor: reviewer,
|
||||
data: { decisions, leaseEpoch: newEpoch, note: input.note },
|
||||
});
|
||||
return { ok: true, doc: res.doc, gate: 'rejected' };
|
||||
}
|
||||
|
||||
// Approve: count DISTINCT approving reviewers against the quorum.
|
||||
const approvals = new Set(decisions.filter(d => d.decision === 'approve').map(d => d.reviewer))
|
||||
.size;
|
||||
const required = job.reviewPolicy?.requiredApprovals ?? 1;
|
||||
const satisfied = approvals >= required;
|
||||
|
||||
const res = await repo.revUpdateJob(jobId, productId, job.rev, {
|
||||
stage: satisfied ? 'testing' : 'review',
|
||||
reviewDecisions: decisions,
|
||||
});
|
||||
if (!res.ok) {
|
||||
if (res.reason === 'not_found') return { ok: false, reason: 'not_found' };
|
||||
continue; // conflict → re-read + retry
|
||||
}
|
||||
await repo.appendEvent({
|
||||
jobId,
|
||||
productId,
|
||||
type: satisfied ? 'review_approved' : 'review_decision',
|
||||
actor: reviewer,
|
||||
data: { decision: 'approve', approvals, required, decisions },
|
||||
});
|
||||
return { ok: true, doc: res.doc, gate: satisfied ? 'approved' : 'pending' };
|
||||
}
|
||||
|
||||
return { ok: false, reason: 'conflict' };
|
||||
}
|
||||
|
||||
/** One factory's scored explanation for a job (already-weighted breakdown). */
|
||||
export interface FactoryScoreExplain {
|
||||
|
||||
@ -187,6 +187,67 @@ describe('fleetRoutes', () => {
|
||||
expect(body.alerts.some((a: { code: string }) => a.code === 'no_live_capacity')).toBe(true);
|
||||
});
|
||||
|
||||
it('POST /fleet/jobs/:id/review approves through the human gate', async () => {
|
||||
const app = await buildApp();
|
||||
const job = JSON.parse(
|
||||
(await submit(app, { idempotencyKey: 'rev-route', bodyMd: '# task' })).body
|
||||
).job;
|
||||
// Drive the job to building via claim + fenced patch.
|
||||
const claimRes = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/fleet/claim',
|
||||
payload: { factoryId: 'fac_1', capabilities: [] },
|
||||
});
|
||||
const claim = JSON.parse(claimRes.body);
|
||||
await app.inject({
|
||||
method: 'PATCH',
|
||||
url: `/api/fleet/jobs/${job.id}`,
|
||||
payload: { leaseEpoch: claim.job.leaseEpoch, stage: 'building' },
|
||||
});
|
||||
|
||||
const reqRes = await app.inject({
|
||||
method: 'POST',
|
||||
url: `/api/fleet/jobs/${job.id}/review/request`,
|
||||
payload: { requiredApprovals: 1 },
|
||||
});
|
||||
expect(reqRes.statusCode).toBe(200);
|
||||
expect(JSON.parse(reqRes.body).stage).toBe('review');
|
||||
|
||||
const approveRes = await app.inject({
|
||||
method: 'POST',
|
||||
url: `/api/fleet/jobs/${job.id}/review`,
|
||||
payload: { reviewer: 'alice@x.com', decision: 'approve' },
|
||||
});
|
||||
expect(approveRes.statusCode).toBe(200);
|
||||
const approved = JSON.parse(approveRes.body);
|
||||
expect(approved.gate).toBe('approved');
|
||||
expect(approved.stage).toBe('testing');
|
||||
});
|
||||
|
||||
it('POST /fleet/jobs/:id/review/request rejects an unsatisfiable policy with 400', async () => {
|
||||
const app = await buildApp();
|
||||
const job = JSON.parse(
|
||||
(await submit(app, { idempotencyKey: 'rev-bad', bodyMd: '# task' })).body
|
||||
).job;
|
||||
const claimRes = await app.inject({
|
||||
method: 'POST',
|
||||
url: '/api/fleet/claim',
|
||||
payload: { factoryId: 'fac_1', capabilities: [] },
|
||||
});
|
||||
const claim = JSON.parse(claimRes.body);
|
||||
await app.inject({
|
||||
method: 'PATCH',
|
||||
url: `/api/fleet/jobs/${job.id}`,
|
||||
payload: { leaseEpoch: claim.job.leaseEpoch, stage: 'building' },
|
||||
});
|
||||
const res = await app.inject({
|
||||
method: 'POST',
|
||||
url: `/api/fleet/jobs/${job.id}/review/request`,
|
||||
payload: { requiredApprovals: 2, reviewers: ['only-one@x.com'] },
|
||||
});
|
||||
expect(res.statusCode).toBe(400);
|
||||
});
|
||||
|
||||
it('POST /fleet/claim returns claimed:false when nothing is eligible', async () => {
|
||||
const app = await buildApp();
|
||||
const res = await app.inject({
|
||||
|
||||
@ -13,6 +13,8 @@
|
||||
* GET /fleet/jobs/:id/events append-only event stream
|
||||
* GET /fleet/jobs/:id/events/stream live event stream (SSE, resumable)
|
||||
* GET /fleet/metrics fleet metrics + alerts (queue depth, utilization)
|
||||
* POST /fleet/jobs/:id/review/request route a building job into the review gate
|
||||
* POST /fleet/jobs/:id/review submit a reviewer decision (approve/reject)
|
||||
* POST /fleet/jobs/:id/artifacts upload a run output (base64 body → blob + pointer)
|
||||
* GET /fleet/jobs/:id/artifacts list a job's artifact pointers
|
||||
* GET /fleet/artifacts/:artifactId pointer + fresh short-lived SAS download URL
|
||||
@ -34,6 +36,7 @@ import {
|
||||
SubmitJobSchema,
|
||||
ListJobsQuerySchema,
|
||||
PatchJobSchema,
|
||||
SubmitReviewSchema,
|
||||
ClaimSchema,
|
||||
RenewLeaseSchema,
|
||||
ReleaseLeaseSchema,
|
||||
@ -144,6 +147,52 @@ export async function fleetRoutes(app: FastifyInstance) {
|
||||
await trackerBridge.maybeEchoOnTransition(pid, id, req.log);
|
||||
return res.doc;
|
||||
});
|
||||
|
||||
// ── Multi-reviewer human gate (§14 Phase 3) ──
|
||||
app.post('/fleet/jobs/:id/review/request', async req => {
|
||||
await extractAuth(req);
|
||||
const { id } = req.params as { id: string };
|
||||
const pid = getRequestProductId(req);
|
||||
const body = (req.body ?? {}) as { requiredApprovals?: number; reviewers?: string[] };
|
||||
const res = await coordinator.requestReview(id, pid, {
|
||||
requiredApprovals: body.requiredApprovals,
|
||||
reviewers: body.reviewers,
|
||||
});
|
||||
if (!res.ok) {
|
||||
if (res.reason === 'not_found') throw new NotFoundError('Job not found');
|
||||
if (res.reason === 'invalid_state') {
|
||||
throw new ConflictError('only a building job can be routed to review');
|
||||
}
|
||||
if (res.reason === 'invalid_policy') {
|
||||
throw new BadRequestError('requiredApprovals exceeds the number of allowed reviewers');
|
||||
}
|
||||
throw new ConflictError('concurrent update conflict — retry');
|
||||
}
|
||||
await trackerBridge.maybeEchoOnTransition(pid, id, req.log);
|
||||
return res.doc;
|
||||
});
|
||||
|
||||
app.post('/fleet/jobs/:id/review', async req => {
|
||||
await extractAuth(req);
|
||||
const { id } = req.params as { id: string };
|
||||
const pid = getRequestProductId(req);
|
||||
const parsed = SubmitReviewSchema.safeParse(req.body);
|
||||
if (!parsed.success) badRequest(parsed.error.issues);
|
||||
const res = await coordinator.submitReview(id, pid, parsed.data);
|
||||
if (!res.ok) {
|
||||
if (res.reason === 'not_found') throw new NotFoundError('Job not found');
|
||||
if (res.reason === 'invalid_state') {
|
||||
throw new ConflictError('job is not awaiting review');
|
||||
}
|
||||
if (res.reason === 'not_authorized') {
|
||||
throw new BadRequestError('reviewer is not on the allowlist for this job');
|
||||
}
|
||||
throw new ConflictError('concurrent update conflict — retry');
|
||||
}
|
||||
await trackerBridge.maybeEchoOnTransition(pid, id, req.log);
|
||||
return { ...res.doc, gate: res.gate };
|
||||
});
|
||||
|
||||
app.post('/fleet/claim', async req => {
|
||||
await extractAuth(req);
|
||||
const parsed = ClaimSchema.safeParse(req.body);
|
||||
|
||||
@ -135,6 +135,28 @@ export const InsightsSchema = z.object({
|
||||
* claim + fenced transitions (maps to Cosmos `_etag` / If-Match in production;
|
||||
* see repository.revUpdate).
|
||||
*/
|
||||
/**
|
||||
* Multi-reviewer human gate (§14 Phase 3). A job in the `review` stage carries a
|
||||
* policy (how many distinct approvals are required + an optional reviewer
|
||||
* allowlist) and an append-style decision set (one current decision per reviewer).
|
||||
*/
|
||||
export const ReviewPolicySchema = z.object({
|
||||
requiredApprovals: z.number().int().min(1).default(1),
|
||||
reviewers: z.array(z.string()).default([]),
|
||||
});
|
||||
export type ReviewPolicy = z.infer<typeof ReviewPolicySchema>;
|
||||
|
||||
export const REVIEW_DECISIONS = ['approve', 'reject'] as const;
|
||||
export type ReviewDecisionKind = (typeof REVIEW_DECISIONS)[number];
|
||||
|
||||
export const ReviewDecisionSchema = z.object({
|
||||
reviewer: z.string().min(1),
|
||||
decision: z.enum(REVIEW_DECISIONS),
|
||||
at: z.string(),
|
||||
note: z.string().optional(),
|
||||
});
|
||||
export type ReviewDecision = z.infer<typeof ReviewDecisionSchema>;
|
||||
|
||||
export const FleetJobDocSchema = z.object({
|
||||
id: z.string(),
|
||||
productId: z.string().min(1),
|
||||
@ -160,6 +182,12 @@ export const FleetJobDocSchema = z.object({
|
||||
leaseEpoch: z.number().int().nonnegative().default(0),
|
||||
rev: z.number().int().nonnegative().default(0),
|
||||
blockedReason: z.string().optional(),
|
||||
/**
|
||||
* Multi-reviewer human gate state (§14 Phase 3). Both are present only while a
|
||||
* job is (or has been) in `review`; `requestReview` (re)sets them.
|
||||
*/
|
||||
reviewPolicy: ReviewPolicySchema.optional(),
|
||||
reviewDecisions: z.array(ReviewDecisionSchema).optional(),
|
||||
/**
|
||||
* Last Item status echoed to the linked tracker Item (§10 round-trip). Used to
|
||||
* make re-echo of an unchanged outcome a no-op. Optional + provider-managed —
|
||||
@ -350,6 +378,19 @@ export const PatchJobSchema = z.object({
|
||||
});
|
||||
export type PatchJobInput = z.infer<typeof PatchJobSchema>;
|
||||
|
||||
export const RequestReviewSchema = z.object({
|
||||
requiredApprovals: z.number().int().min(1).optional(),
|
||||
reviewers: z.array(z.string()).optional(),
|
||||
});
|
||||
export type RequestReviewInput = z.infer<typeof RequestReviewSchema>;
|
||||
|
||||
export const SubmitReviewSchema = z.object({
|
||||
reviewer: z.string().min(1),
|
||||
decision: z.enum(REVIEW_DECISIONS),
|
||||
note: z.string().optional(),
|
||||
});
|
||||
export type SubmitReviewInput = z.infer<typeof SubmitReviewSchema>;
|
||||
|
||||
export const ClaimSchema = z.object({
|
||||
productId: z.string().min(1).optional(),
|
||||
factoryId: z.string().min(1),
|
||||
|
||||
Loading…
Reference in New Issue
Block a user