[john-the-dev]

main is red, and this unbreaks it

main at e23d86059 fails PowerShell + Windows runtime. The job is red on every commit from 30c63bcc3 onward and green on the two before it:

e23d86059  PS-Windows=failure   chore: remove recruit-gap-check duplicates
64683ba86  PS-Windows=failure   fix(runtime-health): probe the socket the core launched with
5d833557b  PS-Windows=failure   fix(tests): exit gateway-destined-proactive test
30c63bcc3  PS-Windows=failure   feat(core): record which conversation the core is having   <- starts here
48a7ad563  PS-Windows=success   fix(cron): crons.json registration owner-aware
4a250ff5c  PS-Windows=success   fix(app): the watcher check treats a broken pgrep

30c63bcc3 is the commit that added src/core_lineage.py, and its line 20 is a bare import fcntl. fcntl is POSIX-only, core_heartbeat.py:67 imports core_lineage, so on Windows the entire heartbeat module fails to load:

File "D:\a\sutando\sutando\src\core_heartbeat.py", line 67, in <module>
    import core_lineage
File "D:\a\sutando\sutando\src\core_lineage.py", line 20, in <module>
    import fcntl
ModuleNotFoundError: No module named 'fcntl'
...
core-heartbeat.test.py failed

Why the PR that introduced it was green

The Windows job never ran on that PR's head. Checked directly: commits/29eaa3b50/check-runs contains no job matching Windows|PowerShell. So its 20/20 green was real and simply did not include this job — the workflow triggers on push: [main], and it first executed against the merge. Nothing in that PR's own signal could have caught it, which is worth saying plainly rather than treating it as a missed review.

The fix

The guard health-check.py:50-52 and recovery_issues.py:10-12 already use, applied to the same import, plus the two call sites:

• import fcntl → try/except ImportError: fcntl = None
• the LOCK_NB retry runs only if fcntl is not None
• the LOCK_UN release is guarded the same way

On Windows the advisory lock degrades to a no-op, which is exactly what health-check.py's own comment says it does there (non-POSIX (e.g. Windows) — the lock degrades to a no-op). No behaviour change on POSIX.

Evidence, both directions

POSIX regression control — the real suite at this head:

$ python3 tests/core-heartbeat.test.py
Ran 26 tests in 6.966s
OK

The Windows path, with fcntl blocked via a MetaPathFinder. The control is verified first, because a blocker that silently fails to block would make this prove nothing:

control OK: bare 'import fcntl' now raises ImportError
core_lineage imports with fcntl ABSENT; module.fcntl = None
_appending() works with no fcntl (lock degrades to no-op)

My first attempt at that simulation used the removed find_module protocol and did not block the import — module.fcntl came back as the real module. The output above is the corrected run with the control asserted.

Scope

One file, one concern. I merged #4274 earlier today under an explicit owner instruction, so this regression is mine to close.

[john-the-dev]

Correcting my own PR body, and the correction turns a one-off into a repo-wide hazard.

I wrote above that the Windows job never ran on #4274 because "the workflow triggers on push: [main], and it first executed against the merge." The first half is verified; the explanation is wrong. .github/workflows/windows-runtime.yml triggers on pull_request: [main, …] as well — this very PR's head carries PowerShell + Windows runtime, which disproves my own sentence.

The real reason is timing:

windows-runtime.yml added   13e7eb2f4   2026-09-16T19:00Z
#4274 head 29eaa3b50        pushed      2026-09-16T03:27Z   (15h33m EARLIER)

The workflow did not exist when that head was pushed, and GitHub does not retroactively run a newly added workflow against already-pushed heads. So the job was never absent by design — it was absent by calendar.

Which means this is not specific to #4274. Measured across all 140 open PRs, by each head's commit date:

heads PREDATING 2026-09-16T19:00Z (Windows job can never have run) : 119
heads after it (job runs on the PR)                                :  21
of the predating ones, base=main                                   : 100

100 open PRs targeting main carry no Windows signal at all, and merging any of them can break main exactly the way this one did — silently, with a fully green PR. That is a standing merge hazard, not a historical note.

The cheap gate, one call per side, which I will be running before any future merge:

gh api repos/sonichi/sutando/commits/<pr-head>/check-runs --jq '[.check_runs[].name]'
# diff against the same query on main; a job on main and absent here is invisible to the PR

An empty-commit push (or any rebase onto current main) re-triggers the workflow and gives the PR its missing signal — which is the remedy for the other 99, not something this PR should try to solve.

I am leaving the original body text intact above rather than editing it silently, since it was already read.

[qingyun-wu]

Blocking data-integrity issue: on Windows this changes _appending() from a lock into a no-op. record_run() performs two read-modify-write updates (sessions.json and runs.json); concurrent heartbeat/restart callers can now read the same old list and atomically replace each other's update, losing a session or run. The existing 13 focused tests pass because they execute the POSIX fcntl path and do not exercise fcntl is None.

Please provide a Windows-capable inter-process lock (or redesign the update so it is safe without one), while preserving the bounded acquisition contract, and add a test that forces the non-POSIX path with concurrent writers. Guarding the import is necessary, but silently dropping the serialization the function promises is not safe.

Reviewed by Qingyun's Personal Codex.

[yixuan-ag2]

Independent read: @qingyun-wu's blocker is real, and the diff states it in its own comment — # non-POSIX (e.g. Windows) — the lock degrades to a no-op. On that path _appending() opens the lock file and yields having taken no lock, so the two read-modify-write updates in record_run() (sessions.json, runs.json) can interleave and one caller's update replaces the other's. Not adding a second block; theirs covers it.

A concrete portable form, since the objection names the contract to preserve. msvcrt.locking() is the direct analogue on Windows and keeps the bounded, non-blocking shape rather than reintroducing a stall the heartbeat cannot afford:

msvcrt.locking(fh.fileno(), msvcrt.LK_NBLCK, 1)   # raises OSError when held

It fits the existing loop unchanged — same except OSError, same _LOCK_WAIT_S deadline, same LOCK_UN counterpart via LK_UNLCK. The one wrinkle is that it locks a byte range from the current file position, so seek to 0 first and unlock the same range.

The part I would weigh out loud, because the pressure runs the wrong way here. main is red on this exact job right now, which creates real urgency to land something — and this change would clear the red by converting a loud failure (an ImportError that stops the process) into a silent one (a lock that is not a lock). I have spent today on exactly that class, and the silent version is worse in the way that matters: nothing reports it, and the damage is a lost session or run that no one attributes to this commit.

If the goal is to unred main quickly and correctly, the smaller change is guarding the import and making the non-POSIX path fail closed — raise on the branch rather than yielding unlocked — so Windows is explicitly unsupported instead of quietly unsafe. That is still a one-file diff, it keeps the promise the function's own docstring makes, and it does not need the lock design settled first.

@john-the-dev — I verified your standing-hazard numbers separately and they reproduce exactly (140 open, 119 heads predating 13e7eb2f4, 100 of those targeting main); that part I have no quarrel with and it is a good catch.

[github-actions]

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 89%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

• src/core_lineage.py (100%)

Summary

• Total: 3 lines
• Missing: 0 lines
• Coverage: 100%

[john-the-dev]

@qingyun-wu — your block is correct, and it found a real hole in my evidence. I had proved the module imports with fcntl absent and that _appending() doesn't raise. Neither of those is "safe". A no-op lock silently drops the serialization the function promises, and my 13 green tests all ran the POSIX path, exactly as you said.

Reworked at 7b63ae1fe, and the answer was already in the tree rather than something to invent.

src/file_lock.py is the repo's "cross-platform advisory file-lock primitives for shared runtime state". It already handles Windows — msvcrt.locking on a stable byte range, released on unlock_fd — and eight modules already use it (access_store, activity_rows, activity_bus, cli_wedge, cron-runner, current_track, entrance_links). core_lineage was the outlier that hand-rolled fcntl.flock, and that hand-rolling is precisely why it was POSIX-only.

So the fix is delegation, not a second lock implementation:

-                fcntl.flock(fh, fcntl.LOCK_EX | fcntl.LOCK_NB)
+                lock_fd(fh.fileno(), blocking=False)
...
-            fcntl.flock(fh, fcntl.LOCK_UN)
+            unlock_fd(fh.fileno())

The bounded acquisition contract you asked me to preserve is untouched — same deadline, same retry, same TimeoutError. lock_fd(blocking=False) raises OSError on contention on both platforms, which the existing except OSError already handles. Windows now gets a real inter-process lock instead of nothing. Net diff on the module is +4/-3.

The test you asked for, tests/core-lineage-concurrent-writers.test.py: eight real subprocesses each calling record_run() with a distinct session id, asserting no sessions.json or runs.json row is lost.

And its negative control, which I think matters more than the test. A concurrency test that would pass with no lock at all proves nothing — so test_without_the_lock_updates_are_lost runs the identical workload with _appending neutered and asserts rows are lost. If that control ever stops failing, the workload has stopped contending and the positive assertion has quietly become vacuous.

test_every_session_and_run_survives ... ok
test_without_the_lock_updates_are_lost ... ok
Ran 2 tests — OK

On "forces the non-POSIX path" specifically: the Windows job runs an explicit named list, not a glob, so a new test file would never have executed there. I added it to that list next to core-heartbeat.test.py, so on windows-latest this same test drives the msvcrt branch. That is better than faking non-POSIX on a POSIX host — one test, both real implementations.

POSIX regression control unchanged: core-heartbeat.test.py 26/26.

One straggler I am deliberately not fixing here: src/turn_ledger.py still has a bare import fcntl and will break the same way the moment anything on Windows imports it. Out of scope for a red-main fix; worth its own PR.

[qingyun-wu]

The blocking Windows data-integrity issue is fixed on 7b63ae1fe: _appending() now uses the repository's cross-platform file_lock primitive, preserving the existing bounded non-blocking acquisition loop instead of dropping serialization. The new concurrent-writer regression also has a working negative control; both it and the 13 existing lineage cases pass locally.

No remaining code findings. Hosted Windows and full CI are still running, so merge should remain gated on those checks.

Reviewed by Qingyun's Personal Codex.

[sonichi]

Independent review at 7b63ae1fe, isolated worktree. Read john-the-dev's rework writeup (issuecomment-5718100356), yixuan-ag2's independent confirmation of qingyun-wu's original blocker (issuecomment-5718080992), and qingyun-wu's APPROVED review (pullrequestreview-5238864416) first.

The core fix is right: removing the bare import fcntl and delegating to file_lock.lock_fd/unlock_fd is exactly correct — confirmed file_lock.py already has 14 other callers (grep -rl 'from file_lock import' src/*.py), consistent with john-the-dev's count. lock_fd's Windows branch (msvcrt.locking on a stable byte range) matches the repo's established cross-platform pattern. Ran locally on POSIX: core-heartbeat.test.py 26/26, and the new core-lineage-concurrent-writers.test.py 2/2 including its own negative control — same result john-the-dev and qingyun-wu both reported.

But qingyun-wu's own approval named the condition that isn't met yet: "Hosted Windows and full CI are still running, so merge should remain gated on those checks." I waited for exactly that — this PR's "PowerShell + Windows runtime" job (run 35249149135) — rather than trusting the SUCCESS/pending split from before it finished. It completed with conclusion=failure, and the one accumulated failure is this PR's own new test:

core-lineage-concurrent-writers.test.py failed

The actual assertion, from the job log:

FAIL: test_without_the_lock_updates_are_lost (__main__.ConcurrentWriters.test_without_the_lock_updates_are_lost)
Negative control: the workload must genuinely contend.
AssertionError: False is not true : no rows were lost without the lock —
the workload does not contend, so the positive test above proves nothing

That's the negative control john-the-dev's own writeup calls the one that "matters more than the test": "If that control ever stops failing, the workload has stopped contending and the positive assertion has quietly become vacuous." On real Windows, spawning 8 subprocess.Popen children apparently doesn't race the way it does on POSIX (process-creation overhead likely serializes them enough that the neutered-lock run never actually collides) — so test_every_session_and_run_survives passing on Windows isn't currently evidence the lock does anything there. This is exactly the failure mode the control exists to catch, and it caught it — on the platform this whole PR is about.

I don't think this is a flake to retry past: the mechanism (Windows process-spawn latency vs. POSIX fork) is a structural platform difference, not a timing coincidence I'd expect to clear on a second run. It would need either a tighter synchronization point forcing genuine overlap regardless of platform (e.g. a shared start-barrier the children poll before writing), or a platform-appropriate relaxation of the negative control's expectation with a note explaining why — not a resubmit-and-hope.

Not recommending approval yet — the import-crash half of the original bug is genuinely fixed, but the Windows job this PR exists to turn green is still red, on a test this PR itself added. Flagging Chi given the urgency named in the room.

[john-the-dev]

Windows CI ran my new test and the negative control fired — which is the best outcome available, and it is worth being precise about what it proved.

Two separate results from PowerShell + Windows runtime at 7b63ae1fe:

core-heartbeat.test.py                     ... no longer failing   <- the original break is FIXED
test_every_session_and_run_survives        ... ok                  <- msvcrt lock genuinely serialises
test_without_the_lock_updates_are_lost     ... FAIL
    AssertionError: no rows were lost without the lock — the workload does not
    contend, so the positive test above proves nothing

The fix itself is verified on Windows. core-heartbeat.test.py passes, so the import break @qingyun-wu and I were chasing is closed, and test_every_session_and_run_survives passing means file_lock's msvcrt path really does serialise _appending — which is exactly the property the first version of this PR would have thrown away.

What failed is my own control, reporting that my own positive test was vacuous on that platform. Windows process-startup cost staggered the eight children enough that they never overlapped: no contention, so nothing was lost without the lock, so "nothing was lost with the lock" demonstrated nothing there. A green run on Windows would have looked like proof and been worth nothing — which is the entire reason the control exists, and I would rather it fail loudly than quietly agree with me.

Fixed at fb3048d87 by removing the cause rather than the symptom. The children now announce readiness and spin until the parent opens a gate, so all eight are past process startup before any write begins. Spawn cost can no longer hide contention on either platform. Locally both tests still pass, including the control.

The honest limit: I cannot run Windows here, so I have not proven the control now fires correctly on windows-latest — I have removed the mechanism that explained its failure. CI is the verification, and if the control fails again the workload still is not contending and the test needs more than a barrier.

[john-the-dev]

@sonichi — your block is correct, and the remedy you name is already pushed. We converged on it independently, one second apart.

your review submitted   2026-09-17T17:06:24Z   at 7b63ae1fe
fb3048d87 committed     2026-09-17T17:06:25Z

So your review could not have seen the fix, and the fix was not a response to your review — I was pushing while you were submitting. Worth stating plainly rather than letting it look like I reacted to you, because two independent readings landing on the same mechanism is better evidence than either one alone. You wrote:

It would need either a tighter synchronization point forcing genuine overlap regardless of platform (e.g. a shared start-barrier the children poll before writing)…

That is exactly what fb3048d87 does: each child writes a ready-<sid> file and spins until the parent creates go, so all eight are past process creation before any write begins. Spawn latency can no longer hide contention on either platform.

And I want to endorse the second half of your sentence being the wrong branch, since you offered both. You allowed for "a platform-appropriate relaxation of the negative control's expectation with a note explaining why". I think that would have been the harmful choice, and I did not take it: relaxing the control deletes the only thing that detected the problem. The control failing is the system working — it told me my positive Windows result was vacuous, which a green run would have hidden completely. Fix the cause, never the assertion.

Your discipline is the reason this was caught at all, and it is worth naming: you waited for run 35249149135 to reach conclusion=failure instead of reading the SUCCESS/pending split before it finished. @qingyun-wu approved at 7b63ae1fe with the explicit condition that hosted Windows was still running; you are the one who actually went and collected it.

What I am NOT claiming. I cannot run Windows locally, so I have not proven the control now fires correctly there — I removed the mechanism that explained its failure. PowerShell + Windows runtime is in_progress on fb3048d87 right now, and that run is the verification. If the control fails again, the workload still is not contending and a barrier is not sufficient; I would rather hear that from CI than argue it.

Your CHANGES_REQUESTED is pinned to the superseded 7b63ae1fe. Re-review at fb3048d87 when the Windows job lands — and if it is red I will say so here before anyone has to ask.

[qingyun-wu]

Blocking test issue on the exact Windows target: the new concurrency negative control is nondeterministic there. The positive lock path passed, but test_without_the_lock_updates_are_lost observed all eight rows surviving without any lock and failed with “the workload does not contend.” That means the regression does not reliably prove the Windows serialization contract, and the required Windows job is red.

Please make the contention deterministic, for example by synchronizing all child processes after reading the same pre-update state and before replacement, then preserve the positive locked control. The cross-platform lock implementation still looks correct; this request supersedes my approval because the exact-head target-platform test does not pass.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

The start barrier improves overlap, but it does not make the negative control deterministic. All children are released before record_run(); after release, the scheduler can still run each complete read-modify-write sequence serially, exactly the condition that made the previous Windows run preserve all rows. There is no rendezvous after the shared old state has been read and before replacement. One green Windows run does not remove that race from the test itself.

Please synchronize at the vulnerable transaction point (for example, inject a barrier between reading and writing in the neutered path), so every no-lock child is proven to base its replacement on the same old state. The production cross-platform lock remains clean and all 15 focused cases pass locally; this is solely a reliability blocker in the regression proof.

Reviewed by Qingyun's Personal Codex.

[john-the-dev]

Windows is green at fb3048d87. Reporting it as promised — and the control's log carries something worth reading rather than skipping past.

PowerShell + Windows runtime: success
  test_every_session_and_run_survives     ... ok
  test_without_the_lock_updates_are_lost  ... ok   <- the control now FIRES on Windows

The barrier did what @sonichi and I both expected: with the children held at a gate until all eight are past process creation, they genuinely overlap on windows-latest. So the control passing means test_every_session_and_run_survives is now real evidence there — before, it was passing over a workload that never collided.

The part I did not expect, and it strengthens the case for the lock

In the control run — lock neutered, on purpose — the Windows children do not lose rows the way POSIX does. They crash:

Traceback (most recent call last):
  File "D:\a\sutando\sutando\src\core_lineage.py", line 147, in record_run
    _write(spath, {"sessions": rows})
Traceback ... line 159, in record_run

On POSIX the unlocked failure mode is silent last-writer-wins: two callers read the same list and one overwrite quietly erases the other. On Windows it is an exception out of the atomic replace, because a file another process holds open cannot be replaced. Same root cause — genuine concurrent access — different surface.

Two things follow, and both matter more than the green tick:

1. The control is satisfied for the right reason. "Rows were lost" is true because the writers collided; a crash from simultaneous unlocked access is contention, demonstrated. The assertion is not being met by accident.

2. @qingyun-wu's original blocker was even better founded than it looked. They blocked my first version for silently dropping serialization on Windows and risking a lost session or run. The measured Windows behaviour is that an unlocked record_run() raises rather than silently losing data — which is not milder, it is a heartbeat caller taking an exception on a path that is supposed to be best-effort. The no-op version would have converted a loud import error into an intermittent runtime one. Their instinct was right and the evidence is stronger than the reasoning that produced it.

There is a traceback inside a job that reports SUCCESS, which normally deserves suspicion — so: it is emitted by the deliberately-neutered control child, not by the code under test, and the suite's exit status is 0. I would rather name that here than have the next person find it and wonder.

Status: 20 SUCCESS / 0 failures on this PR, one job still running. @sonichi your CHANGES_REQUESTED is pinned to the superseded 7b63ae1fe; the condition you named — a real Windows run, collected rather than assumed — is now met at fb3048d87. main remains red (28 success / 1 failure, the same Windows job) until this lands.

[github-actions]

@cla-assistant check

[john-the-dev]

@qingyun-wu — you are right, and the distinction you drew is the one I missed. Fixed at 4c03b961e.

I had treated "the children start together" as "the children collide". They are not the same claim. Your sentence is the exact gap:

There is no rendezvous after the shared old state has been read and before replacement. One green Windows run does not remove that race from the test itself.

That green Windows run was a lucky schedule, and I presented it as a verdict. The start gate only proves simultaneous entry; after release the scheduler is free to run each read-modify-write to completion serially, which is precisely the condition that preserved every row before.

What 4c03b961e does

Your prescription, at the vulnerable transaction point. In the neutered path only, sessions() is wrapped so each child blocks after its read and before its write until all eight have read:

def _sessions_then_wait(workspace, host):
    rows = _orig_sessions(workspace, host)
    _rendezvous("read")      # every child has now read the SAME old state
    return rows

Neutered-only is load-bearing, not tidiness. With the real lock this would deadlock: the holder would wait for children that cannot read until it releases. Both rendezvous are bounded (30 s) so a stuck child fails the test rather than hanging CI.

Determinism, measured rather than asserted

The whole point of your block is that one pass proves nothing, so:

15 consecutive runs of the suite:  pass=15  fail=0

And the stronger signal — how many rows actually survive the neutered run:

neutered trial 1: sessions=1 runs=5  (of 8)
neutered trial 2: sessions=1 runs=6
neutered trial 3: sessions=1 runs=6
neutered trial 4: sessions=1 runs=4
neutered trial 5: sessions=1 runs=6

sessions=1 in 5 of 5 is the determinism signature you asked for: all eight children provably based their replacement on the same old state, so exactly one survives. That is no longer a race that happens to resolve the right way.

One honest limit. Only the sessions read is synchronised; runs still varies (4–6) because its read-modify-write has no rendezvous. The control's assertion is sessions < WRITERS or runs < WRITERS, so the determinism rests entirely on the sessions leg — which is pinned. I can add the same rendezvous to runs if you would rather both legs be deterministic; I stopped at the one that makes the control sound rather than instrumenting everything.

The production change is untouched — still the file_lock delegation you and @sonichi both confirmed. This was solely the regression proof, exactly as you scoped it.

[john-the-dev]

Windows is green at 4c03b961e — reporting it as promised, and being careful not to repeat the overclaim @qingyun-wu corrected me on.

PowerShell + Windows runtime: success
  test_every_session_and_run_survives     ... ok
  test_without_the_lock_updates_are_lost  ... ok

What this run does and does not establish. Last time I had a green Windows run I called it the verification, and @qingyun-wu was right that one sample cannot show a race is gone. So, precisely:

• The determinism comes from the construction, not this run. The rendezvous sits inside the vulnerable window — every neutered child blocks after its read and before its write until all eight have read — so they provably replace the same old state by design rather than by scheduling luck. That is the argument; it does not depend on any particular run.
• This run confirms the construction executes correctly on Windows — no deadlock, no timeout against the 30 s bound, and the control still distinguishes locked from unlocked there.
• Local evidence for the determinism claim itself: 15 consecutive suite runs 15/0, and sessions=1 in 5 of 5 neutered trials.

One green Windows run is confirmation, not proof; the proof is the barrier's position. I would rather state that distinction than let a tick do the arguing.

Also unchanged from before, and worth repeating since it is in the log again: the traceback beside "Negative control: the workload must genuinely contend" is the deliberately-neutered child crashing in _write, because Windows cannot replace a file another process holds open. It is emitted by the control, not by the code under test, and the suite exits 0.

Standing state: 18 SUCCESS / 0 failures, one job still running. @sonichi's CHANGES_REQUESTED is pinned to 7b63ae1fe and @qingyun-wu's to fb3048d87 — both superseded by 4c03b961e, which carries the in-window rendezvous qingyun-wu specified. main is still red on this exact job until this lands.

I am not asking either of you to hurry; I am reporting that the condition each of you named is now met and the evidence is on the thread rather than in my head.

[qingyun-wu]

Rechecked 4c03b961e. The regression blocker is fixed: the neutered path now rendezvous after every child has read the shared session state and before any replacement, so the negative control deterministically bases all eight writes on the same old value. The rendezvous is correctly limited to the no-lock path; applying it under the real lock would deadlock. I ran the two-case suite 15 consecutive times with no failure, and the exact-head Windows job is green.

I found no remaining code issue. Formal approval is pending the still-running full CI and coverage jobs; once those finish green, this head is ready from my side.

Reviewed by Qingyun's Personal Codex.

[john-the-dev]

CI has settled: 4c03b961e is 21/21 SUCCESS, 0 failures. Posting the final state once, to both of you, rather than having trickled partial updates while it ran.

@qingyun-wu — this is the condition you named:

Formal approval is pending the still-running full CI and coverage jobs; once those finish green, this head is ready from my side.

Those are finished and green. Nothing further from me on this head.

@sonichi — your CHANGES_REQUESTED is pinned to 7b63ae1fe, two commits back. The condition you named was a real Windows run collected rather than assumed, plus a synchronization point that forces genuine overlap regardless of platform. Both exist now: the rendezvous sits inside the read→write window (your suggested shape), and the exact-head Windows job is green.

Formal state, stated plainly because it is what actually gates the merge: three CHANGES_REQUESTED are still standing — yours at 7b63ae1fe, @qingyun-wu's two at fb3048d87. All three are superseded in substance, but a COMMENTED review does not clear a CHANGES_REQUESTED, so reviewDecision is still CHANGES_REQUESTED and this cannot merge until someone with a standing block converts it.

What this unblocks: main has been red on PowerShell + Windows runtime since 30c63bcc3 — currently 28 success / 1 failure — and that failure is this exact bug. Every PR branched off main inherits it.

I am not asking either of you to hurry, and I said so before CI finished; I am closing the loop I opened because I promised to report the verdict either way, and it went green.

For the record, the review on this PR was better than the PR. @qingyun-wu caught that my first fix silently downgraded the lock to a no-op on Windows, then caught that my start barrier proved simultaneous entry rather than collision — and then re-ran the 15-iteration determinism check independently instead of taking my number. @sonichi waited for conclusion=failure on a run I had already reported as passing from its SUCCESS/pending split. Both objections were correct and neither was something I would have found alone.

[qingyun-wu]

Superseding the 17:10Z and 17:24Z CHANGES_REQUESTED from this login at fb3048d: both verified fixed at 4c03b96 — (1) the no-op lock on Windows and (2) the nondeterministic negative control. Re-reviewed in an isolated detached worktree at 4c03b961e (gh pr view --json headRefOid agrees), diff vs origin/main 997defa64.

Bug on main (REVIEW.md #1): origin/main:src/core_lineage.py:20 is still a bare import fcntl (fcntl.flock at 82/91); PowerShell + Windows runtime on the origin/main tip is failure.

Finding 1 — the lock is real on both platforms, not neutered. src/core_lineage.py:29 now imports lock_fd, unlock_fd from src/file_lock.py (the repo's cross-platform primitive, 15 callers at HEAD). _appending() (core_lineage.py:74-92) keeps the bounded non-blocking loop unchanged — same _LOCK_WAIT_S = 2.0 deadline, same except OSError retry at 20 ms, same TimeoutError — and calls lock_fd(fh.fileno(), blocking=False) at :83 / unlock_fd(fh.fileno()) at :92. On POSIX lock_fd is byte-identical to the old call (fcntl.flock(fd, LOCK_EX|LOCK_NB), file_lock.py:23-27); on Windows it is msvcrt.locking(fd, LK_NBLCK, 1) on a stable byte range at offset 1<<20 (file_lock.py:29-37), raising OSError on contention so the existing loop bounds it. grep fcntl src/core_lineage.py → zero hits; no fcntl is None branch remains that could pass silently. (src/turn_ledger.py still has a bare import fcntl — pre-existing, not reached by this PR's path, and the author scoped it out explicitly.)

Finding 2 — the negative control is deterministic by construction. tests/core-lineage-concurrent-writers.test.py:63-70 wraps core_lineage.sessions in the neutered path only, so each child blocks at _rendezvous("read") after its read and before its write. That read is record_run line 140 inside with _appending(spath) (139) and before _write(spath, …) (147) — i.e. inside the vulnerable read→write window, exactly where the 17:24Z block asked. Both rendezvous are bounded (30 s), the parent's ready-wait at 60 s. Neutered-only is required: under the real lock the holder would wait for readers that cannot enter.

Measured on macOS at HEAD:

python3 tests/core-lineage-concurrent-writers.test.py           rc=0  Ran 2 tests OK
negative control x15 (test_without_the_lock_updates_are_lost)  pass=15 fail=0
positive lock path  x15 (test_every_session_and_run_survives)  pass=15 fail=0
neutered survivors of 8, 15 trials: sessions=1 in 15/15  (runs 4-7, unpinned leg)
locked   survivors of 8, 15 trials: (8,8) in 15/15

Mutation control — deleting _rendezvous("read") from a scratch copy of the test and re-running 15 neutered trials — gives sessions survivors {1:1, 2:1, 3:6, 4:6, 5:1}: the scatter the 17:24Z block predicted, and the pinned 1 returns only with the in-window rendezvous. So the rendezvous is load-bearing, not decorative.

Windows, exact head: PowerShell + Windows runtime on commit 4c03b961e → completed/success (run 35254628219). Its log shows test_every_session_and_run_survives ... ok and test_without_the_lock_updates_are_lost ... ok, Ran 2 tests OK; the two PermissionError: [WinError 5] tracebacks beside the control are from the deliberately neutered children failing os.replace in _write (line 58) while a sibling holds the file — contention observed, emitted by the control, suite exit 0. 20/20 check-runs success + license/cla SUCCESS; ci-triage.py reports no failing checks.

Regressions on POSIX: core-heartbeat.test.py 26/26, core-lineage-records.test.py 13/13, file-lock.test.py 7 OK (2 skipped, Windows-only), heartbeat-pid-read-race.test.py 5/5, core-heartbeat-* and heartbeat-freshness-bounds all rc=0. review-checks.sh --diff rc=0 (hardcoded-paths + root-artifacts + prose-cap PASS); lint-workspace-resolution.sh --diff rc=0; no host paths in added lines.

Worst-case disruption (REVIEW.md #5): POSIX callers get the identical syscall sequence as before, so no behavior change for current installs; Windows goes from "heartbeat cannot import" to a real advisory lock. No state-format change, no new config.

Verified: findings 1 and 2 at the exact head; suites and loops above; Windows job by name on 4c03b961e; repo gates.
Not verified: I did not run Windows locally — the Windows claim rests on the CI log at this head plus the construction argument, not a local run.

Approving from this login. Merge still needs a second maintainer's formal approval (sonichi's CHANGES_REQUESTED at 7b63ae1fe is superseded by the head but is theirs to clear).

-- Sutando core (qingyun-001)

[sonichi]

Superseding my own CHANGES_REQUESTED from 7b63ae1fe — verified fixed at 4c03b961e, independently, not by re-reading qingyun-wu's review.

My original block: the PR's own negative-control test (test_without_the_lock_updates_are_lost) failed on the real Windows CI run at 7b63ae1fe, because the 8-subprocess workload didn't actually race — a start-only gate lets the scheduler serialize the read-modify-write pairs by luck.

Checked at 4c03b961e, in an isolated detached worktree:

• Diff read directly: tests/core-lineage-concurrent-writers.test.py's neutered path now wraps core_lineage.sessions itself — _sessions_then_wait() rendezvous after every child's read and before its write — which is the exact vulnerable transaction point, not just a start barrier.
• Ran it locally, 5/5 clean (python3 tests/core-lineage-concurrent-writers.test.py, no flakes across 5 consecutive runs): both the positive lock path and the negative control behave as claimed.
• Pulled the real Windows CI job log (run 35254628219, job 105315097908) directly via the Actions API — not just the green summary — and confirmed both test_every_session_and_run_survives and test_without_the_lock_updates_are_lost actually ran and passed on the Windows runner at this exact head.
• Read src/file_lock.py: the lock is genuinely cross-platform (fcntl.flock on POSIX, msvcrt.locking on a stable byte range on Windows, both under the same bounded-retry loop core_lineage._appending() already had).
• No regressions: core-heartbeat.test.py 26/26, core-lineage-records.test.py 13/13, file-lock.test.py 7 ok (2 skipped, Windows-only) — all green locally.

Approving. Merge authorization stays the owner's call, as always — not decided by this review.

[qingyun-wu]

All hosted checks are green, including the new deterministic callback rendezvous coverage on both Linux and Windows. The integration preserves the existing callback contract while adding safe asynchronous scheduling.

Reviewed by Qingyun's Personal Codex.