[General] Root-cause corrections for three backlog reports (#1204, #1288, #1199) #1440
BILLKISHORE
started this conversation in
General
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
Area
Coding agent and CLI
Topic
You mentioned you are still reading the closed backlog to find recurring bugs and problems worth picking up. While the queue was open I read a batch of those reports against the code on
main, now97b994c3. Three of them have accurate symptoms, real logs, and correct line references, but a root cause that does not survive reading the surrounding code. Two of the three would make things worse if implemented as written.Posting them here so they are not carried into an Issue at face value. Something is wrong in all three cases, and in each one the cause sits somewhere other than where the report puts it.
#1204, shutdown archives resident sessions. The trace is right:
worker_archive_and_shutdowncloses sessions with reason"killed", andcloseKeepsResumeEntryspares only"shutdown"and"update". The suggested fix is to pass"shutdown"in that handler. That breaks callers, because one level out instopWorkerUntrackedthearchiveSessionflag is what picks between the two messages:Archiving is the whole contract of that message. Making it stop archiving breaks the callers that select it on purpose, with no error to show for it, including the two that pass
worker.descriptor.archiveOnStop === true.daemon-supervisor-process.test.tsalso asserts the current behavior in "archives resident roots and cancels their heartbeats on explicit daemon shutdown", and the heartbeat cancellation in that same test reads like the reason to keep it: a stopped daemon cannot run a session's heartbeats, so listing it as live would advertise autonomy it no longer has. Whetherdaemon stopshould leave sessions resumable is a product decision for you rather than a defect to patch.#1288, no setting to raise the worker heap. The report checks
settings.jsonand the CLI flags and concludes there is no way to raise the V8 cap. There is one it did not check.createCliSubprocessLaunchSpecpassesprocess.execArgvthrough to the worker andcreateCliSubprocessEnvspreadsprocess.env, soNODE_OPTIONSreaches workers by inheritance. That does not close the issue, since a running daemon will not pick up a new value and an env var is not a discoverable setting, but it changes the work from "add the only escape hatch" to "promote an existing one", which needs a decision on naming, scope, and precedence against an inheritedNODE_OPTIONS. The retention that fills the heap is described in the now closed #1054 and #1063, and a bigger cap does not address either.#1199, no stale-socket probe. The report states that the existence of the socket file alone is treated as a conflict.
prepareDaemonSocketPathprobes first:An orphaned file is unlinked and reused. There are three of these probes in that file. The error only fires when something accepted a connection, which means the dying worker was still listening at that moment, matching the summary of the report. The later
ss -xashowing no listener fits too, since by then the old process had finished dying. That puts the defect in the restart ordering against a dying predecessor. The descriptor poisoning half of that issue, three strikes to a permanentfailedwith no recovery surface, is independent and looks like the more useful half.I grouped these because they fail the same way. Each one is well written and lands on the right file and line, then draws a conclusion that one more level of indirection contradicts. Anything pulled out of the backlog on the strength of its root-cause section is worth a pass against the code first.
Happy to work through more of the closed queue this way if it is useful, or to go deeper on any one of these.
All reactions