← back to Cli Printing Press
fix(cli): FedEx retro batch — 5 small fixes (WU-2, WU-3, WU-4, WU-5, WU-7) (#526)
e761d04d9d0670871e49586352f4bf3e9077f816 · 2026-05-03 01:33:05 -0700 · Trevin Chow
* fix(cli): bump --help Highlights description budget 80 → 200 runes
The root command's `--help` Long block renders novel-feature descriptions
under "Highlights (not in the official API docs):" and previously truncated
each one to 80 runes. Most novel-feature descriptions in `research.json`
are 100-200 chars (legitimately useful explanations), so the 80-rune cap
turned every line into an ellipsed half-sentence — FedEx 14/15 lines
truncated, Dub 3/4 lines truncated.
Bumped the truncate budget to 200 runes. Cobra's wrap logic still handles
narrow terminals, so the higher cap only lets full sentences land before
the wrap. Updated TestRootLongStaysUnderSizeBudget's body-size assertion
from 2000 → 5000 chars to accommodate the new per-line cap (worst case:
15 features × 200 runes = 3000 chars + headline + framing).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cli): regen-merge propagates spec.yaml from fresh tree to library
Before this fix, regen-merge silently skipped spec.yaml at the CLI root —
shouldClassifyFile only allowed .go, go.mod, and go.sum. So when a user
edited the source spec (e.g., to add an `mcp:` block for the Cloudflare
pattern) and ran regen-merge, the library's spec.yaml stayed at the
pre-enrichment shape. Downstream tools that read the library spec
(mcp-sync, dogfood, scorecard) then operated on the stale contract,
re-emitting raw endpoint tools and undoing the optimization.
This was the actual root cause behind the FedEx retro's WU-4 ("mcp-sync
respects endpoint_tools=hidden"). The mcp_tools.go.tmpl template already
gates emission on the spec field correctly; the gap was that the spec
field never reached the library because regen-merge didn't see spec.yaml
as a classifiable file.
Fix: extend shouldClassifyFile to include spec.yaml and spec.json at the
CLI root. Both classify as TEMPLATED-CLEAN when present in both trees
(per the existing non-.go branch in classifyFiles), so Apply overwrites
with fresh — same path go.mod takes today. Spec is generator-owned;
fresh always wins.
Test: TestClassifySpecYamlPropagates pins the verdict and end-to-end
proves source-spec mcp blocks propagate to the library copy after merge.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(skills): polish-skill maps MCP scorecard dims to spec fields
The polish skill's "Push higher without gaming" section had no concrete
guidance on the four MCP scorecard dims (mcp_token_efficiency,
mcp_tool_design, mcp_remote_transport, mcp_surface_strategy). When these
came in below max, agents misclassified the fix path as "feature add to
a generator-owned file, retro candidate" — leaving real, polish-time
fixes on the table.
These dims all have direct mcp.* spec field counterparts (documented in
docs/SPEC-EXTENSIONS.md). The fix is a spec edit + regenerate (or
regen-merge into the published library), not generator-template work.
Polish CAN do this.
Added a sub-section "MCP scorecard dims map to spec fields, not
generator code" with a dim → spec-field mapping table and a threshold
recommendation for when to apply the Cloudflare pattern (>50 typed
endpoints). Cobratree-walked novel commands continue to surface
automatically and don't need spec changes.
This was the F6 finding from the FedEx retro: polish skill's
classifier said MCP Remote Transport 5/10 was a "feature add to
generator file" when the actual fix was a 1-line `mcp.transport` field.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cli): regen-merge preserves hand-added go.mod requires
renderMergedGoMod previously seeded the merged require set from
freshMF.Require only. Published-only requires (deps the agent added
post-generation, e.g., `go get modernc.org/sqlite` for a hand-built
local store) silently dropped, and the next `go build` failed with
"no required module provides package <X>".
Fix: after adding fresh's requires, walk pubMF.Require and append any
path not already added by fresh. Fresh wins on shared paths (newer
generator templates pin newer versions; downgrading would break
indirect deps); published-only entries survive.
Reporting: planGoModMerge moved published-only entries from
RemovedRequires → PreservedRequires (new field) so the dry-run report
accurately describes what survives the merge. Added the
RemovedRequires field doc to mark it reserved for a future
"intentionally drop" mode; currently always empty.
Test: TestRenderMergedGoModPreservesPublishedOnlyRequires builds a
fixture with a hand-added sqlite dep, runs both planGoModMerge and
renderMergedGoMod, and verifies the dep lands in PreservedRequires
and survives in the merged go.mod with its original version.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cli): regen-merge skips lost-registration injection for preserved hosts
When a host file's Apply verdict is TEMPLATED-BODY-DRIFT,
TEMPLATED-WITH-ADDITIONS, NOVEL, or NOVEL-COLLISION, Apply preserves
the published file verbatim — the AddCommand calls inside it survive
the merge in-place. Lost-registration extraction was still flagging
those calls as "lost from fresh", and Apply was re-injecting them on
top of the already-present originals. Result: duplicate AddCommand
lines in root.go (and other host files), which crashes the resulting
CLI at startup with "command is already added".
Was the FedEx retro's WU-2 root cause: hand-edited root.go classified
TEMPLATED-BODY-DRIFT (decl set matched, function body drifted),
extraction flagged 8 hand-added rootCmd.AddCommand and 2
trackCmd.AddCommand calls, Apply re-injected them — the published
root.go ended up with two identical blocks of 10 calls.
Fix: extractLostRegistrations now takes a verdict map and skips host
files whose verdict means published is preserved. Only TEMPLATED-CLEAN
and NEW-TEMPLATE-EMISSION hosts get overwritten by Apply with fresh
content, so those are the only ones that need lost-call restoration.
Test: TestExtractLostRegistrationsSkipsPreservedHosts builds a
synthetic fixture with a hand-added AddCommand in published, runs
extraction four times with different verdict maps, and verifies the
nil/clean cases flag the call (1 lost) while the
preserved-verdict cases produce zero (0 lost). Existing tests pass nil
for the verdict map (no skip — preserves their behavior).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(cli): tighten comments and consolidate verdict-preservation predicate
Post-batch /simplify cleanup across the 6 retro fixes:
- Add Verdict.PreservesPublished() method consolidating the
"this verdict means Apply leaves published in place" dichotomy that
registrations.go was checking with an inline switch on four
Verdict constants. Single source of truth for future call sites.
- Remove speculative GoModMerge.RemovedRequires field — the merge no
longer drops requires, so the field would always be empty. Add it
back when there's an actual intentionally-drop mode.
- Trim incident references and PR-narration from comments per
AGENTS.md "no dates, incidents, or ticket numbers in code comments":
• root.go.tmpl: drop FedEx/Dub line counts (commit-message material)
• registrations_test.go: drop "FedEx retro WU-2 root cause" anchor
- Trim WHAT-narration in three comments that crossed from WHY into
restating the code:
• mcp_size.go::estimateMCPTokens: collapse "Sources scanned" enum
• gomod.go::planGoModMerge: drop the operator-explainer paragraph
• helpers.go::shouldClassifyFile: collapse the multi-paragraph
rationale into the single sentence that names the WHY
No behavior changes; tests + golden suite pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(cli): apply review feedback from /review pass
Five small follow-ups surfaced by code review on the FedEx retro batch:
- Trim "Before this fix" PR-narration from two test docstrings that
the prior simplify pass missed (mcp_size_test.go, classify_test.go).
AGENTS.md "no dates, incidents, or ticket numbers in code comments".
- Inline the Verdict.PreservesPublished() predicate back at its single
call site in registrations.go and remove the method. Reviewer
correctly flagged this as premature abstraction — extract when the
second/third call site arrives, per AGENTS.md "three similar lines
is better than a premature abstraction."
- Add PreservedRequires to printHumanRegenReport's go.mod section so
operators running regen-merge without --json can see hand-added
deps reported as preserved. JSON consumers already had this; the
human path was overlooked.
- Update the regen-merge plan doc's example JSON: removed_requires →
preserved_requires (the field rename happened in the simplify
commit but the doc example stayed stale).
No behavior changes; tests + golden suite pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Files touched
M docs/plans/2026-05-01-001-feat-regen-merge-subcommand-plan.mdM internal/cli/regen_merge.goM internal/generator/generator.goM internal/generator/root_long_test.goM internal/generator/templates/root.go.tmplM internal/pipeline/regenmerge/classify_test.goM internal/pipeline/regenmerge/correctness_fixes_test.goM internal/pipeline/regenmerge/gomod.goM internal/pipeline/regenmerge/gomod_test.goM internal/pipeline/regenmerge/helpers.goM internal/pipeline/regenmerge/regenmerge.goM internal/pipeline/regenmerge/registrations.goM internal/pipeline/regenmerge/registrations_test.goM skills/printing-press-polish/SKILL.md
Diff
commit e761d04d9d0670871e49586352f4bf3e9077f816
Author: Trevin Chow <trevin@trevinchow.com>
Date: Sun May 3 01:33:05 2026 -0700
fix(cli): FedEx retro batch — 5 small fixes (WU-2, WU-3, WU-4, WU-5, WU-7) (#526)
* fix(cli): bump --help Highlights description budget 80 → 200 runes
The root command's `--help` Long block renders novel-feature descriptions
under "Highlights (not in the official API docs):" and previously truncated
each one to 80 runes. Most novel-feature descriptions in `research.json`
are 100-200 chars (legitimately useful explanations), so the 80-rune cap
turned every line into an ellipsed half-sentence — FedEx 14/15 lines
truncated, Dub 3/4 lines truncated.
Bumped the truncate budget to 200 runes. Cobra's wrap logic still handles
narrow terminals, so the higher cap only lets full sentences land before
the wrap. Updated TestRootLongStaysUnderSizeBudget's body-size assertion
from 2000 → 5000 chars to accommodate the new per-line cap (worst case:
15 features × 200 runes = 3000 chars + headline + framing).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cli): regen-merge propagates spec.yaml from fresh tree to library
Before this fix, regen-merge silently skipped spec.yaml at the CLI root —
shouldClassifyFile only allowed .go, go.mod, and go.sum. So when a user
edited the source spec (e.g., to add an `mcp:` block for the Cloudflare
pattern) and ran regen-merge, the library's spec.yaml stayed at the
pre-enrichment shape. Downstream tools that read the library spec
(mcp-sync, dogfood, scorecard) then operated on the stale contract,
re-emitting raw endpoint tools and undoing the optimization.
This was the actual root cause behind the FedEx retro's WU-4 ("mcp-sync
respects endpoint_tools=hidden"). The mcp_tools.go.tmpl template already
gates emission on the spec field correctly; the gap was that the spec
field never reached the library because regen-merge didn't see spec.yaml
as a classifiable file.
Fix: extend shouldClassifyFile to include spec.yaml and spec.json at the
CLI root. Both classify as TEMPLATED-CLEAN when present in both trees
(per the existing non-.go branch in classifyFiles), so Apply overwrites
with fresh — same path go.mod takes today. Spec is generator-owned;
fresh always wins.
Test: TestClassifySpecYamlPropagates pins the verdict and end-to-end
proves source-spec mcp blocks propagate to the library copy after merge.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(skills): polish-skill maps MCP scorecard dims to spec fields
The polish skill's "Push higher without gaming" section had no concrete
guidance on the four MCP scorecard dims (mcp_token_efficiency,
mcp_tool_design, mcp_remote_transport, mcp_surface_strategy). When these
came in below max, agents misclassified the fix path as "feature add to
a generator-owned file, retro candidate" — leaving real, polish-time
fixes on the table.
These dims all have direct mcp.* spec field counterparts (documented in
docs/SPEC-EXTENSIONS.md). The fix is a spec edit + regenerate (or
regen-merge into the published library), not generator-template work.
Polish CAN do this.
Added a sub-section "MCP scorecard dims map to spec fields, not
generator code" with a dim → spec-field mapping table and a threshold
recommendation for when to apply the Cloudflare pattern (>50 typed
endpoints). Cobratree-walked novel commands continue to surface
automatically and don't need spec changes.
This was the F6 finding from the FedEx retro: polish skill's
classifier said MCP Remote Transport 5/10 was a "feature add to
generator file" when the actual fix was a 1-line `mcp.transport` field.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cli): regen-merge preserves hand-added go.mod requires
renderMergedGoMod previously seeded the merged require set from
freshMF.Require only. Published-only requires (deps the agent added
post-generation, e.g., `go get modernc.org/sqlite` for a hand-built
local store) silently dropped, and the next `go build` failed with
"no required module provides package <X>".
Fix: after adding fresh's requires, walk pubMF.Require and append any
path not already added by fresh. Fresh wins on shared paths (newer
generator templates pin newer versions; downgrading would break
indirect deps); published-only entries survive.
Reporting: planGoModMerge moved published-only entries from
RemovedRequires → PreservedRequires (new field) so the dry-run report
accurately describes what survives the merge. Added the
RemovedRequires field doc to mark it reserved for a future
"intentionally drop" mode; currently always empty.
Test: TestRenderMergedGoModPreservesPublishedOnlyRequires builds a
fixture with a hand-added sqlite dep, runs both planGoModMerge and
renderMergedGoMod, and verifies the dep lands in PreservedRequires
and survives in the merged go.mod with its original version.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cli): regen-merge skips lost-registration injection for preserved hosts
When a host file's Apply verdict is TEMPLATED-BODY-DRIFT,
TEMPLATED-WITH-ADDITIONS, NOVEL, or NOVEL-COLLISION, Apply preserves
the published file verbatim — the AddCommand calls inside it survive
the merge in-place. Lost-registration extraction was still flagging
those calls as "lost from fresh", and Apply was re-injecting them on
top of the already-present originals. Result: duplicate AddCommand
lines in root.go (and other host files), which crashes the resulting
CLI at startup with "command is already added".
Was the FedEx retro's WU-2 root cause: hand-edited root.go classified
TEMPLATED-BODY-DRIFT (decl set matched, function body drifted),
extraction flagged 8 hand-added rootCmd.AddCommand and 2
trackCmd.AddCommand calls, Apply re-injected them — the published
root.go ended up with two identical blocks of 10 calls.
Fix: extractLostRegistrations now takes a verdict map and skips host
files whose verdict means published is preserved. Only TEMPLATED-CLEAN
and NEW-TEMPLATE-EMISSION hosts get overwritten by Apply with fresh
content, so those are the only ones that need lost-call restoration.
Test: TestExtractLostRegistrationsSkipsPreservedHosts builds a
synthetic fixture with a hand-added AddCommand in published, runs
extraction four times with different verdict maps, and verifies the
nil/clean cases flag the call (1 lost) while the
preserved-verdict cases produce zero (0 lost). Existing tests pass nil
for the verdict map (no skip — preserves their behavior).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(cli): tighten comments and consolidate verdict-preservation predicate
Post-batch /simplify cleanup across the 6 retro fixes:
- Add Verdict.PreservesPublished() method consolidating the
"this verdict means Apply leaves published in place" dichotomy that
registrations.go was checking with an inline switch on four
Verdict constants. Single source of truth for future call sites.
- Remove speculative GoModMerge.RemovedRequires field — the merge no
longer drops requires, so the field would always be empty. Add it
back when there's an actual intentionally-drop mode.
- Trim incident references and PR-narration from comments per
AGENTS.md "no dates, incidents, or ticket numbers in code comments":
• root.go.tmpl: drop FedEx/Dub line counts (commit-message material)
• registrations_test.go: drop "FedEx retro WU-2 root cause" anchor
- Trim WHAT-narration in three comments that crossed from WHY into
restating the code:
• mcp_size.go::estimateMCPTokens: collapse "Sources scanned" enum
• gomod.go::planGoModMerge: drop the operator-explainer paragraph
• helpers.go::shouldClassifyFile: collapse the multi-paragraph
rationale into the single sentence that names the WHY
No behavior changes; tests + golden suite pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(cli): apply review feedback from /review pass
Five small follow-ups surfaced by code review on the FedEx retro batch:
- Trim "Before this fix" PR-narration from two test docstrings that
the prior simplify pass missed (mcp_size_test.go, classify_test.go).
AGENTS.md "no dates, incidents, or ticket numbers in code comments".
- Inline the Verdict.PreservesPublished() predicate back at its single
call site in registrations.go and remove the method. Reviewer
correctly flagged this as premature abstraction — extract when the
second/third call site arrives, per AGENTS.md "three similar lines
is better than a premature abstraction."
- Add PreservedRequires to printHumanRegenReport's go.mod section so
operators running regen-merge without --json can see hand-added
deps reported as preserved. JSON consumers already had this; the
human path was overlooked.
- Update the regen-merge plan doc's example JSON: removed_requires →
preserved_requires (the field rename happened in the simplify
commit but the doc example stayed stale).
No behavior changes; tests + golden suite pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
...6-05-01-001-feat-regen-merge-subcommand-plan.md | 2 +-
internal/cli/regen_merge.go | 3 +
internal/generator/generator.go | 4 +-
internal/generator/root_long_test.go | 14 ++--
internal/generator/templates/root.go.tmpl | 7 +-
internal/pipeline/regenmerge/classify_test.go | 38 ++++++++++
.../pipeline/regenmerge/correctness_fixes_test.go | 2 +-
internal/pipeline/regenmerge/gomod.go | 24 ++++++-
internal/pipeline/regenmerge/gomod_test.go | 58 +++++++++++++++
internal/pipeline/regenmerge/helpers.go | 9 +--
internal/pipeline/regenmerge/regenmerge.go | 21 +++++-
internal/pipeline/regenmerge/registrations.go | 19 ++++-
internal/pipeline/regenmerge/registrations_test.go | 82 ++++++++++++++++++++--
skills/printing-press-polish/SKILL.md | 15 ++++
14 files changed, 271 insertions(+), 27 deletions(-)
diff --git a/docs/plans/2026-05-01-001-feat-regen-merge-subcommand-plan.md b/docs/plans/2026-05-01-001-feat-regen-merge-subcommand-plan.md
index 6ee97d4c..c3908107 100644
--- a/docs/plans/2026-05-01-001-feat-regen-merge-subcommand-plan.md
+++ b/docs/plans/2026-05-01-001-feat-regen-merge-subcommand-plan.md
@@ -289,7 +289,7 @@ was-templated(F in tree) = marker-on-line-2(F) OR (file is named like a template
"merged": false,
"preserved_module_path": "github.com/mvanhorn/printing-press-library/library/.../postman-explore",
"added_requires": ["github.com/x/y v1.2.3"],
- "removed_requires": [],
+ "preserved_requires": ["modernc.org/sqlite v1.50.0"],
"preserved_replaces": ["github.com/local/fork => ./fork"]
}
}
diff --git a/internal/cli/regen_merge.go b/internal/cli/regen_merge.go
index 9f8666d7..2d5e8a25 100644
--- a/internal/cli/regen_merge.go
+++ b/internal/cli/regen_merge.go
@@ -173,6 +173,9 @@ func printHumanRegenReport(w io.Writer, report *regenmerge.MergeReport, applied
if len(report.GoMod.AddedRequires) > 0 {
fmt.Fprintf(w, " added requires: %v\n", report.GoMod.AddedRequires)
}
+ if len(report.GoMod.PreservedRequires) > 0 {
+ fmt.Fprintf(w, " preserved requires: %v\n", report.GoMod.PreservedRequires)
+ }
if len(report.GoMod.PreservedReplaces) > 0 {
fmt.Fprintf(w, " preserved local replaces: %v\n", report.GoMod.PreservedReplaces)
}
diff --git a/internal/generator/generator.go b/internal/generator/generator.go
index 4b949293..70eb4cf5 100644
--- a/internal/generator/generator.go
+++ b/internal/generator/generator.go
@@ -1987,11 +1987,11 @@ func (g *Generator) renderRootProjectFiles(promotedCommands []PromotedCommand, p
// the CLIs that benefit most from the absorb work in the first place.
//
// Size is bounded two ways:
- // 1. per-line truncation via the template's truncate helper (80 runes)
+ // 1. per-line truncation via the template's truncate helper (200 runes)
// 2. a soft cap on total feature lines rendered (MaxHighlightLines);
// overflow becomes a "…and N more — see README" breadcrumb so a
// verbose absorb output doesn't blow up --help
- const maxHighlightLines = 15 // ~300-char overhead ceiling in the worst case
+ const maxHighlightLines = 15 // ~3000-char description ceiling in the worst case
shownNovel := g.NovelFeatures
overflow := 0
if len(shownNovel) > maxHighlightLines {
diff --git a/internal/generator/root_long_test.go b/internal/generator/root_long_test.go
index 9509db02..f870835b 100644
--- a/internal/generator/root_long_test.go
+++ b/internal/generator/root_long_test.go
@@ -182,12 +182,12 @@ func runGoVet(t *testing.T, dir string) error {
//
// Budget (enforced by truncate helper in the template):
// - Headline clipped to 120 runes
-// - Top 3 novel features (cap in Go; remaining 7 dropped)
-// - Each feature description clipped to 80 runes
+// - Top 15 novel features (cap in Go; remaining dropped with overflow breadcrumb)
+// - Each feature description clipped to 200 runes
//
-// Upper bound: Long should never exceed ~1500 chars in practice even
-// when every field is maxed out. We assert a slightly looser cap (2000)
-// so trivial copy tweaks don't break the test.
+// Upper bound: Long should never exceed ~4000 chars even at the worst
+// case (15 features × 200 chars + headline + framing). We assert a
+// slightly looser cap (5000) so trivial copy tweaks don't break the test.
func TestRootLongStaysUnderSizeBudget(t *testing.T) {
t.Parallel()
@@ -220,8 +220,8 @@ func TestRootLongStaysUnderSizeBudget(t *testing.T) {
require.NotEqual(t, -1, longEnd, "Long raw string should close")
longBody := content[longStart : longStart+longEnd]
- assert.LessOrEqual(t, len(longBody), 2000,
- "root --help Long should stay under 2000 chars; got %d chars. Body:\n%s",
+ assert.LessOrEqual(t, len(longBody), 5000,
+ "root --help Long should stay under 5000 chars; got %d chars. Body:\n%s",
len(longBody), longBody)
// All 10 features render (under the 15-item cap) — we dropped the
diff --git a/internal/generator/templates/root.go.tmpl b/internal/generator/templates/root.go.tmpl
index 43601a49..cb00a9bb 100644
--- a/internal/generator/templates/root.go.tmpl
+++ b/internal/generator/templates/root.go.tmpl
@@ -91,7 +91,10 @@ func newRootCmd(flags *rootFlags) *cobra.Command {
the CLIs that benefit most from the work.
Size is bounded two ways:
- • per-line: headline truncated to 120 runes, each description to 80
+ • per-line: headline truncated to 120 runes, each description to 200.
+ The 200-rune cap fits typical novel-feature descriptions
+ (~100-150 chars) so most lines render in full; Cobra wrap still
+ handles narrow terminals.
• total: overflow beyond 15 features becomes a "…and N more" line
(NovelOverflowCount set in Go); stops runaway absorb outputs
from producing a wall of text in --help
@@ -109,7 +112,7 @@ func newRootCmd(flags *rootFlags) *cobra.Command {
Highlights (not in the official API docs):
{{- range .TopNovelFeatures}}
- • {{goRawSafe .Command}} {{goRawSafe (truncate 80 .Description)}}
+ • {{goRawSafe .Command}} {{goRawSafe (truncate 200 .Description)}}
{{- end}}
{{- if .NovelOverflowCount}}
…and {{.NovelOverflowCount}} more — see README.md for the full list
diff --git a/internal/pipeline/regenmerge/classify_test.go b/internal/pipeline/regenmerge/classify_test.go
index b2431988..04436027 100644
--- a/internal/pipeline/regenmerge/classify_test.go
+++ b/internal/pipeline/regenmerge/classify_test.go
@@ -1,6 +1,7 @@
package regenmerge
import (
+ "os"
"path/filepath"
"testing"
@@ -141,6 +142,43 @@ func TestClassifyOutsideCwdRejected(t *testing.T) {
assert.Contains(t, err.Error(), "outside the current working directory")
}
+// TestClassifySpecYamlPropagates pins the contract that spec.yaml at the CLI
+// root is classified (and therefore overwritten by Apply) so source-spec
+// changes propagate into the library copy. Without this, downstream tools
+// (mcp-sync, dogfood, scorecard) read a stale library spec and miss
+// source-side enrichments such as `mcp.endpoint_tools: hidden`.
+func TestClassifySpecYamlPropagates(t *testing.T) {
+ t.Parallel()
+
+ pubDir := t.TempDir()
+ freshDir := t.TempDir()
+
+ // Both trees have a spec.yaml at the root; published is older, fresh
+ // includes a new mcp: block. Either content works; we only check the
+ // verdict here. Apply is covered by the existing TEMPLATED-CLEAN path.
+ require.NoError(t, os.WriteFile(filepath.Join(pubDir, "spec.yaml"),
+ []byte("name: x\nversion: \"0.1.0\"\n"), 0o644))
+ require.NoError(t, os.WriteFile(filepath.Join(freshDir, "spec.yaml"),
+ []byte("name: x\nversion: \"0.1.0\"\nmcp:\n endpoint_tools: hidden\n"), 0o644))
+
+ // Minimal go.mod on both sides so the planGoModMerge path doesn't error
+ // out before classification reaches spec.yaml.
+ require.NoError(t, os.WriteFile(filepath.Join(pubDir, "go.mod"),
+ []byte("module x\n\ngo 1.23\n"), 0o644))
+ require.NoError(t, os.WriteFile(filepath.Join(freshDir, "go.mod"),
+ []byte("module x\n\ngo 1.23\n"), 0o644))
+
+ report, err := Classify(pubDir, freshDir, Options{Force: true})
+ require.NoError(t, err)
+ require.NotNil(t, report)
+
+ verdicts := verdictMap(report)
+ got, ok := verdicts["spec.yaml"]
+ require.True(t, ok, "spec.yaml must participate in classification; got verdicts: %v", verdicts)
+ assert.Equal(t, VerdictTemplatedClean, got,
+ "spec.yaml present in both trees should be TEMPLATED-CLEAN so Apply overwrites with fresh")
+}
+
// --- fixture helpers ---
// postmanFixture returns published, fresh dirs for the postman-explore
diff --git a/internal/pipeline/regenmerge/correctness_fixes_test.go b/internal/pipeline/regenmerge/correctness_fixes_test.go
index fbd41b3f..52ec8b5e 100644
--- a/internal/pipeline/regenmerge/correctness_fixes_test.go
+++ b/internal/pipeline/regenmerge/correctness_fixes_test.go
@@ -74,7 +74,7 @@ type rootFlags struct{}
"internal/cli/root.go": freshRoot,
})
- regs, err := extractLostRegistrations(pubDir, freshDir)
+ regs, err := extractLostRegistrations(pubDir, freshDir, nil)
require.NoError(t, err)
// novel.go must NOT appear in lost-registrations output, even though pub
diff --git a/internal/pipeline/regenmerge/gomod.go b/internal/pipeline/regenmerge/gomod.go
index 55848b1d..48881506 100644
--- a/internal/pipeline/regenmerge/gomod.go
+++ b/internal/pipeline/regenmerge/gomod.go
@@ -62,9 +62,10 @@ func planGoModMerge(publishedDir, freshDir string) (*GoModMerge, error) {
plan.AddedRequires = append(plan.AddedRequires, fmt.Sprintf("%s %s", path, ver))
}
}
+ // Published-only requires are preserved (see GoModMerge.PreservedRequires).
for path, ver := range pubReqs {
if _, ok := freshReqs[path]; !ok {
- plan.RemovedRequires = append(plan.RemovedRequires, fmt.Sprintf("%s %s", path, ver))
+ plan.PreservedRequires = append(plan.PreservedRequires, fmt.Sprintf("%s %s", path, ver))
}
}
@@ -115,9 +116,28 @@ func renderMergedGoMod(publishedDir, freshDir string) ([]byte, error) {
return nil, fmt.Errorf("setting go version: %w", err)
}
}
+ // Merged require set: union of published and fresh.
+ //
+ // Fresh wins on shared paths (newer generator templates pin newer
+ // versions, and dropping fresh's pin would silently downgrade indirect
+ // deps). Published-only requires are preserved so deps the agent added
+ // after the original generation (e.g., `go get modernc.org/sqlite` for
+ // a hand-built local store) survive a regen-merge. Without this, the
+ // merged go.mod would drop the dep and `go build` would fail with
+ // "no required module provides package <X>" on the next build.
+ freshReqPaths := map[string]bool{}
for _, r := range freshMF.Require {
+ freshReqPaths[r.Mod.Path] = true
if err := merged.AddRequire(r.Mod.Path, r.Mod.Version); err != nil {
- return nil, fmt.Errorf("adding require %s: %w", r.Mod.Path, err)
+ return nil, fmt.Errorf("adding fresh require %s: %w", r.Mod.Path, err)
+ }
+ }
+ for _, r := range pubMF.Require {
+ if freshReqPaths[r.Mod.Path] {
+ continue
+ }
+ if err := merged.AddRequire(r.Mod.Path, r.Mod.Version); err != nil {
+ return nil, fmt.Errorf("adding published-only require %s: %w", r.Mod.Path, err)
}
}
diff --git a/internal/pipeline/regenmerge/gomod_test.go b/internal/pipeline/regenmerge/gomod_test.go
index 7e6bbfe0..9a60f889 100644
--- a/internal/pipeline/regenmerge/gomod_test.go
+++ b/internal/pipeline/regenmerge/gomod_test.go
@@ -97,3 +97,61 @@ replace github.com/x/y => github.com/upstream/fork v9.9.9
assert.Equal(t, "github.com/x/y", r.Old.Path)
assert.Equal(t, "./local-fork", r.New.Path, "published's local-path replace wins over fresh's version-replace")
}
+
+// TestRenderMergedGoModPreservesPublishedOnlyRequires pins the contract that
+// requires present in published but absent from fresh survive the merge.
+// Typical case: agent ran `go get modernc.org/sqlite` after generation to
+// build a hand-coded local store; the dep isn't in the spec and won't be in
+// the fresh tree's go.mod. Without preservation, the merged go.mod drops the
+// dep and `go build` fails on the next sweep.
+func TestRenderMergedGoModPreservesPublishedOnlyRequires(t *testing.T) {
+ t.Parallel()
+
+ tmp := t.TempDir()
+ pubDir := filepath.Join(tmp, "pub")
+ freshDir := filepath.Join(tmp, "fresh")
+ require.NoError(t, os.MkdirAll(pubDir, 0o755))
+ require.NoError(t, os.MkdirAll(freshDir, 0o755))
+
+ // Published has a hand-added sqlite dep on top of fresh's baseline.
+ pubGoMod := []byte(`module github.com/example/monorepo/library/foo
+
+go 1.23.0
+
+require (
+ github.com/spf13/cobra v1.9.1
+ modernc.org/sqlite v1.50.0
+)
+`)
+ freshGoMod := []byte(`module foo-pp-cli
+
+go 1.23.0
+
+require github.com/spf13/cobra v1.9.1
+`)
+ require.NoError(t, writeFileAtomic(filepath.Join(pubDir, "go.mod"), pubGoMod))
+ require.NoError(t, writeFileAtomic(filepath.Join(freshDir, "go.mod"), freshGoMod))
+
+ // Plan: published-only require lands in PreservedRequires.
+ plan, err := planGoModMerge(pubDir, freshDir)
+ require.NoError(t, err)
+ require.NotNil(t, plan)
+ require.Len(t, plan.PreservedRequires, 1)
+ assert.Contains(t, plan.PreservedRequires[0], "modernc.org/sqlite",
+ "sqlite dep must be reported as preserved so operators can see hand-additions survive")
+
+ // Render: merged go.mod still requires sqlite.
+ bytes, err := renderMergedGoMod(pubDir, freshDir)
+ require.NoError(t, err)
+ parsed, err := modfile.Parse("merged-go.mod", bytes, nil)
+ require.NoError(t, err)
+
+ gotPaths := map[string]string{}
+ for _, req := range parsed.Require {
+ gotPaths[req.Mod.Path] = req.Mod.Version
+ }
+ assert.Equal(t, "v1.50.0", gotPaths["modernc.org/sqlite"],
+ "hand-added sqlite must survive the merge with its published version")
+ assert.Equal(t, "v1.9.1", gotPaths["github.com/spf13/cobra"],
+ "shared deps stay at fresh's version")
+}
diff --git a/internal/pipeline/regenmerge/helpers.go b/internal/pipeline/regenmerge/helpers.go
index 18b9006a..d7b1d274 100644
--- a/internal/pipeline/regenmerge/helpers.go
+++ b/internal/pipeline/regenmerge/helpers.go
@@ -70,15 +70,16 @@ func shouldWalkDir(name string) bool {
}
// shouldClassifyFile returns true for files that participate in classification.
-// Includes .go and a small allowlist of root-level config files. Compiled
-// binaries (no extension at the CLI dir root) and other non-source files are
-// skipped.
+// Includes .go and a small allowlist of root-level generator-owned files.
+// spec.yaml / spec.json are generator-owned (downstream tools — mcp-sync,
+// dogfood, scorecard — re-parse them at runtime), so source-spec changes
+// must propagate via Apply's TEMPLATED-CLEAN path.
func shouldClassifyFile(rel string) bool {
if strings.HasSuffix(rel, ".go") {
return true
}
switch filepath.Base(rel) {
- case "go.mod", "go.sum":
+ case "go.mod", "go.sum", "spec.yaml", "spec.json":
return true
}
return false
diff --git a/internal/pipeline/regenmerge/regenmerge.go b/internal/pipeline/regenmerge/regenmerge.go
index fa030dbb..09921091 100644
--- a/internal/pipeline/regenmerge/regenmerge.go
+++ b/internal/pipeline/regenmerge/regenmerge.go
@@ -125,8 +125,12 @@ type GoModMerge struct {
Merged bool `json:"merged"`
PreservedModulePath string `json:"preserved_module_path"`
AddedRequires []string `json:"added_requires,omitempty"`
- RemovedRequires []string `json:"removed_requires,omitempty"`
- PreservedReplaces []string `json:"preserved_replaces,omitempty"`
+ // PreservedRequires lists requires present in published but absent from
+ // fresh that the merge keeps. Typical case: deps the agent added after
+ // the original generation (e.g., `go get modernc.org/sqlite` for a
+ // hand-built local store).
+ PreservedRequires []string `json:"preserved_requires,omitempty"`
+ PreservedReplaces []string `json:"preserved_replaces,omitempty"`
}
// MergeReport is the full output of Classify (and Apply, with applied flags
@@ -186,7 +190,18 @@ func Classify(publishedDir, freshDir string, opts Options) (*MergeReport, error)
}
report.Files = files
- regs, err := extractLostRegistrations(pubAbs, freshAbs)
+ // Build a verdict map keyed by relative path so the lost-registration
+ // extractor can skip hosts that Apply preserves verbatim. Without this,
+ // AddCommand calls in TEMPLATED-BODY-DRIFT or TEMPLATED-WITH-ADDITIONS
+ // host files (root.go after Phase-3 hand-edits is the canonical case)
+ // get flagged as "lost" and re-injected by Apply on top of the
+ // preserved file — producing duplicate AddCommand lines that crash
+ // at startup with "command is already added".
+ verdicts := make(map[string]Verdict, len(files))
+ for _, fc := range files {
+ verdicts[fc.Path] = fc.Verdict
+ }
+ regs, err := extractLostRegistrations(pubAbs, freshAbs, verdicts)
if err != nil {
return nil, fmt.Errorf("extracting lost registrations: %w", err)
}
diff --git a/internal/pipeline/regenmerge/registrations.go b/internal/pipeline/regenmerge/registrations.go
index b55ae211..dc323156 100644
--- a/internal/pipeline/regenmerge/registrations.go
+++ b/internal/pipeline/regenmerge/registrations.go
@@ -25,7 +25,16 @@ import (
// "Host file" is any internal/cli/*.go file in published that contains at
// least one AddCommand call (root.go, plus resource-parents like
// category.go).
-func extractLostRegistrations(publishedDir, freshDir string) ([]LostRegistration, error) {
+//
+// pubVerdicts maps relative path → Apply verdict. Hosts whose verdict means
+// the published file is preserved verbatim (TEMPLATED-BODY-DRIFT,
+// TEMPLATED-WITH-ADDITIONS, NOVEL, NOVEL-COLLISION) are skipped — re-injecting
+// AddCommand calls into a file that already has them would duplicate the
+// calls and crash the resulting CLI at startup with
+// "command is already added". Pass an empty map (or a map with all entries
+// classified as TEMPLATED-CLEAN) to opt out of the filter; callers in the
+// Classify pipeline always pass the populated map.
+func extractLostRegistrations(publishedDir, freshDir string, pubVerdicts map[string]Verdict) ([]LostRegistration, error) {
pubCLIDir := filepath.Join(publishedDir, "internal", "cli")
freshCLIDir := filepath.Join(freshDir, "internal", "cli")
@@ -81,6 +90,14 @@ func extractLostRegistrations(publishedDir, freshDir string) ([]LostRegistration
if _, existsInFresh := freshHostBasenames[filepath.Base(host)]; !existsInFresh {
continue
}
+ // Skip hosts whose Apply verdict preserves published verbatim — the
+ // AddCommand calls already survive the merge in-place; re-injection
+ // would duplicate them.
+ relPath := filepath.ToSlash(filepath.Join("internal", "cli", filepath.Base(host)))
+ switch pubVerdicts[relPath] {
+ case VerdictTemplatedBodyDrift, VerdictTemplatedWithAdditions, VerdictNovel, VerdictNovelCollision:
+ continue
+ }
var lost, skipped []string
for _, call := range pubCalls[host] {
if _, present := freshCallSet[call.normalized]; present {
diff --git a/internal/pipeline/regenmerge/registrations_test.go b/internal/pipeline/regenmerge/registrations_test.go
index 97633c3c..7e2b6422 100644
--- a/internal/pipeline/regenmerge/registrations_test.go
+++ b/internal/pipeline/regenmerge/registrations_test.go
@@ -16,7 +16,7 @@ func TestExtractLostRegistrationsPostmanExplore(t *testing.T) {
pubDir, freshDir := postmanFixture(t)
- regs, err := extractLostRegistrations(pubDir, freshDir)
+ regs, err := extractLostRegistrations(pubDir, freshDir, nil)
require.NoError(t, err)
// Group by host file.
@@ -144,7 +144,7 @@ func newRssCmd(rootFlags) *cobra.Command { return nil }
pubDir, freshDir := buildSyntheticFixture(t,
map[string]string{"internal/cli/root.go": pubCLI},
map[string]string{"internal/cli/root.go": freshCLI})
- regs, err := extractLostRegistrations(pubDir, freshDir)
+ regs, err := extractLostRegistrations(pubDir, freshDir, nil)
require.NoError(t, err)
assert.Empty(t, regs, "arg-shape variation alone should not produce lost registrations")
}
@@ -173,7 +173,7 @@ func newSubCmd() *cobra.Command { return nil }
pubDir, freshDir := buildSyntheticFixture(t,
map[string]string{"internal/cli/root.go": chainedCLI},
map[string]string{"internal/cli/root.go": chainedCLI})
- regs, err := extractLostRegistrations(pubDir, freshDir)
+ regs, err := extractLostRegistrations(pubDir, freshDir, nil)
require.NoError(t, err)
assert.Empty(t, regs, "identical chained-call AddCommand should match via text fallback")
}
@@ -213,7 +213,7 @@ func newSubCmd() *cobra.Command { return nil }
pubDir, freshDir := buildSyntheticFixture(t,
map[string]string{"internal/cli/root.go": pubCLI},
map[string]string{"internal/cli/root.go": freshCLI})
- regs, err := extractLostRegistrations(pubDir, freshDir)
+ regs, err := extractLostRegistrations(pubDir, freshDir, nil)
require.NoError(t, err)
// pub registers newSubCmd under parentCmd; fresh registers it under
@@ -223,3 +223,77 @@ func newSubCmd() *cobra.Command { return nil }
assert.Contains(t, regs[0].Calls, "parentCmd.AddCommand(newSubCmd())",
"lost call should preserve the parentCmd parent (not deduped against rootCmd's call)")
}
+
+// TestExtractLostRegistrationsSkipsPreservedHosts pins the contract that hosts
+// whose Apply verdict preserves published verbatim (TEMPLATED-BODY-DRIFT,
+// TEMPLATED-WITH-ADDITIONS, NOVEL, NOVEL-COLLISION) do NOT contribute lost
+// registrations. The published file already has the calls; re-injection
+// would duplicate them and crash the resulting CLI at startup with
+// "command is already added".
+func TestExtractLostRegistrationsSkipsPreservedHosts(t *testing.T) {
+ t.Parallel()
+
+ // pub/root.go has hand-added AddCommand call; fresh/root.go is the
+ // generator's pristine version without it.
+ pubRoot := `package cli
+
+import "github.com/spf13/cobra"
+
+func Execute() {
+ rootCmd := &cobra.Command{Use: "x"}
+ rootCmd.AddCommand(newGenericCmd())
+ rootCmd.AddCommand(newHandAddedCmd())
+ _ = rootCmd.Execute()
+}
+
+func newGenericCmd() *cobra.Command { return nil }
+func newHandAddedCmd() *cobra.Command { return nil }
+`
+ freshRoot := `package cli
+
+import "github.com/spf13/cobra"
+
+func Execute() {
+ rootCmd := &cobra.Command{Use: "x"}
+ rootCmd.AddCommand(newGenericCmd())
+ _ = rootCmd.Execute()
+}
+
+func newGenericCmd() *cobra.Command { return nil }
+func newHandAddedCmd() *cobra.Command { return nil }
+`
+ pubDir, freshDir := buildSyntheticFixture(t,
+ map[string]string{"internal/cli/root.go": pubRoot},
+ map[string]string{"internal/cli/root.go": freshRoot})
+
+ // Without the verdict map (nil → no skip), the call is correctly
+ // flagged as lost so it can be re-injected into a TEMPLATED-CLEAN host.
+ regsNoFilter, err := extractLostRegistrations(pubDir, freshDir, nil)
+ require.NoError(t, err)
+ require.Len(t, regsNoFilter, 1, "without verdict filter, lost call must be flagged")
+ assert.Contains(t, regsNoFilter[0].Calls, "rootCmd.AddCommand(newHandAddedCmd())")
+
+ // With root.go marked TEMPLATED-BODY-DRIFT (Apply preserves published
+ // verbatim), the same call must NOT be flagged — it already lives in
+ // the file that survives the merge.
+ verdicts := map[string]Verdict{"internal/cli/root.go": VerdictTemplatedBodyDrift}
+ regsWithFilter, err := extractLostRegistrations(pubDir, freshDir, verdicts)
+ require.NoError(t, err)
+ assert.Empty(t, regsWithFilter,
+ "TEMPLATED-BODY-DRIFT host must contribute zero lost registrations to avoid duplicate AddCommand injection")
+
+ // Same for TEMPLATED-WITH-ADDITIONS: published is preserved.
+ verdicts["internal/cli/root.go"] = VerdictTemplatedWithAdditions
+ regsAdditions, err := extractLostRegistrations(pubDir, freshDir, verdicts)
+ require.NoError(t, err)
+ assert.Empty(t, regsAdditions,
+ "TEMPLATED-WITH-ADDITIONS host must also be skipped — published is preserved")
+
+ // TEMPLATED-CLEAN host (rare for hand-added registrations but possible
+ // when the calls are inside a function whose decl-set still matches):
+ // fresh wins, so injection is required.
+ verdicts["internal/cli/root.go"] = VerdictTemplatedClean
+ regsClean, err := extractLostRegistrations(pubDir, freshDir, verdicts)
+ require.NoError(t, err)
+ require.Len(t, regsClean, 1, "TEMPLATED-CLEAN host must still need injection so the lost call survives")
+}
diff --git a/skills/printing-press-polish/SKILL.md b/skills/printing-press-polish/SKILL.md
index 0ede58a7..360aae08 100644
--- a/skills/printing-press-polish/SKILL.md
+++ b/skills/printing-press-polish/SKILL.md
@@ -468,6 +468,21 @@ The ship gates are a floor, not a ceiling. After they pass, look at scorecard di
3. **If the deficit is structural, document and accept.** Some dimensions assume capabilities the CLI's domain doesn't have (a read-only API scored against write-workflow dimensions, a CLI with no auth scored on auth dimensions, a small API penalized on `surface_strategy` thresholds calibrated for large APIs). Note the reason in `skipped_findings` and move on.
4. **Never add scaffolding to satisfy the scorer.** Fake commands, fake tests, fake flags, or boilerplate prose written purely to nudge a number — those degrade the CLI to satisfy the proxy. The scorer is imperfect by design (the "scoring may be imperfect" caveat in AGENTS.md applies). Trust the underlying judgment, not the number.
+#### MCP scorecard dims map to spec fields, not generator code
+
+When `mcp_token_efficiency`, `mcp_tool_design`, `mcp_remote_transport`, or `mcp_surface_strategy` are below max, the fix is almost always a spec edit + regenerate (or `regen-merge` from a freshly-generated tree), **not** a generator-template change. Polish CAN address these — do not classify them as "feature add to a generator-owned file, retro candidate."
+
+| Weak dim | Spec field that fixes it | What to add to `spec.yaml`'s `mcp:` block |
+|---|---|---|
+| `mcp_remote_transport` | `mcp.transport` | `transport: [stdio, http]` (default is stdio-only; HTTP costs nothing and lets the same binary serve cloud-hosted agents) |
+| `mcp_token_efficiency`, `mcp_surface_strategy` | `mcp.endpoint_tools`, `mcp.orchestration` | `endpoint_tools: hidden` + `orchestration: code` (Cloudflare pattern: ~70 raw endpoint tools collapse to `<api>_search` + `<api>_execute`; all endpoints still reachable via execute) |
+| `mcp_tool_design` | `mcp.intents` | Define multi-step intent compositions for the workflows the API supports |
+| `mcp_description_quality` | `mcp-descriptions.json` (override file at the CLI root) | Per-tool description overrides; thin spec-derived descriptions get richer text without spec edits |
+
+Recommended threshold: at >50 typed endpoints, default to recommending all four (`transport`, `endpoint_tools=hidden`, `orchestration=code`, `intents` for the headline workflows). Below 30, `transport=[stdio, http]` is the only zero-cost win. The full reference is `docs/SPEC-EXTENSIONS.md`.
+
+After editing the spec, regenerate (or `regen-merge` the changes into the published library) so the new `mcp:` block reaches templates. Cobratree-walked novel commands continue to surface as MCP tools either way; they don't need spec changes.
+
Rule of thumb: if your fix would still be valuable if the scorecard didn't exist, do it. If the only motivation is "to push the score," don't.
## Display delta and emit result block
← d364472b chore(main): release 3.7.0 (#523)
·
back to Cli Printing Press
·
feat(cli): OAuth2 client_credentials grant + bearer_token Au 593ce970 →