Repository navigation
mcp: verify_scene runs in steps, so a host can pause inside a long check - #1272
Merged
Merged
Conversation
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.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
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.
What does this PR do?
verify_sceneruns every check in one synchronous stretch. On a large building with the usage checks on that stretch is long: 16 to 18 seconds on an Apple-silicon Mac for a six-storey building of about 1,300 nodes, several times that on a small server. Nothing else on the server runs meanwhile, so a host's timers and I/O wait for it.author_sceneends with the same checks, so every conversion of such a building ends with that stretch, and a host that has to do something on a schedule while a call runs (renew a lock, answer a heartbeat) cannot.#1271 let a host pause between the calls of a
run_batch. This PR makes the checks themselves stop, so the pause reaches inside one long call:verify_sceneis written in steps (verifySceneSteps): it stops after each registered check, and the circulation measurement, which is most of the time, stops after each floor (measureUsageSteps,usageIssueSteps).verifySceneruns the same steps through in one go, so its answer is unchanged.verify_scenecall too, as well as after each call.author_scene's closing checks pause on a host that gives the hook.SceneCheck.run); one that returns its findings works as before.Measured through a host that pauses, on that building: the longest uninterrupted stretch of an
author_scenecall goes from 18.5 s to 2.8 s. The call takes the same time in total (22 s).Renamed, since the hook is no longer about batches only:
betweenBatchCallsis nowbetweenSteps(createPascalMcpServer,registerTools).RUN_BATCH_STEPSis nowAGENT_OPERATION_STEPS.run_batch, besideAGENT_OPERATION_STEPS.verify_scene, andcreateBatchOperationis gone (AGENT_OPERATIONS.run_batchruns the steps through). The hook was added in #1271; a host that passes the old name has to rename it.Is it safe? The same contract as #1271. Without the hook nothing changes: every call is uninterrupted. With it, other work of the host runs between the steps, so only a host whose scene no other call can reach meanwhile (a bridge of its own for each request) should give it.
verify_scenereads the scene it was handed at its start. Inauthor_scenethe design is already written when the checks run, so on a host that serves concurrent calls on one bridge another call could change the scene before the report is made; that host leaves the hook out, as before.How to test
bun testinpackages/core(7,186 pass) and inpackages/mcp(1,531 pass).packages/core/src/agent-operations/verify-scene-typed.test.ts: taken a step at a time,verify_scenestops more than five times on a two-room scene and answers the report it answers in one go.packages/mcp/src/tools/paused-steps.test.ts(wasrun-batch-pause.test.ts): on a host with the hook,verify_scenepauses between its checks, a batch pauses inside averify_scenecall as well as after each call, and both answer what they answer without the hook.packages/mcp/src/tools/author-scene.test.ts: on a host with the hook,author_scenepauses during its closing checks and reports the same gate.Ran locally: both suites above,
biome checkon both packages,check-typesfor the eleven affected packages.apps/editor'scheck-typesdid not run clean in my checkout for unrelated missing modules (react-scan,geist), so CI is the check for it.Screenshots / screen recording
N/A — non-visual change
Checklist
bun devbun checkto verify)mainbranch🤖 Generated with Claude Code
Note
Medium Risk
Renames a public host hook (
betweenBatchCalls→betweenSteps) and changes how long checks are scheduled on the event loop; outcomes are intended to matchinOneGowhen the hook is omitted.Overview
Long agent operations (
verify_scene,run_batch, usage circulation checks, andauthor_scene's closing verify) are now expressed as resumable generators (Steps,AgentOperationSteps) with synchronous behavior unchanged viainOneGo.Core:
verifySceneStepsyields after each registered scene check (and nested generators, e.g. usage checks per measurement and per floor inmeasureUsageSteps).run_batchusescreateBatchStepswith asteppedmap so a batch canyield*inside calls likeverify_scene.createBatchOperationandRUN_BATCH_STEPSare replaced byAGENT_OPERATION_STEPS(run_batch+verify_scene).SceneCheck.runmay return either issues or a generator.MCP: Host callback
betweenBatchCalls→betweenSteps; shared tools route any stepped operation throughpausedBetweenSteps(not batches only).author_sceneawaits pauses during closingverifySceneSteps.Hosts without the hook see no behavioral change; hosts with per-request scene isolation can let timers/I/O run between steps while preserving the same final results.
Reviewed by Cursor Bugbot for commit 2c3bb1f. Bugbot is set up for automated code reviews on this repo. Configure here.