fix: tighten canonical review integrity follow-up

[fujiwaranosai850]

Summary

• fail closed when canonical PR review context re-resolves without a canonical URL
• always leave a visible blocked hold comment before demoting review heartbeat issues into Refining
• preserve and extend canonical PR routing regression coverage for empty feedback, missing URL-scoped diff context, and review-heartbeat integrity holds

Validation

• TAP version 13

Subtest: formatPrFeedback

# Subtest: preserves canonical PR context even when no comments were retrieved
ok 1 - preserves canonical PR context even when no comments were retrieved
  ---
  duration_ms: 1.095685
  type: 'test'
  ...
# Subtest: includes branch name in conflict resolution instructions
ok 2 - includes branch name in conflict resolution instructions
  ---
  duration_ms: 0.187851
  type: 'test'
  ...
# Subtest: uses fallback branch name when not provided
ok 3 - uses fallback branch name when not provided
  ---
  duration_ms: 0.191788
  type: 'test'
  ...
# Subtest: includes step-by-step instructions for conflict resolution
ok 4 - includes step-by-step instructions for conflict resolution
  ---
  duration_ms: 0.165949
  type: 'test'
  ...
# Subtest: correctly formats changes_requested feedback
ok 5 - correctly formats changes_requested feedback
  ---
  duration_ms: 0.160639
  type: 'test'
  ...
# Subtest: includes comment location information when available
ok 6 - includes comment location information when available
  ---
  duration_ms: 0.153106
  type: 'test'
  ...
# Subtest: uses correct base branch in rebase command
ok 7 - uses correct base branch in rebase command
  ---
  duration_ms: 0.153156
  type: 'test'
  ...
1..7

ok 1 - formatPrFeedback

duration_ms: 3.265071
type: 'suite'
...

Subtest: canonical PR dispatch routing

# Subtest: fails closed when canonical status lookup no longer resolves
ok 1 - fails closed when canonical status lookup no longer resolves
  ---
  duration_ms: 16.781159
  type: 'test'
  ...
# Subtest: returns an explicitly canonical PR context when authoritative diff loads
ok 2 - returns an explicitly canonical PR context when authoritative diff loads
  ---
  duration_ms: 9.416284
  type: 'test'
  ...
# Subtest: fails closed when canonical diff lookup cannot load URL-scoped context
ok 3 - fails closed when canonical diff lookup cannot load URL-scoped context
  ---
  duration_ms: 7.614505
  type: 'test'
  ...
# Subtest: returns authoritative canonical PR context only with a loaded diff
ok 4 - returns authoritative canonical PR context only with a loaded diff
  ---
  duration_ms: 7.84019
  type: 'test'
  ...
# Subtest: preserves canonical feedback routing even when comments are empty
ok 5 - preserves canonical feedback routing even when comments are empty
  ---
  duration_ms: 8.597273
  type: 'test'
  ...
# Subtest: keeps legacy issue-scoped review context non-canonical
ok 6 - keeps legacy issue-scoped review context non-canonical
  ---
  duration_ms: 0.181739
  type: 'test'
  ...
1..6

ok 2 - canonical PR dispatch routing

duration_ms: 50.952404
type: 'suite'
...

Subtest: GitHubProvider.getPrStatus — closed PR handling

# Subtest: returns url:null when no PR has ever been created
ok 1 - returns url:null when no PR has ever been created
  ---
  duration_ms: 0.80912
  type: 'test'
  ...
# Subtest: returns url:null when timeline returns empty array (no PRs at all)
ok 2 - returns url:null when timeline returns empty array (no PRs at all)
  ---
  duration_ms: 0.175488
  type: 'test'
  ...
# Subtest: returns url:closedPrUrl when a closed-without-merge PR exists
ok 3 - returns url:closedPrUrl when a closed-without-merge PR exists
  ---
  duration_ms: 0.179194
  type: 'test'
  ...
# Subtest: prefers open PR over closed PR
ok 4 - prefers open PR over closed PR
  ---
  duration_ms: 0.250437
  type: 'test'
  ...
# Subtest: prefers merged PR over closed PR
ok 5 - prefers merged PR over closed PR
  ---
  duration_ms: 0.242041
  type: 'test'
  ...
# Subtest: ignores non-CLOSED states in timeline when returning closed PR
ok 6 - ignores non-CLOSED states in timeline when returning closed PR
  ---
  duration_ms: 0.213679
  type: 'test'
  ...
# Subtest: detects merge conflicts via mergeable field
ok 7 - detects merge conflicts via mergeable field
  ---
  duration_ms: 0.141925
  type: 'test'
  ...
# Subtest: distinguishes mergeable states
ok 8 - distinguishes mergeable states
  ---
  duration_ms: 0.159136
  type: 'test'
  ...
# Subtest: handles unknown mergeable state
ok 9 - handles unknown mergeable state
  ---
  duration_ms: 0.15019
  type: 'test'
  ...
1..9

ok 3 - GitHubProvider.getPrStatus — closed PR handling

duration_ms: 3.837196
type: 'suite'
...

Subtest: GitHubProvider.getPrStatusByUrl

# Subtest: preserves comment-only feedback semantics for canonical PR routing
ok 1 - preserves comment-only feedback semantics for canonical PR routing
  ---
  duration_ms: 0.21412
  type: 'test'
  ...
# Subtest: preserves changes-requested fallback when reviewDecision is empty
ok 2 - preserves changes-requested fallback when reviewDecision is empty
  ---
  duration_ms: 0.0856
  type: 'test'
  ...
1..2

ok 4 - GitHubProvider.getPrStatusByUrl

duration_ms: 0.37531
type: 'suite'
...

Subtest: GitHubProvider canonical URL helpers

# Subtest: throws when diff lookup fails after PR identity resolves
ok 1 - throws when diff lookup fails after PR identity resolves
  ---
  duration_ms: 0.27287
  type: 'test'
  ...
# Subtest: throws when review comment retrieval fails after PR identity resolves
ok 2 - throws when review comment retrieval fails after PR identity resolves
  ---
  duration_ms: 0.215682
  type: 'test'
  ...
1..2

ok 5 - GitHubProvider canonical URL helpers

duration_ms: 0.547151
type: 'suite'
...

Subtest: GitLabProvider.getPrStatus — closed MR handling

# Subtest: returns url:null when no MR has ever been created
ok 1 - returns url:null when no MR has ever been created
  ---
  duration_ms: 0.169045
  type: 'test'
  ...
# Subtest: returns url:closedMrUrl when a closed-without-merge MR exists
ok 2 - returns url:closedMrUrl when a closed-without-merge MR exists
  ---
  duration_ms: 0.079388
  type: 'test'
  ...
# Subtest: prefers open MR over closed MR
ok 3 - prefers open MR over closed MR
  ---
  duration_ms: 0.123671
  type: 'test'
  ...
# Subtest: prefers merged MR over closed MR
ok 4 - prefers merged MR over closed MR
  ---
  duration_ms: 0.05346
  type: 'test'
  ...
# Subtest: handles multiple closed MRs — returns the first found
ok 5 - handles multiple closed MRs — returns the first found
  ---
  duration_ms: 0.047108
  type: 'test'
  ...
1..5

ok 6 - GitLabProvider.getPrStatus — closed MR handling

duration_ms: 0.551089
type: 'suite'
...

Subtest: GitLabProvider.getPrStatusByUrl

# Subtest: preserves comment-driven feedback semantics for canonical MR routing
ok 1 - preserves comment-driven feedback semantics for canonical MR routing
  ---
  duration_ms: 0.146323
  type: 'test'
  ...
# Subtest: preserves unresolved-discussion changes-requested semantics
ok 2 - preserves unresolved-discussion changes-requested semantics
  ---
  duration_ms: 0.090158
  type: 'test'
  ...
1..2

ok 7 - GitLabProvider.getPrStatusByUrl

duration_ms: 0.334384
type: 'suite'
...

Subtest: GitLabProvider.getPrReviewCommentsByUrl

# Subtest: reuses canonical MR comment retrieval semantics
ok 1 - reuses canonical MR comment retrieval semantics
  ---
  duration_ms: 0.688654
  type: 'test'
  ...
# Subtest: throws when canonical MR comment retrieval fails after identity resolution
ok 2 - throws when canonical MR comment retrieval fails after identity resolution
  ---
  duration_ms: 0.126005
  type: 'test'
  ...
1..2

ok 8 - GitLabProvider.getPrReviewCommentsByUrl

duration_ms: 0.883428
type: 'suite'
...

Subtest: GitLabProvider canonical URL helpers

# Subtest: throws when diff lookup fails after MR identity resolves
ok 1 - throws when diff lookup fails after MR identity resolves
  ---
  duration_ms: 0.229704
  type: 'test'
  ...
1..1

ok 9 - GitLabProvider canonical URL helpers

duration_ms: 0.289315
type: 'suite'
...

Subtest: canonical-pr ledger

# Subtest: records superseded canonical PRs when a replacement is recorded
ok 1 - records superseded canonical PRs when a replacement is recorded
  ---
  duration_ms: 19.707701
  type: 'test'
  ...
# Subtest: refreshes canonical PR status without clobbering supersession history
ok 2 - refreshes canonical PR status without clobbering supersession history
  ---
  duration_ms: 12.025656
  type: 'test'
  ...
# Subtest: serializes concurrent updates so replacement and refresh do not lose data
ok 3 - serializes concurrent updates so replacement and refresh do not lose data
  ---
  duration_ms: 59.327477
  type: 'test'
  ...
# Subtest: fails closed when stored canonical PR is no longer linked to the issue
ok 4 - fails closed when stored canonical PR is no longer linked to the issue
  ---
  duration_ms: 5.179312
  type: 'test'
  ...
1..4

ok 10 - canonical-pr ledger

duration_ms: 97.183924
type: 'suite'
...

Subtest: work_finish: PR validation and conflict resolution

# Subtest: isConflictResolutionCycle
    # Subtest: should detect merge_conflict transition in audit log
    ok 1 - should detect merge_conflict transition in audit log
      ---
      duration_ms: 4.003949
      type: 'test'
      ...
    # Subtest: should return false when no merge_conflict transition exists
    ok 2 - should return false when no merge_conflict transition exists
      ---
      duration_ms: 3.899003
      type: 'test'
      ...
    # Subtest: should handle missing audit log gracefully
    ok 3 - should handle missing audit log gracefully
      ---
      duration_ms: 0.80969
      type: 'test'
      ...
    # Subtest: should skip malformed JSON lines in audit log
    ok 4 - should skip malformed JSON lines in audit log
      ---
      duration_ms: 1.467137
      type: 'test'
      ...
    1..4
ok 1 - isConflictResolutionCycle
  ---
  duration_ms: 10.659716
  type: 'suite'
  ...
# Subtest: validatePrExistsForDeveloper: conflict detection
    # Subtest: rejects completion when current branch does not match canonical PR branch
    ok 1 - rejects completion when current branch does not match canonical PR branch
      ---
      duration_ms: 4.641895
      type: 'test'
      ...
    # Subtest: should validate error message format when PR still conflicting
    ok 2 - should validate error message format when PR still conflicting
      ---
      duration_ms: 0.14488
      type: 'test'
      ...
    # Subtest: should include branch name in error message
    ok 3 - should include branch name in error message
      ---
      duration_ms: 0.159647
      type: 'test'
      ...
    1..3
ok 2 - validatePrExistsForDeveloper: conflict detection
  ---
  duration_ms: 5.259296
  type: 'suite'
  ...
# Subtest: catch block precedence
    # Subtest: should correctly check for validation error type
    ok 1 - should correctly check for validation error type
      ---
      duration_ms: 0.115325
      type: 'test'
      ...
    # Subtest: should handle non-Error exceptions gracefully
    ok 2 - should handle non-Error exceptions gracefully
      ---
      duration_ms: 0.113702
      type: 'test'
      ...
    1..2
ok 3 - catch block precedence
  ---
  duration_ms: 0.341808
  type: 'suite'
  ...
# Subtest: audit logging
    # Subtest: should log rejection with correct fields
    ok 1 - should log rejection with correct fields
      ---
      duration_ms: 0.121116
      type: 'test'
      ...
    # Subtest: should log successful conflict resolution with correct fields
    ok 2 - should log successful conflict resolution with correct fields
      ---
      duration_ms: 0.038913
      type: 'test'
      ...
    1..2
ok 4 - audit logging
  ---
  duration_ms: 0.210643
  type: 'suite'
  ...
1..4

ok 11 - work_finish: PR validation and conflict resolution

duration_ms: 23.772175
type: 'suite'
...
1..11

tests 51

suites 15

pass 51

fail 0

cancelled 0

skipped 0

todo 0

duration_ms 315.284233

• TAP version 13

Subtest: E2E pipeline

# Subtest: reviewPass
    # Subtest: should move review issues to Refining when canonical PR re-resolution fails
    ok 1 - should move review issues to Refining when canonical PR re-resolution fails
      ---
      duration_ms: 14.029671
      type: 'test'
      ...
    1..1
ok 1 - reviewPass
  ---
  duration_ms: 14.755475
  type: 'suite'
  ...
# Subtest: projectTick — reviewPolicy gating
    # Subtest: reviewPolicy: agent should dispatch reviewer
    ok 1 - reviewPolicy: agent should dispatch reviewer
      ---
      duration_ms: 64.382376
      type: 'test'
      ...
    1..1
ok 2 - projectTick — reviewPolicy gating
  ---
  duration_ms: 64.575406
  type: 'suite'
  ...
# Subtest: role:level labels
    # Subtest: dispatch should replace old role:level label
    ok 1 - dispatch should replace old role:level label
      ---
      duration_ms: 26.879006
      type: 'test'
      ...
    # Subtest: projectTick should dispatch reviewer when review:agent label present
    ok 2 - projectTick should dispatch reviewer when review:agent label present
      ---
      duration_ms: 37.16028
      type: 'test'
      ...
    1..2
ok 3 - role:level labels
  ---
  duration_ms: 64.550639
  type: 'suite'
  ...
1..3

ok 1 - E2E pipeline

duration_ms: 144.698545
type: 'suite'
...
1..1

tests 4

suites 4

pass 4

fail 0

cancelled 0

skipped 0

todo 0

duration_ms 381.122101

Addresses issue #244