feat: create file locks on concurrent builds and scaffolding verification/tests to prevent race conditions and conflicts while reviewing
This commit is contained in:
5 files changed
+56
No files matched your search
@@ -51,11 +51,43 @@ GRAPH_STATE_FILE, so an inherited live state file would silently win.
|
|||||||
import json
|
import json
|
||||||
import os
|
import os
|
||||||
import signal
|
import signal
|
||||||
|
import fcntl
|
||||||
import subprocess
|
import subprocess
|
||||||
|
import sys
|
||||||
import time
|
import time
|
||||||
|
|
||||||
PER_COMMAND_TIMEOUT_SECS = 900
|
PER_COMMAND_TIMEOUT_SECS = 900
|
||||||
TOTAL_DEADLINE_SECS = float(os.environ.get("ADVERSARY_RUN_CHECKS_DEADLINE_SECS") or 3300)
|
TOTAL_DEADLINE_SECS = float(os.environ.get("ADVERSARY_RUN_CHECKS_DEADLINE_SECS") or 3300)
|
||||||
|
STACK_LOCK_PATH = os.path.expanduser("~/.cache/coyote/review-stack.lock")
|
||||||
|
STACK_LOCK_WAIT_SECS = float(os.environ.get("ADVERSARY_STACK_LOCK_WAIT_SECS") or 1200)
|
||||||
|
|
||||||
|
|
||||||
|
def acquire_stack_lock():
|
||||||
|
"""Serialize with the probe lane's shared-stack work (advisory flock on a
|
||||||
|
well-known path; the probe is instructed to flock the same file around
|
||||||
|
stack boot/teardown). Bounded wait so a hogged lock can never turn into a
|
||||||
|
false red verification: on timeout we proceed WITHOUT the lock and note it
|
||||||
|
on stderr. The fd is held for the process lifetime (released on exit)."""
|
||||||
|
try:
|
||||||
|
os.makedirs(os.path.dirname(STACK_LOCK_PATH), exist_ok=True)
|
||||||
|
fd = os.open(STACK_LOCK_PATH, os.O_CREAT | os.O_RDWR)
|
||||||
|
except OSError as e:
|
||||||
|
sys.stderr.write(f"WARN: review-stack lock unavailable ({e}); running unlocked\n")
|
||||||
|
return None
|
||||||
|
limit = time.monotonic() + STACK_LOCK_WAIT_SECS
|
||||||
|
while True:
|
||||||
|
try:
|
||||||
|
fcntl.flock(fd, fcntl.LOCK_EX | fcntl.LOCK_NB)
|
||||||
|
return fd
|
||||||
|
except OSError:
|
||||||
|
if time.monotonic() >= limit:
|
||||||
|
os.close(fd)
|
||||||
|
sys.stderr.write(
|
||||||
|
"WARN: review-stack lock still held after bounded wait — proceeding "
|
||||||
|
"WITHOUT it; shared-stack collisions with the probe lane are possible\n"
|
||||||
|
)
|
||||||
|
return None
|
||||||
|
time.sleep(5)
|
||||||
DRAIN_TIMEOUT_SECS = 10
|
DRAIN_TIMEOUT_SECS = 10
|
||||||
TAIL_LINES = 50
|
TAIL_LINES = 50
|
||||||
MAX_TAIL_CHARS = 8000
|
MAX_TAIL_CHARS = 8000
|
||||||
@@ -181,6 +213,7 @@ def main():
|
|||||||
)
|
)
|
||||||
deadline = time.monotonic() + TOTAL_DEADLINE_SECS
|
deadline = time.monotonic() + TOTAL_DEADLINE_SECS
|
||||||
results = []
|
results = []
|
||||||
|
_stack_lock = acquire_stack_lock() if cmds else None # noqa: F841 — held until exit
|
||||||
for cmd in cmds:
|
for cmd in cmds:
|
||||||
remaining = deadline - time.monotonic()
|
remaining = deadline - time.monotonic()
|
||||||
if remaining <= 0:
|
if remaining <= 0:
|
||||||
|
|||||||
@@ -436,6 +436,13 @@ nodes:
|
|||||||
(cross-domain concerns from the leaders' reports + teammate alerts, or "None"),
|
(cross-domain concerns from the leaders' reports + teammate alerts, or "None"),
|
||||||
`operational_history` (from prior_art_notes; "" when the lane didn't run),
|
`operational_history` (from prior_art_notes; "" when the lane didn't run),
|
||||||
`org_context_section` (from org_context_notes; "" when the lane didn't run).
|
`org_context_section` (from org_context_notes; "" when the lane didn't run).
|
||||||
|
Prior review decisions supplied in the caller context are SETTLED: never
|
||||||
|
invert or re-litigate one on the same line without NEW evidence (the line
|
||||||
|
changed, a new failure appeared). If you believe a settled decision is
|
||||||
|
wrong, do not re-flag it as a finding — surface it in `cross_file` as an
|
||||||
|
explicit conflict citing the prior decision, so the human sees a
|
||||||
|
disagreement, not a flip-flop.
|
||||||
|
|
||||||
prompt: |
|
prompt: |
|
||||||
Domain reports ({{file_count}} changed files, grouped into the domain slices below):
|
Domain reports ({{file_count}} changed files, grouped into the domain slices below):
|
||||||
{{domain_reports}}
|
{{domain_reports}}
|
||||||
|
|||||||
@@ -101,6 +101,14 @@ instructions: |
|
|||||||
|
|
||||||
## Rules
|
## Rules
|
||||||
|
|
||||||
|
0. **Shared-stack discipline (parallel review lanes).** Wrap every stack boot/teardown
|
||||||
|
(docker compose, dev servers, fixed-port services) in
|
||||||
|
`flock "$HOME/.cache/coyote/review-stack.lock" <command>` — the adversary's
|
||||||
|
verification stage holds the same lock. A held port NEVER justifies removing or
|
||||||
|
stopping a container you did not create THIS RUN: wait on the lock, pick another
|
||||||
|
port, or return INCONCLUSIVE naming the conflict. Killing another lane's container
|
||||||
|
is a defect, not a remedy.
|
||||||
|
|
||||||
1. **Never modify implementation code.** Your only writes are new/updated TEST files (in the
|
1. **Never modify implementation code.** Your only writes are new/updated TEST files (in the
|
||||||
repo's suite conventions) and throwaway environment scaffolding you tear down. The
|
repo's suite conventions) and throwaway environment scaffolding you tear down. The
|
||||||
implementer owns all fixes.
|
implementer owns all fixes.
|
||||||
|
|||||||
@@ -407,6 +407,11 @@ nodes:
|
|||||||
The workspace may contain test files from an aborted earlier attempt of
|
The workspace may contain test files from an aborted earlier attempt of
|
||||||
this lane; reconcile rather than duplicate.
|
this lane; reconcile rather than duplicate.
|
||||||
|
|
||||||
|
Shared-stack rule: wrap any stack boot/teardown in
|
||||||
|
`flock "$HOME/.cache/coyote/review-stack.lock" <command>` (the adversary's
|
||||||
|
verification stage holds the same lock), and NEVER remove or stop containers
|
||||||
|
you did not create this run — a held port means wait, re-port, or INCONCLUSIVE.
|
||||||
|
|
||||||
Derive expected behaviors from the plan/contract BEFORE reading
|
Derive expected behaviors from the plan/contract BEFORE reading
|
||||||
implementation code, run existing suites first, then author spec-first
|
implementation code, run existing suites first, then author spec-first
|
||||||
tests for the uncovered delta. Return your USAGE_PROBE verdict.
|
tests for the uncovered delta. Return your USAGE_PROBE verdict.
|
||||||
|
|||||||
@@ -314,6 +314,9 @@ instructions: |
|
|||||||
Diff: <git range like main...HEAD, or 'worktree' for uncommitted changes>
|
Diff: <git range like main...HEAD, or 'worktree' for uncommitted changes>
|
||||||
Rigor: <rigor> Security posture: <posture>
|
Rigor: <rigor> Security posture: <posture>
|
||||||
Plan / acceptance criteria: <pasted verbatim — enables the adversary lane; omit when none>
|
Plan / acceptance criteria: <pasted verbatim — enables the adversary lane; omit when none>
|
||||||
|
Prior review decisions: <on re-review rounds ONLY: earlier rounds' resolved/rejected
|
||||||
|
findings verbatim (rejected-finding log lines, round-comment reconciliations) — settled
|
||||||
|
decisions the reviewers must not re-litigate; omit on the first round>
|
||||||
Local-run recipe / usage suites: <from the plan — enables the probe lane; omit when none>"
|
Local-run recipe / usage suites: <from the plan — enables the probe lane; omit when none>"
|
||||||
--variables {"verification_commands": "[\"cargo test --all\", \"cargo clippy -- -D warnings\"]"}
|
--variables {"verification_commands": "[\"cargo test --all\", \"cargo clippy -- -D warnings\"]"}
|
||||||
<— the cargo commands above are only an EXAMPLE: replace them with THIS project's exact build/test/lint commands; never guess, and pass "[]" when none apply>
|
<— the cargo commands above are only an EXAMPLE: replace them with THIS project's exact build/test/lint commands; never guess, and pass "[]" when none apply>
|
||||||
|
|||||||
Reference in new issue
Block a user