diff --git a/.codearbiter/tech-stack.md b/.codearbiter/tech-stack.md index 7cec8964..5e905932 100644 --- a/.codearbiter/tech-stack.md +++ b/.codearbiter/tech-stack.md @@ -74,6 +74,10 @@ python .github/scripts/test_readinjectlib.py # pre-read hook entry point — governed-file injection, dedup, fail-open, dormancy # (pre-read.py): AC-03/09/10/12, miss, and dormant-repo paths python .github/scripts/test_pre_read.py + +# plugins/ca/hooks/tests/ — the hook enforcement + helper logic unittest suite +# (statusline, _ledgerlib, guards, etc.); run in CI via unittest discover +python -m unittest discover -s plugins/ca/hooks/tests -p "test_*.py" ``` Only when `plugins/ca/tools/**` changed: diff --git a/.github/scripts/test_provenancelib.py b/.github/scripts/test_provenancelib.py index c1352d47..aa576cdd 100644 --- a/.github/scripts/test_provenancelib.py +++ b/.github/scripts/test_provenancelib.py @@ -74,6 +74,57 @@ def test_round_trip(self): ) +class WriteProvenanceAtomicTest(unittest.TestCase): + """reliability-016: write_provenance is routed through the temp-file + + os.replace atomic-write pattern, so a crash mid-write never truncates or + corrupts a previously-written provenance record.""" + + def test_crash_mid_write_leaves_previous_record_intact(self): + from unittest.mock import patch + + record_v1 = pl.new_record("tech-stack", created="2026-06-26") + record_v2 = pl.new_record("tech-stack", created="2026-07-01") + + with tempfile.TemporaryDirectory() as tmp: + path = os.path.join(tmp, ".codearbiter", ".provenance", "tech-stack.json") + pl.write_provenance(path, record_v1) + self.assertEqual(pl.read_provenance(path), record_v1) + + # Simulate a crash between the temp-file write and the atomic + # rename (the exact window write_text_atomic exists to close). + with patch("os.replace", side_effect=OSError("simulated crash")): + with self.assertRaises(OSError): + pl.write_provenance(path, record_v2) + + # The destination must still hold the PRIOR, complete record — + # never truncated, never partially overwritten. + self.assertEqual(pl.read_provenance(path), record_v1) + + # No stray .tmp file should linger in the provenance directory. + leftovers = [ + n for n in os.listdir(os.path.dirname(path)) if n != "tech-stack.json" + ] + self.assertEqual(leftovers, [], f"no temp file should linger: {leftovers}") + + def test_crash_during_json_serialization_leaves_previous_record_intact(self): + """A crash while dumping JSON (before the atomic write even starts) + must also never touch the previously-written file.""" + from unittest.mock import patch + + record_v1 = pl.new_record("tech-stack", created="2026-06-26") + + with tempfile.TemporaryDirectory() as tmp: + path = os.path.join(tmp, ".codearbiter", ".provenance", "tech-stack.json") + pl.write_provenance(path, record_v1) + self.assertEqual(pl.read_provenance(path), record_v1) + + with patch("json.dumps", side_effect=ValueError("simulated crash")): + with self.assertRaises(ValueError): + pl.write_provenance(path, pl.new_record("tech-stack", created="2026-07-01")) + + self.assertEqual(pl.read_provenance(path), record_v1) + + class BatchHashTest(unittest.TestCase): """AC-02: batch_hash issues a single git hash-object --stdin-paths call, order-preserving.""" diff --git a/.github/scripts/test_taskwriter.py b/.github/scripts/test_taskwriter.py index 0053fd36..80310a40 100644 --- a/.github/scripts/test_taskwriter.py +++ b/.github/scripts/test_taskwriter.py @@ -355,6 +355,26 @@ def _failing_replace(src, dst): self.assertEqual(f.read(), original_content, "board was truncated/corrupted by an interrupted write") + def test_missing_board_returns_1_no_file_created(self): + """coverage-004: an uninitialized repo (no .codearbiter/open-tasks.md) + must exit 1 with the 'no board at' stderr message and must not create + any file as a side effect.""" + import io + import tempfile + import contextlib + import taskwrite + + d = tempfile.mkdtemp() # deliberately no .codearbiter/ dir at all + taskwrite.project_root = lambda: d + + stderr = io.StringIO() + with contextlib.redirect_stderr(stderr): + rc = taskwrite.main(["add", "x"]) + + self.assertEqual(rc, 1) + self.assertIn("no board at", stderr.getvalue()) + self.assertEqual(os.listdir(d), [], "no file should be created for an uninitialized repo") + if __name__ == "__main__": unittest.main() diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1946e958..c11b1895 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -314,12 +314,15 @@ jobs: ver=$(node -p "JSON.parse(require('fs').readFileSync('plugins/ca-sandbox/.claude-plugin/plugin.json','utf8')).version") # First introduction of the plugin: plugin.json does not exist on base. # That is never a silent-staleness trap (nothing is published yet), so - # allow it - without this, `git show` errors and `bash -e` kills the step. - base_json=$(git show "origin/$BASE:plugins/ca-sandbox/.claude-plugin/plugin.json" 2>/dev/null || true) - if [ -z "$base_json" ]; then + # allow it - but only when the file is genuinely absent at $BASE. Any + # other git failure (transient fetch error, corrupted object store, + # a race on origin/$BASE) must fail the step, not be swallowed as + # "first introduction". + if ! git cat-file -e "origin/$BASE:plugins/ca-sandbox/.claude-plugin/plugin.json" 2>/dev/null; then echo "ca-sandbox is new on $BASE (no prior plugin.json) - first introduction, version bump not required" exit 0 fi + base_json=$(git show "origin/$BASE:plugins/ca-sandbox/.claude-plugin/plugin.json") base_ver=$(printf '%s' "$base_json" | node -p "JSON.parse(require('fs').readFileSync(0,'utf8')).version") if [ "$ver" != "$base_ver" ]; then echo "payload changed and version bumped: $base_ver -> $ver" diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 8d65b3b3..26c18413 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -36,10 +36,12 @@ jobs: - name: Resolve and verify version against the manifest id: v + env: + CONFIRM: ${{ github.event.inputs.confirm }} run: | set -euo pipefail MANIFEST=$(node -p "require('./plugins/ca/.claude-plugin/plugin.json').version") - REQ="${{ github.event.inputs.confirm }}" + REQ="$CONFIRM" if [ "$REQ" != "$MANIFEST" ]; then echo "::error::requested '$REQ' does not equal plugin.json version '$MANIFEST' — bump/align first" exit 1 @@ -67,11 +69,12 @@ jobs: - name: Create the tag and GitHub Release (idempotent) env: GH_TOKEN: ${{ github.token }} + SUMMARY: ${{ github.event.inputs.summary }} run: | set -euo pipefail TAG="${{ steps.v.outputs.tag }}" VER="${{ steps.v.outputs.version }}" - SUM="${{ github.event.inputs.summary }}" + SUM="$SUMMARY" if [ -n "$SUM" ]; then TITLE="codeArbiter $VER: $SUM"; else TITLE="codeArbiter $VER"; fi # An annotated tag needs a committer identity; the runner has none by default. diff --git a/.gitignore b/.gitignore index 5c5fd53d..a803ef9c 100644 --- a/.gitignore +++ b/.gitignore @@ -10,6 +10,11 @@ legacy/ # never shared. Each developer's own .claude/settings.json wires the plugin hooks. .claude/settings.json +# Local Claude Code overrides (may carry machine-local secrets like FARM_API_KEY +# injected via `env`). Repo-tracked guardrail so the exclusion travels with the +# repo instead of relying on each developer's global gitignore. +.claude/settings.local.json + # Transient ADR-authoring marker under the root-level state dir (set by /ca:adr). # Not project state. .codearbiter/.markers/ diff --git a/README.md b/README.md index 617be9d7..5d30f767 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ Every intent routes through a gated skill or reviewer agent. Nothing commits until the gates are green. Decisions go through SMARTS. The audit trail is append-only. Claude Code plugin -version 2.8.0 +version 2.8.1 commands skills agents diff --git a/plugins/ca-sandbox/.claude-plugin/plugin.json b/plugins/ca-sandbox/.claude-plugin/plugin.json index e501fa3d..77250f32 100644 --- a/plugins/ca-sandbox/.claude-plugin/plugin.json +++ b/plugins/ca-sandbox/.claude-plugin/plugin.json @@ -2,7 +2,7 @@ "name": "ca-sandbox", "displayName": "codeArbiter Sandbox", "description": "Locally-hosted Codespace equivalent for codeArbiter. Pulls an untrusted repo into an ephemeral, isolated Docker container with no host-filesystem access and configurable egress, caches dependencies by content hash, then tears the box down. Requires Docker and nixpacks on PATH.", - "version": "0.1.1", + "version": "0.1.2", "author": { "name": "arbiterForge" }, "license": "AGPL-3.0-only", "homepage": "https://github.com/arbiterForge/codeArbiter", diff --git a/plugins/ca-sandbox/tools/build.ts b/plugins/ca-sandbox/tools/build.ts index 56677b22..6db4f4a5 100644 --- a/plugins/ca-sandbox/tools/build.ts +++ b/plugins/ca-sandbox/tools/build.ts @@ -34,7 +34,7 @@ * cache/tag/relocation logic is unit-testable without real docker, while the * docker-gated test drives the real defaults. */ -import { spawn } from "node:child_process"; +import { spawn, type SpawnOptionsWithoutStdio } from "node:child_process"; import { mkdtemp, writeFile, readFile, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; @@ -66,7 +66,13 @@ const DOCKER_ENV = { ...process.env, MSYS_NO_PATHCONV: "1" }; // explicitly so the build is robust regardless of the daemon default. const BUILD_ENV = { ...DOCKER_ENV, DOCKER_BUILDKIT: "1" }; -function run(cmd: string, args: string[], opts: object = {}): Promise { +// `opts.env`, when supplied, is an explicit typed override (e.g. BUILD_ENV +// below) rather than an unconstrained spread — a future opts literal is now +// compiler-checked to stay a well-formed env override (must preserve +// MSYS_NO_PATHCONV explicitly, not drop it by accident). +type RunOpts = { env?: NodeJS.ProcessEnv } & Omit; + +function run(cmd: string, args: string[], opts: RunOpts = {}): Promise { return new Promise((resolve) => { const c = spawn(cmd, args, { env: DOCKER_ENV, ...opts }); let stdout = ""; diff --git a/plugins/ca/.claude-plugin/plugin.json b/plugins/ca/.claude-plugin/plugin.json index 0b343762..29cae898 100644 --- a/plugins/ca/.claude-plugin/plugin.json +++ b/plugins/ca/.claude-plugin/plugin.json @@ -2,7 +2,7 @@ "name": "ca", "displayName": "codeArbiter", "description": "Orchestration layer for Claude Code. Routes every intent through gated skills and reviewer agents, drives spec-driven TDD, mechanically enforces the commit and audit-trail gates, decides via SMARTS, and keeps an append-only audit trail. Requires Python 3 on PATH. Dormant until you opt a repo in; run /ca:init to activate.", - "version": "2.8.0", + "version": "2.8.1", "author": { "name": "arbiterForge" }, "license": "AGPL-3.0-only", "homepage": "https://github.com/arbiterForge/codeArbiter", diff --git a/plugins/ca/hooks/_hooklib.py b/plugins/ca/hooks/_hooklib.py index a5eec40a..da0b16bf 100755 --- a/plugins/ca/hooks/_hooklib.py +++ b/plugins/ca/hooks/_hooklib.py @@ -563,18 +563,22 @@ def is_deploy_path(rel, root): return path_in_globs(rel, root, DEPLOY_DEFAULT_GLOBS, _DEPLOY_DECL_RE) -def write_text_atomic(path, text): +def write_text_atomic(path, text, newline=None): """Write `text` to `path` atomically: a sibling temp file in the same dir, then os.replace() into place (atomic on POSIX; a same-volume rename on Windows). A crash between open() and the rename never leaves a half-written file at `path`. The gate-marker writers (migration-pass / security-pass) rely on this so a partial digest set can't be read back as an unrecognized token and force a spurious gate re-run (migration-002). On any failure the temp - file is cleaned up and the original `path` is left untouched.""" + file is cleaned up and the original `path` is left untouched. + + `newline` is passed through to open()/fdopen() unchanged (default None keeps + the prior text-mode translation behaviour for existing callers); pass "\\n" + to force LF output regardless of platform (e.g. for a canonical-EOL file).""" d = os.path.dirname(path) or "." fd, tmp = tempfile.mkstemp(dir=d, prefix=os.path.basename(path) + ".", suffix=".tmp") try: - with os.fdopen(fd, "w", encoding="utf-8") as f: + with os.fdopen(fd, "w", encoding="utf-8", newline=newline) as f: f.write(text) os.replace(tmp, path) except Exception: diff --git a/plugins/ca/hooks/_provenancelib.py b/plugins/ca/hooks/_provenancelib.py index ee28eaba..10476471 100644 --- a/plugins/ca/hooks/_provenancelib.py +++ b/plugins/ca/hooks/_provenancelib.py @@ -91,6 +91,8 @@ import re import subprocess +import _hooklib + # --------------------------------------------------------------------------- # Constants # --------------------------------------------------------------------------- @@ -206,18 +208,19 @@ def new_record(doc, *, interview_derived=False, entries=None, created=None): def write_provenance(path, record): - """Write `record` as pretty JSON to `path`. + """Write `record` as pretty JSON to `path`, atomically. Format: indent=2, sorted keys, ensure_ascii=False, trailing newline, utf-8, LF line endings (canonical EOL for this repo). Creates the parent directory - if it does not exist. + if it does not exist. Routed through _hooklib.write_text_atomic (sibling + temp file + os.replace) so a crash mid-write leaves the previous provenance + record intact instead of a truncated/corrupt file (reliability-016). """ parent = os.path.dirname(path) if parent: os.makedirs(parent, exist_ok=True) - with open(path, "w", encoding="utf-8", newline="\n") as f: - json.dump(record, f, indent=2, sort_keys=True, ensure_ascii=False) - f.write("\n") + text = json.dumps(record, indent=2, sort_keys=True, ensure_ascii=False) + "\n" + _hooklib.write_text_atomic(path, text, newline="\n") def read_provenance(path): diff --git a/plugins/ca/hooks/statusline.py b/plugins/ca/hooks/statusline.py index c79e90df..756d8a3b 100755 --- a/plugins/ca/hooks/statusline.py +++ b/plugins/ca/hooks/statusline.py @@ -77,13 +77,7 @@ import _ledgerlib ledger_update = _ledgerlib.ledger_update _tx_accumulate = _ledgerlib._tx_accumulate - _agg_reqs = _ledgerlib._agg_reqs - _totals = _ledgerlib._totals - api_cost = _ledgerlib.api_cost - price_for = _ledgerlib.price_for - ledger_path = _ledgerlib.ledger_path persist_sess_start = _ledgerlib.persist_sess_start - API_PRICES = _ledgerlib.API_PRICES except Exception: # pragma: no cover — never let an import break the statusline _ledgerlib = None @@ -490,11 +484,11 @@ def _arbiter_state_uncached(cad): # --------------------------------------------------------------------------- ledger # The token/cost ledger (pricing table, transcript accumulation, JSON persistence, -# burn samples) lives in _ledgerlib (extracted T-12). Its functions — ledger_update, -# _tx_accumulate, _agg_reqs, _totals, api_cost, price_for, ledger_path, API_PRICES — -# are bound into this module at import time (see the guarded import at the top), so -# the cost segment and the test suite reach them unchanged. Only burn_spark stays -# here: it is the render bridge that turns the lib's numeric samples into a colored +# burn samples) lives in _ledgerlib (extracted T-12). Its functions actually used +# from this module — ledger_update, _tx_accumulate, persist_sess_start — are bound +# into this module at import time (see the guarded import at the top), so the cost +# segment and the test suite reach them unchanged. Only burn_spark stays here: it +# is the render bridge that turns the lib's numeric samples into a colored # sparkline, so it keeps the ANSI dependency out of the lib. def burn_spark(rec): """Sparkline of recent per-message token burn — real per-API-call totals diff --git a/plugins/ca/hooks/tests/test_statusline.py b/plugins/ca/hooks/tests/test_statusline.py index 1bb657e1..5c81cef8 100644 --- a/plugins/ca/hooks/tests/test_statusline.py +++ b/plugins/ca/hooks/tests/test_statusline.py @@ -604,9 +604,10 @@ def test_render_is_deterministic_for_same_input(self): def test_ledger_functions_reachable_via_statusline(self): # The unmodified test suite reaches these via sl.*; the extraction must keep - # them bound on the statusline module. - for name in ("ledger_update", "_tx_accumulate", "_agg_reqs", "_totals", - "api_cost", "price_for", "ledger_path", "API_PRICES"): + # them bound on the statusline module. Only the names this module (and its + # test suite) actually use are re-bound — the rest were dead re-binds + # removed as part of architecture-002. + for name in ("ledger_update", "_tx_accumulate", "persist_sess_start"): self.assertTrue(hasattr(sl, name), f"sl.{name} missing after extraction") def test_burn_spark_returns_string(self): diff --git a/plugins/ca/includes/routing-table.md b/plugins/ca/includes/routing-table.md index ca1c763a..0f406edd 100644 --- a/plugins/ca/includes/routing-table.md +++ b/plugins/ca/includes/routing-table.md @@ -11,7 +11,7 @@ command is **invoked**; the orchestrator **routes** to a skill; a skill **dispat | Autonomous sprint | `/sprint` → `SPRINT.md` (brainstorm → plan → `subagent-driven-development`) | per-task impl authors + reviewers | One interactive spec gate; hard gates never auto-decided; every auto-decision logged | | Bug fix | `/fix` → `tdd` (bug variant) | impl author | Failing regression test before any fix code | | Docs edit / dep bump / revert | `/chore` (type-scaled gates) | `dependency-reviewer` for deps | No behavioral code; suite green for deps/revert; exits via `commit-gate` | -| Exploratory throwaway spike | `/spike` → `spike` skill → `spike/` branch | — | Never merges or PRs; `commit-gate`-exempt (nothing on the branch can land); exits to a findings note or `/feature` | +| Exploratory throwaway spike | `/spike` (self-contained command) → `spike/` branch | — | Never merges or PRs; `commit-gate`-exempt (nothing on the branch can land); exits to a findings note or `/feature` | | Behavior-preserving restructure | `/refactor` → `refactor` skill | `tdd` Phase 1 (new seams only) | No refactor without parity-coverage proof | | Unknown defect / investigation | `/debug` → `debug` skill | — | No code change in the skill; one named exit | | Commit | `/commit` → `commit-gate` | — | No commit without all nine gates green | diff --git a/plugins/ca/tools/exec.ts b/plugins/ca/tools/exec.ts index fed8b67f..07cda177 100644 --- a/plugins/ca/tools/exec.ts +++ b/plugins/ca/tools/exec.ts @@ -10,7 +10,7 @@ * mutation.ts -> exec.ts, never back). This is a move, not a rewrite: behaviour * is identical to the prior in-farm.ts definitions. */ -import { spawn } from "node:child_process"; +import { spawn, type SpawnOptionsWithoutStdio } from "node:child_process"; import { readFile } from "node:fs/promises"; import path from "node:path"; @@ -73,6 +73,13 @@ export function scrubbedEnv(extra?: NodeJS.ProcessEnv): NodeJS.ProcessEnv { return env; } +// `opts` excludes `env`, `cwd`, and `shell` so a caller can never re-introduce +// a raw env (and thus silently override scrubbedEnv()'s CodeQL #5 scrub), +// shadow the explicit `cwd` param, or opt into shell interpolation through the +// spread below — the compiler now enforces the single-scrub-path and +// argv-array invariants the header comment describes. +type RunOpts = Omit; + // `timeoutMs` (opts) bounds the child's wall-clock; 0/undefined disables the // timeout (used by git, which must not be killed mid-operation). On timeout the // child tree is killed and a RunResult tagged `timedOut` resolves, so the caller @@ -81,7 +88,7 @@ export function run( cmd: string, args: string[], cwd?: string, - opts: object = {}, + opts: RunOpts = {}, timeoutMs = 0, ): Promise { return new Promise((resolve) => {