feat(automation): gated executor with safety rails (Arc D phase 3a)#45
Conversation
Add src/automation_executor.py: the only layer that mutates real repos, built safe by construction. Every git/gh call goes through an injected CommandRunner (real subprocess in prod, fake in tests), so the full command sequence and all rails are unit-tested without touching a repo or network. Rails (all tested): - require_approved is the hard pre-gate; an unapproved proposal runs nothing. - dry_run defaults to True; in dry-run no mutating command and no apply_change run. - context-PR refuses protected/default branches (case-insensitive), rejects unsafe/flag-like branch names, requires a non-empty default branch, never uses --force, skips dirty worktrees and non-git repos. - fail-fast: a failed checkout/commit never reaches push or gh pr create; an apply_change exception returns 'failed' and best-effort restores the branch. - catalog-seed apply is local + reversible (only fills missing fields). - mark_executed (added to automation_proposals): APPROVED -> EXECUTED only. Content generation (managed block, catalog seeds) is supplied by the caller and computed fresh at execution time; phase 3b wires it to the recovery primitives and adds the propose/approve/execute CLI. 27 executor tests + mark_executed tests. Full suite 2394 passed, ruff clean. /code-review (adversarial) applied: fixed case-insensitive branch bypass, empty default_branch, flag-like branch injection, and apply_change strand.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a80e743069
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) | ||
|
|
||
| # Run checkout first, then apply the change, then stage/commit/push/PR. | ||
| checkout = runner(["git", "checkout", "-b", branch], plan.repo_path) |
There was a problem hiding this comment.
When a repo is clean but currently checked out to any non-default branch, this git checkout -b forks the automation branch from the current HEAD while the PR is opened against default_branch, so the PR can include unrelated commits from the operator's local branch. I checked git checkout -h and -b <branch> only creates/checks out a new branch; because no start point is passed here, the executor needs to first verify/checkout the declared default branch or create the branch from that ref.
Useful? React with 👍 / 👎.
| result = runner(args, plan.repo_path) | ||
| if result.returncode != 0: | ||
| return ExecutionResult( | ||
| proposal.proposal_id, "failed", f"{args[1]}: {result.stderr}".strip() |
There was a problem hiding this comment.
If apply_change succeeds but git add, git commit, or git push fails, this returns failed without undoing the checkout or the file changes; for example, a missing git user config or a commit hook failure leaves the repo on the automation branch with staged/dirty changes. The dirty check ran before these mutations, so the next execution can either skip as dirty or fail because the branch already exists; mirror the restore/cleanup used for apply_change exceptions on these failure paths.
Useful? React with 👍 / 👎.
1 participant
Arc D — Phase 3a: the gated executor (real-repo writes, safe by construction)
The only layer that mutates real repos. Every git/gh call goes through an injected
CommandRunner(real subprocess in prod, fake in tests), so the exact command sequence and every rail are unit-tested with no repo or network touched.Safety rails (all tested)
require_approvedis the hard pre-gate — an unapproved proposal runs nothing.dry_rundefaults to True; in dry-run, no mutating command and noapply_changerun (only the read-only dirty check).^[A-Za-z0-9._]...— no leading-), requires non-empty default branch, never--force, skips dirty worktrees + non-git repos.gh pr create; anapply_changeexception returnsfailedand best-effort restores the default branch (no stranding)._merge_catalog_seeds).mark_executed(added toautomation_proposals):APPROVED → EXECUTEDonly — the lifecycle can't skip approval.Design seam
Content generation (the managed block, the catalog seed dict) is caller-supplied and computed fresh at execution time via
ExecutionPlan.apply_change/seeds— phase 3b wires those to the existing recovery primitives and adds thepropose → approve → executeCLI.Gates
tests/test_automation_executor.py) +mark_executedlifecycle tests. Full suite 2394 passed, 2 skipped.ruff check .clean./code-reviewapplied: fixed case-insensitive branch bypass, emptydefault_branch, flag-like-branch injection (-b --force/--head), and theapply_changestrand-on-failure hole. Content fields (-m/--title/--body) are flag-consumed values (safe;pr_bodylegitimately starts with-).Next — phase 3b: wire fresh content generation (managed block + seeds) and the operator CLI workflow (
--propose-automation,--approve-proposal,--reject-proposal,--execute-proposals [--apply]).