fix(state): record the session before the scrape, and recover a lost URI - #45
Merged
Merged
Conversation
Reported from the field driving an iOS simulator: `artisan_start` exceeded the MCP client's 60s tool timeout, the app came up and served on 8183, and `artisan_status` answered `running: false` about it. The operator recovered by hand-writing the state file, which then could not hot restart because the documented schema omits `stdinPipe`. Three fixes, all on our side of the 60s: - `start` wrote the session only AFTER scraping the VM Service URI, the last and longest thing it waits for. It now writes as soon as the child PIDs are known, carrying the pid, the FIFO, the ports and the device, with `booting: true` and a null URI saying the record is incomplete rather than wrong. Both the plain and the CDP path. - A connected command that finds no URI now reads the last one out of the session log and persists it, so an interrupted start heals on the next call rather than needing a hand-written file. The regex and the normaliser moved to `lib/src/state/vm_service_log.dart` so the scrape and the recovery share one definition; `StartCommand.normalizeVmServiceUri` stays as a delegate. - The schema docblock now lists `stdinPipe`, `stdinHolderPid` and `booting`, and the reload / hot-restart error stops blaming an old artisan for what a hand-written file is the likelier cause of. The same report confirmed the per-project sessions work: two agents thought they had corrupted each other's state and had not, because the two session directories kept them apart.
The reported symptom chain started at `status`: it answered `running: false` about an app that was serving on 8183, so the operator concluded the start had failed and went looking for a recovery, which is where the hand-written state file and the missing `stdinPipe` came in. With the session now written before the scrape, `status` reports the app as running. This makes it say the rest: `booting: true` in the payload, and a line naming what is missing and how it gets filled in.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
`artisan_start`'s MCP description still said it writes to `~/.artisan/state.json` and that "ONLY ONE Flutter app per machine can be tracked at a time (single-slot state)". Both have been false since 0.0.10, and the field report shows an agent acting on them: it concluded it had corrupted a sibling's session when the two were never sharing one. The description now says sessions are per project, and carries the line the timeout fix needs a reader to have: if the call times out the app is probably still starting rather than broken, call `artisan_status`, and do not re-run start or hand-write the state file. The skill reference had the same paragraph plus a recovery section quoting the old "older artisan" error text. Both synced, `booting` documented in the schema table, and a new section covering the interrupted-start path. Skill version 0.0.4 -> 0.0.5.
anilcancakir
added a commit
that referenced
this pull request
Aug 20, 2026
Patch release for #45, from a field report driving an iOS simulator through MCP: `artisan_start` exceeded the client's 60s tool timeout, the app came up and served, and `artisan_status` said `running: false` about it. The 60s belongs to the client; the write ordering was ours. The session is now recorded before the VM Service scrape, a connected command recovers a missing URI from the session log, `status` reports `booting` and names what is missing, the schema docblock documents `stdinPipe`, and `artisan_start`'s MCP description stopped claiming the state is one global file.
anilcancakir
added a commit
that referenced
this pull request
Aug 20, 2026
0.0.10 moved the session to `~/.artisan/sessions/<hash>/`. The code changed; ten documentation files did not. The field report behind #45 has an agent saying it had corrupted a sibling's `state.json` when the two were never sharing one. It was not confused: `artisan_start`'s own description told it "ONLY ONE Flutter app per machine can be tracked at a time". It reasoned correctly from a document that was wrong. Swept the skill (including law 2, which stated the old model as a rule), the MCP tool descriptions, and the command and MCP doc pages. Every remaining mention is still true: the legacy pointer exists, is read as a fallback, and is what the hand-written recovery targets; each was read in context rather than replaced on sight. Also corrected: the "no app detected" recovery said to remove the pointer, which does not clear the session; two quoted error strings no longer exist; and neither carried the `booting: true` path.
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
1 participant
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.
Reported from the field
Driving an iOS simulator through MCP:
Confirmed in the transcript:
MCP server "fluttersdk" tool "artisan_start" timed out after 60sondevice=50BAD9FA-..., then the app was found installed and serving on 8183.Why it happened
startwrote the session after_runVmServiceScrape, the last and longest thing it waits for. The MCP client kills the call at 60s; an iOS build takes longer. The spawned process is detached by the wrapper, so the app survives, but the process that was going to record it does not. Result: a running app with no session,statusreportingrunning: false, andstopwith no pid to reap.The 60s belongs to the client and we cannot change it. The write ordering is ours.
Fixes
vmServiceUri: nullandbooting: trueso a reader can tell an incomplete record from a wrong one.lib/src/state/vm_service_log.dartso the scrape and the recovery share one definition;StartCommand.normalizeVmServiceUristays as a delegate for the existing public entry point.stdinPipe— and that docblock is the recipe an operator reaches for precisely whenstarthas failed them, which is exactly how the reporter then hithot-restartexit 2. It now documentsstdinPipe,stdinHolderPidandbooting, and the reload / hot-restart error stops blaming an old artisan for what a hand-written file is now the likelier cause of.A note on the same report
It also confirmed the per-project sessions work: two agents each thought they had corrupted the other's
state.jsonand had not, because the two directories under~/.artisan/sessions/kept them apart.Testing
New:
test/commands/start_command_early_state_test.dart(the booting record carries what an interrupted start needs; the finished record drops the marker) and a recovery case intest/console/artisan_application_test.dartdriving the real log-read-and-persist path.Caught while refactoring: my first pass at the shared normaliser dropped the trailing-slash case, turning
.../ws/into.../ws/ws. An existing test failed and the behaviour is restored verbatim.dart formatzero diff,dart analyzezero issues, 1225 tests green.