fix(scheduler): accept safe Unicode branch refs - #696
Conversation
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughGit ref 검증이 Unicode 문자를 허용하도록 변경되었습니다. 제어 문자, 공백 문자, ChangesGit ref 검증
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
Current-head operational regression evidence for
Local current-head validation:
No active Actions runs remain for superseded head |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
27226f6 to
b906b44
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
b906b44 to
f9d15e0
Compare
seonghobae
left a comment
There was a problem hiding this comment.
Re-evaluate the exact current head after the latest governance and security checks complete.
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current headf9d15e0db11e286b091d61b2557f17804cb5e2f7. -
Head SHA:
f9d15e0db11e286b091d61b2557f17804cb5e2f7 -
Workflow run: 30837286675
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["CI script: pr_review_merge_scheduler.py"]
S1 --> I1["review and security gate shell path"]
I1 --> R1["Review risk: CI script: pr_review_merge_scheduler.py"]
R1 --> V1["bash -n plus Strix self-test"]
Evidence --> S2["Test: test_pr_review_merge_scheduler.py"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test: test_pr_review_merge_scheduler.py"]
R2 --> V2["targeted test run"]
OpenCode Review Overview
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow: scheduled-security-scan.yml"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow: scheduled-security-scan.yml"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["CI script: pr_review_merge_scheduler.py"]
S2 --> I2["review and security gate shell path"]
I2 --> R2["Review risk: CI script: pr_review_merge_scheduler.py"]
R2 --> V2["bash -n plus Strix self-test"]
Evidence --> S3["Test: test_pr_review_merge_scheduler.py"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test: test_pr_review_merge_scheduler.py"]
R3 --> V3["targeted test run"]
|
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current headf9d15e0db11e286b091d61b2557f17804cb5e2f7. -
Head SHA:
f9d15e0db11e286b091d61b2557f17804cb5e2f7 -
Workflow run: 30837286675
-
Workflow attempt: 2
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["CI script: pr_review_merge_scheduler.py"]
S1 --> I1["review and security gate shell path"]
I1 --> R1["Review risk: CI script: pr_review_merge_scheduler.py"]
R1 --> V1["bash -n plus Strix self-test"]
Evidence --> S2["Test: test_pr_review_merge_scheduler.py"]
S2 --> I2["regression suite"]
I2 --> R2["Review risk: Test: test_pr_review_merge_scheduler.py"]
R2 --> V2["targeted test run"]
Dismissed as stale after the head moved from f9d15e0 to 6d34d64. The underlying coverage failure was traced to mixed CodeQL action versions in scheduled-security-scan.yml and corrected on the live head. This dismissal does not grant approval; the new head must complete all checks and receive an independent exact-head review.
Dismissed as stale after the head moved from f9d15e0 to 6d34d64. The underlying coverage failure was traced to mixed CodeQL action versions in scheduled-security-scan.yml and corrected on the live head. This dismissal does not grant approval; the new head must complete all checks and receive an independent exact-head review.
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head re-review requested. The root cause of the prior coverage failure was the mixed CodeQL action refs in scheduled-security-scan.yml; the current head aligns them at v4.37.4. All direct current-head security workflows are green and there are no unresolved review threads.
|
Dependency note: this branch now includes the |
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head bounded evidence and found no blocking issues.
Findings
No blocking findings.
Summary
Inspected changed files .github/workflows/scheduled-security-scan.yml, scripts/ci/pr_review_merge_scheduler.py, tests/test_pr_review_merge_scheduler.py. Approval sufficiency: PR intent (fix(scheduler): accept safe Unicode branch refs) is confirmed by the diff and dispatch tests; evidence affirmatively supports behavior, structure, verification, and compatibility. Verification posture: current-head focused hunks + CodeGraph changed-scope evidence reviewed; Failed GitHub Check evidence reports no completed failed checks for head 6d34d64. Linter/static: no changed-source lint findings; repo test contract python3 -m pytest tests with python 3.12/3.13 workflow versions. TDD/regression: valid/invalid ref tuples (tests/test_pr_review_merge_scheduler.py:165) plus full Unicode dispatch payload assertion (line 1967) cover the regression that broke the sweep. Coverage: Coverage execution evidence reports Result PASS - supported repository test suites passed. Docstring coverage: Coverage execution evidence reports configured repository docstring gates passed or advisory. DAG: base-to-head changed-flow flowchart below (validate_git_ref -> client_payload pr_head_ref -> central workflow dispatch -> OpenCode/Strix workflows; CodeQL init/analyze alignment -> scheduled security scan). PoC/execution: no OPENCODE_EXECUTION_RECEIPT present; evidence is trusted tests, diff, and Coverage PASS. DDD/domain: scheduler dispatch domain unchanged; only ref-validation policy adjusted. CDD/context: central .github workflows context; the scheduled-security-scan portion is a documented dependency of #723 per author note. Similar issues: no similar unresolved issues found in CodeGraph blast radius. Claim/concept check: git-check-ref-format-style component rules (.lock, dot parts, @{, ..) match the implementation and tests. Standards search: git ref-format rules (no component starting with '.', none ending '.lock', no '@{', no '..', no control/space) are mirrored by the bounded branch. Compatibility/convention: GIT_REF_ASCII_SAFE_CHARS and has_unsafe_character follow snake_case repo convention; no new exposed identifiers or API fields. Breaking-change/backcompat: every previously accepted ASCII ref remains accepted; only previously-rejected Unicode/format refs change; CodeQL bump is patch-level v4.37.0->v4.37.4 applied to both init and analyze. Implementation completeness: no placeholder bodies; validator fully implemented and tested. Performance: O(n) per-ref validation on dispatch; no hot path impact. Developer experience: scheduler operators no longer blocked by Unicode branch sweeps; evidence is the dispatch test at tests/test_pr_review_merge_scheduler.py:1967 and Coverage PASS. User experience: non-web surface - CI review-comment/dispatch logs; aligned CodeQL refs avoid mixed-version failures; evidence is the workflow diff and absence of failed checks. Visual/DOM: non-web interaction surface (CLI/workflow review output) reviewed; no DOM changes. Accessibility/i18n: i18n-positive - emoji and Korean branch refs now dispatch correctly; no UI surface affected. Supply-chain/license: codeql-action pinned SHA f205ea1c3313d32999d8d6a48b4f6530d4437b38 (# v4.37.4); checkout/harden-runner refs unchanged; no new dependencies. Packaging: pyproject requires-python >=3.10; str.isascii and unicodedata are stdlib and available; no packaging changes. Security/privacy: injection resistance preserved for structured argv/JSON dispatch (no shell interpolation of refs); no secrets touched; no sequential/exposed identifier surface.
Approval sufficiency: bounded evidence supplied affirmative approval evidence for changed files, coverage/docstring posture, risk surfaces, and current-head verification; approval is not based merely on the absence of known blockers.
Verification posture: CodeGraph evidence was initialized and bounded current-head evidence reviewed for changed-file evidence including .github/workflows/scheduled-security-scan.yml, scripts/ci/pr_review_merge_scheduler.py, tests/test_pr_review_merge_scheduler.py.
Linter/static: workflow/static review evidence is bounded by the current-head GitHub Checks gate and changed-file evidence.
TDD/regression: coverage execution evidence and focused changed hunks were reviewed from bounded-review-evidence.md.
Coverage: coverage execution evidence reports supported repository test suites passed.
Docstring coverage: coverage execution evidence reports configured repository docstring gates passed or docstring coverage was advisory.
DAG: CodeGraph/source-backed behavior map connects .github/workflows/scheduled-security-scan.yml to the affected review, runtime, or workflow path and required checks.
PoC/execution: coverage-evidence job executed on the current head and reported PASS.
DDD/domain: workflow and repository-governance invariants were reviewed against changed files in bounded evidence.
CDD/context: CodeGraph evidence, changed-file history, and focused hunks were reviewed from bounded-review-evidence.md.
Similar issues: changed-file history evidence was reviewed for comparable local precedents.
Claim/concept check: bounded evidence, repository source, current-head workflow evidence, and, where numeric, scientific, statistical, or literature-backed claims are affected, original-paper/formula evidence and parameter-recovery expectations were used for claims.
Standards search: standards and external-source claims require trusted bounded source evidence prepared outside the isolated model process; no evidence-backed standards blocker is present in bounded evidence.
Compatibility/convention: changed workflow/script conventions, object naming, and reserved-word safety for schema/API/config/code surfaces were checked in bounded evidence.
Breaking-change/backcompat: deployment evidence and changed-file history were checked for backward-compatibility risk.
Performance: changed surfaces were checked for performance risk in bounded evidence.
Developer experience: changed automation, review, test, setup, and maintenance surfaces were checked for helpful or obstructive DX impact in bounded evidence.
User experience: connected user, operator, API, CLI, documentation, review-comment, status-check, rendering, and workflow-reader behavior was checked for contradictions against code, docs, and tests in bounded evidence.
Visual/DOM: deterministic repair does not infer browser runtime execution; source-backed DOM/UI evidence and trusted workflow receipts were reviewed when present, and non-web surfaces used API/CLI/log/docs/workflow evidence instead.
Accessibility/i18n: accessibility, localization, and human-readable text surfaces were checked where UI, CLI, API message, docs, logs, or review text changed.
Supply-chain/license: dependency, package, model, container, and external-tool changes were checked in bounded evidence.
Packaging: package, build, test, lint, and security contracts were checked in bounded evidence.
Security/privacy: workflow-token, review-gate, and repository-automation security/privacy boundaries were checked in bounded evidence.
Adversarial validation
{"status":"passed","probes":[{"path":"scripts/ci/pr_review_merge_scheduler.py","line":561,"hypothesis":"The rewritten validate_git_ref lets an invalid ref containing a .lock component or a dot component pass through into the dispatch payload.","attack_or_counterexample":"ref='feature/main.lock' (git refuses slash-separated components ending in .lock) and 'feature/.tmp/main' (component starting with a dot).","evidence":"Trusted source trace at scripts/ci/pr_review_merge_scheduler.py:561: the bounded branch splits the ref on '/' and rejects any part that is '.', starts with '.', or ends with '.lock', so 'feature/main.lock' and 'feature/.tmp/main' raise ValueError; the trusted changed test at tests/test_pr_review_merge_scheduler.py:171-187 includes both in the bad_ref tuple asserting pytest.raises(ValueError), and Coverage execution evidence reports the supported repository test suite passed. source-line-sha256=d082b2a1c69b618aaf79212e21df24fcd947cb0fd445228a3711a141e47953d5","outcome":"falsified"},{"path":"tests/test_pr_review_merge_scheduler.py","line":165,"hypothesis":"The fix still rejects the exact Unicode production branch that broke the organization sweep, so the PR fails its stated goal.","attack_or_counterexample":"ref='🎨-palette-ux-improvement-13325911538352561627' (emoji category So, allowed) and '기능/달력-개선' (Korean Lo letters, no C/Z category).","evidence":"Trusted source trace at tests/test_pr_review_merge_scheduler.py:165: the valid_ref tuple asserts validate_git_ref returns both refs unchanged, and the trusted dispatch test at tests/test_pr_review_merge_scheduler.py:1967 asserts json.loads(dispatch_calls[1][1])['client_payload']['pr_head_ref'] equals the Unicode head_ref through dispatch_strix_evidence/dispatch_opencode_review; Coverage execution evidence reports the supported repository test suite passed. source-line-sha256=e4b043a376bc1595000a813fbdd53d4ccb49d69344d563ff36c2f9ee94376cbc","outcome":"falsified"},{"path":".github/workflows/scheduled-security-scan.yml","line":93,"hypothesis":"The CodeQL steps keep mixed action refs (init and analyze at different versions), re-triggering the exact-head coverage/workflow failure this PR claims to fix.","attack_or_counterexample":"Compare both github/codeql-action/init and github/codeql-action/analyze pins in the changed hunk at scheduled-security-scan.yml:93-100.","evidence":"Trusted changed diff at .github/workflows/scheduled-security-scan.yml:93-100 pins both init and analyze to the same SHA f205ea1c3313d32999d8d6a48b4f6530d4437b38 (# v4.37.4), removing the mixed v4.37.0 refs that previously failed the exact-head coverage contract (author conversation note at 2026-08-03T19:19:33Z); Coverage execution evidence reports Result PASS and Failed GitHub Check evidence reports no completed failed checks for head 6d34d6445575da00a91505a4edcf8914a8a090d6. source-line-sha256=20081fe69905f85a4909cb816fb14f18fb35c075a52641678fe8082c633e5f0a","outcome":"falsified"}],"residual_risk":"Unicode format characters (Cf category, e.g. ZWJ used in some emoji sequences) are conservatively rejected, so exotic emoji ZWJ-joined branch names would still fail validation; the removed module constant GIT_REF_RE has no covering cross-file usage evidence, low risk because the scheduler is invoked as a script."}- Result: APPROVE
- Reason: Verified the Unicode-capable ref validator preserves conservative rejection of metacharacters, control/separator Unicode, dot components, .lock, @{, .., // and HEAD; CodeQL init/analyze pins are aligned at v4.37.4; trusted tests and Coverage execution evidence PASS; no failed checks and no unresolved threads.
- Head SHA:
6d34d6445575da00a91505a4edcf8914a8a090d6 - Workflow run: 30845590617
- Workflow attempt: 1
Summary
.lock,@{,.., and repeated slashesProduction evidence
The current-main organization sweep failed in run 30684158698 while processing
ContextualWisdomLab/codec-carver:ValueError: invalid git ref: '🎨-palette-ux-improvement-13325911538352561627'git check-ref-format --branchaccepts that exact ref. The scheduler passes refs as structured subprocess arguments and JSON payload fields, so an ASCII-only allowlist is not required for shell safety.Validation
python3 -m pytest -q tests/test_pr_review_merge_scheduler.py— 110 passedpython3 -m ruff check scripts/ci/pr_review_merge_scheduler.py tests/test_pr_review_merge_scheduler.py— passedgit diff --check origin/main...HEAD— passedgit check-ref-format --branchandvalidate_git_refSummary by CodeRabbit
개선 사항
.lock으로 끝나거나 하이픈으로 시작하는 ref는 사용할 수 없습니다.테스트