fix(pr-review): preserve addressed findings

.github/actions/claude-pr-review/action.yml

@@ -81,6 +81,7 @@ runs:
           --event-head "${{ github.event.pull_request.head.sha }}" \
           --event-base "${{ github.event.pull_request.base.sha }}" \
           --state-dir "$STATE_DIR" \
+          --resolve-threads "${{ inputs.github_identity_token != '' }}" \
           --review-depth "$REVIEW_DEPTH" \
           --premortem "$PREMORTEM"
 
@@ -175,9 +176,10 @@ runs:
           1. Respect the frozen mode and SHA scope in review-input.json. For an
              incremental review, discover new findings only in review.diff. Use the
              current files and full.diff to decide whether prior findings were fixed.
-          2. Examine every open manifest finding and return exactly one prior_findings
-             disposition (`open` or `resolved`) for it. Never resolve human-authored
-             threads.
+          2. Examine every open manifest finding. Return a prior_findings disposition
+             (`open` or `resolved`) for each finding you assessed. An omitted finding
+             remains open; only an explicit `resolved` disposition closes one. Never
+             resolve human-authored threads.
           3. Put only high-confidence, actionable defects in findings. Severity and
              confidence are separate: uncertain concerns belong in open_questions
              with medium or low confidence, phrased as concrete verification requests.

.github/actions/claude-pr-review/review_pipeline.py

@@ -585,6 +585,7 @@ def import_legacy_findings(
         manifest["findings"][legacy_id] = {
             "fingerprint": None,
             "status": "open",
+            "thread_resolution": "unresolved",
             "severity": "major",
             "confidence": "high",
             "title": title,
@@ -616,9 +617,23 @@ def sync_manifest_threads(
         resolved = thread_status.get(str(finding["thread_id"]))
         if resolved is True:
             finding["status"] = "resolved"
+            finding["thread_resolution"] = "confirmed"
         elif resolved is False:
-            finding["status"] = "open"
-            finding["resolved_sha"] = None
+            if (
+                finding.get("status") == "resolved"
+                and finding.get("thread_resolution") == "confirmed"
+            ):
+                finding["status"] = "open"
+                finding["resolved_sha"] = None
+                finding["thread_resolution"] = "unresolved"
+            elif finding.get("status") == "resolved":
+                # An unresolved GitHub thread is expected when publication used
+                # the job token and deliberately skipped thread resolution.
+                # Preserve the code-level disposition until a capable identity
+                # can resolve the thread.
+                finding["thread_resolution"] = "skipped"
+            else:
+                finding["thread_resolution"] = "unresolved"
 
 
 def write_github_output(values: dict[str, Any]) -> None:
@@ -853,6 +868,7 @@ def prepare(args: argparse.Namespace) -> None:
         "commentable_lines": commentable_lines,
         "sticky_comment_id": sticky_comment_id,
         "publisher_login": publisher_login,
+        "thread_resolution_enabled": args.resolve_threads == "true",
         "pipeline_version": pipeline_version,
         "rubric_version": rubric_version,
     }
@@ -974,12 +990,11 @@ def compile_review(
     ]
     if len(returned_prior_ids) != len(set(returned_prior_ids)):
         raise PipelineError("prior_findings contains duplicate finding IDs")
-    if set(returned_prior_ids) != expected_prior_ids:
-        missing = sorted(expected_prior_ids - set(returned_prior_ids))
-        unknown = sorted(set(returned_prior_ids) - expected_prior_ids)
+    unknown = sorted(set(returned_prior_ids) - expected_prior_ids)
+    if unknown:
         raise PipelineError(
-            "prior_findings must disposition every open finding exactly once "
-            f"(missing={missing}, unknown={unknown})"
+            "prior_findings references findings that are not open "
+            f"(unknown={unknown})"
         )
 
     scope_summary = public_text(
@@ -992,6 +1007,9 @@ def compile_review(
         )
 
     resolution_ids: list[str] = []
+    resolution_enabled = bool(
+        review_input.get("thread_resolution_enabled", False)
+    )
     for disposition in model_output["prior_findings"]:
         if not isinstance(disposition, dict):
             raise PipelineError("prior_findings contains a non-object")
@@ -1014,7 +1032,23 @@ def compile_review(
                 )
             item["resolved_sha"] = head
             if item.get("thread_id"):
-                resolution_ids.append(item["thread_id"])
+                item["thread_resolution"] = (
+                    "pending" if resolution_enabled else "skipped"
+                )
+        elif item.get("thread_id"):
+            item["thread_resolution"] = "unresolved"
+
+    if resolution_enabled:
+        resolution_ids = sorted(
+            {
+                str(item["thread_id"])
+                for item in manifest["findings"].values()
+                if isinstance(item, dict)
+                and item.get("status") == "resolved"
+                and item.get("thread_id")
+                and item.get("thread_resolution") != "confirmed"
+            }
+        )
 
     changed_paths = set(scope["full_pr_paths"])
     commentable = {
@@ -1119,6 +1153,7 @@ def compile_review(
         manifest["findings"][item_id] = {
             "fingerprint": item_id,
             "status": "open",
+            "thread_resolution": "unresolved",
             "severity": finding["severity"],
             "confidence": finding["confidence"],
             "title": finding["title"],
@@ -1768,13 +1803,19 @@ def require_frozen_pull() -> None:
             payload,
             before_write=require_frozen_pull,
         )
-        resolve_threads(
+        resolved_threads = resolve_threads(
             payload["resolve_thread_ids"],
             enabled=os.environ.get("GH_RESOLVE_THREADS", "").lower() == "true",
             before_write=require_frozen_pull,
         )
 
         manifest = payload["manifest"]
+        for finding in manifest["findings"].values():
+            if (
+                isinstance(finding, dict)
+                and finding.get("thread_id") in resolved_threads
+            ):
+                finding["thread_resolution"] = "confirmed"
         for finding_id_value, posted in posted_comments.items():
             finding = manifest["findings"].get(finding_id_value)
             if isinstance(finding, dict):
@@ -1848,6 +1889,11 @@ def parser() -> argparse.ArgumentParser:
     prepare_parser.add_argument("--event-head")
     prepare_parser.add_argument("--event-base")
     prepare_parser.add_argument("--state-dir", required=True)
+    prepare_parser.add_argument(
+        "--resolve-threads",
+        choices=("true", "false"),
+        default="false",
+    )
     prepare_parser.add_argument(
         "--review-depth",
         choices=("standard", "deep"),

.github/actions/claude-pr-review/test_review_pipeline.py

@@ -42,6 +42,7 @@ def review_input(*, mode: str = "full"):
         "commentable_lines": {"src/example.py": [10, 11, 12]},
         "sticky_comment_id": None,
         "publisher_login": "github-actions[bot]",
+        "thread_resolution_enabled": False,
         "pipeline_version": "sha256:pipeline",
         "rubric_version": "sha256:rubric",
     }
@@ -98,8 +99,29 @@ def test_action_exposes_optional_github_identity_token(self):
             "${{ inputs.github_identity_token != '' }}",
             action,
         )
+        self.assertIn(
+            '--resolve-threads "${{ inputs.github_identity_token != \'\' }}"',
+            action,
+        )
         self.assertNotIn("GH_TOKEN: ${{ steps.review", action)
 
+    def test_workflow_loads_actions_from_trusted_main(self):
+        workflow = (
+            Path(pipeline.__file__).parents[2] / "workflows" / "claude.yml"
+        ).read_text(encoding="utf-8")
+
+        self.assertIn(
+            "uses: megaeth-labs/documentation/"
+            ".github/actions/claude-pr-review@main",
+            workflow,
+        )
+        self.assertIn(
+            "uses: megaeth-labs/documentation/"
+            ".github/actions/claude-interactive@main",
+            workflow,
+        )
+        self.assertNotIn("uses: ./.github/actions/claude-", workflow)
+
     def test_output_schema_uses_action_compatible_dialect(self):
         schema_path = Path(pipeline.__file__).with_name(
             "review-output.schema.json"
@@ -419,19 +441,63 @@ def test_configured_identity_thread_links_from_stateful_review(self):
         self.assertEqual(finding["thread_id"], "THREAD")
         self.assertEqual(finding["comment_id"], 99)
 
-    def test_unresolved_github_thread_reopens_manifest_finding(self):
+    def test_manually_reopened_github_thread_reopens_manifest_finding(self):
         manifest = review_input()["manifest"]
         manifest["findings"]["F-1"] = {
             "status": "resolved",
             "resolved_sha": "old",
             "thread_id": "THREAD",
+            "thread_resolution": "confirmed",
         }
         pipeline.sync_manifest_threads(
             manifest,
             [{"id": "THREAD", "isResolved": False}],
         )
         self.assertEqual(manifest["findings"]["F-1"]["status"], "open")
         self.assertIsNone(manifest["findings"]["F-1"]["resolved_sha"])
+        self.assertEqual(
+            manifest["findings"]["F-1"]["thread_resolution"],
+            "unresolved",
+        )
+
+    def test_skipped_github_resolution_does_not_reopen_finding(self):
+        manifest = review_input()["manifest"]
+        manifest["findings"]["F-1"] = {
+            "status": "resolved",
+            "resolved_sha": "old",
+            "thread_id": "THREAD",
+            "thread_resolution": "skipped",
+        }
+
+        pipeline.sync_manifest_threads(
+            manifest,
+            [{"id": "THREAD", "isResolved": False}],
+        )
+
+        self.assertEqual(manifest["findings"]["F-1"]["status"], "resolved")
+        self.assertEqual(
+            manifest["findings"]["F-1"]["thread_resolution"],
+            "skipped",
+        )
+
+    def test_legacy_resolved_finding_migrates_to_skipped_resolution(self):
+        manifest = review_input()["manifest"]
+        manifest["findings"]["F-1"] = {
+            "status": "resolved",
+            "resolved_sha": "old",
+            "thread_id": "THREAD",
+        }
+
+        pipeline.sync_manifest_threads(
+            manifest,
+            [{"id": "THREAD", "isResolved": False}],
+        )
+
+        self.assertEqual(manifest["findings"]["F-1"]["status"], "resolved")
+        self.assertEqual(
+            manifest["findings"]["F-1"]["thread_resolution"],
+            "skipped",
+        )
 
     def test_clean_incremental_review_is_compact(self):
         payload = pipeline.compile_review(
@@ -476,18 +542,36 @@ def test_uncertain_item_becomes_open_question(self):
             payload["review_body"],
         )
 
-    def test_every_open_prior_finding_requires_a_disposition(self):
+    def test_omitted_prior_finding_remains_open(self):
         value = review_input()
         value["manifest"]["findings"]["F-existing"] = {
             "status": "open",
             "severity": "major",
             "thread_id": "THREAD",
         }
+        payload = pipeline.compile_review(value, clean_output())
+
+        self.assertEqual(
+            payload["manifest"]["findings"]["F-existing"]["status"],
+            "open",
+        )
+        self.assertEqual(payload["resolve_thread_ids"], [])
+
+    def test_unknown_prior_finding_still_fails(self):
+        output = clean_output()
+        output["prior_findings"] = [
+            {
+                "finding_id": "F-unknown",
+                "disposition": "open",
+                "reason": "The issue remains.",
+            }
+        ]
+
         with self.assertRaisesRegex(
             pipeline.PipelineError,
-            "disposition every open finding",
+            "not open",
         ):
-            pipeline.compile_review(value, clean_output())
+            pipeline.compile_review(review_input(), output)
 
     def test_skip_with_open_finding_preserves_manifest(self):
         value = review_input(mode="skip")
@@ -520,6 +604,7 @@ def test_invalid_finding_path_fails_loudly(self):
 
     def test_resolved_prior_finding_produces_thread_resolution(self):
         value = review_input()
+        value["thread_resolution_enabled"] = True
         value["manifest"]["findings"]["F-existing"] = {
             "status": "open",
             "severity": "major",
@@ -539,6 +624,52 @@ def test_resolved_prior_finding_produces_thread_resolution(self):
             payload["manifest"]["findings"]["F-existing"]["status"],
             "resolved",
         )
+        self.assertEqual(
+            payload["manifest"]["findings"]["F-existing"][
+                "thread_resolution"
+            ],
+            "pending",
+        )
+
+    def test_resolved_prior_finding_skips_thread_without_identity(self):
+        value = review_input()
+        value["manifest"]["findings"]["F-existing"] = {
+            "status": "open",
+            "severity": "major",
+            "thread_id": "THREAD",
+        }
+        output = clean_output()
+        output["prior_findings"] = [
+            {
+                "finding_id": "F-existing",
+                "disposition": "resolved",
+                "reason": "The retry now clears partial state.",
+            }
+        ]
+
+        payload = pipeline.compile_review(value, output)
+
+        self.assertEqual(payload["resolve_thread_ids"], [])
+        self.assertEqual(
+            payload["manifest"]["findings"]["F-existing"][
+                "thread_resolution"
+            ],
+            "skipped",
+        )
+
+    def test_capable_identity_resolves_skipped_thread_without_reopening(self):
+        value = review_input()
+        value["thread_resolution_enabled"] = True
+        value["manifest"]["findings"]["F-existing"] = {
+            "status": "resolved",
+            "severity": "major",
+            "thread_id": "THREAD",
+            "thread_resolution": "skipped",
+        }
+
+        payload = pipeline.compile_review(value, clean_output())
+
+        self.assertEqual(payload["resolve_thread_ids"], ["THREAD"])
 
     def test_inline_finding_cannot_resolve_without_thread_id(self):
         value = review_input()
@@ -586,6 +717,7 @@ def test_existing_open_finding_is_not_reposted(self):
 
     def test_resolved_prior_does_not_suppress_distinct_same_symbol_finding(self):
         value = review_input()
+        value["thread_resolution_enabled"] = True
         value["manifest"]["findings"]["F-existing"] = {
             "status": "open",
             "severity": "major",

.github/workflows/claude.yml

@@ -40,7 +40,7 @@ jobs:
         with:
           fetch-depth: 1
 
-      - uses: ./.github/actions/claude-interactive
+      - uses: megaeth-labs/documentation/.github/actions/claude-interactive@main
         with:
           claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
           allowed_bots: "mega-putin"
@@ -88,7 +88,7 @@ jobs:
             echo "skip=false" >> "$GITHUB_OUTPUT"
           fi
 
-      - uses: ./.github/actions/claude-pr-review
+      - uses: megaeth-labs/documentation/.github/actions/claude-pr-review@main
         if: steps.workflow-change.outputs.skip != 'true'
         with:
           claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}