fix(schedule): prevent pinned startup lock collisions

[vishr]

Closes #184.

Summary

• use a shared reader rendezvous for pinned scheduled-job startup while keeping deploys, execs, and exclusive jobs as writers
• wait up to 10 seconds for brief schedule/deploy handoffs and distinguish contention from flock failures
• prevent systemd from coalescing distinct cron minutes with AccuracySec=1s
• make aggregate divergence wording accurate for schedule-only degradation

Verification

• just check
• Linux util-linux concurrency tests for concurrent readers, writer exclusion, bounded-wait success, timeout skip, and infrastructure errors
• local cavecrew review: clean after test hardening
• GitHub Copilot review: no issues

[Copilot]

🔵 Needs a closer look

Lock infrastructure failures need distinct handling, and the new skip reason requires documentation.

Pull request overview

Fixes pinned scheduled-job startup lock collisions through shared coordination, bounded waits, precise timers, and corrected divergence messaging.

Changes:

• Adds reader/writer rendezvous locking with bounded waits.
• Adds concurrency and failure-path test coverage.
• Sets timer accuracy to 1 second and updates related documentation.

File summaries

File	Description
site/src/content/docs/reference/errors.mdx	Updates documented divergence error text.
site/src/content/docs/guides/schedule-a-job.mdx	Documents scheduling coordination and timer behavior.
internal/onebox/operation_errors.go	Corrects divergence wording.
internal/engine/schedule.go	Implements scheduling locks and timer accuracy.
internal/engine/schedule_test.go	Adds concurrency and error-handling coverage.
internal/engine/schedule_execution.go	Applies rendezvous locking to durable runners.
internal/engine/lock.go	Adds bounded application-lock rendezvous waits.
internal/engine/lock_test.go	Verifies bounded application-lock behavior.

Review details

Suppressed comments (2)

internal/engine/lock.go:103

• This command now reserves exit code 76 for schedule-lock contention, but acquireLock still treats every other nonzero result as an application-lock collision and falls through to holder inspection (the res.ExitCode != 76 path below). A permissions/I/O/unsupported-flock failure can therefore be reported as deploy lock held ... or collapse into the generic retry error instead of preserving the infrastructure failure. Handle nonzero codes other than 76 immediately, including the command's stderr, before inspecting the durable application lock.

			create = "/usr/bin/flock --exclusive --timeout " + strconv.Itoa(scheduleRendezvousWaitSeconds) + " --conflict-exit-code 76 " +
				q(e.names().ScheduleRunLock()) + " /bin/sh -c " + q(create)

site/src/content/docs/guides/schedule-a-job.mdx:86

• The new bounded rendezvous introduces another recorded skip reason (the application scheduling lock remained busy for 10s), including contention with an exclusive scheduled job or writer handoff. The guide's later “Every run leaves a record” definition still says a skip only met the same job or an application operation holding the deploy lock, so operators will encounter a documented reason that the public explanation omits. Update that skip-reason paragraph along with this coordination description.

Readers and writers wait briefly for a handoff; a timer that still collides does
not modify Docker beside a deploy: it records a `skipped` run with the reason
and exits cleanly. An application lock older than its TTL is treated as expired,

• Files reviewed: 8/8 changed files
• Comments generated: 0
• 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

Unresolved flock error classification and startup-timeout coordination issues remain, along with a documentation correction.

Review details

Suppressed comments (3)

internal/engine/lock.go:143

• Because --conflict-exit-code 76 reserves 76 for rendezvous contention, this res.ExitCode != 1 exception assumes every exit 1 came from the inner set -C lock-file creation. A real /usr/bin/flock failure can also return 1, so it will be misclassified as an existing application lock and may be retried/broken or reported as holder contention instead of surfacing the flock error. Give the expected lock-file collision a distinct sentinel (or verify the lock-file condition) and treat every other flock failure as an infrastructure error.

				return 0, fmt.Errorf("persist epoch: %s", strings.TrimSpace(res.Stderr))
			}

internal/engine/schedule.go:372

• The 10-second blocking acquire runs before the runner installs its signal traps, while the generated service sets TimeoutStartSec to the job's configured timeout. A valid job with timeout: 1s can therefore be killed by systemd while waiting behind a writer and recorded as timeout rather than reaching the 75) branch that records the promised contention skip. Bound this wait relative to the service timeout or reserve the rendezvous budget in the generated unit so contention still produces a clean skipped firing.

		"exec 8>" + q(names.ScheduleRunLock()),
		"lock_code=0; /usr/bin/flock " + rendezvousMode + " --timeout " + strconv.Itoa(scheduleRendezvousWaitSeconds) + " --conflict-exit-code 75 8 || lock_code=$?; case $lock_code in 0) ;; 75) skip 'the application scheduling lock remained busy for " + strconv.Itoa(scheduleRendezvousWaitSeconds) + "s' ;; *) echo 'onebox: cannot acquire the application scheduling lock' >&2; exit \"$lock_code\" ;; esac",
		"if [ -e " + q(applicationLock) + " ] && [ \"$(" + lockAgeCmd(applicationLock) + ")\" -le " + strconv.Itoa(ttlSeconds) + " ]; then skip 'an application operation holds the deploy lock'; fi",

site/src/content/docs/guides/schedule-a-job.mdx:81

• A pinned durable runner still holds fd 8 after acquiring the release lease: it keeps the shared rendezvous through prepare and checkpoint publication before unlocking it (internal/engine/schedule_execution.go:128-135). This wording says the lock lasts only through lease establishment, so it understates the scope and can make the expected deploy handoff timing misleading.

container run. A pinned job takes that rendezvous as a shared reader only while
it establishes its immutable release lease, so different pinned jobs may start

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

[Copilot]

🟡 Changes recommended

Moderate lock-exit-code and flock compatibility issues remain unresolved.

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

Review details

Suppressed comments (1)

internal/engine/schedule.go:376

• requireScheduleHost currently accepts any flock found by command -v, but this generated command now requires the util-linux --conflict-exit-code interface (the Linux test explicitly skips when that option is absent). On a host with an older or different flock, schedule apply can install units that fail before writing a skip or run record. Probe /usr/bin/flock --help for the required options, or reject the host before installation.

		"lock_code=0; /usr/bin/flock " + rendezvousMode + " --timeout " + waitSeconds + " --conflict-exit-code 75 8 || lock_code=$?; case $lock_code in 0) ;; 75) skip " + q(busyReason) + " ;; *) echo 'onebox: cannot acquire the application scheduling lock' >&2; exit \"$lock_code\" ;; esac",

• Files reviewed: 8/8 changed files
• Comments generated: 3
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The unlock-order regression test can pass when the unlock command is missing and should be corrected.

Review details

Suppressed comments (1)

internal/engine/schedule_test.go:304

• This check does not detect a missing unlock: strings.Index returns -1, so -1 > strings.Index(runner, "docker rm -f") is false and the test passes even if the new unlock command is removed. Check unlock < 0 explicitly (and keep the ordering check) so this regression is actually covered.

	if strings.Index(runner, "flock --unlock 8") > strings.Index(runner, "docker rm -f") {
		t.Fatalf("pinned runner kept the application rendezvous through per-job cleanup:\n%s", runner)
	}

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

[Copilot]

🔵 Needs a closer look

Rendezvous contention can be reported inaccurately as a scheduled-job conflict.

Review details

Suppressed comments (1)

internal/engine/lock.go:159

• The explicit conflict code only tells us that the exclusive schedule rendezvous could not be acquired; it does not identify the holder. A concurrent application operation can also hold this writer rendezvous past the 10-second bound, so reporting every code-200 result as a scheduled-job conflict is inaccurate. Use a generic rendezvous-contention message (or inspect the durable lock only after acquisition) here.

		if useScheduleLock && res.ExitCode == flockConflictExitCode {
			return 0, fmt.Errorf("deploy lock held by a scheduled job — wait for the job to finish")

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

[Copilot]

🔵 Needs a closer look

The zero-wait path passes --timeout 0 to flock, which can block indefinitely instead of performing a non-blocking attempt.

Review details

Suppressed comments (1)

internal/engine/schedule.go:376

• When scheduleRendezvousWait returns zero (for example, a 1s job), this still emits --timeout 0. In util-linux flock, zero means no timeout/block indefinitely; it is not the immediate, non-blocking attempt described by the helper comment. A short job behind a writer can therefore hang until systemd kills it instead of recording a skip. Emit --nonblock when the wait is zero and use --timeout only for positive waits.

		"lock_code=0; /usr/bin/flock " + rendezvousMode + " --timeout " + waitSeconds + " --conflict-exit-code " + strconv.Itoa(flockConflictExitCode) + " 8 || lock_code=$?; case $lock_code in 0) ;; " + strconv.Itoa(flockConflictExitCode) + ") skip " + q(busyReason) + " ;; *) echo 'onebox: cannot acquire the application scheduling lock' >&2; exit \"$lock_code\" ;; esac",

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

[Copilot]

🟡 Changes recommended

A critical lock-file write failure can be misclassified as contention and must be fixed.

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

Review details

• Files reviewed: 13/13 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The broad locking and scheduling changes require final human review.

Review details

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