From 279f9ec563ae6fbc38d6ca203c77922e213d1d8c Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Sun, 12 Apr 2026 01:50:23 +0200 Subject: [PATCH] Make protected-main doctor completion actually clear stale-lock warnings Sandbox doctor runs repaired lock state in a worktree, but base main kept stale lock entries and stayed degraded. This change syncs the repaired lock registry back to the protected workspace and tightens completion language from 'musafe' to 'safe'. Constraint: Protected main must stay read-only for direct maintenance writes Rejected: Force in-place doctor writes on main | violates protected-base workflow Confidence: high Scope-risk: moderate Reversibility: clean Directive: Keep doctor JSON output machine-readable when adding new completion metadata Tested: node --check bin/multiagent-safety.js; node --test test/metadata.test.js; manual doctor->scan on /home/deadpool/Documents/multiagent-safety Not-tested: full npm test (existing file-level failures in test/install.test.js and test/fuzzing.test.js) --- AGENTS.md | 9 +++- README.md | 1 + bin/multiagent-safety.js | 74 +++++++++++++++++++++++++-- templates/AGENTS.multiagent-safety.md | 1 + test/install.test.js | 55 +++++++++++++++++++- 5 files changed, 131 insertions(+), 9 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 641def9..3386bee 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -92,9 +92,14 @@ OMX runtime state typically lives under `.omx/`: - For git isolation, each agent must start on a dedicated branch via `scripts/agent-branch-start.sh "" ""`. - Do not implement changes directly on `main` or other base branches; all edits must happen on dedicated agent branches/worktrees. - If the current local branch already contains accidental edits, move them to an agent branch/worktree first, then continue implementation. -- Agent completion defaults to `scripts/codex-agent.sh`, which auto-finishes the branch (auto-commit changed files, push/create PR, attempt merge, clean branch/worktree, and pull the local base branch after merge). -- If codex-agent auto-finish cannot complete, run `scripts/agent-branch-finish.sh --branch "" --via-pr` and keep the branch open until checks/review pass. +- Treat the base branch (`main` or the user's current local base branch) as read-only while the agent branch is active. +- Agent completion defaults to `scripts/codex-agent.sh`, which auto-finishes the branch (auto-commit changed files, push/create PR, attempt merge, and pull the local base branch after merge). +- Auto-finish keeps the sandbox branch/worktree by default so conflict follow-ups and audits stay reproducible. +- Use explicit cleanup when done: `gx cleanup --branch ""` (or `gx cleanup` for all merged agent branches). +- If codex-agent auto-finish cannot complete, immediately run `scripts/agent-branch-finish.sh --branch "" --via-pr` and keep the branch open until checks/review pass. - If merge/rebase conflicts block auto-finish, run a conflict-resolution review pass in that sandbox branch, then rerun `agent-branch-finish.sh --via-pr` until merged. +- Completion is not valid until these are true: commit exists on the agent branch, branch is pushed to `origin`, and PR/merge status is produced by `agent-branch-finish.sh` or `codex-agent`. +- Per-message loop is mandatory: for every new user message/task, start a fresh agent branch/worktree, claim ownership locks, implement and verify, finish via PR/merge cleanup, then repeat for the next message/task. 1. Explicit ownership before edits diff --git a/README.md b/README.md index eea933d..f169fe0 100644 --- a/README.md +++ b/README.md @@ -292,6 +292,7 @@ and asks `[y/N]` whether to update immediately (default is `N`). - Non-interactive setup: skips global installs by default; use `--yes-global-install` to force. - In already-initialized repos, `setup` / `install` / `fix` block writes on protected `main` by default; start an agent branch first. Use `--allow-protected-base-write` only for emergency in-place maintenance. - `gx doctor` on protected `main` auto-starts an isolated `agent/gx/...-gx-doctor` worktree branch and applies repairs there. + It also syncs repaired `.omx/state/agent-file-locks.json` back to your protected workspace so stale-lock warnings clear immediately. - `gx setup` and `gx doctor` always refresh `.githooks/pre-commit` from templates, so Codex sub-branch enforcement stays repaired. - `scripts/codex-agent.sh` now auto-runs finish automation after a Codex session when `origin` exists: auto-commit changed files, run PR/merge automation, and keep merged agent branches/worktrees by default. diff --git a/bin/multiagent-safety.js b/bin/multiagent-safety.js index 37ab219..5f2a234 100755 --- a/bin/multiagent-safety.js +++ b/bin/multiagent-safety.js @@ -811,8 +811,59 @@ function runDoctorInSandbox(options, blocked) { throw nestedResult.error; } + let lockSyncResult = { + status: 'skipped', + note: 'sandbox doctor did not complete successfully', + }; + if (nestedResult.status === 0) { + const sandboxLockPath = path.join(metadata.worktreePath, LOCK_FILE_RELATIVE); + const baseLockPath = path.join(blocked.repoRoot, LOCK_FILE_RELATIVE); + if (!fs.existsSync(sandboxLockPath)) { + lockSyncResult = { + status: 'skipped', + note: `${LOCK_FILE_RELATIVE} missing in sandbox worktree`, + }; + } else { + const sourceContent = fs.readFileSync(sandboxLockPath, 'utf8'); + const destinationContent = fs.existsSync(baseLockPath) ? fs.readFileSync(baseLockPath, 'utf8') : ''; + if (sourceContent === destinationContent) { + lockSyncResult = { + status: 'unchanged', + note: `${LOCK_FILE_RELATIVE} already in sync`, + }; + } else { + fs.mkdirSync(path.dirname(baseLockPath), { recursive: true }); + fs.writeFileSync(baseLockPath, sourceContent, 'utf8'); + lockSyncResult = { + status: 'synced', + note: `${LOCK_FILE_RELATIVE} synced from sandbox`, + }; + } + } + } + if (options.json) { - if (nestedResult.stdout) process.stdout.write(nestedResult.stdout); + if (nestedResult.stdout) { + if (nestedResult.status === 0) { + try { + const parsed = JSON.parse(nestedResult.stdout); + process.stdout.write( + JSON.stringify( + { + ...parsed, + sandboxLockSync: lockSyncResult, + }, + null, + 2, + ) + '\n', + ); + } catch { + process.stdout.write(nestedResult.stdout); + } + } else { + process.stdout.write(nestedResult.stdout); + } + } if (nestedResult.stderr) process.stderr.write(nestedResult.stderr); } else { console.log( @@ -823,6 +874,17 @@ function runDoctorInSandbox(options, blocked) { if (startResult.stderr) process.stderr.write(startResult.stderr); if (nestedResult.stdout) process.stdout.write(nestedResult.stdout); if (nestedResult.stderr) process.stderr.write(nestedResult.stderr); + if (nestedResult.status === 0) { + if (lockSyncResult.status === 'synced') { + console.log( + `[${TOOL_NAME}] Synced repaired lock registry back to protected branch workspace (${LOCK_FILE_RELATIVE}).`, + ); + } else if (lockSyncResult.status === 'unchanged') { + console.log(`[${TOOL_NAME}] Lock registry already synced in protected branch workspace.`); + } else { + console.log(`[${TOOL_NAME}] Lock registry sync skipped: ${lockSyncResult.note}.`); + } + } } if (typeof nestedResult.status === 'number') { @@ -2093,7 +2155,8 @@ function doctor(rawArgs) { assertProtectedMainWriteAllowed(options, 'doctor'); const fixPayload = runFixInternal(options); const scanResult = runScanInternal({ target: options.target, json: false }); - const musafe = scanResult.errors === 0 && scanResult.warnings === 0; + const safe = scanResult.errors === 0 && scanResult.warnings === 0; + const musafe = safe; if (options.json) { process.stdout.write( @@ -2101,6 +2164,7 @@ function doctor(rawArgs) { { repoRoot: scanResult.repoRoot, branch: scanResult.branch, + safe, musafe, fix: { operations: fixPayload.operations, @@ -2123,11 +2187,11 @@ function doctor(rawArgs) { printOperations('Doctor/fix', fixPayload, options.dryRun); printScanResult(scanResult, false); - if (musafe) { - console.log(`[${TOOL_NAME}] ✅ Repo is correctly musafe.`); + if (safe) { + console.log(`[${TOOL_NAME}] ✅ Repo is fully safe.`); } else { console.log( - `[${TOOL_NAME}] ⚠️ Repo is not fully musafe yet (${scanResult.errors} error(s), ${scanResult.warnings} warning(s)).`, + `[${TOOL_NAME}] ⚠️ Repo is not fully safe yet (${scanResult.errors} error(s), ${scanResult.warnings} warning(s)).`, ); } setExitCodeFromScan(scanResult); diff --git a/templates/AGENTS.multiagent-safety.md b/templates/AGENTS.multiagent-safety.md index 7e79035..02715e9 100644 --- a/templates/AGENTS.multiagent-safety.md +++ b/templates/AGENTS.multiagent-safety.md @@ -16,6 +16,7 @@ - Use explicit cleanup when done: `gx cleanup --branch ""` (or `gx cleanup` for all merged agent branches). - If codex-agent auto-finish cannot complete, immediately run `scripts/agent-branch-finish.sh --branch "" --via-pr` and keep the branch open until checks/review pass. - If merge/rebase conflicts block auto-finish, run a conflict-resolution review pass in that sandbox branch, then rerun `agent-branch-finish.sh --via-pr` until merged. +- Completion is not valid until these are true: commit exists on the agent branch, branch is pushed to `origin`, and PR/merge status is produced by `agent-branch-finish.sh` or `codex-agent`. - Per-message loop is mandatory: for every new user message/task, start a fresh agent branch/worktree, claim ownership locks, implement and verify, finish via PR/merge cleanup, then repeat for the next message/task. 1. Explicit ownership before edits diff --git a/test/install.test.js b/test/install.test.js index fe8c57f..759918d 100644 --- a/test/install.test.js +++ b/test/install.test.js @@ -325,6 +325,57 @@ test('doctor on protected main auto-runs in a sandbox branch/worktree', () => { assert.equal(currentBranch.stdout.trim(), 'main'); }); +test('doctor on protected main syncs repaired stale lock state back to base workspace', () => { + const repoDir = initRepoOnBranch('main'); + seedCommit(repoDir); + attachOriginRemoteForBranch(repoDir, 'main'); + + let result = runNode(['setup', '--target', repoDir, '--no-global-install'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + + result = runCmd('git', ['add', '.'], repoDir); + assert.equal(result.status, 0, result.stderr); + result = runCmd('git', ['commit', '-m', 'apply gx setup'], repoDir, { + ALLOW_COMMIT_ON_PROTECTED_BRANCH: '1', + }); + assert.equal(result.status, 0, result.stderr || result.stdout); + result = runCmd('git', ['push', 'origin', 'main'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + + const lockPath = path.join(repoDir, '.omx', 'state', 'agent-file-locks.json'); + fs.writeFileSync( + lockPath, + JSON.stringify( + { + locks: { + 'package.json': { + branch: 'agent/non-existent', + claimed_at: '2026-01-01T00:00:00Z', + allow_delete: false, + }, + }, + }, + null, + 2, + ) + '\n', + ); + + const scanBefore = runNode(['scan', '--target', repoDir], repoDir); + assert.equal(scanBefore.status, 1, scanBefore.stderr || scanBefore.stdout); + assert.match(scanBefore.stdout, /stale-branch-lock/); + + result = runNode(['doctor', '--target', repoDir], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + assert.match(result.stdout, /doctor detected protected branch 'main'/); + assert.match(result.stdout, /Synced repaired lock registry back to protected branch workspace/); + + const lockState = JSON.parse(fs.readFileSync(lockPath, 'utf8')); + assert.deepEqual(lockState.locks, {}); + + const scanAfter = runNode(['scan', '--target', repoDir], repoDir); + assert.equal(scanAfter.status, 0, scanAfter.stderr || scanAfter.stdout); +}); + test('setup pre-commit blocks codex session commits on non-agent branches by default', () => { const repoDir = initRepo(); @@ -1353,7 +1404,7 @@ test('fix repairs stale lock issues so scan becomes clean', () => { assert.equal(result.status, 0, result.stdout + result.stderr); }); -test('doctor repairs setup drift and confirms repo is musafe', () => { +test('doctor repairs setup drift and confirms repo is safe', () => { const repoDir = initRepo(); let result = runNode(['setup', '--target', repoDir], repoDir); @@ -1386,7 +1437,7 @@ test('doctor repairs setup drift and confirms repo is musafe', () => { result = runNode(['doctor', '--target', repoDir], repoDir); assert.equal(result.status, 0, result.stderr || result.stdout); assert.match(result.stdout, /Doctor\/fix/); - assert.match(result.stdout, /Repo is correctly musafe/); + assert.match(result.stdout, /Repo is fully safe/); const repairedHook = fs.readFileSync(path.join(repoDir, '.githooks', 'pre-commit'), 'utf8'); assert.match(repairedHook, /AGENTS\.md\|\.gitignore/);