Strix 프론트엔드 이메일 스캔에 백엔드 컨텍스트 포함 - #206
Merged
Merged
Conversation
Contributor
Contributor
|
Note
|
| Layer / File(s) | Summary |
|---|---|
Env var and resolver scripts/ci/strix_quick_gate.sh |
Adds STRIX_INPUT_FILE_ROOT and resolve_trusted_input_file(label, input_file) which canonicalizes paths and enforces regular-file, in-root constraints. |
Integrate resolver into LLM handling scripts/ci/strix_quick_gate.sh |
Replace direct local-file checks for STRIX_LLM_FILE/LLM_API_KEY_FILE and update resolved_llm_api_base_for_model() to resolve LLM_API_BASE_FILE via the trusted resolver. |
Frontend Email Context Scoping
| Layer / File(s) | Summary |
|---|---|
Gate detection and context emission scripts/ci/strix_quick_gate.sh |
pull_request_scope_context_files() sets needs_frontend_email_api_context when specific frontend email/app/API-client/threading paths change and conditionally emits an expanded backend trusted-base file list for the email flow. |
Tests: STRIX_INPUT_FILE_ROOT, frontend-email helper, and negative cases scripts/ci/test_strix_quick_gate.sh |
Set STRIX_INPUT_FILE_ROOT in many test helpers, add run_pull_request_target_frontend_email_context_scope_case (temp repo base/head variant, fake strix, assertions), invoke it for multiple frontend entrypoints, and add negative tests for LLM/API-base and required env files outside the trusted input root plus Vertex-ignore and override-precedence cases. |
Control-flow hardening and batch handling
| Layer / File(s) | Summary |
|---|---|
Config vs scan return code propagation scripts/ci/strix_quick_gate.sh |
run_strix_once() returns 2 on configuration failures; retry and fallback callers capture and immediately propagate 2 instead of treating it as retryable scan failures. |
Batch execution and final exit handling scripts/ci/strix_quick_gate.sh |
Batch run and rebalancing capture child return codes with ` |
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
- Seongho-Bae/naruon#201: Modifies the same CI gate scoping logic and related tests for trusted-base PR context in
scripts/ci/strix_quick_gate.shandscripts/ci/test_strix_quick_gate.sh.
Poem
🐰 I guard the root where secrets sleep,
I hop through tests and paths to keep.
I mark the mail that changed today,
And trust the base to light the way.
🥕 CI hop, secure and neat!
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 6.90% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The PR title is in Korean (Hangul) and translates to 'Include backend context in Strix frontend email scan'. It directly relates to the main change: enabling Strix to scan with backend context (auth, email, config, models, threading) when frontend email files change. The title is specific and accurately reflects the primary objective. |
| Linked Issues check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | ✅ Passed | Check skipped because no linked issues were found for this pull request. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
fix/strix-frontend-email-context-20260521
Comment @coderabbitai help to get the list of available commands and usage tips.
Contributor
1 similar comment
Contributor
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/ci/test_strix_quick_gate.sh (1)
2174-2174: ⚡ Quick winInclude the changed path in the assertion labels.
All five invocations currently report the same case name, so a failure won't tell you which frontend entrypoint regressed. Deriving the label from
changed_filewould make the test output much easier to triage.♻️ Suggested tweak
run_pull_request_target_frontend_email_context_scope_case() { local changed_file="${1:?changed file is required}" + local case_name="pull-request-target-frontend-email-context:$changed_file" local tmp_dir tmp_dir="$(mktemp -d)" @@ - assert_equals "0" "$rc" "case=pull-request-target-frontend-email-context exit code" - assert_file_contains "$output_log" "scan ok with frontend email backend authorization context" "case=pull-request-target-frontend-email-context output" + assert_equals "0" "$rc" "case=$case_name exit code" + assert_file_contains "$output_log" "scan ok with frontend email backend authorization context" "case=$case_name output"Also applies to: 2317-2318
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/ci/test_strix_quick_gate.sh` at line 2174, The test assertions currently use identical labels, making failures ambiguous; update each of the five assertion/test invocation lines to incorporate the changed file path by appending or prefixing ${changed_file} to their label text so each assertion name is unique and shows which frontend entrypoint failed (use the existing variable changed_file in the label string). Locate the five invocations around the original lines (and the duplicates at the later locations around 2317-2318) and modify their label argument to include ${changed_file} while preserving the existing message content and quoting.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@scripts/ci/test_strix_quick_gate.sh`:
- Line 2174: The test assertions currently use identical labels, making failures
ambiguous; update each of the five assertion/test invocation lines to
incorporate the changed file path by appending or prefixing ${changed_file} to
their label text so each assertion name is unique and shows which frontend
entrypoint failed (use the existing variable changed_file in the label string).
Locate the five invocations around the original lines (and the duplicates at the
later locations around 2317-2318) and modify their label argument to include
${changed_file} while preserving the existing message content and quoting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f5fb5491-04f6-472e-96a8-4d59d6569d72
📒 Files selected for processing (2)
scripts/ci/strix_quick_gate.shscripts/ci/test_strix_quick_gate.sh
Contributor
Contributor
Contributor
1 similar comment
Contributor
Contributor
|
PR governance metadata gate is not ready for
|
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
Contributor
1 similar comment
Contributor
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/ci/test_strix_quick_gate.sh (1)
2194-2270:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThis helper does not actually prove the backend context stays on the trusted base.
Only
"$changed_file"differs between base and head here. Every backend context file keeps the sameBASE_*content in both commits, so this test would still pass if the gate accidentally copied those files from the PR head instead of the trusted base. Rewrite the backend context files with distinctHEAD_*markers in the head commit and assert those markers are absent in the scoped target.Also applies to: 2293-2297
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/ci/test_strix_quick_gate.sh` around lines 2194 - 2270, The test currently only checks that BASE_* markers exist in the scoped target but doesn't prove the gate didn't copy head backend files; modify the fake head commit produced by this script so each backend context file in the head contains unique HEAD_* markers (e.g., HEAD_EMAIL_API_MARKER, HEAD_AUTH_MARKER, HEAD_EMAIL_MODEL_MARKER, HEAD_CONFIG_MARKER, HEAD_ROUTER_MARKER, HEAD_THREADING_MARKER) and then add assertions that those HEAD_* markers are NOT present in the target files (backend/api/emails.py, backend/api/auth.py, backend/db/models.py, backend/core/config.py, backend/main.py, backend/services/threading_service.py) in addition to the existing BASE_* checks; update the part that writes the fake head content (the heredoc creating "$fake_strix") to include those HEAD_* markers and add grep -Fq -- checks that exit non-zero if any HEAD_* marker is found in the corresponding "$target_path/..." file so the test fails if head markers were copied into the scoped target.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/ci/strix_quick_gate.sh`:
- Around line 165-169: The eager validation block for LLM_API_BASE_FILE causes
premature exit and should be removed or guarded so Vertex models can
short-circuit; either delete the if-block that calls resolve_trusted_input_file
for LLM_API_BASE_FILE, or wrap it behind the same Vertex-model check used by
resolved_llm_api_base_for_model (i.e., only call resolve_trusted_input_file when
the chosen model is not a Vertex model), ensuring resolution is deferred to
resolved_llm_api_base_for_model so shared CI exports of LLM_API_BASE_FILE don’t
block Vertex scans.
In `@scripts/ci/test_strix_quick_gate.sh`:
- Around line 2894-2902: The test helpers call env to run
./scripts/ci/strix_quick_gate.sh but currently do not unset
STRIX_INPUT_FILE_ROOT, so exported values leak into the RUNNER_TEMP fallback
path; update the env invocations that set PATH, RUNNER_TEMP,
FAKE_STRIX_CALL_LOG, STRIX_DISABLE_PR_SCOPING, STRIX_LLM_FILE, LLM_API_KEY_FILE
and LLM_API_BASE_FILE to also include -u STRIX_INPUT_FILE_ROOT (add the flag to
both env invocations referenced around the existing env ... bash
"./scripts/ci/strix_quick_gate.sh" >"$output_log" 2>&1 blocks and the similar
block at the 2965-2973 location) so the tests properly exercise the RUNNER_TEMP
fallback.
---
Outside diff comments:
In `@scripts/ci/test_strix_quick_gate.sh`:
- Around line 2194-2270: The test currently only checks that BASE_* markers
exist in the scoped target but doesn't prove the gate didn't copy head backend
files; modify the fake head commit produced by this script so each backend
context file in the head contains unique HEAD_* markers (e.g.,
HEAD_EMAIL_API_MARKER, HEAD_AUTH_MARKER, HEAD_EMAIL_MODEL_MARKER,
HEAD_CONFIG_MARKER, HEAD_ROUTER_MARKER, HEAD_THREADING_MARKER) and then add
assertions that those HEAD_* markers are NOT present in the target files
(backend/api/emails.py, backend/api/auth.py, backend/db/models.py,
backend/core/config.py, backend/main.py, backend/services/threading_service.py)
in addition to the existing BASE_* checks; update the part that writes the fake
head content (the heredoc creating "$fake_strix") to include those HEAD_*
markers and add grep -Fq -- checks that exit non-zero if any HEAD_* marker is
found in the corresponding "$target_path/..." file so the test fails if head
markers were copied into the scoped target.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ddec5731-61b6-4a04-aaa0-bc6b36357e04
📒 Files selected for processing (2)
scripts/ci/strix_quick_gate.shscripts/ci/test_strix_quick_gate.sh
Contributor
|
PR governance metadata gate is not ready for
|
Contributor
Contributor
1 similar comment
Contributor
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/ci/strix_quick_gate.sh (1)
1727-1738:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail closed when
LLM_API_BASE_FILEcannot be read.Line 1733 uses a bare
catinside a function called viaif ! ...; in Bash, this suppressesset -efor the function body, allowing a read failure to fall through and return success on line 1738. An unreadableLLM_API_BASE_FILEis silently treated as "unset" instead of a configuration failure.Suggested fix
local llm_api_base_value - llm_api_base_value="$(cat -- "$resolved_llm_api_base_file")" + if ! llm_api_base_value="$(cat -- "$resolved_llm_api_base_file")"; then + echo "ERROR: LLM_API_BASE_FILE could not be read." >&2 + return 2 + fi llm_api_base_value="${llm_api_base_value%%/generateContent*}" llm_api_base_value="${llm_api_base_value%%:generateContent*}" llm_api_base_value="$(trim_whitespace "$llm_api_base_value")"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/ci/strix_quick_gate.sh` around lines 1727 - 1738, The read of LLM_API_BASE_FILE is currently done with a bare command substitution which hides a cat failure; change it to explicitly check the cat exit status so a missing/unreadable file fails fast: replace the bare llm_api_base_value="$(cat -- "$resolved_llm_api_base_file")" with an if ! llm_api_base_value="$(cat -- "$resolved_llm_api_base_file")"; then return 2; fi, then continue trimming (llm_api_base_value="${llm_api_base_value%%/generateContent*}" etc.) and keep the existing empty check using trim_whitespace; this ensures resolve_trusted_input_file and the subsequent cat both fail closed instead of falling through.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@scripts/ci/strix_quick_gate.sh`:
- Around line 1727-1738: The read of LLM_API_BASE_FILE is currently done with a
bare command substitution which hides a cat failure; change it to explicitly
check the cat exit status so a missing/unreadable file fails fast: replace the
bare llm_api_base_value="$(cat -- "$resolved_llm_api_base_file")" with an if !
llm_api_base_value="$(cat -- "$resolved_llm_api_base_file")"; then return 2; fi,
then continue trimming
(llm_api_base_value="${llm_api_base_value%%/generateContent*}" etc.) and keep
the existing empty check using trim_whitespace; this ensures
resolve_trusted_input_file and the subsequent cat both fail closed instead of
falling through.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 499b95ba-4b36-4503-a3ed-789af2c69cfd
📒 Files selected for processing (2)
scripts/ci/strix_quick_gate.shscripts/ci/test_strix_quick_gate.sh
Contributor
This was referenced May 21, 2026
This was referenced May 24, 2026
This was referenced May 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No linked issue.
PR 제목 (Title)
Strix 프론트엔드 이메일 스캔에 백엔드 컨텍스트 포함
목적 (Purpose)
PR #202의
pull_request_targetStrix 차단을 해소하기 위한 trusted-base 선행 수정입니다. 프론트엔드 이메일 조회 파일 변경 시 Strix가 필요한 백엔드 인증/이메일/설정/모델/라우터/스레딩 컨텍스트를 함께 보도록 합니다.주요 변경 사항 (Key Changes)
변경 범위 / 영향도 (Scope / Impacted Areas)
Tests
timeout 900 bash scripts/ci/test_strix_quick_gate.sh→test_strix_quick_gate: PASSbash -n scripts/ci/strix_quick_gate.sh scripts/ci/test_strix_quick_gate.sh→ 통과git diff --check -- scripts/ci/strix_quick_gate.sh scripts/ci/test_strix_quick_gate.sh→ 통과변경 사항 표 (Change Table: Component | Change | Notes)
scripts/ci/strix_quick_gate.shscripts/ci/test_strix_quick_gate.shSequence Diagram(s) (If behavior/flow changed, include Mermaid)
Summary by CodeRabbit