docs(planning): implementation plan for the three operator follow-ups - #686
docs(planning): implementation plan for the three operator follow-ups#686ginccc wants to merge 1 commit into
Conversation
Written so another agent can pick any of them up cold: shared background (what the operator is, how a capability is an allow-list entry not a tool, the gate model), then per-task problem/route/decisions/tests/risks, plus the verification traps in both repos. Records two findings worth not rediscovering: converse_with_agent is the WRONG route for test-drive (it needs enableBuiltInTools, which setup-api never sends, plus dynamicAgents.* which escalation-flags.ts deliberately flags), and exempting a conversation POST is currently impossible because verifyGateInstalled rejects any exempt starting http.post: - so the approve-every-message option is the only one available without a security-boundary edit.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds a hand-off plan for Platform Operator follow-ups, preserving Task A’s rationale and planning Tasks B and C.
Changes:
- Documents operator architecture and HITL constraints.
- Plans conversation persistence/history.
- Plans invalid JSON request-body validation.
Suppressed comments (4)
planning/operator-followups-plan.md:260
- This implementation route was disproved while implementing Task A. EDDI-Manager#156 found that method-based
grantsWriteCapabilitytreats these POSTs as writes; putting them inREAD_ENDPOINTSincorrectly removes the read-only prompt rules. The merged implementation keeps the three GETs inREAD_ENDPOINTSand places the three POSTs inTEST_DRIVE_WRITES/WRITE_ENDPOINTS, so this reasoning record should capture that correction.
1. **`src/lib/operator/tool-scopes.ts`**
- Add the six endpoints above to `READ_ENDPOINTS` (all of them — `READ_ENDPOINTS` is the
*granted* set, not "only GETs"; the gate is what distinguishes them). Group them under a
`// Test-drive: talk to an agent or group` comment explaining that the POSTs are gated and that
planning/operator-followups-plan.md:407
- This guard runs inside the tool executor, but gated POST tools are paused before their executor is invoked. The invalid body would therefore still consume the human approval that §D says Task C prevents. The plan needs validation in a pre-gate path (plus a gated end-to-end test proving no pause is created), or it must explicitly narrow the goal to preventing only the HTTP request.
In `HttpCallToolsProvider` (the executor lambda, ~line 182), before calling
`apiCallExecutor.execute(...)`:
planning/operator-followups-plan.md:419
- A hard-coded
requestBodylookup misses generated tools whereMcpApiToolBuilderrenames that variable on collision with a path/query parameter (for example, torequestBodyBody). In that valid OpenAPI case invalid JSON still reaches the API. Validate the fully rendered request body that will be sent, rather than one argument-map key, and add a collision test.
Where to read the argument: `templateDataFor(memory, toolRequest)` already deserialises
`toolRequest.arguments()` into a map and merges it. Read `requestBody` from that map rather than
re-parsing the arguments string.
planning/operator-followups-plan.md:280
- These scope expectations no longer match the merged implementation: a read-only operator deliberately cannot create conversation records, so the POST endpoints are absent there. Keeping this assertion would send the next agent toward the same capability-classification regression fixed in EDDI-Manager#156.
- the six endpoints are present in `endpointsForScope("read_only")` and `("read_write")`;
- `/resume`, `/state`, `/cancel` are **absent** from both — assert this explicitly, it is the
regression that matters;
- `buildToolApprovals()` still returns exempt `["http.get:*"]` only (proves A1 did not silently
become A2);
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| **Status:** all three are unimplemented. They are independent — do them in any order, or in parallel | ||
| in separate worktrees. |
| |---|---|---|---| | ||
| | A | Operator can converse with other agents / groups (test-drive) | EDDI-Manager | ~half a day | | ||
| | B | Operator conversation survives navigation + restart; history tab | EDDI-Manager | ~1 day | | ||
| | C | A tool call whose body is invalid JSON fails at the API, not before | EDDI | ~2 hours | |
| - A conversation in `AWAITING_HUMAN` should be visibly marked; selecting it must restore | ||
| `isPaused` and the approval card. `useApprovalStatus(conversationId, isPaused)` already drives | ||
| that surface — set `isPaused` from the hydrated snapshot's `conversationState`. |
A hand-off document for the three open operator follow-ups, written so another agent can pick any of them up cold — no chat history needed.
Structure: shared background first (what the operator actually is, why a capability is an allow-list entry rather than a tool, the gate model, the constraints already decided in
operator-write-scope-plan.md§5, and the verification traps in both repos), then per-task problem → route → decisions → tests → risks.Two findings in it are worth not rediscovering:
converse_with_agentis the wrong route for A. It needsenableBuiltInTools+builtInToolsWhitelist(whichsetup-apinever sends) anddynamicAgents.enabled+allowDelegation— whichescalation-flags.tsdeliberately flags to approvers as an escalation. Using EDDI's own REST conversation endpoints fits the operator's architecture instead.verifyGateInstalledrejects anyexemptbeginninghttp.post:— including a narrow one — so activation would fail its own verification. Approve-every-message is the only option available without a deliberate security-boundary edit, and the plan says so rather than leaving the next agent to discover it.Task A has since been implemented in EDDI-Manager#156; the plan is kept as the reasoning record and for B and C.