docs(adr): Agent registration

[ggallen]

Summary

• Adds ADR (unnumbered) for agent registration: agents are declared in config.yaml (both org and per-repo) instead of being compiled into the fullsend binary.
• Adds implementation plan with five phases: config schema, CLI commands (fullsend agent add/list/update/remove), runtime agent resolution, cleanup of hardcoded agent map, and transition to authoritative config.

Key design decisions:

• Agent entries can be URLs (with auto-pinning of commit SHA and integrity hash) or local paths
• Entries support string shorthand or object form with optional name override
• fullsend run <name> resolves agents from config.yaml at runtime, loading harnesses directly from URLs — no intermediate wrapper files on disk
• Additive merge model: config overlays scaffold agents, enabling gradual migration
• Once all first-party agents are extracted, config becomes authoritative (tracked by a follow-up issue)

Test plan

• ADR review and approval
• Implementation plan review

🤖 Generated with Claude Code

[qodo-code-review]

PR Summary by Qodo

Add ADR and implementation plan for config-driven agent registration

📝 Documentation 🕐 10-20 Minutes

AI Description

• Document decision to move agent registration from compiled binary to config.yaml.
• Specify agent entry schema supporting pinned URLs (SHA+hash) and local paths.
• Outline phased rollout: schema, CLI commands, wrapper generation, cleanup, migration.

Diagram

graph TD
  A["Fullsend binary"] --> B["Scaffold harnesses"] --> G["Wrapper generation"]
  A --> C["Hardcoded agent map"] --> G
  D[("Org/per-repo config.yaml")] --> E["fullsend agent CLI"] --> G

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Central agent registry service

• ➕ Single canonical source of truth; avoids per-repo config drift
• ➕ Can support richer metadata (versions, signatures, deprecation)
• ➖ Adds operational complexity and an availability dependency
• ➖ Harder to support fully offline/local-path agents

2. Package-based distribution (OCI/GitHub Releases)

• ➕ Leverages existing versioning and artifact integrity primitives
• ➕ Simplifies pinning to immutable digests/tags
• ➖ More tooling and format decisions up front
• ➖ Still needs a local config to select which agents are installed

Recommendation: The config-driven approach is the right first step: it unifies first-party and third-party agents, enables per-repo extensibility without code changes, and keeps the initial migration low-risk via an additive merge model. The main follow-up to watch is tightening integrity/allowlist enforcement consistently across org/per-repo modes once the CLI and wrapper-generation phases land.

Files changed (2) +616 / -0

Documentation (2) +616 / -0

NNNN-config-driven-agents.mdAdd ADR for config-declared agents (URLs/paths, merge model, CLI surface) +189/-0

Add ADR for config-declared agents (URLs/paths, merge model, CLI surface)

• Introduces an Accepted ADR proposing agent registration in org and per-repo config via an 'agents' list. Defines entry formats (string shorthand or object with 'name'/'source'), pinning rules for remote URLs (SHA + sha256 fragment), additive merge behavior with scaffold agents, and a planned 'fullsend agent' CLI for lifecycle management.

docs/ADRs/NNNN-config-driven-agents.md

config-driven-agents.mdAdd phased implementation plan for config-driven agents +427/-0

Add phased implementation plan for config-driven agents

• Adds a detailed, five-phase plan covering config schema changes, validation rules, install-time seeding defaults, CLI subcommands, merged-agent wrapper generation, removal of hardcoded external agent map/scaffold triage assets, and eventual transition to authoritative config-only discovery. Includes proposed file touch points and test coverage expectations per phase.

docs/plans/config-driven-agents.md

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 4:50 PM UTC · Ended 4:55 PM UTC
Commit: ea2ca95 · View workflow run →

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (4)

Context used

✅ Compliance rules (platform): 51 rules

✅ Skills: writing-user-docs, writing-adrs

1. docs/architecture.md not updated 📜 Skill insight ⚙ Maintainability

Description

This PR adds a new Accepted ADR but does not update docs/architecture.md accordingly. That
leaves the architecture living document out of sync with the newly accepted decision.

Code

docs/ADRs/NNNN-config-driven-agents.md[R2-20]

+title: "NNNN. Config-driven agents"
+status: Accepted
+relates_to:
+  - agent-architecture
+  - agent-infrastructure
+topics:
+  - agents
+  - per-repo
+  - configuration
+  - extensibility
+---
+
+# NNNN. Config-driven agents
+
+Date: 2026-06-29
+
+## Status
+
+Accepted

Relevance

⭐⭐⭐ High

Team updates docs/architecture.md when accepting ADRs (e.g., ADR acceptance PR #1578, related
architecture updates in #2334).

PR-#1578
PR-#2334

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1062100 requires updating docs/architecture.md when an ADR is accepted. The ADR
is explicitly Accepted, and docs/architecture.md contains the relevant Agent Registry section
that should be updated to reflect the new decision.

docs/ADRs/NNNN-config-driven-agents.md[2-20]
docs/architecture.md[217-230]
Skill: writing-adrs

Agent prompt

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

## Issue description
A newly accepted ADR must be accompanied by an update to `docs/architecture.md` that reflects the decision and links to the ADR.

## Issue Context
This ADR changes how agents are registered/discovered; `docs/architecture.md` has an `## Agent Registry` section that should gain a minimal `**Decided:**` entry (or equivalent) referencing this ADR.

## Fix Focus Areas
- docs/ADRs/NNNN-config-driven-agents.md[2-20]
- docs/architecture.md[217-230]

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

---

2. ADR filename not zero-padded ✗ Dismissed 📜 Skill insight ⚙ Maintainability

Description

The new ADR file is named with the literal NNNN-... placeholder instead of a real four-digit,
zero-padded numeric prefix, which violates the repository’s ADR naming convention. Because the ADR
frontmatter validator in CI only targets numerically prefixed ADR filenames, this ADR can be skipped
and downstream links/indexing and metadata cross-reference checks (e.g., relates_to) may not run
correctly.

Code

docs/ADRs/NNNN-config-driven-agents.md[R1-16]

+---
+title: "NNNN. Config-driven agents"
+status: Accepted
+relates_to:
+  - agent-architecture
+  - agent-infrastructure
+topics:
+  - agents
+  - per-repo
+  - configuration
+  - extensibility
+---
+
+# NNNN. Config-driven agents
+
+Date: 2026-06-29

Relevance

⭐⭐⭐ High

ADR conventions are CI-enforced; repo added ADR linting and naming/number consistency fixes in PRs
like #39/#719.

PR-#39
PR-#719

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1062098 requires ADR filenames to start with a four-digit, zero-padded number, but
the added ADR is named docs/ADRs/NNNN-config-driven-agents.md, and the implementation plan also
links to that placeholder path. Repo documentation further specifies ADRs must use a unique 4-digit
number, and the CI frontmatter linter filters files by the regex ^\d{4}-.*\.md$, so a NNNN-...
filename will not match and therefore will be skipped by hack/lint-adr-frontmatter, preventing
validation of frontmatter fields and cross-references.

docs/ADRs/NNNN-config-driven-agents.md[1-16]
docs/plans/config-driven-agents.md[1-4]
docs/ADRs/0001-use-adrs-for-decision-making.md[39-63]
hack/lint-adr-frontmatter[154-157]
Skill: writing-adrs

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 filename must follow the required `NNNN-<slug>.md` convention where `NNNN` is a real four-digit, zero-padded numeric ADR ID, but this PR adds an ADR using the literal placeholder (`docs/ADRs/NNNN-config-driven-agents.md`). This breaks ADR naming conventions and can cause the file to be excluded from CI frontmatter validation (which only checks `^\d{4}-.*\.md$`), weakening validation of ADR metadata such as `relates_to` cross-references.

## Issue Context
- The repository requires ADR filenames to start with a unique 4-digit numeric prefix (zero-padded).
- The implementation plan document links to the placeholder ADR path, so renaming the ADR requires updating intra-doc links.
- The ADR frontmatter validator/linter in CI filters to numerically prefixed ADR filenames; placeholder names like `NNNN-...` will be skipped.
- The ADR content (frontmatter/title/H1) also uses the `NNNN` placeholder and should be updated to the chosen ADR number (typically shown as an integer without leading zeros in the title/H1).

## Fix Focus Areas
- docs/ADRs/NNNN-config-driven-agents.md[1-16]
- docs/plans/config-driven-agents.md[1-5]
- hack/lint-adr-frontmatter[154-157]
- docs/ADRs/0001-use-adrs-for-decision-making.md[39-63]

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

---

3. Consequences bullets are multi-sentence 📜 Skill insight ⚙ Maintainability

Description

The ADR Consequences section includes multi-sentence bullets and nested sub-bullets, rather than
3–5 one-sentence bullet points. This reduces scannability and violates the required consequences
format.

Code

docs/ADRs/NNNN-config-driven-agents.md[R151-181]

+## Consequences
+
+- **Extensibility.** Anyone can add an agent to the fullsend installation in their repo with
+  `fullsend agent add <url-or-path>` — no code change, no PR to
+  fullsend. Local paths support development and private harnesses.
+- **Uniform path.** First-party and third-party agents are registered
+  the same way. Extracting an agent to a standalone repo is a config
+  update, not a code change.
+- **Gradual migration.** The additive merge model means agents can be
+  extracted from the scaffold one at a time. Users only need config
+  entries for agents that have moved or been added — unchanged
+  scaffold agents continue to work. Once all agents are extracted,
+  config becomes authoritative and the scaffold fallback is removed.
+- **Org config dependency removed for per-repo installs.**
+  `allowed_remote_resources` in per-repo config eliminates the need
+  to read org config for base composition validation.
+- **Migration.** No forced migration. Existing installations continue
+  to work without changes:
+  - When `agents` is empty in config, the code falls back to the
+    current scaffold-based agent discovery. This fallback is
+    permanent until the config is populated — there is no deadline.
+  - Per-repo configs that lack `allowed_remote_resources` inherit
+    the org-level allowlist (if present) or use the scaffold
+    fallback path, which does not require an allowlist.
+  - To opt in, users can run `fullsend agent add` to populate the
+    list manually, or the config is backfilled automatically the
+    next time they run `fullsend admin install --upgrade`,
+    `fullsend github` (re-install), or `fullsend repos sync`
+    ([ADR 0057](0057-repos-management.md)).
+  - A deprecation notice is logged when the fallback is used, to
+    nudge users toward populating the config.

Relevance

⭐⭐ Medium

Repo reviews sometimes accept ADR wording tweaks, but no evidence of strict consequences bullet
formatting enforcement.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1062091 requires ## Consequences to have 3–5 one-sentence bullet points. The new
ADR’s consequences bullets include multiple sentences and a nested list under Migration.

docs/ADRs/NNNN-config-driven-agents.md[151-181]
Skill: writing-adrs

Agent prompt

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

## Issue description
`## Consequences` must be 3–5 bullets and each bullet must be exactly one sentence; the current section uses multi-sentence bullets and nested bullets.

## Issue Context
Keep the consequences as crisp, single-sentence statements; move detailed migration mechanics into the implementation plan.

## Fix Focus Areas
- docs/ADRs/NNNN-config-driven-agents.md[151-181]

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

---

View more (2)

4. ADR exceeds 100 lines 📜 Skill insight ⚙ Maintainability

Description

The new ADR’s body content is significantly longer than the 100-line maximum, increasing the risk it
is deciding too many things and duplicating details better suited for plans/architecture docs. This
violates the ADR length constraint.

Code

docs/ADRs/NNNN-config-driven-agents.md[R14-189]

+# NNNN. Config-driven agents
+
+Date: 2026-06-29
+
+## Status
+
+Accepted
+
+## Context
+
+Which agents fullsend knows about is currently compiled into the binary.
+Scaffold-embedded harnesses (`internal/scaffold/fullsend-repo/harness/`)
+define the canonical agent set, and a hardcoded `externalAgents` map in
+`baseurl.go` extends it for agents extracted to standalone repos. Both
+`HarnessNames()` and `HarnessWrappersLayer` read from these compiled-in
+sources to enumerate and generate harness wrappers.
+
+This creates three problems:
+
+1. **Adding an agent requires a code change.** A user who wants to run a
+   custom agent must modify the fullsend binary or work around it.
+2. **First-party agents extracted to external repos need special
+   handling.** The `externalAgents` map is a one-off mechanism that pins
+   commit SHAs and content hashes in Go source.
+3. **First-party and third-party agents follow different paths.**
+   Fullsend's own agents are discovered from the scaffold embed;
+   external agents are discovered from a hardcoded map. There is no
+   single, uniform mechanism.
+
+The triage agent extraction to `fullsend-ai/agents`
+([ADR 0045](0045-forge-portable-harness-schema.md) Phase 4) is the
+first agent to move out of the scaffold. More will follow. The
+registration mechanism must support both first-party extractions and
+user-defined agents without code changes to fullsend.
+
+## Decision
+
+Make agent registration a **config-level concept** in both org config
+and per-repo config.
+
+**Config schema.** Add an `agents` list to both `OrgConfig` and
+`PerRepoConfig`. Each entry is either a string shorthand or an object
+with an explicit name. The source can be a URL (for remote harnesses)
+or a local path (for harnesses on disk):
+
+```yaml
+# Per-repo config (.fullsend/config.yaml in target repo)
+version: "1"
+agents:
+  # String shorthand — name derived from filename
+  - https://raw.githubusercontent.com/fullsend-ai/fullsend/<sha>/internal/scaffold/fullsend-repo/harness/code.yaml#sha256=<hash>
+  - harness/my-custom-agent.yaml
+
+  # Object form — explicit name overrides filename
+  - name: triage
+    source: https://raw.githubusercontent.com/fullsend-ai/agents/<sha>/harness/triage.yaml#sha256=<hash>
+  - name: lint
+    source: harness/my-linter.yaml
+allowed_remote_resources:
+  - https://raw.githubusercontent.com/fullsend-ai/fullsend/
+  - https://raw.githubusercontent.com/fullsend-ai/agents/
+```
+
+An entry's source is treated as a URL if it starts with `https://`,
+and as a local path otherwise. Local paths resolve relative to the
+`.fullsend/` directory. When `name` is omitted (string shorthand), the
+agent name is derived from the filename (`triage.yaml` -> `triage`).
+When `name` is provided, it overrides the filename-derived name. Role
+and slug are read from the harness content at
+install/wrapper-generation time, not stored in config. URL entries
+include an integrity hash fragment for content verification
+([ADR 0038](0038-universal-harness-access.md)); local paths do not
+require one since the content is already trusted on disk.
+
+**`allowed_remote_resources` in per-repo config.** This field currently
+exists only in org config. Per-repo config gains it to support base
+composition without an org config repo. Both are checked at load time;
+per-repo entries are additive.
+
+**Install seeding.** During install, the default first-party agent
+URLs are populated into `agents` — in org config for org-mode
+installs (`fullsend admin install <org>`), in per-repo config for
+per-repo installs (`fullsend github` / `fullsend admin install
+<owner/repo>`). They use the same format as any user-added agent. The
+`--agents` flag continues to control which roles are installed; the
+mapping from role to default harness URL is a build-time constant (not
+a runtime concept).
+
+**CLI surface.** A `fullsend agent` subcommand group manages the
+agents list in config:
+
+| Command | Effect |
+|---------|--------|
+| `fullsend agent add <url-or-path>` | Fetch/read harness, validate, append to config |
+| `fullsend agent list` | List registered agents with name, role, source |
+| `fullsend agent update <name> [sha]` | Re-pin agent to latest HEAD or a specific commit |
+| `fullsend agent remove <name>` | Remove agent entry from config |
+
+`agent add` accepts a URL or a local path and validates the harness
+before appending it to config. `agent update` re-pins a URL-based
+agent to a new commit (latest HEAD or a specific SHA). Both commands
+automatically resolve unpinned URLs to fully pinned form — commit SHA
+and integrity hash are resolved and computed by the CLI so users never
+need to construct pinned URLs manually. The stored config entry is
+always fully pinned.
+
+**Additive merge model.** Config entries are merged with
+scaffold-discovered agents, not a replacement. Scaffold agents form
+the base set; config entries add new agents or override scaffold
+entries with the same name. The merge rule: config wins over scaffold
+when names collide. This enables gradual migration — agents can be
+extracted one at a time without requiring every config to list all
+agents.
+
+`fullsend agent list` shows the merged view (scaffold + config) so
+the full agent set is visible in one place.
+
+**Wrapper generation.** `HarnessWrappersLayer` (org mode) builds the
+merged agent set and generates wrappers from it. Per-repo install
+does the same. For URL entries, both generate the same thin wrapper
+format (`base:` + `role:` + `slug:`). For local path entries, the
+harness file is used directly — no wrapper is needed since the
+content is already on disk.
+
+**Removal of compiled-in agent map.** The `externalAgents` map,
+`IsExternalAgent()`, and the external-agent branches in
+`HarnessBaseURL()`/`HarnessContentHash()` are deleted.
+`HarnessNames()` returns only scaffold-embedded names (used as the
+base set for the additive merge).
+
+**Transition to authoritative config.** The additive model is a
+migration aid. Once all first-party agents have been extracted from
+the scaffold and all installations have populated `agents` in config,
+a follow-up change makes config authoritative: scaffold agents are no
+longer discovered, and `agents` is the complete list. An issue will
+be filed to track this transition once the last agent is extracted.
+
+## Consequences
+
+- **Extensibility.** Anyone can add an agent to the fullsend installation in their repo with
+  `fullsend agent add <url-or-path>` — no code change, no PR to
+  fullsend. Local paths support development and private harnesses.
+- **Uniform path.** First-party and third-party agents are registered
+  the same way. Extracting an agent to a standalone repo is a config
+  update, not a code change.
+- **Gradual migration.** The additive merge model means agents can be
+  extracted from the scaffold one at a time. Users only need config
+  entries for agents that have moved or been added — unchanged
+  scaffold agents continue to work. Once all agents are extracted,
+  config becomes authoritative and the scaffold fallback is removed.
+- **Org config dependency removed for per-repo installs.**
+  `allowed_remote_resources` in per-repo config eliminates the need
+  to read org config for base composition validation.
+- **Migration.** No forced migration. Existing installations continue
+  to work without changes:
+  - When `agents` is empty in config, the code falls back to the
+    current scaffold-based agent discovery. This fallback is
+    permanent until the config is populated — there is no deadline.
+  - Per-repo configs that lack `allowed_remote_resources` inherit
+    the org-level allowlist (if present) or use the scaffold
+    fallback path, which does not require an allowlist.
+  - To opt in, users can run `fullsend agent add` to populate the
+    list manually, or the config is backfilled automatically the
+    next time they run `fullsend admin install --upgrade`,
+    `fullsend github` (re-install), or `fullsend repos sync`
+    ([ADR 0057](0057-repos-management.md)).
+  - A deprecation notice is logged when the fallback is used, to
+    nudge users toward populating the config.
+
+## References
+
+- [ADR 0033](0033-per-repo-installation-mode.md) -- per-repo installation mode
+- [ADR 0038](0038-universal-harness-access.md) -- URL-based resource references and integrity hashes
+- [ADR 0045](0045-forge-portable-harness-schema.md) -- harness composition via `base:` URLs
+- [ADR 0057](0057-repos-management.md) -- repos management for per-repo installations
+- [Implementation plan](../plans/config-driven-agents.md)

Relevance

⭐⭐ Medium

No clear historical evidence of enforcing a hard 100-line ADR body limit in prior reviews/PRs.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1062092 requires ADR content to be <= 100 lines (excluding frontmatter). The added
ADR spans through line 189 with substantial body content after the frontmatter, exceeding this
limit.

docs/ADRs/NNNN-config-driven-agents.md[14-189]
Skill: writing-adrs

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 content exceeds the 100-line maximum (excluding frontmatter), which violates the ADR length constraint.

## Issue Context
This ADR includes extensive schema details, CLI UX, merge rules, and migration plan content that can be moved/condensed (e.g., into `docs/plans/config-driven-agents.md`) while keeping the ADR focused on the core decision.

## Fix Focus Areas
- docs/ADRs/NNNN-config-driven-agents.md[14-189]

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

---

5. Decision section has multiple decisions 📜 Skill insight ⚙ Maintainability

Description

The ADR Decision section bundles multiple distinct decisions (schema, CLI behavior, wrapper
generation, removal of maps, migration/authoritative transition). This violates the requirement that
each ADR record exactly one decision.

Code

docs/ADRs/NNNN-config-driven-agents.md[R49-149]

+## Decision
+
+Make agent registration a **config-level concept** in both org config
+and per-repo config.
+
+**Config schema.** Add an `agents` list to both `OrgConfig` and
+`PerRepoConfig`. Each entry is either a string shorthand or an object
+with an explicit name. The source can be a URL (for remote harnesses)
+or a local path (for harnesses on disk):
+
+```yaml
+# Per-repo config (.fullsend/config.yaml in target repo)
+version: "1"
+agents:
+  # String shorthand — name derived from filename
+  - https://raw.githubusercontent.com/fullsend-ai/fullsend/<sha>/internal/scaffold/fullsend-repo/harness/code.yaml#sha256=<hash>
+  - harness/my-custom-agent.yaml
+
+  # Object form — explicit name overrides filename
+  - name: triage
+    source: https://raw.githubusercontent.com/fullsend-ai/agents/<sha>/harness/triage.yaml#sha256=<hash>
+  - name: lint
+    source: harness/my-linter.yaml
+allowed_remote_resources:
+  - https://raw.githubusercontent.com/fullsend-ai/fullsend/
+  - https://raw.githubusercontent.com/fullsend-ai/agents/
+```
+
+An entry's source is treated as a URL if it starts with `https://`,
+and as a local path otherwise. Local paths resolve relative to the
+`.fullsend/` directory. When `name` is omitted (string shorthand), the
+agent name is derived from the filename (`triage.yaml` -> `triage`).
+When `name` is provided, it overrides the filename-derived name. Role
+and slug are read from the harness content at
+install/wrapper-generation time, not stored in config. URL entries
+include an integrity hash fragment for content verification
+([ADR 0038](0038-universal-harness-access.md)); local paths do not
+require one since the content is already trusted on disk.
+
+**`allowed_remote_resources` in per-repo config.** This field currently
+exists only in org config. Per-repo config gains it to support base
+composition without an org config repo. Both are checked at load time;
+per-repo entries are additive.
+
+**Install seeding.** During install, the default first-party agent
+URLs are populated into `agents` — in org config for org-mode
+installs (`fullsend admin install <org>`), in per-repo config for
+per-repo installs (`fullsend github` / `fullsend admin install
+<owner/repo>`). They use the same format as any user-added agent. The
+`--agents` flag continues to control which roles are installed; the
+mapping from role to default harness URL is a build-time constant (not
+a runtime concept).
+
+**CLI surface.** A `fullsend agent` subcommand group manages the
+agents list in config:
+
+| Command | Effect |
+|---------|--------|
+| `fullsend agent add <url-or-path>` | Fetch/read harness, validate, append to config |
+| `fullsend agent list` | List registered agents with name, role, source |
+| `fullsend agent update <name> [sha]` | Re-pin agent to latest HEAD or a specific commit |
+| `fullsend agent remove <name>` | Remove agent entry from config |
+
+`agent add` accepts a URL or a local path and validates the harness
+before appending it to config. `agent update` re-pins a URL-based
+agent to a new commit (latest HEAD or a specific SHA). Both commands
+automatically resolve unpinned URLs to fully pinned form — commit SHA
+and integrity hash are resolved and computed by the CLI so users never
+need to construct pinned URLs manually. The stored config entry is
+always fully pinned.
+
+**Additive merge model.** Config entries are merged with
+scaffold-discovered agents, not a replacement. Scaffold agents form
+the base set; config entries add new agents or override scaffold
+entries with the same name. The merge rule: config wins over scaffold
+when names collide. This enables gradual migration — agents can be
+extracted one at a time without requiring every config to list all
+agents.
+
+`fullsend agent list` shows the merged view (scaffold + config) so
+the full agent set is visible in one place.
+
+**Wrapper generation.** `HarnessWrappersLayer` (org mode) builds the
+merged agent set and generates wrappers from it. Per-repo install
+does the same. For URL entries, both generate the same thin wrapper
+format (`base:` + `role:` + `slug:`). For local path entries, the
+harness file is used directly — no wrapper is needed since the
+content is already on disk.
+
+**Removal of compiled-in agent map.** The `externalAgents` map,
+`IsExternalAgent()`, and the external-agent branches in
+`HarnessBaseURL()`/`HarnessContentHash()` are deleted.
+`HarnessNames()` returns only scaffold-embedded names (used as the
+base set for the additive merge).
+
+**Transition to authoritative config.** The additive model is a
+migration aid. Once all first-party agents have been extracted from
+the scaffold and all installations have populated `agents` in config,
+a follow-up change makes config authoritative: scaffold agents are no
+longer discovered, and `agents` is the complete list. An issue will
+be filed to track this transition once the last agent is extracted.

Relevance

⭐⭐ Medium

No strong prior evidence found that reviewers enforce “one decision per ADR” vs bundled decisions.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1062089 requires each ADR to record exactly one decision. The ## Decision section
contains several separately scoped decisions (config schema, per-repo allowlist, install seeding,
CLI commands, merge model, wrapper generation, compiled-in map removal, and transition plan).

docs/ADRs/NNNN-config-driven-agents.md[49-149]
Skill: writing-adrs

Agent prompt

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

## Issue description
This ADR records multiple distinct decisions inside a single `## Decision` section.

## Issue Context
To comply, keep one clear decision in this ADR (e.g., “agents are declared in config”) and move the rest to either:
- separate ADRs (e.g., CLI surface, additive merge model, authoritative transition), and/or
- the implementation plan document.

## Fix Focus Areas
- docs/ADRs/NNNN-config-driven-agents.md[49-149]

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

---

6. ExternalAgents docs mismatch 🐞 Bug ⚙ Maintainability

Description

The ADR describes a hardcoded externalAgents map in internal/scaffold/baseurl.go, but the
current codebase has no such map. This makes the ADR’s “current state” description (and related plan
steps) inaccurate and likely to mislead implementation work.

Code

docs/ADRs/NNNN-config-driven-agents.md[R24-29]

+Which agents fullsend knows about is currently compiled into the binary.
+Scaffold-embedded harnesses (`internal/scaffold/fullsend-repo/harness/`)
+define the canonical agent set, and a hardcoded `externalAgents` map in
+`baseurl.go` extends it for agents extracted to standalone repos. Both
+`HarnessNames()` and `HarnessWrappersLayer` read from these compiled-in
+sources to enumerate and generate harness wrappers.

Relevance

⭐⭐⭐ High

Review history shows ADR factual-accuracy corrections are accepted (e.g., fixes to inaccurate ADR
statements in #1688/#1814).

PR-#1688
PR-#1814

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The ADR asserts externalAgents exists and is used for harness enumeration, but the referenced code
file contains only scaffold URL/hash helpers and wrapper generation uses those helpers directly;
there is no external-agent map to remove.

docs/ADRs/NNNN-config-driven-agents.md[24-37]
docs/plans/config-driven-agents.md[272-288]
internal/scaffold/baseurl.go[13-83]
internal/layers/harnesswrappers.go[88-118]

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 new ADR claims there is a hardcoded `externalAgents` map in `baseurl.go` that extends scaffold harness discovery for extracted agents. In the current repo state, `internal/scaffold/baseurl.go` contains only scaffold URL/hash helpers and `HarnessNames()`—there is no `externalAgents` map or `IsExternalAgent()` helper.

This makes the ADR’s Context section (and Phase 4a of the plan) inaccurate, which will confuse readers/implementers about what actually exists today and what needs to be removed.

### Issue Context
- Wrapper generation currently directly uses `scaffold.HarnessBaseURLWithHash(...)`.
- `internal/scaffold/baseurl.go` has no external-agent branching.

### Fix Focus Areas
- docs/ADRs/NNNN-config-driven-agents.md[24-40]
- docs/plans/config-driven-agents.md[272-289]
- internal/scaffold/baseurl.go[13-83]
- internal/layers/harnesswrappers.go[88-118]

### Suggested fix
1. Update the ADR Context to describe the current mechanism accurately (scaffold-embedded harnesses + base URL/hash helpers).
2. Update the implementation plan Phase 4a to remove/replace the `externalAgents` deletion steps, since those symbols are not present, and adjust the phase description to match the actual cleanup needed in this repo.
3. If an external-agent map existed historically, clarify that it was removed earlier (and link the relevant ADR/PR if desired), rather than describing it as current state.

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

---

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 4:58 PM UTC · Ended 5:02 PM UTC
Commit: ea2ca95 · View workflow run →

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 5:06 PM UTC · Ended 5:13 PM UTC
Commit: ea2ca95 · View workflow run →

[github-actions]

Site preview

Preview: https://0bea3843-site.fullsend-ai.workers.dev

Commit: ab1a403bc0da3d8c5d082ffab6ee775544a8d7ac

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 5:19 PM UTC · Ended 5:46 PM UTC
Commit: ea2ca95 · View workflow run →

[ralphbean]

I think this needs some changes. See inline comments.

[ralphbean]

LGTM.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 5:49 PM UTC · Completed 6:03 PM UTC
Commit: d2d28f5 · View workflow run →

[fullsend-ai-review]

Review

Findings

Medium

• [internal-consistency] docs/plans/agent-registration.md:322 — Phase 4g instructs updating tests to "remove IsExternalAgent skip branches" in internal/scaffold/scaffold_test.go and internal/harness/scaffold_integration_test.go. However, IsExternalAgent does not exist anywhere in the current codebase (grep returns zero results). These test update instructions reference a non-existent identifier, which will cause confusion during implementation.
  Remediation: Remove or correct the references to IsExternalAgent in Phase 4g. If this identifier is expected to be introduced by a prior PR (e.g., the triage extraction), note that dependency explicitly.

• [coherence-gap] docs/ADRs/0058-agent-registration.md:61 — The ADR states "Per-repo config is read from the base branch, not the PR branch, so a PR cannot inject an attacker-controlled allowed_remote_resources entry or agent source." However, the security model does not address how this interacts with ADR 0033's per-repo installation mode where the config repo IS the target repo. An attacker with merge rights can still inject malicious entries via a merged PR.
  Remediation: Clarify the trust boundary for per-repo installations: Is CODEOWNERS protection on .fullsend/config.yaml required? How does this interact with the base-branch read model?

Low

• [scope-drift] docs/ADRs/0058-agent-registration.md:38 — The ADR re-introduces an agents config key that was previously removed by ADR 0045 Phase 4, with different semantics (URL/path sources vs role/name/slug tuples). The cross-reference added to ADR 0045 explicitly notes the supersession, which addresses the formal linkage concern.

• [internal-consistency] docs/plans/agent-registration.md:89 — Phase 1c says call sites that construct ComposeOpts.OrgAllowlist must merge both org and per-repo allowlists. The field name OrgAllowlist and its doc comment imply org-only scope. Adding per-repo entries into this field is a semantic mismatch that could cause maintenance confusion.
  Remediation: Add a note in Phase 1c to rename OrgAllowlist to Allowlist (or similar) or update its doc comment to reflect merged semantics.

• [concurrency-safety] docs/ADRs/0058-agent-registration.md:62 — The ADR notes "single-user CLI operations; no concurrency guard on config read/write." Concurrent fullsend agent add and fullsend run could produce a partially-written config file. This is already documented as an accepted limitation.

• [schema-compatibility] docs/ADRs/0058-agent-registration.md:80 — The Consequences section notes a custom unmarshaler will detect and reject old-format entries. The plan's Phase 1a defines the unmarshaler for string-vs-mapping dispatch but does not mention old-format detection logic. Explicit old-format detection with a user-friendly error message (as the ADR promises) should be called out in the plan.
  Remediation: Add a step in Phase 1a or 1b to implement old-format detection (reject mapping nodes containing role or slug keys with a descriptive error message).

• [file-naming-convention] docs/plans/agent-registration.md — The implementation plan filename agent-registration.md does not follow the established naming convention for ADR-related implementation plans. Existing plans use the format adr-NNNN-description-phaseN.md.
  Remediation: Consider renaming to adr-0058-agent-registration.md to match the established pattern.

Previous run

Review

Findings

Medium

• [architectural-contradiction] docs/ADRs/0058-agent-registration.md — This ADR proposes re-adding an agents: list to OrgConfig and PerRepoConfig, reusing the same YAML key that ADR 0045 Phase 4 removed. The ADR explicitly acknowledges this at lines 39–42 and in Consequences (line 82–84), explaining the different semantics (harness source URLs vs role/name/slug identity tuples) and proposes a custom unmarshaler for old-format rejection. However, per project ADR conventions, reversing a decided removal trajectory warrants a formal supersession annotation on ADR 0045 rather than an inline parenthetical note, so the decision trail is clear to future readers.
  Remediation: Add a supersession note to ADR 0045 Phase 4 (or add ADR 0058 to ADR 0045's relates_to frontmatter) so readers following the decision chain discover the relationship from both directions.

• [edge-case-handling] docs/ADRs/0058-agent-registration.md:58 — The ADR states "config wins on name collision" but does not define what constitutes the "name" used for collision detection. Config entries support an optional name override, and harness files contain a role field. If a config entry specifies name: triage but the harness at the URL has role: custom-triage, it is ambiguous which field is the collision key. This could lead to unexpected shadowing or duplicate agents.
  Remediation: Clarify in the ADR or implementation plan which field serves as the collision key for additive merge (config name, harness role, or derived filename), and specify behavior when these disagree.

Low

• [concurrency-safety] docs/ADRs/0058-agent-registration.md:62 — The ADR notes "single-user CLI operations; no concurrency guard on config read/write." Concurrent fullsend agent add and fullsend run could produce a partially-written config file. This is already documented as an accepted limitation.

• [internal-consistency] docs/plans/agent-registration.md:89 — Phase 1c says "Merge logic in internal/harness/compose.go (matchingAllowedPrefix) must check both org and per-repo allowlists." However, matchingAllowedPrefix is a stateless helper that receives a pre-constructed []string allowlist. The actual merge of org and per-repo allowlists must happen at the caller site, not inside matchingAllowedPrefix.

• [schema-compatibility] docs/ADRs/0058-agent-registration.md:80 — The Consequences section correctly notes a custom unmarshaler will detect and reject old-format entries. This is a forward implementation requirement — without the unmarshaler, Go's default yaml.Unmarshal would produce a type mismatch error with no user-friendly message.

• [scope-expansion] docs/plans/agent-registration.md — The 5-phase implementation plan should explicitly reference how it relates to ADR 0045's migration phases, since ADR 0058 re-introduces config-level registration that ADR 0045 removed.

• [potential-stale-terminology] docs/plans/agent-registration.md — The plan uses "config-level agent registration" terminology. Since ADR 0045 Phase 4 already removed the agents block from OrgConfig (confirmed in codebase), the plan should acknowledge it is re-introducing config-level registration with a new schema, not describing the current state.

• [missing-cross-references] docs/architecture.md — The new Decided section references ADR 0058 but does not mention the relationship with ADR 0045, which established harness-level agent identity. A brief note connecting the two decisions would help readers.

Previous run

Review

Findings

High

• [adr-numbering] docs/ADRs/NNNN-agent-registration.md — ADR uses placeholder NNNN instead of a sequential number. The highest existing ADR on main is 0057, so the next should be 0058. The placeholder propagates to cross-references in docs/architecture.md and docs/plans/agent-registration.md. Merging with NNNN creates a non-functional reference chain.
  Remediation: Rename the file to 0058-agent-registration.md and replace all occurrences of NNNN with 0058 across the three files.

Medium

• [schema-compatibility] docs/ADRs/NNNN-agent-registration.md:44 — The ADR proposes re-adding an agents YAML key to OrgConfig with a different schema (list of URL/path entries) than the one ADR 0045 Phase 4 removed (list of role/name/slug identity tuples). Since Phase 4 removed the Go struct field, old config files with the prior-format agents: block are currently silently ignored by yaml.Unmarshal. Re-adding the field with a new type would cause those old entries to fail parsing or produce corrupt data. The ADR acknowledges the key reuse but does not address how old-format entries are handled during the transition.
  Remediation: Add a consequence or migration note addressing how old config files with the prior agents: schema will behave. Consider a custom unmarshaler that detects and rejects old-format entries with a clear error, or a schema version bump.

Low

• [missing-authorization] docs/ADRs/NNNN-agent-registration.md — This PR adds 534 lines across 3 files proposing an architectural change with no linked issue. While this is a docs-only PR (ADR + implementation plan), a linked issue documenting the intent would improve traceability.

• [incomplete-architecture-update] docs/architecture.md — The new Decided entry under Agent Registry uses bulleted lists with ADR citations, matching the established pattern. Minor: it could include more detail about consequences or trade-offs to match the most detailed existing entries.

• [architectural-coherence] docs/ADRs/NNNN-agent-registration.md — The ADR re-adds an agents list to OrgConfig, which ADR 0045 Phase 4 explicitly removed. The ADR acknowledges the change in semantics (harness source URLs rather than role/name/slug identity tuples), but per project ADR conventions, reversing a decided removal trajectory should be captured as a formal supersession rather than an inline parenthetical note.

• [internal-consistency] docs/plans/agent-registration.md:89 — Phase 1c says "Merge logic in internal/harness/compose.go (matchingAllowedPrefix) must check both org and per-repo allowlists." However, matchingAllowedPrefix is a stateless helper that receives a pre-constructed []string allowlist — it has no knowledge of org vs per-repo config. The actual merge of allowlists would need to happen at the caller site, not inside matchingAllowedPrefix.

Previous run (2)

Review

Findings

High

• [adr-numbering] docs/ADRs/NNNN-agent-registration.md — ADR uses placeholder NNNN instead of a sequential number. The highest existing ADR on main is 0057, so the next should be 0058. The placeholder appears in the filename, the title (frontmatter and heading), and in docs/architecture.md's cross-reference (ADR NNNN). This project has encountered numbering conflicts before (ADR 0038 notes it was numbered to avoid collision with PR Design: cross-org issue filing with bidirectional authorization (#672) #980). Leaving NNNN risks merge conflicts, broken cross-references, and confusion.
  Remediation: Renumber to 0058 (or next available). Update the filename (0058-agent-registration.md), the title in both frontmatter and heading, and the docs/architecture.md reference.

Medium

• [technical-accuracy] docs/ADRs/NNNN-agent-registration.md:30 — The Context section states "a hardcoded externalAgents map in baseurl.go extends it for agents extracted to standalone repos." No such map exists in internal/scaffold/baseurl.go or anywhere in the repository — externalAgents, ExternalAgent, and IsExternalAgent all return zero grep hits. The ADR's motivation is partly grounded in a mechanism that does not exist in the current codebase, and the implementation plan's Phase 4a references removing these non-existent constructs.
  Remediation: Correct the Context section to reference the actual mechanism for handling extracted agents, or note that externalAgents is planned to be added in a concurrent PR.

• [incomplete-architecture-update] docs/architecture.md — The new Decided: entry under Agent Registry is a single-sentence pointer: "Agents are declared in config.yaml... See [ADR NNNN]." The established pattern throughout architecture.md (Agent Infrastructure lines 38–48, Agent Harness lines 82–110, Agent Identity Provider lines 140–146) uses bulleted lists with concise decision summaries and ADR citations in parentheses. The current entry is too sparse for a reader to understand the decision without leaving the document.
  Remediation: Expand to match the established pattern — use a bullet list with key design details (config-level registration, URL/path entries, additive merge, runtime resolution) and the ADR link in parentheses.

Low

• [architectural-coherence] docs/ADRs/NNNN-agent-registration.md — The ADR re-adds an agents list to OrgConfig, which ADR 0045 Phase 4 explicitly removed. The ADR does acknowledge the change in semantics ("harness source URLs rather than role/name/slug identity tuples"), but per project ADR conventions, reversing a decided removal trajectory should be captured as a formal supersession rather than an inline parenthetical note.

• [internal-consistency] docs/plans/agent-registration.md:89 — Phase 1c says "Merge logic in internal/harness/compose.go (matchingAllowedPrefix) must check both org and per-repo allowlists." However, matchingAllowedPrefix is a stateless helper that receives a pre-constructed []string allowlist — it has no knowledge of org vs per-repo config. The actual merge point is ComposeOpts.OrgAllowlist, assembled by callers. The plan should clarify that the allowlist merge happens at the call site, not inside matchingAllowedPrefix.

---

Labels: PR adds documentation: a new ADR and implementation plan under docs/

---

Labels: ADR and implementation plan for agent registration directly covers harness config and loading.

Previous run

Review

Findings

Medium

• [architectural-contradiction] docs/ADRs/0058-agent-registration.md — This ADR proposes re-adding an agents: list to OrgConfig and PerRepoConfig, reusing the same YAML key that ADR 0045 Phase 4 removed. The ADR explicitly acknowledges this at lines 39–42 and in Consequences (line 82–84), explaining the different semantics (harness source URLs vs role/name/slug identity tuples) and proposes a custom unmarshaler for old-format rejection. However, per project ADR conventions, reversing a decided removal trajectory warrants a formal supersession annotation on ADR 0045 rather than an inline parenthetical note, so the decision trail is clear to future readers.
  Remediation: Add a supersession note to ADR 0045 Phase 4 (or add ADR 0058 to ADR 0045's relates_to frontmatter) so readers following the decision chain discover the relationship from both directions.

• [edge-case-handling] docs/ADRs/0058-agent-registration.md:58 — The ADR states "config wins on name collision" but does not define what constitutes the "name" used for collision detection. Config entries support an optional name override, and harness files contain a role field. If a config entry specifies name: triage but the harness at the URL has role: custom-triage, it is ambiguous which field is the collision key. This could lead to unexpected shadowing or duplicate agents.
  Remediation: Clarify in the ADR or implementation plan which field serves as the collision key for additive merge (config name, harness role, or derived filename), and specify behavior when these disagree.

Low

• [concurrency-safety] docs/ADRs/0058-agent-registration.md:62 — The ADR notes "single-user CLI operations; no concurrency guard on config read/write." Concurrent fullsend agent add and fullsend run could produce a partially-written config file. This is already documented as an accepted limitation.

• [internal-consistency] docs/plans/agent-registration.md:89 — Phase 1c says "Merge logic in internal/harness/compose.go (matchingAllowedPrefix) must check both org and per-repo allowlists." However, matchingAllowedPrefix is a stateless helper that receives a pre-constructed []string allowlist. The actual merge of org and per-repo allowlists must happen at the caller site, not inside matchingAllowedPrefix.

• [schema-compatibility] docs/ADRs/0058-agent-registration.md:80 — The Consequences section correctly notes a custom unmarshaler will detect and reject old-format entries. This is a forward implementation requirement — without the unmarshaler, Go's default yaml.Unmarshal would produce a type mismatch error with no user-friendly message.

• [scope-expansion] docs/plans/agent-registration.md — The 5-phase implementation plan should explicitly reference how it relates to ADR 0045's migration phases, since ADR 0058 re-introduces config-level registration that ADR 0045 removed.

• [potential-stale-terminology] docs/plans/agent-registration.md — The plan uses "config-level agent registration" terminology. Since ADR 0045 Phase 4 already removed the agents block from OrgConfig (confirmed in codebase), the plan should acknowledge it is re-introducing config-level registration with a new schema, not describing the current state.

• [missing-cross-references] docs/architecture.md — The new Decided section references ADR 0058 but does not mention the relationship with ADR 0045, which established harness-level agent identity. A brief note connecting the two decisions would help readers.

Previous run (2)

Review

Findings

High

• [adr-numbering] docs/ADRs/NNNN-agent-registration.md — ADR uses placeholder NNNN instead of a sequential number. The highest existing ADR on main is 0057, so the next should be 0058. The placeholder propagates to cross-references in docs/architecture.md and docs/plans/agent-registration.md. Merging with NNNN creates a non-functional reference chain.
  Remediation: Rename the file to 0058-agent-registration.md and replace all occurrences of NNNN with 0058 across the three files.

Medium

• [schema-compatibility] docs/ADRs/NNNN-agent-registration.md:44 — The ADR proposes re-adding an agents YAML key to OrgConfig with a different schema (list of URL/path entries) than the one ADR 0045 Phase 4 removed (list of role/name/slug identity tuples). Since Phase 4 removed the Go struct field, old config files with the prior-format agents: block are currently silently ignored by yaml.Unmarshal. Re-adding the field with a new type would cause those old entries to fail parsing or produce corrupt data. The ADR acknowledges the key reuse but does not address how old-format entries are handled during the transition.
  Remediation: Add a consequence or migration note addressing how old config files with the prior agents: schema will behave. Consider a custom unmarshaler that detects and rejects old-format entries with a clear error, or a schema version bump.

Low

• [missing-authorization] docs/ADRs/NNNN-agent-registration.md — This PR adds 534 lines across 3 files proposing an architectural change with no linked issue. While this is a docs-only PR (ADR + implementation plan), a linked issue documenting the intent would improve traceability.

• [incomplete-architecture-update] docs/architecture.md — The new Decided entry under Agent Registry uses bulleted lists with ADR citations, matching the established pattern. Minor: it could include more detail about consequences or trade-offs to match the most detailed existing entries.

• [architectural-coherence] docs/ADRs/NNNN-agent-registration.md — The ADR re-adds an agents list to OrgConfig, which ADR 0045 Phase 4 explicitly removed. The ADR acknowledges the change in semantics (harness source URLs rather than role/name/slug identity tuples), but per project ADR conventions, reversing a decided removal trajectory should be captured as a formal supersession rather than an inline parenthetical note.

• [internal-consistency] docs/plans/agent-registration.md:89 — Phase 1c says "Merge logic in internal/harness/compose.go (matchingAllowedPrefix) must check both org and per-repo allowlists." However, matchingAllowedPrefix is a stateless helper that receives a pre-constructed []string allowlist — it has no knowledge of org vs per-repo config. The actual merge of allowlists would need to happen at the caller site, not inside matchingAllowedPrefix.

Previous run (3)

Review

Findings

High

• [adr-numbering] docs/ADRs/NNNN-agent-registration.md — ADR uses placeholder NNNN instead of a sequential number. The highest existing ADR on main is 0057, so the next should be 0058. The placeholder propagates to cross-references in docs/architecture.md and docs/plans/agent-registration.md. Merging with NNNN creates a non-functional reference chain.
  Remediation: Rename the file to 0058-agent-registration.md and replace all occurrences of NNNN with 0058 across the three files.

Medium

• [schema-compatibility] docs/ADRs/NNNN-agent-registration.md:44 — The ADR proposes re-adding an agents YAML key to OrgConfig with a different schema (list of URL/path entries) than the one ADR 0045 Phase 4 removed (list of role/name/slug identity tuples). Since Phase 4 removed the Go struct field, old config files with the prior-format agents: block are currently silently ignored by yaml.Unmarshal. Re-adding the field with a new type would cause those old entries to fail parsing or produce corrupt data. The ADR acknowledges the key reuse but does not address how old-format entries are handled during the transition.
  Remediation: Add a consequence or migration note addressing how old config files with the prior agents: schema will behave. Consider a custom unmarshaler that detects and rejects old-format entries with a clear error, or a schema version bump.

Low

• [missing-authorization] docs/ADRs/NNNN-agent-registration.md — This PR adds 534 lines across 3 files proposing an architectural change with no linked issue. While this is a docs-only PR (ADR + implementation plan), a linked issue documenting the intent would improve traceability.

• [incomplete-architecture-update] docs/architecture.md — The new Decided entry under Agent Registry uses bulleted lists with ADR citations, matching the established pattern. Minor: it could include more detail about consequences or trade-offs to match the most detailed existing entries.

• [architectural-coherence] docs/ADRs/NNNN-agent-registration.md — The ADR re-adds an agents list to OrgConfig, which ADR 0045 Phase 4 explicitly removed. The ADR acknowledges the change in semantics (harness source URLs rather than role/name/slug identity tuples), but per project ADR conventions, reversing a decided removal trajectory should be captured as a formal supersession rather than an inline parenthetical note.

• [internal-consistency] docs/plans/agent-registration.md:89 — Phase 1c says "Merge logic in internal/harness/compose.go (matchingAllowedPrefix) must check both org and per-repo allowlists." However, matchingAllowedPrefix is a stateless helper that receives a pre-constructed []string allowlist — it has no knowledge of org vs per-repo config. The actual merge of allowlists would need to happen at the caller site, not inside matchingAllowedPrefix.

Previous run (4)

Review

Findings

High

• [adr-numbering] docs/ADRs/NNNN-agent-registration.md — ADR uses placeholder NNNN instead of a sequential number. The highest existing ADR on main is 0057, so the next should be 0058. The placeholder appears in the filename, the title (frontmatter and heading), and in docs/architecture.md's cross-reference (ADR NNNN). This project has encountered numbering conflicts before (ADR 0038 notes it was numbered to avoid collision with PR Design: cross-org issue filing with bidirectional authorization (#672) #980). Leaving NNNN risks merge conflicts, broken cross-references, and confusion.
  Remediation: Renumber to 0058 (or next available). Update the filename (0058-agent-registration.md), the title in both frontmatter and heading, and the docs/architecture.md reference.

Medium

• [technical-accuracy] docs/ADRs/NNNN-agent-registration.md:30 — The Context section states "a hardcoded externalAgents map in baseurl.go extends it for agents extracted to standalone repos." No such map exists in internal/scaffold/baseurl.go or anywhere in the repository — externalAgents, ExternalAgent, and IsExternalAgent all return zero grep hits. The ADR's motivation is partly grounded in a mechanism that does not exist in the current codebase, and the implementation plan's Phase 4a references removing these non-existent constructs.
  Remediation: Correct the Context section to reference the actual mechanism for handling extracted agents, or note that externalAgents is planned to be added in a concurrent PR.

• [incomplete-architecture-update] docs/architecture.md — The new Decided: entry under Agent Registry is a single-sentence pointer: "Agents are declared in config.yaml... See [ADR NNNN]." The established pattern throughout architecture.md (Agent Infrastructure lines 38–48, Agent Harness lines 82–110, Agent Identity Provider lines 140–146) uses bulleted lists with concise decision summaries and ADR citations in parentheses. The current entry is too sparse for a reader to understand the decision without leaving the document.
  Remediation: Expand to match the established pattern — use a bullet list with key design details (config-level registration, URL/path entries, additive merge, runtime resolution) and the ADR link in parentheses.

Low

• [architectural-coherence] docs/ADRs/NNNN-agent-registration.md — The ADR re-adds an agents list to OrgConfig, which ADR 0045 Phase 4 explicitly removed. The ADR does acknowledge the change in semantics ("harness source URLs rather than role/name/slug identity tuples"), but per project ADR conventions, reversing a decided removal trajectory should be captured as a formal supersession rather than an inline parenthetical note.

• [internal-consistency] docs/plans/agent-registration.md:89 — Phase 1c says "Merge logic in internal/harness/compose.go (matchingAllowedPrefix) must check both org and per-repo allowlists." However, matchingAllowedPrefix is a stateless helper that receives a pre-constructed []string allowlist — it has no knowledge of org vs per-repo config. The actual merge point is ComposeOpts.OrgAllowlist, assembled by callers. The plan should clarify that the allowlist merge happens at the call site, not inside matchingAllowedPrefix.

---

Labels: PR adds documentation: a new ADR and implementation plan under docs/

[fullsend-ai-review]

See the review comment for full details.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:10 PM UTC · Completed 6:22 PM UTC
Commit: e897ec4 · View workflow run →

[fullsend-ai-review]

See the review comment for full details.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:32 PM UTC · Completed 6:44 PM UTC
Commit: ad22962 · View workflow run →

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:56 PM UTC · Completed 7:24 PM UTC
Commit: ab1a403 · View workflow run →

[fullsend-ai-retro]

🤖 Finished Retro · ❌ Failure · Started 12:44 AM UTC · Completed 12:51 AM UTC
Commit: ab1a403 · View workflow run →