fix(checkpoint): scope every approval gate to one run by JustinJLeopard · Pull Request #13 · JustinJLeopard/JustAi

JustinJLeopard · 2026-08-04T20:52:13Z

The defect

The gate an R2 task waits on was named after session_ref:

gates/gate_<session_ref>-plan-<index>.json

session_ref is a human label. justai run leaves it empty unless --session
is passed, the module default is the shared string sprint-2, and the
dashboard minted dashboard-<epoch-second> — two runs started in the same
second produced the same string. Two ordinary concurrent runs therefore agreed
on a label and a plan index, agreed on a filename, and waited on the same file.

One operator approval released both — including the run the operator never
looked at. The R1 veto collided the same way in the other direction, and gate
cleanup was free to delete a concurrent run's pending approval.

Reproduction (red before green)

tests/test_gate_identity_collision.py runs two real interpreters entering
through justai.cli.main, so the identity under test is the one the shipped
CLI derives. Against the previous implementation, on a clean export of the base
commit, the unapproved run reports:

[0] Change a published interface [R2] → R2 approved by operator

and executes — cleared through a hard gate by a decision nobody made about it.
The test asserts that consequence before it asserts the mechanism, so a
regression fails on the behaviour rather than on two paths matching.

Both parametrizations fail on base (ordinary-cli-defaults, i.e. no
--session at all, and reused-session-ref) and pass here. A positive control
in the same test releases the second run the moment its own gate is written, so
"still waiting" is the gate holding it and not the test failing to observe a
release.

The fix

A run id is a UUID and nothing else — not derived from the label, the clock,
the process, or the goal, because each of those collides between two runs
started together. It is never read from the environment, which would hand one
run's identity to every later run in the API server's single interpreter (the
same rule, and the same reason, as auto mode in #8).

Gates live at gates/<run_id>/plan-<index>.json, identified by a
GateIdentity of run id plus plan index.
Records carry the run they decided. A record that contradicts its own
location — another run's id, another plan index — is read as no decision,
not as an approval. Fail-closed: the R2 task keeps waiting.
A hand-written {"status": "approved"} still works. The path already scopes
it, and demanding metadata the operator was never told to write would break
the documented workflow without making anything safer.
Gates in the old layout are not consulted at all, in either direction. A file
that names no run decides nothing.
cleanup_run takes a run id and nothing else, so it cannot express a wider
target. A finished run cannot delete a concurrent run's pending approval.
--run-id resumes a run against gates already on disk; omitting it mints a
fresh identity, which is what an ordinary run wants.

session_ref stays what it always was: a label for tracing and memory that
scopes nothing.

Gate lifecycle (added after review)

Scoping fixed which file a decision lands in. Review found three ways the
lifecycle around that file still lost it. All three are fixed in 8dd5be3,
each with a real-process counterexample that fails against 16b4734.

1. An approval that arrived early was overwritten

The R2 branch announced its gate and then wrote pending to it
unconditionally, destroying whatever was already on disk — which is exactly
the decision it was about to wait for:

a --run-id resume approved before the restart;
a dashboard operator using the run id POST /api/run returns before the
run reaches a gate, which is the reason that endpoint returns it;
anyone answering the Discord notification inside its ten-second send.

The run then waited forever for a decision it had been given and had itself
deleted.

The marker is now created, never written over — one os.link, which fails
rather than replaces, so no reader catches it half-created and no writer loses
a race. Preserving what is on disk costs nothing in the other direction: a
record that cannot speak for this run (another run's id, another plan index,
malformed) reads as no decision whether it is kept or overwritten, so an R2
task holding one keeps waiting, and echo '{"status":"approved"}' > gate still
resolves it.

2. Cleanup unlinked the lock while holding it

flock excludes the holders of one inode, so unlinking the lock file under
the lock ended exclusion rather than the run. A process queued on that file
woke holding an inode nothing could reach by name; the next process found the
name free, created a second lock, and both then drove the same run's gates at
once.

Two real interpreters, both reporting they hold one run's lock:

queued: LOCK-QUEUEING / LOCK-HELD
later:  LOCK-QUEUEING / LOCK-HELD

cleanup_run now removes the run's records and keeps the lock.
run_gate_lock additionally re-takes its lock if the file it acquired is no
longer the one at the path — which is what makes an external /tmp reaper, or
the sweep below, safe. That is a revalidation, not a lease: nothing expires,
nothing is stolen, and a live holder is never displaced.

3. Directories leaked, and nothing collected them

Keeping the lock means a finished run's directory outlives it, and a run
interrupted before cleanup already left one. sweep_gate_dirs() bounds both:
it collects directories holding no gate record once they are older than
JUSTAI_GATE_TOMBSTONE_TTL (default one hour), and each run sweeps after
cleaning up.

The ordering is the design, and it is why no lease is needed:

take the run's lock without waiting — a run mid-checkpoint holds it, and
a parked R2 gate holds it for as long as the operator takes;
confirm the lock is still the one at the path, and the directory is spent;
rename the directory out of the namespace — renaming is not deleting;
release, then delete. No unlink ever happens under a held lock.

What is honestly not swept, at any age:

Kept	Why
A run still holding a gate record	An interrupted R2 gate is a pending approval nobody answered, and is what `--run-id` resumes against. Retained deliberately and indefinitely — an operator who is done with one removes the directory.
A run whose lock is held	It is still going.
A directory holding a file JustAi did not write	Deleting something unrecognised is not cleanup.
Anything under `gates/` not named by a run id	This module did not write it.

The one-hour default is a margin, not a retention policy: a spent directory's
only content is a zero-byte lock, so the delay guards the microseconds another
process spends between creating a directory and locking the file inside it.

Fail-closed behaviour is unchanged, and pinned

R2 still waits for an approval (including in auto mode), R3 still never
proceeds, an unreadable or malformed gate is never an approval, and auto mode
still skips only the R1 wait. Preserving a refused record does not soften any
of that — test_a_record_that_decides_nothing_leaves_the_gate_closed pins a
foreign run id, a foreign plan index, and unreadable JSON as still closed.

Migrated callers

The previous evaluate(task, task_id=...) string contract had 15 callers across
five test modules. tests/test_auto_mode_isolation.py — the #8 auto-mode guard
— keeps its strength: each veto now names the run it is planted for, and the
API case pauses the run before its gate so both vetoes are genuinely
pre-planted rather than racing it. Verified by mutation: dropping the vetoes
fails 4 of its tests.

Four existing assertions encoded the old "cleanup removes the directory"
contract and now assert what cleanup honestly leaves: no gate records, and a
lock that outlives them.

Evidence

Check	Result
Full suite, clean env	517 passed, 14 subtests (444 at base of the branch, 494 at `16b4734`)
Reversed collection order	517 passed, 14 subtests
`ruff check justai/ tests/`	clean (base clean; no new findings)
Collision probe vs base	3 failed, for the intended reason
`tests/test_gate_lifecycle.py` vs `16b4734`	20 failed, 3 passed
`tests/test_gate_lifecycle.py` here	23 passed

The 3 that already passed at 16b4734 are the ones that were already true:
fail-closed reads of a refused record, the operator override, and cleanup's
containment.

Known sharp edges

run_gate_lock is still not reentrant — each call opens its own descriptor
and flock blocks a second one even within a process — so taking it around
cleanup_run, which locks internally, self-deadlocks. The one call site is
correct and both docstrings say so.
_claim_gate uses os.link, which every plausible runtime root supports
(tmpfs, ext4, xfs, overlayfs, NFS) but FAT/exFAT and some FUSE mounts do not.
A runtime root on one of those would raise rather than silently misbehave. No
fallback is wired, because an untested branch for a filesystem nobody points
JUSTAI_RUNTIME_ROOT at is worse than a loud error.
Tests that exercise the orchestrator without redirecting GATE_SIGNAL_DIR
now leave ~20 empty directories under the real /tmp/justai/gates per suite
run, where cleanup used to rmdir them. They are lock-only, and any later
run sweeps them once they are an hour old. Pointing JUSTAI_RUNTIME_ROOT at
a temp dir for the suite would remove the residue; that is test
infrastructure and is not changed here.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RCzyihd6aGZ95zH12LboGv

The gate an R2 task waited on was named after `session_ref`: `gates/gate_<session_ref>-plan-<index>.json`. That is a human label — `justai run` leaves it empty unless `--session` is passed, the module default is the shared string `sprint-2`, and the dashboard reused `dashboard-<epoch-second>`. Two ordinary concurrent runs therefore agreed on a label and a plan index, agreed on a filename, and waited on the same file. One operator approval released both, including the run the operator never looked at. The R1 veto collided the same way, in the other direction, and gate cleanup was free to delete a concurrent run's pending approval. Reproduced with two real interpreters entering through the shipped CLI: against the previous implementation the unapproved run prints `R2 approved by operator` and executes, on a decision nobody made about it. A run id is now a UUID and nothing else — not derived from the label, the clock, the process, or the goal, since each of those collides between two runs started together, and never read from the environment, which would hand one run's identity to every later run in the API server's single interpreter. Gates live at `gates/<run_id>/plan-<index>.json`, records are written atomically and carry the run they decided, a record contradicting its own location is read as no decision rather than as an approval, and cleanup takes a run id and nothing else so it cannot express a wider target. Gates in the old layout are not consulted at all, in either direction: a file that names no run decides nothing. `session_ref` stays what it always was — a label for tracing and memory that scopes nothing. `--run-id` resumes a run against gates already on disk; omitting it mints a fresh identity. Fail-closed behaviour is unchanged and pinned: R2 still waits for an approval, R3 still never proceeds, an unreadable gate is never an approval, and auto mode still skips only the R1 wait. Callers of the previous `evaluate(task, task_id=...)` string contract are migrated. `test_auto_mode_isolation.py` keeps the PR #8 guard intact by naming the run each veto is planted for, and its API case now pauses the run before its gate so both vetoes are genuinely pre-planted rather than racing it. Suite: 494 passed, 14 subtests, in both collection orders (444 at base). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NN4rzeBo2F6eu7dhTMhPPG

vercel · 2026-08-04T20:52:19Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
justai-demo	Ready	Preview	Aug 4, 2026 10:53pm

Scoping every gate to a run id fixed which file a decision lands in. Three things around that file still lost it. An approval that arrived before the run recorded its gate was destroyed by the recording. The R2 branch announced the gate and then wrote `pending` to it unconditionally, so everything already on disk went with it — a `--run-id` resume approved before the restart, a dashboard operator using the run id `POST /api/run` returns before the run reaches a gate, or anyone answering the Discord notification inside its ten-second send. The run then waited forever for a decision it had been given and had itself deleted. The marker is now created rather than written, via `os.link`, so it only appears where nothing has decided yet. A record that cannot speak for this run is still no decision, so an R2 task holding one keeps waiting, and `echo > gate` still resolves it. Cleanup unlinked the lock file while holding it. `flock` excludes the holders of one inode, so that ended exclusion rather than the run: a process queued on that file woke holding an inode nothing could reach by name, the next process found the name free and created a second lock, and both then drove the same run's gates at once. Reproduced with two real interpreters, both printing LOCK-HELD for one run. Cleanup now removes the run's records and keeps the lock, and `run_gate_lock` re-takes its lock if the file it acquired is no longer the one at the path — which is what makes an external /tmp reaper, or the sweep below, safe. It is a revalidation, not a lease: nothing expires and a live holder is never displaced. Keeping the lock means the directory outlives the run, and a run interrupted before cleanup already left one. `sweep_gate_dirs` bounds both: it collects directories that hold no gate record once they are older than `JUSTAI_GATE_TOMBSTONE_TTL` (default one hour), and each run sweeps after cleaning up. It takes the run's lock without waiting, renames the directory out of the namespace while holding it, and deletes only after releasing, so no unlink ever happens under a held lock. A run still going, still holding a decision, or holding a file JustAi did not write is never swept — an interrupted R2 gate is a pending approval nobody answered and is what `--run-id` resumes against, so those are retained deliberately and indefinitely. `tests/test_gate_lifecycle.py` adds 23 tests, 20 of which fail against the parent commit: the preapproved resume and the approval written inside the notification window both hang there, and the queued-lock case exhibits two simultaneous holders. Four existing assertions that encoded "cleanup removes the directory" now assert what cleanup honestly leaves. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RCzyihd6aGZ95zH12LboGv

An approval is a decision to execute one task in one run, once. Scoping gates to a run id fixed which file a decision lands in; it left four ways the decision could still be acted on twice, thrown away, or never checked. One approval released two executions. The checkpoint stage held the run's lock for the gate loop and released it there, so everything the decision authorises ran unserialised: a second `--run-id` process queued for that lock, woke the moment the first stage ended, read the same approval — cleanup had not yet re-taken the lock to remove it — and dispatched the same work. `own_run` now claims a run for one process from before the first stage until after the terminal cleanup, and refuses a second process outright, because a queued one executes the same run against the same decision a moment later. Cleanup moves after dispatch and takes the claim instead of the lock, which `flock` would block even inside one process. The lock file is still never unlinked. The sweep deleted directories for their names. Anything under `gates/` called `.trash-*` was removed without this module ever having written one, so an operator's own `.trash-operator-owned/keep.txt` went with it; and a directory was parked under `.trash-<run_id>`, one name per run, with whatever already sat there deleted to make room. A directory now leaves the namespace under `.trash-<run_id>-<fresh uuid>` carrying a marker that names this module, the run, and that name — and only a directory whose name, marker and contents all agree is deleted, entry by entry, never through a symbolic link. A run killed at its gate was kept for the life of the machine. "Holds a record" meant "never collected, at any age", which is right for a gate an operator may still answer and wrong as an end state. `run_state` names what a directory is — active, resumable, abandoned, terminal, foreign — and `prune_abandoned_runs` collects the abandoned ones on a window measured in days plus a count bound. A run is dated by its records, since an approval is written into an existing file and leaves the directory's own timestamp alone. None of this was checked by anything. Every green check a pull request could show was static analysis, a Node build, or a preview deploy, so a change that broke the fail-closed behaviour still showed all green. `python-ci.yml` runs ruff and the full suite on 3.12 and 3.13, with the gate and concurrency suite named separately, against the pull request's head SHA rather than the merge preview GitHub synthesises from it. Evidence: 557 passed (ruff clean) on CPython 3.13.12 and 3.12.13. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SToNzf8WL9ciGotn5UoDzV

chatgpt-codex-connector · 2026-08-08T16:20:07Z

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

Copilot

Pull request overview

This PR fixes a concurrency defect in JustAi’s checkpoint approval gates by scoping every operator gate and cleanup action to a unique per-run UUID, preventing cross-run approval/veto collisions and unsafe concurrent resumes.

Changes:

Introduces run_identity.py and threads a validated run_id through CLI/API/orchestrator, with gates stored at gates/<run_id>/plan-<index>.json.
Hardens gate lifecycle and concurrency: claim-only pending markers, per-run ownership (own_run) to prevent double execution, safe cleanup + bounded sweeping/pruning of gate directories.
Adds Python CI workflow and extensive multi-process regression tests covering identity, lifecycle, retention, and run ownership semantics.

Reviewed changes

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

Show a summary per file

File	Description
`tests/test_sprint8.py`	Updates CLI run test to assert `run_id=None` for the ordinary (fresh identity) case.
`tests/test_sprint7.py`	Updates checkpoint tests to use `GateIdentity` with fresh run ids per test.
`tests/test_slice2.py`	Makes trace-event assertion resilient to formatting/wrapping changes.
`tests/test_orchestrator.py`	Isolates gate dirs per test and updates checkpoint tests to the new gate identity API.
`tests/test_llm_checkpoint_cli_coverage.py`	Updates gate read/write roundtrip test to use `GateIdentity` + per-test gate root + `cleanup_run`.
`tests/test_gate_run_ownership.py`	Adds multi-process tests ensuring one process owns a run end-to-end (no queued resumes / double execution).
`tests/test_gate_run_identity.py`	Adds comprehensive tests for run id validation, gate scoping, legacy gate ignoring, cleanup containment, and fail-closed behavior.
`tests/test_gate_retention.py`	Adds tests for safe sweeping/pruning semantics (no deletion by name, bounded retention, symlink safety).
`tests/test_gate_lifecycle.py`	Adds multi-process lifecycle tests for “no overwrite”, “no unlink under lock”, and directory lifecycle invariants.
`tests/test_gate_identity_collision.py`	Adds the two-interpreter reproduction proving one approval cannot release two concurrent runs.
`tests/test_completion_integrity.py`	Updates local stub signature to match the new checkpoint gate identity shape.
`tests/test_ci_workflows.py`	Adds tests asserting Python CI exists, runs on PRs, pins actions, and names critical suites.
`tests/test_auto_mode_isolation.py`	Updates auto-mode isolation tests to plant vetoes scoped to explicit `run_id`s and remove API races.
`tests/gate_lifecycle_child.py`	Adds a child-process helper for lifecycle tests (hold lock / clean / sweep).
`tests/gate_harness.py`	Adds shared harness for multi-process gate tests, stubbing orchestration while keeping gates real.
`tests/gate_collision_child.py`	Adds child-process entrypoint that runs through `justai.cli.main` for realistic identity reproduction.
`README.md`	Documents run ids, gate paths, resume semantics, and gate directory lifecycle/retention behavior.
`justai/run_identity.py`	Implements minting and validation for run ids (UUID-based, canonical hex; rejects labels/timestamps).
`justai/orchestrator.py`	Threads `run_id` through orchestration, scopes gates via `GateIdentity`, and performs cleanup/sweep/prune under run ownership.
`justai/cli.py`	Adds `--run-id`, passes it through to orchestrator, and handles invalid ids / active-run refusal cleanly.
`justai/checkpoint.py`	Replaces string gate ids with `GateIdentity`, adds per-run locking/ownership, atomic gate IO, and bounded retention collectors.
`justai/api.py`	Mints a UUID run id server-side and returns it immediately alongside `session_ref` for operator gating.
`docs/TESTING.md`	Documents the new Python CI workflow and what it does/does not prove.
`docs/ARCHITECTURE.md`	Updates architecture boundaries/flow to include run identity and run-scoped gates + ownership.
`CHANGELOG.md`	Records the signature/layout changes and new CI coverage for Python.
`.github/workflows/python-ci.yml`	Adds a pinned, PR-running Python CI workflow running ruff + named gate suite + full pytest on 3.12/3.13.

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

+    lock_file = directory / LOCK_FILENAME
+    try:
+        fd = os.open(lock_file, os.O_CREAT | os.O_RDWR, 0o600)
+    except OSError:
+        return None


vercel Bot deployed to Preview August 4, 2026 21:57 View deployment

vercel Bot deployed to Preview August 4, 2026 22:53 View deployment

JustinJLeopard marked this pull request as ready for review August 8, 2026 16:20

Copilot AI lite review requested due to automatic review settings August 8, 2026 16:20

Copilot started reviewing on behalf of JustinJLeopard August 8, 2026 16:20 View session

JustinJLeopard merged commit 3df6912 into main Aug 8, 2026
9 checks passed

JustinJLeopard deleted the codex/justai-gate-run-identity-20260804 branch August 8, 2026 16:21

Copilot AI reviewed Aug 8, 2026

View reviewed changes

Comment thread justai/checkpoint.py

Comment on lines +552 to +556

lock_file = directory / LOCK_FILENAME

try:

fd = os.open(lock_file, os.O_CREAT | os.O_RDWR, 0o600)

except OSError:

return None

JustinJLeopard mentioned this pull request Aug 9, 2026

config: llama.cpp local profile for the pipeline #19

Merged

Navigation Menu

fix(checkpoint): scope every approval gate to one run - #13

fix(checkpoint): scope every approval gate to one run#13
JustinJLeopard merged 3 commits into
mainfrom
codex/justai-gate-run-identity-20260804

JustinJLeopard commented Aug 4, 2026 •

edited

Loading

Uh oh!

vercel Bot commented Aug 4, 2026 •

edited

Loading

Uh oh!

chatgpt-codex-connector Bot commented Aug 8, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

JustinJLeopard commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The defect

Reproduction (red before green)

The fix

Gate lifecycle (added after review)

1. An approval that arrived early was overwritten

2. Cleanup unlinked the lock while holding it

3. Directories leaked, and nothing collected them

Fail-closed behaviour is unchanged, and pinned

Migrated callers

Evidence

Known sharp edges

Uh oh!

vercel Bot commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Aug 8, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

JustinJLeopard commented Aug 4, 2026 •

edited

Loading

vercel Bot commented Aug 4, 2026 •

edited

Loading