Harden devctl lock, and unblock releases

[quantizor]

Releases have been silently failing since #10, and devctl lock had three ways to mislead you. Both are fixed here.

Releases publish again. A changeset with malformed frontmatter could not be parsed, so the Release workflow failed on every push to main and nothing shipped. Three merged pull requests are still waiting on it. Changesets are markdown, and the build gate skips markdown to save CI minutes, so nothing read one before it merged. A new check parses changeset frontmatter on any pull request that touches it.

devctl lock accepts its own options. Passing --timeout or --acquire-timeout before the -- used to hand them to the guarded command, which then failed with a confusing error from env. Options now reach the lock and the command receives exactly what follows --, including a nested -- or an empty argument. Forgetting the -- entirely, or misspelling an option, is now rejected instead of silently changing what runs.

A contended lock says who holds it. It used to block in silence, which reads as a hang and invites killing the run that is actually making progress. It now names the holder, how long that run has been going, and which servers it paused or left up, and repeats while it waits. --acquire-timeout 0 fails immediately for scripts that would rather not wait. All of this goes to stderr, so the guarded command still owns stdout.

--no-pause no longer accepts a command that destroys the state it guards. A migration run that wiped and rebuilt a local database reported success while the seeded rows were gone: the lock serialized access, but the still-running server held the old files open and wrote its cached pages back over them. A locks entry can now say where its state lives, and lock checks that state around the command. A change made while a declaring server stayed up is a loud failure naming the servers to stop and the command to re-run; the same change with nothing running is just a note. Locks written as plain names keep working exactly as before.

The check is deliberately modest, and the CLI contract says where it stops: it flags the risk rather than proving damage, sees only the path you declare, and samples very large files rather than reading them whole.

Docs move with the code: the codebase map covers the new modules and test target, and the design record no longer claims register --write never shipped.

Verified with the unit suite and the end-to-end smoke gate, the latter repeated to confirm the new timing assertions are stable.

[Copilot]

Pull request overview

This PR hardens devctl lock end-to-end (CLI parsing, contention UX, and a guard against silent state clobbering) by updating the CLI, wire protocol, daemon lock behavior, config schema, and tests to match the documented contract.

Changes:

• Fix devctl lock argument parsing by requiring -- and capturing the guarded command verbatim via .postTerminator, rejecting missing terminators and unknown options.
• Add contention visibility via lock.status so waiting acquires can name the lock holder (pid, age, paused/live servers) and provide bounded, repeated stderr notices.
• Add optional lock state-path declarations and resource fingerprinting to detect and report mutations, escalating to a new resource-mutated error under --no-pause with live declarers.

Reviewed changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
Tests/DevCtlKitTests/ResourceIdentityTests.swift	New unit tests for ResourceFingerprint behaviors (inode changes, sampling caps, directory determinism expectations).
Tests/DevCtlKitTests/ProjectConfigTests.swift	Tests for bare-string vs object-form lock declarations and state-path resolution/conflict behavior.
Tests/DevCtlDaemonCoreTests/ResourceLockTests.swift	Router-level tests for lock.status and --no-pause holder recording.
Tests/DevCtlCLITests/LockWaitTests.swift	New CLI-unit tests for contention notice wording and acquire scheduling behavior.
Tests/DevCtlCLITests/LockParsingTests.swift	New CLI-unit tests asserting the corrected terminator-based parsing contract.
Tests/DevCtlCLITests/LockIdentityTests.swift	New CLI-unit tests for identity-verdict messaging and hint composition.
Sources/DevCtlKit/Resource/ResourceIdentity.swift	Implements resource fingerprint capture/compare logic (files, directories, symlinks) with caps/sampling.
Sources/DevCtlKit/Protocol/Wire.swift	Adds resource-mutated code, lock.status method, and expands lock holder/result payloads.
Sources/DevCtlKit/Paths/Paths.swift	Refactors SHA256 helpers to a full-hex function and redefines hash8 as its prefix.
Sources/DevCtlKit/Model/Models.swift	Introduces LockDeclaration (bare string or {name,path}) and updates ServerSpec.locks type.
Sources/DevCtlKit/Config/ProjectConfig.swift	Updates project config model to use LockDeclaration for locks.
Sources/DevCtlKit/Config/LockResource.swift	New helper to centralize declarer detection and state-path resolution across specs.
Sources/DevCtlDaemonCore/Supervisor/ServerSupervisor.swift	Adapts server status locks reporting to the new declaration type (names only).
Sources/DevCtlDaemonCore/Control/ControlServer.swift	Adds lock.status routing and records lock holder pause/live metadata plus resolved statePath.
Sources/devctl/CLI.swift	Implements new parsing strategy, contention notices to stderr, acquire scheduling, and identity guard behavior.
scripts/smoke.sh	Extends smoke coverage for option leakage, contention visibility, fail-fast acquire, and identity checking.
README.md	Updates CLI feature list and clarifies recovery via devctl config init.
Package.swift	Adds DevCtlCLITests test target for CLI parsing/contract behavior.
docs/design.md	Updates references to AGENTS.md and documents locks object form and identity checking behavior.
docs/cli-contract.md	Documents new resource-mutated code and the updated devctl lock contract details.
BACKLOG.md	Removes resolved lock parsing/no-pause clobber items; adds follow-ups for streaming hashing and post-hold risk window.
AGENTS.md	Updates codebase map and smoke coverage description to include new lock/config behaviors.
.changeset/lock-parsing-and-contention.md	Changeset for lock parsing fix and contention notices.
.changeset/lock-identity-guard.md	Changeset for identity guard and new failure mode under --no-pause.

---

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

[quantizor]

@pullfrog review

[Copilot]

Pull request overview

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

Suppressed comments (4)

Sources/DevCtlDaemonCore/Control/ControlServer.swift:1086

• acquireLock uses try? await mergedSpecs(...) and then falls back to an empty spec list. When devservers.json is invalid, this can incorrectly acquire the lock without pausing any declarers (and also skip state-path conflict detection), which defeats the lock’s safety guarantees.

        let shouldPause = params.pause ?? true
        let merged = try? await mergedSpecs(project: params.project)
        for spec in merged?.specs ?? []
        where LockResource.declares(resource: params.resource, spec: spec) {

Sources/DevCtlDaemonCore/Control/ControlServer.swift:1113

• statePath resolution during lock acquire also falls back to merged?.specs ?? []. If config loading failed, this returns nil instead of surfacing config-invalid or conflicting lock-path errors, so the CLI won’t be able to perform the identity check reliably.

        /** Refusing here is correct when devctl cannot tell which state the lock
            guards: taking it anyway would report on the wrong file. */
        let statePath = try LockResource.statePath(
            project: params.project, resource: params.resource, specs: merged?.specs ?? [])

Sources/DevCtlDaemonCore/Control/ControlServer.swift:1079

• acquireLock swallows mergedSpecs failures (try?) when the same holder re-acquires. If devservers.json is temporarily invalid, this will silently drop statePath (and potentially change behavior mid-hold) instead of returning the config-invalid error the rest of the daemon uses.

This issue also appears in the following locations of the same file:

• line 1083
• line 1110

        if let existing = resourceLocks[key], existing.pid == params.holderPid {
            let specs = (try? await mergedSpecs(project: params.project))?.specs ?? []
            return LockResult(
                live: existing.live, paused: existing.paused,
                statePath: try LockResource.statePath(
                    project: params.project, resource: params.resource, specs: specs))

Sources/devctl/CLI.swift:1805

• On contention, announcedAt is only set when lock.status returns a holder. If the daemon is older (or lock.status fails), announcedAt stays nil, first stays true, and the CLI will attempt lock.status every retry (once per second) despite the comment saying it avoids hundreds of round trips.

                if due {
                    /** An older daemon without lock.status degrades to silence
                        here rather than failing the acquire. */
                    let holder = try? await client.request(
                        .lockStatus,
                        params: LockStatusParams(project: project, resource: resource),
                        expecting: LockStatusResult.self
                    ).holder
                    if let holder {
                        Self.note(
                            first
                                ? LockNotice.contended(
                                    budgetSeconds: acquireTimeout, holder: holder, now: Date(),
                                    resource: resource)
                                : LockNotice.stillWaiting(
                                    elapsedSeconds: elapsed, holder: holder,
                                    remainingSeconds: max(acquireTimeout - elapsed, 0),
                                    resource: resource))
                        announcedAt = elapsed
                    }

[quantizor]

@pullfrog review