Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ docs/
governance.md # Who controls the agents and their config
repo-readiness.md # Test coverage baseline and readiness criteria
code-review.md # How agents review code, security sub-agents
tekton-pipeline-review.md # Reviewing Tekton tasks/pipelines as a distinct domain
architectural-invariants.md # Enforcing things that must always be true
multi-tenancy.md # How agents preserve tenant isolation boundaries
migration-path.md # Incremental path from human-driven to agent-driven
landscape.md # Survey of AI code review tools (time-sensitive)
experiments/ # Logs/results from practical experiments
```
Expand Down
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,10 @@ This is not a product spec. It's an evolving exploration of a hard problem space
- [Governance](docs/problems/governance.md) — Who controls the agents and their configuration?
- [Repo Readiness](docs/problems/repo-readiness.md) — Test coverage, CI/CD maturity, what's needed before agents can be trusted
- [Code Review](docs/problems/code-review.md) — How agents review code, including security-focused sub-agents
- [Tekton Pipeline Review](docs/problems/tekton-pipeline-review.md) — Reviewing Tekton task and pipeline definitions as a distinct domain
- [Architectural Invariants](docs/problems/architectural-invariants.md) — Enforcing things that must always be true, grounded in the existing architecture repo
- [Multi-tenancy](docs/problems/multi-tenancy.md) — How agents understand and preserve tenant isolation boundaries
- [Migration Path](docs/problems/migration-path.md) — How to get from today's workflow to agent-driven development incrementally
- **[docs/landscape.md](docs/landscape.md)** — Survey of existing AI code review tools and how they relate to our goals (time-sensitive — check the date)
- **[docs/experiments/](docs/experiments/)** — Logs and results from trying things in practice

Expand Down
53 changes: 53 additions & 0 deletions docs/problems/code-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,56 @@ A human reviewer can say "I'm not sure about this, let me think" or "I need some
- How does an agent express uncertainty? Confidence scores? Explicit "I don't know" signals?
- Should there be a minimum number of agent reviewers that agree before auto-merge?

## The heterogeneous codebase problem

The sub-agent decomposition above implicitly assumes reviewing Go code — controllers, operators, services. But konflux-ci is heterogeneous. The org contains Go controllers, React frontends, Tekton task/pipeline YAML with embedded shell, Python tooling, and pure shell scripts. A "correctness agent" for a Go reconciler and a "correctness agent" for a Tekton task are solving fundamentally different problems.

### Tekton task review is a distinct discipline

Reviewing a Tekton task change is not like reviewing application code. The concerns are different:

- **Embedded shell scripts** — most task logic lives in `script:` fields inside step definitions. These are shell scripts embedded in YAML, which means the review agent needs to understand bash semantics (quoting, word splitting, exit codes, pipelines, subshells) *and* the YAML embedding (indentation sensitivity, multiline strings). A Go correctness agent won't catch `$VARIABLE` vs `${VARIABLE}` bugs or missing `set -euo pipefail`.
- **Image references** — each step declares a container image. Reviewing whether the right image is used, whether digest pinning is correct, whether the image actually contains the tools the script calls — this is a supply chain concern that doesn't exist in normal code review. See [architectural-invariants.md](architectural-invariants.md) and the ADR-0046 drift scanner experiment for how this plays out in practice.
- **Result propagation** — Tekton tasks communicate between steps and between tasks via results (written to `/tekton/results/`). A common bug class is result name mismatches: task A writes to `IMAGE_URL`, task B reads `IMAGE_DIGEST`. This is a stringly-typed interface with no compile-time checking. A review agent needs to trace result flows across the pipeline, not just within a single task.
- **Workspace and volume bindings** — tasks declare workspaces; pipelines bind them. A mismatch means silent data loss or a task reading stale data from a previous run. This is another cross-boundary concern that per-file review misses.
- **Parameter threading** — parameters flow from pipeline → task → step script via `$(params.foo)` substitution. Missing parameters, wrong types, or unused parameters are common bugs that require understanding the full parameter chain.
- **When expressions and matrix** — conditional execution and fan-out patterns add control flow that lives in YAML, not in code. Reviewing whether the conditions are correct requires understanding both the YAML semantics and the pipeline's intended behavior.

### What this means for the sub-agent model

The current 6-agent decomposition may need a 7th: a **pipeline/task domain agent** that understands Tekton semantics specifically. Alternatively, the correctness agent needs to be parameterized per content type — one profile for Go, one for Tekton YAML, one for shell scripts. Either way, treating all code review as homogeneous will produce poor results on the repos that matter most (build-definitions defines every build pipeline in the system).

See also [tekton-pipeline-review.md](tekton-pipeline-review.md) for a deeper treatment of this problem.

## What human reviewers actually catch (and miss)

The sub-agent model is well-reasoned in theory. But grounding it in what actually happens during human review reveals gaps and opportunities.

### What experienced human reviewers catch that agents will struggle with

- **"This works but it's the wrong approach"** — a PR that correctly implements a feature but creates a maintenance burden, introduces an unnecessary dependency, or solves the problem at the wrong layer. This requires taste and experience with the codebase's evolution, not just correctness checking.
- **"This interacts badly with X"** — cross-system awareness. A change to the build-service that looks correct in isolation but breaks an assumption the integration-service makes. Human reviewers catch this because they've been burned before. Agents would need explicit cross-repo dependency models.
- **"We tried this before and it didn't work"** — institutional memory. A PR that reintroduces a pattern that was previously removed for good reasons. This is in the git history, but surfacing it requires knowing *what to look for*.
- **"This is technically a bug fix but it changes user-visible behavior"** — the tier escalation problem from [intent-representation.md](intent-representation.md), but at the code review level. Experienced reviewers recognize when a "fix" is actually a behavior change that needs broader discussion.

### What human reviewers miss that agents could be better at

- **Consistent application of conventions** — humans get tired and inconsistent across reviews. Agents can mechanically verify every naming convention, error handling pattern, and API contract every time.
- **Comprehensive edge case analysis** — humans skim happy paths and spot-check edge cases. Agents can systematically enumerate error conditions, nil checks, and boundary values.
- **Cross-file impact analysis** — humans often review file-by-file. Agents can trace a change through the call graph and identify every affected code path.
- **Dependency version analysis** — checking whether a dependency update introduces known vulnerabilities, behavioral changes, or license issues. Tedious for humans, natural for agents.
- **Detecting subtle injection patterns** — prompt injection in code comments, commit messages, and string literals. Humans don't think about this; agents can be specifically trained to look for it.

### The implication for sub-agent design

The "what humans catch that agents won't" list suggests the sub-agents need more than just the diff and surrounding code. They need:

- **Codebase history** — not just current state, but why things are the way they are
- **Cross-repo dependency models** — explicit maps of which repos depend on which, and at what interfaces
- **"Previously rejected" patterns** — a knowledge base of approaches that were tried and abandoned, with reasons

These are expensive context sources. The question is whether they're loaded proactively (into every review) or reactively (when a sub-agent's confidence is low and it wants more context before escalating).

## Open questions

- Can we quantify review quality? How do we know if an agent's review is as good as a human's?
Expand All @@ -166,3 +216,6 @@ A human reviewer can say "I'm not sure about this, let me think" or "I need some
- How do we prevent review agents from being "rubber stamps" — always approving because they're optimizing for throughput?
- What's the right interface for review feedback? GitHub PR comments? A structured report? Both?
- How do we handle multi-repo changes where the review needs to consider changes across repos together?
- How do we handle the heterogeneity of content types across the org? One correctness agent, or specialized agents per content type?
- Should review agents have different context loading strategies for different repo types (Go service vs. Tekton tasks vs. React frontend)?
- How do we capture and expose "institutional memory" to review agents without creating an injection surface?
150 changes: 150 additions & 0 deletions docs/problems/migration-path.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
# Migration Path

How do we get from today's human-driven development workflow to agent-driven development? The vision doc says "start anywhere, learn everywhere" — but in practice, the first steps matter. A bad first experience will kill adoption. A good one builds momentum.

## The current state

Today, development across konflux-ci looks roughly like this:

1. Humans triage issues and prioritize work
2. Humans (sometimes with coding agents) implement changes and open PRs
3. Humans review PRs, sometimes with CI checks (linters, unit tests, integration tests)
4. Humans approve and merge
5. CI/CD pipelines build, test, and deploy

There's no unified review tooling. Some repos have thorough CI; others have minimal checks. CODEOWNERS files exist in some repos but not all. Test coverage varies wildly (see [repo-readiness.md](repo-readiness.md)). The process is informal and relies heavily on individual maintainer knowledge.

The gap between this and "fully autonomous agents handling routine development" is large. Bridging it requires incremental steps that deliver value at each stage, not a big-bang rollout.

## The sequencing problem

Multiple problem areas need to be solved, and they have dependencies:

- **Agents can't review well** without understanding intent → need the intent system
- **The intent system** needs governance to define tiers → need governance decisions
- **Governance** needs experimentation data to make good decisions → need agents running somewhere
- **Agents running somewhere** need repos to be ready → need readiness improvements
- **Readiness improvements** could be done by agents → need agents to be trusted first

This is circular. The way out is to start with the lowest-risk, highest-signal activities and iterate.

## Proposed phases

### Phase 0: Observation (now → weeks)

**Goal:** Understand what actually happens today. Build the data foundation for later decisions.

**Actions:**
- Run [agentready](https://github.com/ambient-code/agentready) assessments across all repos in the org to get a baseline readiness score
- Extend the coverage dashboard with additional readiness signals: CODEOWNERS presence, CI job reliability (flaky test rate), linter enforcement, `CLAUDE.md` or equivalent presence
- Catalog the content types per repo (Go, Tekton YAML, shell, React, Python) to understand the heterogeneity agents will face
- Identify 2-3 candidate repos for Phase 1 based on: high test coverage, reliable CI, active maintainers willing to participate, and manageable scope

**Delivers:** A clear picture of where we are and which repos are closest to ready. No agents touching production yet.

### Phase 1: Shadow review (weeks → months)

**Goal:** Run review agents in parallel with human reviewers. Compare agent decisions to human decisions. Build confidence (or learn where agents fail).

**Actions:**
- Deploy review agents on candidate repos in comment-only mode — agents post review comments but cannot approve or block
- Start with a single review concern (e.g., correctness only) rather than all 6 sub-agents at once. This reduces noise and makes it easier to evaluate agent quality.
- Track metrics: agreement rate between agent and human reviewers, false positive rate (agent flags something human wouldn't), false negative rate (human catches something agent missed), review latency
- Collect feedback from human reviewers: are the agent's comments useful? Distracting? Wrong?
- Iterate on agent prompts, context loading, and sub-agent decomposition based on real data

**Delivers:** Data on agent review quality. Human reviewers get a second opinion. No risk — agents can't approve or merge anything.

**Key decision point:** Do agent reviews add value? If the false positive rate is too high, humans will ignore them (and then ignore real findings). If the agreement rate is too low, the agents aren't ready for autonomy.

### Phase 2: Assisted review (months)

**Goal:** Agents become required reviewers but humans retain merge authority.

**Actions:**
- Add agent review as a required status check on candidate repos — PRs can't merge without agent review completing, but a human still approves
- Expand to multiple review sub-agents (correctness + security, then intent alignment)
- Introduce the pre-PR review pattern: developers using coding agents run the review sub-agents locally before opening a PR
- Begin CODEOWNERS cleanup: ensure guarded paths are properly configured on candidate repos
- Start filing issues from drift detection agents (like the ADR-0046 scanner) — agents identify problems, humans decide what to do

**Delivers:** Faster review cycles (agent provides immediate first-pass feedback). Quality baseline from agent reviews. CODEOWNERS infrastructure ready for Phase 3.

### Phase 3: Conditional autonomy (months → ongoing)

**Goal:** Agents can auto-merge specific categories of changes on graduated repos.

**Actions:**
- Start with Tier 0 changes only: dependency updates that pass CI, linter fixes, documentation typo fixes. These are the lowest-risk changes with the clearest intent ("we always want these").
- Implement the minimal intent verification needed: for Tier 0, the intent is implicit ("this is a category we always approve"). The agent verifies the change actually falls in this category.
- Require all review sub-agents to pass before auto-merge (unanimous approval for the initial rollout — loosen later based on data)
- Monitor closely: any bad merge triggers automatic revert to Phase 2 for that repo
- Gradually expand the set of auto-mergeable change types as confidence grows

**Delivers:** Actual autonomous merges for the safest change categories. Real-world data on the autonomy model. A mechanism for expanding autonomy incrementally.

### Phase 4: Full autonomy for graduated repos

**Goal:** Agents handle Tier 0 and Tier 1 changes autonomously. Tier 2+ still requires human authorization.

**Actions:**
- Implement the intent system (git-based ledger or equivalent) for Tier 2+ changes
- Agents can implement and merge tactical changes (bug fixes with linked issues) autonomously
- Human-guarded paths via CODEOWNERS remain inviolable
- Periodic human audits of agent-merged changes (weekly? monthly?) to catch drift

**Delivers:** The vision for routine changes. Humans focus on strategic decisions and guarded paths.

## Which repos first?

Based on the [repo-readiness](repo-readiness.md) data and practical considerations:

**Strong candidates for Phase 1:**
- **release-service** (87.5% coverage) — well-tested Go service with active maintenance
- **notification-service** (85.0% coverage) — smaller scope, high coverage, less security-critical
- **repository-validator** (82.4% coverage) — focused scope, good coverage

**Interesting but harder:**
- **build-definitions** — the most critical repo, but it's Tekton YAML, not Go. Needs the Tekton-specific review capability from [tekton-pipeline-review.md](tekton-pipeline-review.md) before agents can review effectively.
- **integration-service** (68.4% coverage) — central to the system but coverage could be higher

**Not yet:**
- Repos with <50% coverage
- Repos with no coverage data
- Security-critical infrastructure repos (regardless of coverage)

## The bootstrap problem

Several pieces of infrastructure need to exist before agents can operate:

- **Agent GitHub identity** — bot accounts or GitHub App installations that agents use to post comments and status checks. These need to be set up, permissioned, and secured.
- **Agent execution environment** — where do agents run? Local developer machines (for pre-PR review)? CI infrastructure (for PR-level review)? Dedicated agent infrastructure?
- **Agent configuration** — CLAUDE.md files, system prompts, context loading configuration per repo. Who writes the initial version? How is it tested?
- **Monitoring and alerting** — dashboards for agent activity, alert on anomalies, mechanism to revoke agent authority in an emergency

Each of these needs to be solved before Phase 1 can start, but none of them need to be solved perfectly. Start simple and iterate.

## Anti-patterns to avoid

- **Big-bang rollout** — don't try to enable agents on all repos simultaneously. The failure modes are different per repo and need individual attention.
- **Skipping shadow mode** — the temptation is to go straight to agent autonomy on "easy" repos. Shadow mode builds the data and trust foundation that makes autonomy defensible.
- **Optimizing for speed over safety** — the goal is not "agents merge as fast as possible." The goal is "agents merge correctly." Speed is a bonus of correctness, not a substitute for it.
- **Ignoring the human experience** — if human reviewers find agent comments noisy, unhelpful, or annoying, they'll disable or ignore them. Agent output quality matters more than agent output volume.
- **Assuming homogeneity** — what works for a Go controller repo won't work for build-definitions. The migration path per repo depends on the repo's content type, test infrastructure, and maintainer culture.

## Relationship to other problem areas

- **Repo readiness** — determines which repos are candidates for each phase
- **Autonomy spectrum** — Phase 3-4 implement the graduated autonomy model
- **Governance** — who decides when a repo moves between phases? Who can revert a repo to a lower phase?
- **Code review** — Phases 1-2 are specifically about validating the review sub-agent model
- **Intent representation** — needed by Phase 4, but the simpler phases can proceed without it

## Open questions

- What's the minimum viable shadow mode? Can we run a single review agent on a single repo as a GitHub Action and start collecting data this week?
- How do we handle the organizational change management? Developers need to understand what the agents are doing, why, and how to give feedback. What's the communication plan?
- How do we handle repos where the maintainers don't want agent involvement? Is participation mandatory at the org level, or opt-in per repo?
- What metrics define success at each phase? What's the threshold for moving to the next phase?
- How do we handle the cost? Running multiple LLM-powered review agents on every PR across 30+ repos is expensive. What's the cost model and who pays?
- Can Phase 0 and Phase 1 run concurrently on different repos, or do we need Phase 0 data before starting Phase 1?
Loading