From a2b88ec909369c5abadda1b15f9251bae87f7987 Mon Sep 17 00:00:00 2001 From: NagyVikt Date: Sat, 11 Apr 2026 13:03:07 +0200 Subject: [PATCH] Keep Codex workflow changes isolated and safer by default This captures the pending musafety workflow hardening: Codex session commits now require agent/* branches by default, startup sandbox scripts gained stronger argument handling, and status self-update prompts default to no. The change also updates docs/templates/tests and publishes the next package version. Constraint: Protected branches must stay safe for interactive maintainers while still supporting sandboxed agent automation Rejected: Keep Codex non-agent commits allowed by default | too easy to accidentally write from protected/non-isolated branches Confidence: high Scope-risk: moderate Reversibility: clean Directive: If relaxing branch guards, update both hook templates and install tests in the same change Tested: npm test (41/41 pass) Not-tested: end-to-end npm publish in CI for this exact commit --- .gitignore | 15 ++++ AGENTS.md | 61 +++++++++++++++ README.md | 4 +- bin/multiagent-safety.js | 2 +- package-lock.json | 4 +- package.json | 21 ++++- templates/codex/skills/musafety/SKILL.md | 1 + templates/githooks/pre-commit | 35 +++++++++ templates/scripts/agent-branch-start.sh | 45 +++++++++-- templates/scripts/codex-agent.sh | 49 ++++++++++-- test/install.test.js | 97 +++++++++++++++++++++++- 11 files changed, 316 insertions(+), 18 deletions(-) diff --git a/.gitignore b/.gitignore index 451fec9..c0ef42d 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,18 @@ .omx/ node_modules oh-my-codex/ + +# multiagent-safety:START +scripts/agent-branch-start.sh +scripts/agent-branch-finish.sh +scripts/codex-agent.sh +scripts/agent-worktree-prune.sh +scripts/agent-file-locks.py +scripts/install-agent-git-hooks.sh +scripts/openspec/init-plan-workspace.sh +.githooks/pre-commit +oh-my-codex/ +.codex/skills/musafety/SKILL.md +.claude/commands/musafety.md +.omx/state/agent-file-locks.json +# multiagent-safety:END diff --git a/AGENTS.md b/AGENTS.md index 7c9cb3f..b753ab3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -74,3 +74,64 @@ OMX runtime state typically lives under `.omx/`: - `.omx/project-memory.json` - `.omx/plans/` - `.omx/logs/` + + +## Multi-Agent Execution Contract (multiagent-safety) + +0. Session plan comment + read gate (required) + +- Before editing, each agent must post a short session comment/handoff note that includes: + - plan/change name (or checkpoint id), + - owned files/scope, + - intended action. +- Before deleting/replacing code, each agent must read the latest session comments/handoffs first and confirm the target code is in their owned scope. +- If ownership is unclear or overlaps, stop that edit, post a blocker comment, and let the leader/integrator reassign scope. +- For git isolation, each agent must start on a dedicated branch via `scripts/agent-branch-start.sh "" ""`. +- Agent completion must use `scripts/agent-branch-finish.sh` (merge into `dev`, push, delete agent branch). + +1. Explicit ownership before edits + +- Assign each agent clear file/module ownership. +- Do not edit files outside your assigned scope unless the leader reassigns ownership. + +2. Preserve parallel safety + +- Assume other agents are editing nearby code concurrently. +- Never revert unrelated changes authored by others. +- If another change conflicts with your approach, adapt and report the conflict in handoff. + +3. Verify before completion + +- Run required local checks for the area you changed. +- Do not mark work complete without command output evidence. + +4. Required handoff format (every agent) + +- Files changed +- Behavior touched +- Verification commands + results +- Risks / follow-ups + +## OpenSpec Plan Workspace (recommended) + +When work needs a durable planning phase, scaffold a plan workspace before implementation: + +```bash +bash scripts/openspec/init-plan-workspace.sh "" +``` + +Expected shape: + +```text +openspec/plan// + summary.md + checkpoints.md + planner/plan.md + planner/tasks.md + architect/tasks.md + critic/tasks.md + executor/tasks.md + writer/tasks.md + verifier/tasks.md +``` + diff --git a/README.md b/README.md index 6049618..3d7fa3a 100644 --- a/README.md +++ b/README.md @@ -197,7 +197,7 @@ bash scripts/openspec/init-plan-workspace.sh # optional OpenSpec p No command defaults to `musafety status` (non-mutating health/status view). `musafety status` reports CLI/runtime info, global OMX/OpenSpec service status, and repo safety service state. When run in an interactive terminal, default `musafety` checks npm for a newer version first -and asks `[Y/n]` whether to update immediately (default is `Y`). +and asks `[y/N]` whether to update immediately (default is `N`). - Interactive setup: prompts for Y/N approval before global OMX/OpenSpec install. - Interactive prompt is strict (`[y/n]`) and waits for explicit answer. @@ -272,10 +272,12 @@ multiagent.protectedBranches - direct commits to protected branches (defaults: `dev`, `main`, `master`; configurable via `musafety protect ...`) - protected-branch commits are blocked regardless of commit client (including VS Code Source Control) +- Codex-session commits on non-`agent/*` branches are blocked by default (`multiagent.codexRequireAgentBranch=true`) - overlapping file ownership between agents - unapproved deletions of claimed files - risky stale/missing lock state - accidental loss of critical guardrail files +- in-place branch bootstrap requires explicit opt-in (`--in-place --allow-in-place`) - setup also writes a managed `.gitignore` block so generated musafety scripts/hooks stay out of normal git status noise by default - includes `oh-my-codex/` by default to keep local OMX source clones out of repo status - pass `--no-gitignore` if you want to keep tracking these files in git diff --git a/bin/multiagent-safety.js b/bin/multiagent-safety.js index a480f4c..e4b52a2 100755 --- a/bin/multiagent-safety.js +++ b/bin/multiagent-safety.js @@ -1266,7 +1266,7 @@ function maybeSelfUpdateBeforeStatus() { ? autoApproval : promptYesNo( `Update now? (${NPM_BIN} i -g ${packageJson.name}@latest)`, - true, + false, ); if (!shouldUpdate) { diff --git a/package-lock.json b/package-lock.json index 3c111e5..4802760 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "musafety", - "version": "0.4.9", + "version": "5.0.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "musafety", - "version": "0.4.9", + "version": "5.0.0", "license": "MIT", "bin": { "multiagent-safety": "bin/multiagent-safety.js", diff --git a/package.json b/package.json index fd794dd..a0c95c2 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "musafety", - "version": "0.4.9", + "version": "5.0.0", "description": "Simple setup command for hardened multi-agent collaboration safety in git repos.", "license": "MIT", "preferGlobal": true, @@ -9,7 +9,24 @@ "multiagent-safety": "bin/multiagent-safety.js" }, "scripts": { - "test": "node --test test/*.test.js" + "test": "node --test test/*.test.js", + "agent:codex": "bash ./scripts/codex-agent.sh", + "agent:branch:start": "bash ./scripts/agent-branch-start.sh", + "agent:branch:finish": "bash ./scripts/agent-branch-finish.sh", + "agent:cleanup": "bash ./scripts/agent-worktree-prune.sh --base dev", + "agent:hooks:install": "bash ./scripts/install-agent-git-hooks.sh", + "agent:locks:claim": "python3 ./scripts/agent-file-locks.py claim", + "agent:locks:allow-delete": "python3 ./scripts/agent-file-locks.py allow-delete", + "agent:locks:release": "python3 ./scripts/agent-file-locks.py release", + "agent:locks:status": "python3 ./scripts/agent-file-locks.py status", + "agent:plan:init": "bash ./scripts/openspec/init-plan-workspace.sh", + "agent:protect:list": "musafety protect list", + "agent:branch:sync": "musafety sync", + "agent:branch:sync:check": "musafety sync --check", + "agent:safety:setup": "musafety setup", + "agent:safety:scan": "musafety scan", + "agent:safety:fix": "musafety fix", + "agent:safety:doctor": "musafety doctor" }, "engines": { "node": ">=18" diff --git a/templates/codex/skills/musafety/SKILL.md b/templates/codex/skills/musafety/SKILL.md index ceb2cd8..261f485 100644 --- a/templates/codex/skills/musafety/SKILL.md +++ b/templates/codex/skills/musafety/SKILL.md @@ -32,4 +32,5 @@ musafety scan - Prefer `musafety doctor` for one-step repair + verification. - Keep agent work isolated (`agent/*` branches + lock claims). +- For one-command Codex sandbox startup, use `bash scripts/codex-agent.sh "" ""`. - Do not bypass protected branch safeguards unless explicitly required. diff --git a/templates/githooks/pre-commit b/templates/githooks/pre-commit index 2c77555..f1c28fe 100755 --- a/templates/githooks/pre-commit +++ b/templates/githooks/pre-commit @@ -43,6 +43,41 @@ MSG exit 1 fi +codex_require_agent_branch_raw="${MUSAFETY_CODEX_REQUIRE_AGENT_BRANCH:-$(git config --get multiagent.codexRequireAgentBranch || true)}" +if [[ -z "$codex_require_agent_branch_raw" ]]; then + codex_require_agent_branch_raw="true" +fi +codex_require_agent_branch="$(printf '%s' "$codex_require_agent_branch_raw" | tr '[:upper:]' '[:lower:]')" + +should_require_codex_agent_branch=0 +case "$codex_require_agent_branch" in + 1|true|yes|on) should_require_codex_agent_branch=1 ;; + 0|false|no|off) should_require_codex_agent_branch=0 ;; + *) should_require_codex_agent_branch=1 ;; +esac + +if [[ "$should_require_codex_agent_branch" == "1" && "${MUSAFETY_ALLOW_CODEX_ON_NON_AGENT:-0}" != "1" ]]; then + is_codex_session=0 + if [[ -n "${CODEX_THREAD_ID:-}" || -n "${OMX_SESSION_ID:-}" || "${CODEX_CI:-0}" == "1" ]]; then + is_codex_session=1 + fi + + if [[ "$is_codex_session" == "1" && "$branch" != agent/* ]]; then + cat >&2 <<'MSG' +[codex-branch-guard] Codex agent commit blocked on non-agent branch. +Use isolated branch/worktree first: + bash scripts/agent-branch-start.sh "" "" +Then commit from the created agent/* branch. + +Temporary bypass (not recommended): + MUSAFETY_ALLOW_CODEX_ON_NON_AGENT=1 git commit ... +Disable this rule for a repo (not recommended): + git config multiagent.codexRequireAgentBranch false +MSG + exit 1 + fi +fi + if [[ "$branch" == agent/* ]]; then if ! python3 scripts/agent-file-locks.py validate --branch "$branch" --staged; then cat >&2 <<'MSG' diff --git a/templates/scripts/agent-branch-start.sh b/templates/scripts/agent-branch-start.sh index 61ca938..2f5614c 100755 --- a/templates/scripts/agent-branch-start.sh +++ b/templates/scripts/agent-branch-start.sh @@ -1,11 +1,13 @@ #!/usr/bin/env bash set -euo pipefail -TASK_NAME="${1:-task}" -AGENT_NAME="${2:-agent}" -BASE_BRANCH="${3:-dev}" +TASK_NAME="task" +AGENT_NAME="agent" +BASE_BRANCH="dev" WORKTREE_MODE=1 +ALLOW_IN_PLACE=0 WORKTREE_ROOT_REL=".omx/agent-worktrees" +POSITIONAL_ARGS=() while [[ $# -gt 0 ]]; do case "$1" in @@ -25,25 +27,52 @@ while [[ $# -gt 0 ]]; do WORKTREE_MODE=0 shift ;; + --allow-in-place) + ALLOW_IN_PLACE=1 + shift + ;; --worktree-root) WORKTREE_ROOT_REL="${2:-.omx/agent-worktrees}" shift 2 ;; --) shift + while [[ $# -gt 0 ]]; do + POSITIONAL_ARGS+=("$1") + shift + done break ;; -*) echo "[agent-branch-start] Unknown option: $1" >&2 - echo "Usage: $0 [task] [agent] [base] [--in-place] [--worktree-root ]" >&2 + echo "Usage: $0 [task] [agent] [base] [--in-place --allow-in-place] [--worktree-root ]" >&2 exit 1 ;; *) - break + POSITIONAL_ARGS+=("$1") + shift ;; esac done +if [[ "${#POSITIONAL_ARGS[@]}" -gt 3 ]]; then + echo "[agent-branch-start] Too many positional arguments." >&2 + echo "Usage: $0 [task] [agent] [base] [--in-place --allow-in-place] [--worktree-root ]" >&2 + exit 1 +fi + +if [[ "${#POSITIONAL_ARGS[@]}" -ge 1 ]]; then + TASK_NAME="${POSITIONAL_ARGS[0]}" +fi + +if [[ "${#POSITIONAL_ARGS[@]}" -ge 2 ]]; then + AGENT_NAME="${POSITIONAL_ARGS[1]}" +fi + +if [[ "${#POSITIONAL_ARGS[@]}" -ge 3 ]]; then + BASE_BRANCH="${POSITIONAL_ARGS[2]}" +fi + sanitize_slug() { local raw="$1" local slug @@ -83,6 +112,12 @@ if git show-ref --verify --quiet "refs/heads/${branch_name}"; then fi if [[ "$WORKTREE_MODE" -eq 0 ]]; then + if [[ "$ALLOW_IN_PLACE" -ne 1 ]]; then + echo "[agent-branch-start] --in-place is blocked by default to prevent accidental edits on protected branches." >&2 + echo "[agent-branch-start] If you really need it, pass both: --in-place --allow-in-place" >&2 + exit 1 + fi + if ! git diff --quiet || ! git diff --cached --quiet; then echo "[agent-branch-start] Working tree is not clean. Commit/stash changes before starting an in-place branch." >&2 exit 1 diff --git a/templates/scripts/codex-agent.sh b/templates/scripts/codex-agent.sh index fb340d7..9c81e5e 100755 --- a/templates/scripts/codex-agent.sh +++ b/templates/scripts/codex-agent.sh @@ -1,14 +1,51 @@ #!/usr/bin/env bash set -euo pipefail -TASK_NAME="${1:-task}" -AGENT_NAME="${2:-agent}" -BASE_BRANCH="${3:-dev}" +TASK_NAME="${MUSAFETY_TASK_NAME:-task}" +AGENT_NAME="${MUSAFETY_AGENT_NAME:-agent}" +BASE_BRANCH="${MUSAFETY_BASE_BRANCH:-dev}" CODEX_BIN="${MUSAFETY_CODEX_BIN:-codex}" -if [[ $# -ge 1 ]]; then shift; fi -if [[ $# -ge 1 ]]; then shift; fi -if [[ $# -ge 1 ]]; then shift; fi +while [[ $# -gt 0 ]]; do + case "$1" in + --task) + TASK_NAME="${2:-$TASK_NAME}" + shift 2 + ;; + --agent) + AGENT_NAME="${2:-$AGENT_NAME}" + shift 2 + ;; + --base) + BASE_BRANCH="${2:-$BASE_BRANCH}" + shift 2 + ;; + --codex-bin) + CODEX_BIN="${2:-$CODEX_BIN}" + shift 2 + ;; + --) + shift + break + ;; + -*) + break + ;; + *) + TASK_NAME="$1" + shift + if [[ $# -gt 0 && "${1:-}" != -* ]]; then + AGENT_NAME="$1" + shift + fi + if [[ $# -gt 0 && "${1:-}" != -* ]]; then + BASE_BRANCH="$1" + shift + fi + break + ;; + esac +done if ! command -v "$CODEX_BIN" >/dev/null 2>&1; then echo "[codex-agent] Missing Codex CLI command: $CODEX_BIN" >&2 diff --git a/test/install.test.js b/test/install.test.js index e3a3d41..6220a90 100644 --- a/test/install.test.js +++ b/test/install.test.js @@ -27,10 +27,15 @@ function runNodeWithEnv(args, cwd, extraEnv) { } function runCmd(cmd, args, cwd, options = {}) { + const sanitizedEnv = { ...process.env }; + delete sanitizedEnv.CODEX_THREAD_ID; + delete sanitizedEnv.OMX_SESSION_ID; + delete sanitizedEnv.CODEX_CI; + return cp.spawnSync(cmd, args, { cwd, encoding: 'utf8', - env: { ...process.env, ...(options.env || options) }, + env: { ...sanitizedEnv, ...(options.env || options) }, }); } @@ -204,6 +209,38 @@ test('setup provisions workflow files and repo config', () => { assert.equal(secondRun.status, 0, secondRun.stderr || secondRun.stdout); }); +test('setup pre-commit blocks codex session commits on non-agent branches by default', () => { + const repoDir = initRepo(); + + let result = runNode(['setup', '--target', repoDir], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + + result = runCmd('git', ['checkout', '-b', 'feature/codex-test'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + + fs.writeFileSync(path.join(repoDir, 'notes.txt'), 'hello\n', 'utf8'); + result = runCmd('git', ['add', 'notes.txt'], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + + result = runCmd('git', ['commit', '-m', 'codex non-agent commit'], repoDir, { CODEX_THREAD_ID: 'test-thread' }); + assert.notEqual(result.status, 0, result.stdout); + assert.match(result.stderr, /\[codex-branch-guard\] Codex agent commit blocked on non-agent branch\./); +}); + +test('setup agent-branch-start requires --allow-in-place when using --in-place', () => { + const repoDir = initRepo(); + + let result = runNode(['setup', '--target', repoDir], repoDir); + assert.equal(result.status, 0, result.stderr || result.stdout); + + seedCommit(repoDir); + + result = runCmd('bash', ['scripts/agent-branch-start.sh', 'demo', 'bot', 'dev', '--in-place'], repoDir); + assert.notEqual(result.status, 0, result.stdout); + assert.match(result.stderr, /--in-place is blocked by default/); + assert.match(result.stderr, /--in-place --allow-in-place/); +}); + test('default invocation runs non-mutating status output', () => { const repoDir = initRepo(); @@ -271,6 +308,14 @@ exit 1 assert.equal(fs.existsSync(markerPath), true, 'expected self-update command to run'); }); +test('self-update prompt defaults to no when approval is not preconfigured', () => { + const source = fs.readFileSync(cliPath, 'utf8'); + assert.match( + source, + /promptYesNo\(\s*`Update now\?\s*\(\$\{NPM_BIN\} i -g \$\{packageJson\.name\}@latest\)`\s*,\s*false,\s*\)/s, + ); +}); + test('status --json returns cli, services, and repo summary', () => { const repoDir = initRepo(); @@ -429,6 +474,56 @@ test('codex-agent launches codex inside a fresh sandbox worktree', () => { assert.match(branchResult.stdout.trim(), /^agent\/planner\//); }); +test('codex-agent supports --codex-bin override before positional arguments', () => { + const repoDir = initRepo(); + seedCommit(repoDir); + + const setupResult = runNode(['setup', '--target', repoDir, '--no-global-install'], repoDir); + assert.equal(setupResult.status, 0, setupResult.stderr || setupResult.stdout); + + const fakeBin = fs.mkdtempSync(path.join(os.tmpdir(), 'musafety-fake-codex-bin-')); + const fakeCodexPath = path.join(fakeBin, 'my-codex'); + fs.writeFileSync( + fakeCodexPath, + `#!/usr/bin/env bash\n` + + `pwd > "${'${MUSAFETY_TEST_CODEX_CWD}'}"\n` + + `echo "$@" > "${'${MUSAFETY_TEST_CODEX_ARGS}'}"\n`, + 'utf8', + ); + fs.chmodSync(fakeCodexPath, 0o755); + + const cwdMarker = path.join(repoDir, '.codex-agent-cwd-override'); + const argsMarker = path.join(repoDir, '.codex-agent-args-override'); + const launch = runCmd( + 'bash', + [ + 'scripts/codex-agent.sh', + '--codex-bin', + fakeCodexPath, + 'launch-task', + 'planner', + 'dev', + '--model', + 'gpt-5.4-mini', + ], + repoDir, + { + MUSAFETY_TEST_CODEX_CWD: cwdMarker, + MUSAFETY_TEST_CODEX_ARGS: argsMarker, + }, + ); + assert.equal(launch.status, 0, launch.stderr || launch.stdout); + assert.match(launch.stdout, /\[codex-agent\] Launching .* in sandbox:/); + + const launchedCwd = fs.readFileSync(cwdMarker, 'utf8').trim(); + assert.match( + launchedCwd, + new RegExp(`${escapeRegexLiteral(repoDir)}/\\.omx/agent-worktrees/agent__planner__`), + ); + const launchedArgs = fs.readFileSync(argsMarker, 'utf8').trim(); + assert.match(launchedArgs, /--model gpt-5\.4-mini/); +}); + test('sync command rebases current agent branch onto latest origin/dev', () => { const repoDir = initRepo(); seedCommit(repoDir);