diff --git a/operating-contract.md b/operating-contract.md index 93b64dc..109b50a 100644 --- a/operating-contract.md +++ b/operating-contract.md @@ -101,7 +101,6 @@ also hard-blocks the harness push tools. ## Committing -- `pm-commit ': '` is the commit helper for `/data` (a - guarded add + commit). The runner also commits a pre-run safety snapshot of - `/data` before you start, so "git is the undo" holds even before your first - own commit. +- Record `/data`'s revision before each change, then use + `pm-commit --from ': ' -- `. + Exact-path commits keep unrelated owner and agent work out of your undo unit. diff --git a/reflection_runner.py b/reflection_runner.py index 1c010a1..9ad453a 100644 --- a/reflection_runner.py +++ b/reflection_runner.py @@ -72,7 +72,6 @@ import logging import os import signal -import subprocess import sys import threading from pathlib import Path @@ -113,8 +112,6 @@ CLAUDE_CONFIG_DIR = DATA_DIR / "cli-auth" / "claude" CODEX_HOME = DATA_DIR / "cli-auth" / "codex" CLI_PATH = "/usr/local/bin/claude" -# The denylist-guarded `git add -A && git commit` helper baked into the image. -PM_COMMIT = "/app/scripts/pm-commit" # The app-owned operating contract appended to the system prompt each run. # Resolved BESIDE this runner (not under a fixed /data path) so it works in # both of the runner's homes — the platform's backend/scripts tree and the @@ -406,39 +403,6 @@ def write_static_timeout_brief(brief_path: Path) -> bool: ) -def _safety_snapshot(label: str) -> None: - """Best-effort git snapshot of /data BEFORE Reflection mutates anything. - - The nightly run rewrites skills, fixes apps, and writes reports — edits to - agent-owned files under /data that the "git is the undo" contract promises are - recoverable. Until now that promise rested entirely on the agent's own - `pm-commit` discipline MID-run, so an early edit before the first commit had - no pre-state restore point beyond LAST night's. Committing the current tree as - the very first thing the run does guarantees one. - - `--allow-broad` so a full day's accumulated changes aren't refused by - pm-commit's 50-file guard; a no-op (nothing changed) exits 0. Any failure is - logged and swallowed — a snapshot must NEVER block the night's run. - """ - try: - proc = subprocess.run( - [PM_COMMIT, "--allow-broad", label], - cwd=str(DATA_DIR), - capture_output=True, - text=True, - timeout=120, - ) - if proc.returncode == 0: - _log("pre-run safety snapshot committed (or no-op)") - else: - _log( - f"WARN pre-run snapshot rc={proc.returncode}: " - f"{(proc.stderr or '').strip()[:200]}" - ) - except Exception as exc: - _log(f"WARN pre-run snapshot failed: {exc!r}") - - def _log(message: str) -> None: """Appends one timestamped line to the reflection log. @@ -1520,12 +1484,6 @@ async def run() -> int: f"effort={effort or '(default)'} cwd={DATA_DIR}" ) - # Guaranteed pre-run restore point: commit /data BEFORE the agent rewrites - # skills or apps, so "git is the undo" holds even if tonight's run edits a - # file before its own first pm-commit. Best-effort; never blocks. - from datetime import date - _safety_snapshot(f"reflection: pre-run safety snapshot {date.today().isoformat()}") - try: rc = await _run_agent_choice( primary, goal=goal, system_prompt=system_prompt, env=env, diff --git a/tests/canonical-outcomes.test.mjs b/tests/canonical-outcomes.test.mjs index dadf3e9..fe72044 100644 --- a/tests/canonical-outcomes.test.mjs +++ b/tests/canonical-outcomes.test.mjs @@ -3,6 +3,10 @@ import fs from 'node:fs' import test from 'node:test' const fetchSource = fs.readFileSync(new URL('../fetch.sh', import.meta.url), 'utf8') +const runnerSource = fs.readFileSync( + new URL('../reflection_runner.py', import.meta.url), + 'utf8', +) const statusSource = fs.readFileSync( new URL('../ui/LastNightStatus.jsx', import.meta.url), 'utf8', @@ -13,8 +17,10 @@ test('the platform supervisor is the only cron outcome writer', () => { assert.doesNotMatch(fetchSource, /api\/admin\/activity\/emit/) }) -test('the wrapper never sweeps unattended changes into a broad safety-net commit', () => { +test('Reflection never sweeps unattended changes into a broad safety-net commit', () => { assert.doesNotMatch(fetchSource, /pm-commit\s+--allow-broad/) + assert.doesNotMatch(runnerSource, /pm-commit[\s\S]*--allow-broad/) + assert.doesNotMatch(runnerSource, /_safety_snapshot/) assert.doesNotMatch(fetchSource, /nightly safety-net commit/) }) diff --git a/tests/fetch-inputs.test.mjs b/tests/fetch-inputs.test.mjs index bea6555..9677506 100644 --- a/tests/fetch-inputs.test.mjs +++ b/tests/fetch-inputs.test.mjs @@ -315,10 +315,6 @@ async def run_codex_sdk_turn(**kwargs): try { await run({ REFLECTION_DRY: '0', - // The real runner takes a best-effort pre-run safety snapshot before - // it starts the provider. Leave enough headroom for that unrelated - // host work so this test deterministically reaches the fake provider - // and exercises cancellation of its in-flight tool. REFLECTION_TIMEOUT: '3', REFLECTION_LOG_MAX_BYTES: '1048576', REFLECTION_RUNNER: fakeRunner, diff --git a/tests/test_reflection_runner.py b/tests/test_reflection_runner.py index 2b2e730..58496e9 100644 --- a/tests/test_reflection_runner.py +++ b/tests/test_reflection_runner.py @@ -311,7 +311,6 @@ async def test_generic_primary_failure_tries_the_distinct_configured_fallback(se mock.patch.object(reflection_runner, "seed_brief_template"), mock.patch.object(reflection_runner, "build_goal", return_value="goal"), mock.patch.object(reflection_runner, "build_env", return_value={}), - mock.patch.object(reflection_runner, "_safety_snapshot"), mock.patch.object(reflection_runner, "_log"), mock.patch.object(reflection_runner, "todays_brief_path", return_value=Path(raw) / "missing.html"), mock.patch.object(reflection_runner, "_run_agent_choice", runner),