Skip to content

fix(tool): scope read state by actor and instance#1282

Open
onlyfeng wants to merge 1 commit into
XiaomiMiMo:mainfrom
onlyfeng:codex/upstream-read-state-scope
Open

fix(tool): scope read state by actor and instance#1282
onlyfeng wants to merge 1 commit into
XiaomiMiMo:mainfrom
onlyfeng:codex/upstream-read-state-scope

Conversation

@onlyfeng

@onlyfeng onlyfeng commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Scope runtime read-state marks by session, actor, and instance directory so edit validation does not leak across subagents or get over-cleared when one project instance is disposed.

Root Cause

The current read-state baseline still relies primarily on conversation history. Runtime marks added by the read tool need the same isolation boundaries as the execution context: actor-specific subagents must not share read marks, and disposing one project instance must not clear marks that belong to another live instance.

Changes

  • Add runtime read-state storage grouped by session, actor, and owning instance directory.
  • Mark successful text/image/PDF reads in ReadTool.
  • Check runtime read-state before falling back to historical read-tool messages.
  • Add a narrow LocalContext.tryUse() helper and Instance.directoryOrUndefined for directory-aware module state.
  • Clear only read marks owned by a disposed instance directory.
  • Add regression tests for same-session reads, actor isolation, and per-directory cleanup.

Validation

CI checks on this PR:

  • lint passed
  • typecheck passed
  • test is still red only in unit shards 1/4 and 4/4

Targeted local validation on macOS with the same Bun version used by CI (bun@1.3.14):

  • bun typecheck passed
  • bun test --timeout 60000 test/tool/read-state.test.ts passed: 6 pass, 0 fail
  • bun test --timeout 60000 test/tool/edit.test.ts -t "tool.edit" passed: 25 pass, 0 fail

Upstream Main Baseline

As of 2026-06-24, latest upstream/main@4a96a3730190dcd513b221064954e64be1fbe70d still has a failing test workflow: https://github.com/XiaomiMiMo/MiMo-Code/actions/runs/28081064643

Baseline failures observed there:

  • unit shard 1/4: Instance.provide directory safety > rejects system paths containing secrets
  • unit shard 1/4: History.backfill > indexes existing text and tool parts
  • unit shard 1/4: Claude Code local MCP server is pending until explicitly connected
  • unit shard 1/4: CheckpointContext producer (tryStartCheckpointWriter) > populates context before spawn and cleans up via Effect.ensuring after settle
  • unit shard 1/4: parentSessionID end-to-end (Axis A wiring) > clean parent checkpoint -> splitover plugin reads parent's path -> no ReAct reentry fired
  • unit shard 3/4: WorkflowRuntime cancel cascade > cancel during an in-flight fan-out reclaims every child (no orphan) timed out after 20000ms
  • unit shard 4/4: session.llm.stream > sends anthropic tool_use blocks with tool_result immediately after them

Local fork origin/main@27b6435b4eb7c836a3c17906589e7d70b83fb898 was also reviewed. It contains fork-side follow-up work, but the read-state implementation and tests in this PR match the corresponding fork-main read-state files. The only relevant-file delta is unrelated protected-path tightening in src/project/instance.ts, so this PR remains narrowly scoped to upstream main.

Local Note

The original branch push was done with --no-verify because the machine used at that time had bun@1.3.11 while the pre-push hook requires bun@^1.3.14. The validation above was rerun with bun@1.3.14.

@onlyfeng

Copy link
Copy Markdown
Contributor Author

Leaving context before marking this ready for review.

This PR is intentionally scoped to the runtime read-state isolation change: scoping read state by actor and instance, plus the focused tests around that behavior.

Targeted local verification passed:

  • bun typecheck
  • bun test --timeout 60000 test/tool/read-state.test.ts
  • bun test --timeout 60000 test/tool/edit.test.ts -t "tool.edit"

The current full CI unit failures appear unrelated to this PR. The failing areas look like existing upstream baseline issues or separate fixes, including MCP eager-connect/pending behavior (#1269), fixture/root path handling, history FTS isolation, checkpoint writer expectations, and Anthropic cache_control ordering.

I am not folding those baseline/test-only fixes into this PR, so maintainers can evaluate the runtime change independently.

@onlyfeng onlyfeng marked this pull request as ready for review June 24, 2026 07:57
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