feat(jobs): refuse to mutate alongside another operation's job container

[vishr]

Replaces #181, which I am closing. Refs #179 — does not close it.

Why this instead of #181

#181 did two things: refuse while a job container is live, and write the terminal record for runs whose client never did. Five review rounds found seven defects, every one of them in the second half — the epoch grouping, a result branch that could never fire, a reconciler that closed the job it was about to run, wrong operator attribution. A mutation test then showed the tests were largely inert: all four call sites could be deleted with the suite green.

So this is the first half only. One question, asked at two call sites, small enough to verify. The reconciliation work goes back to #179 as unstarted, to be built properly rather than repaired again.

What it does

A sealed job run executes attached over SSH. When the client goes away the container keeps running — the daemon owns it — while nothing on the workstation does. The lock is then either dropped outright (Ctrl-C releases it on its own background context) or expires on a dead heartbeat, and the next deploy or job run proceeds alongside a live data-changing container.

ob deploy and ob job run now refuse while another operation's job container is running, naming it and how to inspect it.

Two decisions worth reviewing

It asks Docker, not the journal. A journal records what a client managed to write, and the failure this exists for is a client that did not write. A run that recorded its own interruption looks finished on paper while its container keeps going; a re-run plan appends a second invocation to the same journal; a deploy-phase gate container never appears in a job-run journal at all. One docker ps sees all three. #181 learned this the expensive way.

Matching is on operation AND epoch. A sealed plan carries one operation id for its whole life and is re-runnable, and AcquireLock (lock.go:155) hands the lock straight back to a caller presenting the id already written in it. Matching the operation alone would let a second run of one plan reclaim the lock from a live first run and then exempt that run's container as its own — two concurrent migration containers, which is the single thing this prevents. ob resume reaches the same hole, since it carries the interrupted attempt's id. A container with no epoch cannot be shown to belong to this invocation and is not exempt from it.

Placement follows the diagnostics: after the staleness checks so a stale plan is told it is stale rather than told about a container, and after preflight so an unreachable daemon is reported by the check that exists for that — while still preceding every mutation.

Known gaps, not fixed here

• Only ob deploy and ob job run check. ob rollback, ob abort, ob service apply, ob exec, ob secrets push, ob schedule run and bootstrap mutate under the same lock without it. ob abort is the sharpest: it reverts the release through recoverInterrupted, not deployCore. Tracked on Sealed manual job can complete after client disconnect but remain permanently INCOMPLETE #179.
• Hoisting the check into AcquireLock is not a safe one-liner: bootstrap takes the lock before Docker exists, and rollback/secrets push/service apply pass release ids as the deploy id, which would make the rollback case self-exempt.
• Non-compose hook jobs get no labels, so they are invisible to this.
• The probe uses the caller's context, so a wedged daemon can hang a deploy under the lock.

Verification

Six unit tests on the predicate — foreign operation, own container, an earlier invocation of the same operation, an empty operation label, a missing epoch — plus two integration tests that drive ob deploy and ob job run against a host reporting a foreign container. Both call sites verified pinned: deleting either makes a test fail. That is the check #181 did not have.

go test ./... — all packages pass.

https://claude.ai/code/session_01JaxHfqFZk8GdrBNbtQZ6c2

[Copilot]

🟡 Changes recommended

The new refusal error messages currently suggest docker rm -f with truncated container IDs (can be ambiguous), and the new refusal test file includes misleading/unfinished comments that should be cleaned up.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR adds a safety gate to prevent ob deploy and sealed manual ob job run from mutating the application while a one-off job container from another operation (or another epoch of the same operation) is still running on the host, using Docker labels (ob.operation, ob.epoch) as the source of truth.

Changes:

• Add a Docker-based probe (docker ps --filter label=ob.operation) and refusal predicate to detect foreign / prior-epoch job containers.
• Invoke this refusal in both deploy (deployCore) and manual job execution (RunJobWithJournalID) before any mutation.
• Add unit tests for the predicate and integration-style tests ensuring deploy/job-run stop before starting when a foreign container is present.

File summaries

File	Description
internal/onebox/service_test.go	Update fakes to handle the new docker ps label probe used by the refusal gate.
internal/engine/job.go	Call the refusal gate after plan staleness checks and before creating a job container.
internal/engine/job_test.go	Add an integration-style test ensuring RunJobWithJournalID refuses before starting the job.
internal/engine/job_reconcile.go	Implement the Docker-based job-container probe and refusal logic.
internal/engine/job_reconcile_test.go	Add unit tests for the refusal predicate and parsing edge cases.
internal/engine/deploy.go	Call the refusal gate after preflight and before any deploy mutations / gate jobs.
internal/engine/deploy_test.go	Add an integration-style test ensuring deploy refuses before rolling workloads when a foreign container exists.

Review details

Suppressed comments (1)

internal/engine/job_reconcile_test.go:92

• The trailing comment block is unfinished and refers to recording operators, which isn’t part of this PR’s refusal-only behavior. Leaving it here reads like a missing test or incomplete change; please remove it (or add the corresponding implementation/tests in the appropriate PR).

// The reconciling operator must not be recorded as the interrupted run's.
// Audit takes the last non-empty operator in an epoch group, so stamping it
// here rewrites the row to name whoever deployed next.

• Files reviewed: 7/7 changed files
• Comments generated: 2
• Review effort level: Lite

---

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

[Copilot]

🔵 Needs a closer look

The new refusal error message asserts “no process owning it” without evidence, which can mislead operators in realistic lock-reclaim scenarios.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

internal/engine/job_containers.go:53

• The refusal message claims the foreign container is running "with no process owning it", but this code only knows that the container exists and its labels; it does not (and cannot) prove whether some process is still attached/controlling it. This can mislead operators in cases where the workload is still actively driven but the lock was reclaimed.

• Files reviewed: 7/7 changed files
• Comments generated: 0 new
• Review effort level: Lite