fix(generator): Classic update --name, apply and --group refuse an ambiguous name

[ktn-jamf]

Stacked on #401. Its base is fix/generator-quote-spec-strings, so review only the commits listed below. GitHub retargets this PR to main when #401 merges.

Why

Jamf Pro allows two records to share a name. Three generated Classic paths acted on one of them silently:

• update --name on all 33 Classic resources that take it asked the server's /name/ endpoint for the record, then wrote to whichever record came back. It reported success. A test with two records named "Baseline" wrote to the server's choice and exited 0. delete --name and apply already refused this case.
• --group on the generated bulk commands took the first case-insensitive match. With groups "LAB" and "Lab", delete --group Lab --yes deleted the members of "LAB".
• apply on mac and mobile apps resolved the ID, then fetched the record again by name. It could merge one app's body into another app's ID.

What changed (generator/classic/generator.go, then make generate)

• update --name resolves the name from the collection with resolveClassicNameToIDForApply, then writes by ID. A name that no record has, or that two records share, writes nothing. With --no-input, or when stdin is not a terminal, a collision is refused and the error names the colliding IDs.
• classicFindIDsByName collects every case-insensitive match. A unique exact-case match wins, and any other ambiguity is refused with the colliding IDs. A group list over 4 MiB is refused instead of being searched in part.
• App apply fetches its merge base by the resolved ID.
• The delete collision hint now says "delete" instead of "update".

The device-action --group commands, such as computer-inventory erase, use the hand-written resolver in internal/resolve. pro.go replaces the generated versions. The resolvers PR covers them.

Verification

go test ./internal/commands/pro/generated/ ./internal/commands/ -run 'ClassicUpdateByName|GroupFlagDelete|ClassicApply|ClassicFindIDByName|FetchClassicGroupMemberIDs|ClassicNameCollision' -count=1
make generate && make verify-generated

The first commit adds the repro tests alone, and they fail. For example, update sent PUT …/osxconfigurationprofiles/id/22 and reported success, and delete --group Lab deleted devices 701 and 702. The later commits make them pass. The no-match guard, the exact-case rule, the apply fetch and the --no-input refusal were each mutation-checked. Regeneration leaves the tree clean.

One existing test changed, with approval. In bulk_delete_test.go, the subtest "first match returned when duplicates exist" now asserts the refusal, which names IDs: 5, 6. The file's other subtests are unchanged.

Review covered security and test quality. Five of five mutations were killed.

🤖 Generated with Claude Code

[neilmartin83]

Warning

⚠️ Needs changes. Makes Classic update --name, app apply and --group refuse an ambiguous name instead of acting on the server's pick or the first match.
Blocking: (1), (2). Two nice-to-haves sit in the collapsed section.

Rating: 4/5

• The fix itself is correct and well tested. It would be a 5 once CI has run against main (1) and the PR picks one rule for "which record does this name mean" (2).

Findings

⚠️ IMPORTANT (1) (reliability) — .github/workflows/ci.yaml:7: CI has never run on this PR

ci.yaml fires on pull_request to main. Until #401 merged, this PR targeted fix/generator-quote-spec-strings. Retargeting fires edited, which the default trigger types leave out, and gh pr checks 405 reports no checks. #401 was squash-merged as c82bacb2, so the branch still carries its eight pre-squash commits. GitHub's diff now shows 59 files, where this PR's own change is 41.

Failure scenario: the PR merges green-by-absence. Nothing in CI ran make verify-generated, go vet, the guard tests, or a lint on the head being merged.

Suggested fix: rebase onto main and drop the eight #401 commits (7f4c8dc1..5dd89577), or merge main in. Then push. Both trees are byte-identical (git diff 5dd89577 c82bacb2 is empty), so the rebase is mechanical.

Fixed when: the PR diff shows only this change's files, and ci.yaml has a green run on the new head.

⚠️ IMPORTANT (2) (design) — generator/classic/generator.go:3127: two different rules for an ambiguous name, and the more permissive one is on the destructive path

classicFindIDsByName, used by --group, lets a unique exact-case match win. resolveClassicNameToIDForApply (:2124), used by update --name, delete --name and apply, treats every case-insensitive match as a collision. In the same tenant, the same name now resolves under one rule and is refused under the other. The PR says the hand-written internal/resolve group resolver goes next, so whichever rule it copies becomes the precedent.

Failure scenario: computer groups LAB (7) and Lab (8) exist. pro classic-computer-groups update --name Lab --no-input refuses, naming IDs 7 and 8. pro computer-inventory delete --group Lab --yes deletes group 8's members without a word. The destructive command is the one that does not stop.

Suggested fix: use one rule. My preference is to drop the exact-case preference and refuse any case-insensitive collision, because an ID always resolves it and two names that differ only by case are almost always an accident. If exact-case-wins is the deliberate choice, apply it in resolveClassicNameToIDForApply as well. Either way, write the rule into .claude/rules/classic-api.md under "A name is resolved to at most one record" so the resolvers PR inherits it rather than choosing again.

Fixed when: both resolvers give the same answer for LAB/Lab, a test pins that answer on both paths, and the rule is written down.

Nice-to-have suggestions (2 items)

💡 NICE-TO-HAVE (3) (code-quality) — generator/classic/generator.go:3075: classicFindIDByName is now called only by bulk_delete_test.go. It returns "" for both "not found" and "ambiguous", which is the conflation this PR removes. A later caller will bring the bug back. Delete it, point those five assertions at classicFindIDsByName, and fix the comment that names it at registry.go:1324.

💡 NICE-TO-HAVE (4) (documentation) — CHANGELOG.md:37: the CHANGELOG says "a group list larger than 4 MiB is refused". readClassicGroupBody also limits the group's membership GET. That is the limit a real tenant reaches first, at roughly 20k members (~180 B each). Before, such a group was cut short silently and a subset of its members was deleted. Refusing is the right fix, but the CHANGELOG should say that it applies to the membership read as well. The error also needs a remedy, for example passing device IDs directly.

This covers all findings — addressing the above gets this PR to merge-ready.

Review coverage and scope

• Correctness: all three update template branches, the app apply merge base, bulk and single delete, the --group matcher and size limit, and ResolveClassicNameToID's caller in pro_blueprints.go. client.Do errors on ≥400 (client.go:178), so a failed list GET cannot read as "not found"
• Design: the resolver asymmetry is (2). Replacing the /name/ endpoint with collection lookup matches delete --name and apply
• Security: new template sites interpolate manifest tokens that validateManifestTokens limits to [A-Za-z0-9_./-], so the goStr rule from #401 is not broken
• Silent failure: the deleted fetchClassicProfileByName swallowed every error, and nothing replaces it with a swallow. The truncate-and-partially-match on group bodies is gone
• Test coverage: repro-first commits. All three update forms are tested for sole match, no match and collision; the piped-stdin refusal, exact-case, ambiguous and sole-match --group on two surfaces, and the oversized list are tested. Not covered: the membership-body limit, see (4)
• Reliability: CI gap is (1)
• Performance: update --name now costs one collection GET instead of a /name/ GET. delete and apply already paid that cost. Acceptable
• Verification: targeted tests pass; make generate in a clean copy of the head leaves no diff; full go test ./... green
• [na] Frontend, dependencies, migrations.

Diff: 41 files, +1064/−360, against the stack base 5dd89577 (tree-identical to main's c82bacb2), head 88f5485e. About 220 lines are template source; the rest is make generate output and tests. Lanes run: none. The author's description reports security and test-quality lanes, and the template change was small enough to trace in full by hand. Findings were self-scored, with no verifier pass. Conventions: root CLAUDE.md, generator/CLAUDE.md, and .claude/rules/{classic-api,coding-style,credentials-and-auth}.md, none of which this PR changes.

Prior review status

No prior reviews.

_{🤖 Generated by the pr-review:review skill v1.39.0 · reviewed head 88f5485e}

[neilmartin83]

Warning

⚠️ Needs changes. Makes Classic update --name, app apply and --group refuse an ambiguous name instead of acting on the server's pick or the first match.
Blocking: (1). Round 1's CI gap is fixed. Nice-to-haves are collapsed.

Rating: 3/5

• Would be a 5 once update --name finds a record named 2024 or true again (1). Round 1's (2) moved to nice-to-have on wire evidence, see that finding.

Findings

⚠️ IMPORTANT (1) (correctness, via devil-advocate) — generator/classic/generator.go:2130: update --name no longer finds a record whose name reads as a number or boolean

The resolver builds list items with xmlconv.ExtractListItems, and coerceValue (internal/xmlconv/xmlconv.go:205) turns true, false and any strconv.ParseFloat-able text into bool or float64. itemName, _ := item["name"].(string) then yields "", so the name never matches. delete --name and apply already had this hole. This PR moves update --name on 33 resources onto it from the /name/ endpoint, which matched any name. Two records both named 2024 are also invisible to the collision refusal. Verified by calling resolveClassicNameToIDForApply against a list fixture on this head merged with main: 2024, true and 1.50 resolve to "" with no error, and 007 and Lab resolve to ID 5. Inf and NaN also parse as floats.

Failure scenario: a policy or computer group named 2024 → pro classic-policies update --name 2024 --from-file p.xml used to update it through /name/2024, and now fails with no policy found with name "2024" and writes nothing. apply on the same body takes the create branch.

Suggested fix: read <name> as text. classicFindIDsByName already walks the list with xml.Decoder and never coerces, so collect its case-insensitive matches for the XML branch and keep the exact-case rule out of it:

-		items, err = xmlconv.ExtractListItems(body)
-		...
-	for _, item := range items {
-		itemName, _ := item["name"].(string)
+		matches = classicFoldedNameMatches(body, name) // new helper split out of classicFindIDsByName

Decode the JSON fallback into []struct{ ID json.Number; Name string } rather than map[string]any.

Fixed when: update --name, delete --name and apply resolve records named 2024, true and 1.50, refuse two records both named 2024, and a fixture test pins all of it.

Nice-to-have suggestions (6 items)

💡 NICE-TO-HAVE (2) (design, open 2 rounds) — generator/classic/generator.go:3127: the two resolvers still use different ambiguity rules

classicFindIDsByName lets a unique exact-case match win, and resolveClassicNameToIDForApply refuses every case-insensitive collision. Write one rule into .claude/rules/classic-api.md under "A name is resolved to at most one record" so #408 inherits it rather than choosing again.

Severity re-derived (round 2): round 1's scenario, groups LAB and Lab, cannot be created. On pro-nmartin, a Classic create of a computer group, mobile device group or printer whose name differs from an existing one only by case answers 409 Error: Duplicate name. So does a rename to such a name, and so does a trailing space. The asymmetry is real in code and not reachable on the wire through these paths.

Promote to IMPORTANT when: a tenant is shown holding two group names that differ only by case.

💡 NICE-TO-HAVE (3) (code-quality, open 2 rounds) — generator/classic/generator.go:3075: classicFindIDByName is now called only by bulk_delete_test.go. It returns "" for both "not found" and "ambiguous", which is the conflation this PR removes. Delete it, point those assertions at classicFindIDsByName, and fix the comment that names it at registry.go:1324.

💡 NICE-TO-HAVE (4) (documentation, open 2 rounds) — CHANGELOG.md:37: the 4 MiB limit also applies to the membership GET in readClassicGroupBody, which a real tenant reaches first, at roughly 17k to 23k members. Passing the group's ID hits the same cap, so the error and the CHANGELOG need to say so and name the remedy, which is passing device IDs. The performance and devil-advocate lanes reached the same limit independently.

💡 NICE-TO-HAVE (5) (usability, via usability-reviewer+devil-advocate) — internal/commands/pro/generated/classic_registry.go:616: the collision refusal and chooser now serve update, and neither fits it

The --no-input refusal tells an update --name caller to "use update with a specific ID", which is the command they ran, and never says to pass the ID as <id>. It is a plain fmt.Errorf, so it exits 1 with no hint, where the repo uses exitcode.New(...).WithHint(...). The interactive chooser lists only [n] ID: x for records that share a name, so the operator picks a write target blind, and 0 and x both return a bare aborted. Template source is generator/classic/generator.go:2153.

Fixed when: the refusal says to run the command with one of the IDs as <id> instead of --name, carries a hint and a non-1 exit code, and the chooser shows one distinguishing field per candidate.

💡 NICE-TO-HAVE (6) (performance, via performance-reviewer+devil-advocate) — internal/commands/pro/generated/classic_registry.go:566: update --name now reads the whole collection with an unbounded io.ReadAll

On the generic branch it used to cost zero GETs before the PUT, and now costs a full list. A slow list on a large tenant can pass the gateway's 90s edge, and the error is a bare listing resources: %w with no hint to pass an <id>. The sibling group reads in the same PR are capped at 4 MiB.

Fixed when: a timed-out or 504 list lookup tells the caller to pass an <id>, and the read has a stated bound.

💡 NICE-TO-HAVE (7) (silent-failure, via silent-failure-hunter) — generator/classic/generator.go:1514: if ferr != nil || len(fullBody) == 0 wraps a nil ferr when the by-ID GET returns 200 with an empty body, so the user reads fetching existing mac_application for merge-put: %!w(<nil>). Give the empty-body case its own message, as the update path already does.

This covers all findings — addressing the above gets this PR to merge-ready.

Review coverage and scope

• Correctness: name coercion is (1); merge of main (#401, #403, #407) is conflict-free
• Design: resolver asymmetry is (2), re-derived on wire evidence
• Security: names never reach a URL now; collision errors print IDs only
• Silent failure: decode-error and unparseable-list paths, see nice-to-haves
• Test coverage: 5 lane mutations on uncovered branches, all survived
• Reliability: CI green on 6f6f5c42; head merged with current main regenerates clean, builds, go test ./... passes
• Performance: one collection GET per update --name; unbounded read noted
• Project rules compliance: .claude/rules/{classic-api,coding-style,credentials-and-auth}.md, generator/CLAUDE.md
• [na] Frontend, dependencies, migrations.

Diff: 41 files, +1064/−360, head 6f6f5c42, merge base b3ad3f29; own patch byte-identical to round 1's, incremental range is two merges of main. Merges cleanly onto current main (with #404). Lanes run: security-reviewer, usability-reviewer, performance-reviewer, scope-reviewer, silent-failure-hunter, devil-advocate, test-quality-reviewer. Never run: fidelity-reviewer, inapplicable (no linked issue). Wire probes on pro-nmartin; every prreview- object created was deleted. Conventions: 1 root + 1 nested CLAUDE.md, 3 rules, from the worktree.

PR decomposition

Recommendation: deliver as a single PR. The template source is about 220 lines in generator/classic/generator.go; everything else is make generate output and tests beside the behaviour they pin. update --name/apply by ID, the non-terminal refusal, and --group ambiguity all rewrite the same helpers and regenerate the same classic_registry.go, so a split conflicts on generated output. --group is the only defensible seam and is not worth the regeneration churn. #408 covers the hand-written resolver, so no ordering dependency.

Prior review status

#	Location	Rounds	State	Notes
(1)	.github/workflows/ci.yaml:7	1	✅ Fixed	main merged in; diff is 41 files and CI passed on 6f6f5c42
(2)	generator/classic/generator.go:3127	2	🔴 Open	No change, no reply. Re-derived to nice-to-have, now (2)
(3)	generator/classic/generator.go:3075	2	🔴 Open	No change, no reply, now (3)
(4)	CHANGELOG.md:37	2	🔴 Open	No change, no reply, now (4)

What's done well

• ✅ The 4 MiB cap refuses rather than truncates, which closes the silent partial-match the old reader had.

_{🤖 Generated by the pr-review:review skill v1.39.0 · reviewed head 6f6f5c42}

[neilmartin83]

Warning

⚠️ Needs changes. Makes Classic update --name, app apply and --group refuse an ambiguous name instead of acting on the server's pick or the first match.
Blocking: (1), now confirmed on the wire. No code changed since round 2. This round closes three findings that reached three rounds; see Dispositions.

Rating: 3/5

• Would be a 5 once update --name finds a record named 2024 or true again (1). The rest is nice-to-have.

Findings

⚠️ IMPORTANT (1) (correctness, via devil-advocate, open 2 rounds) — generator/classic/generator.go:2130: update --name no longer finds a record whose name reads as a number or boolean

The resolver builds list items with xmlconv.ExtractListItems, and coerceValue (internal/xmlconv/xmlconv.go:205) turns true, false and any strconv.ParseFloat-able text into bool or float64. itemName, _ := item["name"].(string) then yields "", so the name never matches. delete --name and apply already had this hole. This PR moves update --name on 33 resources onto it from the /name/ endpoint, which matched any name. Two records both named 2024 are also invisible to the collision refusal.

Wire-confirmed this round on pro-nmartin, head 6f6f5c42: a disposable printer named 2024 (id 228) was created. pro classic-printers get --name 2024 returned it, since get still uses /name/. update --name 2024 --from-file u.xml and -n delete --name 2024 --yes both answered no printer found with name "2024". The printer was deleted afterwards and its GET now 404s.

Failure scenario: a policy or computer group named 2024 → pro classic-policies update --name 2024 --from-file p.xml used to update it through /name/2024, and now fails with no policy found with name "2024" and writes nothing. apply on the same body takes the create branch, which then 409s on the duplicate name or creates a second record where names are not enforced unique.

Cross-PR note: #408 fixes this same coercion class in the hand-written resolver (internal/resolve/group_numeric_name_repro_test.go, TestResolveComputerGroup_NumericLookingClassicNameIsNotCoerced). This PR leaves the generated twin with the defect. Both PRs should read <name> as text, so the two resolvers do not drift apart again.

Suggested fix: read <name> as text. classicFindIDsByName already walks the list with xml.Decoder and never coerces, so collect its case-insensitive matches for the XML branch and keep the exact-case rule out of it:

-		items, err = xmlconv.ExtractListItems(body)
-		...
-	for _, item := range items {
-		itemName, _ := item["name"].(string)
+		matches = classicFoldedNameMatches(body, name) // new helper split out of classicFindIDsByName

Decode the JSON fallback into []struct{ ID json.Number; Name string } rather than map[string]any.

Fixed when: update --name, delete --name and apply resolve records named 2024, true and 1.50, refuse two records both named 2024, and a fixture test pins all of it.

Nice-to-have suggestions (3 items)

💡 NICE-TO-HAVE (5) (usability, via usability-reviewer+devil-advocate, open 2 rounds) — internal/commands/pro/generated/classic_registry.go:616: the collision refusal and chooser now serve update, and neither fits it. The --no-input refusal tells an update --name caller to "use update with a specific ID", which is the command they ran, and does not say to pass the ID as <id>. It is a plain fmt.Errorf, so it exits 1 with no hint, where the repo uses exitcode.New(...).WithHint(...). The interactive chooser lists only [n] ID: x, so the operator picks a write target with no other detail. Template source: generator/classic/generator.go:2153. Fixed when: the refusal says to run the command with one of the IDs as <id> instead of --name, carries a hint and a non-1 exit code, and the chooser shows one distinguishing field per candidate.

💡 NICE-TO-HAVE (6) (performance, via performance-reviewer+devil-advocate, open 2 rounds) — internal/commands/pro/generated/classic_registry.go:566: update --name now reads the whole collection with an unbounded io.ReadAll. On a large tenant a slow list can pass the gateway's 90s edge, and the error is a bare listing resources: %w with no hint to pass an <id>. Fixed when: a timed-out or 504 list lookup tells the caller to pass an <id>, and the read has a stated bound.

💡 NICE-TO-HAVE (7) (silent-failure, via silent-failure-hunter, open 2 rounds) — generator/classic/generator.go:1514: if ferr != nil || len(fullBody) == 0 wraps a nil ferr when the by-ID GET returns 200 with an empty body, so the user reads fetching existing mac_application for merge-put: %!w(<nil>). Give the empty-body case its own message, as the update path already does.

This covers all findings — addressing the above gets this PR to merge-ready.

Dispositions

Disposition (round 3) — dismissed. Was (2), generator/classic/generator.go:3127, open 3 rounds. Round 2 showed on the wire that a Classic create or rename to a name that differs from an existing one only in case answers 409 Error: Duplicate name, so the LAB/Lab scenario cannot occur through these paths. The precedent concern is also settled: #408's hand-written group resolver (internal/resolve/resolve.go:696) uses the same rule as classicFindIDsByName, a unique exact match first and then a unique case-insensitive match. Both group resolvers agree.

Disposition (round 3) — retired, out of scope. Was (3), generator/classic/generator.go:3075, open 3 rounds. classicFindIDByName is unexported and only bulk_delete_test.go calls it. Deleting it changes no command's behaviour, so it does not block this PR. Delete it when the list-parsing helper for (1) is written, because that change replaces this code anyway.

Disposition (round 3) — retired, out of scope. Was (4), CHANGELOG.md:37, open 3 rounds. The finding is correct. readClassicGroupBody also limits the membership GET (classic_registry.go:1479), and the error GET …/id/<n>: response exceeds 4 MiB gives no remedy. But the PR replaces main's silent io.LimitReader(resp.Body, 4<<20) truncation (main:classic_registry.go:1524), which deleted only some of a large group's members, with a refusal. That is safer in every case, so it does not block. The remedy text (pass device IDs) belongs in the same follow-up as (6), which needs the same "too large, here is what to do" hint on the list read.

Review coverage and scope

• Correctness: (1) re-checked on the wire this round, see the finding
• Design: the resolver asymmetry was dismissed after it was compared with #408's rule
• Security: names never reach a URL now. Collision errors print IDs only. Unchanged since round 2
• Silent failure: the group-body cap replaces silent truncation, as main confirms
• Test coverage: carried from round 2, where five lane mutations on uncovered branches all survived
• Reliability: CI ci passes on 6f6f5c42
• Performance: carried, (6)
• Project rules compliance: .claude/rules/{classic-api,coding-style,credentials-and-auth}.md, generator/CLAUDE.md, read in round 2. The rules on main did not change between the rounds
• Cross-PR: #408 overlaps at the group-resolver rule, which agrees, and at name coercion, which #408 fixes and this PR does not. See (1)
• [na] Frontend, dependencies, migrations.

Head 6f6f5c42 is unchanged since round 2, so every determination is carried and none is re-graded. This round took the three §13 dispositions that came due and re-checked (1) on the wire. It ran no lanes: every eligible lane is already in ever, and the code did not change. Wire probe on pro-nmartin: one printer was created and deleted. Findings were self-scored, with no verifier pass.

Prior review status

#	Location	Rounds	State	Notes
R1 (1)	.github/workflows/ci.yaml:7	1	✅ Fixed	Fixed in round 2
(1)	generator/classic/generator.go:2130	2	🔴 Open	No change and no reply. Now wire-confirmed
(2)	generator/classic/generator.go:3127	3	💬 Dismissed	See Dispositions
(3)	generator/classic/generator.go:3075	3	💬 Retired	See Dispositions
(4)	CHANGELOG.md:37	3	💬 Retired	See Dispositions
(5)	classic_registry.go:616	2	🔴 Open	No change
(6)	classic_registry.go:566	2	🔴 Open	No change
(7)	generator/classic/generator.go:1514	2	🔴 Open	No change

_{🤖 Generated by the pr-review:review skill v1.39.0 · reviewed head 6f6f5c42}

[ktn-jamf]

Thanks, Neil. Round 3 is addressed.

• (1) Numeric and boolean names: fixed in 415542b. Classic list lookups now read <name> as text, so update --name, delete --name and apply find records named 2024, true and 1.50. Two records both named 2024 are refused as a collision. The XML walk is split out of classicFindIDsByName into classicFoldedNameMatches, and both callers use the same exact-case-first rule. The JSON fallback decodes into a typed id/name struct. extractClassicName had the same coercion problem, which blocked apply on these names, so it now reads the body's <name> as text too.
• (5) Collision refusal: fixed in d814106. The refusal exits 2 with a hint to run the same command with one of the IDs as <id> in place of --name. The chooser shows each candidate's name beside its ID.
• (7) Empty merge-put body: fixed in d814106. It has its own message now.
• (6) List size limit: left for a follow-up.

One existing test changed: TestGenerateRegistry_WithApplyHelpers checked that the registry contains xmlconv.ToMap and xmlconv.ExtractListItems, which are the coercing calls this fix removes. It now checks for classicFoldedNameMatches and classicNarrowToExactName instead.

Revert proof: the new classic_name_reads_as_text_test.go covers update, delete and apply on all three names, with XML and JSON lists, plus the collision. Run against the code without the fix, 15 of its 21 subtests failed, for example no policy found with name "2024". With the fix, all 21 pass.

c33809e merges the latest main.

[neilmartin83]

Tip

✅ Merge-ready. Two of the commits since round 3 are fixes. 415542b5 reads <name> as text, which fixes the blocking (1). d814106f fixes (5) and (7). c33809e1 merges main. I wire-checked (1) on pro-nmartin: update --name, apply and delete --name now all find records named 2024 and true.

Rating: 5/5

• The blocking finding is fixed and wire-verified. The one finding still open, (6) the unbounded list read, is retired below as the follow-up Keaton named.

Prior findings status

#	Location	Rounds	State	Notes
(1)	generator/classic/generator.go:2130	3	✅ Fixed	classicFoldedNameMatches walks the list with xml.Decoder. extractClassicName now reads the body's <name> as text, which was also blocking apply. The JSON fallback decodes into a typed id/name struct. Wire, pro-nmartin, see the table below. The 28 NumberOrBoolean/NumericNameShared/CollisionRefusal/ChooserShows subtests pass, and two records named 2024 are refused with exit 2
(5)	classic_registry.go:616	3	✅ Fixed	The refusal is exitcode.Wrap(Usage, …). Its hint is "run again with one of these IDs as in place of --name", with a rename-or-update hint for apply, and apply now passes its own verb. The chooser shows ID and Name. findClassicProfileByName still reaches *ClassicNameCollisionError through Unwrap
(7)	generator/classic/generator.go:1514	3	✅ Fixed	The empty merge-put body has its own message. A nil %w is gone
(6)	classic_registry.go:566	3	💬 Retired	See Dispositions

Wire check on pro-nmartin at 415542b5, with disposable printers 229 (2024) and 230 (true), both deleted afterwards (each GET now answers not_found):

Command	2024	true
update --name <n> --from-file …	updated id 229 (location changed)	updated id 230
apply --yes --from-file …	Replaced printer "2024" (id: 229)	Replaced printer "true" (id: 230)
delete --name <n> --yes	deleted	deleted

In round 3, the same update --name 2024 and delete --name 2024 answered no printer found with name "2024".

Dispositions

Disposition (round 4) — retired, follow-up. Was (6), internal/commands/pro/generated/classic_registry.go:566, open 3 rounds. update --name reads the whole collection with no stated bound and no remedy on a timeout. The author deferred it on the PR. It is a NICE-TO-HAVE, and it fits the follow-up round 3 already set out for retired (4), the remedy text on a list that is too large.

Review coverage and scope

• Correctness: the new XML walk (depth-2 items, depth-3 id/name, exact-case-first narrowing shared with classicFindIDsByName), extractClassicName's decoder (direct <name> before <general><name>, with or without the wrapper), and the typed JSON fallback, which now errors on a malformed list where it used to ignore it
• Tests: ./generator/..., ./internal/commands/pro/generated and ./internal/commands pass at 415542b5. make verify-generated reports "Generated code is up to date". CI ci passes. git merge-tree with current main is clean
• Changed existing test: TestGenerateRegistry_WithApplyHelpers now checks for the non-coercing helpers in place of xmlconv.ToMap/ExtractListItems. That is the right change, because those were the defect
• Generated-code boundary: changes come from generator/classic/generator.go and are regenerated
• Wire: pro-nmartin as above. The collision path was not wire-checked, because Classic printers refuse a duplicate name. It is covered by tests
• Project rules: root CLAUDE.md, the three .claude/rules/*.md and generator/CLAUDE.md, from the session load
• [na] Platform and School profiles: the delta does not touch either

Grading: two author commits since 6f6f5c42, plus a main merge. No lanes dispatched, since every eligible lane is already in ever. Head graded: 415542b5.

_{🤖 Generated by the pr-review:review skill v1.39.0 · reviewed head 415542b}