From 46579e8ced3b466d08f24a218bd0ce8bb99cd8b6 Mon Sep 17 00:00:00 2001 From: Danil Pismenny Date: Tue, 4 Aug 2026 20:42:06 +0300 Subject: [PATCH 1/3] minor --- README.md | 2 +- README.ru.md | 2 +- memory-bank/features/FT-017/README.md | 3 +- memory-bank/features/FT-017/brief.md | 14 +- memory-bank/features/FT-017/decision-log.md | 128 +++++++++--------- memory-bank/features/FT-017/design.md | 25 ++-- .../features/FT-017/implementation-plan.md | 24 ++-- 7 files changed, 95 insertions(+), 103 deletions(-) diff --git a/README.md b/README.md index 2263acb..4330dbe 100644 --- a/README.md +++ b/README.md @@ -263,7 +263,7 @@ State files: ### Local real-Codex E2E smoke test -The normal Bats suite uses a fake Codex CLI. To exercise the real local Codex +The normal Go test suite uses a fake Codex CLI. To exercise the real local Codex CLI, run this opt-in test from a `start-issue` checkout: ```bash diff --git a/README.ru.md b/README.ru.md index b6ab6d0..886150f 100644 --- a/README.ru.md +++ b/README.ru.md @@ -110,7 +110,7 @@ State files: ### Локальный E2E smoke test с реальным Codex -Обычный Bats-набор использует fake Codex CLI. Для проверки с реальным локальным +Обычный Go test-набор использует fake Codex CLI. Для проверки с реальным локальным Codex из checkout `start-issue` выполните opt-in команду: ```bash diff --git a/memory-bank/features/FT-017/README.md b/memory-bank/features/FT-017/README.md index 682ccb9..6f02d52 100644 --- a/memory-bank/features/FT-017/README.md +++ b/memory-bank/features/FT-017/README.md @@ -33,5 +33,4 @@ Git delivery. for live GitHub-writing verification. - [decision-log.md](decision-log.md) - Historical release-distribution decisions retained from the earlier FT-017 - migration package. + FPF decisions, evidence provenance, and the remaining live-verification gate. diff --git a/memory-bank/features/FT-017/brief.md b/memory-bank/features/FT-017/brief.md index 42c9353..6e08987 100644 --- a/memory-bank/features/FT-017/brief.md +++ b/memory-bank/features/FT-017/brief.md @@ -43,7 +43,7 @@ supported Codex CLI while closing the remaining capability-contract gap. | --- | --- | --- | --- | --- | | `MET-01` | Human-gate capability contract visibility | One implicit `workspace-write` command | Every run reports either restricted or full-delivery permissions | Dry-run/help assertions | | `MET-02` | Full Git delivery reachability | GitHub/network/Git writes are not guaranteed | An explicitly authorized mode can edit, test, commit, push, and create/update a PR | Deterministic command tests plus opt-in live E2E evidence | -| `MET-03` | Supported Codex command compatibility | Compatibility can drift with CLI option placement | Generated commands are accepted by the supported Codex CLI contract | Bats command-shape coverage and real-Codex smoke validation | +| `MET-03` | Supported Codex command compatibility | Compatibility can drift with CLI option placement | Generated commands are accepted by the supported Codex CLI contract | Go command-shape tests and real-Codex smoke validation | ### Scope @@ -86,8 +86,10 @@ supported Codex CLI while closing the remaining capability-contract gap. ### Constraints / Assumptions -- `ASM-01` The supported reference environment is Codex CLI `0.145.0`, whose - global permission options are accepted before the `exec` subcommand. +- `ASM-01` Issue #37 reproduces the rejected argument order with Codex CLI + `0.144.6`; the feature must verify the exact supported command grammar with + the approved executable before acceptance. The repository does not pin an + installed Codex version. - `ASM-02` Full delivery requires independently configured GitHub authentication and repository authorization; `start-issue` can select a launcher policy but cannot grant those external capabilities. @@ -173,8 +175,8 @@ supported Codex CLI while closing the remaining capability-contract gap. | Check ID | Covers | How to check | Expected result | Evidence path | | --- | --- | --- | --- | --- | -| `CHK-01` | `EC-01` - `EC-04`, `SC-01` - `SC-04`, `NEG-01` | `make test` | Syntax, shellcheck, memory-bank audit, and deterministic Bats coverage pass for both modes and FT-015 regressions. | Local terminal/CI test output | -| `CHK-02` | `EC-01`, `EC-05`, `SC-01`, `SC-05`, `NEG-02` | Review `--help`, `--human-gate-help`, README files, and spec alongside output assertions | All surfaces state the same default, opt-in, capability, risk, and troubleshooting contract. | Review diff and Bats output | +| `CHK-01` | `EC-01` - `EC-04`, `SC-01` - `SC-04`, `NEG-01` | `make test` | Go formatting/vet/tests, memory-bank audit, and deterministic human-gate coverage pass for both modes and FT-015 regressions. | Local terminal/CI test output | +| `CHK-02` | `EC-01`, `EC-05`, `SC-01`, `SC-05`, `NEG-02` | Review `--help`, `--human-gate-help`, README files, and spec alongside Go output assertions | All surfaces state the same default, opt-in, capability, risk, and troubleshooting contract. | Review diff and Go test output | | `CHK-03` | `EC-02`, `EC-03`, `EC-06`, `SC-02`, `SC-03`, `SC-06`, `NEG-03` | With explicit approval, run the real-Codex full-delivery E2E procedure from FT-017's plan | Supported Codex accepts the command and the isolated fixture records commit, push, PR, terminal status, and retained artifacts. | Retained E2E artifact directory and fixture PR URL | ### Test matrix @@ -198,5 +200,5 @@ supported Codex CLI while closing the remaining capability-contract gap. | Evidence ID | Artifact | Producer | Path contract | Reused by checks | | --- | --- | --- | --- | --- | | `EVID-01` | Local and CI test output | implementer / CI | Terminal output and GitHub Actions job | `CHK-01` | -| `EVID-02` | Documentation diff plus help assertions | implementer / reviewer | Changed docs and Bats output | `CHK-02` | +| `EVID-02` | Documentation diff plus help assertions | implementer / reviewer | Changed docs and Go test output | `CHK-02` | | `EVID-03` | Live-E2E log, state files, commit/PR identifiers | approved operator | Retained E2E artifact path printed by runner | `CHK-03` | diff --git a/memory-bank/features/FT-017/decision-log.md b/memory-bank/features/FT-017/decision-log.md index af7ca3f..a5d3eaf 100644 --- a/memory-bank/features/FT-017/decision-log.md +++ b/memory-bank/features/FT-017/decision-log.md @@ -5,15 +5,15 @@ doc_function: reference purpose: "Records FPF analysis and accepted local decisions for FT-017. It does not own feature scope, selected design, acceptance criteria, or execution sequence." derived_from: - brief.md - - ../../../.github/workflows/ci.yml - - ../../../.github/workflows/release.yml - - ../../../install.sh + - design.md + - implementation-plan.md + - https://github.com/openai/codex/blob/main/codex-rs/exec/src/cli.rs status: active audience: humans_and_agents must_not_define: - - ft_016_scope - - ft_016_selected_design - - ft_016_acceptance_criteria + - ft_017_scope + - ft_017_selected_design + - ft_017_acceptance_criteria - implementation_sequence --- @@ -21,88 +21,82 @@ must_not_define: ## Purpose and Ownership -This log records why `DEC-01` remains open. The canonical owner of the blocker and the verify contract is [brief.md](brief.md). A selected solution belongs in a future `design.md`, not here. +This log records FPF decisions for FT-017. `brief.md` owns problem space and +acceptance; `design.md` owns the selected solution; `implementation-plan.md` +owns execution sequencing. This file records rationale and provenance only. -## DL-01 — Multi-platform Go release distribution contract +## DL-01 — Permission boundary and default -**Status:** accepted on 2026-07-22 by feature requester. +**Status:** accepted by FPF review on 2026-08-04. -### FPF framing +### Facts -- **Bounded context:** distribution is separate from CLI-semantic parity. It owns the relationship among a compiled artifact, release assets, installer/update selection, and the platform on which a user executes the artifact. -- **Evidence boundary:** facts below come only from the current repository and issue #34. The issue requests a Go binary but defines neither supported OS/architecture targets nor asset-selection rules. -- **Decision criterion:** provide Go releases for the requester-selected operating systems with explicit platform assets, verifiable integrity, and no inferred reduction of platform support. - -### Available facts - -1. `install.sh` downloads one fixed asset named `start-issue` and one fixed checksum file named `start-issue.sha256`. -2. `.github/workflows/release.yml` builds the current sole release asset on `ubuntu-latest`. -3. `.github/workflows/ci.yml` verifies installation on both `ubuntu-latest` and `macos-latest`. -4. A Bash release artifact is portable across those CI operating systems; a Go executable is platform-specific. -5. Issue #34 requires Go to become the primary distribution artifact and requires installation/release workflows to publish it successfully, but does not state the intended OS/architecture matrix or compatibility policy. +- Issue #37 states that `workspace-write` does not provide the network and Git + metadata writes needed for normal GitHub delivery. +- Existing FT-015 behavior uses `workspace-write` and must remain compatible. +- Full delivery can create external GitHub state and must therefore be opt-in. ### Decision -| Area | Accepted contract | -| --- | --- | -| Target matrix | `linux/amd64`, `linux/arm64`, `darwin/amd64`, `darwin/arm64`, and `windows/amd64`. The operating systems are requester-selected; the architecture set follows the explicit `dapi/port-selector` release pattern. | -| Build/release | Use GoReleaser v2 with `CGO_ENABLED=0`, one statically built executable per target, `start-issue--` asset names, and a SHA-256 `checksums.txt` manifest. During the v1-to-v2 cutover, also upload a `start-issue` bridge and its `start-issue.sha256` checksum for the v1 updater. | -| POSIX install | Adapt the referenced install-script strategy: detect `uname -s`/`uname -m`, select the matching asset, download it, verify its checksum from `checksums.txt`, and install it under the public name `start-issue`. | -| Windows delivery | Publish `start-issue-windows-amd64.exe` as a first-class release asset and document manual download/PATH installation. The existing POSIX shell installer is not a Windows installer. | -| Cutover | No separate human release-approval gate. The normal tag-triggered release proceeds only after `CHK-01` through `CHK-03` are green. | - -### Resolution rationale +Keep two semantic modes: `restricted` and `full-delivery`. `restricted` is the +built-in default. Full delivery requires an explicit CLI/environment selection +and a visible warning before Codex starts. -The requester directly chose macOS, Linux, and Windows and delegated release-strategy selection to this feature. The selected GoReleaser layout and target architecture set are grounded in the referenced `dapi/port-selector` repository: its `.goreleaser.yml` uses the exact five targets, `CGO_ENABLED=0`, binary-format archives, and `checksums.txt`; its installer performs POSIX OS/architecture detection. The decision preserves explicit asset integrity while avoiding a false claim that the POSIX installer supports Windows. +### FPF rationale -### Rejected alternatives +The launcher capability boundary is a separate bounded context from task-level +approval decisions. Least privilege is the selection criterion: absent an +explicit user choice, preserve the existing restricted behavior. The semantic +names keep the public contract independent from Codex's low-level flags. -- A single cross-platform `start-issue` Go asset is rejected: compiled Go executables are platform-specific. -- A narrower target matrix is rejected: the requester selected all three operating systems and the referenced strategy supplies the matching explicit matrix. -- A release approval gate is rejected: the requester explicitly said it is unnecessary; automated evidence gates remain mandatory. +## DL-02 — Full-delivery command mapping -## DL-02 — Go toolchain and Windows update boundary +**Status:** accepted by FPF review on 2026-08-04, pending live verification. -**Status:** accepted on 2026-07-22 by feature owner under delegated release-strategy choice. +### Evidence -### FPF framing and facts - -- The toolchain is an execution-environment contract, not a user-facing CLI capability; it must be deterministic in local, CI, and release paths. -- The referenced `dapi/port-selector` release pattern pins `go 1.21` in `go.mod` and GitHub Actions. This repository currently has no Go toolchain contract. -- A POSIX process can replace its executable through the existing install/update style; Windows generally locks a running executable. The referenced release strategy documents a Windows binary download rather than a shell installer. +- The official Codex `exec` CLI source marks + `dangerously_bypass_approvals_and_sandbox` as a global option for `exec`. +- The same source marks `model`, `json`, and `output-last-message` as global + options compatible with the `exec` command. +- The local repository has no installed `codex` executable, so live parser + validation cannot be performed in this worktree. ### Decision -1. Pin Go `1.24` in `go.mod`, `mise.toml`, and CI/release setup for this migration. -2. The initial Windows contract is binary release plus manual installation and manual update: `start-issue update` on Windows must not try to overwrite its running `.exe`; it returns a clear instruction naming the matching release asset. POSIX retains verified automatic install/update behavior. +Map `full-delivery` to: + +```text +codex [--model MODEL] --dangerously-bypass-approvals-and-sandbox exec \ + --cd WORKTREE --json --output-last-message PATH - +``` -### Rationale and risk control +Keep restricted mode on the existing `codex exec --cd WORKTREE +--sandbox workspace-write --json --output-last-message PATH -` path. Do not +use `--ask-for-approval` in the generated command because issue #37 identifies +that spelling/placement as the compatibility failure under investigation. -Go 1.24 is the explicit baseline because its linker emits a Mach-O `LC_UUID`, which current macOS releases require. The Windows manual-update behavior avoids an unsafe or undeclared helper-process design. It is a documented platform-specific delivery difference, not a hidden parity exception, because the Bash baseline has no Windows runtime contract. +### Rationale and limits -## DL-03 — ID-01 dry-run worktree-path conflict handling +The bypass mapping is the only documented current CLI mechanism found that +explicitly covers both approvals and sandboxing. It is intentionally treated +as a high-risk capability switch, not as authorization for destructive or +production actions. `CHK-03` and `AG-01` remain mandatory before acceptance. -**Status:** accepted on 2026-07-24 by the feature requester. +## DL-03 — Current implementation grounding -### Case and approved expectation +**Status:** accepted by FPF review on 2026-08-04. -- **Stable case ID:** `ID-01` -- **Parity case:** `worktree-path-conflict-dry-run` in - `cmd/start-issue/parity_integration_test.go` -- **Bash baseline expectation:** accepts the supplied conflict choice during - `--dry-run` and reports `Worktree path already exists` before continuing - down the selected reuse path. -- **Go expectation:** reports `Worktree path exists; would prompt for reuse or - delete/recreate` without consuming a choice or selecting a reuse/delete - outcome. +The feature package must target the current Go implementation under +`cmd/start-issue/`, its Go tests, `test/e2e/human-gate.sh`, `Makefile`, and the +README/spec documentation. The earlier references to `scripts/lib/start_issue` +and Bats were stale artifacts from the pre-Go implementation and have been +removed from the execution plan. -### User-visible rationale and acceptance +## Open evidence gate -When a worktree path conflicts, the Go dry-run tells the user that a choice is -still required. This avoids presenting one stdin-supplied choice as the -determined outcome of a non-executing command and, importantly, avoids the -legacy path in which the delete/recreate selection can reach mutation logic -before Bash's later dry-run check. The different dry-run diagnostic is -user-visible and is intentionally accepted for `ID-01`; all other observable -records, fake-command logs, and filesystem state remain subject to `CHK-01` -parity. +The exact supported release/version matrix is not asserted locally. Before +`delivery_status: done`, the approved live Codex executable must accept both +command forms and a retained E2E artifact must prove the declared full-delivery +behavior. If that verification fails, reject `full-delivery` and keep the +restricted path as the safe fallback. diff --git a/memory-bank/features/FT-017/design.md b/memory-bank/features/FT-017/design.md index 73fba04..7350899 100644 --- a/memory-bank/features/FT-017/design.md +++ b/memory-bank/features/FT-017/design.md @@ -31,10 +31,12 @@ capability boundary around that lifecycle. The solution must keep restricted behavior safe by default while giving an operator one deliberate, visible way to authorize end-to-end Git delivery. -The local reference CLI is Codex `0.145.0`. Its approval and sandbox flags are -global options, while JSONL and last-message outputs are `exec` options. The -design therefore needs a stable semantic contract owned by `start-issue`, not -an unchecked string of arbitrary Codex arguments. +Issue #37 reproduces the obsolete `--ask-for-approval` placement with Codex +`0.144.6`. The current upstream Codex `exec` source exposes +`--dangerously-bypass-approvals-and-sandbox` as a global option and keeps the +JSONL/last-message contract on `exec`. The design therefore owns a semantic +contract rather than passing arbitrary Codex arguments; the exact approved +release remains a live-verification concern. ## C4 Applicability @@ -66,10 +68,9 @@ enforces credentials and repository authorization independently. - `SOL-01` Add one semantic configuration axis named human-gate permissions with exactly two values: `restricted` and `full-delivery`. Resolve it as CLI option → environment variable → built-in `restricted`. -- `SOL-02` Map `restricted` to Codex global options - `--ask-for-approval never --sandbox workspace-write` and map explicit - `full-delivery` to - `--ask-for-approval never --sandbox danger-full-access`. +- `SOL-02` Keep `restricted` on the existing `--sandbox workspace-write` + command and map explicit `full-delivery` to the global + `--dangerously-bypass-approvals-and-sandbox` option. - `SOL-03` Build the command in supported grammar order: `codex`, global model and permission options, `exec`, then worktree and batch-output options. - `SOL-04` Print the resolved semantic mode and a concise capability statement @@ -96,8 +97,8 @@ enforces credentials and repository authorization independently. | Trade-off ID | Decision | Benefit | Cost / Risk | | --- | --- | --- | --- | | `TRD-01` | Expose two semantic modes instead of raw Codex controls | Small, testable public contract with stable operator meaning | Advanced Codex policies are not expressible through this feature. | -| `TRD-02` | Use `danger-full-access` for explicit full delivery | Provides network and Git metadata writes required by the delivery contract | Batch commands are unsandboxed and must be treated as high risk. | -| `TRD-03` | Keep `never` approval for batch execution | Preserves unattended human-gate semantics | Capability errors cannot escalate mid-run and must be diagnosed clearly. | +| `TRD-02` | Use the explicit Codex bypass option for full delivery | Covers the approvals and sandbox boundaries implicated by issue #37 | Batch commands are unsandboxed and must be treated as high risk. | +| `TRD-03` | Use the explicit bypass mode for unattended batch execution | Preserves unattended human-gate semantics for the selected full-delivery mode | Capability errors cannot escalate mid-run and must be diagnosed clearly. | ## Accepted Local Decisions @@ -120,8 +121,8 @@ enforces credentials and repository authorization independently. | --- | --- | --- | --- | | `CTR-01` | `--human-gate-permissions restricted\|full-delivery` | CLI parser / config resolver | CLI value wins over environment; invalid or empty explicit values fail before issue fetch. | | `CTR-02` | `START_ISSUE_HUMAN_GATE_PERMISSIONS` | shell environment / config resolver | Used only when CLI input is absent; unset resolves to `restricted`. | -| `CTR-03` | Restricted Codex command | launcher / Codex | `codex [--model MODEL] --ask-for-approval never --sandbox workspace-write exec --cd WORKTREE --json --output-last-message PATH -`. | -| `CTR-04` | Full-delivery Codex command | launcher / Codex | Same shape as `CTR-03`, with `--sandbox danger-full-access`; selected only by explicit `full-delivery`. | +| `CTR-03` | Restricted Codex command | launcher / Codex | `codex [--model MODEL] exec --cd WORKTREE --sandbox workspace-write --json --output-last-message PATH -`. | +| `CTR-04` | Full-delivery Codex command | launcher / Codex | `codex [--model MODEL] --dangerously-bypass-approvals-and-sandbox exec --cd WORKTREE --json --output-last-message PATH -`; selected only by explicit `full-delivery`. | | `CTR-05` | Permission status output | launcher / operator | Reports semantic mode and capability boundary before execution and in dry-run; full delivery includes an unsandboxed-execution warning. | ## Invariants diff --git a/memory-bank/features/FT-017/implementation-plan.md b/memory-bank/features/FT-017/implementation-plan.md index 757d928..e0e798a 100644 --- a/memory-bank/features/FT-017/implementation-plan.md +++ b/memory-bank/features/FT-017/implementation-plan.md @@ -35,13 +35,9 @@ existing FT-015 batch, state, status, and resume behavior. | Path / module | Current role | Why relevant | Reuse / mirror | | --- | --- | --- | --- | -| `scripts/start-issue` | Initializes shared CLI state and sources modules | New resolved permission state needs a safe default | Follow existing agent/model state initialization | -| `scripts/lib/start_issue/cli.sh` | Parses public options and validates mode combinations | Owns the new CLI input and early rejection path | Follow `--human-gate`/`--model` value parsing patterns | -| `scripts/lib/start_issue/config.sh` | Resolves config values and sources | Owns CLI/environment/default precedence | Mirror model resolution without adding project/user persistence | -| `scripts/lib/start_issue/agent.sh` | Builds and runs Codex human-gate commands | Main permission mapping and supported grammar change surface | Keep array-based command construction and FT-015 state helpers | -| `scripts/lib/start_issue/output.sh` | Renders help, dry-run, and runtime status | Must expose mode, capabilities, and warning consistently | Extend current human-gate help and dry-run output | -| `test/helpers/fake-bin/codex` | Deterministic Codex command double | Must validate global option order and both sandbox mappings | Extend argument capture/rejection behavior | -| `test/start_issue.bats` | Public CLI and human-gate regression suite | Existing tests cover restricted command, DONE, HUMAN_GATE, and errors | Add precedence, invalid value, full-delivery, and order assertions | +| `cmd/start-issue/main.go` | Go CLI parser, config resolution, launcher, help, and human-gate state | Owns the new option, command mapping, and diagnostics | Extend existing options and array-based `exec.Cmd` construction | +| `cmd/start-issue/main_test.go` | Deterministic Go regression suite | Existing tests cover human-gate command, DONE, HUMAN_GATE, and errors | Add precedence, invalid value, full-delivery, and order assertions | +| `cmd/start-issue/parity_integration_test.go` | Go/Bash observable parity coverage | Protects unaffected legacy behavior during the Go implementation | Keep non-human-gate parity cases green | | `test/e2e/human-gate.sh` | Opt-in real-Codex smoke runner | Closest existing live verification surface | Add a separately guarded full-delivery scenario only after approval | | `README.md`, `README.ru.md`, `doc/spec.md` | Public and canonical behavior docs | Must match help and command behavior | Update together with output assertions | @@ -59,15 +55,15 @@ existing FT-015 batch, state, status, and resume behavior. | Open Question ID | Question | Why unresolved | Blocks | Default action / escalation owner | | --- | --- | --- | --- | --- | -| `OQ-01` | Which future Codex versions remain compatible after `0.145.0`? | The external CLI has no repository-owned stability guarantee. | Does not block implementation; affects future maintenance | Treat `0.145.0` as the tested baseline and update adapter/docs together on command-shape failure. | +| `OQ-01` | Which future Codex versions remain compatible after the issue baseline? | The external CLI has no repository-owned stability guarantee and no local executable is installed. | Does not block deterministic implementation; blocks final live acceptance | Treat the approved live executable as the acceptance baseline and update adapter/docs together on command-shape failure. | | `OQ-02` | Which isolated fixture repository/issue should receive the live full-delivery PR? | Live target selection is operator-owned and may change. | `STEP-06` only | Require explicit target and approval through `AG-01`; never infer from global focus or an unrelated repo. | ## Environment Contract | Area | Contract | Used by | Failure symptom | | --- | --- | --- | --- | -| setup | Bash, Git, jq, shellcheck, Bats, fake agent binaries, and the current modular source tree | `STEP-01` - `STEP-05` | `make test` dependency or fixture failure | -| supported Codex | Local reference is `codex-cli 0.145.0`; approval/sandbox options must be accepted before `exec` | `STEP-03`, `STEP-06` | Real CLI rejects command before emitting `thread.started` | +| setup | Go, Bash, Git, and the current Go source tree; deterministic tests use fakes | `STEP-01` - `STEP-05` | `make test` dependency or fixture failure | +| supported Codex | Issue baseline is `0.144.6`; the approved live executable must accept the recorded command forms | `STEP-03`, `STEP-06` | Real CLI rejects command before emitting `thread.started` | | deterministic test | `make test` is canonical and must not use network or real agent binaries | `CHK-01`, `STEP-02` - `STEP-05` | External side effects or nondeterministic test failures | | live access | Explicit opt-in, authenticated `gh`, real Codex, authorized fixture repo/issue, network, and permission to push/create a PR | `STEP-06` | Missing auth, push rejection, absent PR, or no terminal status | | secrets | Credentials remain in existing authenticated tools/environment and never enter tracked files or command output | all steps | Token-like data appears in diff, logs, or state artifacts | @@ -99,10 +95,10 @@ existing FT-015 batch, state, status, and resume behavior. | Step ID | Actor | Implements | Goal | Touchpoints | Artifact | Verifies | Evidence IDs | Check command / procedure | Blocked by | Needs approval | Escalate if | | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | --- | -| `STEP-01` | agent | `REQ-03`, `SOL-01`, `SD-02`, `CTR-01`, `CTR-02`, `FM-03` | Add CLI/environment/default resolution and fail-fast validation | `scripts/start-issue`, `cli.sh`, `config.sh` | Resolved mode and source | `CHK-01`, `NEG-01` | `EVID-01` | Focused Bats tests, then `make test` | `PRE-01` | none | Validation occurs after fetch or mutation | -| `STEP-02` | agent | `REQ-07`, `INV-01`, `INV-02` | Extend fake Codex and tests before changing launcher behavior | Fake Codex, Bats suite | Red/green command contract tests | `CHK-01`, `SC-01` - `SC-04` | `EVID-01` | `bats test/start_issue.bats` | `STEP-01` | none | Fake cannot distinguish global and exec arguments | -| `STEP-03` | agent | `REQ-01`, `REQ-02`, `REQ-04`, `SOL-02`, `SOL-03`, `CTR-03`, `CTR-04`, `INV-04` | Build validated restricted/full-delivery commands in supported order | `agent.sh` | Array-based Codex command mapping | `CHK-01`, `SC-02`, `SC-03` | `EVID-01` | Focused Bats tests; inspect `--dry-run` command | `STEP-02`, `PRE-02` | none | Supported Codex rejects generated grammar | -| `STEP-04` | agent | `REQ-01`, `REQ-06`, `SOL-04`, `CTR-05`, `FM-02`, `FM-04` | Add capability output, warning, dedicated help, and public docs | `output.sh`, README files, spec | Consistent operator contract | `CHK-02`, `SC-05`, `NEG-02` | `EVID-02` | Help assertions and documentation review | `STEP-03` | none | Docs imply permission equals credentials or product authorization | +| `STEP-01` | agent | `REQ-03`, `SOL-01`, `SD-02`, `CTR-01`, `CTR-02`, `FM-03` | Add CLI/environment/default resolution and fail-fast validation | `cmd/start-issue/main.go`, `main_test.go` | Resolved mode and source | `CHK-01`, `NEG-01` | `EVID-01` | Focused Go tests, then `make test` | `PRE-01` | none | Validation occurs after fetch or mutation | +| `STEP-02` | agent | `REQ-07`, `INV-01`, `INV-02` | Extend the fake Codex process and Go tests before changing launcher behavior | `cmd/start-issue/main_test.go` | Red/green command contract tests | `CHK-01`, `SC-01` - `SC-04` | `EVID-01` | `go test ./cmd/start-issue` | `STEP-01` | none | Fake cannot distinguish global and exec arguments | +| `STEP-03` | agent | `REQ-01`, `REQ-02`, `REQ-04`, `SOL-02`, `SOL-03`, `CTR-03`, `CTR-04`, `INV-04` | Build validated restricted/full-delivery commands in supported order | `cmd/start-issue/main.go` | Array-based Codex command mapping | `CHK-01`, `SC-02`, `SC-03` | `EVID-01` | Focused Go tests; inspect `--dry-run` command | `STEP-02`, `PRE-02` | none | Supported Codex rejects generated grammar | +| `STEP-04` | agent | `REQ-01`, `REQ-06`, `SOL-04`, `CTR-05`, `FM-02`, `FM-04` | Add capability output, warning, dedicated help, and public docs | `cmd/start-issue/main.go`, README files, spec | Consistent operator contract | `CHK-02`, `SC-05`, `NEG-02` | `EVID-02` | Help assertions and documentation review | `STEP-03` | none | Docs imply permission equals credentials or product authorization | | `STEP-05` | agent | `REQ-05`, `REQ-07`, `SOL-05`, `INV-05`, `RB-01`, `RB-02` | Run full deterministic regression and simplify review | All changed runtime/tests/docs | Green local suite and complexity verdict | `CHK-01`, `CHK-02`, `SC-04` | `EVID-01`, `EVID-02`, `EVID-09` | `make test`; inspect diff for unnecessary branches/abstractions | `STEP-01` - `STEP-04` | none | FT-015 state/resume behavior changes | | `STEP-06` | human + agent | `REQ-08`, `SOL-06`, `SD-04`, `SC-06`, `NEG-03`, `RB-03` | Extend/run isolated live full-delivery verification and retain evidence | E2E runner and approved fixture repo/issue | E2E log, state artifacts, commit and PR URL | `CHK-03`, `EC-06` | `EVID-03` | Follow canonical cmux caller-tab procedure and poll to terminal PASS/failure | `STEP-05`, `PRE-03`, `OQ-02` | `AG-01` | Target/auth/caller context is missing, or any unexpected external scope appears | From f7835455445159efa1676bbfa8fed76ca164236e Mon Sep 17 00:00:00 2001 From: Danil Pismenny Date: Tue, 4 Aug 2026 21:07:30 +0300 Subject: [PATCH 2/3] Add Codex human-gate delivery permissions --- README.md | 48 +++++- README.ru.md | 44 ++++- cmd/start-issue/main.go | 86 +++++++++- cmd/start-issue/main_test.go | 160 +++++++++++++++++- doc/spec.md | 41 +++-- memory-bank/features/FT-017/brief.md | 8 +- memory-bank/features/FT-017/decision-log.md | 15 +- .../features/FT-017/implementation-plan.md | 9 +- test/e2e/human-gate.sh | 60 ++++++- 9 files changed, 422 insertions(+), 49 deletions(-) diff --git a/README.md b/README.md index 4330dbe..43c2bf9 100644 --- a/README.md +++ b/README.md @@ -152,6 +152,7 @@ second runtime implementation. | `--prompt-file PATH` | Prompt template file for the selected agent. With `init`, the file content to write. Mutually exclusive with `--prompt`. | | `--improve-prompt` | Ask the selected agent to generate a reviewable improved prompt template proposal, then exit before creating a worktree. | | `--human-gate` | Codex-only batch mode for issue work. Runs `codex exec`, exits on `STATUS: DONE`, and resumes the same session on `STATUS: HUMAN_GATE`. | +| `--human-gate-permissions restricted\|full-delivery` | Select the human-gate capability contract. CLI overrides `START_ISSUE_HUMAN_GATE_PERMISSIONS`; default is `restricted`. | | `--human-gate-help` | Show dedicated help for the Codex human-gate workflow, including prompt contract, exit codes, and state files. | | `--prompt-output-file PATH` | Proposal output path for `--improve-prompt`. | | `--no-init` | Do not run `init.sh` even if it exists in the created worktree. | @@ -182,6 +183,7 @@ Related Claude Code marketplace workflows: | `START_ISSUE_PROMPT` | Inline prompt template used when no CLI prompt is provided. It overrides project and user prompt files. Mutually exclusive with `START_ISSUE_PROMPT_FILE` when no CLI prompt is provided. | | `START_ISSUE_PROMPT_FILE` | Prompt template file used when no CLI prompt is provided. It overrides project and user prompt files. Mutually exclusive with `START_ISSUE_PROMPT` when no CLI prompt is provided. | | `START_ISSUE_WORKTREE_DIR` | Default parent directory for created worktrees when `--worktree-dir` is not provided. Built-in default: `~/worktrees`. | +| `START_ISSUE_HUMAN_GATE_PERMISSIONS` | Human-gate capability contract when the CLI option is absent: `restricted` or `full-delivery`. Built-in default: `restricted`. | | `START_ISSUE_DUMP_PROMPT` | When set to `1`, dry-run output includes the full rendered prompt instead of only summary information. | ## Configuration Files @@ -230,6 +232,35 @@ The batch flow: This mode is intentionally Codex-only. `--human-gate` with any other agent fails clearly instead of being ignored. +Human-gate permissions are explicit: + +- `restricted` is the default. It uses `--sandbox workspace-write` and supports + working-tree edits, but network access, Git metadata writes, push, and PR + delivery are not guaranteed. +- `full-delivery` is an explicit opt-in. It runs Codex with + `--dangerously-bypass-approvals-and-sandbox`, allowing the normal issue + workflow to read GitHub context, edit, test, commit, push, and create or + update a PR when the current `gh` session and repository permissions allow + it. This is unsandboxed execution. + +Select the mode with the CLI (highest precedence), the environment, or the +safe built-in default: + +```bash +start-issue 123 --agent codex --human-gate \ + --human-gate-permissions full-delivery + +START_ISSUE_HUMAN_GATE_PERMISSIONS=full-delivery \ + start-issue 123 --agent codex --human-gate +``` + +Full delivery changes launcher capability only. It does not authorize +destructive Git operations, production/security changes, or product decisions; +the prompt must still return `STATUS: HUMAN_GATE` for those. Before using it, +verify `gh auth status`, the selected account, the remote, and repository write +access. A restricted capability failure should be handled by manual delivery or +an explicit full-delivery rerun, not reported as a task-level product decision. + When the workflow is about to block for a branch/worktree decision, it prints `Waiting for input: ...`. Before handing control to an interactive agent or Codex batch run, it prints `Handing off to in `. A non-zero @@ -283,18 +314,29 @@ test/e2e/human-gate.sh --scenario human-gate Exit the resumed Codex session to let the script verify the artifacts. +To validate actual commit, push, and PR creation in the private fixture, use +the separately authorized unsandboxed scenario. It creates and retains a unique +remote branch and PR as evidence: + +```bash +START_ISSUE_E2E=1 START_ISSUE_E2E_FULL_DELIVERY=1 \ + test/e2e/human-gate.sh --scenario full-delivery +``` + #### Scenarios and checks | Scenario | Command | What it verifies | | --- | --- | --- | | `done` | `START_ISSUE_E2E=1 make e2e-human-gate` | A real Codex batch run emits `thread.started`, saves `thread-id`, `events.jsonl`, and `last-message.txt`, ends with `STATUS: DONE`, and leaves no fixture change other than `.start-issue` state. | | `human-gate` | `START_ISSUE_E2E=1 test/e2e/human-gate.sh --scenario human-gate` | The same artifact and clean-worktree checks, plus the reported explicit `codex resume --include-non-interactive ` handoff. The operator exits the resumed interactive session before the script can finish. | +| `full-delivery` | `START_ISSUE_E2E=1 START_ISSUE_E2E_FULL_DELIVERY=1 test/e2e/human-gate.sh --scenario full-delivery` | The current Codex accepts the global full-delivery option and completes a unique fixture commit, push, and PR; the runner prints the retained PR URL. | -Both scenarios verify authenticated `gh`, a real rather than fake Codex binary, +All scenarios verify authenticated `gh`, a real rather than fake Codex binary, and the required `codex exec` help interface (`--output-last-message`, without the obsolete `--ask-for-approval` flag). The selected Codex executable is -printed in the test output. They do not prove application behavior beyond this -human-gate protocol and are intentionally excluded from CI. +printed in the test output. The `done` and `human-gate` scenarios do not prove +application behavior beyond this protocol; `full-delivery` additionally proves +the explicitly authorized fixture delivery path. All are excluded from CI. ### CI sandbox E2E diff --git a/README.ru.md b/README.ru.md index 886150f..995d859 100644 --- a/README.ru.md +++ b/README.ru.md @@ -77,6 +77,31 @@ Batch flow: Режим намеренно поддерживается только для Codex. `--human-gate` с любым другим agent завершается явной ошибкой. +Права human-gate задаются явно: + +- `restricted` — безопасный default. Он использует `--sandbox workspace-write` + и разрешает редактирование worktree, но не гарантирует network, запись Git + metadata, push и доставку PR. +- `full-delivery` — явный opt-in. Codex запускается с + `--dangerously-bypass-approvals-and-sandbox`, поэтому при корректной `gh` + session и правах репозитория workflow может прочитать GitHub context, + изменить и проверить код, сделать commit/push и создать или обновить PR. + Это unsandboxed execution. + +```bash +start-issue 123 --agent codex --human-gate \ + --human-gate-permissions full-delivery + +START_ISSUE_HUMAN_GATE_PERMISSIONS=full-delivery \ + start-issue 123 --agent codex --human-gate +``` + +Приоритет: CLI, environment, затем `restricted`. Full delivery расширяет только +технические возможности launcher и не разрешает destructive Git operations, +production/security changes или product decisions: для них prompt по-прежнему +обязан вернуть `STATUS: HUMAN_GATE`. Перед запуском проверьте `gh auth status`, +выбранный account, remote и write access к репозиторию. + Перед ожиданием решения по конфликту branch/worktree команда печатает `Waiting for input: ...`, а перед передачей управления agent — `Handing off to in `. Ненулевой код `codex exec` считается @@ -130,18 +155,29 @@ test/e2e/human-gate.sh --scenario human-gate Выйдите из возобновлённой Codex-сессии, после чего скрипт проверит артефакты. +Для реальной проверки commit, push и создания PR в private fixture используется +отдельно подтверждаемый unsandboxed scenario. Он сохраняет уникальные remote +branch и PR как evidence: + +```bash +START_ISSUE_E2E=1 START_ISSUE_E2E_FULL_DELIVERY=1 \ + test/e2e/human-gate.sh --scenario full-delivery +``` + #### Сценарии и проверки | Сценарий | Команда | Что проверяется | | --- | --- | --- | | `done` | `START_ISSUE_E2E=1 make e2e-human-gate` | Реальный Codex batch run выдаёт `thread.started`, сохраняет `thread-id`, `events.jsonl` и `last-message.txt`, заканчивается `STATUS: DONE` и не меняет fixture worktree за пределами `.start-issue` state. | | `human-gate` | `START_ISSUE_E2E=1 test/e2e/human-gate.sh --scenario human-gate` | Те же проверки артефактов и чистоты worktree, а также явный handoff `codex resume --include-non-interactive `. Перед завершением скрипта оператор выходит из возобновлённой interactive session. | +| `full-delivery` | `START_ISSUE_E2E=1 START_ISSUE_E2E_FULL_DELIVERY=1 test/e2e/human-gate.sh --scenario full-delivery` | Текущий Codex принимает global full-delivery option и выполняет уникальные fixture commit, push и PR; runner печатает сохранённый PR URL. | -Оба сценария проверяют авторизованный `gh`, реальный, а не fake Codex binary, и +Все сценарии проверяют авторизованный `gh`, реальный, а не fake Codex binary, и обязательный интерфейс справки `codex exec` (`--output-last-message`, без устаревшего флага `--ask-for-approval`). Выбранный Codex executable печатается -в test output. Они не доказывают поведение приложения за пределами human-gate -protocol и намеренно не входят в CI. +в test output. `done` и `human-gate` не доказывают поведение приложения за +пределами protocol; `full-delivery` дополнительно проверяет явно разрешённую +доставку в fixture. Все сценарии намеренно не входят в CI. ### CI sandbox E2E @@ -253,6 +289,7 @@ boundaries. Новые возможности должны сохранять э | `--prompt-file PATH` | Файл prompt template для выбранного агента. С `init` - содержимое файла, которое нужно записать. Нельзя использовать вместе с `--prompt`. | | `--improve-prompt` | Попросить выбранного агента сгенерировать reviewable proposal улучшенного prompt template и выйти до создания worktree. | | `--human-gate` | Codex-only batch mode для issue workflow. Запускает `codex exec`, выходит на `STATUS: DONE` и резюмирует ту же сессию на `STATUS: HUMAN_GATE`. | +| `--human-gate-permissions restricted\|full-delivery` | Выбрать capability contract human-gate. CLI имеет приоритет над `START_ISSUE_HUMAN_GATE_PERMISSIONS`; default — `restricted`. | | `--human-gate-help` | Показать отдельную справку по Codex human-gate workflow: prompt contract, exit codes и state files. | | `--prompt-output-file PATH` | Путь для proposal-файла в режиме `--improve-prompt`. | | `--no-init` | Не запускать `init.sh`, даже если он есть в созданном worktree. | @@ -283,6 +320,7 @@ boundaries. Новые возможности должны сохранять э | `START_ISSUE_PROMPT` | Inline prompt template, который используется, если prompt не задан через CLI. Перебивает project и user prompt files. Нельзя использовать вместе с `START_ISSUE_PROMPT_FILE`, когда prompt не задан через CLI. | | `START_ISSUE_PROMPT_FILE` | Файл prompt template, который используется, если prompt не задан через CLI. Перебивает project и user prompt files. Нельзя использовать вместе с `START_ISSUE_PROMPT`, когда prompt не задан через CLI. | | `START_ISSUE_WORKTREE_DIR` | Родительская директория по умолчанию для создаваемых worktree, если `--worktree-dir` не передан. Встроенное значение по умолчанию: `~/worktrees`. | +| `START_ISSUE_HUMAN_GATE_PERMISSIONS` | Capability contract human-gate при отсутствии CLI option: `restricted` или `full-delivery`. Built-in default: `restricted`. | | `START_ISSUE_DUMP_PROMPT` | Если задана в `1`, dry-run выводит полный rendered prompt вместо краткой информации. | ## Файлы конфигурации diff --git a/cmd/start-issue/main.go b/cmd/start-issue/main.go index eb0d6bd..4ecff4e 100644 --- a/cmd/start-issue/main.go +++ b/cmd/start-issue/main.go @@ -59,6 +59,7 @@ func versionFromBuildInfo(info *debug.BuildInfo, fallback ...string) string { type options struct { repo, base, worktreeDir, agent, model, promptFile, prompt, command string promptOutput, worktreeDirSource string + humanGatePermissions, humanGatePermissionsSource string issue string dryRun, noInit, flat, ai, improvePrompt, humanGate, project, user, force bool mode string @@ -138,10 +139,18 @@ func main() { } func parse(args []string) (options, error) { - o := options{worktreeDir: os.Getenv("START_ISSUE_WORKTREE_DIR")} + o := options{ + worktreeDir: os.Getenv("START_ISSUE_WORKTREE_DIR"), + humanGatePermissions: "restricted", + humanGatePermissionsSource: "built-in default", + } if o.worktreeDir != "" { o.worktreeDirSource = "START_ISSUE_WORKTREE_DIR" } + if permissions := os.Getenv("START_ISSUE_HUMAN_GATE_PERMISSIONS"); permissions != "" { + o.humanGatePermissions = permissions + o.humanGatePermissionsSource = "START_ISSUE_HUMAN_GATE_PERMISSIONS" + } var err error for len(args) > 0 { a := args[0] @@ -199,6 +208,11 @@ func parse(args []string) (options, error) { o.promptOutput, err = value() case "--human-gate": o.humanGate = true + case "--human-gate-permissions": + o.humanGatePermissions, err = value() + if err == nil { + o.humanGatePermissionsSource = "CLI" + } case "--project": o.project = true case "--user": @@ -239,6 +253,9 @@ func parse(args []string) (options, error) { if o.mode != "" && o.issue != "" { return o, fmt.Errorf("Use either %s or , not both.", o.mode) } + if !validHumanGatePermissions(o.humanGatePermissions) { + return o, fmt.Errorf("Invalid human-gate permissions %q. Use restricted or full-delivery.", o.humanGatePermissions) + } if o.worktreeDir == "" && o.mode == "" { home, err := userHomeDir() if err != nil { @@ -250,6 +267,10 @@ func parse(args []string) (options, error) { return o, nil } +func validHumanGatePermissions(value string) bool { + return value == "restricted" || value == "full-delivery" +} + func userHomeDir() (string, error) { home, err := os.UserHomeDir() if err != nil { @@ -425,7 +446,7 @@ func runWithReader(o options, reader *bufio.Reader) error { if o.dryRun { fmt.Printf(" [DRY-RUN] Would run: git worktree add -b %s %s %s\n", branch, worktree, o.base) if o.humanGate { - return humanGate(model, worktree, rendered, true) + return humanGate(model, worktree, rendered, o.humanGatePermissions, o.humanGatePermissionsSource, true) } return launchSelected(options{dryRun: true}, agent, model, worktree, rendered) } @@ -1743,7 +1764,7 @@ func canonicalPath(path string) string { func launchSelected(o options, agent, model, worktree, prompt string) error { if o.dryRun { if o.humanGate { - return humanGate(model, worktree, prompt, true) + return humanGate(model, worktree, prompt, o.humanGatePermissions, o.humanGatePermissionsSource, true) } if agent == "none" { printManualNextSteps(model, worktree) @@ -1756,7 +1777,7 @@ func launchSelected(o options, agent, model, worktree, prompt string) error { if !o.dryRun { printAgentHandoff(agent, worktree) } - return humanGate(model, worktree, prompt, false) + return humanGate(model, worktree, prompt, o.humanGatePermissions, o.humanGatePermissionsSource, false) } if !o.dryRun && agent != "none" { printAgentHandoff(agent, worktree) @@ -1824,16 +1845,21 @@ func normalizePromptProposal(result string) string { } return strings.TrimSpace(strings.Join(lines, "\n")) } -func humanGate(model, worktree, prompt string, dryRun bool) error { +func humanGate(model, worktree, prompt, permissions, permissionsSource string, dryRun bool) error { runID := os.Getenv("START_ISSUE_RUN_ID") if runID == "" { runID = time.Now().Format("20060102-150405") } dir := filepath.Join(worktree, ".start-issue", "runs", runID) events, last := filepath.Join(dir, "events.jsonl"), filepath.Join(dir, "last-message.txt") - args := []string{"exec", "--cd", worktree, "--sandbox", "workspace-write", "--json", "--output-last-message", last, "-"} - if model != "" { - args = append([]string{"exec", "--model", model}, args[1:]...) + args := humanGateArgs(model, worktree, last, permissions) + fmt.Printf(" State dir: %s\n", dir) + fmt.Printf(" Human-gate permissions: %s (%s)\n", permissions, permissionsSource) + if permissions == "full-delivery" { + fmt.Println(" WARNING: Codex will run without approvals or sandboxing for GitHub and Git delivery.") + fmt.Println(" Requires authenticated GitHub access and repository write permission; destructive or production actions still require HUMAN_GATE.") + } else { + fmt.Println(" Restricted mode: working-tree edits only; network, Git metadata writes, push, and PR delivery are not guaranteed.") } if dryRun { threadID := filepath.Join(dir, "thread-id") @@ -1888,6 +1914,21 @@ func humanGate(model, worktree, prompt string, dryRun bool) error { return fmt.Errorf("No recognized final status found. Inspect: %s", last) } +func humanGateArgs(model, worktree, lastMessage, permissions string) []string { + args := []string{} + if model != "" { + args = append(args, "--model", model) + } + if permissions == "full-delivery" { + args = append(args, "--dangerously-bypass-approvals-and-sandbox") + } + args = append(args, "exec", "--cd", worktree) + if permissions == "restricted" { + args = append(args, "--sandbox", "workspace-write") + } + return append(args, "--json", "--output-last-message", lastMessage, "-") +} + func captureThreadID(events string) (string, error) { eventsBody, err := os.ReadFile(events) if err != nil { @@ -2159,6 +2200,9 @@ Options: --improve-prompt Ask the selected agent to improve the selected prompt template and write a reviewable proposal --human-gate Codex-only batch mode that resumes on HUMAN_GATE + --human-gate-permissions + Permission contract for --human-gate + Default: START_ISSUE_HUMAN_GATE_PERMISSIONS or restricted --human-gate-help Show detailed help for the human-gate mode --prompt-output-file Output path for --improve-prompt proposal @@ -2215,6 +2259,7 @@ Environment variables: START_ISSUE_PROMPT START_ISSUE_PROMPT_FILE START_ISSUE_WORKTREE_DIR + START_ISSUE_HUMAN_GATE_PERMISSIONS START_ISSUE_DUMP_PROMPT Examples: @@ -2224,6 +2269,7 @@ Examples: start-issue 123 --agent codex start-issue 123 --agent codex --model gpt-5.2 start-issue 123 --agent codex --human-gate + start-issue 123 --agent codex --human-gate --human-gate-permissions full-delivery start-issue 123 --agent claude --model sonnet start-issue 123 --agent kimi --prompt-file .start-issue/prompt.md start-issue 123 --no-agent # Only create worktree @@ -2254,8 +2300,28 @@ func humanGateHelp() { Usage: start-issue --agent codex --human-gate + start-issue --agent codex --human-gate \ + --human-gate-permissions full-delivery start-issue --human-gate-help +Permission modes: + restricted (default) + Uses Codex workspace-write sandboxing. Working-tree edits are supported, + but network access, Git metadata writes, push, and PR delivery are not + guaranteed. Select with START_ISSUE_HUMAN_GATE_PERMISSIONS=restricted or + --human-gate-permissions restricted. + + full-delivery (explicit opt-in) + Runs Codex with --dangerously-bypass-approvals-and-sandbox so a normal + issue workflow can read GitHub context, edit, test, commit, push, and + create or update a PR. This is unsandboxed execution. It requires an + authenticated gh session and repository write permission. It does not + authorize destructive, production, security, or product decisions; those + still require STATUS: HUMAN_GATE. + +Precedence: + --human-gate-permissions, START_ISSUE_HUMAN_GATE_PERMISSIONS, restricted. + Flow: The normal issue workflow creates or reuses the worktree, renders the prompt, and runs Codex in batch mode. The final message must contain one @@ -2280,6 +2346,10 @@ Final status examples: Troubleshooting: Inspect events.jsonl and last-message.txt when batch parsing fails. The explicit thread id is saved before status handling when available. + If restricted mode cannot read GitHub or write Git metadata, either finish + delivery manually or explicitly select full-delivery after reviewing its risk. + If full delivery cannot push or create a PR, verify gh auth status, the + selected GitHub account, remote URL, and repository permissions. If automatic resume fails, run: codex resume --include-non-interactive `) } diff --git a/cmd/start-issue/main_test.go b/cmd/start-issue/main_test.go index cace48e..0e64277 100644 --- a/cmd/start-issue/main_test.go +++ b/cmd/start-issue/main_test.go @@ -58,6 +58,44 @@ func TestParseTracksWorktreeDirectorySource(t *testing.T) { } } +func TestParseHumanGatePermissionsPrecedenceAndValidation(t *testing.T) { + t.Setenv("HOME", t.TempDir()) + t.Setenv("START_ISSUE_HUMAN_GATE_PERMISSIONS", "") + + o, err := parse([]string{"1"}) + if err != nil { + t.Fatal(err) + } + if o.humanGatePermissions != "restricted" || o.humanGatePermissionsSource != "built-in default" { + t.Fatalf("default permissions = %q (%s)", o.humanGatePermissions, o.humanGatePermissionsSource) + } + + t.Setenv("START_ISSUE_HUMAN_GATE_PERMISSIONS", "full-delivery") + o, err = parse([]string{"1"}) + if err != nil { + t.Fatal(err) + } + if o.humanGatePermissions != "full-delivery" || o.humanGatePermissionsSource != "START_ISSUE_HUMAN_GATE_PERMISSIONS" { + t.Fatalf("environment permissions = %q (%s)", o.humanGatePermissions, o.humanGatePermissionsSource) + } + + o, err = parse([]string{"1", "--human-gate-permissions", "restricted"}) + if err != nil { + t.Fatal(err) + } + if o.humanGatePermissions != "restricted" || o.humanGatePermissionsSource != "CLI" { + t.Fatalf("CLI permissions = %q (%s)", o.humanGatePermissions, o.humanGatePermissionsSource) + } + + if _, err := parse([]string{"1", "--human-gate-permissions", "unlimited"}); err == nil || !strings.Contains(err.Error(), "Use restricted or full-delivery") { + t.Fatalf("invalid CLI permissions error = %v", err) + } + t.Setenv("START_ISSUE_HUMAN_GATE_PERMISSIONS", "unlimited") + if _, err := parse([]string{"1"}); err == nil || !strings.Contains(err.Error(), "Use restricted or full-delivery") { + t.Fatalf("invalid environment permissions error = %v", err) + } +} + func TestUserHomeDirRejectsUnavailableOrRelativeHome(t *testing.T) { t.Setenv("HOME", "") if runtime.GOOS != "windows" { @@ -1117,6 +1155,8 @@ func TestUsageListsCompatibilityEntryPoints(t *testing.T) { "--update", "--install", "--human-gate-help", + "--human-gate-permissions ", + "START_ISSUE_HUMAN_GATE_PERMISSIONS", "Agent selection precedence:", ".start-issue/agent in the git root", "Prompt template precedence:", @@ -1129,6 +1169,24 @@ func TestUsageListsCompatibilityEntryPoints(t *testing.T) { } } +func TestHumanGateHelpExplainsPermissionContract(t *testing.T) { + output := captureStdout(t, humanGateHelp) + for _, want := range []string{ + "restricted (default)", + "full-delivery (explicit opt-in)", + "--dangerously-bypass-approvals-and-sandbox", + "authenticated gh session", + "repository write permission", + "destructive, production, security, or product decisions", + "START_ISSUE_HUMAN_GATE_PERMISSIONS", + "gh auth status", + } { + if !strings.Contains(output, want) { + t.Fatalf("human-gate help missing %q:\n%s", want, output) + } + } +} + func TestAIBranchPromptPreservesTransliterationAndTagConstraints(t *testing.T) { bin, log := t.TempDir(), filepath.Join(t.TempDir(), "prompt") writeExecutable(t, filepath.Join(bin, "pi"), "#!/bin/sh\nlast=''\nfor arg do last=$arg; done\nprintf '%s' \"$last\" > '"+log+"'\nprintf '%s\\n' feature/issue-34-ispravit-tsap\n") @@ -1772,7 +1830,7 @@ func TestHumanGateSavesThreadIDBeforeDone(t *testing.T) { t.Setenv("CODEX_LAST", "STATUS: DONE") t.Setenv("START_ISSUE_FAKE_CODEX_REJECT_ASK_FOR_APPROVAL", "1") - if err := humanGate("", worktree, "prompt", false); err != nil { + if err := humanGate("", worktree, "prompt", "restricted", "built-in default", false); err != nil { t.Fatal(err) } threadID, err := os.ReadFile(filepath.Join(worktree, ".start-issue", "runs", "done", "thread-id")) @@ -1789,7 +1847,7 @@ func TestHumanGateSavesThreadIDWhenFinalMessageIsMissing(t *testing.T) { t.Setenv("CODEX_EVENTS", `{"type":"thread.started","thread_id":"thread-recovery"}`) t.Setenv("CODEX_SKIP_LAST", "1") - err := humanGate("", worktree, "prompt", false) + err := humanGate("", worktree, "prompt", "restricted", "built-in default", false) if err == nil || !strings.Contains(err.Error(), "No recognized final status found") { t.Fatalf("humanGate error = %v, want missing final-status error", err) } @@ -1808,7 +1866,7 @@ func TestHumanGateExecFailureReturnsExitCodeOne(t *testing.T) { t.Setenv("CODEX_LAST", "STATUS: DONE") t.Setenv("CODEX_EXEC_EXIT", "42") - err := humanGate("", worktree, "prompt", false) + err := humanGate("", worktree, "prompt", "restricted", "built-in default", false) var exit exitError if !errors.As(err, &exit) || exit.code != 1 { t.Fatalf("got %T %v, want human-gate exit code 1", err, err) @@ -1836,11 +1894,13 @@ func TestHumanGateDryRunShowsAllStateArtifacts(t *testing.T) { t.Setenv("START_ISSUE_RUN_ID", "plan") dir := filepath.Join(worktree, ".start-issue", "runs", "plan") output := captureStdout(t, func() { - if err := humanGate("", worktree, "prompt", true); err != nil { + if err := humanGate("", worktree, "prompt", "restricted", "built-in default", true); err != nil { t.Fatal(err) } }) for _, want := range []string{ + "Human-gate permissions: restricted (built-in default)", + "Restricted mode: working-tree edits only", "--output-last-message " + filepath.Join(dir, "last-message.txt"), "> " + filepath.Join(dir, "events.jsonl"), "Would write captured thread ID: " + filepath.Join(dir, "thread-id"), @@ -1857,6 +1917,73 @@ func TestHumanGateDryRunShowsAllStateArtifacts(t *testing.T) { } } +func TestHumanGateArgsMapPermissionModesInSupportedOrder(t *testing.T) { + worktree := "/tmp/worktree" + last := "/tmp/last-message.txt" + restricted := humanGateArgs("gpt-test", worktree, last, "restricted") + if got, want := fmt.Sprint(restricted), "[--model gpt-test exec --cd /tmp/worktree --sandbox workspace-write --json --output-last-message /tmp/last-message.txt -]"; got != want { + t.Fatalf("restricted args = %s, want %s", got, want) + } + fullDelivery := humanGateArgs("gpt-test", worktree, last, "full-delivery") + if got, want := fmt.Sprint(fullDelivery), "[--model gpt-test --dangerously-bypass-approvals-and-sandbox exec --cd /tmp/worktree --json --output-last-message /tmp/last-message.txt -]"; got != want { + t.Fatalf("full-delivery args = %s, want %s", got, want) + } +} + +func TestHumanGateFullDeliveryDryRunShowsResolvedModeAndCommand(t *testing.T) { + worktree := t.TempDir() + t.Setenv("START_ISSUE_RUN_ID", "full-delivery-plan") + output := captureStdout(t, func() { + err := launchSelected(options{ + dryRun: true, + humanGate: true, + humanGatePermissions: "full-delivery", + humanGatePermissionsSource: "CLI", + }, "codex", "gpt-test", worktree, "prompt") + if err != nil { + t.Fatal(err) + } + }) + wantCommand := "codex --model gpt-test --dangerously-bypass-approvals-and-sandbox exec --cd " + for _, want := range []string{ + "Human-gate permissions: full-delivery (CLI)", + wantCommand, + "WARNING: Codex will run without approvals or sandboxing", + } { + if !strings.Contains(output, want) { + t.Fatalf("full-delivery dry-run missing %q:\n%s", want, output) + } + } + if strings.Contains(output, "--sandbox workspace-write") { + t.Fatalf("full-delivery dry-run retained restricted sandbox:\n%s", output) + } +} + +func TestHumanGateFullDeliveryReportsWarningAndCompletes(t *testing.T) { + worktree, bin := t.TempDir(), t.TempDir() + writeFakeCodex(t, bin) + t.Setenv("PATH", bin+string(os.PathListSeparator)+os.Getenv("PATH")) + t.Setenv("START_ISSUE_RUN_ID", "full-delivery") + t.Setenv("CODEX_EVENTS", `{"type":"thread.started","thread_id":"thread-full-delivery"}`) + t.Setenv("CODEX_LAST", "STATUS: DONE") + + output := captureStdout(t, func() { + if err := humanGate("gpt-test", worktree, "prompt", "full-delivery", "CLI", false); err != nil { + t.Fatal(err) + } + }) + for _, want := range []string{ + "Human-gate permissions: full-delivery (CLI)", + "WARNING: Codex will run without approvals or sandboxing", + "destructive or production actions still require HUMAN_GATE", + "STATUS: DONE", + } { + if !strings.Contains(output, want) { + t.Fatalf("full-delivery output missing %q:\n%s", want, output) + } + } +} + func TestHumanGatePreservesCallerWorkingDirectory(t *testing.T) { worktree, bin, log := t.TempDir(), t.TempDir(), filepath.Join(t.TempDir(), "cwd") writeFakeCodex(t, bin) @@ -1869,7 +1996,7 @@ func TestHumanGatePreservesCallerWorkingDirectory(t *testing.T) { if err != nil { t.Fatal(err) } - if err := humanGate("", worktree, "prompt", false); err != nil { + if err := humanGate("", worktree, "prompt", "restricted", "built-in default", false); err != nil { t.Fatal(err) } got, err := os.ReadFile(log) @@ -1889,7 +2016,7 @@ func TestHumanGateRejectsDoneWithoutThreadID(t *testing.T) { t.Setenv("CODEX_EVENTS", `{"type":"item.completed"}`) t.Setenv("CODEX_LAST", "STATUS: DONE") - err := humanGate("", worktree, "prompt", false) + err := humanGate("", worktree, "prompt", "restricted", "built-in default", false) if err == nil || !strings.Contains(err.Error(), "did not capture thread_id") { t.Fatalf("got %v", err) } @@ -1904,7 +2031,7 @@ func TestHumanGateResumeFailureReturnsExitCodeTwo(t *testing.T) { t.Setenv("CODEX_LAST", "STATUS: HUMAN_GATE") t.Setenv("CODEX_RESUME_EXIT", "1") - err := humanGate("", worktree, "prompt", false) + err := humanGate("", worktree, "prompt", "restricted", "built-in default", false) var exit exitError if !errors.As(err, &exit) || exit.code != 2 { t.Fatalf("got %T %v", err, err) @@ -1928,9 +2055,28 @@ fi if [ -n "$START_ISSUE_CWD_LOG" ]; then pwd > "$START_ISSUE_CWD_LOG" fi +while [ "$#" -gt 0 ] && [ "$1" != "exec" ] && [ "$1" != "resume" ]; do + case "$1" in + --model) + shift 2 + ;; + --dangerously-bypass-approvals-and-sandbox) + shift + ;; + *) + printf '%s\n' "unexpected global option: $1" >&2 + exit 1 + ;; + esac +done if [ "$1" = "exec" ]; then + shift last="" while [ "$#" -gt 0 ]; do + if [ "$1" = "--dangerously-bypass-approvals-and-sandbox" ]; then + printf '%s\n' "full-delivery option must precede exec" >&2 + exit 1 + fi if [ "$1" = "--output-last-message" ]; then last="$2" shift 2 diff --git a/doc/spec.md b/doc/spec.md index 20ad2d7..c56381b 100644 --- a/doc/spec.md +++ b/doc/spec.md @@ -72,6 +72,7 @@ Agent-specific behavior должен быть централизован за е | `--prompt-file` | Файл prompt template | См. приоритет prompt | | `--improve-prompt` | Сгенерировать reviewable proposal улучшенного prompt template и выйти до создания worktree | false | | `--human-gate` | Codex-only batch mode для issue workflow с resume на `STATUS: HUMAN_GATE` | false | +| `--human-gate-permissions ` | Capability contract для human-gate; CLI имеет приоритет над `START_ISSUE_HUMAN_GATE_PERMISSIONS` | `restricted` | | `--human-gate-help` | Показать отдельную справку по human-gate mode | false | | `--prompt-output-file` | Путь proposal-файла для `--improve-prompt` | Для `.md`: рядом с source как `*.improved.md`; для остальных файлов: `.improved`; иначе `.start-issue/prompt.improved.md` | | `--no-init` | Пропустить запуск `init.sh` | false | @@ -202,11 +203,14 @@ git rev-parse --show-toplevel 1. Режим валиден только для `agent=codex`; для остальных agent он завершается явной ошибкой. 2. До agent launch workflow остается обычным: parse input, resolve config, fetch issue, plan branch, create/reuse worktree, run optional `init.sh`, render prompt. -3. Вместо интерактивного Codex launch выполняется: +3. Permission mode разрешается в порядке CLI + `--human-gate-permissions`, `START_ISSUE_HUMAN_GATE_PERMISSIONS`, built-in + `restricted`. Другие значения отклоняются до issue fetch и worktree mutation. +4. В restricted mode вместо интерактивного Codex launch выполняется: ```bash -codex exec \ - [--model "$MODEL"] \ +codex [--model "$MODEL"] \ + exec \ --cd "$WORKTREE_PATH" \ --sandbox workspace-write \ --json \ @@ -214,21 +218,36 @@ codex exec \ - ``` -4. Rendered prompt передается в `codex exec` через stdin. -5. Из JSONL event stream извлекается `thread_id` из события `thread.started`. -6. Saved `last-message.txt` является единственным источником final status. -7. Поддерживаются только два terminal status: +5. В explicit full-delivery mode выполняется: + +```bash +codex [--model "$MODEL"] \ + --dangerously-bypass-approvals-and-sandbox \ + exec \ + --cd "$WORKTREE_PATH" \ + --json \ + --output-last-message "$STATE_DIR/last-message.txt" \ + - +``` + +6. Full delivery требует authenticated `gh`, корректный remote и repository + write permission. Это unsandboxed execution, но оно не авторизует destructive, + production/security или product decisions: они остаются `HUMAN_GATE`. +7. Rendered prompt передается в `codex exec` через stdin. +8. Из JSONL event stream извлекается `thread_id` из события `thread.started`. +9. Saved `last-message.txt` является единственным источником final status. +10. Поддерживаются только два terminal status: - `STATUS: DONE` - `STATUS: HUMAN_GATE` -8. На `STATUS: DONE` команда завершается с кодом `0`, не открывая Codex TUI. -9. На `STATUS: HUMAN_GATE` выполняется: +11. На `STATUS: DONE` команда завершается с кодом `0`, не открывая Codex TUI. +12. На `STATUS: HUMAN_GATE` выполняется: ```bash codex resume --include-non-interactive "$thread_id" ``` -10. `codex resume --last` не используется как primary mechanism. -11. `codex exec --ephemeral` не используется, потому что session должна быть resumable. +13. `codex resume --last` не используется как primary mechanism. +14. `codex exec --ephemeral` не используется, потому что session должна быть resumable. Dedicated help доступен через: diff --git a/memory-bank/features/FT-017/brief.md b/memory-bank/features/FT-017/brief.md index 6e08987..339ed3c 100644 --- a/memory-bank/features/FT-017/brief.md +++ b/memory-bank/features/FT-017/brief.md @@ -10,7 +10,7 @@ derived_from: - ../FT-015/feature.md - https://github.com/dapi/start-issue/issues/37 status: active -delivery_status: planned +delivery_status: in_progress audience: humans_and_agents must_not_define: - implementation_sequence @@ -87,9 +87,9 @@ supported Codex CLI while closing the remaining capability-contract gap. ### Constraints / Assumptions - `ASM-01` Issue #37 reproduces the rejected argument order with Codex CLI - `0.144.6`; the feature must verify the exact supported command grammar with - the approved executable before acceptance. The repository does not pin an - installed Codex version. + `0.144.6`. Local parser validation confirms both selected command forms on + Codex CLI `0.145.0`; live full-delivery behavior still requires the explicit + `CHK-03` approval gate. The repository does not pin an installed version. - `ASM-02` Full delivery requires independently configured GitHub authentication and repository authorization; `start-issue` can select a launcher policy but cannot grant those external capabilities. diff --git a/memory-bank/features/FT-017/decision-log.md b/memory-bank/features/FT-017/decision-log.md index a5d3eaf..66bf206 100644 --- a/memory-bank/features/FT-017/decision-log.md +++ b/memory-bank/features/FT-017/decision-log.md @@ -59,8 +59,9 @@ names keep the public contract independent from Codex's low-level flags. `dangerously_bypass_approvals_and_sandbox` as a global option for `exec`. - The same source marks `model`, `json`, and `output-last-message` as global options compatible with the `exec` command. -- The local repository has no installed `codex` executable, so live parser - validation cannot be performed in this worktree. +- The local Codex CLI `0.145.0` accepts both selected command forms through its + parser/help path. No agent session or external GitHub write was performed by + this parser validation. ### Decision @@ -95,8 +96,8 @@ removed from the execution plan. ## Open evidence gate -The exact supported release/version matrix is not asserted locally. Before -`delivery_status: done`, the approved live Codex executable must accept both -command forms and a retained E2E artifact must prove the declared full-delivery -behavior. If that verification fails, reject `full-delivery` and keep the -restricted path as the safe fallback. +The exact future release/version matrix is not asserted locally. Before +`delivery_status: done`, a retained approved E2E artifact must prove the +declared full-delivery behavior on the selected Codex executable. If that +verification fails, reject `full-delivery` and keep the restricted path as the +safe fallback. diff --git a/memory-bank/features/FT-017/implementation-plan.md b/memory-bank/features/FT-017/implementation-plan.md index e0e798a..750e031 100644 --- a/memory-bank/features/FT-017/implementation-plan.md +++ b/memory-bank/features/FT-017/implementation-plan.md @@ -22,6 +22,11 @@ must_not_define: Implement the accepted FT-017 permission-mode contract while preserving the existing FT-015 batch, state, status, and resume behavior. +Deterministic implementation, documentation, command-shape coverage, and local +Codex CLI `0.145.0` parser validation are complete. `STEP-06` remains pending +because the real full-delivery run requires explicit `AG-01` authorization and +creates retained fixture GitHub state. + ## Grounding / Support References | Document | Role in this plan | Facts reused | Conflict action | @@ -55,7 +60,7 @@ existing FT-015 batch, state, status, and resume behavior. | Open Question ID | Question | Why unresolved | Blocks | Default action / escalation owner | | --- | --- | --- | --- | --- | -| `OQ-01` | Which future Codex versions remain compatible after the issue baseline? | The external CLI has no repository-owned stability guarantee and no local executable is installed. | Does not block deterministic implementation; blocks final live acceptance | Treat the approved live executable as the acceptance baseline and update adapter/docs together on command-shape failure. | +| `OQ-01` | Which future Codex versions remain compatible after the issue baseline? | The external CLI has no repository-owned stability guarantee; local parser validation covers `0.145.0` only. | Does not block deterministic implementation; blocks claims about future versions | Treat the approved live executable as the acceptance baseline and update adapter/docs together on command-shape failure. | | `OQ-02` | Which isolated fixture repository/issue should receive the live full-delivery PR? | Live target selection is operator-owned and may change. | `STEP-06` only | Require explicit target and approval through `AG-01`; never infer from global focus or an unrelated repo. | ## Environment Contract @@ -63,7 +68,7 @@ existing FT-015 batch, state, status, and resume behavior. | Area | Contract | Used by | Failure symptom | | --- | --- | --- | --- | | setup | Go, Bash, Git, and the current Go source tree; deterministic tests use fakes | `STEP-01` - `STEP-05` | `make test` dependency or fixture failure | -| supported Codex | Issue baseline is `0.144.6`; the approved live executable must accept the recorded command forms | `STEP-03`, `STEP-06` | Real CLI rejects command before emitting `thread.started` | +| supported Codex | Issue failure baseline is `0.144.6`; local parser validation covers `0.145.0`; the approved live executable must complete the recorded full-delivery flow | `STEP-03`, `STEP-06` | Parser rejection or no `thread.started` event | | deterministic test | `make test` is canonical and must not use network or real agent binaries | `CHK-01`, `STEP-02` - `STEP-05` | External side effects or nondeterministic test failures | | live access | Explicit opt-in, authenticated `gh`, real Codex, authorized fixture repo/issue, network, and permission to push/create a PR | `STEP-06` | Missing auth, push rejection, absent PR, or no terminal status | | secrets | Credentials remain in existing authenticated tools/environment and never enter tracked files or command output | all steps | Token-like data appears in diff, logs, or state artifacts | diff --git a/test/e2e/human-gate.sh b/test/e2e/human-gate.sh index 9f1efa2..780b507 100755 --- a/test/e2e/human-gate.sh +++ b/test/e2e/human-gate.sh @@ -10,12 +10,15 @@ scenario="done" usage() { cat <<'EOF' -Usage: START_ISSUE_E2E=1 test/e2e/human-gate.sh [--scenario done|human-gate] +Usage: START_ISSUE_E2E=1 test/e2e/human-gate.sh [--scenario done|human-gate|full-delivery] Runs start-issue against a real Codex CLI using the private fixture repository dapi/start-issue-e2e-fixture and its control issue #1. It deletes the temporary clone after a successful run; set START_ISSUE_E2E_KEEP=1 to retain it. The HUMAN_GATE scenario opens Codex resume interactively; exit it to continue. +FULL_DELIVERY also requires START_ISSUE_E2E_FULL_DELIVERY=1. It authorizes an +unsandboxed Codex run that creates a unique fixture commit, remote branch, and +pull request. Those remote artifacts are retained as evidence. EOF } @@ -40,8 +43,13 @@ while [[ $# -gt 0 ]]; do esac done -[[ "$scenario" == "done" || "$scenario" == "human-gate" ]] || fail "scenario must be done or human-gate" +[[ "$scenario" == "done" || "$scenario" == "human-gate" || "$scenario" == "full-delivery" ]] || \ + fail "scenario must be done, human-gate, or full-delivery" [[ "${START_ISSUE_E2E:-}" == "1" ]] || fail "set START_ISSUE_E2E=1 to authorize a real Codex session" +if [[ "$scenario" == "full-delivery" ]]; then + [[ "${START_ISSUE_E2E_FULL_DELIVERY:-}" == "1" ]] || \ + fail "set START_ISSUE_E2E_FULL_DELIVERY=1 to authorize unsandboxed GitHub delivery" +fi start_issue_bin="${START_ISSUE_E2E_BINARY:-$repo_root/.build/start-issue}" [[ -x "$start_issue_bin" ]] || fail "start-issue executable not found: $start_issue_bin" @@ -59,23 +67,53 @@ printf '%s' "$codex_exec_help" | grep -Fq -- '--output-last-message' || \ if printf '%s' "$codex_exec_help" | grep -q -- '--ask-for-approval'; then fail "installed codex exec still advertises --ask-for-approval; use a current Codex CLI" fi +if [[ "$scenario" == "full-delivery" ]]; then + codex_help="$(codex --help 2>&1)" || fail "codex --help failed" + printf '%s' "$codex_help" | grep -Fq -- '--dangerously-bypass-approvals-and-sandbox' || \ + fail "resolved codex does not support the full-delivery permission option" +fi fixture_root="$(mktemp -d "${TMPDIR:-/tmp}/start-issue-human-gate.XXXXXX")" fixture_dir="$fixture_root/fixture" worktree_parent="$fixture_root/worktrees" log_path="$fixture_root/e2e.log" expected_status="DONE" +permission_args=() if [[ "$scenario" == "human-gate" ]]; then expected_status="HUMAN_GATE" fi -prompt=$(cat </dev/null || \ + fail "delivery commit does not contain the expected marker" + git -C "$worktree_path" ls-remote --exit-code --heads origin "$delivery_branch" >/dev/null || \ + fail "remote delivery branch is missing: $delivery_branch" + pr_url="$(gh pr list --repo "$fixture_repo" --state open --head "$delivery_branch" --json url --jq '.[0].url // empty')" + [[ -n "$pr_url" ]] || fail "full-delivery pull request is missing for $delivery_branch" + printf 'Full-delivery PR: %s\n' "$pr_url" +fi + unexpected_changes="$(git -C "$worktree_path" status --porcelain | awk '$0 !~ /^\?\? \.start-issue\// { print }')" [[ -z "$unexpected_changes" ]] || fail "fixture worktree has unexpected changes: $unexpected_changes" From f3db24b06807415bf3718b13d0f64cb41c0708de Mon Sep 17 00:00:00 2001 From: Danil Pismenny Date: Tue, 4 Aug 2026 21:19:50 +0300 Subject: [PATCH 3/3] Address human-gate permission review --- README.md | 6 +++--- README.ru.md | 6 +++--- cmd/start-issue/main.go | 6 +++++- cmd/start-issue/main_test.go | 7 +++++-- doc/spec.md | 2 +- memory-bank/features/FT-017/design.md | 2 +- memory-bank/features/FT-017/implementation-plan.md | 2 +- test/e2e/human-gate.sh | 5 +++-- 8 files changed, 22 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 43c2bf9..4d4c29a 100644 --- a/README.md +++ b/README.md @@ -152,7 +152,7 @@ second runtime implementation. | `--prompt-file PATH` | Prompt template file for the selected agent. With `init`, the file content to write. Mutually exclusive with `--prompt`. | | `--improve-prompt` | Ask the selected agent to generate a reviewable improved prompt template proposal, then exit before creating a worktree. | | `--human-gate` | Codex-only batch mode for issue work. Runs `codex exec`, exits on `STATUS: DONE`, and resumes the same session on `STATUS: HUMAN_GATE`. | -| `--human-gate-permissions restricted\|full-delivery` | Select the human-gate capability contract. CLI overrides `START_ISSUE_HUMAN_GATE_PERMISSIONS`; default is `restricted`. | +| `--human-gate-permissions restricted\|full-delivery` | Select the human-gate capability contract. Requires `--human-gate`; CLI overrides `START_ISSUE_HUMAN_GATE_PERMISSIONS`; default is `restricted`. | | `--human-gate-help` | Show dedicated help for the Codex human-gate workflow, including prompt contract, exit codes, and state files. | | `--prompt-output-file PATH` | Proposal output path for `--improve-prompt`. | | `--no-init` | Do not run `init.sh` even if it exists in the created worktree. | @@ -316,7 +316,7 @@ Exit the resumed Codex session to let the script verify the artifacts. To validate actual commit, push, and PR creation in the private fixture, use the separately authorized unsandboxed scenario. It creates and retains a unique -remote branch and PR as evidence: +remote branch, PR, and local diagnostic fixture as evidence: ```bash START_ISSUE_E2E=1 START_ISSUE_E2E_FULL_DELIVERY=1 \ @@ -329,7 +329,7 @@ START_ISSUE_E2E=1 START_ISSUE_E2E_FULL_DELIVERY=1 \ | --- | --- | --- | | `done` | `START_ISSUE_E2E=1 make e2e-human-gate` | A real Codex batch run emits `thread.started`, saves `thread-id`, `events.jsonl`, and `last-message.txt`, ends with `STATUS: DONE`, and leaves no fixture change other than `.start-issue` state. | | `human-gate` | `START_ISSUE_E2E=1 test/e2e/human-gate.sh --scenario human-gate` | The same artifact and clean-worktree checks, plus the reported explicit `codex resume --include-non-interactive ` handoff. The operator exits the resumed interactive session before the script can finish. | -| `full-delivery` | `START_ISSUE_E2E=1 START_ISSUE_E2E_FULL_DELIVERY=1 test/e2e/human-gate.sh --scenario full-delivery` | The current Codex accepts the global full-delivery option and completes a unique fixture commit, push, and PR; the runner prints the retained PR URL. | +| `full-delivery` | `START_ISSUE_E2E=1 START_ISSUE_E2E_FULL_DELIVERY=1 test/e2e/human-gate.sh --scenario full-delivery` | The current Codex accepts the global full-delivery option and completes a unique fixture commit, push, and PR; the runner prints the retained PR URL and local artifact path. | All scenarios verify authenticated `gh`, a real rather than fake Codex binary, and the required `codex exec` help interface (`--output-last-message`, without diff --git a/README.ru.md b/README.ru.md index 995d859..59923a9 100644 --- a/README.ru.md +++ b/README.ru.md @@ -157,7 +157,7 @@ test/e2e/human-gate.sh --scenario human-gate Для реальной проверки commit, push и создания PR в private fixture используется отдельно подтверждаемый unsandboxed scenario. Он сохраняет уникальные remote -branch и PR как evidence: +branch, PR и локальный diagnostic fixture как evidence: ```bash START_ISSUE_E2E=1 START_ISSUE_E2E_FULL_DELIVERY=1 \ @@ -170,7 +170,7 @@ START_ISSUE_E2E=1 START_ISSUE_E2E_FULL_DELIVERY=1 \ | --- | --- | --- | | `done` | `START_ISSUE_E2E=1 make e2e-human-gate` | Реальный Codex batch run выдаёт `thread.started`, сохраняет `thread-id`, `events.jsonl` и `last-message.txt`, заканчивается `STATUS: DONE` и не меняет fixture worktree за пределами `.start-issue` state. | | `human-gate` | `START_ISSUE_E2E=1 test/e2e/human-gate.sh --scenario human-gate` | Те же проверки артефактов и чистоты worktree, а также явный handoff `codex resume --include-non-interactive `. Перед завершением скрипта оператор выходит из возобновлённой interactive session. | -| `full-delivery` | `START_ISSUE_E2E=1 START_ISSUE_E2E_FULL_DELIVERY=1 test/e2e/human-gate.sh --scenario full-delivery` | Текущий Codex принимает global full-delivery option и выполняет уникальные fixture commit, push и PR; runner печатает сохранённый PR URL. | +| `full-delivery` | `START_ISSUE_E2E=1 START_ISSUE_E2E_FULL_DELIVERY=1 test/e2e/human-gate.sh --scenario full-delivery` | Текущий Codex принимает global full-delivery option и выполняет уникальные fixture commit, push и PR; runner печатает сохранённые PR URL и local artifact path. | Все сценарии проверяют авторизованный `gh`, реальный, а не fake Codex binary, и обязательный интерфейс справки `codex exec` (`--output-last-message`, без @@ -289,7 +289,7 @@ boundaries. Новые возможности должны сохранять э | `--prompt-file PATH` | Файл prompt template для выбранного агента. С `init` - содержимое файла, которое нужно записать. Нельзя использовать вместе с `--prompt`. | | `--improve-prompt` | Попросить выбранного агента сгенерировать reviewable proposal улучшенного prompt template и выйти до создания worktree. | | `--human-gate` | Codex-only batch mode для issue workflow. Запускает `codex exec`, выходит на `STATUS: DONE` и резюмирует ту же сессию на `STATUS: HUMAN_GATE`. | -| `--human-gate-permissions restricted\|full-delivery` | Выбрать capability contract human-gate. CLI имеет приоритет над `START_ISSUE_HUMAN_GATE_PERMISSIONS`; default — `restricted`. | +| `--human-gate-permissions restricted\|full-delivery` | Выбрать capability contract human-gate. Требует `--human-gate`; CLI имеет приоритет над `START_ISSUE_HUMAN_GATE_PERMISSIONS`; default — `restricted`. | | `--human-gate-help` | Показать отдельную справку по Codex human-gate workflow: prompt contract, exit codes и state files. | | `--prompt-output-file PATH` | Путь для proposal-файла в режиме `--improve-prompt`. | | `--no-init` | Не запускать `init.sh`, даже если он есть в созданном worktree. | diff --git a/cmd/start-issue/main.go b/cmd/start-issue/main.go index 4ecff4e..0b92737 100644 --- a/cmd/start-issue/main.go +++ b/cmd/start-issue/main.go @@ -253,6 +253,9 @@ func parse(args []string) (options, error) { if o.mode != "" && o.issue != "" { return o, fmt.Errorf("Use either %s or , not both.", o.mode) } + if o.humanGatePermissionsSource == "CLI" && !o.humanGate { + return o, errors.New("--human-gate-permissions requires --human-gate.") + } if !validHumanGatePermissions(o.humanGatePermissions) { return o, fmt.Errorf("Invalid human-gate permissions %q. Use restricted or full-delivery.", o.humanGatePermissions) } @@ -2201,7 +2204,7 @@ Options: prompt template and write a reviewable proposal --human-gate Codex-only batch mode that resumes on HUMAN_GATE --human-gate-permissions - Permission contract for --human-gate + Requires --human-gate; permission contract for it Default: START_ISSUE_HUMAN_GATE_PERMISSIONS or restricted --human-gate-help Show detailed help for the human-gate mode --prompt-output-file @@ -2321,6 +2324,7 @@ Permission modes: Precedence: --human-gate-permissions, START_ISSUE_HUMAN_GATE_PERMISSIONS, restricted. + --human-gate-permissions requires --human-gate. Flow: The normal issue workflow creates or reuses the worktree, renders the diff --git a/cmd/start-issue/main_test.go b/cmd/start-issue/main_test.go index 0e64277..174f7c6 100644 --- a/cmd/start-issue/main_test.go +++ b/cmd/start-issue/main_test.go @@ -79,7 +79,7 @@ func TestParseHumanGatePermissionsPrecedenceAndValidation(t *testing.T) { t.Fatalf("environment permissions = %q (%s)", o.humanGatePermissions, o.humanGatePermissionsSource) } - o, err = parse([]string{"1", "--human-gate-permissions", "restricted"}) + o, err = parse([]string{"1", "--human-gate", "--human-gate-permissions", "restricted"}) if err != nil { t.Fatal(err) } @@ -87,7 +87,10 @@ func TestParseHumanGatePermissionsPrecedenceAndValidation(t *testing.T) { t.Fatalf("CLI permissions = %q (%s)", o.humanGatePermissions, o.humanGatePermissionsSource) } - if _, err := parse([]string{"1", "--human-gate-permissions", "unlimited"}); err == nil || !strings.Contains(err.Error(), "Use restricted or full-delivery") { + if _, err := parse([]string{"1", "--human-gate-permissions", "restricted"}); err == nil || !strings.Contains(err.Error(), "requires --human-gate") { + t.Fatalf("permission flag without human-gate error = %v", err) + } + if _, err := parse([]string{"1", "--human-gate", "--human-gate-permissions", "unlimited"}); err == nil || !strings.Contains(err.Error(), "Use restricted or full-delivery") { t.Fatalf("invalid CLI permissions error = %v", err) } t.Setenv("START_ISSUE_HUMAN_GATE_PERMISSIONS", "unlimited") diff --git a/doc/spec.md b/doc/spec.md index c56381b..0c76298 100644 --- a/doc/spec.md +++ b/doc/spec.md @@ -72,7 +72,7 @@ Agent-specific behavior должен быть централизован за е | `--prompt-file` | Файл prompt template | См. приоритет prompt | | `--improve-prompt` | Сгенерировать reviewable proposal улучшенного prompt template и выйти до создания worktree | false | | `--human-gate` | Codex-only batch mode для issue workflow с resume на `STATUS: HUMAN_GATE` | false | -| `--human-gate-permissions ` | Capability contract для human-gate; CLI имеет приоритет над `START_ISSUE_HUMAN_GATE_PERMISSIONS` | `restricted` | +| `--human-gate-permissions ` | Capability contract для human-gate; требует `--human-gate`; CLI имеет приоритет над `START_ISSUE_HUMAN_GATE_PERMISSIONS` | `restricted` | | `--human-gate-help` | Показать отдельную справку по human-gate mode | false | | `--prompt-output-file` | Путь proposal-файла для `--improve-prompt` | Для `.md`: рядом с source как `*.improved.md`; для остальных файлов: `.improved`; иначе `.start-issue/prompt.improved.md` | | `--no-init` | Пропустить запуск `init.sh` | false | diff --git a/memory-bank/features/FT-017/design.md b/memory-bank/features/FT-017/design.md index 7350899..9f97571 100644 --- a/memory-bank/features/FT-017/design.md +++ b/memory-bank/features/FT-017/design.md @@ -119,7 +119,7 @@ enforces credentials and repository authorization independently. | Contract ID | Input / Output | Producer / Consumer | Semantics / Constraints | | --- | --- | --- | --- | -| `CTR-01` | `--human-gate-permissions restricted\|full-delivery` | CLI parser / config resolver | CLI value wins over environment; invalid or empty explicit values fail before issue fetch. | +| `CTR-01` | `--human-gate-permissions restricted\|full-delivery` | CLI parser / config resolver | CLI value wins over environment; it requires `--human-gate`; invalid or empty explicit values fail before issue fetch. | | `CTR-02` | `START_ISSUE_HUMAN_GATE_PERMISSIONS` | shell environment / config resolver | Used only when CLI input is absent; unset resolves to `restricted`. | | `CTR-03` | Restricted Codex command | launcher / Codex | `codex [--model MODEL] exec --cd WORKTREE --sandbox workspace-write --json --output-last-message PATH -`. | | `CTR-04` | Full-delivery Codex command | launcher / Codex | `codex [--model MODEL] --dangerously-bypass-approvals-and-sandbox exec --cd WORKTREE --json --output-last-message PATH -`; selected only by explicit `full-delivery`. | diff --git a/memory-bank/features/FT-017/implementation-plan.md b/memory-bank/features/FT-017/implementation-plan.md index 750e031..d3156b7 100644 --- a/memory-bank/features/FT-017/implementation-plan.md +++ b/memory-bank/features/FT-017/implementation-plan.md @@ -94,7 +94,7 @@ creates retained fixture GitHub state. | Approval Gate ID | Trigger | Applies to | Why approval is required | Approver / evidence | | --- | --- | --- | --- | --- | -| `AG-01` | Running a real full-delivery session that can commit, push, and create/update a PR | `STEP-06`, `WS-4`, `CHK-03` | The run is unsandboxed and creates external GitHub state | User names/approves the fixture target; retained log and PR URL record approval context | +| `AG-01` | Running a real full-delivery session that can commit, push, and create/update a PR | `STEP-06`, `WS-4`, `CHK-03` | The run is unsandboxed and creates external GitHub state | User names/approves the fixture target; retained fixture directory, log, state artifacts, and PR URL record approval context | ## Work Order diff --git a/test/e2e/human-gate.sh b/test/e2e/human-gate.sh index 780b507..47c89ea 100755 --- a/test/e2e/human-gate.sh +++ b/test/e2e/human-gate.sh @@ -18,7 +18,8 @@ clone after a successful run; set START_ISSUE_E2E_KEEP=1 to retain it. The HUMAN_GATE scenario opens Codex resume interactively; exit it to continue. FULL_DELIVERY also requires START_ISSUE_E2E_FULL_DELIVERY=1. It authorizes an unsandboxed Codex run that creates a unique fixture commit, remote branch, and -pull request. Those remote artifacts are retained as evidence. +pull request. Its temporary fixture and diagnostic artifacts are retained as +evidence automatically. EOF } @@ -176,7 +177,7 @@ unexpected_changes="$(git -C "$worktree_path" status --porcelain | awk '$0 !~ /^ [[ -z "$unexpected_changes" ]] || fail "fixture worktree has unexpected changes: $unexpected_changes" printf 'PASS: real Codex human-gate %s scenario. State: %s\n' "$scenario" "$state_dir" -if [[ "${START_ISSUE_E2E_KEEP:-}" == "1" ]]; then +if [[ "$scenario" == "full-delivery" || "${START_ISSUE_E2E_KEEP:-}" == "1" ]]; then printf 'The temporary fixture is preserved at: %s\n' "$fixture_root" else git -C "$fixture_dir" worktree remove --force "$worktree_path"