fix(debugger): keep a crashed app's console log instead of deleting it - #705
Draft
latekvo wants to merge 1 commit into
Draft
fix(debugger): keep a crashed app's console log instead of deleting it#705latekvo wants to merge 1 commit into
latekvo wants to merge 1 commit into
Conversation
`LogFileWriter.close()` unlinks the log file, and both JS-runtime debugger
blueprints call it from `dispose`. When the CDP socket drops the registry's
terminated cascade disposes the service with no tool call involved, so the
console output captured before the crash is destroyed at the moment a
developer would read it — at the very path `debugger-log-registry` already
handed them:
before crash: totalEntries=4 file=~/.argent/tmp/argent-logs-39841-*.log
stat: 300 bytes grep CRITICAL: 4 lines
--- terminate the CDP socket ---
stat: No such file or directory grep CRITICAL: 0 lines
The skills prescribe exactly this sequence — "returns the log file path …
then read the returned file for details" — and the failure-scenarios row for
"the app may have crashed" says nothing about the logs being gone. Calling
the tool again mints a fresh session reporting `totalEntries: 0`, which its
own description defines as "no log data has been captured yet".
`dispose` now keeps the file when the teardown was caused by the runtime
dying, and still removes it on an explicit teardown. Since a kept file has
no owner left to clean it up — and a tool-server killed outright never
closed its writer either — the writer prunes log files older than a day when
it opens a new one. Age-based, because several tool-servers can run at once.
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: pre-existing since the log registry landed in #30 (2026-03-18), untouched by that PR.
The defect
LogFileWriter.close()ends infs.unlinkSync(this.filePath)(log-file-writer.ts:231), and both JS-runtime debugger blueprints call it fromdispose. When the CDP socket drops - the app crashed or was force-quit - the blueprint emitsterminated, the registry's cascade callsdispose, and the log file is unlinked. No tool call is involved. The socket dropping is sufficient.Real tool-server over HTTP, mock Metro + CDP:
That is the exact artifact the documented workflow tells you to read:
debugger-log-registry's description: "Returns the log file path... call this first for an overview, then read the returned file for details."argent-metro-debugger/SKILL.md:85: "Logs are written to a flat log file on disk. Use the log-registry -> grep pattern",:90"Search the file usingGreporRead"references/failure-scenarios.md:10is the one row that describes this exact situation - "The app may have crashed or been closed" - and it does not mention that the log file was just destroyed.Calling the tool again does not recover it: the registry nulls the instance, so the in-memory clusters die with the writer and a fresh session is minted with a new path and
totalEntries: 0- which that tool's own description defines as "no log data has been captured yet". An agent reads that as "the app logged nothing", the opposite of the truth.The fix
disposekeeps the file when the teardown was caused by the runtime dying, and still removes it on an explicit teardown. The discriminator has to live in the blueprint: every teardown funnels throughregistry._teardown->instance.dispose(), which takes no cause argument. Thedisconnectedhandler is the only unexpected-death entry point for these two services - neither blueprint declaresrecoverable, so_recoverFailedServicescan never dispose them.Explicit teardowns that must keep unlinking, and do:
index.ts:386registry.dispose()on shutdown,flow-run.ts:642,stop-simulator-server.ts:61,stop-all-simulator-servers.ts:49.react-profiler-start.ts:165,170also disposes the service, but both call sites are already guarded on the service being non-RUNNING or!cdp.isConnected(), i.e. the runtime is already dead - so no healthy session loses its logs there.The cost, and why the pruner is in the same PR.
~/.argent/tmphas exactly one writer and no sweeper anywhere in the repo. Files already leak today whenever the tool-server is killed rather than shut down - this machine had 8 orphans, the oldest from Jul 29. Keeping the file on crash adds one orphan per crash, so the change has to carry its own cleanup or it trades a data-loss bug for an unbounded-growth bug. The writer now prunesargent-logs-*.logolder than 24h when it opens a new one: age-based rather than delete-all because several tool-servers can run concurrently, and 24h far exceeds any debugger session, so a live writer's file is never a candidate. This also drains the pre-existing leak.Verification
The repro, both directions, through the real HTTP entry point (identical script, two builds): on
mainthe file is gone after the crash; with the fixstatstill reports 300 bytes and all fourCRITICAL pre-crashlines at the path the tool had already returned.Discriminating test -
test/metro/log-survives-crash.test.ts, realRegistry+ blueprint + both tools against a mock Metro/CDP:expected false to be trueat the post-crashexistsSyncThe second case pins the cost:
disposeServicemust still remove the file.The pruner has its own test, and all three mutations of it are killed:
pruneStaleLogs(dir)callSuite: full tool-server run 3087 passed / 6 failed vs.
main's 3083 passed / 6 failed on the same box (+3 new tests, same pre-existingboot-device-hotbootfailures, which are an unrelated env-isolation defect - see the follow-up PR).tsc --noEmit -p tsconfig.test.jsonclean, eslint--max-warnings 0clean, prettier clean.Known limitation
This makes the already-handed-out path readable across a crash. It does not help an agent that first calls
debugger-log-registryafter the crash: it still gets a fresh session with a new file and no signal that a previous one ended or where its logs are. Worth a follow-up; it needs a design decision about session identity rather than a lifecycle fix.Touches
log-file-writer.ts, which #647 also edits (different region - that PR adds source attribution).