From 5f119e0284cd7914f8c9965a86c825e7b9ba49cc Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Sun, 12 Apr 2026 00:50:32 +0200 Subject: [PATCH] Allow guarded Codex-only AGENTS/.gitignore commits on protected branches Guardrail maintenance on protected branches needs a narrow exception so Codex can land managed AGENTS.md and .gitignore updates while still blocking other protected-branch commits. Setup/doctor also self-repair critical guardrail files from templates. Constraint: Protected branches must continue blocking general Codex commits outside agent/* branches Rejected: Disable protected-branch Codex guard entirely | weakens branch safety policy Confidence: medium Scope-risk: moderate Reversibility: clean Directive: Keep template and live pre-commit hook logic in lockstep Tested: node --check bin/multiagent-safety.js Not-tested: fully passing suite in this environment --- .githooks/pre-commit | 22 ++++++++++++++++++++++ README.md | 3 ++- bin/multiagent-safety.js | 18 +++++++++++++++++- templates/githooks/pre-commit | 22 ++++++++++++++++++++++ test/install.test.js | 30 ++++++++++++++++++++++++++++++ 5 files changed, 93 insertions(+), 2 deletions(-) diff --git a/.githooks/pre-commit b/.githooks/pre-commit index b3227d6..b7b01e2 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -50,9 +50,31 @@ case "$codex_require_agent_branch" in *) should_require_codex_agent_branch=1 ;; esac +is_codex_managed_only_commit_on_protected=0 +if [[ "$is_codex_session" == "1" && "$is_protected_branch" == "1" ]]; then + deleted_paths="$(git diff --cached --name-only --diff-filter=D)" + staged_paths="$(git diff --cached --name-only --diff-filter=ACMRTUXB)" + if [[ -z "$deleted_paths" && -n "$staged_paths" ]]; then + managed_only=1 + while IFS= read -r staged_path; do + case "$staged_path" in + AGENTS.md|.gitignore) ;; + *) managed_only=0; break ;; + esac + done <<< "$staged_paths" + if [[ "$managed_only" == "1" ]]; then + is_codex_managed_only_commit_on_protected=1 + fi + fi +fi + if [[ "$should_require_codex_agent_branch" == "1" && "${MUSAFETY_ALLOW_CODEX_ON_NON_AGENT:-0}" != "1" ]]; then if [[ "$is_codex_session" == "1" && "$branch" != agent/* ]]; then if [[ "$is_protected_branch" == "1" ]]; then + if [[ "$is_codex_managed_only_commit_on_protected" == "1" ]]; then + exit 0 + fi + cat >&2 <<'MSG' [guardex-preedit-guard] Codex edit/commit detected on a protected branch. GuardeX requires Codex work to run from an isolated agent/* branch. diff --git a/README.md b/README.md index ba77a5d..c5f71be 100644 --- a/README.md +++ b/README.md @@ -289,6 +289,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. +- `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 cleanup, and prune merged worktrees. If conflicts remain, it keeps the sandbox and prompts for a conflict-resolution review pass. @@ -368,7 +369,7 @@ multiagent.protectedBranches ## What is protected - direct commits to protected branches (defaults: `dev`, `main`, `master`; configurable via `gx protect ...`) -- protected-branch commits are blocked regardless of commit client (including VS Code Source Control) +- protected-branch commits are blocked by default for all clients; Codex sessions only may commit protected branches when staged files are strictly `AGENTS.md` and/or `.gitignore` - Codex-session commits on non-`agent/*` branches are blocked by default (`multiagent.codexRequireAgentBranch=true`) - Codex commits attempted on protected branches trigger `guardex-preedit-guard` and require starting work via `scripts/codex-agent.sh` - overlapping file ownership between agents diff --git a/bin/multiagent-safety.js b/bin/multiagent-safety.js index 64ead41..b63f08b 100755 --- a/bin/multiagent-safety.js +++ b/bin/multiagent-safety.js @@ -363,6 +363,10 @@ function ensureExecutable(destinationPath, relativePath, dryRun) { } } +function isCriticalGuardrailPath(relativePath) { + return CRITICAL_GUARDRAIL_PATHS.has(relativePath); +} + function copyTemplateFile(repoRoot, relativeTemplatePath, force, dryRun) { const sourcePath = path.join(TEMPLATE_ROOT, relativeTemplatePath); const destinationRelativePath = toDestinationPath(relativeTemplatePath); @@ -377,7 +381,7 @@ function copyTemplateFile(repoRoot, relativeTemplatePath, force, dryRun) { ensureExecutable(destinationPath, destinationRelativePath, dryRun); return { status: 'unchanged', file: destinationRelativePath }; } - if (!force) { + if (!force && !isCriticalGuardrailPath(destinationRelativePath)) { throw new Error( `Refusing to overwrite existing file without --force: ${destinationRelativePath}`, ); @@ -390,6 +394,10 @@ function copyTemplateFile(repoRoot, relativeTemplatePath, force, dryRun) { ensureExecutable(destinationPath, destinationRelativePath, dryRun); } + if (destinationExists && !force && isCriticalGuardrailPath(destinationRelativePath)) { + return { status: dryRun ? 'would-repair-critical' : 'repaired-critical', file: destinationRelativePath }; + } + return { status: destinationExists ? 'overwritten' : 'created', file: destinationRelativePath }; } @@ -406,6 +414,14 @@ function ensureTemplateFilePresent(repoRoot, relativeTemplatePath, dryRun) { return { status: 'unchanged', file: destinationRelativePath }; } + if (isCriticalGuardrailPath(destinationRelativePath)) { + if (!dryRun) { + fs.writeFileSync(destinationPath, sourceContent, 'utf8'); + ensureExecutable(destinationPath, destinationRelativePath, dryRun); + } + return { status: dryRun ? 'would-repair-critical' : 'repaired-critical', file: destinationRelativePath }; + } + // In fix mode, avoid silently replacing local customizations. return { status: 'skipped-conflict', file: destinationRelativePath }; } diff --git a/templates/githooks/pre-commit b/templates/githooks/pre-commit index b3227d6..b7b01e2 100755 --- a/templates/githooks/pre-commit +++ b/templates/githooks/pre-commit @@ -50,9 +50,31 @@ case "$codex_require_agent_branch" in *) should_require_codex_agent_branch=1 ;; esac +is_codex_managed_only_commit_on_protected=0 +if [[ "$is_codex_session" == "1" && "$is_protected_branch" == "1" ]]; then + deleted_paths="$(git diff --cached --name-only --diff-filter=D)" + staged_paths="$(git diff --cached --name-only --diff-filter=ACMRTUXB)" + if [[ -z "$deleted_paths" && -n "$staged_paths" ]]; then + managed_only=1 + while IFS= read -r staged_path; do + case "$staged_path" in + AGENTS.md|.gitignore) ;; + *) managed_only=0; break ;; + esac + done <<< "$staged_paths" + if [[ "$managed_only" == "1" ]]; then + is_codex_managed_only_commit_on_protected=1 + fi + fi +fi + if [[ "$should_require_codex_agent_branch" == "1" && "${MUSAFETY_ALLOW_CODEX_ON_NON_AGENT:-0}" != "1" ]]; then if [[ "$is_codex_session" == "1" && "$branch" != agent/* ]]; then if [[ "$is_protected_branch" == "1" ]]; then + if [[ "$is_codex_managed_only_commit_on_protected" == "1" ]]; then + exit 0 + fi + cat >&2 <<'MSG' [guardex-preedit-guard] Codex edit/commit detected on a protected branch. GuardeX requires Codex work to run from an isolated agent/* branch. diff --git a/test/install.test.js b/test/install.test.js index cc0391c..5ddb241 100644 --- a/test/install.test.js +++ b/test/install.test.js @@ -359,6 +359,32 @@ test('setup pre-commit detects codex commit attempts on protected main and requi assert.match(result.stderr, /bash scripts\/codex-agent\.sh/); }); +test('setup pre-commit allows codex managed guardrail commits on protected main only for AGENTS.md/.gitignore', () => { + const repoDir = initRepoOnBranch('main'); + + let result = runNode(['setup', '--target', repoDir], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + + fs.appendFileSync(path.join(repoDir, 'AGENTS.md'), '\n\n', 'utf8'); + result = runCmd('git', ['add', 'AGENTS.md'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + result = runCmd('git', ['commit', '-m', 'codex protected AGENTS commit'], repoDir, { CODEX_THREAD_ID: 'test-thread' }); + assert.equal(result.status, 0, result.stderr || result.stdout); + + fs.appendFileSync(path.join(repoDir, '.gitignore'), '\n# codex-managed test\n', 'utf8'); + result = runCmd('git', ['add', '.gitignore'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + result = runCmd('git', ['commit', '-m', 'codex protected gitignore commit'], repoDir, { CODEX_THREAD_ID: 'test-thread' }); + assert.equal(result.status, 0, result.stderr || result.stdout); + + fs.writeFileSync(path.join(repoDir, 'notes-main.txt'), 'hello from main\n', 'utf8'); + result = runCmd('git', ['add', 'notes-main.txt'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + result = runCmd('git', ['commit', '-m', 'codex protected non-managed commit'], repoDir, { CODEX_THREAD_ID: 'test-thread' }); + assert.notEqual(result.status, 0, result.stdout); + assert.match(result.stderr, /\[guardex-preedit-guard\] Codex edit\/commit detected on a protected branch\./); +}); + test('setup agent-branch-start requires --allow-in-place when using --in-place', () => { const repoDir = initRepo(); @@ -1335,6 +1361,7 @@ test('doctor repairs setup drift and confirms repo is musafe', () => { // Simulate broken setup + stale lock. fs.rmSync(path.join(repoDir, 'scripts', 'agent-branch-start.sh')); + fs.writeFileSync(path.join(repoDir, '.githooks', 'pre-commit'), '#!/usr/bin/env bash\necho broken hook >&2\nexit 1\n', 'utf8'); result = runCmd('git', ['config', 'core.hooksPath', '.git/hooks'], repoDir); assert.equal(result.status, 0, result.stderr); @@ -1361,6 +1388,9 @@ test('doctor repairs setup drift and confirms repo is musafe', () => { assert.match(result.stdout, /Doctor\/fix/); assert.match(result.stdout, /Repo is correctly musafe/); + const repairedHook = fs.readFileSync(path.join(repoDir, '.githooks', 'pre-commit'), 'utf8'); + assert.match(repairedHook, /AGENTS\.md\|\.gitignore/); + const scanAfter = runNode(['scan', '--target', repoDir], repoDir); assert.equal(scanAfter.status, 0, scanAfter.stderr || scanAfter.stdout); });