feat(compass-agent): SEA-1741 register native comms/lifecycle tools + headless approval in entrypoint - #202
Open
seal-agent wants to merge 3 commits into
Open
feat(compass-agent): SEA-1741 register native comms/lifecycle tools + headless approval in entrypoint#202seal-agent wants to merge 3 commits into
seal-agent wants to merge 3 commits into
Conversation
… headless approval in entrypoint Wire the four comms + two lifecycle native tools into cli.ts main() so the container agent's comms_post_message / agents_spawn_peer emissions resolve as session natives rather than 'unknown tool'. The brokers wrap the existing socket transport (which structurally satisfies CommsTransport/LifecycleTransport); the tools merge into customTools and flow through the same customTools -> state.tools -> #withNatives natives path as the MCP tools (SEA-1532 invariant). Pin the headless approval policy (autoApprove: true) so the write-approval natives auto-execute in a container with no human to approve (frozen design). Closes H3 gaps 1 (registration) + 3 (headless approval). Gap 2 (canned tool-call scripting) and the assembled H4 run remain; SEA-1787 stays open. Refs SEA-1741 SEA-1787 Co-authored-by: Matt Wilkinson <matt@sealedsecurity.com>
…arg-shuffle path + lock the arity invariant Addresses the sole review round on #202 (SEA-1741). The load-bearing comment justifying the `as ToolDefinition[]` cast named the wrong SDK runtime arm: it claimed a marker-less `AgentTool` is passed through verbatim as its definition. In fact the SDK classifies a marker-less AgentTool as a `CustomTool` (`isCustomTool`, sdk.ts:876) and runs it through `customToolToDefinition` (sdk.ts:915), which invokes `execute` with the CustomTool arg order `(toolCallId, params, onUpdate, ctx, signal)` (sdk.ts:927) — whereas `AgentTool.execute` is `(toolCallId, params, signal, onUpdate, ctx)` (pi-agent-core types.ts:612-616). So args 3-5 arrive shuffled. The wiring is correct today only because every native's `execute` reads solely `(toolCallId, params)`; the false comment was masking that latent trap (the exact shape a future cancellation-plumbing change would hit, silently receiving `onUpdate` in the signal slot with no compile error). - cli.ts: rewrite the cast comment to state the real conversion-arm mechanism plus the arg-shuffle, and the invariant it rests on (execute must read only toolCallId/params until the SDK gains a real native-registration seam). Add the external-invariant note on `autoApprove: true` (Runner-exec-only headless entrypoint; gate on an explicit headless signal if that ever changes). - cli.test.ts: add a deterministic regression test pinning every native's `execute.length === 2`. Arrow bodies have no `arguments`, so consuming a shuffled arg 3-5 requires a formal param, which reddens this test. Red-verified: injecting a 3rd param fails with arity 3 naming the tool; reverted. No behavior change — comment + test only. compass-agent:ci green (427 pass, typecheck + lint clean). Spec-impact: none. Refs SEA-1741 Co-authored-by: Matt Wilkinson <matt@sealedsecurity.com>
…re, not a total guard Addresses review round 2 on #202 (SEA-1741): 0 high, 1 medium, 1 low. Both findings are documentation/test-adequacy accuracy, the same class as round 1, introduced by the round-1 fix. - MEDIUM: the cli.ts cast comment claimed the invariant is "enforced by a test ... (a sentinel in the signal position must not be observed)" — describing a behavioral sentinel-observation test. The test actually added is a structural arity pin (`execute.length === 2`), which plants no sentinel. A maintainer wiring cancellation would look for the described guard and find a weaker one. - LOW: both comments claimed a body can reach a shuffled arg ONLY by declaring a formal param that pushes `execute.length` past 2. False for arrow-compatible forms — a rest param (`...args`) or a defaulted param (`signal = …`) reads arg 3 while keeping `.length === 2`. Fix (comment-only; the arity test is a legitimate tripwire and its body is unchanged): rewrite both comments to state the arity pin is a TRIPWIRE on the likely regression (a plain positional 3rd param), explicitly note it does NOT catch a rest or defaulted param, and name the load-bearing guard as the invariant itself (natives read only toolCallId/params) rather than the arity check. Retitle the test accordingly. Rejected the reviewer's alternative (a behavioral sentinel test): an equivalence/sentinel test has its own soundness holes (read-but-unused, internal-only effects), so swapping one overclaim for another repeats the flagged sin. Honest documentation of a proportional tripwire is the correct fix. Red-verified: injecting a plain positional 3rd param reddens the arity test with `arity: 3 ≠ 2` naming the tool; reverted. compass-agent:ci green (427 pass, typecheck + lint clean). No behavior change. Refs SEA-1741 Co-authored-by: Matt Wilkinson <matt@sealedsecurity.com>
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.
Wire the four comms + two lifecycle native tools into cli.ts main() so the
container agent's comms_post_message / agents_spawn_peer emissions resolve as
session natives rather than 'unknown tool'. The brokers wrap the existing
socket transport (which structurally satisfies CommsTransport/LifecycleTransport);
the tools merge into customTools and flow through the same customTools -> state.tools
-> #withNatives natives path as the MCP tools (SEA-1532 invariant). Pin the
headless approval policy (autoApprove: true) so the write-approval natives
auto-execute in a container with no human to approve (frozen design).
Closes H3 gaps 1 (registration) + 3 (headless approval). Gap 2 (canned
tool-call scripting) and the assembled H4 run remain; SEA-1787 stays open.
Refs SEA-1741 SEA-1787
Co-authored-by: Matt Wilkinson matt@sealedsecurity.com
Parked design fork (for Matt's gate)
Shipping assumption: the native
AgentTool[]are registered viacustomToolswith a single documented, runtime-verified cast to theToolDefinitionarm of(CustomTool | ToolDefinition)[](see the soundness comment at the cast site incli.ts).Why the cast is sound (grounded firsthand):
customToolsis a union(CustomTool | ToolDefinition)[]; a non-CustomToolentry passes through verbatim as its definition (pi-coding-agent sdk.ts:2258-2259).ToolDefinition.executeparam order(toolCallId, params, signal, onUpdate, ctx)==AgentTool's exactly (onlyCustomToolswapssignal/onUpdate).ToolDefinitionarm is generic variance on the optionalrenderCall/renderResult(AgentToolTTheme=unknownvsToolDefinitionTheme/Component) — fields these headless tools never define. Isolated tsc probe confirmed error TS2322 onrenderCallonly.The fork (Matt's call): bridge locally (this documented cast) vs fix the SDK type surface upstream so
AgentToolis assignable toToolDefinitionwithout a cast. Local cast ships now (overnight posture); the upstream question is parked for review/merge.Design line refs (verified verbatim on this checkout)
design.md:596-598("the container runs headless with write-approval tools auto-executing (yolo default) — there is no human to approve. This task pins that approval policy in the entrypoint").cli.tschange:design.md:600-602.design.md:659+:779.Scope
Closes H3 gaps 1 (native tool registration) + 3 (headless approval). Gap 2 (canned provider scripting tool-calls for legs 3/4/5) and the assembled H4 run remain — SEA-1787 stays open until all three land and the assembled run is green.