fix: retain generated smoke failure artifacts - #1302
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 46 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: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthrough생성 앱 스모크 실행이 선택 계획과 구조화된 명령 결과를 사용하도록 변경되었다. 실패 시 출력, 진단 코드, 아티팩트, 복구 요약을 케이스별로 기록하며 REST SPA 계약 스모크와 매트릭스 리포트 검증이 추가되었다. Changes생성 앱 스모크 리포팅
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant SmokeRunner as 생성 스모크 실행기
participant CommandRunner as runSmokeCaseCommand
participant Executor as executeCommand
participant FailureReporter as recordSmokeCaseFailure
participant MatrixReport as 매트릭스 리포트
SmokeRunner->>CommandRunner: 선택된 케이스 명령 실행
CommandRunner->>Executor: stdout/stderr 캡처 요청
Executor-->>CommandRunner: CommandRunResult 반환
CommandRunner->>FailureReporter: 실패 결과와 projectDir 전달
FailureReporter->>MatrixReport: failureEvidence 기록
MatrixReport-->>SmokeRunner: 복구 정보와 아티팩트 경로 출력
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 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 |
e198609 to
bd4135e
Compare
📊 Benchmark Results✅ All benchmarks passed
Updated: 2026-07-10T23:12:39.260Z · Commit: f1d4e56 |
bd4135e to
d5c3145
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
scripts/create-croco-app-generated-smoke.mts (2)
1514-1575: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win동일한 실패 처리 try/catch 패턴이 여러 함수에 반복됩니다.
runSmokeCaseCommand,runExpectedSmokeCaseCommand(여기), 그리고runValidation(Line 2006-2013),runGraphQLContractDriftCanaries(Line 2078-2085)에 걸쳐 "getCommandResultFromError→appendSmokeCaseOutput→recordSmokeCaseFailure→createSmokeFailureError던지기" 흐름이 거의 동일하게 4곳에 중복되어 있습니다. 공통 헬퍼로 추출하면 향후 실패 처리 로직을 수정할 때 누락 위험을 줄일 수 있습니다.♻️ 제안: 공통 실패 처리 헬퍼 추출
function withSmokeStepFailureHandling<T extends CommandRunResult>( report: GeneratedSmokeReport, caseResult: SmokeCaseResult, step: SmokeStepResult, projectDir: string, execute: () => T, ): T { try { return execute(); } catch (error) { const commandResult = getCommandResultFromError(error); if (commandResult) { appendSmokeCaseOutput(caseResult, step.label, commandResult); } recordSmokeCaseFailure(report, caseResult, step, error, projectDir); throw createSmokeFailureError(caseResult, step, error); } }🤖 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/create-croco-app-generated-smoke.mts` around lines 1514 - 1575, 중복된 명령 실패 처리 로직을 공통 헬퍼로 추출하세요. `withSmokeStepFailureHandling`을 추가해 `getCommandResultFromError`부터 출력 기록, `recordSmokeCaseFailure`, `createSmokeFailureError` 재던지기까지 캡슐화하고, `runSmokeCaseCommand`, `runExpectedSmokeCaseCommand`, `runValidation`, `runGraphQLContractDriftCanaries`의 try/catch를 해당 헬퍼 호출로 교체하세요.
2046-2114: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGraphQL 드리프트 카나리 중 일부만 실패해도 앞선 성공 카나리의 출력이 버퍼에서 누락됩니다.
commandResults배열은 3개의runGraphQLSnapshotCanary호출이 모두 성공해야만 완성되고, 그 뒤에야for루프에서appendSmokeCaseOutput이 호출됩니다. 두 번째 또는 세 번째 카나리가throw하면 배열 리터럴 평가가 중단되어, 이미 성공적으로 실행된 이전 카나리의 stdout/stderr는smokeCaseOutputBuffers에 전혀 기록되지 않습니다. catch 블록은 오직 실패를 유발한 카나리 자신의commandResult만 복구합니다.이는 "실패 케이스마다 stdout/stderr를 보존한다"는 PR의 핵심 목표를 이 스텝에서 부분적으로 무력화합니다. 각 카나리 실행 직후 즉시
appendSmokeCaseOutput을 호출하도록 바꿔야 앞선 카나리의 출력도 항상 보존됩니다.🐛 제안: 카나리 실행 직후 즉시 출력 캡처
- const commandResults = [ - runGraphQLSnapshotCanary( - packageDir, - snapshotPath, - originalSnapshot, - withStaleGraphQLOperationBaseline, - ["graphql-operation-removed"], - ), - runGraphQLSnapshotCanary( - packageDir, - snapshotPath, - originalSnapshot, - withChangedGraphQLFieldTypeBaseline, - ["graphql-schema-breaking-change"], - ), - runGraphQLSnapshotCanary( - packageDir, - snapshotPath, - originalSnapshot, - withGraphQLResolverMetadataDriftBaseline, - GRAPHQL_RESOLVER_METADATA_DRIFT_CODES, - ), - ]; - - for (const result of commandResults) { - appendSmokeCaseOutput(caseResult, step.label, result); - } + const canaryScenarios: ReadonlyArray<{ + readonly mutate: (snapshot: GraphQLContractSnapshotJson) => GraphQLContractSnapshotJson; + readonly expectedDiagnosticCodes: readonly string[]; + }> = [ + { mutate: withStaleGraphQLOperationBaseline, expectedDiagnosticCodes: ["graphql-operation-removed"] }, + { mutate: withChangedGraphQLFieldTypeBaseline, expectedDiagnosticCodes: ["graphql-schema-breaking-change"] }, + { mutate: withGraphQLResolverMetadataDriftBaseline, expectedDiagnosticCodes: GRAPHQL_RESOLVER_METADATA_DRIFT_CODES }, + ]; + const commandResults = canaryScenarios.map(({ mutate, expectedDiagnosticCodes }) => { + const result = runGraphQLSnapshotCanary( + packageDir, + snapshotPath, + originalSnapshot, + mutate, + expectedDiagnosticCodes, + ); + appendSmokeCaseOutput(caseResult, step.label, result); + return result; + });🤖 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/create-croco-app-generated-smoke.mts` around lines 2046 - 2114, Update the GraphQL drift canary orchestration so each run’s output is appended immediately after that canary completes, rather than collecting results in a single array first. Refactor the three calls in the step using a sequential loop or equivalent helper that invokes runGraphQLSnapshotCanary and then appendSmokeCaseOutput for each result, while preserving diagnostic code aggregation and existing failure handling in the surrounding try/catch.
🤖 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/create-croco-app-generated-smoke-report.mts`:
- Around line 308-343: Refactor shouldIncludeSmokeFailureArtifact to remove the
hardcoded rest-spa-contracts path exception. Define a
caseName-to-allowed-relative-paths map or equivalent configuration near the
helper, then check normalizedPath against the paths configured for the current
case, preserving the existing global artifact rules and .croco handling. Use the
map so future case-specific source files can be added without modifying the
function’s conditional logic.
In `@scripts/create-croco-app-generated-smoke.mts`:
- Around line 1240-1251: 중복된 최상위 케이스 실패 처리 로직을 공통 헬퍼로 추출하세요. 현재 catch 블록과
runSpaBeSplitContractSmoke의 동일한 caseResult.status 검사, commandResult 기록, fallback
스텝 생성 및 recordSmokeCaseFailure 호출을 recordUnhandledCaseFailure(report,
caseResult, projectDir, error) 같은 함수로 통합하고, 두 위치에서 해당 헬퍼를 호출하도록 수정하세요.
- Around line 1637-1652: appendSmokeCaseOutput에서 케이스별 stdout과 stderr 누적 크기에 전체
상한을 적용하세요. commandCaptureMaxBytes와 별도로 모든 스텝의 출력 합계가 설계된 최대 크기를 넘지 않도록 남은 용량만
추가하고 초과분은 잘라내며 output.outputTruncated를 true로 설정하세요.
---
Outside diff comments:
In `@scripts/create-croco-app-generated-smoke.mts`:
- Around line 1514-1575: 중복된 명령 실패 처리 로직을 공통 헬퍼로 추출하세요.
`withSmokeStepFailureHandling`을 추가해 `getCommandResultFromError`부터 출력 기록,
`recordSmokeCaseFailure`, `createSmokeFailureError` 재던지기까지 캡슐화하고,
`runSmokeCaseCommand`, `runExpectedSmokeCaseCommand`, `runValidation`,
`runGraphQLContractDriftCanaries`의 try/catch를 해당 헬퍼 호출로 교체하세요.
- Around line 2046-2114: Update the GraphQL drift canary orchestration so each
run’s output is appended immediately after that canary completes, rather than
collecting results in a single array first. Refactor the three calls in the step
using a sequential loop or equivalent helper that invokes
runGraphQLSnapshotCanary and then appendSmokeCaseOutput for each result, while
preserving diagnostic code aggregation and existing failure handling in the
surrounding try/catch.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 555dfbb4-b8fa-4abf-826b-1a6adc65b9f4
📒 Files selected for processing (3)
scripts/create-croco-app-generated-smoke-report.mtsscripts/create-croco-app-generated-smoke.mtsscripts/tests/create-croco-app-generated-smoke.spec.ts
d5c3145 to
bbaaf61
Compare
35006c9 to
4db0801
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/create-croco-app-generated-smoke-matrix.mts`:
- Around line 77-183: 중복된 "rest-spa-contracts" 매직 스트링을 단일 공유 상수로 통합하세요. 케이스 정의를
소유한 GENERATED_SMOKE_MATRIX_CASES 파일에서 REST_SPA_CONTRACT_SMOKE_CASE_NAME을
export하고, create-croco-app-generated-smoke.mts의 기존 상수 및
create-croco-app-generated-smoke-report.mts의
caseSpecificSmokeFailureArtifactPaths 키가 해당 상수를 import해 사용하도록 변경해 이름 변경 시 모든 참조가
함께 검증되게 하세요.
In `@scripts/create-croco-app-generated-smoke.mts`:
- Around line 3230-3270: Update executeCommand so stdout and stderr are streamed
to the CI console while the command runs, while still being captured for
readCappedCommandOutput and toCommandRunResult; replace the current
spawnSync-only file-descriptor wiring with an appropriate spawn-based
implementation or otherwise preserve incremental progress output for successful
long-running commands.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 04336dca-d87f-4fee-8a04-5abd138b855a
📒 Files selected for processing (5)
.github/workflows/ci.ymlscripts/create-croco-app-generated-smoke-matrix.mtsscripts/create-croco-app-generated-smoke-report.mtsscripts/create-croco-app-generated-smoke.mtsscripts/tests/create-croco-app-generated-smoke.spec.ts
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
🤖 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/create-croco-app-generated-smoke-matrix.mts`:
- Around line 77-183: 중복된 "rest-spa-contracts" 매직 스트링을 단일 공유 상수로 통합하세요. 케이스 정의를
소유한 GENERATED_SMOKE_MATRIX_CASES 파일에서 REST_SPA_CONTRACT_SMOKE_CASE_NAME을
export하고, create-croco-app-generated-smoke.mts의 기존 상수 및
create-croco-app-generated-smoke-report.mts의
caseSpecificSmokeFailureArtifactPaths 키가 해당 상수를 import해 사용하도록 변경해 이름 변경 시 모든 참조가
함께 검증되게 하세요.
In `@scripts/create-croco-app-generated-smoke.mts`:
- Around line 3230-3270: Update executeCommand so stdout and stderr are streamed
to the CI console while the command runs, while still being captured for
readCappedCommandOutput and toCommandRunResult; replace the current
spawnSync-only file-descriptor wiring with an appropriate spawn-based
implementation or otherwise preserve incremental progress output for successful
long-running commands.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 04336dca-d87f-4fee-8a04-5abd138b855a
📒 Files selected for processing (5)
.github/workflows/ci.ymlscripts/create-croco-app-generated-smoke-matrix.mtsscripts/create-croco-app-generated-smoke-report.mtsscripts/create-croco-app-generated-smoke.mtsscripts/tests/create-croco-app-generated-smoke.spec.ts
🛑 Comments failed to post (2)
scripts/create-croco-app-generated-smoke-matrix.mts (1)
77-183: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
"rest-spa-contracts" 케이스 이름이 3개 파일에 매직 스트링으로 중복됩니다.
이 배열의 182번째 줄(
"rest-spa-contracts")은scripts/create-croco-app-generated-smoke.mts의REST_SPA_CONTRACT_SMOKE_CASE_NAME상수(Line 246, 1177)와scripts/create-croco-app-generated-smoke-report.mts의caseSpecificSmokeFailureArtifactPaths키(Line 147)와 동일한 리터럴을 독립적으로 반복합니다. 세 곳 중 한 곳에서 케이스 이름이 바뀌면 나머지는 컴파일 오류 없이 조용히 어긋나게 됩니다(예:assertGeneratedSmokeMatrixContract는 통과하지만 아티팩트 허용목록만 무효화됨).♻️ 제안: 공유 상수를 하나의 소스로 통일
- { name: "rest-spa-contracts", tier: "spine-blocking" }, + { name: REST_SPA_CONTRACT_SMOKE_CASE_NAME, tier: "spine-blocking" },
REST_SPA_CONTRACT_SMOKE_CASE_NAME을 이 파일(또는create-croco-app-generated-smoke-report.mts)에서 export하고,create-croco-app-generated-smoke.mts와create-croco-app-generated-smoke-report.mts가 이를 import하도록 정리하세요. import 순환을 피하려면 상수를 case 정의를 소유한 이 파일에 두는 것이 자연스럽습니다.🤖 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/create-croco-app-generated-smoke-matrix.mts` around lines 77 - 183, 중복된 "rest-spa-contracts" 매직 스트링을 단일 공유 상수로 통합하세요. 케이스 정의를 소유한 GENERATED_SMOKE_MATRIX_CASES 파일에서 REST_SPA_CONTRACT_SMOKE_CASE_NAME을 export하고, create-croco-app-generated-smoke.mts의 기존 상수 및 create-croco-app-generated-smoke-report.mts의 caseSpecificSmokeFailureArtifactPaths 키가 해당 상수를 import해 사용하도록 변경해 이름 변경 시 모든 참조가 함께 검증되게 하세요.scripts/create-croco-app-generated-smoke.mts (1)
3230-3270: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the current file around executeCommand and nearby call sites. sed -n '3200,3295p' scripts/create-croco-app-generated-smoke.mts echo '--- PREVIOUS VERSION (if available) ---' # Show the same region from the parent revision, if the repository has one. if git rev-parse --verify HEAD^ >/dev/null 2>&1; then git show HEAD^:scripts/create-croco-app-generated-smoke.mts | sed -n '3200,3295p' fiRepository: croco-dev/framework
Length of output: 149
🏁 Script executed:
#!/bin/bash set -euo pipefail # Compare the exact stdio behavior across the current and parent revision, if available. python3 - <<'PY' import subprocess, os, textwrap path = "scripts/create-croco-app-generated-smoke.mts" def show(ref): try: out = subprocess.check_output(["git", "show", f"{ref}:{path}"], text=True, stderr=subprocess.STDOUT) return out except subprocess.CalledProcessError as e: return None for ref in ["HEAD", "HEAD^"]: data = show(ref) if data is None: print(f"[{ref}] unavailable") continue for needle in ["stdio:", "spawnSync(", "inherit", "pipe", "ignore"]: pass # Print only the executeCommand block and any stdio line. lines = data.splitlines() start = None for i, line in enumerate(lines): if line.startswith("function executeCommand("): start = i break if start is not None: for line in lines[start:start+45]: print(line) print("-----") PYRepository: croco-dev/framework
Length of output: 149
🏁 Script executed:
#!/bin/bash set -euo pipefail # Find any mention of replayCommandOutput and whether command stdout/stderr is ever echoed during success. rg -n "replayCommandOutput|toCommandRunResult|spawnSync\(|stdio:" scripts/create-croco-app-generated-smoke.mtsRepository: croco-dev/framework
Length of output: 682
🏁 Script executed:
#!/bin/bash set -euo pipefail # Read only the relevant small ranges around the command execution and replay helpers. sed -n '3160,3345p' scripts/create-croco-app-generated-smoke.mtsRepository: croco-dev/framework
Length of output: 5260
🏁 Script executed:
#!/bin/bash set -euo pipefail # Check whether this file ever streams subprocess output directly to the terminal elsewhere. rg -n "stdio:\s*\[|stdio:\s*'inherit'|stdio:\s*\"inherit\"|process\.stdout|process\.stderr|console\.(log|error)\(" scripts/create-croco-app-generated-smoke.mtsRepository: croco-dev/framework
Length of output: 1221
🏁 Script executed:
#!/bin/bash set -euo pipefail # Show only the diff hunk that touches executeCommand and stdio, with minimal context. git diff --unified=3 HEAD^ HEAD -- scripts/create-croco-app-generated-smoke.mts | sed -n '/function executeCommand(/,/function readCappedCommandOutput(/p'Repository: croco-dev/framework
Length of output: 1460
🏁 Script executed:
#!/bin/bash set -euo pipefail # If the previous command is too broad, show just the changed lines near the stdio capture. git diff --unified=0 HEAD^ HEAD -- scripts/create-croco-app-generated-smoke.mts | rg -n -C 3 "spawnSync|stdio|executeCommand|replayCommandOutput"Repository: croco-dev/framework
Length of output: 3966
명령 출력이 성공 시에도 즉시 보이도록 복원해야 합니다.
spawnSync를 파일 디스크립터로만 연결하면 장시간 실행되는 성공 스텝의 stdout/stderr가 종료 전까지 CI 로그에 전혀 나타나지 않습니다. 콘솔과 파일에 동시에 스트리밍하도록spawn기반으로 바꾸거나, 최소한 진행 상황이 남는 방식이 필요합니다.🤖 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/create-croco-app-generated-smoke.mts` around lines 3230 - 3270, Update executeCommand so stdout and stderr are streamed to the CI console while the command runs, while still being captured for readCappedCommandOutput and toCommandRunResult; replace the current spawnSync-only file-descriptor wiring with an appropriate spawn-based implementation or otherwise preserve incremental progress output for successful long-running commands.
|
CodeRabbit follow-up:
|
4db0801 to
5c903d2
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/create-croco-app-generated-smoke.mts`:
- Around line 3302-3310: Update readCommandOutputSegment to loop readSync calls
until the requested length is filled or no more bytes are available, tracking
the current offset and file position. Decode the accumulated bytes with a
streaming-safe UTF-8 approach so multibyte characters split across reads are
preserved and do not produce replacement characters.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 549a4626-5805-47a6-9758-17fe7e63ab79
📒 Files selected for processing (5)
.github/workflows/ci.ymlscripts/create-croco-app-generated-smoke-matrix.mtsscripts/create-croco-app-generated-smoke-report.mtsscripts/create-croco-app-generated-smoke.mtsscripts/tests/create-croco-app-generated-smoke.spec.ts
5c903d2 to
ffd3a89
Compare
Fixes #1241.
Summary
Verification
pnpm vitest run scripts/tests/create-croco-app-generated-smoke.spec.tspassed (16 tests).pnpm create-croco-app:smoke rest-spa-contractspassed, including shared gates and REST contract assertions.pnpm create-croco-app:smoke graphql-standalone-apipassed, including GraphQL drift canaries.pnpm testandpnpm typecheckpassed again in the pre-push hook (225 and 224 tasks).pnpm checkpassed. Direct TypeScript validation of all changed smoke scripts, YAML parsing of the CI workflow, andgit diff HEAD --checkpassed.Self-review
Summary by CodeRabbit