fix(tray): glance clause/header consistency, saturated-core guard, instance root, lifecycle journal - #945
Conversation
The glance error clause split on the first bare '.' or ':', which lands inside the two tokens mcpproxy errors are most likely to carry: a server:tool identifier and a host:port. "invalid arguments for memory:create_entities: at '/entities': ..." rendered as "invalid arguments for memory", reading as an accusation against the server. A separator now ends a clause only when whitespace or the end of the message follows it. The header could also disagree with the list under it, two ways: - The Clients segment counts every client while the list caps at five, so "8 active - 3 idle" sat above five rows with nothing to account for the difference. A truncated list now carries a "+N more" row; gaining or losing that row is structural, N alone is rewritten in place. - The call count came from the 30s usage poll while the rows arrive over SSE, so a call with a row on screen was not in the number above it for up to half a minute. The header now reads glanceCallsThisHour(now:), which adds the live calls the poll has not answered for yet and drops them the moment it does. Only records the core's own aggregate counts (type tool_call) are added, so the correction cannot overshoot. Related #934
…ournal Three macOS-tray changes that share the path-resolution seam. #936 — instance root override. The tray resolved every path through homeDirectoryForCurrentUser, which ignores $HOME, so a dev build run out of a scratch bundle still read and wrote the real ~/.mcpproxy: two QA runs could not proceed in parallel, and the autostart sidecar wrote the scratch bundle's login-item state over the user's real one on every launch. InstancePaths now resolves the socket, config, database, sidecar and journal in one place, relocatable with MCPPROXY_HOME, and a core this tray spawns is handed the same root (--data-dir/--config). Unset in normal use and every path resolves exactly where it did before. MCPPROXY_SOCKET_PATH still outranks the root. An over-long socket path (sockaddr_un caps sun_path at 103 bytes) is reported instead of failing to bind in silence. #933 — a live core with a saturated listen backlog is no longer launched over. onlyEverRefused treated "refusals, nothing ever accepted" as proof of a stale socket, but on macOS a full backlog is ECONNREFUSED too, so a core that was already saturated when the episode began looked identical to a dead one's leftovers. DataDirectoryLock asks the question the socket cannot: does a live process hold bbolt's exclusive lock on the data directory? The kernel drops flock on process death, so there is no such thing as a stale one. When it is held the tray waits again — the attach watch picks the core up as soon as it drains — instead of spawning cores that die on that same lock. A refusing socket whose database nobody holds is still escalated over, so a genuinely stale socket does not deadlock startup. #862 — shutdown-reason logging and lifecycle diagnostics. The tray recorded nothing about its own lifecycle, so a silent exit was unattributable after the fact and macOS purges the Info tier of the unified log within hours. Every start and stop now lands in <root>/tray-lifecycle.jsonl and in os_log at Notice level, with a reason, an uptime and a pid: app launch/termination, caught signals, core spawn/terminate/exit, and one line per periodic update check. The reason is claimed by whoever initiates the stop (first claim wins), and a shutdown nobody claimed is recorded as unattributed rather than as something plausible. A run that recorded no ending is reported at the NEXT launch, which is the only way a SIGKILL-class death (jetsam, pkill) is ever attributable. Related #936 Related #933 Related #862
Deploying mcpproxy-docs with
|
| Latest commit: |
29dc4ac
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://be059da6.mcpproxy-docs.pages.dev |
| Branch Preview URL: | https://fix-tray-glance-lifecycle.mcpproxy-docs.pages.dev |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
📦 Build ArtifactsWorkflow Run: View Run Available Artifacts
How to DownloadOption 1: GitHub Web UI (easiest)
Option 2: GitHub CLI gh run download 30688597906 --repo smart-mcp-proxy/mcpproxy-go
|
… own sidecar A relocated tray spawns its core with both --data-dir <root> and --config <root>/mcp_config.json (#936). On a root nothing has used before, that config does not exist, and LoadFromFile refuses a missing explicit path — so the documented flow could never come up: the core exited immediately with "no such file or directory" and the tray's retry ladder ran out against it. The seeding is done at the serve entry point rather than inside LoadFromFile, which is also the hot-reload path: a config file that vanishes under a running proxy must fail the reload and keep the live config, never become a fresh default with no servers in it. Second half of the same instance: the core read its autostart sidecar from a hardcoded ~/.mcpproxy, so a QA instance reported the real install's login-item state as its own while the sidecar its own tray wrote was read by nothing. The reader now follows the data directory. Verified live with the exact argv the tray emits, on an isolated port: a root that did not exist comes up (/ready 200), and with the real ~/.mcpproxy sidecar saying enabled:false the QA instance reports its own enabled:true. Related #936
…f remembering it The saturated-core guard latched its verdict: once a live process was seen holding config.db, dataDirectoryHeld stayed true for the rest of the episode, and the only path that re-probed the lock was gated on evidence that same flag made unreachable. So a saturated core that then died — SIGKILL, jetsam, kill -9 — left its socket file behind and the tray re-armed its deadline on a fact that had stopped being true. It sat in .waitingForCore forever, a state that by its own design offers neither Stop nor Retry, and quitting the app was the only way out. Before the guard existed the same sequence recovered at the first deadline. A lock is a fact about right now, not accumulated evidence: flock is released by the kernel the instant its holder dies. So it is asked fresh at each deadline and never carried between them, and onlyEverRefused goes back to being purely what the socket said. Related #933
…s lethal Three things wrong with the lifecycle diagnostic, all of which made it worse than no diagnostic. The clean/unclean classifier read only the LAST record. Production writes appTerminating first — so a reason exists whatever happens next — and then tears the core down, appending coreTerminated after it. Every logout, restart, launchd stop and caught signal therefore came back at the next launch as "this was SIGKILL-class (jetsam, pkill, power loss) or a crash". The question is whether the previous RUN recorded a reason, so it is now asked of the records since that run's appLaunched. SIG_IGN on TERM/INT/HUP was wrong twice. An ignored disposition survives execve, and Go's runtime deliberately preserves an inherited SIG_IGN for SIGHUP and SIGINT, so every core the tray spawned went on ignoring kill -INT and kill -HUP for its whole life. A no-op sigaction handler suppresses the default action just as well, is all EVFILT_SIGNAL needs, and is reset across execve. And delivery ran on the main queue only, so a wedged main thread turned pkill -TERM, launchctl kill TERM and a launchd stop into no-ops — the signal ignored and nothing left to act on it. Handling now runs on its own queue, the hop to main is made by the terminate closure, and if the app is still here five seconds later the signal's default action is restored and re-raised. Catching a termination signal may delay the exit; it must never prevent it. Related #862
Two ways the header still contradicted the rows under it. The live-call list was cleared on ANY completed poll, with no snapshot boundary and no call identity, while the poll is an await across the network and SSE rows land on the same actor while it is in flight. A response that had snapshotted 12 before a live call happened deleted that call's increment, so the header fell from 13 back to 12 under a visible thirteenth row and stayed there for up to 30 seconds; the reverse ordering double-counted a call the poll had already included. Both are settled by one boundary — the instant the poll was ISSUED — so a live call counts only while it is strictly newer than the poll currently answering. And the polled total carried no hour of its own while the live half was already filtered by hour, so after a UTC rollover the two halves described different hours and the header reported the previous hour's total as this one's. The total now records which hour it is for and is dropped when that hour ends. Related #934
Cross-model review round — all 7 findings fixed (TDD)Four commits on top of P11. A fresh The tray emits Red: Green (4 tests) + live, using the exact argv the tray emits against a root that did not exist, on an isolated port: 2. Once the data-directory lock was seen held, the tray could wait forever — Confirmed: A lock is a fact about now, not accumulated evidence (the kernel drops Red: Green — and the two existing cases (live saturated core is never launched over; a genuinely stale socket still is) both still pass. 3. Every logout / signal shutdown reported as a SIGKILL-class crash — Confirmed. Red: P24. Header could jump backwards, or overcount, around each usage poll — Confirmed. Fixed with a single boundary: the instant the poll was issued (stamped before the 5. Confirmed, including that the old test enshrined the wrong value. Red: 6. Both halves confirmed and both fixed. Three tests, including one that blocks the main thread on a semaphore and requires a real 7. The relocated autostart sidecar was written but never read — Confirmed. GatesPre-push hooks ran
Deliberately not done
|
…ast iteration The poll test compared the tick count after cancellation against the count taken at cancellation. Those two are not the same thing: the cancellation can land while an iteration is already suspended inside the source call, past that iteration's cancellation check, and that iteration still unwinds through the sleeper once. On a loaded runner that settling tick arrives after the snapshot, so CI saw 4 where it wanted 3. Wait for the loop to RETURN instead of for a slice of wall clock, cap the settling at the one in-flight iteration, then sample the counter twice to show it is frozen. Verified the teeth: with `Task.isCancelled` removed from loadList all three assertions fail (86277 ticks past the ceiling); restored, 25x2 runs under full CPU load are green where the old shape failed 3 in 15. Also stop the sheet-attachment test skipping itself on a slow runner (poll for the attachment rather than sleeping 0.2 s), and widen the menu-open drain window, which is the only thing standing between an async fetch regression and a vacuous pass. Related #945
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Live QA verified: error clause no longer splits inside server:tool identifiers, glance header reconciles with the list, saturated-but-live core no longer launched over (all 4 lock states probed), instance-root override, lifecycle shutdown journal. All checks green incl. E2E.
2 participants
Four macOS tray issues, one branch. All Swift; no Go touched.
#934 — glance error clause cut mid-identifier, header disagreeing with the list
Error clause.
firstClausesplit on the first bare.,:or newline, which lands inside the two tokens mcpproxy errors are most likely to carry — aserver:toolidentifier and a host:port.invalid arguments for memory:create_entities: at '/entities': got string, want arrayrendered asinvalid arguments for memory, reading as an accusation against the server. A separator is now a clause boundary only when whitespace or the end of the message follows it. Full message still in the tooltip.Header vs list, both causes from the issue:
+N morerow. Gaining or losing that row is structural (waits for menu close, FR-023); N alone is rewritten in place, so a twelfth client cannot leave+6 moreunder a header that counted twelve.AppState.glanceCallsThisHour(now:)— the polled total plus live calls the poll has not answered for, dropped the moment it does, so the two cannot accumulate. Only records the core's own aggregate counts (type == "tool_call", perUsageAggregate.Apply) are added, so the correction cannot overshoot and jump back at the next poll.#933 — a saturated-but-live core is no longer launched over
onlyEverRefusedread "refusals, nothing ever accepted" as proof of a stale socket. On macOS a full listen backlog isECONNREFUSEDtoo, so a core already saturated when the episode began is indistinguishable from a dead core's leftovers to every socket probe there is.New signal, independent of the socket:
DataDirectoryLockasks whether a live process holds bbolt's exclusiveflockon the data directory (<socket dir>/config.db— derived from the socket, so a tray pointed at another instance asks about that instance). The kernel releasesflockon process death, so unlike a leftover socket file it cannot be stale. It is taken withLOCK_SH | LOCK_NBand released immediately: shared still conflicts with bbolt's exclusive lock, while two trays probing at once do not report each other as live cores.When the lock is held the tray re-arms the deadline and keeps waiting, with the attach watch still running underneath, so the core is picked up as soon as it drains — instead of spawning cores that die on that same lock and ending on a retry-exhaustion error about a healthy core. A refusing socket whose database nobody holds is still escalated over, so a genuinely stale socket does not deadlock startup.
#936 — instance-root override (the half that had not shipped)
InstancePathsresolves the socket, config, database, autostart sidecar and lifecycle journal in one place, relocatable withMCPPROXY_HOME. A core this tray spawns is handed the same root (serve --data-dir <root> --config <root>/mcp_config.json); with the variable unset the command is byte-for-byte["serve"]as before and every path resolves exactly where it did.MCPPROXY_SOCKET_PATHstill outranks the root (it names one core, not one instance). A socket path oversun_path's 103 bytes is reported at launch rather than failing to bind in silence. Documented indocs/tray-debug.md, including what is deliberately NOT relocated (the core's log directory, and the preferences domain —cfprefsdhonours neither$HOMEnor this variable).#862 — shutdown-reason logging and lifecycle diagnostics
<instance root>/tray-lifecycle.jsonl(JSON lines, so a kill mid-write costs the last record and not the file) plusos_logat Notice — the Info tier is purged within hours, which is why the original incident had no context left. Records:appLaunched,appTerminating,signalReceived,coreLaunched,coreTerminated,coreExited,updateCheck; each with a reason, the tray's uptime and a pid.applicationWillTerminateis told the app is going, never why.unattributed (no initiator claimed this shutdown)rather than as something plausible.SIGTERM/SIGINT/SIGHUPare caught, recorded and routed through the normal quit path; logout/restart is claimed viaNSWorkspace.willPowerOffNotification.appTerminatingis reported at the NEXT launch, with the last thing it did and how long it had been up — the only way a SIGKILL-class death (jetsam,pkill) is attributable at all.Under XCTest the shared journal goes to a scratch file: the suite drives the real
CoreProcessManager, so without that it would append phantom cores to the developer's own live install. Pinned by a test.Verification
cd native/macos/MCPProxy && swift testBaseline before this branch was 720 tests; 46 added.
Red-before-green, e.g. #933. With
DataDirectoryLock.probestubbed to always answer.free(i.e. the pre-fix behaviour), the new test reproduces the reported bug exactly — doomed spawns over a live core:and with the real probe:
The #934 and #936/#862 suites were likewise written first and run red (missing symbols / wrong header count) before the implementations landed.
No Go files touched, so
scripts/run-linter.shandmake swaggerare not in play. Nothing under~/.mcpproxywas created or modified by the test runs (checked explicitly after each full run — the journal isolation test exists because the first run did create one).Deliberately deferred
cmd/mcpproxy/main.go), so the gap this PR closes is the tray side. Streamable-HTTP session resilience across restarts is explicitly tracked separately.cfprefsdignores both$HOMEandMCPPROXY_HOME; a dev instance still needs a distinct bundle id for isolated defaults, and the first-run gate lives there. Documented rather than worked around.