100%

S3 - Per-repo checkpoint + journal

Goal

S3 - Per-repo checkpoint + journal.

Implementation Sketch

North-star contract: mission.md — /commit-push’s terminal outcome is ALWAYS (a) committed to HEAD AND pushed to origin, or (b) a visible hard halt the user acts on; NEVER a silently-growing local-only queue reported as success. No separate design doc exists on disk; the load-bearing spec is discussions/01-commit-push-durability/discussion.md:82-100 + the S2 section body (per-repo-journal restructures the same apply() seam S2 finished editing).

Restructure target

  • apply() (state_machine.py:1251) is the single hottest / highest-fanout function in scripts/commit_push/ (hotspot 40800, span 798, fanout 31, betweenness 7907 — the file’s outlier). S3 collapses its TWO separate batch passes into ONE per-repo interleave:
    • today: Step 5 stage_and_commit_all commits ALL repos (:1571-1789), THEN Step 7 push_all pushes ALL committed repos (:1827-1881) — a mid-run death between the two batches strands every already-committed repo with NO push attempted.
    • target: one for repo: commit -> journal -> push -> journal -> next iteration.
  • Regression concentrates inside apply()’s Step 5-8.5 window (:1558-1928), edited by three consecutive efforts (S1, S2, BUG-07-532) in one cycle — land carefully, this is the file’s most heavily pinned function (24 direct tests).

Read-first order (before touching apply())

  • bug-tracker/plans/BUG-07-532/content/root-cause-analysis--s-c7f5a90c.md + fix-consensus--s-13b690b9.md — the exhaustive 4-way exit-path enumeration BUG-07-532 already proved; S3 re-proves it exhaustive under the NEW per-repo-loop shape.
  • plans/commit-push-durability/content/preserve-hard-halts--s-4ddb5995.md (S2, completed, full — not the summary) — the payload.commits/payload.pushed field shape S3 persists.

Reuse contract (never re-derive)

  • Per-repo push step: _push_one (push.py:221) — the shared primitive both push_all/push_pending already call (similar=0.912). A new push-one-repo helper is a fresh LEAK:algorithmic-duplication.
  • Journal write primitive: _atomic_write_text (state_machine.py:31) — the same primitive _write_state already uses.
  • Journal location: resolve_git_dir(repo) (queue_git.py:114) — the durable cross-run-id home .commit-push-apply.lock already uses; NEVER the run-id-namespaced scratch dir the ephemeral state.json run journal (_write_state, :2254) uses (invisible to a FUTURE run under a different run_id).
  • Journal entry shape: the {repo,sha,subject,branch} / {repo,remote_branch} fields S2 wired (next_action.schema.json:4528-4575) — persist verbatim, do not re-derive.
  • Committed-not-pushed discovery: _recover_clean_ahead (state_machine.py:443) / detect_unpushed (repos.py:238) already scan HEAD-ahead-of-tracking repos — the journal-read-on-entry mechanism SUPERSEDES (explicitly), never duplicates, this scan.
  • Advisory lock: extend .commit-push-apply.lock (staging.py:180-262) to span commit AND its now-adjacent push instead of releasing in the finally right after git commit — widen BUG-07-287’s discipline, closing the parallel-session race the batch gap left open; do not invent a new lock.

Preserve-constraints (do NOT touch)

  • Do NOT widen DEFERRABLE_HALT_REASONS — S1 removed the defer-as-success park path; S3 introduces no new deferral.
  • Do NOT disturb drain_on_entry / _drain_on_entry_all (state_machine.py:2475, defer_path.py) — a DIFFERENT durability mechanism (parked queue refs refs/commit-push-queue/*) S1/S4/S5 depend on. S3’s “on entry, journal-driven push first” wires at the SAME run_state_machine entry ALONGSIDE _drain_on_entry_all, never replacing it.
  • Do NOT delete _recover_stranded_siblings / _build_recovery_commit_descriptors / _stranded_commit_descriptors — BUG-07-532 owns them and is still open (deletion mid-open-bug = routing.md §2 active-fix-containment violation).

Decision rules

  • Exit-path exhaustiveness (GAP:conditional-fallthrough-not-exhaustive): the per-repo iteration’s NEW closed exit set — commit success/fail x push success/fail x journal-write success/fail — is enumerated EXHAUSTIVELY and explicitly match-guarded; no combination silently falls through to a success terminal. BUG-07-532 already paid twice for under-enumerating this exact function’s exit set.
  • Entry sequencing: on entry, read each repo’s journal; a committed_not_pushed phase pushes FIRST (before any new commit lands this run) — deterministic, replacing _recover_clean_ahead’s incidental Step-7.5 recovery.
  • Comment hygiene (COMMENT_HYGIENE_DRIFT:inaccurate-comment): every # Step 5/6/7/7.5 boundary comment (:1558,1791,1827,1883,1919) describes the batch-then-batch shape — update ALL in the SAME commit that reshapes the loop, not as a follow-up.

Close-the-class follow-on (FLAG only — do NOT act in S3)

  • Once the per-repo interleave lands, the “earlier-committed sibling stranded because a LATER repo failed” bug CLASS BUG-07-532’s recovery machinery patches becomes structurally unreachable (no batch window remains between commit and push). This is a “close the class” simplification for the section/bug that closes BUG-07-532 AFTER it closes — NOT an S3 deletion (BUG-07-532 open; coordinate, do not remove).

Verification

  • Falsifier (a) is an L12 production-entry-point concern (layer-coverage.md §1.2): a mocked function-call-order test cannot prove survival across a real process kill. Drive the REAL python -m scripts.commit_push run entry point (extend _run_self_test, cli.py:556): spawn it as a real child, SIGKILL after repo N’s journal write lands but BEFORE repo N’s push completes, re-invoke run_state_machine in a FRESH process, assert it reads committed_not_pushed for repo N and pushes it before repo N+1. Idiom: scripts/test_all_runtime/tests/test_process_identity.py::test_process_tree_termination_escalates_term_to_kill.
  • Negative pins (INVERTED-TDD): (a) positive — mid-run-death-then-resume PUSHES the stranded repo before any new commit; (b) negative — repo N’s push happens before repo N+1’s commit is even attempted (proving the interleave REPLACED the batch order, not merely present alongside it).
  • Regression scope: apply()’s 24 direct tests each encode the two-batch shape; re-run the FULL test_clean_ahead_push_path.py + test_push_failure_classification.py + test_state_machine.py suites, not a scoped subset (S2’s still-load-bearing directive).
  • Hygiene scope: scripts/commit_push/** is TOOLING-scope (CLAUDE.md §HYGIENE / CODING RULES SCOPE) — universal SRP/SSOT + STRUCTURE/LEAK/GAP/INVERTED-TDD families apply; the compiler-only L1-L10 matrix does NOT; L12 production-entry-point discipline still applies.

Graph-facet caveat

  • This section’s intel dossier ran on a DEGRADED-BUT-ADMISSIBLE snapshot: insights / co_change / test_topology / sibling_divergence facets STALE (every hotspot / community / co-change / coverage row corroborated against live source reads, never trusted from the graph alone). MISSING_ABSTRACTION / surprise-coupling returned REAL zero rows (no candidate nominated — not “clean guaranteed”). symbol-plans is structurally empty for scripts/** Python (MENTIONS_CODE gotcha) — ownership established by direct plan/bug-body reading only.

Work Items

  • After each repo commits, push it before entering the next repo
  • After each repo commits, push it before entering the next repo: merge the current two separate batch loops (stage_and_commit_all’s Step-5 commit-all loop, state_machine.py:1571-1789; push_all’s Step-7 push-all loop, :1827-1881) into one per-repo commit-then-push iteration. Reuse the existing shared _push_one primitive (push.py:221) as the per-repo push step — never author a new push-one-repo helper (push_all and push_pending already converged on _push_one at similar=0.912; a third variant is a fresh LEAK:algorithmic-duplication). Extend the per-repo advisory lock (.commit-push-apply.lock, staging.py:180-262) to span both the commit and its immediately-following push instead of releasing it in the finally right after git commit — closing, not narrowing, the BUG-07-287 parallel-session race window now that commit and push are adjacent. Exhaustively enumerate and explicitly match-guard every new per-repo exit-path combination this interleave introduces (commit success/fail x push success/fail x journal-write success/fail) so no combination silently falls through to a success terminal — BUG-07-532 already proved this exact function’s exit-path set was under-enumerated twice; do not reintroduce that defect class here.
  • Write a durable run journal before/after each commit and each push
  • Write a durable PER-REPO journal — anchored at each repo’s persistent .git dir via resolve_git_dir(repo) (queue_git.py:114), the same durable cross-run-id location .commit-push-apply.lock already uses (staging.py:180) — NEVER the ephemeral, run-id-namespaced scratch_dir the existing state.json run journal (_write_state, state_machine.py:2254-2261) uses today. Reuse the existing _atomic_write_text primitive (state_machine.py:31) for the write. Write a journal entry before AND after each commit and each push for that repo, so the entry’s phase field distinguishes committed_not_pushed from committed_and_pushed. This must be discoverable by a FUTURE run under a DIFFERENT run_id (the ephemeral state.json cannot serve this role — it is invisible outside the run_id-scoped scratch dir that produced it).
  • On entry, a committed_not_pushed journal state pushes first before any new work
  • Falsifier (a): kill after each journal boundary -> committed_not_pushed recovered, next run pushes first

Intel dossier — pointer and tier disposition

Dossier: per-repo-journal--s-117cfbf9.intel.md (this section’s content/intel/ sidecar)

Read that dossier from line 1 through EOF before acting on this section. This block is a pointer and an audit record, never a substitute: not the BLUF, not a summary, not selected tiers.

Dossier tierTitleLinesDispositionWhere / why
00. Bottom line up front30integratedImplementation Sketch -> Restructure target, Reuse contract, Close-the-class follow-on
11. The target, verbatim29integratedImplementation Sketch (north-star contract line + Read-first order; mission.md / discussion.md:82-100 pointers)
22. Terrain — symbols in the objective area (with files)61integratedRestructure target, Reuse contract (every apply()/stage_and_commit_all/push_all/_push_one/_recover_clean_ahead/resolve_git_dir file:line pointer)
33. Code-graph recon104integratedRestructure target (hotspot/local-blast-radius/regression window), Verification -> Regression scope, Graph-facet caveat
3D3D. Diagnostic / observability surface20integratedVerification -> Falsifier (a) (real subprocess-kill via python -m scripts.commit_push run / _run_self_test extension / test_process_identity idiom)
3H3H. Hygiene constraints19integratedDecision rules (exit-path exhaustiveness, comment hygiene), Reuse contract (LEAK:algorithmic-duplication), Preserve-constraints, Close-the-class follow-on, Verification (negative pins, L12, hygiene scope)
44. Cluster / family — sibling sections + related bugs28integratedRead-first order (BUG-07-532 + S2), Preserve-constraints (drain_on_entry S1/S4/S5, BUG-07-532 don’t-delete), Close-the-class follow-on
55. Conformance audit — MISSING_ABSTRACTION (graph-computed duplication, ori-platform)14integratedReuse contract (LEAK risk is prospective, reuse _push_one), Graph-facet caveat (MISSING_ABSTRACTION zero-row meaning)
66. Plan ownership — who owns this code now11integratedPreserve-constraints + Close-the-class follow-on (BUG-07-532 owns the recovery machinery; S3 owns the ordering), Graph-facet caveat (symbol-plans empty)
77. Prior art — solved elsewhere19integratedReuse contract (in-house recombination: state.json shape, .commit-push-apply.lock location, _push_one, SIGKILL idiom), Verification -> Falsifier (a)
88. Sentiment — community heat5not_applicableSentiment corpus is scoped to public reference-language issue trackers; scripts/commit_push/ is internal AI-seam wrapper tooling with no public tracker and structurally zero community-heat signal — it imposes no constraint on this section.
99. Declared coverage gaps15integratedGraph-facet caveat (insights/co_change/test_topology/sibling_divergence stale; zero-row vs caveated-empty distinction; symbol-plans MENTIONS_CODE gotcha)
1010. Recommended recon entry points (for /continue-roadmap execution)20integratedRead-first order + Reuse contract (recon entry points folded into the section’s own file:line decision pointers), Verification (test_process_identity + _run_self_test entry points)