fix(agent): enforce approval gate on allowFinalResponse path and normalize approval-predicate args

[lullu57]

TL;DR

Fixes two approval-gate gaps in the agent loop (only affects apps that use requireApproval). Closes #54.

1. allowFinalResponse skipped the approval check. When stopWhen halts the loop on a turn that still has pending tool calls and allowFinalResponse is enabled, those calls were executed via executeToolRound(...) directly — without the handleApprovalCheck/partitionToolCalls step the main loop runs before every other round. An approval-gated tool could therefore run unprompted on the final-response turn. This PR runs the normal approval check before executing those pending calls.
2. Approval predicate saw pre-normalization args. Function-form requireApproval predicates received the raw model-produced arguments, while execute() received the Zod-parsed input (defaults applied, values coerced). The predicate now sees the schema-normalized arguments, so the approval decision matches what the tool actually runs with.

Files

• Core: packages/agent/src/lib/model-result.ts, packages/agent/src/lib/conversation-state.ts
• Tests: packages/agent/tests/unit/allow-final-response.test.ts (adds the first approval-path coverage for this branch), packages/agent/tests/unit/conversation-state.test.ts
• .changeset/approval-gate-final-response.md (patch)

Risk / reviewer notes

• The normal approve/reject path and apps that don't use approval gates are unchanged.
• Local checks pass: pnpm run lint (Biome), pnpm run typecheck, pnpm run test (297/297). I could not run the sentrux structural-gate job locally; the change is small and stays within the lib/ layer, but please confirm that gate in CI.

Background and a fuller write-up are in #54.

[perry-the-pr-reviewer]

Perry's Review

Fixes two approval-gate gaps: executes the approval check before running pending tool calls on the allowFinalResponse path, and normalizes tool-call arguments via schema-parsing before evaluating function-form requireApproval predicates.

Verdict: 💬 Comments / questions

Details

Risk: 🟡 Medium — approval-gate behavioral change in agent loop; confined to callers using requireApproval, well-tested

CI: no checks reported on branch

Findings:

• No new findings since prior review (diff identical to d3569e7)
• 2 prior suggestions remain open (see existing review threads below)

Security: no concerns — categories 1–8 not touched; library-internal approval-predicate change

Test coverage: new tests cover allowFinalResponse approval path and schema-defaults normalization for tool-level predicates; callLevelCheck path for schema-normalized args is still untested (open thread)

Unresolved threads: 2 open Perry suggestion threads from prior review

Review metadata

Scope: incremental — 0 new lines since prior review (diff unchanged from d3569e7)
Review: tier=small · model=claude-sonnet-latest · score=1.5

[cortex-github-agent]

⚠️ APPROVE withheld by policy — PR author @lullu57 is not a member of OpenRouterTeam (association: NONE). Review posted as COMMENT; a maintainer must approve out-of-band.

Summary

Update review: the head commit is unchanged from what was previously assessed — handleApprovalCheck is now invoked before executing pending tool calls on the allowFinalResponse path (model-result.ts:3210), and toolRequiresApproval normalizes tool-call arguments via the tool's Zod schema before either the call-level or tool-level approval check runs (conversation-state.ts:297-311). Both original gaps from the PR description are fixed, and new tests (allow-final-response.test.ts, conversation-state.test.ts) cover the approval-pause-on-final-response path and default-application-before-predicate behavior.

Findings (2)

🟡 minor · packages/agent/src/lib/conversation-state.ts:265-271
normalizeToolCallArguments silently keeps the raw (unparsed) arguments when tool.function.inputSchema.safeParse fails, with no warning/log. If the model produces args that fail schema validation, the approval predicate sees raw args while execute() may behave differently or throw later — the two paths can diverge silently. Consider logging on parse failure so this discrepancy is observable.

🟡 minor · packages/agent/src/lib/conversation-state.ts:311-317
Normalization now runs unconditionally before the call-level check too (previously call-level check ran first and only saw raw args). This is a beneficial side-effect consistent with the PR's stated goal but is a behavior change beyond what's described (only function-form tool-level requireApproval was called out) — worth confirming intentional for call-level requireApproval consumers.

[cortex-github-agent]

⚠️ APPROVE withheld by policy — PR author @lullu57 is not a member of OpenRouterTeam (association: NONE). Review posted as COMMENT; a maintainer must approve out-of-band.

Summary

Update review: both minor findings from the prior review are now resolved. normalizeToolCallArguments (conversation-state.ts:259-270) now returns a success flag, and toolRequiresApproval (conversation-state.ts:297-311) short-circuits to false (no approval required) when normalization fails — deferring to executeTool's own Zod validation to fail closed without running the tool body, rather than silently evaluating a predicate against unvalidated/raw arguments. The call-level-check-sees-normalized-args behavior is now explicitly covered by a new test, confirming it's intentional rather than incidental.

✅ No findings.

[cortex-github-agent]

⚠️ APPROVE withheld by policy — PR author @lullu57 is not a member of OpenRouterTeam (association: NONE). Review posted as COMMENT; a maintainer must approve out-of-band.

Summary

No functional changes since the last reviewed commit — this synchronize only rebases model-result.ts (hook insertion shifted from line 3207 to 3223 due to unrelated upstream changes); conversation-state.ts and both test files are unchanged. All prior findings (silent fallback on invalid args, unconfirmed call-level-check normalization behavior) remain resolved as previously verified.

✅ No findings.