Repository navigation
feat(orcad): add --print-service and --doctor - #17852
klaidliadon wants to merge 4 commits into
Conversation
A data root long enough to push the daemon's endpoint past `sun_path` killed the daemon at startup — but orcad's own listener name is about 5 bytes shorter, so there is a band where orcad came up, reported ready, and served clients with terminal survival silently off. Every terminal then died on the next restart, which is the one thing forking a detached daemon exists to prevent. Found on DSM: a root of 86 characters reproduced it, 84 was fine. orcad now refuses at startup with `ORCAD_EXIT_CONFIGURATION` (78). This is not one of the unenumerable runtime reasons `startOrcadDaemon` fail-opens for: it is known before anything is attempted, permanent until an operator changes a value they supplied, and pure arithmetic. That puts it with the bind address and the instance lock, which already refuse, and `RestartPreventExitStatus=78` keeps a supervisor from restart-spinning on it. The check runs ahead of the instance lock deliberately — it reads nothing and creates nothing, so refusing a root should not first take a lock on it. **The limit is verified against the kernel rather than asserted**, because past it there is no single failure to detect: on Linux a 109-110 byte path fails EADDRINUSE against a free name, and 111+ reports a successful `listen` with no directory entry at all — which looks healthy to everything except the dev+ino reads daemon endpoint ownership is built on. The test binds at the limit and one byte past it and asserts a stat-able entry, so a wrong constant fails it. Supporting changes, all in service of that: - **Consolidate the daemon's endpoint path builders into one leaf module** (`daemon-runtime-paths.ts`), so the budget measures the real names. `daemon-spawner` re-exports `getDaemonSocketPath` / `getDaemonTokenPath` / `getDaemonPidPath` from it rather than moving them, so every existing caller imports the same name from the same place and only the definition moved. `daemon-endpoint-ownership` likewise re-exports `getDaemonSocketBindPath`. - **Measure bytes, not characters.** A non-ASCII root spends more of the budget than its length suggests. `ssh-control-socket` now shares the constant and the byte measurement instead of keeping its own copy of the 104/108 split. Boundary verified end to end against the built orcad: 85 starts with a live daemon socket, 86 exits 78. Refs stablyai#10726 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UacRCfD4XPe29bD3ZWNc57
Two flags for the supervised deployment orcad exists to serve: - `orcad --print-service` generates a systemd unit (and a launchd plist) for a given data root, bind and port. - `orcad --doctor` audits an installed one, then probes the live host — the unit as written, the daemon socket, the data root — and reports what it actually measured rather than what it expected. **`KillMode=process`, not `mixed`.** Measured on systemd 219: under `mixed` a `systemctl restart` destroyed the daemon, its shell, the session and its scrollback; the identical unit under `process` kept all four. `mixed` SIGTERMs the main process and then SIGKILLs everything still in the control group, and `setsid()` escapes the process group, not the cgroup — so the detached daemon dies with it. The generator emits `process` and the doctor reports `mixed` as critical, including in units orcad itself generated before this was settled. **systemd values are quoted, and the readers parse quotes.** Both halves matter together: a data root containing a space would otherwise round-trip into a unit that silently pins a different root, and an audit that could not parse its own output would call every quoted unit malformed. **An unreadable service definition is reported as unreadable, not absent.** Those are different operator actions — one is a permissions fix, the other an install — and collapsing them sent people to reinstall a unit that was already there. The doctor also answers whether the data root leaves room for the daemon's socket, reusing the budget from the preceding commit. It is critical when a unit file pins the root and a warning — naming whose root it measured — when it falls back to the invoking shell's, because those are different claims. `ORCAD_ENTRY_FILENAME` is named once in `orcad-artifacts` because a generated unit execs it. `OrcadArtifact.optional` goes with it: nothing set it, the filter was a no-op, and the one artifact that would want it cannot be an entry there at all since its filename is derived per platform-arch. Field-tested on a Synology NAS (DSM, systemd 219) and on a root runner; several commits' worth of defects found that way are folded in. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UacRCfD4XPe29bD3ZWNc57
|
the model provider had a temporary upstream failure. The provider refused or dropped the request mid-run — nothing in your repo or your credentials is at fault. Re-trigger Pullfrog; if it keeps happening, check the provider's status page.
|
Greptile SummaryThe PR adds systemd and launchd service generation plus host auditing and live diagnostic probes to
Confidence Score: 4/5The PR is not yet safe to merge because generated launchd jobs can still receive a log path that their configured run-as account cannot write. Service generation selects Files Needing Attention: src/main/orcad/orcad-service-command.ts, src/shared/supervisor-service-render.ts
|
| Filename | Overview |
|---|---|
| src/main/orcad/orcad-service-command.ts | Adds service generation and doctor orchestration, but launchd log paths are not derived from the configured run-as account. |
| src/shared/supervisor-service-render.ts | Renders systemd and launchd definitions with quoting, XML escaping, root-account refusal, and terminal-preserving supervisor settings. |
| src/shared/supervisor-service-file-read.ts | Reads generated and installed service definitions and now correctly decodes the XML entities emitted by the launchd renderer. |
| src/main/daemon/daemon-runtime-paths.ts | Centralizes daemon endpoint paths and exposes deterministic Unix socket-path budget checks. |
| src/main/orcad/orcad-entry.ts | Adds an early socket-path preflight and maps permanent path overflow to the supervisor configuration exit code. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
CLI[orcad service flags] --> P{Command}
P -->|--print-service| G[Resolve service configuration]
G --> R[Render systemd unit or launchd plist]
P -->|--doctor| D[Discover and read installed definition]
D --> A[Audit parsed configuration]
D --> H[Probe executable, data root, socket, and endpoint]
A --> O[Report findings]
H --> O
Reviews (2): Last reviewed commit: "fix(orcad): decode plist entities, refus..." | Re-trigger Greptile
…agent log Three defects the review found in the generator/reader pair, each one a case of the two halves not agreeing on the same encoding or the same identity. `readPlistString` returned the XML-escaped text. The generator writes `&` as `&`, so the doctor compared `/srv/a&b` against the caller's `/srv/a&b` and reported the plist it had just produced as pinning a different data root -- and stat-ed an ExecStart path that was never on disk. Decoded on one pass, so a path literally containing `<` survives as itself rather than decoding twice. This is the plist half of the rule the systemd splitter already followed. `assertNotRoot` compared against the name only. systemd's `User=` takes a numeric uid too, so `--user 0` rendered exactly the unit the guard exists to refuse, and the audit read it back as an ordinary account and reported `Runs as 0.` as healthy. Guard and audit now share `isRootAccount`. A LaunchAgent got `StandardOutPath=/var/log/orcad.log`. That path is root-owned on macOS and the agent runs unprivileged, so it cannot open its own log, and launchd reports the failure against the file rather than the job -- it reads as a broken install for a reason the plist never states. The renderer does no I/O and so cannot resolve a home directory, so it now emits the keys only when the caller resolved a path; `printService` supplies `~/Library/Logs/orcad.log` for an agent and `/var/log/orcad.log` for a daemon. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UacRCfD4XPe29bD3ZWNc57
|
All three of Greptile's findings were real and are fixed in 9a05941. Each was the same shape of defect — the generator and the reader disagreeing about an encoding or an identity — so all three got the fix on both halves rather than only where the symptom showed. 1. Plist entities remained encoded (P1). Answered inline. The reader now decodes, one pass, and 2. 3. LaunchAgent pointed at an unwritable log path. Correct, and the failure mode is nastier than it looks: The renderer is pure by contract and does no I/O, so it cannot resolve a home directory, and launchd expands no Verified the escape/decode round trip against a real plist parser rather than only against our own reader: rendering Tests added: four in Note on the confidence score: none of the three touched Pushed as a new commit, not a force-push, so #17853 (stacked on this branch) is undisturbed. |
|
Cross-posting from #17853 (the PR stacked on this one): Greptile reviewed the whole stack there and filed two P1s that are against files this PR adds, so they never appeared on this thread. Both verified against the tree. 1. The generated launchd plist is not well-formed XML
Column 15 is the The systemd renderer at 123-124 carries the same sentence as
2.
|
| user: options.user ?? userInfo().username, | ||
| bind: options.bind, | ||
| port: options.port, | ||
| logPath: platform === 'launchd' ? launchdLogPath(options.scope) : undefined |
There was a problem hiding this comment.
When --user differs from the account generating the launchd definition, UserName uses the requested account while the log path still uses the generator's home directory—or /var/log for system scope—causing the job to open a path its run-as account cannot write and preventing reliable startup or logging.
There was a problem hiding this comment.
Correct, and correct about system scope too — which is the part I had wrong. Fixed in 125bd54.
My previous commit swapped one guess for another. /var/log/orcad.log is root-owned while assertNotRoot guarantees the job runs unprivileged, so a LaunchDaemon cannot create it either; and the generating user's home is the run-as account's home only when --user did not name someone else, which this process cannot resolve.
So no path is defaulted at all now. The renderer emits StandardOutPath/StandardErrorPath only when the caller passes one explicitly, and printService no longer passes anything — it emits a warning instead, naming both reasons and telling the operator to create a file the run-as account owns, add the two keys, and pair it with a newsyslog.d entry since nothing rotates it. Guessing a path the job cannot open is worse than naming none, because launchd reports that failure against the log path rather than against the job, so it reads as a broken install for a reason the file never states.
systemd is unaffected: journald captures the unit's output with nothing configured, which is exactly why the asymmetry is now written down rather than papered over with a plausible default.
…essing a log path Every generated plist was malformed XML, in both scopes. The header comment carried the regeneration command verbatim, and XML forbids the string `--` anywhere inside a comment body -- so `--print-service` made the document unparseable. expat rejects it, `plutil -lint` rejects it, and launchctl will not load the job: the entire darwin path was dead on arrival. The flags are now named rather than written, with the constraint stated in the file so it is not "fixed" back. renderSystemd carries the same sentence safely because `#` comments have no such rule, which is why this was darwin-only. Nothing caught it because no test parsed the output it asserted against. There is now a well-formedness assertion over both scopes, plus one proving a `--` in a generated path cannot reach a comment. `--service-path` extended conventional discovery instead of selecting. Naming the definition discovery already finds -- the obvious way to ask for a report on your own install -- collected that file twice, so `auditDuplicates` reported `multiple_services_one_root` as CRITICAL with exit 1 while every live probe was skipped as ambiguous. A false critical produced by naming the file you wanted audited. An explicit path now selects. The launchd log path guessed an account's home. `UserName` honours `--user` while the path used the generating user's home, and `/var/log` is root-owned while the job runs unprivileged -- so both scopes could emit a log the job cannot open, which launchd reports against the path rather than the job. No path is defaulted now; generation warns and says what to add, since none can be shown writable by the run-as account from here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UacRCfD4XPe29bD3ZWNc57
|
Both cross-posted findings from #17853 verified and fixed in 125bd54, along with the log-path P1 filed here. 1. Malformed plist XML — confirmed, and worse than "a lint nit"This one was real and I had already missed it once: my earlier verification of the plist used a hand-rolled replica that omitted the comment header, so it passed. Re-running against the actual template from XML forbids The flags are now named rather than written ( Re-verified all four variants (both scopes x with/without log keys) against the real template with Python 2.
|

ELI5
Setting up
orcadas a supervised service meant hand-writing a systemd unit andhoping.
orcad --print-servicewrites one for you;orcad --doctorreads aninstalled one back, probes the live host, and tells you which settings would
destroy your running terminals.
What Changed
Two flags on
orcad, and the modules behind them:orcad --print-servicegenerates a systemd unit (and a launchd plist) for agiven data root, bind and port. Writes the file to stdout; placement and enable
hints go to stderr, so redirecting stdout gives you the file and nothing else.
orcad --doctoraudits an installed definition, then probes the live host —the unit as written, the daemon socket, the data root — and reports what it
measured, not what it expected.
Supporting modules under
src/shared/supervisor-*(render, audit, probe,evidence audit, file read) and
src/main/orcad/supervisor-*(discovery, hostobservations, generation warnings, daemon socket budget).
Why
Three things reviewers will want to look at directly:
1.
KillMode=process, notmixedMeasured on systemd 219: under
mixed, asystemctl restartdestroyed thedaemon, its shell, the session and its scrollback; the identical unit under
processkept all four.mixedSIGTERMs the main process and then SIGKILLs everything still in thecontrol group — and the detached daemon is in that control group, because
setsid()escapes the process group, not the cgroup.So the generator emits
process, and the doctor reportsmixedas critical —including in units orcad itself generated before this was settled, which is why
the finding names the generator as a possible source rather than assuming an
operator wrote it.
The cost of
processis real and stated rather than hidden: systemd no longerreaps what the runtime leaves behind. orcad owns that lifecycle through the daemon
pid-record it adopts on the next start, and a supervisor that "helpfully" cleans up
is exactly what breaks the guarantee.
2. systemd values are quoted, and the readers parse quotes
Both halves matter together. A data root containing a space would otherwise
round-trip into a unit that silently pins a different root — and an audit that
could not parse its own generator's output would call every quoted unit malformed.
supervisor-service-file-read.tsexists for the reading half.3. Unreadable is not absent
A service definition that exists but cannot be read is reported as unreadable, not
missing. Those are different operator actions — one is a permissions fix, the other
an install — and collapsing them sent people to reinstall a unit that was already
sitting there.
The doctor also answers whether the data root leaves room for the daemon's socket,
reusing the budget from #17851. It is critical when a unit file pins the
root, and a warning naming whose root it measured when it falls back to the
invoking shell's — because those are different claims and conflating them would
have the doctor assert something it did not check.
ORCAD_ENTRY_FILENAMEis named once inorcad-artifactsbecause a generated unitexecs it.
OrcadArtifact.optionalgoes with it: nothing set it, the filter was ano-op, and the one artifact that would want it cannot be an entry there at all
since its filename is derived per platform-arch.
Linked Issue
Refs #15956 — that issue asks for the PTY daemon to survive a supervised restart,
and lists documenting/shipping a correct unit as one acceptable shape. This gives
operators a generator and an audit that get
KillModeright; it does notrestructure the daemon's lifetime, so
Refsrather thanFixes.Visual Proof
N/A— CLI output on a headless runtime, no UI surface. Sample generated unit anddoctor output can be pasted into review on request.
Testing
Unit-level coverage for the render, audit, file-read, generation-warning and socket
budget modules, including the quoting round-trip (generate → read back → same
values) and the unreadable-vs-absent distinction.
Field-tested on a Synology NAS (DSM, systemd 219) against a built artifact, and
on a root runner. Several commits' worth of defects found that way are folded in —
the
KillModemeasurement, the fallback-root warning, and the unreadable-servicecase all came from running this against real units rather than fixtures.
Not run by me in this environment:
pnpm lint,pnpm typecheck,pnpm testand
pnpm build. The host this was developed and field-tested on cannot installthe full toolchain. The gates were run separately and reported clean; I'd rather
say that plainly than tick a box I didn't personally execute. CI is the authority
here.
AI Disclosure
Claude Opus 5 wrote the commits and this description. I reviewed them and ran the
NAS field testing.
Review
Windows is not a supervised-orcad target and the command says so rather than
emitting something unusable. Path handling goes through
node:path.the intent — it is a host-setup tool — and the wording says so, so nobody reads
its answer as being about a paired remote runtime.
invocation. Nothing runs at runtime or in a hot path.
operator places the file themselves, so nothing here needs elevated privileges.
Generated units pin an explicit data root and default to a loopback bind,
with widening left as an explicit operator choice. The doctor reads service files
and reports parse results; it does not print credentials or tokens.
orcad's flag surface. The onlyshared-file change is
orcad-artifacts, whose removedoptionalfield has noconsumer in the tree.
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Stacked on #17851 (
fix/daemon-socket-path-budget) — that PR consolidates the daemon'sendpoint path builders, which is what lets the doctor ask about them without
pulling the daemon into the CLI's import graph. Please land that one first; this
diff is only readable on top of it.
feat/orca-supervisor-commandstacks on this in turn and adds theorca supervisor print|doctorfront door.The companion docs correction is a separate PR so it can land immediately.
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)