fix: correlation runId ambiguity + approval-recording handling (review findings)#10
Merged
Merged
Conversation
…rator Review finding (blocker): runId is per-turn (shared across a turn's tool calls), so two calls in one turn without a toolCallId collided on run:<runId> and the 2nd silently overwrote the binding. The registry now distinguishes lanes (toolCall/run/none) — only toolCallId is per-call-unique (full concurrency); the runId lane is fail-closed on ambiguity like the no-ID lane. Also (minor) replace the raw NUL key separator with JSON.stringify([session,toolName]).
…enforcement gap
Review findings: (1) onResolution ignored a { ok:false } from resolveApproval — it now throws so the failure is surfaced. The installed OpenClaw runtime calls onResolution fire-and-forget (notifyPluginApprovalResolution), so the throw is logged but does NOT gate execution; this enforcement gap is not adapter-fixable and is documented as a known cooperative-grade limitation / upstream blocker in AGENTS.md + README. (2) Reworded the ambiguity block reason to 'Helio cannot correlate ambiguous concurrent tool calls' (covers runId + no-ID collisions).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses review findings against the
before_tool_callpath (PR #9).Fixes
runId-lane ambiguity (blocker).runIdis per-turn (shared by every tool call in one agent turn), so two calls in a turn without atoolCallIdcollided onrun:<runId>and the 2nd silently overwrote the binding → mis-correlated/audit. The registry now distinguishes lanes (toolCall/run/none): onlytoolCallIdis per-call-unique (full concurrency); therunIdlane is now fail-closed on ambiguity like the no-ID lane. Tests added (same-runIdcollision, distinctrunIds, sequential reuse).onResolutionnow throws on a{ ok:false }fromresolveApprovalinstead of swallowing it. Verified against the installed OpenClaw runtime thatonResolutionis invoked fire-and-forget (notifyPluginApprovalResolution:Promise.resolve(onResolution(...)).catch(log.warn)), so the throw is logged but does not gate execution. This enforcement gap is not adapter-fixable — documented as a known cooperative-grade limitation / upstream blocker inREADME.md+AGENTS.md. It does not affect the/evaluatefail-closed guarantee (the host enforcesblock).JSON.stringify([session, toolName])(ASCII, unambiguous); ambiguity block reason reworded to "Helio cannot correlate ambiguous concurrent tool calls" (now coversrunId+ no-ID).Verification
pnpm verifygreen — 98 tests, typecheck, lint, format, build.Not in scope
The approval-recording enforcement gap is tracked as an upstream OpenClaw item (needs an awaited/deny-capable approval callback); nothing further is fixable adapter-side.