Skip to content

feat(supervisor): implement plan completion verification#105

Merged
donk8r merged 3 commits into
masterfrom
feature/require-plan-done
Jul 12, 2026
Merged

feat(supervisor): implement plan completion verification#105
donk8r merged 3 commits into
masterfrom
feature/require-plan-done

Conversation

@donk8r

@donk8r donk8r commented Jul 12, 2026

Copy link
Copy Markdown
Member
  • Add require_plan_complete configuration option
  • Implement pre-gate to ensure all plan tasks are finished before completion
  • Create open_plan_tasks utility to track incomplete items
  • Integrate plan verification and checklists into verifier context
  • Add session context rendering and plan advisory messages for the verifier
  • Track plan-based blocks in supervisor stats and session info
  • Add unit tests for context rendering and plan advisories

@donk8r
feat(supervisor): implement plan completion verification
ac5b99b 
- Add require_plan_complete configuration option
- Implement pre-gate to ensure all plan tasks are finished before completion
- Create open_plan_tasks utility to track incomplete items
- Integrate plan verification and checklists into verifier context
- Add session context rendering and plan advisory messages for the verifier
- Track plan-based blocks in supervisor stats and session info
- Add unit tests for context rendering and plan advisories
@github-actions

github-actions Bot commented Jul 12, 2026

Copy link
Copy Markdown

Octomind — developer:brief (octohub:glm)

📦 Brief: Plan completion verification pre-gate — blocks done claims while the live plan has open tasks

Overall risk: 🟡 MEDIUM · Cards: 1

# Change Risk Confidence
1 New deterministic pre-gate refuses done when plan has open items; verifier now receives session goal + plan checklist as context 🟡 ●●●
Card 1/1: Plan completion pre-gate + verifier session context · 🟡 · ●●●

INTENT
Prevent "drift-by-omission": an agent self-reports done while its own decomposed plan still has open tasks, silently dropping parts of the work. The commit adds a free (no model call) deterministic pre-gate mirroring the existing mutation pre-gate, and enriches the LLM verify-gate with durable session context so terse follow-up turns ("continue") can be verified against the real goal.

WHAT CHANGED

  • New config field require_plan_complete = true (default ON) in [supervisor.gate] — a second deterministic pre-gate alongside require_check_after_mutation.
  • open_plan_tasks() reads the live plan storage and returns titles of non-completed tasks; empty when no active plan exists.
  • When the agent self-reports done and open plan items exist, a <pay-attention> advisory is injected (carrying PLAN_GATE_MARKER) and the turn re-runs, sharing the same gate_iterations budget as the mutation pre-gate.
  • verify() signature gained a context: &str parameter; the verifier prompt now includes a SESSION CONTEXT block with the anchor intent and live plan checklist, plus prompt instructions to resolve terse requests against that context.
  • plan_blocks counter added to supervisor stats and surfaced in /info display as "plan-blocks".

IMPACT RADIUS

  • verify() is called from exactly one site — api_executor.rs#L485 — which was updated to pass the new context argument. No other callers exist.
  • The pre-gate runs inside execute_api_call_and_process_response, the main response-processing loop. It fires only on self-reported done (same trigger condition as the mutation pre-gate), so non-done turns are unaffected.
  • /info output gains a "plan-blocks" activity line — cosmetic, no downstream consumers beyond the CLI display.

RISK
🟡 MEDIUM. The plan pre-gate and mutation pre-gate share a single gate_iterations counter and the same max_iterations budget. They run sequentially in the same function: mutation gate first (line 336), plan gate second (line 367). If the mutation pre-gate consumes iterations on a turn where both conditions are true, the plan pre-gate may hit the exhausted budget and fall through to the LLM gate without nudging — the open-plan items go unenforced. In practice this requires both "code changed without a check" AND "open plan items" on the same done claim, which is plausible but not the common path. The fall-through is fail-open (LLM gate or acceptance), not a hard block, so the worst case is a missed nudge, not a stuck session.

QUESTIONS

  • Should the plan pre-gate have its own iteration budget instead of sharing gate_iterations with the mutation pre-gate? The shared budget means one pre-gate can starve the other.

📎 Source

📂 Files changed (7 files, ~120 lines)

{tokens:197953,cost:0.2869232}

@donk8r
feat(supervisor): enhance session compression and verification
bf9a2a4 
- Allow updating session intent and requests during compression
- Record successful shell command outputs as session evidence
- Implement deterministic verification for file:line references
- Add ground truth verification using git diffs and command tails
- Introduce EvidenceLedger to track mutated paths and command outputs
- Refactor verify-gate to use GateInput for better context handling
- Track last known gate gaps in ChatSession for continuity
- Update supervisor prompts to prioritize ground truth over agent claims
- Add unit tests for file reference validation and evidence rendering
- Bump dependencies in Cargo.toml and refine config templates
@github-actions

github-actions Bot commented Jul 12, 2026

Copy link
Copy Markdown

Octomind — developer:brief (octohub:glm)

Single caller confirmed. Now I have all the evidence needed. Let me compose the brief.

📦 Brief: Supervisor verification enhanced with ground-truth evidence, file:line citation checks, and intent pivot tracking

Overall risk: 🟡 MEDIUM · Cards: 4

# Change Risk Confidence
1 Verify-gate now receives runtime ground truth (git diff + last command output) and prior gaps alongside the agent's narrative 🟡 ●●●
2 Deterministic file:line reference validation flags fabricated file citations in done answers 🟡 ●●●
3 Anchor intent now replaces on supply (was first-write-wins), seeded from summarizer's original request to track user pivots 🟢 ●●●
4 EvidenceLedger tracks mutated file paths and last successful shell command output for ground-truth rendering 🟢 ●●●
Card 1/4: Verify-gate receives runtime ground truth and prior gaps · 🟡 · ●●●

INTENT
The verify-gate previously judged completion claims against the agent's own narrative (recorded actions + session context). This change feeds it deterministic, agent-unalterable ground truth — the actual git diff of mutated files and the last command's recorded output — plus gaps from the previous verification pass, so the verifier judges real state, not the agent's story.

WHAT CHANGED

  • verify() signature changed from 5 positional &str params to a GateInput<'a> struct with 7 fields (added ground_truth and prior_gaps).
  • The verifier prompt now includes a GROUND TRUTH block (working-tree diff, new-file heads, MISSING notes, last command output) and a PREVIOUSLY FLAGGED GAPS block.
  • ChatSession gained last_gate_gaps: Vec<String> — gaps from the last gate pass are stored on Gaps and cleared on Pass and on new user turns, so re-verification confirms each gap is closed instead of judging from scratch.
  • Ground truth is rendered via render_ground_truth(), which runs git diff HEAD -- <paths> synchronously as a subprocess.

IMPACT RADIUS

  • verify() has exactly one caller: api_executor.rs:510. The signature change is fully contained — no other call sites exist.
  • last_gate_gaps is reset in messages.rs:258 (new user turn) and api_executor.rs:529 (gate PASS), and set in api_executor.rs:540 (gate Gaps). All three session entry points that construct ChatSession (core.rs init, clone, test fixture) were updated with the new field.

RISK
🟡 MEDIUM. render_ground_truth shells out to git synchronously (std::process::Command) on the async runtime path — right before the gate's LLM call. This is a blocking subprocess invocation inside an async function. For a typical repo this is fast, but in a large working tree git diff HEAD can take hundreds of milliseconds, stalling the runtime thread. The function fails open (returns empty string on any git error), so correctness is safe, but the blocking call is a latency concern on large repos.

DIVERGENCE
🧩 Incomplete change — render_ground_truth uses std::process::Command (blocking) in an async context. The codebase uses tokio::process::Command elsewhere for subprocess execution. This is a blocking call on the async runtime, which is the pattern the project's own conventions warn against.

📎 Source

Card 2/4: Deterministic file:line reference validation · 🟡 · ●●●

INTENT
The claim-check system already caught fabricated «verbatim» quotes. This extends the same deterministic, no-model-call check to file:line references in the agent's final answer — flagging references where the file doesn't exist or the line is beyond EOF.

WHAT CHANGED

  • New unverified_file_refs(response: &str) -> Vec<String> in detect.rs — regex-extracts path:line patterns, checks file existence and line count on disk, returns flagged refs.
  • The claim-check block in api_executor.rs now runs both unverified_citations and unverified_file_refs; if either finds issues, it injects a combined <pay-attention> note and re-runs the turn (same bounded re-run loop).
  • Regex is deliberately high-precision: requires a / in the path and a letter-started extension, excludes URL interiors via preceding-char guard.

IMPACT RADIUS

  • unverified_file_refs is called only from api_executor.rs:427, inside the claim_check config guard. No other consumers.
  • The function does synchronous filesystem I/O (std::fs::read_to_string, Path::exists) on the async path, same as the existing unverified_citations pattern.

RISK
🟡 MEDIUM. The regex r"(?:^|[^A-Za-z0-9_./:-])(/?(?:[A-Za-z0-9_@~.-]+/)+[A-Za-z0-9_.-]+\.[A-Za-z][A-Za-z0-9]{0,7}):([0-9]+)" is complex. The preceding-char guard [^A-Za-z0-9_./:-] is designed to exclude URLs, but a file:line reference immediately after a closing paren ) or quote " would match (those chars are not in the exclusion set), which is correct. However, references like src/foo.rs:1 embedded inside markdown link syntax [text](src/foo.rs:1) — the ] before ( is excluded, but the path after ( starts with / which IS in the exclusion set [^A-Za-z0-9_./:-]... actually ( is not in that set so it would match. This means markdown link URLs containing path:line could be false-flagged if the path exists and the line is beyond EOF. Low practical impact since the agent's answer rarely embeds real file paths inside markdown link URLs.

QUESTIONS
The MAX_CHECKED_FILE size guard (2MB) silently skips large files — a reference to a 3MB generated file with a wrong line number would pass unchecked. Is this an acceptable tradeoff, or should the line-count check use a streaming line counter for large files?

📎 Source

Card 3/4: Anchor intent now replaces on supply (was first-write-wins) · 🟢 · ●●●

INTENT
The anchor's intent field was first-write-wins — once set, it never changed, even if the user explicitly pivoted to a new task. This made the session goal go stale across compactions. The change allows intent to be replaced when a new one is supplied, with the compaction path seeding it from the summarizer's original_request (verbatim user words), which itself now tracks explicit user pivots.

WHAT CHANGED

  • Anchor::extend() — removed the self.intent.is_empty() guard; any non-empty supplied intent now replaces.
  • apply_compression() — intent seed now prefers summary.original_request (the summarizer's verbatim user request) over the existing anchor intent; falls back to "Free-form conversation session" only if both are empty.
  • Compression prompt and schema updated: original_request now carries forward unchanged OR quotes the pivot request verbatim if the user explicitly abandoned the prior task.
  • recite.rs doc updated from "immutable (first-write-wins)" to "only moves when a compaction carries a sanctioned user pivot".

IMPACT RADIUS

  • Anchor::extend is called from apply_compression (the only non-test caller). The intent is read by render_session_context (gate input) and recite.rs (context tail recitation). Both consumers already handle dynamic intent — they read anchor.intent at call time, so they automatically benefit from the pivot tracking.

RISK
🟢 LOW. The change is well-guarded: the compaction path only supplies a new intent when original_request is non-empty, and the summarizer is instructed to only change original_request on an explicit user pivot. A stale goal can now heal on the next compaction. No downstream consumer depends on intent being immutable.

📎 Source

Card 4/4: EvidenceLedger tracks mutated paths and last command output · 🟢 · ●●●

INTENT
The EvidenceLedger previously recorded only a compact action log. This adds two ground-truth signals: which files successful mutations touched (for diff scoping) and the output of the last successful shell command (the decisive check before done).

WHAT CHANGED

  • EvidenceLedger gained mutated_paths: Vec<String> (capped at 16, deduped, insertion-ordered) and last_command: Option<(String, String)> (command + output tail, capped at 2000 chars).
  • record() now extracts paths from path/file_path parameters of successful mutation calls.
  • record_command_output() keeps only the latest successful shell output, tail-truncated.
  • Both are reset on reset() (new user turn).
  • Shell command output recording is triggered in response.rs after each successful shell tool call.

IMPACT RADIUS

  • mutated_paths() and last_command() are consumed only by render_ground_truth() in api_executor.rs:503-506. No other consumers.
  • record_command_output is called from response.rs:667, inside the tool execution loop.

RISK
🟢 LOW. The path extraction keys on ["path", "file_path"] — conventional parameter names for file-mutation tools. Tools using different parameter names (e.g., target, destination) would not be tracked, silently degrading ground-truth diff coverage to the file-head fallback path. This is a precision gap, not a correctness issue — the gate still fails open. The cap of 16 paths means a task touching >16 files gets partial diff coverage, which is acceptable for a verification heuristic.

📎 Source

📂 Files changed (11 files, ~400 lines)

{tokens:206746,cost:0.3095234}

@donk8r
chore(deps): upgrade dependencies and refactor rmcp
99bc64e 
- Upgrade agent-client-protocol, tokenizers, tungstenite, and rmcp
- Bump minimad, reedline, termimad, sha1, and rand versions
- Remove jsonrpcmsg dependency
- Migrate agent-client-protocol schema imports to v1
- Box InitializeRequest in Command enum to reduce stack size
- Replace Content with ContentBlock in rmcp calls and tool results
- Align model usage across session and supervisor modules
@github-actions

github-actions Bot commented Jul 12, 2026

Copy link
Copy Markdown

Octomind — developer:brief (octohub:glm)

📦 Brief: Dependency upgrades (rmcp 2.x, ACP 1.x, tokenizers 0.23) with API adaptation

Overall risk: 🟡 MEDIUM · Cards: 1

# Change Risk Confidence
1 rmcp 1.8→2.2 + ACP 0.14→1.2 API migration; tokenizers, reedline, termimad, tokio-tungstenite bumps 🟡 ●●●
Card 1/1: rmcp 2.x + ACP 1.x API migration with dependency bumps · 🟡 · ●●●

INTENT
Upgrade four major dependencies to current versions — rmcp 1.8→2.2, agent-client-protocol 0.14→1.2, tokenizers 0.22→0.23, plus minor bumps to reedline, termimad, tokio-tungstenite. The commit message is chore(deps): upgrade dependencies and refactor rmcp.

WHAT CHANGED

  • rmcp 2.x type renames: rmcp::model::Content → rmcp::model::ContentBlock everywhere (6 call sites across 4 files). extract_content() no longer accesses .raw — ContentBlock is now a direct enum, so the match is on the item itself instead of &item.raw.
  • ACP 1.x schema restructure: all schema imports moved from agent_client_protocol::schema:: to agent_client_protocol::schema::v1::. ProtocolVersion is now imported separately from the parent schema module (it left the v1 namespace).
  • ACP InitializeRequest boxed: the Command::Initialize variant now holds Box<InitializeRequest> instead of a bare value, with *req unboxing at the handler and Box::new(req) at the sender. Comment explains ACP 1.x grew the struct to ~600 bytes.
  • tokenizers 0.23: added features = ["fancy-regex"] — the comment notes 0.23+ requires an explicit regex backend; fancy-regex is the pure-Rust choice over onig (C dependency).
  • Minor bumps: reedline 0.48→0.49, termimad 0.34→0.35, tokio-tungstenite 0.29→0.30 — no source changes needed.

IMPACT RADIUS

  • McpToolResult helpers (success, error, success_with_metadata, extract_content) are the central result-construction API used by every MCP tool. All call sites now use ContentBlock. Grep confirmed zero remaining rmcp::model::Content or RawContent references — migration is complete.
  • tool_execution.rs and condense.rs both reconstruct CallToolResult after truncation/condensing — both updated to ContentBlock. The error-preservation invariant (was_error → CallToolResult::error) is intact in both paths.
  • ACP Command enum is passed through an mpsc::UnboundedReceiver — boxing InitializeRequest keeps the enum size reasonable. Other variants (Authenticate, NewSession, etc.) remain unboxed.

RISK
🟡 MEDIUM. The rmcp and ACP API migrations are mechanical and complete — no old references remain. The real risk is behavioral drift in the upgraded transitive dependencies: tokenizers 0.23 with fancy-regex may produce slightly different token counts than 0.22's default backend, which affects compression triggers and context-window budgeting. This is not verifiable from the diff alone — it depends on whether the fancy-regex backend tokenizes identically to whatever 0.22 used. The ACP 1.x protocol bump is a major version jump (0.14→1.2) that could change wire-format behavior, but that's external to this codebase.

QUESTIONS

  • Was token-count parity between tokenizers 0.22 (default backend) and 0.23 (fancy-regex) verified? Token counts drive compression decisions and spending limits — even small drift compounds across long sessions.

DIVERGENCE

  • 🧩 Incomplete change signal: tokenizers 0.23 added fancy-regex as an explicit feature, but the comment says "0.23+ requires an explicit regex backend" — this is a forced migration, not a choice. The syntect dependency already used regex-fancy, so the codebase is now consistent on fancy-regex across both. No drift, just alignment.

📎 Source

📂 Files changed (8 files, ~155 lines)

{tokens:180374,cost:0.2635066}

@donk8r
donk8r merged commit 8749f30 into master Jul 12, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@donk8r