Strengthen the loop: triage discussion · pre-merge staging soak · first-class codex+proxy#12
Merged
Merged
Conversation
…nfig keys Foundation for three workflow enhancements. All new keys carry safe defaults so a repo with no .olympus.json (or one missing these blocks) behaves exactly as before. - triage.max_discussion_rounds (default 4) + labels.discussing - testing block (enabled/deploy_cmd/health_cmd/soak_minutes/teardown_cmd + fast_path ceilings) + labels.staging_soaked / labels.soak_failed - harness.kind gains "codex"; new harness.proxy; health_probe now defaults OFF for codex (its backend isn't OpenAI-compatible) while staying ON for claude config.sh, schema, and test_config.py updated together (12/12 green). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QEnZ5MVuPdzhNwXLfsi7Sc
Codex graduates from the generic `custom` template to a built-in harness: - harness.kind=codex builds a real `codex exec --model <m>` argv (no eval), mapping the profile onto codex's sandbox — read-only for investigate/review, workspace-write for implement (the only file-editing profile). The run loop pipes the prompt on stdin and captures stdout, same as claude. - harness.proxy injects HTTPS_PROXY/HTTP_PROXY/ALL_PROXY (+ lowercase) into the harness child for NON-claude harnesses only, so codex reaches its model backend through 172.16.103.81:8888 on staging/testing where a direct connection is blocked. claude is never proxied (it talks to the internal gateway). The token-strip (implement) and proxy now fold into one env prefix. test_harness.py: +12 checks for codex argv/sandbox and proxy scoping; the 23 back-compat checks for claude/custom still pass unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QEnZ5MVuPdzhNwXLfsi7Sc
Triage was single-shot: one comment with do/needs_info/skip, and continuing the conversation required a maintainer to re-apply agent:assess. Now triage can talk the issue through with the reporter before committing to a verdict. - New verdict `discuss`: the agent keeps the thread open (labels the issue agent:discussing, posts its reply with no maintainer-controls clutter) when a short back-and-forth would resolve scope/intent better than a premature decision. Available on the opened path too, so triage can open a conversation on first contact instead of deciding cold. - New TRIGGER_KIND=comment path: a reply from the reporter (or a trusted maintainer) auto-re-runs triage. Guards: anti-self-loop (ignores comments carrying our breadcrumb), open issue only, reporter/maintainer only, and only while the last triage verdict is needs_info/discuss (do/skip/withheld are resolved). The agent reads the full comment thread in discussion mode. - Round cap (triage.max_discussion_rounds, default 4): at the cap the agent is forced to converge (discuss forbidden) and hand off to a human maintainer. - Reusable + consumer + self-dogfood triage workflows gain the issue_comment trigger and pass COMMENT_* env; PR comments are filtered out. compose_comment_body already does the right thing for discuss; +1 test for it. All triage/config/harness unit tests green; run_triage.sh shellcheck-clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QEnZ5MVuPdzhNwXLfsi7Sc
…ging IP The real staging/testing egress proxy is internal infra and belongs only in a consumer's .olympus.json (harness.proxy) or a HARNESS_PROXY secret — never in olympus's committed mechanism. Earlier commits put the literal IP in the schema example + two tests, which trips check-leakage.sh (RFC1918). Swap for an RFC5737 doc range (192.0.2.10) / an example hostname; leakage gate is green again. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QEnZ5MVuPdzhNwXLfsi7Sc
… before merge Adds a pre-merge staging soak gated by .testing.enabled (default off → behavior unchanged). After a review APPROVE, both merge paths classify the PR: - simple (within fast_path ceilings: ≤ max_loc / max_files, contained areas) → the existing allowlist-gated auto-merge fast path, unchanged. - complex → instead of merging, dispatch the pr-soak workflow: deploy the PR head to the consumer's testing environment, poll health for soak_minutes, then label staging-soaked (a human merges — Olympus never auto-merges a soaked PR) or soak-failed. Applies to both agent (auto_merge.sh) and human (post_review.py) PRs, gated by the same trust they already enforce. Mechanism/policy split per roadmap §3.4: Olympus orchestrates; the repo supplies testing.deploy_cmd / health_cmd / soak_minutes / teardown_cmd in .olympus.json. New: classify_pr.sh (pure classify_pr_decision + live gh wrapper), soak.sh (pure soak_comment_body + deploy/poll/label flow), soak_dispatch.sh, reusable soak.yml + consumer pr-soak.yml. auto_merge.sh / post_review.py / review.yml wired to the classify→soak branch. test_soak.py (10) + post_review soak-routing tests (5) + config testing-block tests; registered in ci.yml. All unit suites green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QEnZ5MVuPdzhNwXLfsi7Sc
…elf-soak dogfood - Reusable triage/implement/review/revise workflows gain optional OPENAI_API_KEY + HARNESS_PROXY secrets, injected as OPENAI_API_KEY / OLYMPUS_HARNESS_PROXY env (empty when unset → never overrides a file's harness.proxy). Lets staging keep the real egress proxy in a secret instead of committed config. - New self-soak.yml dogfood wrapper; self-triage.yml already gained issue_comment. SELF-DOGFOOD.md updated (new workflow, codex secrets, dispatch-by-filename). - docs/config-reference.md: new labels, triage.max_discussion_rounds, the full testing block, harness.kind=codex + harness.proxy + codex health_probe default. - docs/cookbooks/README.md: codex promoted to a built-in harness section (+proxy), custom example switched to aider, new "Staging soak before merge" walkthrough. - docs/security.md: deny-list is claude-only; residual risks for codex (no tool deny-list → proxy as egress allow-list) and soak (runs PR code in an isolated testing env); operator-checklist items for both. - README flow diagram shows the discuss loop + simple/complex (soak) merge split and the pluggable harness line. examples/consumer/.olympus.json shows the new blocks (testing disabled by default). Leakage + legacy-naming gates green; all workflow YAML parses. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QEnZ5MVuPdzhNwXLfsi7Sc
`evals/run.sh --harness codex --model gpt-5` now drives the built-in codex harness directly (no hand-written --command), and the health-probe pre-flight is skipped for codex like it is for custom (codex's backend isn't OpenAI-compatible). Usage doc updated; custom example switched to aider; notes the proxy env for staging/testing runs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QEnZ5MVuPdzhNwXLfsi7Sc
…sh 3.2
Surfaced by a local live-path smoke on macOS: when env_prefix is empty (the
common case — claude harness, no proxy, non-implement profile), the run loop's
"${env_prefix[@]}" / "${runner[@]}" expansion aborts with "unbound variable"
under `set -u` on bash 3.2 (macOS self-hosted / macos-latest runners). The
dry-run tests never reach this line, so it stayed latent; the codebase otherwise
assumes bash 4+ (where empty-array expansion is fine). Guard the four call sites
with the portable ${a[@]+"${a[@]}"} idiom — a no-op on bash 4+, crash-free on 3.2.
Pre-existing pattern (the old env_strip expansion had it too); hardened now that
the live path is exercised. test_harness.py + shellcheck still green.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QEnZ5MVuPdzhNwXLfsi7Sc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Strengthens the Olympus loop in three ways, all gated off by default so
existing consumers are unaffected:
do/needs_info/skip comment; continuing needed a manual
agent:assessre-label). It can now hold a real back-and-forth with the reporter.
only consumer CI + the LLM review. Complex PRs can now be tried out in a
testing environment and merged by a human only after a clean soak; trivially
simple PRs still auto-merge.
the generic
customtemplate, with no proxy support. It's now built in andcan reach its backend through an egress proxy on staging/testing.
Mechanism stays in this repo; all new behavior is policy in the consumer's
.olympus.json(mechanism/policy split preserved).1 — Triage multi-round discussion
discussverdict +agent:discussinglabel: triage keeps the thread open(reply with no maintainer-controls clutter) when a short exchange beats a
premature decision. Available on the opened path too.
TRIGGER_KIND=comment: a reply from the reporter or a trustedmaintainer re-runs triage on the full thread. Guards: anti-self-loop
(ignores our own breadcrumb), open issue only, only while the last verdict is
needs_info/discuss (do/skip/withheld are terminal), PR comments filtered out.
triage.max_discussion_rounds(default 4): at the cap, the agent isforced to converge and loops in a human.
issue_comment.2 — Complexity classification + staging soak
.testing.enabled(default false). After a review APPROVE, the PR isclassified: simple (within
fast_pathceilings) keeps the existingallowlist-gated auto-merge; complex is deployed to the testing environment,
soaked for
soak_minutes, then labeledstaging-soakedfor a human to merge(Olympus never auto-merges a soaked PR) or
soak-failed.already enforce. Olympus orchestrates (deploy → poll health → label); the repo
supplies
testing.deploy_cmd/health_cmd/soak_minutes/teardown_cmd.classify_pr.sh,soak.sh,soak_dispatch.sh, reusablesoak.yml,consumer
pr-soak.yml, dogfoodself-soak.yml. Aligns with roadmap §3.4.3 — First-class codex harness + per-environment proxy
harness.kind=codexbuildscodex execitself and maps each surface ontocodex's sandbox (read-only for triage/review, workspace-write for
implement/revise).
health_probedefaults off for codex.harness.proxy(or theHARNESS_PROXYrepo secret — keeps an internal addressout of committed config) exports
HTTPS/HTTP/ALL_PROXYfor the codex childonly; the claude harness is never proxied (it uses the internal gateway).
OPENAI_API_KEY/HARNESS_PROXYsecrets;
evals/run.sh --harness codexqualifies it on the bench.Verification
bash -n+ 7 unit suites (incl. newtest_soak.pyand soak-routing tests intest_post_review.py) + leakage gategh/codex/claude, covering thenon-dry-run paths unit tests don't reach: codex subprocess actually receives
the proxy env and has its GH token stripped (claude does not);
soak.shdeploy→poll→label for all three outcomes;
auto_merge.shclassify→soak/mergebranch; the full triage comment/discussion state machine.
set -u) onthe harness live path — pre-existing, but on lines this PR rewrote — now fixed
portably (no-op on bash 4+).
Security / hygiene
tool deny-list — documented in
security.md: run codex only in a trustedenvironment with the proxy as the single egress path. Soak runs PR code, so the
testing environment must be isolated from prod and the soak runner
egress-firewalled.
consumer's
.olympus.json/HARNESS_PROXYsecret. The leakage gate enforcesthis; source uses RFC5737 doc ranges / example hostnames only.
Operator follow-ups (not done here)
OPENAI_API_KEY+ the proxy (config orHARNESS_PROXYsecret) onstaging/testing runners.
evals/run.sh --harness codexbefore going live (theprompt-shape constraint still applies; parsers stay harness-agnostic).
issue_commenttrigger,pr-soak.yml)and create the
agent:discussing/staging-soaked/soak-failedlabels(scripts also create them best-effort).
@vX.Y.Zinexamples/consumer/*.🤖 Generated with Claude Code