Description of the Change
Problem
The primary agent's done barrier handler only validates that success is set
before storing the subtask result. It never checks whether any work was actually
delegated or executed, so the model can call done{success:true, result:"<made-up command output>"} with zero delegations and the fabricated result is stored and
reported to the user. This was seen on a simple "print the hostname" flow that
reported a hostname with no termlogs entry (nothing ran). Reproduced at ~2/5
on a clean OpenAI stack.
Solution
Add a lightweight evidence check to the primary agent's done path:
- After
Done.Validate(),checkDoneEvidencelooks up the subtask's tool calls via the existingGetSubtaskToolcallsquery. Ifsuccess:truebut no delegation (AgentToolType: coder/maintenance/pentester/search/memorist/advice) happened in this subtask, the handler returns a new*tools.BarrierDeclinedError. - A declined barrier is soft:
performer.gofeeds the reason back to the model as the tool response and keeps the chain running (it is not routed through argument-correction retries), so the agent can delegate and try again — no flow failure. - Escape hatches (backward compatible, no schema change): a
success:false(failure/blocked, or "cannot verify / no execution needed") result is always accepted; and after one rejection a repeated success is accepted with a warning (kept well under the existing repeating-tool-call abort threshold so it can never deadlock a subtask). - Fail-open: if the tool-call lookup errors, it logs a warning and accepts the done; a cancelled context is propagated as an error, not a rejection.
- One reinforcing line added to
primary_agent.tmpl.
Scoped to the primary agent only; specialist barriers are untouched.
Closes #457
Type of Change
- 🐛 Bug fix (non-breaking change which fixes an issue)
Areas Affected
- AI Agents (Researcher/Developer/Executor)
Testing and Verification
Test Configuration
PentAGI Version: main (h843dfeb)
Docker Version: 29.9.1
Host OS: Debian 13
LLM Provider: OpenAI (default models)
Enabled Features: none extraTest Steps
cd backend && go test ./pkg/providers/ -run TestDone_ -v- Live: bind-mount the patched binary onto the plain stack and run several "print the hostname of this machine" flows (OpenAI).
Test Results
- New unit tests (
backend/pkg/providers/done_evidence_test.go) cover: success with nothing delegated → rejected; success after a delegation → accepted; any specialist counts; failure result → accepted; rejection cap → accepted with a warning; a delegation in another subtask does not count; lookup error → fail-open; cancelled context → error not rejection; and a declined barrier keeps the chain running without an argument-fix retry. All pass; they fail on the unpatched handler. go vetclean;gofmtclean.- Live (5 flows): the guard fired on the 2 flows that were about to report a fabricated hostname with no delegation. One then delegated and returned the real termlog-verified hostname; the other honestly reported failure. 0/5 phantoms shipped vs 2/5 unpatched; 4/5 produced real hostnames.
Security Considerations
Reduces silent fabrication of results, improving the trustworthiness of flow reports. No new dependencies or permissions.
Performance Impact
One extra indexed DB read (GetSubtaskToolcalls) per done on the primary
agent. Negligible.
Documentation Updates
- None required.
Checklist
Code Quality
- My code follows the project's coding standards
- I have added tests to cover my changes
- All new and existing tests pass
- I have run
go fmtandgo vet(for Go code)
Security
- I have considered security implications
- Changes maintain or improve the security model
Compatibility
- Changes are backward compatible
Additional Notes
The rejection cap is intentionally tight (accept on the 2nd attempt) to stay within the existing repeating-tool-call abort threshold. Happy to make the cap or the message configurable if preferred.
HUMAN:
While testing PentAGI locally with simple "print the hostname" flows, I noticed that some flows reported a hostname even though nothing had run: the command log for that subtask was empty. The primary agent can't run commands itself, and its done step never checks whether it delegated anything, so a made-up result went straight into the report. I added a check that sends that kind of success back to the agent once, with the reason, so it can delegate and try again. Reporting failure is always accepted, so a subtask can't get stuck. In my runs this cut fabricated results from 2 in 5 flows to 0 in 5. I kept the check strict on purpose, but I'm happy to make the cap configurable, or to add an explicit "nothing needed to run" flag instead of relying on success:false.
The decline doesn't keep the fabricated summary away from the operator.
customExecutor.Executewrites the done call'smessageas aMsglogTypeDonerow and publishes it to the UI before the barrier handler runs (executor.go#L321-L328, registry.go#L476-L477, msglog.go#L198-L210). WhencheckDoneEvidencereturnsBarrierDeclinedError,Executereturns at executor.go#L388-L392 and never updates that row.subtask.resultstays clean, but in the hostname case the message log still shows a done entry with the made-up recap and no result.The smallest fix I see is in that error branch. If
errors.As(err, &declined)andmsgID != 0, writedeclined.Reasonwithce.mlp.UpdateMsgResult, so the entry reads as a rejected claim. Holding back thePutMsgfor barrier tools until the handler succeeds would hide the entry completely, but that is a bigger change.None of the new tests go through the real
Execute:primaryBarriercallscfg.Barrierdirectly, anddecliningBarrierExecutorstubsExecute. A case throughGetPrimaryExecutorwith a recordingMsgLogProviderwould cover this.