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 7 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 7 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: Team Run ID: 📒 Files selected for processing (4)
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>
|
Contextual-Orchestrator를 같이 손보든 어쩌든 해결하세요. Bypass merge 필요하면 가능 (chicken and eggs 상황이라면) + NVIDIA NIM 만 쓰는 건 허용하지 않아요. Contextual-Orchestrator를 쓰세요. |
# Conflicts: # CHANGELOG.md
| # sibling repos, not to ContextualWisdomLab/.github as its own | ||
| # target). Empty for a cross-repo target, matching post_strix_status's | ||
| # existing empty-token skip. | ||
| GITHUB_STATUS_TOKEN: ${{ (github.event.client_payload.target_repository == '' || github.event.client_payload.target_repository == github.repository) && github.token || '' }} |
There was a problem hiding this comment.
📝 Info: Fallback remains repository-scoped
An absent target resolves to the workflow repository, while an explicit matching target selects GITHUB_STATUS_TOKEN. Sibling targets leave it empty and skip publication.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if post_strix_status "github-token" "$GITHUB_STATUS_TOKEN"; then | ||
| exit 0 | ||
| fi |
There was a problem hiding this comment.
📝 Info: Fallback ordering preserves sibling behavior
Self-target runs try GITHUB_STATUS_TOKEN after the app token. Sibling runs skip the empty token, preserving the existing credential sequence.
Was this helpful? React with 👍 or 👎 to provide feedback.
| status_token_count="$(grep -c '^[[:space:]]*GITHUB_STATUS_TOKEN:' "$workflow_file")" | ||
| assert_equals "1" "$status_token_count" "strix workflow defines GITHUB_STATUS_TOKEN once so GitHub can parse repository_dispatch" | ||
| assert_equals "2" "$status_token_count" "strix workflow defines GITHUB_STATUS_TOKEN exactly twice, once per status-publishing job" | ||
| status_token_conditional_count="$(grep -c "GITHUB_STATUS_TOKEN: \${{ (github.event.client_payload.target_repository == '' || github.event.client_payload.target_repository == github.repository) && github.token || '' }}" "$workflow_file")" | ||
| assert_equals "2" "$status_token_conditional_count" "both GITHUB_STATUS_TOKEN declarations use the identical same-repository conditional" | ||
| # Three invocations, not two: the strix job's own inline publish tries | ||
| # the github-token fallback twice (once mid-chain, once as the final | ||
| # last-resort retry after every other credential has failed), while | ||
| # publish-manual-pr-evidence-status tries it once. Both jobs actually | ||
| # consuming their own declared GITHUB_STATUS_TOKEN (not just declaring | ||
| # and ignoring it) is the property under test here. | ||
| github_status_token_fallback_count="$(grep -c 'post_strix_status "github-token" "\$GITHUB_STATUS_TOKEN"' "$workflow_file")" | ||
| assert_equals "3" "$github_status_token_fallback_count" "both status-publishing jobs actually invoke their own GITHUB_STATUS_TOKEN fallback, not just declare it" |
There was a problem hiding this comment.
📝 Info: Contract test couples global counts
The assertions count declarations and calls across the whole workflow. A future unrelated publisher will require updating this test despite preserving both protected jobs.
Was this helpful? React with 👍 or 👎 to provide feedback.
Ports the identical fix already applied to this session's #1476 and #1488 (commit 4323e27 pattern; root-caused in .github#1506, which targets main but has not merged there yet). This PR's exact-head-path-policy check runs its own head-branch copy of scripts/ci/test_strix_quick_gate.sh (plain pull_request trigger, not pull_request_target), and the merge commit onto this branch carried forward the pre-existing bug from both sides (neither main nor this branch's own prior head had the fix), so it needs porting here directly -- confirmed live: run 33455130248/job 99693376191 failed with exactly this assertion. Root cause: assert_opencode_review_uses_codegraph_and_contextual_orchestrator extracted the required-workflow-bootstrap job block from opencode-review.yml with awk '/^ required-workflow-bootstrap:$/,/^[^ ]/'. Every job key in that workflow is indented 2 spaces (never column 0), so the end pattern never matched until EOF, sweeping an unrelated if: line from a later job into the "block" and failing the assertion on unrelated content. Fixed by using an explicit state flag so the end pattern (^ [A-Za-z0-9_-]+:) is only tested starting on the line after the start match, correctly bounding the block to just its own lines. Verified directly: the old awk swept in line 219's `if: github.event.action != 'closed'` from a later job; the new awk captures zero if: lines. Full local run: bash scripts/ci/test_strix_quick_gate.sh -> PASS, exit 0 (was FAIL/exit 1 before this commit). coverage run -m pytest tests -q -> 2126 passed, 1 skipped, 21 subtests. coverage report -> 100%. interrogate -> 100%. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015Gs7KmNvH75nxz1sL8mKjw
|
|
Contextual-Orchestrator와 관계한 것들을 같이 손보든 어쩌든 해결하세요. Bypass merge 필요하면 가능 (chicken and eggs 상황이라면) + NVIDIA NIM 만 쓰는 건 허용하지 않아요. Contextual-Orchestrator를 쓰세요. Timeout은 적어도 3시간으로 잡으세요. 120초 같은 건 당황스럽군요. Opencode와 Noema 는 Coderabbitai 및 Devin 수준으로 실제로 리뷰를 하게 하시오. Strix도 보안 리뷰를 꼼꼼하게 하도록 하시오. 특히 보안 리뷰는 전체 코드로 수행하는 것입니다. Contextual-Orchestrator는 실시간으로 빠르면서 능력이 좋은 모델에 요청을 보내어 시간을 당기시오. |
|
Superseded by protected main. Fresh |
|
Superseded by protected main. Fresh |
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
버그 수정
문서