fix(skills): give a crashed Chromium app a recovery path that exists - #703
Draft
latekvo wants to merge 1 commit into
Draft
fix(skills): give a crashed Chromium app a recovery path that exists#703latekvo wants to merge 1 commit into
latekvo wants to merge 1 commit into
Conversation
The "Was connected, then tool fails" row in the metro-debugger skill's
failure-scenarios reference sends every reader to `restart-app`. On a
Chromium (CDP) target that tool is rejected at the capability gate, so the
one recovery the skill offers for a dead app cannot run there:
POST /tools/restart-app {"udid":"chromium-cdp-19722", ...}
-> HTTP 400
{"error":"Tool 'restart-app' is not supported on chromium app
(no chromium support declared)."}
The row's precondition is reachable on Chromium: with the Electron process
killed, debugger-status, -connect, -evaluate and -log-registry all fail with
a CDP connect error, which is exactly the row's trigger. `launch-app`
carries chromium capability but needs the live renderer, so it fails the
same way.
The row now names the relaunch that does work, and the two tables that list
`restart-app` without scope say which platforms it covers.
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.
Found while reviewing #610 and set aside as out of scope: it is present on
maintoday and #610 does not change the remedy text.The defect
references/failure-scenarios.md, the "Was connected, then tool fails" row, is the skill's only answer to the app died. It sends every reader torestart-app. On a Chromium (CDP) targetrestart-apphas no chromium capability (packages/tool-server/src/tools/restart-app/index.ts:41-46), so the call never reaches a device:The row's precondition is reachable there. With a real Electron app booted via
boot-deviceand then killed, every Chromium-capable debugger tool fails with the row's trigger:So the skill markets Chromium as a first-class target (
SKILL.md:3,:12), gets a Chromium user into this state, and then offers a recovery that is rejected before it runs.launch-appis not a way out either - it carries chromium capability but needs the live renderer, and fails with the same ECONNREFUSED.The fix
One sentence on the row naming the relaunch that does work, plus scope tags on the two tables that list
restart-appwith no platform qualifier.SKILL.md:12already uses "reject Chromium at the capability gate" andargent-device-interactalready uses the bold platform + "instead" shape, so the wording follows what is there.Verification
Full recovery run against a real Electron app, following the new wording end to end:
debugger-evaluate->"result":"probe-v1"restart-appboot-device+electronAppPath{"platform":"chromium","id":"chromium-cdp-36313","booted":true}debugger-connectwith the new id"connected":truedebugger-evaluate->"result":"probe-v1", HTTP 200The browser branch was verified separately (kill the CDP endpoint, relaunch on the same port, reconnect with the same
chromium-cdp-<port>id). The id only changes when the port does, which is why the wording says to take it fromboot-device/list-devices:boot-devicepicks a free port per call and never reuses the dead one unlesselectronPortis passed.node scripts/grade-skills.mjs-> all 15 skills 10.0/10. Prettier clean (the table realignment in the diff is prettier widening the columns).Not changed
restart-app, but its trigger is the verbatim Metro stringMetro at port 8081 has no CDP targets, which the Chromium path never emits - Chromium reportsChromium CDP on port N reported no page targetsinstead. Every platform that can emit the Metro string is covered byrestart-app's capability, Vega included.stale connection,CDP request timed out). They will want the same qualifier once that branch lands; this PR deliberately does not touch rows that only exist there.