Conversation
jazelly
left a comment
There was a problem hiding this comment.
Fresh independent agent review found three actionable issues:
-
[P1] Continuation advances local Harness state before durable remote turn creation.
local-http-bootstrap.ts:105callsprepareHarnessSkillTurn, which immediately advancesnextTurnand records loaded skills inskill-harness.ts:445. If later skill staging orclient.continuefails, retrying the same action batch cannot safely recover and the skill is recorded as delivered. Use pending/commit/rollback semantics, an idempotent request reference, or commit state only after durable continuation start succeeds. -
[P1] MCP execution does not enforce the discovery-time frozen tool catalog.
mcp-client.ts:180resolves calls from the current Codex config, whileresolveMcpToolaround lines 702–714 checks server/filter policy but not membership in this run's discovered catalog. A model can name an undisclosed or newly-added server tool and execute it when policy permits. Persist the catalog name/schema snapshot and validate every call against it before execution. -
[P2] Terminal MCP-loop errors do not consistently clear OAuth state or cancel timed-out provider turns.
local-http-mcp-loop.ts:114–119clears auth only on the normal final path; turn-limit, tool, provider, and timeout errors throw without cleanup. The timeout path around lines 168–178 also leaves the provider turn running. Clean up ephemeral auth on terminal errors and cancel the corresponding turn on local timeout, while preserving intentional waiting-for-user flows.
Validation: TypeScript lint passed for the three relevant workspaces. The focused local HTTP integration test was blocked before test logic because this Windows environment lacks a usable koffi prebuilt and CMake. The PR also lacks a non-gated continuation failure/retry boundary test.
Summary
Test plan