From 5ec27125b1850d7df542d5bc2104b204546b1948 Mon Sep 17 00:00:00 2001 From: Adam Eivy Date: Thu, 3 Sep 2026 21:54:21 +0000 Subject: [PATCH] feat: hold an agent run that shipped clean code for the wrong task (#5994) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PortOS's reviewers answer "is this good code?". None of them could answer "is this the code that was asked for?", because none of them ever see the request — the code-review prompt is handed a unified diff and nothing else. So a CoS agent could ship a PR that was clean, well-reviewed and green in CI, and quietly do something other than the task the user wrote. The run-level evidence gate proved changes existed and shipped; nothing proved they were the requested changes. A finished run's accumulated diff is now re-read against the task's own stated objective, in a fresh context, and answers three questions the quality chain cannot: is anything that was asked for missing, is anything that was not asked for smuggled in, and is the verification evidence real. The verdict is ship, fix-first, or rethink; a rethink records the run as needing attention rather than complete, and raises a Review Hub alert naming what is missing or unrequested. Settings > Code Reviewers picks the local model it runs on (independently of the quality chain, or inheriting it) and can switch it off. Fresh context is the mechanism: the objective comes from the task record, never the agent's transcript, which would hand the reviewer the assumptions that produced the drift in the first place. The gate fails OPEN throughout — a missing local backend, an unreadable diff, or a model that answers with prose all leave the run's outcome exactly as they found it, so a local-model outage can never convert a queue of good runs into held ones. Claude-Session: https://claude.ai/code/session_01ChZ1DzJoaTsPoouRcrf8rJ --- client/src/components/cos/tabs/AgentCard.jsx | 51 ++++- .../components/cos/tabs/AgentCard.test.jsx | 43 ++++ .../components/settings/CodeReviewersTab.jsx | 26 +++ .../settings/CodeReviewersTab.test.jsx | 47 ++++ .../settings/GoalFidelityControls.jsx | 109 ++++++++++ docs/features/product-surfaces.md | 2 +- server/lib/README.md | 3 +- server/lib/cosValidation.js | 20 ++ server/lib/gitCommitProbe.js | 75 ++++++- server/lib/gitCommitProbe.test.js | 61 +++++- server/lib/goalFidelity.js | 202 ++++++++++++++++++ server/lib/goalFidelity.test.js | 111 ++++++++++ server/lib/index.js | 1 + server/lib/validation.test.js | 20 ++ .../agentFinalization.goalFidelity.test.js | 198 +++++++++++++++++ server/services/agentFinalization.js | 147 ++++++++++++- .../agentFinalization.outputHooks.test.js | 9 + .../agentFinalization.prVerification.test.js | 9 + ...tFinalization.primaryCheckoutDrift.test.js | 9 + .../agentFinalization.providerBench.test.js | 9 + .../agentFinalization.successCriteria.test.js | 9 + ...agentFinalization.transcriptRescue.test.js | 9 + server/services/codeReview.js | 121 +++++++++++ server/services/codeReview.test.js | 88 ++++++++ server/services/review.js | 31 +++ 25 files changed, 1394 insertions(+), 16 deletions(-) create mode 100644 client/src/components/settings/GoalFidelityControls.jsx create mode 100644 server/lib/goalFidelity.js create mode 100644 server/lib/goalFidelity.test.js create mode 100644 server/services/agentFinalization.goalFidelity.test.js diff --git a/client/src/components/cos/tabs/AgentCard.jsx b/client/src/components/cos/tabs/AgentCard.jsx index 50cf738c58..53df823882 100644 --- a/client/src/components/cos/tabs/AgentCard.jsx +++ b/client/src/components/cos/tabs/AgentCard.jsx @@ -23,7 +23,8 @@ import { GitPullRequest, Sparkles, RefreshCw, - Copy + Copy, + Target } from 'lucide-react'; import * as api from '../../../services/api'; import OutputBlocks from '../OutputBlocks'; @@ -104,6 +105,52 @@ function TranscriptTruncationNotice({ transcript }) { ); } +// Goal-fidelity verdict (#5994) — did this run build what the task asked for? +// Distinct from the quality reviewers, which never see the request. Rendered +// whatever the verdict: a `ship` is the evidence the gate ran, and its absence +// is what tells the reader the run was never judged against its objective. +// +// `missing` / `unrequested` are model-authored text derived from an untrusted +// diff, so they render as plain list items — never a link, path, or command. +const GOAL_FIDELITY_TONE = { + ship: { border: 'border-port-success/30', text: 'text-port-success', label: 'Delivers the objective' }, + 'fix-first': { border: 'border-port-warning/30', text: 'text-port-warning', label: 'Delivers it, with gaps' }, + rethink: { border: 'border-port-error/40', text: 'text-port-error', label: 'Does not deliver the objective' } +}; + +function GoalFidelityPanel({ review }) { + const tone = GOAL_FIDELITY_TONE[review?.verdict]; + if (!tone) return null; + return ( +
+
+
+ {review.missing?.length > 0 && ( +
+ Asked for but missing: +
    + {review.missing.map((item, i) =>
  • {item}
    • )} +
+
+ )} + {review.unrequested?.length > 0 && ( +
+ Not asked for: +
    + {review.unrequested.map((item, i) =>
  • {item}
    • )} +
+
+ )} + {review.evidence &&

{review.evidence}

} +
+ ); +} + export default function AgentCard({ agent, onPause, onKill, onDelete, onResume, onRelaunch, completed, paused = false, liveOutput, durations, onFeedbackChange, remote, peerName }) { const [expanded, setExpanded] = useState(false); const [now, setNow] = useState(Date.now()); @@ -869,6 +916,8 @@ export default function AgentCard({ agent, onPause, onKill, onDelete, onResume, )} + {agent.result?.goalFidelity && } + {completed && (agent.metadata?.taskSummary || agent.metadata?.malwareScan?.reportUrl) && (
diff --git a/client/src/components/cos/tabs/AgentCard.test.jsx b/client/src/components/cos/tabs/AgentCard.test.jsx index 6089ab6c18..f10443cd21 100644 --- a/client/src/components/cos/tabs/AgentCard.test.jsx +++ b/client/src/components/cos/tabs/AgentCard.test.jsx @@ -416,3 +416,46 @@ describe('AgentCard missing shell explanation', () => { expect(screen.queryByText('No shell')).not.toBeInTheDocument(); }); }); + +// #5994: the goal-fidelity verdict — whether the run built what the task asked +// for, which no quality reviewer can answer because none of them see the request. +describe('AgentCard goal fidelity', () => { + const withReview = (goalFidelity) => ({ ...agent, result: { ...agent.result, goalFidelity } }); + + it('names the missing and unrequested work behind a rethink verdict', () => { + render( + + + + ); + + expect(screen.getByText(/Does not deliver the objective/)).toBeInTheDocument(); + expect(screen.getByText('the retry backoff')).toBeInTheDocument(); + expect(screen.getByText('an unrelated logging refactor')).toBeInTheDocument(); + expect(screen.getByText('no tests were run')).toBeInTheDocument(); + }); + + it('shows a clean ship verdict, and renders nothing at all for a run the gate never judged', () => { + const { unmount } = render( + + + + ); + expect(screen.getByText(/Delivers the objective/)).toBeInTheDocument(); + unmount(); + + render( + + + + ); + expect(screen.queryByText(/Goal fidelity/)).not.toBeInTheDocument(); + }); +}); + diff --git a/client/src/components/settings/CodeReviewersTab.jsx b/client/src/components/settings/CodeReviewersTab.jsx index c405ec97b3..eaa7db426c 100644 --- a/client/src/components/settings/CodeReviewersTab.jsx +++ b/client/src/components/settings/CodeReviewersTab.jsx @@ -4,6 +4,7 @@ import toast from '../ui/Toast'; import Banner from '../ui/Banner'; import * as api from '../../services/api'; import ReviewerPicker from '../cos/ReviewerPicker'; +import GoalFidelityControls from './GoalFidelityControls'; import useReviewerModelOptions from '../../hooks/useReviewerModelOptions'; import { reviewerModelsFromDefaults, reviewerModelsToDefaults, reviewerEffortsFromDefaults, reviewerEffortsToDefaults } from '../../lib/reviewerModels'; import { @@ -39,6 +40,7 @@ export default function CodeReviewersTab() { const [reviewerEfforts, setReviewerEfforts] = useState({}); const [stopMode, setStopMode] = useState(DEFAULT_REVIEW_STOP_MODE); const [reviewerApplies, setReviewerApplies] = useState(false); + const [goalFidelity, setGoalFidelity] = useState({ enabled: true, backend: null, model: null, effort: null }); const [installed, setInstalled] = useState({}); const modelOptions = useReviewerModelOptions(); @@ -60,6 +62,14 @@ export default function CodeReviewersTab() { setReviewerEfforts(reviewerEffortsFromDefaults(defaults)); setStopMode(defaults.stopMode || DEFAULT_REVIEW_STOP_MODE); setReviewerApplies(defaults.reviewerApplies === true); + // `enabled` defaults ON, so an absent block must read as on — not as a + // stored `false` the next save would then persist. + setGoalFidelity({ + enabled: defaults.goalFidelity?.enabled !== false, + backend: defaults.goalFidelity?.backend || null, + model: defaults.goalFidelity?.model || null, + effort: defaults.goalFidelity?.effort || null, + }); setInstalled(defaults.installed && typeof defaults.installed === 'object' && !Array.isArray(defaults.installed) ? defaults.installed : {}); } else { setLoadError(true); @@ -90,6 +100,15 @@ export default function CodeReviewersTab() { reviewerApplies, ...reviewerModelsToDefaults(reviewerModels), ...reviewerEffortsToDefaults(reviewerEfforts), + // Absent keys are dropped rather than sent as null: the schema treats an + // absent scalar as "inherit", and persisting an explicit null would be a + // pin the resolver can't tell from a deliberate one. + goalFidelity: { + enabled: goalFidelity.enabled, + ...(goalFidelity.backend ? { backend: goalFidelity.backend } : {}), + ...(goalFidelity.model ? { model: goalFidelity.model } : {}), + ...(goalFidelity.effort ? { effort: goalFidelity.effort } : {}), + }, }; const ok = await api.updateSettings({ codeReview: payload }, { silent: true }) .then(() => true) @@ -155,6 +174,13 @@ export default function CodeReviewersTab() { }} /> + +