Skip to content

feat(orchestrator): Tier 2 implementer agents - #15

Closed
mashkovd wants to merge 2 commits into
mainfrom
feat/tier2-implementer
Closed

mashkovd wants to merge 2 commits into
mainfrom
feat/tier2-implementer

Conversation

@mashkovd

@mashkovd mashkovd commented May 9, 2026

Copy link
Copy Markdown
Member

Adds Tier 2 implementer agents to the orchestrator.

Fixes addressed from codex review on PR #2:

  • P1: force flag now picks up in-progress proposals (previously only accepted status was discovered)
  • P2: implementer scaffold (implementer.md) excluded from git staging so it doesn't leak into service-repo PRs

Implementer agents (Tier 2) take an `accepted` proposal and open a PR in
the matching sibling repo under mctlhq/. Triggered manually only — no auto-
trigger on .status.yaml change (would require Argo Events Sensor, out of
scope for v1).

What's new:
- orchestrator/run_implementer.py — discovers proposals by glob, marks
  .status.yaml in-progress, gh-clones the target repo, runs the SDK with
  the per-service implementer sub-agent, pushes the branch, opens a PR,
  marks .status.yaml implemented + PR URL.
- orchestrator/options.py — IMPLEMENTER_BUDGET_USD ($3 soft cap) and
  build_implementer_agent_options() with cwd=cloned-repo and add_dirs=[].
- agents/<svc>/.claude/agents/implementer.md for all 7 services. Shared
  body, per-service notes (sanity-check command, repo idiosyncrasies).
- Dockerfile installs `gh` CLI from cli.github.com — required by the
  orchestrator for clone + PR open. Bumps the implied image tag to 1.2.0
  (referenced from the new CWFT in mctl-gitops).
- requirements.txt: PyYAML for .status.yaml IO.

Idempotency: a proposal stuck in `in-progress` is skipped on the next run
unless --force is passed. .status.yaml schema follows the meta-plan
(status / updated_at / updated_by / pr / notes). File absent = proposed.
…ter scaffold

Address codex review on PR #2.

P1 (run_implementer.py:188): discovery hard-filtered status == "accepted",
so a wedged in-progress proposal could never be retried via --force; the
operator had to hand-edit .status.yaml. Thread `include_in_progress` from
the CLI --force flag through `find_accepted_proposals` so a force-run
returns both accepted and in-progress proposals; `implement_one` already
short-circuits in-progress unless force is set, so the two flags are
naturally paired.

P2 (run_implementer.py:232): the orchestrator copies implementer.md into
the cloned repo's .claude/agents/ before running the SDK. If the
sub-agent uses broad staging (git add -A), the runtime scaffold leaks
into the service-repo PR. After copying the file, write the path to
.git/info/exclude (per-clone, untracked-by-design ignore list). This
keeps the repo's own .gitignore untouched — modifying it would itself
be an out-of-scope change. Idempotent: re-runs don't duplicate the
entry.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 09792ecc21

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

print(f"Unknown service '{args.service}'. Available: {', '.join(SERVICES)}", file=sys.stderr)
sys.exit(2)

ensure_auth_for_sdk()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Skip Claude auth when running in --dry-run mode

The CLI currently calls ensure_auth_for_sdk() unconditionally, so python -m orchestrator.run_implementer --dry-run still fails if CLAUDE_CODE_OAUTH_TOKEN/ANTHROPIC_API_KEY are absent. That breaks the advertised dry-run behavior (discovery/spec parsing without executing the SDK) and makes non-auth validation in CI or local troubleshooting impossible. Gate this auth check behind not args.dry_run so dry-run can execute proposal discovery without Claude credentials.

Useful? React with 👍 / 👎.

@mashkovd mashkovd left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cleanup behavior contradicts the stated post-mortem guarantee. The finally block removes target unconditionally, including CalledProcessError and other failures, although the comment says “Keep target dir for post-mortem on failure; clean only on success.” Track success and delete only then, or update the contract if retaining the failed clone is intentionally no longer desired.

@mashkovd

Copy link
Copy Markdown
Member Author

Закрываю: функциональность уже на main, приехала другим путём.

Проверено:

$ git ls-tree -r --name-only origin/main | grep implementer.md
agents/_generic/.claude/agents/implementer.md
agents/mctl-agent/.claude/agents/implementer.md
agents/mctl-agents/.claude/agents/implementer.md
agents/mctl-api/.claude/agents/implementer.md
agents/mctl-docs/.claude/agents/implementer.md

$ git grep -l implementer origin/main -- orchestrator/*.py
orchestrator/run_implementer.py

Tier 2-пайплайн работает в проде: CWFT mctl-agents-implement, тул mctl_trigger_implementer, шепард поверх. То есть PR не протух, а был реализован иначе — обе его правки из ревью Codex (подхват in-progress по force-флагу, исключение implementer.md из staging) в текущем коде присутствуют по смыслу.

Открываю заново, если найдётся расхождение.

@mashkovd mashkovd closed this Aug 19, 2026
@mashkovd
mashkovd deleted the feat/tier2-implementer branch September 2, 2026 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant