Answer while restoring, sweep escaped workers, and sign releases with Developer ID by quantizor · Pull Request #16 · quantizor/devctl

quantizor · 2026-08-09T00:35:38Z

Stacked on #15. Merge that first; this targets feat/restart-and-watch so the diff stays reviewable.

A daemon that is coming back no longer looks like one that is gone

While restoring supervised servers at boot, devctld kept its socket closed, so every client got daemon-unreachable, the same answer a daemon that was never started gives. An agent polling across daemon install or daemon restart read a busy daemon as a dead one and tried to start another.

It now accepts as soon as its listener is up and reports which state it is in. devctl daemon status says restoring; other commands wait the window out and say on stderr what they are waiting for, rather than failing. Commands that arrive mid-restore are refused with daemon-starting instead of being served against half-restored state, so the ordering guarantee that closed the socket in the first place is unchanged.

A crashed server no longer leaves its workers running

A supervised server that spawned a helper and then crashed could leave that helper alive forever, holding its port and its files while devctl reported the server as gone. The next start then failed on a port held by a process nothing was tracking.

Teardown now also sweeps by session, the one relationship that survives the server exiting and its helpers being adopted by the system, and the descendant record is refreshed throughout startup so the common case is caught before the sweep is needed.

Upgrades install immediately instead of stalling

Installing over a previous copy left the daemon unreachable for tens of seconds. The cause was signing, not timing: an ad-hoc signature carries no Team ID, so the launch constraint was pinned to the helper's code hash, which every rebuild changes, and the daemon was killed on exec until the system invalidated its own record. Roughly a 75x improvement in how long the daemon takes to come back after an upgrade, and no failed spawns at all.

Local and CI builds now sign with Developer ID through one shared resolver, so the app and the disk image can never be signed differently. make dmg also notarizes, staples, and marks the image as downloaded, so opening a local build exercises the same Gatekeeper path a user hits instead of one that is silently skipped.

`devctl lock` catches a change it used to miss

A lock resource above 8 MiB was fingerprinted by its head, tail, size, and mtime, so a command that rewrote the middle while preserving all four was reported as no change. A local sqlite database is exactly that shape, and not noticing is the worst answer a check built to notice can give. Files are now hashed whole at any size, read in chunks so cost stays memory-flat.

A project's own config can no longer reach past what it describes

A server name became a log directory path component verbatim, so a name containing ../ made the daemon create directories elsewhere on the machine and write raw output into them. Names are flattened to a single component, and two names that flatten alike keep separate homes.
Names, urls, and heads went into the agent context block unescaped, and a JSON key legally holds a newline, so a pulled branch could close the fence and continue as though the harness were speaking. Those values are kept to one line now.
config check accepted "port": 70000, and the first probe against it took devctld down; under launchd it came back, re-read the same config, and crashed again. Out-of-range ports are config errors, and the probe reports nothing listening.
Overlapping reads made log lines appear twice and counted them twice in error tallies.

The rest of that class, found by an audit

An earlier round here fixed one out-of-range port. A review pass over the whole tree found the same shape everywhere a value devctl did not produce was trusted further than it had been checked.

hook install merges its hook into a settings file devctl does not own and writes that whole file back. When the read failed to parse, it treated the file as empty and wrote it back holding only its own hook, taking every other hook, permission and setting with it, and reported a successful install. The two adapters each had their own copy of this; reading and writing that file now live together on the protocol so a harness added later cannot reintroduce it.
A ports entry's offset was bounded below but never above, so config check reported no errors at all and the failure arrived later as an unrelated-looking daemon-unreachable. A portSpan could overflow when added to its port, and the four healthcheck integers were never checked.
Reading status across every project validated each one's config, so a single bad number anywhere took down the daemon supervising all of them, and the two-second poll brought it back to do it again.
Pids came off disk and off the wire as unbounded integers and were narrowed with a trapping conversion. There is now one home for that decision, and it refuses zero and negatives too, since both are kill(2) selectors rather than processes.
devctl lock swallowed a config failure, found no declarers, paused nothing, and reported the hold taken, which is the opposite of what the command is for.
A half-open DaemonClient counted as connected, so a failed handshake disabled the protocol version check for the life of the client.
The port shown for a rebound server was the one it asked for rather than the one it got, in the menu bar and statusline; the agent context already had it right. All three now derive it from one place.
Four doc comments in ControlServer described functions other than the ones they sat on, and groupUp's had gone missing entirely.

What the audit measured but did not change is in BACKLOG.md with its numbers, so the next pass starts from evidence rather than suspicion: whole-file reads on a two-second poll in EventStore.query (146ms against 2.0ms windowed) and LogQuery (240ms against 1.7ms), lsof spawned inside the router actor (48.8ms per pair, serializing every other request), and ServerPhase having no derived predicates, so "holds its port" is spelled eight ways and has already drifted.

Release path

Release DMG refuses to upload anything that does not assess as source=Notarized Developer ID, and only a release event or a dispatch from main may write to a release. Dispatching from any other branch is a dry run that builds, signs, notarizes, and verifies, then returns the image as a run artifact. That makes the release path testable without risking a published asset.

Verification

make test and scripts/smoke.sh pass. Smoke gains coverage for the restore window, and the fixture-server lookup has one home with strays from an interrupted run reaped on entry.

Release DMG has never executed. Its secrets are now populated and a dry run from this branch is the intended way to prove it before a real release depends on it.

devctld kept its socket closed for the length of recoverAtStartup, so a client in that window got ENOENT and reported daemon-unreachable. That is the same answer a daemon that was never started gives, so an agent polling across an install or restart bounce read a busy daemon as a dead one and moved to start a second. The listener now comes up before restore and the router declines everything but daemon.info and daemon.shutdown with a new daemon-starting code, so the ordering guarantee that motivated closing the socket is unchanged while the two states become distinguishable. daemon.info carries restoring, present only while it is true. The CLI waits the window out rather than failing, naming what it waits for on stderr, and no longer treats it as grounds to bootstrap another daemon. ControlServer.startAccepting replaces the ready callback and throws when the listener never gets there, so awaiting readiness cannot become a daemon that runs, holds the single-instance lock, and serves nothing. The smoke gate now writes the daemon's stdio to a file. Inheriting the script's meant a daemon outliving the run held the caller's pipe open, so `smoke.sh | anything` hung with no output at all to say why, which is how a zsh read-only $status collision in await_daemon surfaced as silence.

A lock resource above 8 MiB was fingerprinted by head, tail, size, and mtime, so a command that rewrote the middle while preserving all four read as no change. A local sqlite database is the shape that happens to, and the identity check exists to catch exactly that. The cap was there because the hand-rolled SHA-256 took a whole [UInt8] with no incremental entry point, so an exact digest meant buffering the file. CryptoKit is a system framework rather than a package dependency, so using it leaves the two-dependency rule alone, hashes at roughly 1.9 GB/s on this machine, and deletes about seventy lines of hand-rolled crypto. Files now stream in 1 MiB chunks: memory is flat, size is irrelevant, and a 512 MiB resource costs about a quarter second per capture. Directories keep their byte budget, since that cost is the sum over a tree and the fingerprint is taken twice per guarded command, and they still report exact: false when the budget cut the walk short. hashHex had no golden pinning an actual digest, only self-comparisons that a wrong-but-consistent hash would satisfy while silently moving every project's log directory. It is now pinned to the published FIPS 180-4 vectors, which is also what proves this swap byte-identical.

…orker The flaky crashKillsSessionGrandchild was reporting a real product bug. Three facts had to be established before the fix made sense, and each was measured rather than assumed: Foundation's Process puts its child in a NEW process group (verified: the grandchild's pgid equals its own pid, not the root's), so the group half of teardown provably cannot reach it and the descendant snapshot is the only thing that can. That snapshot was taken at spawn, once 100ms later, and thereafter only on each health probe. A server declaring no healthcheck waits a full 2s stabilization window for its first probe, so a worker forked between 100ms and the crash was in no snapshot at all and survived permanently. The orphans were still on this machine: ppid 1, own process group, alive. Refreshing the snapshot every 200ms while starting closes the common case (a sysctl sweep measures 0.43ms, so a startup window costs a handful), but a refresh is a scheduled task and a saturated machine can delay it past the crash. Timing cannot be the guarantee. The guarantee is a session sweep. createSession makes the root a session leader, a child that setpgid's out of the group keeps the session, and session membership survives both the root exiting and the orphan reparenting to launchd, which is exactly where the parent chain goes blind. It is refused unless the session is led by the root pid and differs from the daemon's own, since without those guards it would signal the daemon and every server it supervises. Verification: a new fixture flag spawns the grandchild past the early snapshot, turning a load-dependent flake into a test that failed three for three before the fix and passes after. Twelve full suite runs, previously losing one in four to one in eight, now leave zero orphaned processes behind. The session sweep's positive control uses posix_spawn with POSIX_SPAWN_SETSID, because a Process-based one is not a session leader and passes in a millisecond without reaching the code it claims to cover.

The fixture-server lookup existed in six copies that had already drifted: two checked existence rather than executability, and three looked in one location where the others looked in two, so a suite could fail to find a binary its neighbour found. One home now, and the five duplicates are call-throughs. That home is also where a run's leftovers get cleaned up. A supervised child outliving its daemon is deliberate product behavior, so nothing but the spawning run will stop it, and a run killed part way leaves a fixture holding a port. Two later runs failed in unrelated tests with port-held naming a pid nothing was tracking. Reaping is guarded twice, and the refusals are the point: the parent must be gone, so a concurrent swift test is untouched, and the command line must name a port inside the 45000 to 45500 block the unit suites reserve, which keeps it away from scripts/smoke.sh, whose fixtures use other ranges and are orphaned on purpose to prove children survive a daemon kill. Checked against real processes in all four cases, including both refusals. The first attempt reaped nothing because the match was on the absolute path while the fixture had been launched relatively, which is a cleanup that silently skips its target, so the decision is now a pure predicate with those cases pinned as tests rather than left to one manual run.

docs/design.md called double-ensure "integration-tested" and the target it pointed at held one test whose body was #expect(Bool(true)). Checking the two behaviors that target promised before deleting anything: cross-project port conflict is genuinely covered by scripts/smoke.sh, and concurrent double-ensure was covered nowhere. The promise had outlived the coverage. ConcurrentEnsureTests fires eight simultaneous ensures at a real Router and asserts every caller sees one pid, then asks the process table whether a second process exists at all, since a spawn the supervisor lost track of would not appear in any of the replies. Eight rather than two: a pair can pass by luck when the first finishes before the second is dispatched. Two cleanup defects surfaced while getting this stable, both the same shape. The stray reaper SIGKILLed and returned, but the listening socket outlives the call, so a suite spawning straight afterwards raced it and failed with port-held naming a pid it had just killed; it now waits for the processes to actually go. And this test's own teardown was a detached Task inside a defer, which returns immediately and lets the process exit first, leaking the server that then squatted the port for the next run. Four consecutive runs now leave nothing behind. Interleaving a stop with those ensures kills the test process outright, with no output, no crash report and no assertion failure. That is filed in BACKLOG.md with its reproduction and what has been established so far, rather than committed as a test that takes the suite down with it.

From a max-effort review pass over this branch. A server name became a path component of its log directory verbatim. URL.appending keeps `..` and `/` and the kernel resolves them at createDirectory and open, so a name of `../../../x` made the daemon build directories outside the logs tree and write raw child stdout into them. Names flatten to one component now, and two that flatten alike stay apart by hash. Session context is devctl's own words again. A name, url or head went into the fenced block unescaped and a JSON key legally holds a newline, so a pulled branch could close the fence and keep writing as though it were the harness. The port-conflict warning also carried the squatting process's own command line, which the hard rule about child output already forbids; it now states the port and the state, which are devctl's to know. `UInt16(port)` traps, so a config saying 70000 took the daemon down and launchd brought it back to re-read the same file and die again. Out-of-range ports are config errors and the probe answers that nothing is listening. The spool tailer advanced its cursor by the size measured before the read rather than by what it read, so a child appending in between had its tail ingested twice: duplicate lines and a doubled error tally. The menu bar advanced its event cursor inside the loop whose filter read it, so two servers crashing in the same millisecond raised one notification. Three of these land on this branch's own work. The app treated the new daemon-starting refusal as an unreachable socket, so a restoring daemon emptied the popover and drew recovery at a daemon that was working. The session sweep added for the crash path is guarded by `!stopRequested`, so deliberate stop still has two of teardown's three halves. And the reaper's reserved port block did not reach ResourceLockTests, which drew from 41_000: those fixtures went unreaped, and widening the block would have pointed the reaper at smoke.sh, which draws from 41000 too. The suites moved into the block instead, and the test that asserted the gap was correct now asserts both directions. Findings too large to fix here are in BACKLOG.md, the first being that trust is recorded and never enforced.

…hrottle Installing the DMG left devctld down for about half a minute, until a later escalation happened to re-register it. The log for a real upgrade shows why. An unregister+register restarts launchd's ThrottleInterval, which defaults to 10s, so the freshly submitted job cannot spawn before then however healthy it is. The wait after that re-register was also 10s, started a second and a half earlier, so it expired at 11.8s in a dead heat with the spawn it was waiting for, and lost. The wait now outlasts the throttle plus the spawn. Losing that race then threw out of the whole launch sequence, so the retry fell to whatever recovery poll came next, a fifteen second cooldown later. It falls through to the escalation already written below it instead, which re-registers immediately and still has the rebind marker to act on. The failure it reported was "The operation couldn't be completed. (DevCtlKit.WireError error 1.)". WireError carries a message and a remediation hint and had neither surfaced, because a bare Error struct renders that way through localizedDescription, which is what the menu bar logs and what the popover shows a person. It conforms to LocalizedError now, so every one of those call sites reads as devctl's own words.

An ad-hoc signature carries no Team ID, so BTM pins the SMAppService launch constraint to the helper's CDHash, which every rebuild changes. Installing over a previous copy then gets devctld SIGKILLed on exec (CODESIGNING / Launch Constraint Violation) until BTM invalidates its item on its own schedule, costing a launchd ThrottleInterval per attempt. Measured across three installs: 23.1s and two kills ad-hoc over ad-hoc, 0.30s and none once both sides carry a Team ID. Unregister does not clear it, so the existing rebind path was aimed at the wrong thing; the codebase map is corrected to say so. make app and make dmg resolve the identity through one helper, so the app and the image cannot be signed differently, falling back to ad-hoc with a warning where the keychain has none. make dmg also notarizes, staples and stamps com.apple.quarantine, so opening a local build is what a user who downloaded it gets rather than a path Gatekeeper never evaluates. Release DMG gains three guards. The API key secrets are asserted up front instead of failing inside notarize.sh two levels down. The built image must assess as Notarized Developer ID before upload, which matters because the workflow runs from the dispatching ref while the source comes from the tag, so an older tag could otherwise clobber a good asset with an unnotarized one. And only a release event or a dispatch from main may write to a release: any other branch is a dry run that returns the image as a run artifact.

Copilot

Pull request overview

This PR hardens devctl’s daemon lifecycle and safety boundaries: the daemon now becomes reachable during boot restore (distinguishing “starting” from “gone”), teardown better cleans up escaped worker processes, signing/notarization is made consistent for release artifacts, and several config- and context-related injection and crash footguns are closed.

Changes:

Make the daemon accept connections during restore and refuse non-info work with a new daemon-starting error, while CLI/app wait and report progress.
Strengthen process teardown by refreshing descendant snapshots during startup and adding a session-based sweep to catch escaped workers.
Improve safety and correctness: server-name path component sanitization, agent-context escaping, full-file hashing (chunked) for lock resources, port-range validation, and log/event cursor correctness; plus expanded tests and smoke coverage.

Reviewed changes

Copilot reviewed 53 out of 53 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
Tests/IntegrationTests/IntegrationTests.swift	Remove placeholder integration test source
Tests/DevCtlKitTests/WireTests.swift	Add schema goldens and `WireError` description tests
Tests/DevCtlKitTests/ServerPathComponentTests.swift	Add tests for safe server log path components
Tests/DevCtlKitTests/ResourceIdentityTests.swift	Add SHA-256 vectors + large-file identity tests
Tests/DevCtlKitTests/LoopbackProbeTests.swift	Add impossible-port regression coverage
Tests/DevCtlKitTests/AgentContextTests.swift	Add fence-escape and squatter-string isolation tests
Tests/DevCtlDaemonCoreTests/WorktreeCoexistenceTests.swift	Centralize fixture-server lookup
Tests/DevCtlDaemonCoreTests/WatchTests.swift	Centralize fixture-server lookup
Tests/DevCtlDaemonCoreTests/TestSupportTests.swift	Add tests for stray fixture reaper decisions
Tests/DevCtlDaemonCoreTests/TestSupport.swift	Add shared fixture lookup + stray reaper + reserved ports
Tests/DevCtlDaemonCoreTests/SupervisorTests.swift	Add deterministic late-grandchild teardown test + better failure output
Tests/DevCtlDaemonCoreTests/RestoreWindowTests.swift	Add restore-window gate behavior tests
Tests/DevCtlDaemonCoreTests/RestartTests.swift	Centralize fixture-server lookup
Tests/DevCtlDaemonCoreTests/ResourceLockTests.swift	Move random ports into reserved test range
Tests/DevCtlDaemonCoreTests/ProcessTreeTests.swift	Add session-sweep guard and positive control tests
Tests/DevCtlDaemonCoreTests/ConfigInitTests.swift	Centralize fixture-server lookup
Tests/DevCtlDaemonCoreTests/ConcurrentEnsureTests.swift	Add serialized concurrent-ensure single-flight tests
Sources/fixture-server/main.swift	Support delayed grandchild spawn for deterministic races
Sources/DevCtlKit/Resource/ResourceIdentity.swift	Switch to whole-file hashing + handle unreadable file identities
Sources/DevCtlKit/Protocol/Wire.swift	Add `daemon-starting` error code + `LocalizedError` for `WireError`
Sources/DevCtlKit/Paths/Paths.swift	Add safe server path component + chunked file hashing via CryptoKit
Sources/DevCtlKit/Net/PortClaim.swift	Add primary-port range validation and span overflow checks
Sources/DevCtlKit/Net/LoopbackProbe.swift	Prevent trap on invalid port narrowing
Sources/DevCtlKit/Model/Models.swift	Add optional `DaemonInfo.restoring` field
Sources/DevCtlKit/Launchd/AgentRebindPolicy.swift	Extend post-reregister hello wait to cover launchd throttling
Sources/DevCtlKit/Config/WatchPaths.swift	Align “root entry” handling with lock-resource path rules
Sources/DevCtlKit/Config/ProjectConfig.swift	Remove force-unwrap in topo-sort wave computation
Sources/DevCtlKit/Config/ConfigProjection.swift	Include `watch` in projected config
Sources/DevCtlKit/Agent/AgentContext.swift	Quote/escape config-supplied strings and redact squatter command lines
Sources/DevCtlDaemonCore/Supervisor/ServerSupervisor.swift	Refresh descendant snapshots during startup + session sweep on crash teardown
Sources/DevCtlDaemonCore/Supervisor/ProcessTree.swift	Add session-members sweep with safety guards
Sources/DevCtlDaemonCore/LogStore/SpoolTailer.swift	Fix offset advancement to prevent duplicate ingestion
Sources/DevCtlDaemonCore/Control/ControlServer.swift	Add restore gate, canonicalize unregister, and make listener start await-ready
Sources/devctld/main.swift	Accept before restore, then clear restore gate and announce ready after restore
Sources/DevCtlApp/DashboardView.swift	Fix log tail task identity to include project
Sources/DevCtlApp/DaemonModel.swift	Treat `daemon-starting` as reachable/restoring; fix event cursor progression
Sources/DevCtlApp/AgentService.swift	Make rebind recovery continue on poll miss instead of throwing out of launch
Sources/devctl/CLI.swift	Wait out `daemon-starting` with bounded retry; improve daemon status output; identity notes for inexact fingerprints
scripts/smoke.sh	Improve daemon readiness gate, capture daemon logs, and add restore-window assertion
scripts/signing-identity.sh	Add shared identity resolver preferring Developer ID
scripts/make-dmg.sh	Add default notarize+staple+assessment + optional quarantine stamping
scripts/make-app-bundle.sh	Clarify identity sourcing and add ad-hoc warning controls
Package.swift	Remove IntegrationTests test target
Makefile	Resolve `SIGN_IDENTITY` lazily via signing-identity script
docs/design.md	Update design doc to reflect new restore gate and concurrency coverage
docs/cli-contract.md	Document `daemon-starting` and `daemon.restoring` behavior
BACKLOG.md	Update backlog items for resolved/changed areas and newly discovered issues
AGENTS.md	Update agent map to reflect new behaviors and signing/release path
.github/workflows/release-dmg.yml	Add dry-run gating, require notarization, verify assessment before upload
.changeset/session-teardown-sweep.md	Changeset for session-based teardown sweep
.changeset/restoring-daemon-answers.md	Changeset for daemon restore-window behavior
.changeset/exact-large-file-fingerprint.md	Changeset for exact large-file fingerprinting
.changeset/contain-config-supplied-strings.md	Changeset for config-supplied string/path containment fixes

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Everything here is one shape: a value devctl did not produce, trusted further than it had been checked. A repo's devservers.json reached arithmetic that assumed it fit. Ports were already bounded, but a `ports` entry's offset was checked only for being too small, a portSpan overflowed when added to its port, and the four healthcheck integers were never checked at all. The offset case was invisible to the tool built to catch it: `config check` reported no errors and the failure surfaced later as `daemon-unreachable`. Machine-wide status validated every registered project's config, so one unusable number anywhere took down the daemon supervising all of them, and the menu bar's two-second poll brought it back to do it again. Pids came off disk and off the wire as unbounded integers and were narrowed with a trapping conversion, which under launchd KeepAlive is a crash loop rather than a crash. `ProcessTree.narrowed` and `isAlive` are now the one home for that decision, and they refuse zero and negatives too, since both are kill(2) selectors rather than processes. `hook install` merges into a settings file devctl does not own and writes the whole file back, so a read it could not parse became a replacement: every other hook, permission and setting in it was gone, reported as a successful install. Reading and writing that file now live together on the adapter protocol, so a harness added later cannot reintroduce it. `devctl lock` swallowed a config failure, found no declarers, paused nothing, and reported the hold taken, which is the opposite of what the command exists for. A half-open DaemonClient counted as connected, so a failed handshake disabled the protocol version check for the life of the client. Also: the port shown for a rebound server is now the port it is actually on in the menu bar and statusline, which the agent context already had right; four doc comments in ControlServer described functions other than the ones they sat on, and groupUp's had gone missing entirely; the ISO formatter emitted timestamps its own parser rejects for pre-1970 dates.

Copilot

Pull request overview

Copilot reviewed 62 out of 62 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Sources/fixture-server/main.swift:10

The fixture-server usage text line ends mid-sentence ("...snapshot can"), which reads as a truncated help message.

The first acquire now throws on an unreadable config rather than granting a hold that pauses nothing, so a retry reaching the same-holder path with one means the config broke during the hold. Answering with a state path resolved from no specs at all is a worse answer than saying so. Also finishes a fixture-server usage line that stopped mid-sentence, and records that the coverage gate cannot fail: AGENTS.md names a branch threshold and a command that prints it, but the command prints no figure and the toolchain emits no branch data, so every Branches column is zero.

Copilot

Pull request overview

Copilot reviewed 62 out of 62 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

Tests/DevCtlKitTests/ServerPathComponentTests.swift:47

The collision property described in this test’s comment isn’t actually asserted. For example, inputs like "a/b" and "a_b" currently flatten to the same component and should be proven to stay distinct to avoid log-directory sharing.

…ries The hash that was supposed to hold them apart only ran for names that flattened to `.`, `..`, or nothing, so `a/b` and `a_b` both landed on `a_b` and two servers shared one log directory and intermixed their output. Every name the flattening changed now carries the hash; names it left alone keep a readable directory. Found by Copilot on this PR. Its sibling test claimed this property and did not exercise it: `.` against `..` do not flatten to the same text and take the dot-only branch anyway, so it passed against the bug. It now runs pairs that genuinely collide, plus a control asserting an ordinary name is still its own directory, which the collision cases alone would not catch. Docs: CONTRIBUTING now tells a harness author to read and write settings through the protocol rather than reaching for Data(contentsOf:), since hand-rolling that read is what made install clobber the file it merges into. AGENTS.md's coverage line named a branch threshold and a command that prints it; neither existed, so it now names the real reporter, the Lines column, and why branch data is unavailable.

Copilot

Pull request overview

Copilot reviewed 63 out of 63 changed files in this pull request and generated no new comments.

Suppressed comments (1)

Sources/DevCtlDaemonCore/Control/ControlServer.swift:984

targetOwnsPort treats .starting as owning declaredPort/effectivePort, but during .starting the server may not have bound anything yet. That can mask a real unmanaged listener during the pre-stop prepareSpawn(force: true) pass in restartServers, turning an actionable port-held into a later spawn failure after the server has already been stopped.

Consider only treating .starting as owning ports that were actually observed (or named ports materialized), and otherwise let PortGuard.isListening report the conflict.

        let status = await supervisor.status()
        switch status.phase {
        case .running, .starting, .unhealthy:
            break
        case .crashed, .failed, .stopped, .stopping:
            return false
        }
        return status.declaredPort == port || status.effectivePort == port
            || status.observedPort == port || (status.ports?.values.contains(port) ?? false)
    }

Copilot raised it on this PR and it verifies: targetOwnsPort answering true for a starting server skips the squatter check, and restartServers runs that pre-check before it stops anything. Filed rather than fixed, because .starting cannot simply leave the set and the distinction the fix needs (bound versus merely starting) has no name yet.

quantizor added 8 commits August 8, 2026 17:31

Copilot AI lite review requested due to automatic review settings August 9, 2026 00:35

Copilot started reviewing on behalf of quantizor August 9, 2026 00:41 View session

Copilot AI reviewed Aug 9, 2026

View reviewed changes

Comment thread Sources/DevCtlKit/Net/PortClaim.swift Outdated

Comment thread Sources/DevCtlDaemonCore/Control/ControlServer.swift

quantizor requested a lite review from Copilot August 9, 2026 02:17

Copilot started reviewing on behalf of quantizor August 9, 2026 02:17 View session

Copilot AI reviewed Aug 9, 2026

View reviewed changes

quantizor requested a lite review from Copilot August 9, 2026 02:22

Copilot started reviewing on behalf of quantizor August 9, 2026 02:23 View session

Copilot AI reviewed Aug 9, 2026

View reviewed changes

Comment thread Sources/DevCtlKit/Paths/Paths.swift

quantizor requested a lite review from Copilot August 9, 2026 02:29

Copilot started reviewing on behalf of quantizor August 9, 2026 02:29 View session

Copilot AI reviewed Aug 9, 2026

View reviewed changes

quantizor merged commit 22d5743 into main Aug 9, 2026
4 checks passed

quantizor deleted the fix/restoring-daemon branch August 9, 2026 02:45

github-actions Bot mentioned this pull request Aug 9, 2026

Version Packages #14

Merged

Navigation Menu

Answer while restoring, sweep escaped workers, and sign releases with Developer ID - #16

Answer while restoring, sweep escaped workers, and sign releases with Developer ID#16
quantizor merged 12 commits into
feat/restart-and-watchfrom
fix/restoring-daemon

quantizor commented Aug 9, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

quantizor commented Aug 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

A daemon that is coming back no longer looks like one that is gone

A crashed server no longer leaves its workers running

Upgrades install immediately instead of stalling

devctl lock catches a change it used to miss

A project's own config can no longer reach past what it describes

The rest of that class, found by an audit

Release path

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

quantizor commented Aug 9, 2026 •

edited

Loading

`devctl lock` catches a change it used to miss