Alexanderamiri · Oct 24, 2026 · Oct 24, 2026
@@ -19,7 +19,8 @@ Monday 08:00   ──► securityhub-summary Lambda ──► #javabin-infra-ale
 - Uploads `tfplan` + `plan-output.txt` to S3 with SHA256 hash
 - Posts plan output as PR comment on pull requests
 - Runs `scripts/review-plan.py` inline — calls Bedrock (Claude Haiku 5.5) for risk analysis
-- The model gets the whole plan with refresh/read lines stripped (up to 400k chars, ~100k tokens). A larger plan gets a list of every resource change plus as much diff as fits, cut at a resource boundary, and the prompt says exactly which diffs are missing. The PR comment's 60k cut does not affect the review.
+- The model gets the whole plan with refresh/read lines stripped; there is no input cap (Haiku 5.5 has a 1M-token context). The prompt says the plan is complete, gives its change count and `Plan:` line, and explains Terraform's own elision markers (`# (N unchanged attributes hidden)`, `(sensitive value)`, `(known after apply)`, partial `jsonencode` diffs) so the model does not report them as truncation. `REVIEW_MAX_PLAN_CHARS` can optionally set a cap: a plan above it gets a list of every resource change plus as much diff as fits, cut at a resource boundary, and the prompt says exactly which diffs are missing. A response with `stopReason: max_tokens` is discarded (FAILED / retried) rather than trusted.
+- The PR comment shows at most 60k chars of the plan (GitHub's comment limit) and says so when it cuts; this does not affect the review.
 - Structured output via Bedrock Converse tool use: `{risk: LOW|MEDIUM|HIGH, summary: ..., findings: [...]}`
 - Posts review as PR comment
 - HIGH risk on main → sends Slack notification

@@ -24,12 +24,22 @@ else
 fi
 
 # --- Filter plan: strip refresh/read noise, keep only the meaningful diff ---
+# GitHub caps a comment at 65,536 characters, so the plan shown here is cut at
+# PLAN_COMMENT_CHARS. Say so when it is: readers (and anyone pasting the
+# comment into a model) must not mistake the cut for the whole plan. The LLM
+# review reads plan-output.txt itself and is not affected by this cut.
+PLAN_COMMENT_CHARS="${PLAN_COMMENT_CHARS:-60000}"
 PLAN_CLEAN_FILE="$(dirname "$PLAN_FILE")/plan-clean.txt"
-if [ -f "$PLAN_CLEAN_FILE" ]; then
-  PLAN=$(head -c 60000 "$PLAN_CLEAN_FILE")
-else
+if [ ! -f "$PLAN_CLEAN_FILE" ]; then
   # Fallback: filter inline
-  PLAN=$(grep -v -E '^(module\.|data\.|\s+#).*: (Refreshing state|Reading|Read complete)' "$PLAN_FILE" 2>/dev/null | head -c 60000)
+  PLAN_CLEAN_FILE=$(mktemp)
+  grep -v -E '^(module\.|data\.|\s+#).*: (Refreshing state|Reading|Read complete)' "$PLAN_FILE" > "$PLAN_CLEAN_FILE" 2>/dev/null || true
+fi
+PLAN=$(head -c "$PLAN_COMMENT_CHARS" "$PLAN_CLEAN_FILE")
+PLAN_SIZE=$(wc -c < "$PLAN_CLEAN_FILE" | tr -d ' ')
+CUT_NOTE=""
+if [ "$PLAN_SIZE" -gt "$PLAN_COMMENT_CHARS" ]; then
+  CUT_NOTE="> :scissors: Plan output above is cut at ${PLAN_COMMENT_CHARS} of ${PLAN_SIZE} characters (GitHub comment size limit). The full plan is in the workflow log; the LLM review below is not affected by this cut."
 fi
 
 # --- Build comment ---
@@ -44,6 +54,8 @@ ${STATUS}
 ${PLAN}
 \`\`\`
 
+${CUT_NOTE}
+
 </details>
 ENDOFCOMMENT
 

@@ -21,11 +21,14 @@
 MODEL_ID = "eu.anthropic.claude-haiku-5-5"
 REGION = os.environ.get("AWS_REGION", "eu-central-1")
 
-# Plan characters sent to the model. Haiku 5.5 has a 1M-token context; 400k
-# characters is roughly 100-130k tokens, so a whole plan fits with plenty of
-# room. Plans larger than this get a complete change list plus as much of the
-# diff as fits (see build_plan_section) - never a silent cut.
-MAX_PLAN_CHARS = int(os.environ.get("REVIEW_MAX_PLAN_CHARS", "400000"))
+# No input cap by default: the whole plan goes to the model (Haiku 5.5 has a
+# 1M-token context; plan text is roughly 3-4 characters per token). A fixed
+# character cap is what kept cutting real plans - first 80k, then 400k cut a
+# 231-destroy plan (830k chars; destroy diffs print every attribute) to 28 of
+# its changes. REVIEW_MAX_PLAN_CHARS can still set a cap; a plan above it gets
+# a complete change list plus as much diff as fits (see build_plan_section),
+# never a silent cut.
+MAX_PLAN_CHARS = int(os.environ["REVIEW_MAX_PLAN_CHARS"]) if os.environ.get("REVIEW_MAX_PLAN_CHARS") else None
 
 # State refresh / data source read lines: noise for review purposes. Resource
 # addresses start in column 0; diff content is always indented.
@@ -166,8 +169,16 @@ def build_plan_section(plan_text, limit=MAX_PLAN_CHARS):
     otherwise a complete change list, the diff cut at a resource boundary,
     and an exact statement of what was left out."""
     clean = strip_refresh_noise(plan_text)
-    if len(clean) <= limit:
-        return f"Terraform plan output (complete):\n```\n{clean}\n```"
+    if limit is None or len(clean) <= limit:
+        changes = resource_changes(clean)
+        totals = [line.strip() for line in clean.splitlines() if line.startswith("Plan:")]
+        return ("COMPLETE PLAN: the output below is the entire `terraform plan` output, with only "
+                "state-refresh / data-source read lines removed. Nothing was cut: it contains all "
+                f"{len(changes)} resource change headers"
+                + (f" and Terraform's own summary line ({totals[0]})" if totals else "")
+                + ". Review every change in it, and do not say the plan is truncated, partial or "
+                "incomplete.\n\n"
+                f"Terraform plan output (complete):\n```\n{clean}\n```")
 
     lines = clean.splitlines()
     changes = resource_changes(clean)
@@ -221,12 +232,20 @@ def review_plan(plan_text, converse=cli_converse):
 to the internet, removing permission boundaries, or deleting critical infrastructure.
 Routine changes (Lambda updates, tag changes, scaling) should be LOW.
 
-Reading the diff: Terraform shows only changed attributes and hides unchanged
-ones ("# (N unchanged attributes hidden)"). In a changed IAM policy statement an
-unchanged `Effect` is usually not shown, so do not assume an added action is an
-Allow - it may be a Deny. A Sid such as "Deny*" or "Block*" is a strong hint the
-statement denies. Only call out a permission grant when the plan shows
-`Effect = "Allow"` or the statement is new.
+Reading the diff: Terraform itself elides parts of every plan by design. These
+markers are normal Terraform output, not truncation; on their own they are
+never a reason to say the plan or the review is incomplete:
+- "# (N unchanged attributes hidden)", "# (N unchanged blocks hidden)",
+  "# (N unchanged elements hidden)": for an update, Terraform shows only the
+  changed attributes. The hidden ones are not changing.
+- "(sensitive value)": the value is redacted on purpose.
+- "(known after apply)": the value is computed by AWS during apply.
+- jsonencode(...) policy diffs show only the changed statements and fields, so
+  a changed statement can appear without its Effect, Resource or Sid.
+In a changed IAM policy statement an unchanged `Effect` is usually not shown, so
+do not assume an added action is an Allow - it may be a Deny. A Sid such as
+"Deny*" or "Block*" is a strong hint the statement denies. Only call out a
+permission grant when the plan shows `Effect = "Allow"` or the statement is new.
 
 {build_plan_section(plan_text)}"""
 
@@ -235,6 +254,10 @@ def review_plan(plan_text, converse=cli_converse):
     response = converse(messages, REVIEW_TOOL_CONFIG)
     if not response:
         return None
+    if response.get("stopReason") == "max_tokens":
+        # A cut-off tool call may parse but be missing findings; don't trust it.
+        print("Review stopped at the output token limit; discarding the partial answer.", file=sys.stderr)
+        return None
 
     for content in response.get("output", {}).get("message", {}).get("content", []):
         if "toolUse" in content and content["toolUse"]["name"] == "submit_review":

@@ -0,0 +1,49 @@
+"""Tests for post-plan-comment.sh — run: python3 -m unittest discover scripts/tests"""
+
+import os
+import subprocess
+import tempfile
+import unittest
+
+HERE = os.path.dirname(os.path.abspath(__file__))
+SCRIPT = os.path.join(HERE, "..", "post-plan-comment.sh")
+
+
+def post(plan_text, limit):
+    """Run the script with a fake `gh` that copies the comment body; return it."""
+    with tempfile.TemporaryDirectory() as tmp:
+        bin_dir = os.path.join(tmp, "bin")
+        os.mkdir(bin_dir)
+        out = os.path.join(tmp, "body.md")
+        with open(os.path.join(bin_dir, "gh"), "w") as f:
+            f.write(f'#!/bin/sh\nwhile [ "$1" != "--body-file" ]; do shift; done\ncp "$2" "{out}"\n')
+        os.chmod(os.path.join(bin_dir, "gh"), 0o755)
+        plan_dir = os.path.join(tmp, "tf")
+        os.mkdir(plan_dir)
+        for name in ("plan-output.txt", "plan-clean.txt"):
+            with open(os.path.join(plan_dir, name), "w") as f:
+                f.write(plan_text)
+        env = dict(os.environ, PATH=f"{bin_dir}:{os.environ['PATH']}", PR_NUMBER="1",
+                   GITHUB_REPOSITORY="javaBin/platform", PLAN_COMMENT_CHARS=str(limit))
+        subprocess.run(["sh", SCRIPT, os.path.join(plan_dir, "plan-output.txt"), "true"],
+                       cwd=tmp, env=env, check=True, capture_output=True)
+        with open(out) as f:
+            return f.read()
+
+
+class PostPlanCommentTest(unittest.TestCase):
+    PLAN = "".join(f"  # aws_x.r{i} will be created\n" for i in range(100)) + "Plan: 100 to add, 0 to change, 0 to destroy.\n"
+
+    def test_cut_plan_says_it_is_cut(self):
+        body = post(self.PLAN, 500)
+        self.assertIn(f"cut at 500 of {len(self.PLAN)} characters", body)
+        self.assertNotIn("aws_x.r99", body)
+
+    def test_plan_that_fits_has_no_cut_note(self):
+        body = post(self.PLAN, 60000)
+        self.assertIn("aws_x.r99", body)
+        self.assertNotIn("cut at", body)
+
+
+if __name__ == "__main__":
+    unittest.main()
@@ -110,10 +110,63 @@ def test_plan_over_limit_lists_every_change_and_states_what_was_omitted(self):
 
     def test_prompt_warns_about_hidden_unchanged_attributes(self):
         prompt = sent_prompt(plan(1))
-        self.assertIn("hides unchanged", prompt)
+        self.assertIn("shows only the\n  changed attributes", prompt)
         self.assertIn("Deny", prompt)
 
 
+    def test_complete_plan_is_declared_complete_with_its_change_count(self):
+        prompt = sent_prompt(plan(7))
+        self.assertIn("COMPLETE PLAN", prompt)
+        self.assertIn("all 7 resource change headers", prompt)
+        self.assertIn("(Plan: 0 to add, 7 to change, 0 to destroy.)", prompt)
+        self.assertIn("do not say the plan is truncated", prompt)
+
+    def test_oversized_plan_is_not_declared_complete(self):
+        section = rp.build_plan_section(plan(100, body_lines=20), limit=20_000)
+        self.assertNotIn("COMPLETE PLAN", section)
+        self.assertIn("NOTE: this plan is", section)
+
+    def test_prompt_explains_terraform_elision_markers_are_not_truncation(self):
+        prompt = sent_prompt(plan(1))
+        for marker in ("unchanged attributes hidden", "unchanged blocks hidden", "unchanged elements hidden",
+                       "(sensitive value)", "(known after apply)", "jsonencode"):
+            self.assertIn(marker, prompt)
+        self.assertIn("not truncation", prompt)
+
+    def test_no_input_cap_by_default(self):
+        self.assertIsNone(rp.MAX_PLAN_CHARS)
+        text = plan(20_000)
+        self.assertGreater(len(text), 2_000_000)
+        section = rp.build_plan_section(text)
+        self.assertIn("COMPLETE PLAN", section)
+        self.assertIn("aws_iam_role_policy.p19999 will be updated in-place", section)
+
+    def test_large_destroy_plan_is_sent_whole(self):
+        # PR #135 (2026-10-24): 231 destroys, 830k chars, was cut to 28 changes by the old 400k limit.
+        destroy = "".join(
+            f'  # module.iam.aws_iam_role.ci_deploy["team{i}"] will be destroyed\n'
+            f'  - resource "aws_iam_role" "ci_deploy" {{\n'
+            + "".join(f'      - attr_{j}                = "value-{j}" -> null\n' for j in range(66))
+            + "    }\n" for i in range(231))
+        text = "Terraform will perform the following actions:\n\n" + destroy + \
+            "\nPlan: 0 to add, 0 to change, 231 to destroy.\n"
+        self.assertGreater(len(text), 800_000)
+        section = rp.build_plan_section(text)
+        self.assertIn("COMPLETE PLAN", section)
+        self.assertIn('ci_deploy["team230"] will be destroyed', section)
+        self.assertNotIn("NOTE: this plan is", section)
+
+
+class StopReasonTest(unittest.TestCase):
+    def test_answer_cut_at_max_tokens_is_discarded(self):
+        def converse(messages, tool_config):
+            r = tool_response()
+            r["stopReason"] = "max_tokens"
+            return r
+
+        with mock.patch("sys.stderr"):
+            self.assertIsNone(rp.review_plan(plan(1), converse=converse))
+
 
 class ReviewFeedbackTest(unittest.TestCase):
     def test_tainted_and_removed_headers_parse_cleanly(self):

@@ -94,6 +94,16 @@ def test_large_plan_reaches_bedrock_whole_without_refresh_noise(self):
         self.assertIn("aws_iam_role_policy.p2999", prompts[0])
         self.assertNotIn("Refreshing state", prompts[0])
         self.assertNotIn("NOTE: this plan is", prompts[0])
+        self.assertIn("COMPLETE PLAN", prompts[0])
+
+    def test_answer_cut_at_max_tokens_is_retried(self):
+        cut = tool_response("LOW")
+        cut["stopReason"] = "max_tokens"
+        bedrock = FakeBedrock([cut, tool_response("MEDIUM")])
+        h = load(self.s3, bedrock)
+        with mock.patch("sys.stderr"):
+            self.assertEqual(h.handler({"plan_key": PLAN, "id_token": "good"}, None)["risk"], "MEDIUM")
+        self.assertEqual(bedrock.calls, 2)
 
     def test_transient_failures_are_retried(self):
         bedrock = FakeBedrock([RuntimeError("ThrottlingException"), tool_response("MEDIUM")])