fix(pin-corpus): key retirement on site identity, not literal alone (#1006)

[prflow-implementer]

Closes #1006.

Summary

Pin retirement was keyed by literal while revival authorization is keyed by site, so a retained boundary pin sharing a literal with a redundant_retired twin at a different site inherited an unsatisfiable gate finding with no exit (surfaced by PR #1005).

Retirement now keys on site identity — (source_file, helper, literal, resolved_target), the fields RevivalAuthorization already carries. GuardSite gains a classifier-equivalent resolved_target_token (repo-relative path, /__pin_corpus_runtime__/<var> bundle placeholder, or None) so a live site's key matches the manifest's resolved_target cell; the frozen .devflow/ target spelling normalizes to .prflow/ for the comparison. The literal-scoped adjudication ledger lookups stay literal-keyed.

Why 4-tuple (target included)

An executed scan of the frozen manifests shows the 3-tuple (source_file, helper, literal) leaves 7 retire/retain collisions (6 assert_pin_unique pairs differing only by target, plus one bundle case); adding resolved_target leaves zero. This is what satisfies AC4 (the issue's AC1 shorthand omits target; AC4 requires it — noted as an issue-accuracy reflection).

Evidence (driven end-to-end, not code reading)

• Old literal-keying poisoned 62 live retained pins (including the exact PR PRFlow rename Tier 1: atomic directory/vendored-path/config-key/workflow-body migration + /prflow:init #1005 repro at run.sh:2673); site-keying poisons 0, while retirement stays effective (129 retired site keys).
• pin-corpus-lint.py mutation-routing-worktree . returns rc 0 over this branch.

Tests

• test_retirement_is_scoped_to_the_retired_site — AC2 (different-site not reported) + AC3 negative control (own-site still reported).
• test_conflicting_retain_and_retire_dispositions_resolve_per_site — AC4.
• test_resolved_target_token_matches_the_classifier_three_shapes — the file/bundle/None token shapes plus the .devflow/→.prflow/ normalization round-trip and the DEVFLOW-filename non-goal (added in the review-and-fix pass).
• test_malformed_retirement_manifest_site_fields_fail_closed — the new source_file/resolved_target JSON-parse fail-closed arms (added in the review-and-fix pass).
• Updated with rationale: test_authorized_retired_revival_cannot_launder_committed_prose_target, test_a_retired_literal_keeps_the_pre_948_revival_contract, test_convert_presence_is_not_a_retired_wording_disposition, the count-helper tests, and the shared fixture (site-aware).

Frozen manifests are read, never rewritten. No changeset: lib/test/ is stripped from the vendored plugin (internal test/CI infrastructure).

🤖 Generated with Claude Code

[The01Geek]

Requesting review. Implements #1006: pin retirement now keys on site identity (source_file, helper, literal, resolved_target) rather than on the literal alone, so a retained boundary pin sharing a literal with a redundant_retired twin at a different site no longer inherits an unsatisfiable gate finding with no exit.

This is a security-adjacent guard, so the question that matters is whether it narrowed correctly or simply went quiet. I verified the negative control exists and discriminates — a parameterized pair with opposite expectations:

("different-site", "lib/test/old.sh", 0)   # the fix: no longer poisoned
("own-site",       "lib/test/new.sh", 1)   # still reported: not a hole

Please pressure-test exactly that boundary:

• Can any input reach a state where a literal retired at its own site stops being reported? That is the failure this change could introduce, and it would be invisible in a green suite.
• The decision was made on measured evidence that the frozen manifests already record conflicting dispositions for the same literal at different homes (2 literals carry both RETAIN_BOUNDARY and RETIRE_PROSE). Confirm the new keying resolves each to the disposition recorded for its site, and that the byte-frozen manifests are read rather than rewritten.
• Re-adjudicating individual pins is explicitly out of scope; flag any change that amounts to one.

/prflow:review

[prflow-reviewer]

Verdict: APPROVE with notes (correct, well-scoped keying fix; one non-blocking test-hardening suggestion)

Issue Compliance

Reviewed against issue #1006: Pin retirement is keyed by literal while revival authorization is keyed by site — criteria from the issue body's Acceptance Criteria; scope unchanged. Requirement-based checklist items are included in the verification results below.

All acceptance criteria are satisfied:

• Site-keying implemented — _site_retirement_key(source_file, helper, literal, resolved_target) replaces _literal_adjudication_key for retirement, applied symmetrically on the manifest-read side (_strict_retirement_manifest_literals) and the live-site side (GuardSite.retirement_key()).
• Different-site pin no longer reported / own-site still reported — driven end-to-end by test_retirement_is_scoped_to_the_retired_site (0 vs 1 findings, with the own-site case as an explicit negative control).
• RETAIN/RETIRE per-site conflict resolved — test_conflicting_retain_and_retire_dispositions_resolve_per_site.
• Frozen manifests read, not rewritten — the .devflow/→.prflow/ skew is handled at key-build time by _normalize_retirement_target (applied to both comparison sides); manifest bytes untouched.
• Tests updated with rationale, not deleted — each changed expectation carries an accurate issue-#1006 rationale comment, verified against the real frozen manifest rows.

Verification Checklist Results

No FAIL, no INCONCLUSIVE. Engine-self-modifying diff → full checklist scope. CI ground truth for the reviewed HEAD (28a5c14) reports lib + python tests: success and all shards success; the four new/changed tests were additionally re-run in this review's environment and pass. AC7 (lib/test/run.sh zero failures/zero skips) is discharged by the green suite.

Code Review Findings

🟡 Suggestion / Minor

1. The classifier-equivalence coupled invariant has no binding test. test_resolved_target_token_matches_the_classifier_three_shapes asserts _resolved_target_token/_site_retirement_key against hardcoded literals ("docs/x.md", "/__pin_corpus_runtime__/CI_BUNDLE"), and test_pin_corpus_classifier.py independently pins the same sentinel to its own literals. No test imports both modules and asserts lint._resolved_target_token(x) == classifier._portable_target(x) / recover_override_names. A one-sided edit to the classifier's sentinel or its relativization would keep both scripts' own tests green while silently breaking every runtime-bundle/file retirement key match — the exact silent no-op the code's own coupled-invariant comment (pin-corpus-lint.py:1670-1673) warns about. Recommend one cross-module equality test (the harness already loads one module into the other's tests via importlib). (raised by 2/5 agents) — Non-blocking: the coupling is disclosed via a coupled-invariant comment per CLAUDE.md's discipline, which the repo accepts; a binding test would be strictly stronger.
2. Minor fail-closed coverage gap: test_malformed_retirement_manifest_site_fields_fail_closed exercises the source-side JSONDecodeError and the non-string source/target arms, but not an invalid-JSON resolved_target cell (the json.loads(row[target_index]) decode-error arm). Bundle-target retirement and the .devflow/→.prflow/ normalization are covered only at the unit-token level, not end-to-end through scan_changed_sources. Low risk — scan_changed_sources consumes retirement_key() uniformly. (raised by 1/5 agents)

ℹ️ Informational

1. _repo_relative_or_none docstring calls itself "the one copy of the abspath/commonpath/relpath computation the file otherwise repeats" while the same sentence points at a genuine second (raising) copy, _relative_target_path. Behavior-inert wording nit; suggest "the non-raising copy … _relative_target_path is the raising variant." (raised by 1/5 agents)

Verdict Criteria

• No verification checklist FAIL / INCONCLUSIVE.
• No self-contradicting-diff finding — the comment/docstring truthfulness sweep confirmed every load-bearing claim accurate against HEAD and the real manifests.
• No review-agent finding at or above the critical verdict threshold; only Suggestion/Informational findings present → APPROVE with notes.

over-grade annotation: no finding flagged
truthfulness sweep: no finding promoted
intra-diff contradiction scan: no contradiction found

---

Scope verified beyond code-reading: the change is a strict narrowing of retirement identity (site ⊂ literal), so it cannot make retirement over-broad; the documented out-of-repo/unresolvable→None direction is fail-toward-not-matched (routes through ordinary policy, still reports an undeclared pin), and the _normalized_revival_authorization extraction of _repo_relative_or_none was verified behavior-preserving.

[The01Geek]

Review verdict on 28a5c142: APPROVE with notes, 🟡 only. Head is now 90e1d54d (branch update absorbing origin/main), CI 7/7 green on it.

Beyond the review, I verified the property that distinguishes this fix from a hole — the negative control discriminates rather than going quiet:

("different-site", "lib/test/old.sh", 0)   # the fix: no longer poisoned
("own-site",       "lib/test/new.sh", 1)   # still reported: not a hole

Opposite expectations on the same literal, both driven. A gate that simply stopped firing would fail the own-site arm.

The two 🟡 suggestions are about future drift rather than current correctness — the classifier-equivalence invariant is asserted against hardcoded literals rather than bound to the classifier, and the invalid-JSON resolved_target arm is uncovered. Captured as #1057 rather than dropped; the first matters because a silent divergence there would reopen exactly the mismatch this PR closed.

Merging.