fix: avoid truncating lock files before acquiring lock on Windows

[aniruddhaadak80]

Problem / Motivation

pod_name_mutex (src/kiro_crew/pod/runtime.py) used raw fcntl.flock and degraded to a no-op on hosts without fcntl -- so on Windows the pod lifecycle mutex serialized nothing, and its lock file was opened with truncating mode "w".

Why it matters

down and up are independent entry points with no other per-name coordination, and both reclaim the isolated HOME on the down path. Without a working mutex on Windows, a stop that has just confirmed the service gone races a concurrent start whose checkout pin and service definition the stop's sweep would then delete.

What changed (motivation → approach → change)

Migrated pod_name_mutex to the merged cross-platform convention (cf. #9237 / GH-9248, as in deploy/pending.py):

• lock file opened touch(exist_ok=True) + "r+" (writable, which msvcrt.locking needs, but non-truncating)
• exclusion via platform_compat.file_lock (fcntl.flock on POSIX, msvcrt.locking on Windows, fail-closed on both)
• dropped the if fcntl is None: yield; return no-op branch and the now-unused guarded fcntl import; docstring updated
• same-thread re-entry still takes the in-thread counter, never the OS lock

Tests

• New TestPodNameMutexCrossPlatform (test/test_pod.py, runs unskipped on every host): pins the non-truncating open (sentinel content survives acquisition) and same-thread re-entry (no deadlock). Both pass.
• Removed the three rt.fcntl is None skipifs whose no-op premise no longer holds (concurrent port allocation, real-lock smoke, concurrent env writes); POSIX exclusion behavior is unchanged so those suites are unaffected.
• black --target-version py310, flake8, isort: clean on both touched files; mypy on pod/runtime.py: clean.

Pattern harvest

Not generalizable: one-off migration of the last fcntl-only mutex to the established file_lock convention; no new pattern.

Related Issues

Related #9248 (lock-file truncation class); follows the convention merged in #9237.

Checklist

• Single commit with Conventional Commits title
• Regression pins added and passing; neighboring mutex suites unaffected
• Self-review completed; code follows project style guidelines
• No secrets, credentials, or internal references in the diff

Contribution License Agreement

[github-actions]

Design Review (Fable 5, fork) — ✅ PASS

Design-level review of 1c804ecda936c3460ae454c464324a047fc11690 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All three removed skipifs were the only rt.fcntl references, and the mutex was fcntl's only use in runtime.py — the migration is clean. POSIX semantics are unchanged (same blocking flock on the same file, reentrancy preserved), file_lock is the established fail-closed convention this repo already merged for deploy/pending.py, and the change replaces a fail-open no-op branch with a real primitive while strengthening test coverage (three unskips plus two new cross-platform pins). The old skipif's "not a gap" rationale isn't reversed — the migration makes its premise moot rather than weakening a pinned behavior. No contract or data changes; a revert restores the old behavior cleanly.

Design-Verdict: PASS

Correctly retires the last fcntl-only, fail-open-on-Windows mutex onto the merged file_lock convention, with POSIX semantics provably unchanged and stronger pins.

[DESIGN-REVIEWED] 1c804ec

[github-actions]

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of 1c804ecda936c3460ae454c464324a047fc11690 via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All evidence gathered. The migration is mandated by the platform-compat spec table (file_lock, NOT fcntl.flock), so the core change is derived. But the description's motivating race is unreachable on Windows (require_systemd at pod/runtime.py:893 raises on any non-Linux host before the sweep — exactly what the deleted skipif recorded), and the titled truncation harm has no nameable victim: this lock file never carries content. Final review:

First-Principles-Verdict: CONCERNS

The titled truncation fix removes nothing nameable — this lock file never carries content; the change's real, sufficient ground is the platform-compat file_lock mandate.

Not justified as shipped

2. Lock file no longer emptied on acquire — inherited: no reader of this file's bytes exists (the new test writes the only content it will ever hold); the harm belongs to the Lock files opened truncating before the acquire: five sites still carry the Windows defect fixed in 9237 #9248 class, not this file.

What this change ships

Inventory (5 items) — 4 justified

Intent: make the per-pod lifecycle mutex a real lock on Windows instead of a documented no-op — a FIX.

1. Pod up/down mutual exclusion is now real on Windows (was a silent no-op) — justified
2. Lock file no longer emptied on each acquire ("w" → touch + "r+") — inherited: content is meaningless by the test's own admission
3. Guarded fcntl import and the no-op branch deleted (rt.fcntl is gone) — justified
4. Three fcntl-skipped test suites now run on every host — justified
5. New cross-platform pins for non-truncation and same-thread re-entry — justified

Watch

The "Why it matters" race is unreachable: "a stop that has just confirmed the service gone races a concurrent start" cannot happen on Windows, where require_systemd (src/kiro_crew/pod/runtime.py:893) raises before any sweep — exactly what the deleted skipif recorded ("production never reaches the no-op. Only this test could."). All 10 pod_name_mutex/pod_plane_mutex call sites (grep) sit behind that gate. The change still merges on docs/system-specs/common/platform-compat.md's table (file_lock, NOT fcntl.flock, "even in code you believe only runs on POSIX"), which supersedes the pin — but the invariant, not the race, is the justification.
Clears when: a Windows-reachable pod_name_mutex caller is named, or the motivation is corrected to cite the platform-compat invariant.

[FIRST-PRINCIPLES-REVIEWED] 1c804ec

[github-actions]

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed 1c804ecda936c3460ae454c464324a047fc11690 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 1c804ec

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 1c804ecda936c3460ae454c464324a047fc11690 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 1c804ec

[aniruddhaadak80]

Reshaped into the real fix per the Design prescription (latest push, rebased onto current main, single commit):

• pod_name_mutex is migrated to platform_compat.file_lock over a touch + "r+" lock file -- the merged convention from fix(work-ledger): stop the lock-file open from truncating on Windows #9237. Real exclusion on Windows via msvcrt, unchanged blocking flock on POSIX.
• The if fcntl is None: yield; return no-op branch and the now-unused guarded fcntl import are gone; docstring updated. No reviewer-flagged comment survives.
• Description rewritten to match the diff (no more five-site claim).
• New TestPodNameMutexCrossPlatform pins the non-truncating open and same-thread re-entry on every host (both pass); the three rt.fcntl is None skipifs whose premise no longer holds are removed.
• black/flake8/isort/mypy clean on both touched files; body carries ## Pattern harvest.