feat(scaffold): per-repo pre-commit tools registry (L2 additive merge)

[waynesun09]

Summary

Extends the pre-commit tools registry (#1055) with per-repo additive merge:

• L2 additive merge: Target repos place .pre-commit-tools.yaml at repo root to extend the upstream/org registry. New entries append, matching (repo, hook_id) entries override, exclude: true suppresses.
• Supply-chain security: Per-repo registries are read from the base branch (git show origin/${TARGET_BRANCH}:...), not the PR head. Changes take effect only after merge.
• P1 fix: Add uv match entry so hooks with entry: "uv run ..." are recognized alongside uvx.
• P2 fix: Document that shellcheck-py is auto-managed by pre-commit (no registry entry needed).

Three-layer resolution

upstream defaults (fullsend-ai/fullsend)
  → org override: customized/scripts/.pre-commit-tools.yaml  (L1, full replacement)
    → per-repo additive: .pre-commit-tools.yaml at repo root (L2, additive merge)

Changes

File	Change
docs/ADRs/0056-per-repo-precommit-tools-registry.md	New ADR documenting design, security model, merge semantics
resolve-precommit-tools.py	Refactor resolve() to accept dict, add merge_registries(), add --local-registry CLI arg
.pre-commit-tools.yaml	Add uv match entry (P1), shellcheck-py note (P2), update header docs
post-code.sh, post-fix.sh, pre-code.sh, pre-fix.sh	Extract base-branch registry and pass --local-registry
customizing-agents.md	New "Customizing Pre-commit Tool Dependencies" section
test_resolve_precommit_tools.py	13 unit tests for merge, exclude, dedup, uv match, malformed input

Test plan

• python3 internal/scaffold/scripts-tests/test_resolve_precommit_tools.py — 13/13 pass
• go test ./internal/scaffold/... — existing Go tests pass
• make lint — passes
• Manual: hook with entry: "uv run mypy" matches the uv registry entry
• Manual: exclude: true on gitleaks suppresses the upstream entry
• Manual: local registry extends upstream without replacing it

Closes #1270

[github-actions]

Site preview

Preview: https://2007b98a-site.fullsend-ai.workers.dev

Commit: 88729ed5550381dadf9f65ab1d60326a440ba781

[qodo-code-review]

PR Summary by Qodo

feat(scaffold): per-repo pre-commit tools registry with L2 additive merge
✨ Enhancement 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

Description

• Adds merge_registries() to resolve-precommit-tools.py enabling per-repo
 .pre-commit-tools.yaml to extend (append), override (by repo+hook_id key), or suppress
 (exclude: true) upstream registry entries without replacing the entire registry.
• Caller scripts (post-code.sh, post-fix.sh, pre-code.sh, pre-fix.sh) now extract the
 per-repo registry from the **base branch** via git show origin/${TARGET_BRANCH}:... and pass it
 via a new --local-registry CLI arg — preventing untrusted PR content from influencing tool
 installation.
• Fixes P1: adds a uv match_entry so hooks using entry: "uv run ..." are recognized alongside
 the existing uvx entry, with dedup via seen_names.
• Adds 13 unit tests covering merge semantics (new entry, override, exclude, order, dedup, malformed
 input) and end-to-end resolution.
• Documents the three-layer resolution model (upstream → L1 org replacement → L2 per-repo additive)
 in ADR 0056 and the customizing-agents.md guide.

Diagram

graph TD
    GIT["git show origin/BASE_BRANCH"] --> LOCAL_REG["Local Registry\n.pre-commit-tools.yaml"]
    UPSTREAM_REG["Upstream Registry\n.pre-commit-tools.yaml"] --> MERGE
    LOCAL_REG --> MERGE["merge_registries()"]
    MERGE --> RESOLVE["resolve()"]
    PRECOMMIT[".pre-commit-config.yaml"] --> RESOLVE
    RESOLVE --> MANIFEST["JSON Manifest"]
    MANIFEST --> INSTALL["install-precommit-tools.sh"]

    subgraph Callers
        direction LR
        S1["pre-code.sh"] ~~~ S2["pre-fix.sh"] ~~~ S3["post-code.sh"] ~~~ S4["post-fix.sh"]
    end
    Callers --> GIT
    Callers --> RESOLVE

    subgraph Legend
        direction LR
        _file["File"] ~~~ _fn(["Function"]) ~~~ _script{{"Shell Script"}}
    end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. CODEOWNERS gate on .pre-commit-tools.yaml

• ➕ Makes the security contract visible to contributors via required reviewers
• ➕ Prevents accidental merges of malicious registry changes without explicit approval
• ➖ Requires CODEOWNERS configuration in every target repo
• ➖ Adds friction for legitimate tool additions
• ➖ Doesn't replace the base-branch read — would be additive

2. Allowlist-only registry (no arbitrary URL templates)

• ➕ Eliminates the supply-chain risk entirely by restricting to known-safe install sources
• ➖ Severely limits extensibility — the whole point of per-repo registries is custom tools
• ➖ Requires maintaining an allowlist, shifting the maintenance burden upstream

Recommendation: The base-branch-only read via git show is the right security trade-off for this threat model. One alternative worth noting is a PR-review gate (e.g., CODEOWNERS requiring approval for .pre-commit-tools.yaml changes) which could complement the base-branch approach and make the security contract more explicit to contributors.

Files changed (9) +669 / -27

Enhancement (5) +124 / -22

resolve-precommit-tools.pyRefactor resolve() to accept dict; add merge_registries() and --local-registry arg +89/-14

Refactor resolve() to accept dict; add merge_registries() and --local-registry arg

• Refactors 'resolve()' to accept a pre-parsed registry dict instead of a file path. Adds 'merge_registries(upstream, local)' implementing additive merge semantics with validation and warnings. Replaces manual 'sys.argv' parsing with 'argparse' and adds '--local-registry' flag. Extracts 'load_yaml_file()' helper.

internal/scaffold/fullsend-repo/scripts/resolve-precommit-tools.py

post-code.shExtract base-branch registry and pass --local-registry to resolver +7/-2

Extract base-branch registry and pass --local-registry to resolver

• Adds 'git show origin/${TARGET_BRANCH}:.pre-commit-tools.yaml' to extract the per-repo registry from the base branch into a temp file, then passes it via '--local-registry'. Cleans up both temp files on exit.

internal/scaffold/fullsend-repo/scripts/post-code.sh

post-fix.shExtract base-branch registry and pass --local-registry to resolver +7/-2

Extract base-branch registry and pass --local-registry to resolver

• Same base-branch registry extraction pattern as 'post-code.sh', applied to the post-fix agent script.

internal/scaffold/fullsend-repo/scripts/post-fix.sh

pre-code.shExtract base-branch registry and pass --local-registry to resolver +9/-2

Extract base-branch registry and pass --local-registry to resolver

• Detects the default branch via 'git symbolic-ref', extracts the per-repo registry from that branch, and passes it via '--local-registry'. Cleans up both temp files.

internal/scaffold/fullsend-repo/scripts/pre-code.sh

pre-fix.shExtract base-branch registry and pass --local-registry to resolver +12/-2

Extract base-branch registry and pass --local-registry to resolver

• Same as 'pre-code.sh' but also falls back to 'TARGET_BRANCH' env var before auto-detecting the default branch via 'git symbolic-ref'.

internal/scaffold/fullsend-repo/scripts/pre-fix.sh

Bug fix (1) +30 / -5

.pre-commit-tools.yamlAdd uv match_entry (P1 fix) and update header docs +30/-5

Add uv match_entry (P1 fix) and update header docs

• Adds a second registry entry matching 'entry: "uv run ..."' hooks to the same 'uv' binary install, complementing the existing 'uvx' entry. Updates header comments to describe the three-layer customization model and notes that 'shellcheck-py' needs no registry entry.

internal/scaffold/fullsend-repo/scripts/.pre-commit-tools.yaml

Tests (1) +309 / -0

test_resolve_precommit_tools.pyAdd 13 unit tests for merge, exclude, dedup, uv match, and malformed input +309/-0

Add 13 unit tests for merge, exclude, dedup, uv match, and malformed input

• New test file covering 'merge_registries()' (new entry, override, exclude, empty local, order preservation, malformed input) and 'resolve()' (uv match, uvx match, dedup, end-to-end merged registry). Loads the resolver module dynamically via 'importlib'.

internal/scaffold/scripts-tests/test_resolve_precommit_tools.py

Documentation (2) +206 / -0

0056-per-repo-precommit-tools-registry.mdNew ADR 0056: per-repo pre-commit tools registry design +136/-0

New ADR 0056: per-repo pre-commit tools registry design

• Documents the three-layer resolution order (upstream → L1 org → L2 per-repo), merge semantics (extend/override/exclude), and the base-branch-only read security model. Covers validation behavior and CLI interface.

docs/ADRs/0056-per-repo-precommit-tools-registry.md

customizing-agents.mdAdd 'Customizing Pre-commit Tool Dependencies' section +70/-0

Add 'Customizing Pre-commit Tool Dependencies' section

• Adds a new section explaining the three-layer resolution model, L1 vs L2 customization paths, YAML examples for adding and suppressing entries, and the security rationale for base-branch-only reads.

docs/guides/user/customizing-agents.md

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 3:03 PM UTC · Ended 3:10 PM UTC
Commit: 2d8adb7 · View workflow run →

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (5)

Context used

✅ Tickets: 🎫 Expand precommit-tools.yaml registry coverage

✅ Compliance rules (platform): 58 rules

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

1. ADR 0035 not linked 📜 Skill insight ⚙ Maintainability

Description

The ADR Context references ADR 0035 but does not include an explicit cross-reference link. This
reduces traceability for related architectural decisions.

Code

docs/ADRs/0056-per-repo-precommit-tools-registry.md[R33-36]

+The registry can be fully replaced at the org level by placing a
+`.pre-commit-tools.yaml` in `customized/scripts/` (L1 override via the
+layered overlay mechanism from ADR 0035). This works, but repos that
+need just one additional tool must copy the entire upstream registry to

Relevance

⭐⭐⭐ High

Team has accepted ADR cross-reference/link fixes before (e.g., updating ADR links for
correctness/traceability).

PR-#1549
PR-#1814

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1062094 requires related ADRs to be cross-referenced via links in Context; the text
references ADR 0035 without a link.

docs/ADRs/0056-per-repo-precommit-tools-registry.md[33-36]
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 Context section mentions `ADR 0035` but does not link to the related ADR file.

## Issue Context
The compliance rule requires an explicit cross-reference link when the ADR builds on or relates to another ADR.

## Fix Focus Areas
- docs/ADRs/0056-per-repo-precommit-tools-registry.md[33-36]

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

---

2. Guide missing prerequisites section 📜 Skill insight ✧ Quality

Description

The updated guide section introduces new procedural guidance (e.g., how to add/suppress registry
entries) without adding a clearly labeled Prerequisites section before those instructions. This
makes the procedure harder to follow and violates the guide format requirement.

Code

docs/guides/user/customizing-agents.md[R187-243]

+### Customizing Pre-commit Tool Dependencies
+
+Fullsend auto-detects and installs tools required by a target repo's pre-commit hooks. The resolver reads `.pre-commit-config.yaml`, matches hooks against a tools registry, and installs missing dependencies before the authoritative pre-commit check runs.
+
+Only hooks that pre-commit **cannot self-serve** need registry entries:
+- `language: system` — the tool must already be on `PATH`
+- `language: golang` — binary download is faster than Go compilation
+
+Hooks using `language: python`, `language: node`, or `language: docker_image` are handled natively by pre-commit and need no registry entry.
+
+#### Three-layer resolution
+
+```
+upstream defaults (fullsend-ai/fullsend)
+  → org replacement:  customized/scripts/.pre-commit-tools.yaml  (L1)
+    → per-repo additive:  .pre-commit-tools.yaml at repo root    (L2)
+```
+
+| Layer | Location | Behavior |
+|-------|----------|----------|
+| Upstream | Provided at runtime by reusable workflow | Base registry shipped with fullsend |
+| L1 org replacement | `customized/scripts/.pre-commit-tools.yaml` in `.fullsend` config repo | **Completely replaces** upstream registry |
+| L2 per-repo additive | `.pre-commit-tools.yaml` at target repo root | **Merges** with upstream/org registry |
+
+**L1 replacement** works via the layered overlay — the file is copied over the upstream registry at runtime. Use this when your org needs a completely different set of tools.
+
+**L2 additive merge** is designed for repos that need to extend the registry with one or two entries. New entries are appended, entries matching an existing `(repo, hook_id)` key override it, and entries with `exclude: true` suppress the matching upstream entry.
+
+> **Note:** There are two per-repo customization paths with different semantics:
+> - `.fullsend/customized/scripts/.pre-commit-tools.yaml` — L1 full replacement (same overlay mechanism as other layered dirs)
+> - `.pre-commit-tools.yaml` at repo root — L2 additive merge (resolver discovers and merges)
+
+#### Example: adding a custom binary tool
+
+Place `.pre-commit-tools.yaml` at your repo root:
+
+```yaml
+tools:
+  - hook_id: my-linter
+    repo: https://github.com/example/my-linter
+    install:
+      type: binary
+      name: my-linter
+      version: "1.2.3"
+      url_template: "https://github.com/example/my-linter/releases/download/v{version}/my-linter-{triple}.tar.gz"
+      checksums:
+        x86_64: "abc123..."
+        aarch64: "def456..."
+      binary_name: my-linter
+```
+
+This entry is merged with the upstream registry — all upstream tools remain available.
+
+#### Example: suppressing an upstream entry
+
+To prevent an upstream tool from being installed (e.g., if your repo handles it differently):
+

Relevance

⭐⭐ Medium

Guide-format rules (explicit Prerequisites section) not seen in prior accepted feedback; evidence
insufficient.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1062078 requires a prerequisites section before procedural steps in guides; the
added section starts giving instructions without any prerequisites section.

docs/guides/user/customizing-agents.md[187-243]
Skill: writing-user-docs

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 guide contains procedural instructions but does not include a clearly labeled `## Prerequisites` section before the procedural steps.

## Issue Context
The newly added "Customizing Pre-commit Tool Dependencies" section includes instructions like placing `.pre-commit-tools.yaml` at repo root and suppressing upstream entries.

## Fix Focus Areas
- docs/guides/user/customizing-agents.md[187-243]

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

---

3. ADR exceeds 100 lines 📜 Skill insight ⚙ Maintainability

Description

The new ADR 0056-per-repo-precommit-tools-registry.md is 136 lines long, exceeding the 100-line
maximum for ADR content. This makes the ADR harder to review and suggests details should be
moved/condensed into references or other docs.

Code

docs/ADRs/0056-per-repo-precommit-tools-registry.md[R13-136]

+# 56. Per-repo pre-commit tools registry
+
+Date: 2026-06-25
+
+## Status
+
+Accepted
+
+Extends [PR #1055](https://github.com/fullsend-ai/fullsend/pull/1055)
+(registry-based auto-detect/install for pre-commit tool dependencies).
+
+Related: [#1270](https://github.com/fullsend-ai/fullsend/issues/1270)
+
+## Context
+
+PR #1055 introduced `.pre-commit-tools.yaml` — a registry that maps
+pre-commit hook repos and IDs to the system tools they require. The
+resolver (`resolve-precommit-tools.py`) reads this registry and
+produces a JSON manifest consumed by the installer script.
+
+The registry can be fully replaced at the org level by placing a
+`.pre-commit-tools.yaml` in `customized/scripts/` (L1 override via the
+layered overlay mechanism from ADR 0035). This works, but repos that
+need just one additional tool must copy the entire upstream registry to
+add a single entry — losing upstream defaults and creating a
+maintenance burden.
+
+Target repos need a way to **extend** the registry additively without
+replacing it.
+
+## Decision
+
+The resolver discovers a second, per-repo registry at
+`<target-repo>/.pre-commit-tools.yaml` and merges it with the
+upstream/org defaults. The merge is additive: new entries extend the
+registry, entries matching an existing `(repo, hook_id)` key override
+it, and entries with `exclude: true` suppress the matching upstream
+entry.
+
+### Three-layer resolution order
+
+```
+upstream defaults (fullsend-ai/fullsend)
+  → org override: customized/scripts/.pre-commit-tools.yaml  (L1, full replacement)
+    → per-repo additive: .pre-commit-tools.yaml at repo root (L2, additive merge)
+```
+
+After the repo-maintenance overlay runs, the resolver's co-located
+`.pre-commit-tools.yaml` contains either the upstream defaults or the
+org's L1 replacement. The resolver treats this as the "upstream"
+registry and merges the per-repo file on top.
+
+### Merge semantics
+
+The `merge_registries(upstream, local)` function:
+
+1. Builds an ordered dict of upstream entries keyed by `(repo, hook_id)`
+2. For each local entry:
+   - If `exclude: true`: removes the matching upstream entry
+   - Otherwise: inserts or replaces by `(repo, hook_id)` key
+3. New entries are appended after upstream entries (order matters
+   because `seen_names` dedup is first-match-wins)
+4. Returns the merged `{"tools": [...]}` dict
+
+The suppression field is named `exclude` (not `skip`) to avoid
+confusion with the existing `skip_install` field inside `install`
+blocks.
+
+### Security: base-branch-only reads
+
+The per-repo `.pre-commit-tools.yaml` is user-controlled content that
+feeds into the tool installation pipeline. The installer runs outside
+the sandbox on the GHA runner with `PUSH_TOKEN` in the environment.
+A malicious contributor could submit a PR that adds a
+`.pre-commit-tools.yaml` pointing `url_template` to a trojanized
+binary, installing arbitrary pip/npm packages, or overriding a
+legitimate upstream entry (e.g. replacing gitleaks with a backdoor).
+
+**Mitigation**: The caller scripts extract the per-repo registry from
+the **base branch** (via `git show origin/${TARGET_BRANCH}:...`), not
+from the working tree. This means:
+
+- The per-repo registry must be merged into the base branch (reviewed
+  and approved) before it takes effect
+- PRs that add or modify `.pre-commit-tools.yaml` do not have their
+  changes applied until the PR is merged
+- This is consistent with the threat model: the base branch is
+  trusted, PR content is not
+
+### Validation
+
+`merge_registries()` validates the local registry structure before
+merging: checks for dict type, `tools` key presence, and that each
+entry is a dict with `hook_id`. Malformed input emits warnings and
+falls back to the upstream registry unchanged.
+
+### CLI interface
+
+The resolver accepts an optional `--local-registry <path>` argument.
+The caller scripts extract the base-branch registry to a temp file and
+pass it via this flag. When omitted, behavior is identical to today
+(upstream-only resolution).
+
+## Consequences
+
+- Target repos can add custom tool entries by placing
+  `.pre-commit-tools.yaml` at the repo root — no need to duplicate the
+  entire upstream registry.
+- The L1 full-replacement mechanism via `customized/scripts/` remains
+  available for orgs that need complete control.
+- New per-repo registries only take effect after merge to the base
+  branch. This is a deliberate security trade-off: the first agent run
+  after adding the file will not use it. A subsequent run will.
+- Two per-repo customization paths exist and must be clearly
+  documented:
+  - `.fullsend/customized/scripts/.pre-commit-tools.yaml` — L1 full
+    replacement (overlay copies it over the upstream file)
+  - `.pre-commit-tools.yaml` at repo root — L2 additive merge (the
+    resolver discovers and merges it)
+- Entries that share an `install.name` (e.g. `uv` matched by both
+  `match_entry: "uvx"` and `match_entry: "uv"`) interact with dedup:
+  overriding one entry while the other wins via `seen_names` means the
+  override is silently ignored. This is acceptable — the tool still
+  gets installed.

Relevance

⭐⭐ Medium

No prior accepted/rejected suggestions enforcing ADR max line count; ADR reviews focus on accuracy,
not strict length.

PR-#1814
PR-#2465

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1062092 limits ADR content length to 100 lines; this ADR is 136 lines long as added
in the PR.

docs/ADRs/0056-per-repo-precommit-tools-registry.md[13-136]
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 detailed merge semantics, validation behavior, CLI interface notes, and security rationale that can likely be shortened or moved to a reference doc while keeping the ADR focused on the decision.

## Fix Focus Areas
- docs/ADRs/0056-per-repo-precommit-tools-registry.md[13-136]

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

---

View more (2)

4. Consequences bullets not one-sentence 📜 Skill insight ⚙ Maintainability

Description

The ADR Consequences section contains bullets that are multi-sentence and/or include nested
sub-bullets. This violates the requirement for 3–5 one-sentence bullet points.

Code

docs/ADRs/0056-per-repo-precommit-tools-registry.md[R116-135]

+## Consequences
+
+- Target repos can add custom tool entries by placing
+  `.pre-commit-tools.yaml` at the repo root — no need to duplicate the
+  entire upstream registry.
+- The L1 full-replacement mechanism via `customized/scripts/` remains
+  available for orgs that need complete control.
+- New per-repo registries only take effect after merge to the base
+  branch. This is a deliberate security trade-off: the first agent run
+  after adding the file will not use it. A subsequent run will.
+- Two per-repo customization paths exist and must be clearly
+  documented:
+  - `.fullsend/customized/scripts/.pre-commit-tools.yaml` — L1 full
+    replacement (overlay copies it over the upstream file)
+  - `.pre-commit-tools.yaml` at repo root — L2 additive merge (the
+    resolver discovers and merges it)
+- Entries that share an `install.name` (e.g. `uv` matched by both
+  `match_entry: "uvx"` and `match_entry: "uv"`) interact with dedup:
+  overriding one entry while the other wins via `seen_names` means the
+  override is silently ignored. This is acceptable — the tool still

Relevance

⭐⭐ Medium

No historical evidence found enforcing one-sentence Consequences bullets or banning nested bullets
in ADRs.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1062091 requires Consequences to be 3–5 one-sentence bullets; the bullets here
include multiple sentences and nested bullet content.

docs/ADRs/0056-per-repo-precommit-tools-registry.md[116-135]
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 Consequences section does not follow the required format: each bullet must be exactly one sentence (no multi-sentence bullets, no nested lists).

## Issue Context
Current bullets include multi-sentence explanations (e.g., the "security trade-off" bullet) and a bullet that expands into nested sub-bullets describing two customization paths.

## Fix Focus Areas
- docs/ADRs/0056-per-repo-precommit-tools-registry.md[116-135]

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

---

5. Procedures not in numbered steps 📜 Skill insight ✧ Quality

Description

The added guide instructions use prose imperatives (e.g., "Place ...", "To prevent ...") rather than
numbered steps for the procedure. This violates the requirement that procedural content be written
as numbered steps.

Code

docs/guides/user/customizing-agents.md[R219-249]

+#### Example: adding a custom binary tool
+
+Place `.pre-commit-tools.yaml` at your repo root:
+
+```yaml
+tools:
+  - hook_id: my-linter
+    repo: https://github.com/example/my-linter
+    install:
+      type: binary
+      name: my-linter
+      version: "1.2.3"
+      url_template: "https://github.com/example/my-linter/releases/download/v{version}/my-linter-{triple}.tar.gz"
+      checksums:
+        x86_64: "abc123..."
+        aarch64: "def456..."
+      binary_name: my-linter
+```
+
+This entry is merged with the upstream registry — all upstream tools remain available.
+
+#### Example: suppressing an upstream entry
+
+To prevent an upstream tool from being installed (e.g., if your repo handles it differently):
+
+```yaml
+tools:
+  - hook_id: gitleaks
+    repo: https://github.com/zricethezav/gitleaks
+    exclude: true
+```

Relevance

⭐⭐ Medium

No historical evidence that docs procedures must be numbered steps; past doc feedback is mixed and
topic-specific.

PR-#665

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

PR Compliance ID 1062079 requires procedures to use numbered steps; the added section uses prose
directives instead of ordered lists.

docs/guides/user/customizing-agents.md[219-249]
Skill: writing-user-docs

Agent prompt

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

## Issue description
Procedural instructions in the added section are written as prose paragraphs instead of a numbered (ordered) list.

## Issue Context
The section includes at least two procedures:
- Adding a custom binary tool (placing `.pre-commit-tools.yaml` and adding an entry)
- Suppressing an upstream entry (adding an `exclude: true` entry)

## Fix Focus Areas
- docs/guides/user/customizing-agents.md[219-249]

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

---

6. Registry merge can crash 🐞 Bug ☼ Reliability

Description

merge_registries() assumes upstream['tools'] is a list and slices it; if an org-replaced registry
provides a non-list 'tools' value, resolve-precommit-tools.py can raise a TypeError and skip tool
resolution/installation. This can lead to the authoritative pre-commit run proceeding without
required binaries and failing unexpectedly.

Code

internal/scaffold/fullsend-repo/scripts/resolve-precommit-tools.py[59]

+    upstream_tools = (upstream.get("tools") or [])[:]

Relevance

⭐⭐⭐ High

Team frequently accepts defensive validation in scaffold scripts to prevent runtime failures from
malformed inputs.

PR-#337
PR-#586
PR-#2346

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The merge code slices upstream.get('tools') without type-checking, which will throw for non-list
values; this is relevant because the registry is documented as fully replaceable (L1), making
malformed upstream/org registries a realistic input. The post script continues when resolution
fails, so a crash here can translate into missing installs and later pre-commit failures.

internal/scaffold/fullsend-repo/scripts/resolve-precommit-tools.py[50-63]
internal/scaffold/fullsend-repo/scripts/resolve-precommit-tools.py[59-60]
internal/scaffold/fullsend-repo/scripts/.pre-commit-tools.yaml[35-45]
internal/scaffold/fullsend-repo/scripts/post-code.sh[273-289]

Agent prompt

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

## Issue description
`merge_registries()` does `upstream_tools = (upstream.get("tools") or [])[:]`, which crashes if `upstream["tools"]` is a truthy non-list (e.g., dict/string). Since the upstream registry can be replaced by org/per-repo customization, the resolver should be defensive and emit warnings/fallback instead of throwing.

## Issue Context
- The registry is explicitly customizable/replaced (L1 full replacement), so malformed YAML structures are plausible and should not hard-crash tool resolution.
- Post scripts treat resolver failure as non-fatal and continue, which can later cause pre-commit to fail due to missing binaries.

## Fix Focus Areas
- internal/scaffold/fullsend-repo/scripts/resolve-precommit-tools.py[42-88]
- internal/scaffold/fullsend-repo/scripts/resolve-precommit-tools.py[204-218]

## Suggested change
- In `merge_registries()`, validate `upstream.get("tools")`:
 - If missing/None: treat as empty list.
 - If list: copy via `list(...)` or slicing.
 - Otherwise: append a warning like `"upstream registry 'tools' is not a list — treating as empty"` and treat as empty list.
- (Optional) Return a merged dict that preserves other top-level keys from `upstream` (e.g., `merged = dict(upstream); merged["tools"] = merged_tools`).

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

---

[fullsend-ai-review]

🤖 Review · ⚠️ Cancelled · Started 3:14 PM UTC · Ended 3:18 PM UTC
Commit: 2d8adb7 · View workflow run →

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 3:24 PM UTC · Completed 3:38 PM UTC
Commit: 34c0aca · View workflow run →

[fullsend-ai-review]

Review

Findings

Medium

• [scope-creep] docs/ADRs/0056-per-repo-precommit-tools-registry.md — The PR introduces a new L2 additive merge feature (ADR 0056) that may exceed the scope of issue Expand precommit-tools.yaml registry coverage #1270. The issue requests "expanding registry coverage" with specific items (P1 uv match, P2 document shellcheck-py, P3 Go linter strategy). The PR delivers P1 and P2 but adds a substantial new feature (per-repo additive merge with security controls, new resolver flag, shell integration across 4 scripts, 388-line test suite, 78 lines of user documentation).
  Remediation: Either update issue Expand precommit-tools.yaml registry coverage #1270 to explicitly authorize the L2 additive merge feature, or split into two PRs: one for P1+P2 (authorized by Expand precommit-tools.yaml registry coverage #1270), and a separate PR for L2 with its own issue.

Previous run

Review

Findings

Medium

• [scope-creep] docs/ADRs/0056-per-repo-precommit-tools-registry.md — The PR introduces a new L2 additive merge feature (ADR 0056) that exceeds the scope of issue Expand precommit-tools.yaml registry coverage #1270. The issue requests "expanding registry coverage" with specific items (P1 uv match, P2 document shellcheck-py, P3 Go linter strategy). The PR delivers P1 and P2 but adds a substantial new feature (per-repo additive merge with security controls, new resolver flag, shell integration across 4 scripts, 388-line test suite).
  Remediation: Either update issue Expand precommit-tools.yaml registry coverage #1270 to explicitly authorize the L2 additive merge feature, or split into two PRs: one for P1+P2 (authorized by Expand precommit-tools.yaml registry coverage #1270), and a separate PR for L2 with its own issue.

• [architecture-documentation-staleness] docs/architecture.md — ADR 0056 introduces a new L2 per-repo additive merge capability for pre-commit tools, but docs/architecture.md has not been updated to reference it. The architecture document states it is a "living document" that "must always reflect the current state of architectural decisions." ADR 0056 has status "Accepted" and relates to "agent-infrastructure", but no corresponding update appears in architecture.md.
  Remediation: Update docs/architecture.md to reference ADR 0056 in the relevant section (likely under "Configuration layering" near ADR 0035 layered content resolution).

---

Labels: PR adds a new per-repo additive merge feature to scaffold harness scripts with accompanying ADR and user documentation.

Previous run (2)

Review

Findings

Medium

• [edge-case] internal/scaffold/fullsend-repo/scripts/resolve-precommit-tools.py — The resolve function's entry_match_map uses last-writer-wins semantics: if two registry entries (including those added via L2 merge) share the same match_entry value but differ on (repo, hook_id), only the last entry's tool will be matched. merge_registries keys on (repo, hook_id) but does not warn on match_entry collisions, so a per-repo local registry can silently shadow an upstream entry's match mapping.
  Remediation: Track match_entry values in merge_registries and emit a warning when a local entry's match_entry collides with a different upstream entry.

• [scope-creep] docs/ADRs/0056-per-repo-precommit-tools-registry.md — The PR introduces a new L2 additive merge feature (ADR 0056) that may exceed the scope of issue Expand precommit-tools.yaml registry coverage #1270. The issue requests "expanding registry coverage" with specific items (P1 uv match, P2 document shellcheck-py, P3 Go linter strategy). The PR delivers P1 and P2 but adds a substantial new feature (per-repo additive merge with security controls, new resolver flag, shell integration across 4 scripts).
  Remediation: Either update issue Expand precommit-tools.yaml registry coverage #1270 to explicitly authorize the L2 additive merge feature, or split into two PRs: one for P1+P2 (authorized by Expand precommit-tools.yaml registry coverage #1270), and a separate PR for L2 with its own issue.

• [test-file-location] internal/scaffold/scripts-tests/test_resolve_precommit_tools.py — Test file is added to a new scripts-tests/ directory that does not exist on the base branch. All existing script tests (e.g., process-fix-result-test.py, pre-code-test.sh) live alongside their implementation in internal/scaffold/fullsend-repo/scripts/.
  Remediation: Move to internal/scaffold/fullsend-repo/scripts/resolve-precommit-tools-test.py.

• [test-naming-convention] internal/scaffold/scripts-tests/test_resolve_precommit_tools.py — Test file uses test_ prefix naming (test_resolve_precommit_tools.py) but the established pattern uses -test suffix with hyphens (e.g., process-fix-result-test.py).
  Remediation: Rename to resolve-precommit-tools-test.py to match the <name>-test.py convention.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 4:22 PM UTC · Completed 4:33 PM UTC
Commit: e1bf165 · View workflow run →

[ralphbean]

LGTM.

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 7:52 PM UTC · Completed 8:00 PM UTC
Commit: 88729ed · View workflow run →

[fullsend-ai-retro]

Retro: PR #2663 — Per-repo pre-commit tools registry (L2 additive merge)

Timeline

Human-authored feature PR by waynesun09, opened and merged same day (2026-06-25). The review agent ran 3 successful reviews across 3 commits, catching 4 medium-severity findings. The author fixed 3 of 4 (edge-case in match_entry collision handling, test file location/naming, architecture docs staleness). The remaining scope-creep finding was non-blocking per the repo's verdict rules. A single human reviewer (ralphbean) approved with "LGTM" and no substantive comments.

What went well

• Review agent drove real improvements. Three of four findings led to concrete code fixes that would not have occurred without the agent. The iterative re-review model worked as designed — findings were dropped once resolved.
• Finding evolution tracked correctly. Run 1 had 4 findings, Run 2 had 2 (after fixes + new staleness finding), Run 3 had 1 (only the unfixed scope-creep). The sticky history format preserved the audit trail.
• Scope-creep was correctly calibrated. A single medium finding produced a comment-only verdict, which correctly did not block the PR.

Known issues observed (skipped as proposals)

• Post-review 422 errors (2 failed review runs on other PRs during the same window): Already tracked by #2569 and #1067.
• Review dispatch fan-out (18 review runs from 9 dispatcher events, ~2.25x multiplier): Already tracked by #1452, #1418, and #1014.

One proposal below

The agent-human review delta on this PR was stark: the agent caught issues across 4 dimensions (correctness, conventions, documentation, scope), while the human contributed zero findings. The structural gap is that persisting agent findings are easy to overlook when approving.

Proposals filed

• Review status comment should surface unresolved finding count for human reviewers (in fullsend-ai/fullsend)