ADR-0002: Initial fullsend design.

[ggallen]

Initial fullsend design. Have at it.

[qodo-code-review]

Review Summary by Qodo

Initial Fullsend design: issue-to-merge workflow with autonomous agents

📝 Documentation ✨ Enhancement

Walkthroughs

Description

• Introduces comprehensive initial Fullsend design ADR with issue-to-merge workflow
• Defines four-phase workflow: triage, implementation, multi-agent review, post-merge trace
• Specifies label-driven state machine and slash-command control for autonomous agents
• Decomposes system into 15 independent building blocks with clear responsibilities
• Establishes sandboxing principles and security boundaries for agent execution

Diagram

flowchart LR
  Issue["Issue created<br/>title + body + attachments"]
  Triage["Phase A: Triage Agent<br/>duplicate detection + readiness"]
  Triage_Out{Outcome}
  Duplicate["Duplicate<br/>close issue"]
  NotReady["not-ready<br/>request info"]
  Ready["ready-for-coding<br/>proceed to implementation"]
  
  PR["Phase B: PR Agent<br/>implement + test + checks"]
  PR_Out{Checks pass?}
  Review["Phase C: Review Swarm<br/>N parallel reviewers + coordinator"]
  Review_Out{Unanimous?}
  Rework["ready-for-coding<br/>request changes"]
  Merge["ready-for-merge<br/>approved for merge"]
  Manual["requires-manual-review<br/>human decision"]
  
  Merged["PR Merged"]
  Trace["Phase D: Post-merge Trace<br/>flow narrative comment"]
  
  Issue --> Triage
  Triage --> Triage_Out
  Triage_Out -->|duplicate| Duplicate
  Triage_Out -->|insufficient| NotReady
  Triage_Out -->|ready| Ready
  
  Ready --> PR
  PR --> PR_Out
  PR_Out -->|iterate| PR
  PR_Out -->|pass| Review
  
  Review --> Review_Out
  Review_Out -->|split vote| Manual
  Review_Out -->|changes needed| Rework
  Review_Out -->|unanimous| Merge
  
  Rework --> PR
  Merge --> Merged
  Manual --> Merged
  
  Merged --> Trace

Loading

File Changes

1. docs/ADRs/0002-initial-fullsend-design.md 📝 Documentation +436/-0

Comprehensive initial Fullsend design ADR with four-phase workflow

• Comprehensive ADR documenting the initial Fullsend design for GitHub-centric autonomous agent
 workflows
• Defines four workflow phases: triage (duplicate detection, readiness), implementation (PR agent
 with checks), multi-agent review (N reviewers + coordinator), and post-merge trace (audit narrative)
• Specifies label-driven state machine with mutual exclusion rules and slash-command overrides
 (/triage, /code, /review, /flow-trace)
• Decomposes system into 15 independent building blocks covering webhook dispatch, label guards,
 agent runtimes, sandboxes, and observability
• Establishes sandboxing principles with least-privilege access for triage/repro, PR/build-test,
 review, and post-merge trace execution contexts
• Documents coordinator algorithm for unanimous approval, unanimous rework, and split-vote scenarios
 with security severity handling
• Includes detailed workflow narratives, trigger conditions, label transitions, and consequences
 (positive and negative)
• Provides two Mermaid diagrams: end-to-end flow with explicit triggers and layered platform/sandbox
 stack architecture

docs/ADRs/0002-initial-fullsend-design.md

---

2. docs/architecture.md 📝 Documentation +71/-99

Refactor architecture to reference ADR 0002 workflow components

• Replaces generic architecture component descriptions with ADR 0002-specific workflow components
• Maps 15 building blocks from ADR 0002 with direct references and brief responsibility statements
• Removes previous open-ended component questions in favor of concrete ADR 0002 definitions
• Adds shared workflow terms section documenting issue fields, labels, and slash commands
• Simplifies architecture document to serve as index/reference for ADR 0002 rather than standalone
 design

docs/architecture.md

---

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (1) 📎 Requirement gaps (0) 📐 Spec deviations (0)

1. ADR 0002 lacks options 📘 Rule violation ⚙ Maintainability

Description

The new ADR proposes a design decision without documenting at least two alternative solution options
and explicit trade-offs. This fails the design-doc requirement to compare multiple approaches and
justify the selected option.

Code

docs/ADRs/0002-initial-fullsend-design.md[R39-45]

+This document does **not** mandate a single implementation (GitHub App vs Actions vs external runner); it describes **interfaces** (labels, comments, checks) and **responsibilities** so implementations can vary.
+
+## Decision
+
+We propose adopting the following **reference workflow** as the mental model for issue → PR → merge with multiple agents, **label-driven state transitions**, **slash-command overrides**, and **explicit sandbox boundaries** underneath each agent class.
+
+### Actors and triggers

Evidence

PR Compliance ID 131399 requires a section like Options/Alternatives with at least two distinct
options and explicit trade-offs, plus a rationale for the chosen option. The ADR moves directly into
## Decision and the reference workflow narrative without any options/alternatives comparison
section.

Rule 131399: Design docs must compare multiple solution options with explicit trade-offs
docs/ADRs/0002-initial-fullsend-design.md[39-45]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The ADR introduces a proposed design but does not include an explicit multi-option comparison (at least two options) with trade-offs and a documented rationale for the selected approach.

## Issue Context
Compliance requires design docs to include an `Options`/`Alternatives` (or similar) section describing multiple distinct approaches, trade-offs across multiple axes, and an explicit decision rationale.

## Fix Focus Areas
- docs/ADRs/0002-initial-fullsend-design.md[39-210]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Stale merge-approval label 🐞 Bug ✓ Correctness

Description

ADR 0002 defines ready-for-merge as unanimous approval for a review round, but it never specifies
clearing ready-for-merge when the PR head changes (push triggers re-review but only strips
ready-for-review). This can leave a PR/issue marked ready-for-merge even though the current head
SHA hasn’t been reviewed, making the label’s semantics unsafe for any downstream automation/humans
that rely on it.

Code

docs/ADRs/0002-initial-fullsend-design.md[R158-182]

+**When a review run starts:** **remove** **`ready-for-review`**. Reviewers evaluate the current PR head; the coordinator applies outcomes using the algorithm below.
+
+**Review swarm:**
+
+- **Configurable count N** of **independent review agents** run in parallel (separate invocations, separate context windows where applicable).
+- **Coordinator selection:** One reviewer is **randomly chosen** as **coordinator** for this round (deterministic seed optional for auditability).
+- **Coordinator duties:** Collect reviewer outputs; produce **one consolidated GitHub comment** (findings, severity, requested changes); apply the **coordinator algorithm** below; update labels per rules below.
+
+**Coordinator algorithm (merge vs manual vs rework):**
+
+- Each reviewer outputs a structured verdict (e.g. **approve merge**, **request changes**, **comment-only**), including **security severity** when relevant.
+- **Unanimous approve-merge:** **All** counted reviewers **approve merge** with **no** outstanding **request changes** and **no** **conflicting security severities** on whether the PR is safe to merge. Apply **`ready-for-merge`**, remove **`ready-for-review`** and **`requires-manual-review`** if present.
+- **Unanimous request-changes:** **All** agree the PR needs revision (no one approves merge yet). Apply **`ready-for-coding`**, remove **`ready-for-review`** — same as **Changes requested** below (not **`requires-manual-review`**).
+- **Not unanimous** (split votes, some approve and some reject, or **conflicting security severities** that prevent a single merge judgment): Apply **`requires-manual-review`**, remove **`ready-for-review`**, do **not** apply **`ready-for-merge`**. The coordinator comment must summarize **who said what** so humans can decide.
+
+**Outcomes:**
+
+- **Changes requested** (coordinator maps to unanimous rework): Coordinator comment documents issues; remove **`ready-for-review`**, add **`ready-for-coding`** so the PR agent can resume; clear **`requires-manual-review`** if set.
+- **Unanimous merge:** Add **`ready-for-merge`** (and remove **`ready-for-review`**). Actual merge may still require **human approval**, merge queue, or bot merge permission per [governance](../problems/governance.md) — this ADR only defines **agent-visible** labels.
+- **Requires manual review:** Add **`requires-manual-review`** (and remove **`ready-for-review`**); do **not** add **`ready-for-merge`**.
+
+**Re-review policy:**
+
+- **On every push** to the PR head while the change is **in the review stage** (implementation has handed off to review; automation tracks round state—the **`ready-for-review`** label may already have been **cleared** when the current round **started**): **automatically** enqueue a **new** multi-agent review round (same N, new coordinator selection). This keeps review aligned with the latest diff without waiting for humans.
+- **On demand:** **`/review`** in a comment **also** enqueues a review round (re-run or extra pass). **Both** apply: pushes trigger re-review **and** maintainers can force a round via **`/review`** even without a new push.

Evidence

The ADR states that starting a review round strips only ready-for-review, and separately that
unanimous approval applies ready-for-merge; the re-review policy enqueues a new round on every
push but does not describe clearing ready-for-merge when that happens. This combination allows
ready-for-merge to remain after new commits, even though the approval was for a prior round/SHA.

docs/ADRs/0002-initial-fullsend-design.md[158-182]
docs/ADRs/0002-initial-fullsend-design.md[166-177]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`ready-for-merge` is defined as approval for a specific review round, but the ADR doesn’t define how to invalidate it when new commits are pushed (re-review is triggered, but label reset doesn’t mention `ready-for-merge`). This can leave stale approval signals.

### Issue Context
The ADR already enqueues re-review on PR synchronize and strips `ready-for-review` at round start, but it doesn’t specify what happens to `ready-for-merge` (or other outcome labels) when a new round begins due to a push.

### Fix Focus Areas
- docs/ADRs/0002-initial-fullsend-design.md[158-182]

### Proposed fix
Update the Phase C “When a review run starts” (or the PR synchronize trigger handling) to explicitly remove `ready-for-merge` (and likely `requires-manual-review`) when starting a new review round triggered by a new head SHA, ensuring approvals never survive a head change without re-approval.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. PR start leaves not-ready 🐞 Bug ⚙ Maintainability

Description

Phase B allows /code to trigger the PR agent, but the PR-agent run-start reset rules don’t mention
clearing not-ready. This leaves the ADR’s label transition mechanics incomplete for the common
override case, and can result in an implementation run proceeding while the issue remains labeled as
insufficient information.

Code

docs/ADRs/0002-initial-fullsend-design.md[R128-137]

+### Phase B — Implementation (PR agent)
+
+**Entry:** The **`ready-for-coding`** label was **applied** to the issue (trigger event) **or** **`/code`** was invoked—**before** the run strips labels. Issue is **open** and **not** **`duplicate`** (and no other conflicting state).
+
+**Triggers (PR agent):**
+
+1. **`ready-for-coding`** label **added** to the issue.
+2. **`/code`** in a comment.
+
+**When a PR-agent run starts:** **remove** **`ready-for-coding`** and **all later-phase labels** (**`ready-for-review`**, **`ready-for-merge`**, **`requires-manual-review`**). Implementation proceeds from the issue **`title` / `body` / attachments** and triage handoff comments—not from stale review/merge labels.

Evidence

The ADR explicitly permits /code as an entry trigger for the PR agent, but the on-start label
stripping list omits not-ready even though not-ready is defined as a triage outcome indicating
insufficient info. Without an explicit rule, implementations of the label guard/PR-agent reset
semantics won’t have a clear, spec-backed way to normalize labels in this override path.

docs/ADRs/0002-initial-fullsend-design.md[128-137]
docs/ADRs/0002-initial-fullsend-design.md[69-80]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The ADR allows `/code` to start implementation, but doesn’t specify how `not-ready` should be handled in that override flow. This leaves a spec gap for label-state enforcement.

### Issue Context
- `not-ready` is a triage outcome meaning insufficient information.
- `/code` is listed as a PR-agent trigger.
- PR-agent run-start label stripping currently removes `ready-for-coding` and later-phase labels, but not `not-ready`.

### Fix Focus Areas
- docs/ADRs/0002-initial-fullsend-design.md[69-80]
- docs/ADRs/0002-initial-fullsend-design.md[128-137]

### Proposed fix
Add an explicit rule for the `/code` override path, e.g. either:
1) the label guard rejects `/code` when `not-ready` is present unless an explicit “force” form is used, or
2) PR-agent run-start stripping also removes `not-ready` (and documents why), possibly replacing it with `ready-for-coding` or another “override acknowledged” label.
Document the chosen behavior so implementations remain consistent.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more

[arewm]

Some initial comments, but I didn't finish reading.

[ralphbean]

I'm ready to merge this, especially so that we can use it as a baseline to build future decisions and alterations off of. However, run make lint. Looks like there's a trailing newline in here somewhere that'll need to be fixed before this can merge.

This isn't a blocking concern, but I think I'd like to see the flow-trace feature removed here. It's a good idea, but it's not essential for mvp and we might build ourselves into a corner if we build it early.