Skip to content

refactor(mcp): run affected tests and the dashboard through the owner - #2310

Merged
ScriptedAlchemy merged 1 commit into
masterfrom
fleet/mcp-typed-effects
Sep 27, 2026
Merged

ScriptedAlchemy merged 1 commit into
masterfrom
fleet/mcp-typed-effects

Conversation

@ScriptedAlchemy

Copy link
Copy Markdown
Owner

tracedecay_run_affected_tests and tracedecay_dashboard now run through the project's graph-tool owner, on MCP, tracedecay tool, and the tracedecay dashboard CLI (which reaches the daemon over MCP). To support that, the owner's catalog gains a typed entry kind for operations that act beyond reading. It follows #2296 and #2284.

The side-effect entry kind (tracedecay-tool-catalog, the single authority)

pub struct OwnerSideEffectEntryV1 {
    pub effect: EffectClass,                     // SpawnsProcess | BindsServer (new classes)
    pub ceiling_millis: u64,                     // LONG_RUNNING_CEILING_MILLIS (600_000) | INTERACTIVE_CEILING_MILLIS (120_000)
    pub identical_calls: IdenticalCallPolicyV1,  // RunEach: identical concurrent calls are never merged
}
ApplicationSurfaceOperation::owner_side_effect(self) -> Option<OwnerSideEffectEntryV1>
//   RunAffectedTests -> { SpawnsProcess, 600_000, RunEach }
//   Dashboard        -> { BindsServer,   120_000, RunEach }
//   every other operation (all the reads migrated earlier) -> None
  • EffectClass gains spawns_process and binds_server. MutatesProject was left out because no migrated operation needs it.
  • These effects settle within the call. The manifest validator requires an operation receipt, no idempotency key, no reconciliation, and revalidated authority for them. Durable effects keep their stricter contract.
  • The contracts catalog builds these two operations with owner_side_effect_spec(op). The spec takes its effect class, deadline, cancellation (spawns_process: cooperative BeforeAdmission + EffectInFlight; binds_server: not cancellable) and stateless lifecycle from the entry. The read specs are unchanged.
  • The entry is read by:
    • the MCP dispatch contract, so the advertised effect is spawns_process / binds_server with read_only: false;
    • tool_dispatch_ceiling;
    • tool_allows_identical_read_coalescing, which returns false for RunEach.
  • The mcp crate's LONG_RUNNING_TOOL_DISPATCH_CEILING is now defined from the catalog constant.

Migration

  • Typed requests/results: in crates/tracedecay-contracts/src/retrieval/owner_effect_surface.rs. They serialize to the JSON these tools already emitted.

  • Now refused instead of ignored or defaulted:

    • an unknown key;
    • a non-string or missing changed_paths (now a required field);
    • an unknown profile;
    • an out-of-range port (previously it silently became 7341);
    • an unknown dashboard action.
  • Kept in-band: max_tests / timeout_secs bounds violations and "no tests cover" remain in-band typed results, as before.

  • Owner dispatch: the root compute_graph_tool_for_owner runs both under the owner's admitted authorities and the catalog ceiling.

  • Dashboard dependencies: the dashboard's daemon-owned readers, writers and session authorities now reach it through the owner's existing ToolCallRegistryOptions, not a second context type:

    • the profile session store, the savings store, and the session retrieval service and identity;
    • the daemon profile id, the automation reconciler and writer, and the doctor/remote/feedback/PR-autotrack readers;
    • the diagnostics LSP broker, the application executor, the invocation service, and the delivery settlement authority;
    • the code-graph ports and the retained project resolver.

    All of them already lived on the daemon's McpServer; before this change they were passed only on the in-client dispatch path.

  • Deleted: the in-client paths are gone, as are the SessionWorkflow binding row and dispatch group, and the name-keyed effect, verified-journey, cancellation and long-running-ceiling rules for both tools. The compat symbols themselves are untouched.

Fail-before / pass-after (production MCP, harness.call_tool)

These test files were run on unchanged origin/master (697755b):

affected_tests_behavior_test::run_affected_tests_reports_the_cargo_result_for_the_changed_manifest panicked at :125:5:
  the request must be refused, answered Some(... "**kind:** invalid_request\n**message:** `changed_paths` is required ...")
mcp_dashboard_tool_test::tracedecay_dashboard_tool_refuses_without_starting panicked at :27:5:
  the dashboard must refuse, answered Some(... "**host:** 127.0.0.1\n**port:** 46793\n**status:** started\n**url:** http://127.0.0.1:46793/")   ({"bind": ...} was ignored and a server was bound)
test result: FAILED. 4 passed; 2 failed

With this change, all pass:

  • run_affected_tests_reports_the_cargo_result_for_the_changed_manifest. The real cargo test run returns the literal outcome: exit_code 0, passed 1, dispatched_tests ["tests::greeting_is_hello_world"], results [{"test": …, "passed": true, "covers_source_ids": […]}]. It also covers the failing and truncated runs, plus typed refusals for filter, profile: "bench", changed_paths: "src/lib.rs" and […, 7].
  • tracedecay_dashboard_tool_starts_and_returns_url_and_serves_capabilities: the result is exactly {"status":"started","url":"http://127.0.0.1:{port}/","host":"127.0.0.1","port":port}, and a GET of /api/capabilities returns 200 tracedecay-dashboard.
  • identical_concurrent_test_runs_each_run_while_identical_reads_still_ride: two identical concurrent run_affected_tests calls return two different terminal.operation_ids, and the identical-read coalescer's leaders+followers counters do not move. Two identical concurrent tracedecay_files reads add exactly 2 to those counters. This test also passes on master, where both tools were already administrative. It guards the rule rather than failing before.

Runtime journey (debug tracedecay from this branch, isolated HOME/profile, one daemon under systemd-run --user --scope -p MemoryMax=6G -p MemorySwapMax=1G)

Run before the final rebase. After rebasing onto 21552e3, refusals render as isError problem results instead of JSON-RPC errors; the tests assert that newer shape.

tracedecay serve --path corpus (one lib with #[cfg(test)] greeting_is_hello_world):
tools/list: 230 tools
tracedecay_run_affected_tests {"changed_paths": ["src/lib.rs"], "timeout_secs": 240, "format": "json"}: isError=False 1534ms
     {"dispatched_tests":["tests::greeting_is_hello_world"],"exit_code":0,"failed":0,"passed":1,"results":[{"covers_source_ids":[...],"passed":true,"test":"tests::greeting_is_hello_world"}],...
tracedecay_run_affected_tests {"changed_paths": ["src/lib.rs"], "filter": "greeting"}: JSON-RPC error -32603
     tool execution failed: config error: invalid arguments for tracedecay_run_affected_tests: unknown field `filter`, expected one of `changed_paths`, `profile`, `timeout_secs`, `max_tests`
tracedecay_dashboard {"port": 0, "format": "json"}: isError=False 324ms
     {"host":"127.0.0.1","port":33789,"status":"started","url":"http://127.0.0.1:33789/"}
     GET http://127.0.0.1:33789/api/capabilities -> 200 name=tracedecay-dashboard
tracedecay_dashboard {"bind": "127.0.0.1"}: JSON-RPC error -32603
     tool execution failed: config error: invalid arguments for tracedecay_dashboard: unknown field `bind`, expected one of `action`, `host`, `port`
tracedecay_dashboard {"action": "stop", "format": "json"}: isError=False 57ms
     {"previous_url":"http://127.0.0.1:33789/","status":"stopped"}

$ tracedecay tool run_affected_tests --args {"changed_paths":["src/lib.rs"],"format":"json"}
{'dispatched_tests': ['tests::greeting_is_hello_world'], 'passed': 1, 'failed': 0, 'exit_code': 0} termination= completed
$ tracedecay tool dashboard --args {"port":70000}
Error: config error: invalid arguments for tracedecay_dashboard: invalid value: integer `70000`, expected u16
$ tracedecay dashboard --port 0
tracedecay dashboard listening on http://127.0.0.1:32879/

Checks (local)

  • Full mcp_suite (before the final rebases): 558 passed, 24 failed. The 24 split as follows:
  • After rebasing onto 0adc022:
    • mcp_suite -- affected dashboard schema_test protocol_test graph_query_test mcp_cli_serve_test: 113 passed.
    • --lib -- mcp::: 183 passed.
    • lib tests: tracedecay-mcp 389 (one session-workflow test deleted with its group), tracedecay-tool-catalog 6 + tool_catalog_suite 22, mcp-catalog 29, contracts suite 277, daemon-protocol 65.
  • After rebasing onto 453cd45:
    • mcp_suite -- affected dashboard: 9 of this change's tests pass. The one failure is automation_run_list_refuses_a_non_directory_dashboard_root, which is red on master because 21552e3 changed how owner refusals render. schema_test::schema_required_arguments_match_representative_handler_parsers is red on master for the same reason.
    • cargo check --all-targets (both crates, test-transport): clean.
  • clippy: -D warnings over tracedecay tracedecay-mcp tracedecay-contracts tracedecay-daemon-protocol tracedecay-api tracedecay-mcp-catalog tracedecay-tool-catalog tracedecay-cli --all-targets, with and without test-transport, was clean on the pre-rebase head. On current master every crate trips clippy::result_large_err, 298 fn and 71 closure sites, because TraceDecayError grew in 21552e3. That also reproduces on an unmodified master checkout; filed as fix(domain): TraceDecayError too large for clippy since 21552e3764 #2308. None of those sites is code this change added.
  • cargo fmt --all -- --check: clean.
  • pnpm run contracts:generate then contracts:check: up to date. The SDK and the dashboard EffectClass enum were regenerated. pnpm run typecheck (dashboard): clean.
  • Not in this diff: master's Cargo.lock still pins 1.0.0-beta.55 against the beta.56 manifest. I left it alone.

The graph-tool owner's catalog gains a typed side-effect entry kind. The
tool-catalog operation's OwnerSideEffectEntryV1 names what the call does
(EffectClass::SpawnsProcess or BindsServer), its dispatch ceiling (the
long-running ten-minute ceiling for a test run, the interactive ceiling for
the dashboard), and that identical concurrent calls each run. The capability
manifest takes its effect class, cancellation, and deadline from that entry,
settles within the call (operation receipt, no idempotency key), and the MCP
dispatch contract, the dispatch ceiling, and the identical-read coalescer
all read it. The tools migrated earlier stay read entries.

tracedecay_run_affected_tests and tracedecay_dashboard are owner-served
operations with typed requests and results. An unknown key, a mistyped
changed path, an unknown profile, or an out-of-range port is refused instead
of being ignored or silently defaulted. The dashboard's daemon-owned
readers, writers, and session authorities reach it through the owner's
existing tool context. The session-workflow dispatch group, its binding
row, and the name-keyed effect, cancellation, and ceiling rules for both
tools are deleted.
@changeset-bot

changeset-bot Bot commented Sep 27, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: bbbc8ae

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@ScriptedAlchemy
ScriptedAlchemy merged commit f40d360 into master Sep 27, 2026
1 check passed
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-27T09:38:02.887506Z bbbc8ae PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@ScriptedAlchemy
ScriptedAlchemy deleted the fleet/mcp-typed-effects branch September 27, 2026 09:32

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bbbc8aea59

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +945 to +946
| ApplicationSurfaceOperation::RunAffectedTests
| ApplicationSurfaceOperation::Dashboard => match value {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve authoritative settlement for the migrated side effects

When either newly routed operation is cancelled or reaches its deadline after dispatch to the daemon, invoke_application_surface consults application_surface_cancellation_policy, but that match has no arm for RunAffectedTests or Dashboard and therefore classifies both as ReadOnly. The controlled client consequently returns a DuringRead cancellation immediately instead of waiting for effect settlement; a dashboard bind can complete after the caller was told it was cancelled, and an affected-test run can lose its terminal receipt. Classify these operations as AuthoritativeEffect (preferably from their catalog effect metadata) so cancellation cannot abandon an in-flight side effect.

AGENTS.md reference: AGENTS.md:L172-L177

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant