fix(strix): add missing github-token fallback for self-target status publish - #1441
fix(strix): add missing github-token fallback for self-target status publish#1441seonghobae wants to merge 5 commits into
Conversation
…publish The publish-manual-pr-evidence-status job's target-app-token (an OpenCode app-token exchange) is scoped for sibling repositories and always 403s when .github is the repository_dispatch target of its own Strix run -- observed live on PR #1434's own repository_dispatch run (ContextualWisdomLab/.github/actions/runs/33306963425): "target-app-token did not succeed: gh: Resource not accessible by integration (HTTP 403)", then a hard failure since the publisher only tolerates an unpublishable status when the scan result was itself success. The job already declares permissions: statuses: write, so github.token has exactly the scope this self-referential case needs. A near-identical status-publish block ~40 lines earlier in the same file already has this exact fallback (GITHUB_STATUS_TOKEN, conditioned on target_repository == github.repository so it's never attempted -- and never a source of new 403 noise -- for a genuine cross-repo target); this second block had simply drifted without it. Mirrors that existing pattern exactly. Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 31 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
ChangesStrix 상태 게시 수정
Estimated code review effort: 1 (Trivial) | ~5분 Merge Risk: 🔵 Low · up to The workflow now recovers same-repository status publication when the delegated token is unavailable, without changing cross-repository behavior. A bounded merge-readiness risk remains because a failed target-validation path may still publish a failure status using raw dispatch values; this is mergeable with explicit owner awareness and follow-up to gate publication on validated target data. Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (3 skipped: 3 unsupported.) ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…nd declaration Devin Review finding on the prior commit: the smoke test's status_token_count assertion still expected exactly one GITHUB_STATUS_TOKEN declaration, which the prior commit's second declaration (in publish-manual-pr-evidence-status) broke. Fixed by: - Updating the declaration-count assertion from 1 to 2 (one per status-publishing job). - Adding a new assertion pinning both declarations to the identical same-repository conditional expression, so a future divergent copy (e.g. a typo'd comparison) fails closed here. - Adding a new assertion on how many times post_strix_status is actually invoked with the github-token fallback (3: the strix job's own inline publish tries it twice -- mid-chain and as the final last-resort retry -- while publish-manual-pr-evidence-status tries it once), so a declared-but-unused token would also fail closed. Verified against the actual file content, not assumed: ran the full scripts/ci/test_strix_quick_gate.sh (PASS) and the full pytest suite (1882 passed) locally before pushing. Co-Authored-By: Claude <noreply@anthropic.com>
…t timeout) Verified directly from #1441's noema-review job log (99249903390, head d877886): healthz and the launcher's own internal preflight succeeded (past the family-cap fix's stage entirely), but the shell script's separate post-healthz gateway smoke request -- max_tokens now 4096 per #1436's fix, up from 16 -- got zero bytes back for the full 120-second curl timeout. Distinct from both other evidence in this entry: not a bad-request rejection (no response at all) and not Strix's own client code path. Documented with plausible causes (the 120s ceiling never re-examined when max_tokens grew 256x; a real server-side hang for a specific request shape; transient flakiness) but not pursued further this pass -- noted as evidence for whoever picks this up next. Co-Authored-By: Claude <noreply@anthropic.com>
2 participants
Summary
Follow-up to #1434's investigation (item 3 of the coordinator's priority list): fixes the secondary
target-app-tokenHTTP 403 finding onrepository_dispatchStrix runs that target.githubitself.strix.yml'spublish-manual-pr-evidence-statusjob'starget-app-token(an OpenCode app-token exchange) is scoped for sibling repositories, not for.githubas the target of its ownrepository_dispatch— this central repo normally dispatches Strix to siblings, not to itself, so this token can never succeed for that self-referential case. Observed live on PR fix(zdr): cite NVIDIA's own Trial ToS for the nvidia_nim not-ZDR classification #1434's own dispatch run (ContextualWisdomLab/.github/actions/runs/33306963425):target-app-token did not succeed: gh: Resource not accessible by integration (HTTP 403), followed by a hard failure — the publisher only tolerates an unpublishable status when the underlying scan result was itselfsuccess(correct fail-closed behavior, not the bug).permissions: statuses: write, sogithub.tokenhas exactly the scope this case needs. A near-identical status-publish block ~40 lines earlier in the same file already has this exact fallback (aGITHUB_STATUS_TOKENenv var conditioned ontarget_repository == github.repository, so it's never attempted — and never a source of new noise — for a genuine cross-repo target). This second block had simply drifted without it; this PR mirrors the existing pattern exactly.repository_dispatchtargets (the new fallback is empty and skipped in that case, same as today).Test plan
coverage run -m pytest tests -q— 1882 passed, 1 skipped, 21 subtests (unchanged frommain)interrogate— 100.0% (unchanged)coverage report— 99% total, single pre-existing miss inscripts/ci/pingora_edge_policy.py:274(untouched)python3 -c "yaml.safe_load(...)"onstrix.ymltests/test_pr_governance_audit_contract.pypassesNot verified on a live hosted run: reproducing the original failure requires a
repository_dispatchStrix run targeting.githubitself, the same rare trigger path that surfaced the bug.🤖 Generated with Claude Code
https://claude.ai/code/session_015Gs7KmNvH75nxz1sL8mKjw
Generated by Claude Code
Summary by CodeRabbit
버그 수정
문서