fix(ci): stop suppressing stderr in fork first-principles review finalize helper

[aniruddhaadak80]

Problem / Motivation

The complete() check-run finalize helper in fork-first-principles-review.yml redirects both stdout AND stderr to /dev/null:

if gh api --method PATCH "repos/$REPO/check-runs/$CHECK_ID" \



  ... >/dev/null 2>&1; then

When a finalize permanently fails, the ::warning:: line names the check-run and head SHA but cannot carry the underlying error. An operator debugging a stranded finalize sees that it failed but not why.

Why it matters

CI failures in the fork review pipeline are already hard to diagnose. Hiding gh's stderr (the actual HTTP status/error text) removes the only diagnostic an operator has without digging into raw API calls.

What changed (motivation → approach → change)

Removed 2>&1 from the complete() helper's PATCH call so stderr reaches the job log. stdout is still suppressed (the JSON response is noise). This matches the pattern already used in fork-gpt-review.yml, fork-opus-review.yml, fork-design-review.yml, and fork-ux-review.yml.

Tests

• YAML syntax valid (no structural change to the workflow)

• Only one line changed: line 646 of fork-first-principles-review.yml

Manual verification

• Reviewed the four sibling fork review files to confirm they already use >/dev/null || true (no 2>&1)

Related Issues

Fixes #5822

Screenshots / video

N/A — CI workflow change, no UI.

Pattern harvest

Rule candidate: review-prompt

Pattern: a >/dev/null 2>&1 on a finalize/reporting call hides the error text an operator needs to diagnose the failure; suppress stdout only and let stderr through.

Checklist

• Code follows existing conventions

• Single-line change, no behavioral change for success path

• No secrets, no test changes needed

Contribution License Agreement

[bolichen97]

Full-diff/commit-containment audit (current head a288e5e86cfae33b2aac055db8fe4a095fb9e4f1): this PR contains #5603's exact commit (287dfb792265cdf291b5e35611fabf381a6b59d3) before its own fork-review stderr change, while the PR base is declared as main. The cumulative redaction diff is therefore unrelated to this PR's unique CI behavior, and the same chain is carried onward into #6065 and #6072.

If this is intended as a stack, retarget #6031 to #5603's branch so the review diff is only a288e5e86.... If it is independent, rebase onto current main and cherry-pick only that fork-review commit. Either route should remove #5603's cumulative scope before merge; then #6065/#6072 need the same cleanup rather than inheriting this PR again.

[bolichen97]

Description ↔ code mismatch: this is not a single-line change

Blocking finding from a Description-vs-code consistency audit. Criterion: the description says it only changes A, but the diff also changes B, and B affects risk and review scope — here B is a credential-redaction path.

The claim

Only one line changed: line 646 of fork-first-principles-review.yml

Single-line change, no behavioral change

No secrets, no test changes needed

What the diff actually contains

.github/workflows/fork-first-principles-review.yml  +1/-1
src/kiro_crew/cron_script.py                        +31/-13
test/test_cron_script.py                            +71/-0
test/test_spec_builder_routes_coverage.py           +22/-1

Four files, +125/-15. The workflow line is real and matches the description (>/dev/null 2>&1 → >/dev/null). The other three files are undisclosed.

src/kiro_crew/cron_script.py is not incidental — it changes credential-redaction behaviour:

• adds _STDERR_FULL_REDACT_MAX = 4 * 1024 * 1024;
• rewrites _stderr_tail to redact the whole buffer before tailing, instead of tailing then redacting;
• introduces a new behaviour where stderr larger than 4 MiB is replaced with [stderr omitted: too large to redact in full];
• in run_script_sandboxed, replaces redact(stdout)[:200] with a new redact_and_truncate(stdout, 200).

test/test_cron_script.py (+71) and test/test_spec_builder_routes_coverage.py (+22) add cases for exactly those redaction boundaries — which also contradicts "no test changes needed".

Why this blocks

Redact-then-truncate versus truncate-then-redact is a real security difference: the old order can leave a secret spanning the cut boundary partially unredacted, and the new >4 MiB path withholds output entirely. That is a defensible change, but it is a security-boundary change presented as a no-op YAML edit. A reviewer who trusts the description reviews one line of workflow YAML and never looks at the redaction logic, the new public symbol, or the new withholding behaviour. The checklist claim "no test changes needed" is contradicted by 93 lines of new tests in the same diff.

Note this diff is also carried by #6065, whose body likewise describes only its frontend file. The same four files appear in both PRs, which suggests the branch picked up an unrelated commit rather than that the change was intended here.

Required change — pick one

1. Drop the unrelated commit so this PR really is the one-line workflow edit it claims to be, and open the cron_script.py redaction change as its own PR where it can be reviewed as a security change.
2. Or rewrite the description to list all four files, describe the redact-before-tail reordering, the new redact_and_truncate and _STDERR_FULL_REDACT_MAX, and the >4 MiB withholding behaviour; and correct the "single-line", "no behavioral change" and "no test changes needed" claims.

_{Automated Description↔code consistency audit. Only these five classes are treated as blocking: described behaviour absent from code; undisclosed change to user-visible behaviour/API/permissions/security boundary; test-pass claims contradicted by evidence; undisclosed scope beyond what the description says; conflict with spec/docs/an invariant. Wording and style are never flagged.}

[bolichen97]

Audit note — this PR and #5603 are one branch stack, not duplicates

A duplicate-detection sweep flagged these two as near-identical because they share a large block of added lines. They are not duplicates: one branch contains the other's commits, so the shared lines are inherited, and the review diff overstates what this PR actually authored.

#6031's branch literally contains #5603's sole commit as its parent: git log $(git merge-base origin/main pr/6031)..pr/6031 prints a288e5e86 fix(ci): stop suppressing stderr... then 287dfb792 fix(security): redact before slicing at every bounded-logging site, and 287dfb7 is pr/5603's entire branch. Both share merge-base e00c749, so the two diffs are directly comparable, and diff over the shared portion of the two cached patches (from the diff --git a/src/kiro_crew/cron_script.py line to EOF) exits 0 — the redaction change is byte-for-byte the same, not a reimplementation. #6031's own work is one line: dropping 2>&1 from the complete() helper's gh api --method PATCH in .github/workflows/fork-first-principles-review.yml, which #5603 does not touch at all and which is still unfixed on origin/main (line 718). The two PRs even close different issues (#5582 vs #5822). This is the brief's stacked-branch artifact, not duplication.

What is genuinely each PR's own

If #5603 lands, #6031 still carries real, wanted work: the one-line 2>&1 removal in the fork-first-principles complete() helper, still present on origin/main, closing open issue #5822. Its diff should then collapse to that single line once rebased. The converse is the asymmetry that makes the survivor BOTH rather than #6031: if #6031 landed as-is, #5603 would be left empty — but that is precisely the outcome the maintainer forbade, and #6031 carries an unresolved CHANGES_REQUESTED for presenting the redaction change as a no-op YAML edit, so it cannot land in its current shape.

Suggested action

Land #5603 first, then rebase #6031 onto main so its diff shrinks to its own work. Reviewing #6031 before that means reviewing #5603's code twice.

---

_{From a repository-wide duplicate/overlap audit of every pull request open against main (2026-09-02, 330 PRs, one reviewer per PR). Each PR was read as its full merge-base diff plus its description and every comment and review, then compared against each candidate PR's own diff and against origin/main at 1a765b88ceb7. This PR is not being closed — the note is informational. If the reading is wrong, please correct the reasoning rather than just the conclusion.}

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• This PR is OVERLAPPING with PR #5603. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #6031: REBASE. Stacked branch. PR #6031's review diff overstates its authored work by three files; rebasing onto current main (or retargeting the base to PR #5603's branch) collapses it to the single workflow line that is genuinely its own and still unfixed on main. Files: src/kiro_crew/cron_script.py, test/test_cron_script.py. The two independent directions used different labels; the matrix conservatively retains OVERLAPPING for coordination.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.

[aniruddhaadak80]

Addressed both findings via rebase (pushed): the branch no longer contains #5603's commit — rebased onto current main so the diff is exactly the single-line \ork-first-principles-review.yml\ change the description claims (verified: 1 file, +1/-1). Description now also carries the missing Screenshots (N/A) and Pattern harvest sections.

[aniruddhaadak80]

CI triage on head 2789e85: no code-side failure exists. Every monitored lane sits at action_required (fork workflow-approval gate; this branch touches .github/workflows/). Unblocks only via maintainer Approve-and-run plus review. Escalating as maintainer-side; nothing reproduces locally (branch MERGEABLE, single-commit).