Skip to content

fix(review): merge 증거와 current-head 재검증을 fail-closed로 강화#605

Open
seonghobae wants to merge 9 commits into
mainfrom
bolt-precompile-regex-6505304934535388869
Open

fix(review): merge 증거와 current-head 재검증을 fail-closed로 강화#605
seonghobae wants to merge 9 commits into
mainfrom
bolt-precompile-regex-6505304934535388869

Conversation

@seonghobae

@seonghobae seonghobae commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

목적

초기 정규표현식 사전 컴파일 최적화에 더해, 중앙 PR 스케줄러가 오래되거나 출처가 불명확한 증거로 병합하지 않도록 리뷰에서 확인된 신뢰경계를 보강합니다.

주요 변경

  • 외부 head 판별 정규표현식을 모듈 수준에서 사전 컴파일합니다.
  • GitHub 읽기, Actions 제어, dispatch, mutation 자격 증명 경로를 분리하고 Actions에서는 신뢰된 /usr/bin/gh를 고정하며 로컬 검사에서는 portable gh를 사용합니다.
  • current HEAD의 승인, 변경 요청, peer checks, merge 정책, head SHA를 mutation 직전에 다시 조회하여 TOCTOU 병합을 차단합니다.
  • 이름만 같은 check가 아니라 provenanced same-head Strix workflow만 보안 증거로 인정합니다.
  • GraphQL 권한 부족으로 REST fallback을 사용할 때도 GitHub Actions 앱, 동일 저장소 Actions URL, 동일 run ID, 동일 head SHA, 허용된 Strix workflow 이름을 Actions run API로 모두 재증명합니다.
  • scheduler 자체 check를 peer wait 대상에서 제외하고 CheckRun과 StatusContext 이름을 중복 제거합니다.
  • unsafe 상태에서는 auto-merge를 비활성화하고 direct merge와 dispatch를 fail-closed로 보류합니다.
  • command argv, repository/ref/SHA 및 오류 메시지의 민감정보 처리를 강화합니다.

검증

  • python3 -m pytest -q tests/test_pr_review_merge_scheduler.py — 113 passed
  • 전체 테스트 및 커버리지 — 643 passed, 100% line coverage
  • python3 -m ruff check scripts/ci/pr_review_merge_scheduler.py tests/test_pr_review_merge_scheduler.py — passed
  • git diff --check — passed
  • CodeGraph sync 및 변경 심볼 탐색 — passed

PR created automatically by Jules for task 6505304934535388869 started by @seonghobae; scope expanded in response to current-head security review.

@google-labs-jules

Copy link
Copy Markdown

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Current HEAD 63886d61050568bcb72945d022b7ad4962209b6f 기준 감사를 완료했습니다.

  • 별도 worktree에서 CodeGraph init/explore로 두 parser의 호출 경로와 영향 범위를 확인했습니다.
  • 변경은 기존 정규식 문자열과 동일한 module-level compiled pattern으로의 치환이며 반환 의미 변화가 없습니다.
  • python3 -m pytest tests/test_pr_review_merge_scheduler.py -q: 104 passed
  • 두 parser의 match/non-match 직접 회귀 확인: pass
  • git diff --check: pass
  • current-head open code-scanning alerts: 0
  • review threads: 0

현재 merge blocker는 코드 결함이 아니라 중앙 GitHub-hosted runner 포화로 queued 상태인 required checks와 required review입니다. self-approval이나 정책 우회는 하지 않습니다.

@google-labs-jules

Copy link
Copy Markdown

Current HEAD 63886d61050568bcb72945d022b7ad4962209b6f 기준 감사를 완료했습니다.

  • 별도 worktree에서 CodeGraph init/explore로 두 parser의 호출 경로와 영향 범위를 확인했습니다.
  • 변경은 기존 정규식 문자열과 동일한 module-level compiled pattern으로의 치환이며 반환 의미 변화가 없습니다.
  • python3 -m pytest tests/test_pr_review_merge_scheduler.py -q: 104 passed
  • 두 parser의 match/non-match 직접 회귀 확인: pass
  • git diff --check: pass
  • current-head open code-scanning alerts: 0
  • review threads: 0

현재 merge blocker는 코드 결함이 아니라 중앙 GitHub-hosted runner 포화로 queued 상태인 required checks와 required review입니다. self-approval이나 정책 우회는 하지 않습니다.

코드 검토 및 피드백 감사드립니다. 알려주신 대로 모든 검토 결과(테스트, 정규식 일치, 정적 검사 등)가 통과(pass) 상태임을 확인했습니다. 추가로 수정해야 할 내용이 없으므로 이 작업(리뷰 단계)을 마무리하고 제출 대기 중입니다.

@seonghobae

Copy link
Copy Markdown
Contributor Author

Superseding current-HEAD audit: PR head changed after the preceding comment.

Current HEAD is now e559d86e4b8b6c7e691ff3fdd8206fcfaf467309. The second commit is tree-empty; 63886d6..e559d86 changes no source bytes. I fast-forwarded the pinned worktree, ran CodeGraph sync/explore on the exact new HEAD, and reran:

  • python3 -m pytest tests/test_pr_review_merge_scheduler.py -q: 104 passed
  • parser match/non-match behavior: preserved
  • git diff origin/main...HEAD --check: passed
  • current-head open code-scanning alerts: 0
  • review threads: 0

No prior-head queued/running workflow remained when rechecked. Required checks/review remain queued under the central runner saturation; no policy bypass or self-approval was used.

@google-labs-jules

Copy link
Copy Markdown

Superseding current-HEAD audit: PR head changed after the preceding comment.

Current HEAD is now e559d86e4b8b6c7e691ff3fdd8206fcfaf467309. The second commit is tree-empty; 63886d6..e559d86 changes no source bytes. I fast-forwarded the pinned worktree, ran CodeGraph sync/explore on the exact new HEAD, and reran:

  • python3 -m pytest tests/test_pr_review_merge_scheduler.py -q: 104 passed
  • parser match/non-match behavior: preserved
  • git diff origin/main...HEAD --check: passed
  • current-head open code-scanning alerts: 0
  • review threads: 0

No prior-head queued/running workflow remained when rechecked. Required checks/review remain queued under the central runner saturation; no policy bypass or self-approval was used.

감사합니다. 새로운 HEAD 상태에서도 변경 사항 없이 테스트가 정상적으로 통과됨을 확인했습니다.

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.

Findings

1. HIGH Current-head GitHub Checks - Fix failed required checks before approval

  • Problem: Failed same-head checks remain for e559d86e4b8b6c7e691ff3fdd8206fcfaf467309.
  • Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
  • Fix: Read and fix the failed check logs below, then rerun the current-head checks.
  • Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.

Failed checks:

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"]
Loading

@opencode-agent

opencode-agent Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

OpenCode Review Overview

  • Head SHA: 442ee94d38dc35e736e7f32cd316c0d61d8ce89a
  • Workflow run: 29817628164
  • Workflow attempt: 1
  • Gate result: REQUEST_CHANGES (approval step)

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 success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before 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 head 442ee94d38dc35e736e7f32cd316c0d61d8ce89a.

  • Head SHA: 442ee94d38dc35e736e7f32cd316c0d61d8ce89a

  • Workflow run: 29817628164

  • 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"]
Loading

Copilot AI review requested due to automatic review settings July 21, 2026 06:03

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the PR Review Merge Scheduler’s GitHub/CI evidence handling, including new module-level precompiled regexes plus broader changes around Strix provenance, merge/auto-merge safety revalidation, and gh invocation.

Changes:

  • Precompile external-head parsing regexes at module scope and switch parsing helpers to use them.
  • Introduce GITHUB_CLI/TRUSTED_GITHUB_CLI plumbing and split GraphQL calls into read vs mutation helpers.
  • Add new “latest provenanced CheckRun” selection utilities and tighten Strix evidence requirements; add merge-time revalidation and additional CLI input validation.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
scripts/ci/pr_review_merge_scheduler.py Adds precompiled regexes and significantly expands scheduler behavior: gh CLI pinning, mutation path, check-run selection, Strix provenance enforcement, merge-state revalidation, and CLI input validation.
tests/test_pr_review_merge_scheduler.py Updates/extends tests to cover the new scheduler behaviors (mutation path, Strix evidence state changes, running checks filtering, merge revalidation, and input validation).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/ci/pr_review_merge_scheduler.py
Comment thread scripts/ci/pr_review_merge_scheduler.py
Copilot AI review requested due to automatic review settings July 21, 2026 07:54
@seonghobae seonghobae changed the title ⚡ Bolt: 루프 내 반복 컴파일을 피하기 위한 외부 head 정규표현식 미리 컴파일 fix(review): merge 증거와 current-head 재검증을 fail-closed로 강화 Jul 21, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

scripts/ci/pr_review_merge_scheduler.py:3188

  • The PR description/title indicates this change is limited to precompiling regexes for external-head parsing, but this file also introduces substantial behavioral/security changes (e.g., trusted gh path pinning, Strix provenance enforcement, merge-state revalidation, new token separation). Please update the PR description/title to reflect the expanded scope or split these concerns into separate PRs for reviewability.
def parse_external_head_update_reason(reason: str) -> str | None:
    """Extract the external head repository from non-mutable update guidance."""
    match = EXTERNAL_HEAD_UPDATE_RE.search(reason)
    if not match:
        return None
    return match.group(1)


def parse_external_head_merge_reason(reason: str) -> str | None:
    """Extract the external head repository from merge-exclusion guidance."""
    match = EXTERNAL_HEAD_MERGE_RE.search(reason)
    if not match:

Comment thread scripts/ci/pr_review_merge_scheduler.py Outdated
Comment thread scripts/ci/pr_review_merge_scheduler.py
Copilot AI review requested due to automatic review settings July 21, 2026 08:17

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread scripts/ci/pr_review_merge_scheduler.py Outdated
Copilot AI review requested due to automatic review settings July 21, 2026 08:54

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread scripts/ci/pr_review_merge_scheduler.py
Copilot AI review requested due to automatic review settings July 21, 2026 09:06

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before 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 head 442ee94d38dc35e736e7f32cd316c0d61d8ce89a.

  • Head SHA: 442ee94d38dc35e736e7f32cd316c0d61d8ce89a

  • Workflow run: 29817421594

  • 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"]
Loading

@opencode-agent opencode-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 success with required evidence or explicit no-source not-applicable evidence.

  • Regression test: Keep the approval branch checking needs.coverage-evidence.result == success before 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 head 442ee94d38dc35e736e7f32cd316c0d61d8ce89a.

  • Head SHA: 442ee94d38dc35e736e7f32cd316c0d61d8ce89a

  • Workflow run: 29817628164

  • 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"]
Loading

Copilot AI review requested due to automatic review settings July 21, 2026 09:28

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread scripts/ci/pr_review_merge_scheduler.py
Copilot AI review requested due to automatic review settings July 21, 2026 09:37

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants