Skip to content

Commit 56a9424

Browse files
NagyViktNagyVikt
andauthored
Make protected-main doctor completion actually clear stale-lock warnings (#41)
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) Co-authored-by: NagyVikt <nagy.viktordp@gmail.com>
1 parent 1140bf5 commit 56a9424

5 files changed

Lines changed: 131 additions & 9 deletions

File tree

AGENTS.md

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,14 @@ OMX runtime state typically lives under `.omx/`:
9292
- For git isolation, each agent must start on a dedicated branch via `scripts/agent-branch-start.sh "<task-or-plan>" "<agent-name>"`.
9393
- Do not implement changes directly on `main` or other base branches; all edits must happen on dedicated agent branches/worktrees.
9494
- If the current local branch already contains accidental edits, move them to an agent branch/worktree first, then continue implementation.
95-
- 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).
96-
- If codex-agent auto-finish cannot complete, run `scripts/agent-branch-finish.sh --branch "<agent-branch>" --via-pr` and keep the branch open until checks/review pass.
95+
- Treat the base branch (`main` or the user's current local base branch) as read-only while the agent branch is active.
96+
- 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).
97+
- Auto-finish keeps the sandbox branch/worktree by default so conflict follow-ups and audits stay reproducible.
98+
- Use explicit cleanup when done: `gx cleanup --branch "<agent-branch>"` (or `gx cleanup` for all merged agent branches).
99+
- If codex-agent auto-finish cannot complete, immediately run `scripts/agent-branch-finish.sh --branch "<agent-branch>" --via-pr` and keep the branch open until checks/review pass.
97100
- 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.
101+
- 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`.
102+
- 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.
98103

99104
1. Explicit ownership before edits
100105

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,7 @@ and asks `[y/N]` whether to update immediately (default is `N`).
292292
- Non-interactive setup: skips global installs by default; use `--yes-global-install` to force.
293293
- 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.
294294
- `gx doctor` on protected `main` auto-starts an isolated `agent/gx/...-gx-doctor` worktree branch and applies repairs there.
295+
It also syncs repaired `.omx/state/agent-file-locks.json` back to your protected workspace so stale-lock warnings clear immediately.
295296
- `gx setup` and `gx doctor` always refresh `.githooks/pre-commit` from templates, so Codex sub-branch enforcement stays repaired.
296297
- `scripts/codex-agent.sh` now auto-runs finish automation after a Codex session when `origin` exists:
297298
auto-commit changed files, run PR/merge automation, and keep merged agent branches/worktrees by default.

bin/multiagent-safety.js

Lines changed: 69 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -811,8 +811,59 @@ function runDoctorInSandbox(options, blocked) {
811811
throw nestedResult.error;
812812
}
813813

814+
let lockSyncResult = {
815+
status: 'skipped',
816+
note: 'sandbox doctor did not complete successfully',
817+
};
818+
if (nestedResult.status === 0) {
819+
const sandboxLockPath = path.join(metadata.worktreePath, LOCK_FILE_RELATIVE);
820+
const baseLockPath = path.join(blocked.repoRoot, LOCK_FILE_RELATIVE);
821+
if (!fs.existsSync(sandboxLockPath)) {
822+
lockSyncResult = {
823+
status: 'skipped',
824+
note: `${LOCK_FILE_RELATIVE} missing in sandbox worktree`,
825+
};
826+
} else {
827+
const sourceContent = fs.readFileSync(sandboxLockPath, 'utf8');
828+
const destinationContent = fs.existsSync(baseLockPath) ? fs.readFileSync(baseLockPath, 'utf8') : '';
829+
if (sourceContent === destinationContent) {
830+
lockSyncResult = {
831+
status: 'unchanged',
832+
note: `${LOCK_FILE_RELATIVE} already in sync`,
833+
};
834+
} else {
835+
fs.mkdirSync(path.dirname(baseLockPath), { recursive: true });
836+
fs.writeFileSync(baseLockPath, sourceContent, 'utf8');
837+
lockSyncResult = {
838+
status: 'synced',
839+
note: `${LOCK_FILE_RELATIVE} synced from sandbox`,
840+
};
841+
}
842+
}
843+
}
844+
814845
if (options.json) {
815-
if (nestedResult.stdout) process.stdout.write(nestedResult.stdout);
846+
if (nestedResult.stdout) {
847+
if (nestedResult.status === 0) {
848+
try {
849+
const parsed = JSON.parse(nestedResult.stdout);
850+
process.stdout.write(
851+
JSON.stringify(
852+
{
853+
...parsed,
854+
sandboxLockSync: lockSyncResult,
855+
},
856+
null,
857+
2,
858+
) + '\n',
859+
);
860+
} catch {
861+
process.stdout.write(nestedResult.stdout);
862+
}
863+
} else {
864+
process.stdout.write(nestedResult.stdout);
865+
}
866+
}
816867
if (nestedResult.stderr) process.stderr.write(nestedResult.stderr);
817868
} else {
818869
console.log(
@@ -823,6 +874,17 @@ function runDoctorInSandbox(options, blocked) {
823874
if (startResult.stderr) process.stderr.write(startResult.stderr);
824875
if (nestedResult.stdout) process.stdout.write(nestedResult.stdout);
825876
if (nestedResult.stderr) process.stderr.write(nestedResult.stderr);
877+
if (nestedResult.status === 0) {
878+
if (lockSyncResult.status === 'synced') {
879+
console.log(
880+
`[${TOOL_NAME}] Synced repaired lock registry back to protected branch workspace (${LOCK_FILE_RELATIVE}).`,
881+
);
882+
} else if (lockSyncResult.status === 'unchanged') {
883+
console.log(`[${TOOL_NAME}] Lock registry already synced in protected branch workspace.`);
884+
} else {
885+
console.log(`[${TOOL_NAME}] Lock registry sync skipped: ${lockSyncResult.note}.`);
886+
}
887+
}
826888
}
827889

828890
if (typeof nestedResult.status === 'number') {
@@ -2093,14 +2155,16 @@ function doctor(rawArgs) {
20932155
assertProtectedMainWriteAllowed(options, 'doctor');
20942156
const fixPayload = runFixInternal(options);
20952157
const scanResult = runScanInternal({ target: options.target, json: false });
2096-
const musafe = scanResult.errors === 0 && scanResult.warnings === 0;
2158+
const safe = scanResult.errors === 0 && scanResult.warnings === 0;
2159+
const musafe = safe;
20972160

20982161
if (options.json) {
20992162
process.stdout.write(
21002163
JSON.stringify(
21012164
{
21022165
repoRoot: scanResult.repoRoot,
21032166
branch: scanResult.branch,
2167+
safe,
21042168
musafe,
21052169
fix: {
21062170
operations: fixPayload.operations,
@@ -2123,11 +2187,11 @@ function doctor(rawArgs) {
21232187

21242188
printOperations('Doctor/fix', fixPayload, options.dryRun);
21252189
printScanResult(scanResult, false);
2126-
if (musafe) {
2127-
console.log(`[${TOOL_NAME}] ✅ Repo is correctly musafe.`);
2190+
if (safe) {
2191+
console.log(`[${TOOL_NAME}] ✅ Repo is fully safe.`);
21282192
} else {
21292193
console.log(
2130-
`[${TOOL_NAME}] ⚠️ Repo is not fully musafe yet (${scanResult.errors} error(s), ${scanResult.warnings} warning(s)).`,
2194+
`[${TOOL_NAME}] ⚠️ Repo is not fully safe yet (${scanResult.errors} error(s), ${scanResult.warnings} warning(s)).`,
21312195
);
21322196
}
21332197
setExitCodeFromScan(scanResult);

templates/AGENTS.multiagent-safety.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
- Use explicit cleanup when done: `gx cleanup --branch "<agent-branch>"` (or `gx cleanup` for all merged agent branches).
1717
- If codex-agent auto-finish cannot complete, immediately run `scripts/agent-branch-finish.sh --branch "<agent-branch>" --via-pr` and keep the branch open until checks/review pass.
1818
- 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.
19+
- 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`.
1920
- 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.
2021

2122
1. Explicit ownership before edits

test/install.test.js

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,57 @@ test('doctor on protected main auto-runs in a sandbox branch/worktree', () => {
325325
assert.equal(currentBranch.stdout.trim(), 'main');
326326
});
327327

328+
test('doctor on protected main syncs repaired stale lock state back to base workspace', () => {
329+
const repoDir = initRepoOnBranch('main');
330+
seedCommit(repoDir);
331+
attachOriginRemoteForBranch(repoDir, 'main');
332+
333+
let result = runNode(['setup', '--target', repoDir, '--no-global-install'], repoDir);
334+
assert.equal(result.status, 0, result.stderr || result.stdout);
335+
336+
result = runCmd('git', ['add', '.'], repoDir);
337+
assert.equal(result.status, 0, result.stderr);
338+
result = runCmd('git', ['commit', '-m', 'apply gx setup'], repoDir, {
339+
ALLOW_COMMIT_ON_PROTECTED_BRANCH: '1',
340+
});
341+
assert.equal(result.status, 0, result.stderr || result.stdout);
342+
result = runCmd('git', ['push', 'origin', 'main'], repoDir);
343+
assert.equal(result.status, 0, result.stderr || result.stdout);
344+
345+
const lockPath = path.join(repoDir, '.omx', 'state', 'agent-file-locks.json');
346+
fs.writeFileSync(
347+
lockPath,
348+
JSON.stringify(
349+
{
350+
locks: {
351+
'package.json': {
352+
branch: 'agent/non-existent',
353+
claimed_at: '2026-01-01T00:00:00Z',
354+
allow_delete: false,
355+
},
356+
},
357+
},
358+
null,
359+
2,
360+
) + '\n',
361+
);
362+
363+
const scanBefore = runNode(['scan', '--target', repoDir], repoDir);
364+
assert.equal(scanBefore.status, 1, scanBefore.stderr || scanBefore.stdout);
365+
assert.match(scanBefore.stdout, /stale-branch-lock/);
366+
367+
result = runNode(['doctor', '--target', repoDir], repoDir);
368+
assert.equal(result.status, 0, result.stderr || result.stdout);
369+
assert.match(result.stdout, /doctor detected protected branch 'main'/);
370+
assert.match(result.stdout, /Synced repaired lock registry back to protected branch workspace/);
371+
372+
const lockState = JSON.parse(fs.readFileSync(lockPath, 'utf8'));
373+
assert.deepEqual(lockState.locks, {});
374+
375+
const scanAfter = runNode(['scan', '--target', repoDir], repoDir);
376+
assert.equal(scanAfter.status, 0, scanAfter.stderr || scanAfter.stdout);
377+
});
378+
328379
test('setup pre-commit blocks codex session commits on non-agent branches by default', () => {
329380
const repoDir = initRepo();
330381

@@ -1353,7 +1404,7 @@ test('fix repairs stale lock issues so scan becomes clean', () => {
13531404
assert.equal(result.status, 0, result.stdout + result.stderr);
13541405
});
13551406

1356-
test('doctor repairs setup drift and confirms repo is musafe', () => {
1407+
test('doctor repairs setup drift and confirms repo is safe', () => {
13571408
const repoDir = initRepo();
13581409

13591410
let result = runNode(['setup', '--target', repoDir], repoDir);
@@ -1386,7 +1437,7 @@ test('doctor repairs setup drift and confirms repo is musafe', () => {
13861437
result = runNode(['doctor', '--target', repoDir], repoDir);
13871438
assert.equal(result.status, 0, result.stderr || result.stdout);
13881439
assert.match(result.stdout, /Doctor\/fix/);
1389-
assert.match(result.stdout, /Repo is correctly musafe/);
1440+
assert.match(result.stdout, /Repo is fully safe/);
13901441

13911442
const repairedHook = fs.readFileSync(path.join(repoDir, '.githooks', 'pre-commit'), 'utf8');
13921443
assert.match(repairedHook, /AGENTS\.md\|\.gitignore/);

0 commit comments

Comments
 (0)