Skip to content

feat(review): add local multi-agent delivery fleet - #781

Open
pablog12 wants to merge 39 commits into
kunchenguid:mainfrom
pablog12:feat/local-review-fleet
Open

feat(review): add local multi-agent delivery fleet#781
pablog12 wants to merge 39 commits into
kunchenguid:mainfrom
pablog12:feat/local-review-fleet

Conversation

@pablog12

Copy link
Copy Markdown

What changed

  • Adds the opt-in local Codex review fleet: four parallel role reviewers, deterministic consolidation, bounded auto-fix rounds, and a final independent certifier.
  • Binds review, fix, certification, push, recovery, and CI to exact commit/config provenance so stale or out-of-band state fails closed.
  • Runs reviewers cold against immutable Git-free shadows, adds security escalation by changed path, and documents the host-read boundary of Codex read-only mode.
  • Hardens detached rebase ownership, terminal CI state, fake-agent fidelity, and end-to-end recovery journeys.

Default graph

  • test-adversary: Terra xHigh
  • correctness: Terra High
  • architecture: Terra High
  • security: Terra High, escalating to xHigh on configured high-risk paths
  • consolidator: Terra High
  • certifier: Sol xHigh

Profiles remain operator-configurable; the shipped/default graph contains no Luna profiles.

Validation

  • go test ./internal/config ./cmd/fakeagent ./internal/pipeline ./internal/pipeline/steps -count=1
  • targeted race regression plus race-green config, fakeagent, and pipeline packages
  • bash scripts/e2e.sh (full E2E suite)
  • go vet ./...
  • go run ./cmd/genskill --check
  • go build ./cmd/no-mistakes
  • docs npm run build

Security boundary

The immutable shadow removes repository .git, .codex, and .agents/skills from normal discovery and prevents writes. Codex --sandbox read-only is not a host-filesystem confidentiality boundary; deployments reviewing hostile source should use a dedicated OS account or outer container.

@greptile-apps

greptile-apps Bot commented Aug 19, 2026

Copy link
Copy Markdown

Confidence Score: 4/5

The implementation appears safe to merge from a runtime perspective, but the new orchestration subsystem should first be reconciled with the repository’s governing product vision.

The investigated execution, recovery, certification, and delivery paths fail closed and preserve exact head/configuration provenance; the remaining accepted concern is independently actionable product-scope drift rather than a blocking runtime defect.

Files Needing Attention: docs/src/content/docs/reference/global-config.md, docs/src/content/docs/concepts/pipeline.md

Comments Outside Diff (1)

  1. docs/src/content/docs/reference/global-config.md, line 301-353 (link)

    P2 Review fleet expands product scope

    The new configuration coordinates specialized reviewers, a consolidator, and a certifier, conflicting with VISION.md’s explicit statement that no-mistakes is not an agent orchestrator and adding permanent orchestration complexity without reconciling the governing vision.

    Context Used: If there is a VISION.md file at the root of the re... (source)

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Reviews (1): Last reviewed commit: "fix(review): close final fleet delivery ..." | Re-trigger Greptile

@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate: I reviewed the fleet runner, isolation contract, and config tests. review_fleet.enabled defaults false; repo YAML cannot activate it; reviewers/consolidator/certifier are separate, cold, and fail-closed on head/config drift. That part aligns with independent validation.

This is not mergeable from this pass:

  • test (windows-core) failed. no-mistakes CI failure is blocking.
  • PR must be raised via no-mistakes failed. Please re-raise through the gate.

Product-scope note (not a close): VISION says the core pipeline shape stays fixed and that no-mistakes is not an agent orchestrator. This change adds a fleet-only certify step and a Codex-only multi-profile review graph. Opt-in and additive is better than a silent default flip, but the Codex lock-in and orchestration surface need an explicit human product decision before merge even after CI is green. I am not merging it.

@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate: Re-read the fleet runner, certify step, and config tests. review_fleet.enabled still defaults false; repo YAML still cannot activate it; reviewers/consolidator/certifier remain separate, cold, and fail-closed. Codex-only fleet plus the extra certify step still need a human product call (VISION R6/R7) — but this is still not otherwise ready, so I am not flagging that now.

Still blocked:

  • test (windows-core) failed (TestCodexAgent_RunStreamsLargePromptThroughStdin, TestReviewFleetSettingsResolvesRelativeExecutableOnce, TestReviewProfileRunnerIsColdAndIsolatesSkillsPluginsAndEnvironment)
  • PR must be raised via no-mistakes failed

Please fix windows-core and re-raise through the gate. I will not rebase or merge.

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.

2 participants