Fix/studio migration analysis protocol - #1144
Merged
Merged
Conversation
Route analysis read its result out of the Codex text stream, so a progress update or a Markdown-fenced reply could be mistaken for the contract and fail the whole migration. Deliver it through a registered dynamic tool instead: the payload stays in typed JSON-RPC arguments, a rejected result returns as `success: false` so the same turn can correct itself, and a second prompt retries the contract when the first turn produces nothing usable. The turn also no longer runs inside the upload request. Studio starts a background worker, records a lease in `control/analysis-driver.json`, and keeps its heartbeat fresh while Codex works; a later read hands the attempt back to the in-Sandbox script once the lease goes stale, so a Studio restart cannot leave a task analysing forever. The scripted path stays as the fallback and `AGENTKIT_MIGRATION_APP_SERVER=0` pins it, which is what lets the app-server path run by default. - app_server: validate dynamic-tool arguments against the analysis contract, answer rejections with the failing field, and end the turn once the result is in - codex_app_server: register dynamic tools, retry overloaded turns, and keep a `willRetry` notice from ending the current turn - service: start both drivers behind one status contract, recover stalled app-server attempts on read, scan the scripted output for the newest contract JSON, and report diagnostics through the veadk logger (the Studio entrypoint pins the root logger to ERROR, which hid a silent fallback)
Evaluation judging no longer parses the last `codex exec` agent message: the runner writes a durable `judge/request.json` for the batch it is judging and Studio answers it with one Codex app-server turn that returns the verdict through the `reportEvaluation` dynamic tool. The runner keeps owning batching, the thread record, the batch cache and the report, and still validates the verdict before caching it, so a rejected batch falls back to `codex exec` inside the same batch budget. Setting `AGENTKIT_MIGRATION_JUDGE_APP_SERVER=0` pins that scripted judge for the whole run instead of writing requests. The request declares how long the runner will listen, and the turn is sized to answer inside that window, so a batch that is too slow comes back as an answered failure instead of a verdict nobody reads. Two defects came out of the first real runs and are fixed here: the turn is now bounded by wall-clock time instead of the app-server's inactivity window, and a transport failure is answered with an error envelope instead of leaving the runner waiting. The `codex exec` fallback also uses the budget left after the channel rather than the budget the batch started with. `run_tool_turn` carries the shared one-turn contract for both the route analysis and the judge, and the analysis now uses it too. Verified with a real migration-evaluation run (Dify workflow source, Dev Sandbox): the channel delivered the verdict in 285.9s, the runner accepted it without falling back, and the report matches the legacy shape field for field.
…erver turns The migration workspace polled the Sandbox, re-ran the whole analysis to ask the user a question, and left a killed delivery hanging on a lease nobody watched. This puts the long work on Studio-driven Codex turns and gives the page a stream to follow. - Stream the task: an append-only event log plus SSE replaces polling, so a refresh or a second tab resumes from a cursor and progress no longer needs an open page (`events.py`, `routes.py`, `MigrationWorkspace.tsx`). - Answer in place: the analysis turn registers its own `askUser` dynamic tool and waits for the page, so one attempt finishes the analysis instead of a `needs_input` re-run (`analysis_input.py`, `models.py`, `analysis_input` routes). - Lease the delivery: the in-Sandbox driver publishes a heartbeat lease, and a killed run settles as `MIGRATION_DELIVERY_INTERRUPTED` instead of hanging forever. - Close the delivery on a turn: `publishArtifact` re-reads the delivered zip and requires it to match the CLI manifest, and `reportDelivery` may only explain the state the Sandbox published, never turn a failure into a success (`delivery_turn.py`, `contracts.py`, `service.py`). - Show app-server work: turn events are translated back into the `codex exec --json` shape the page already parses, so the activity feed is no longer empty on the app-server driver (`activity.py`). Verified: `pytest tests/frontend -k migration` (517 passed), `node --test frontend/tests/*.test.mjs`, `npx tsc --noEmit`, `ruff check .`, and local end-to-end runs for the analysis, question, delivery and killed-driver paths.
… stream The migration workspace rendered its Codex events as a plain grid of blocks, so an analysis or a migration run read as a different product from the intelligent build it sits next to. - The activity feed now uses `<Blocks groupProcess streaming>`: tool calls fold into expandable process rows while a turn is running and settle when it ends. The stream container drops its own `grid` gap, which the grouped rows supply. - The confirm-migration card loses the "Migration starts only after confirmation" subtitle; the controls below already say what happens. - `NewChatCompactSelect` measures the room it has when it opens: a list that cannot fit below the trigger flips upwards and shrinks to the space it actually has, so a select near the bottom of a card no longer pushes its options past the card edge. It re-measures on resize and scroll.
A finished migration showed "migration complete, verification passed" and then started a second turn that pulled the artifact back. Two things caused it. `publishArtifact` ran inside the closing turn, but `AnalysisActivityLog` discarded every `dynamicToolCall`, so the page showed reasoning and `cat` commands while the actual hand-over was invisible. The delivery turn is the exception to that rule: its tool call *is* the deliverable, so `include_dynamic_tools` records it with the same item shape the intelligent build uses for its result tool, and `complete_dynamic_tools` closes a row the turn ended on before the app-server reported it, which would otherwise leave a running row for a delivery that already landed. The verdict schema then offered all four delivery states and invited a wrong pick, so the turn had to be retried. The schema is now narrowed to the state the Sandbox already published, and the handler keeps its server-side check.
The migration page had already switched to the intelligent build's grouped process rows, but the blocks it built from migration items carried none of the fields that renderer reads. Every Codex row therefore fell back to the generic look: a generic icon, output cut at 2000 characters, no timing in the collapsed header and no computed label. The missing half was the data, not the component. - The activity log keeps the app-server's phase, duration and commandActions, and the activity items carry the native itemType/durationMs/phase the shared row renderer keys off. - Tool rows keep the name the app-server gives them (运行命令 / 修改文件 / MCP · server/tool / 网络搜索), so the shared label rules produce the intelligent build's wording. The migration's own status titles stay as the fallback for the codex exec driver, and its delivery tools keep their migration wording. - A tool_output line is often the last line for a call, so it now carries the name and command of the call it belongs to; without them a running command fell back to the migration's wording and lost its label mid-flight. - The feed reports the run's live status, and stays streaming while Codex still has a running item, which includes a closing turn that lands after the task already settled.
The question window waits on a `concurrent.futures.Future` through `asyncio.to_thread`, but only caught the built-in `TimeoutError`. On 3.10 `asyncio` converts that future's timeout while chaining it back across the thread boundary (`_convert_future_exc`), so the exception escaped the guard, killed the analysis worker and the turn never delivered its questions. Catch the app-server-level timeout the way the rest of the app-server client does. 3.11+ aliases both names to the built-in class, so this stays a no-op there, and the existing "nobody answers" check is the regression test.
The migration page showed what Codex did but never what it cost: no turn duration, no tool time, no tokens. The intelligent build reports all three from the app-server's turn lifecycle, and the migration feed renders the same summary component, so the reader now settles one turn per source. A tool-driven migration turn ends the moment its result lands, which is why the app-server never saw turn_completed and the usage stayed invisible. Turns now settle from the app-server's own turn record read back before the session closes, and the in-Sandbox `codex exec --json` turn — which writes a bare terminal line with snake_case usage and no timestamps — settles from that line, with the total computed the way the app-server reports it. The sandbox exec stream carries no timing at all, so those rows say the duration was not reported rather than inventing one.
The migration page could not report the one turn it is watched for most: `ak migrate` drives Codex itself with `codex exec --json`, and that stream carries no tool duration, no turn duration and no model, so those rows said the numbers were not reported while the intelligent build, the analysis turn and the delivery turn all reported them. The CLI is closed source, but it calls whatever `codex` is on PATH, and the Sandbox already hosts a Codex app-server for the Session. Starting a migration now installs a shim ahead of `codex` on that command's PATH. It answers only the CLI's `codex exec … --json` form — including `resume <id>`, `resume --last`, and a prompt on stdin — drives it as one app-server turn, and writes the app-server's own items back as the `codex exec --json` lines the page already reads: each tool's `duration_ms`, every item's native name, and a closing turn line carrying `startedAt`/`completedAt`/`durationMs` and the model, which `codex exec` can never write. Anything the shim does not recognise, and any failure to reach the app-server before the first log line, goes to the real Codex with its original argv, so the CLI keeps behaving exactly as it did before. The prompt is only read from stdin once the app-server is going to take the turn, because a fallback still has to find it there. Also fixes three shim defects found while testing: a connection lost mid-turn left in-flight requests waiting forever, a usage update carrying only the per-turn increment was dropped, and closing the connection could raise over the turn's own result.
…opwatch `test_sync_tool_calls_use_thread_pool_when_configured` failed on CI with `0.358879437999974 < 0.35`. Two tool calls that each sleep 0.2 s take about 0.2 s when handed to a thread pool and about 0.4 s when run on the caller's thread, so the 0.35 s bound sat only 0.15 s above the outcome it wanted and a loaded runner spent that on overhead. It failed for no reason a reader could act on, and it blocks the merge whenever the machine is busy. What the tests are actually after is whether the two calls were in flight at the same time. `_run_two_sync_tool_calls` now records each call's window and the assertions check the windows overlap (or do not), which holds no matter how slow the machine is.
The migration CLI validates the output after `codex exec` exits and opens a new attempt when a finding blocks the delivery, so a migration turn could claim the migration was done and still be followed by a second one: the report lives past the turn boundary, and the model's own completion claim was never checked. The shim already owns the exec, so it now runs the CLI's own deterministic contract (`scripts/validate_runtime.sh`, the same script the migration prompt tells the model to run) as soon as the turn ends. A fatal or repairable finding is handed straight back into the same thread as repair instructions, and the turn only closes once the contract passes. The repair budget is two rounds, a contract the shim cannot run or read a verdict from never interferes, and the CLI's attempt loop stays the backstop. One exec still reads as one migration turn on the page, and its elapsed time now covers every sub-turn and the validation between them. The migration instruction also states the rule the first attempt broke: findings block completion, and the `ak init` config baseline is not the model's to rewrite.
The analysis turn moved from `codex exec --output-schema` to a Sandbox app-server dynamic tool, where the JSON Schema is only a declaration: a turn that answered with a progress note, a Markdown fence or a probe-shaped payload reached Studio's verifier anyway, and every rejection collapsed into one sentence that named neither the field nor the fix. The first submission that passed also became final, so the failing production run turned a plain Dify export into `MIGRATION_ANALYSIS_UNSUPPORTED` while the deterministic detector the image ships had silently found nothing (PyYAML lives on the 3.12 interpreter, the detector asked `/usr/bin/python3`). Studio now reads the archive itself before the turn: `migration/detection.py` walks the upload, parses whatever DSL is inside and records the files, the framework candidates and the evidence lines it actually saw, with a degraded report instead of a silent empty one. The turn gets one tool per verdict (`reportRecommendation`, `reportNeedsInput`, `reportUnsupported`) and `migration/analysis_contract.py` assembles the stored document, so the model's side of the contract is flat and forgiving: summary is the only required field, unknown keys, unknown frameworks and odd types are folded into notes, scalar values are normalised, and the detected candidates are always merged in. The bar that stays is the one that protects the user: `unsupported` needs two evidence lines that exist in the detected file list, and it fails open when detection itself is degraded. When a turn delivers nothing usable, Studio answers from the detection report (a conservative recommendation, the detected framework first) instead of spending the scripted driver on a second analysis, and the outcome is written to `diagnostics/analysis/model-turn.json` so the reason is readable on the page. The two real Baidu Qianfan and Dify exports that failed in production now reach `analysis_ready` with model-authored recommendations, and the seven payloads from the failed rollout are kept as regression cases.
A migration run can lose its AgentKit CLI after the Codex turn reported a terminal state: the project is finished on disk, but nothing will ever turn it into an artifact, so the task sits in `migrating` forever. Packaging is a pure function of the files on disk, so Studio now does that half itself instead of asking for another fifteen-minute Codex turn: * `delivery_recovery.py` mirrors the CLI's own packaging step - the same file set, exclusions, secrets refusal, manifest, atomic zip and `migration-status.json` - and is run inside the Sandbox by the read path when the driver is gone and the agent finished. * the driver heartbeat now watches the CLI it speaks for and publishes `lost` the moment that process leaves the Sandbox, instead of waiting for the heartbeat to go stale. * the rebuild lease is held by a Studio process and heartbeated, so a rebuild that dies is retried once rather than never.
`License-Check` has been the only red job on this branch: four files added along the way went in without the Apache header every other `.py` in the tree carries.
The allowlist that migration and intelligent build validate against now carries `doubao-seed-2-1-pro-260915` in place of the `260628` build, and gains `deepseek-v4-1-flash-260910`. The Sandbox tools are provisioned with `VOLCENGINE_STUDIO_AGENT_MODEL_NAME`, and that name is what the read path validates when the caller does not pick a model, so the default moves with the list: leaving it behind would reject every run that does not name a model explicitly.
Only the VolcEngine side of this change was asked for; `deepseek-v4-1-flash-260910` does not go on the BytePlus list.
yaozheng-fang
approved these changes
Sep 23, 2026
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
2 participants
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.
No description provided.