fix: reuse authoritative verification profiles - #1411
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (29)
📝 WalkthroughWalkthroughCI와 Release의 검증을 repo, spine, publish 프로필 기반 공유 매니페스트로 통합했다. 변경 분류기, 공통 디스패처, 증거 리포트, 워크플로우 계약 검증 및 provenance 검증을 추가·갱신했다. Changes검증 프로필 통합
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow as CI 또는 Release workflow
participant Classifier as verification-change-classifier.mts
participant Evidence as release-spine-evidence.mts
participant Manifest as verification-manifest.mts
Workflow->>Classifier: 이벤트와 변경 경로 전달
Classifier-->>Workflow: profile 및 실행 플래그 반환
Workflow->>Evidence: profile, base, head 전달
Evidence->>Manifest: createVerificationManifest(profile, context)
Manifest-->>Evidence: EvidenceCommand 목록 반환
Evidence-->>Workflow: 검증 요약과 아티팩트 게시
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
📊 Benchmark Results✅ All benchmarks passed
Updated: 2026-07-21T13:09:32.777Z · Commit: b482c82 |
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 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 @.github/workflows/ci.yml:
- Line 25: Pin the GitHub Actions references to verified full immutable commit
SHAs: update actions/checkout in .github/workflows/ci.yml (25-25),
actions/upload-artifact in .github/workflows/ci.yml (191-191), pnpm/action-setup
in .github/workflows/release.yml (47-47), and actions/setup-node in
.github/workflows/release.yml (51-51), preserving their current major-version
behavior.
- Around line 29-43: Update the Select verification profile step so the
classifier receives the actual GitHub event instead of always using the
pull_request event. Replace the fixed event argument in the invocation of
verification-change-classifier.mts with the existing github.event_name value,
while preserving the current base and head arguments.
In @.github/workflows/release.yml:
- Around line 38-43: Release publish 프로필에서 계산한 base revision을 다음 단계가 사용할 수 있도록
step output으로 내보내세요. 해당 base 값을 GitHub Actions output에 설정한 뒤 verify:publish 단계의
revision 인자에 base와 head(HEAD)를 함께 전달하도록, verification-change-classifier 호출과 관련
step 설정을 업데이트하세요.
In `@package.json`:
- Line 47: production-ready:check 별칭이 production-ready의 공유 디스패처를 사용하도록 변경하세요.
scripts/production-ready-check.mts를 직접 실행하는 경로를 제거하고, 기존 production-ready 정의에
포함된 tracked-file mutation guard와 동일한 디스패처를 재사용하세요.
In `@scripts/release-spine-evidence.mts`:
- Around line 1016-1040: Update the argument-validation branches in the CLI
parser around --profile, --base/--head, and unknown options to throw an existing
argument-validation Problem subtype or a dedicated Problem subtype instead of
generic Error instances. Preserve the current validation conditions and messages
while ensuring every failure follows the RFC 7807 code/category contract.
- Around line 9-14: Update the imports in release-spine-evidence to move
VerificationProfile into its own import type declaration, while keeping
createVerificationManifest in the existing value import.
In `@scripts/tests/ci-workflow.spec.ts`:
- Around line 7-9: Rename the module-level constants rootDir, workflow, and
validateJob to SCREAMING_SNAKE_CASE names, updating all references while
preserving their existing values and behavior.
In `@scripts/tests/verification-change-classifier.spec.ts`:
- Line 1: Move scripts/tests/verification-change-classifier.spec.ts to
src/tests/VerificationChangeClassifier.spec.ts and update its relative imports;
move scripts/tests/ci-workflow.spec.ts to src/tests/CiWorkflow.spec.ts and
update its path calculation; move scripts/tests/provenance-config-check.spec.ts
to src/tests/ProvenanceConfigCheck.spec.ts and update its relative imports.
Ensure all three tests follow the required src/tests/[ClassName].spec.ts layout.
In `@scripts/verification-change-classifier.mts`:
- Line 97: Replace the generic Error throws with dedicated RFC 7807 Problem
subtypes that provide both code and category. Update the unclassified-path
handling at scripts/verification-change-classifier.mts lines 97-97, the CLI
option validation at scripts/verification-change-classifier.mts lines 177-177,
and the provenance configuration validation at
scripts/provenance-config-check.mts lines 15-20; preserve each existing error
condition and message context.
- Around line 7-10: Separate the VerificationProfile type import from the
runtime import in the module’s import section. Keep
verificationImplementationPaths in the existing value import and add a distinct
import type declaration for VerificationProfile, following the project’s
separate type-import grouping convention.
- Around line 24-25: Rename the module-level constants profileStrength and
manifestImplementationPaths to SCREAMING_SNAKE_CASE names, and update every
reference to them in scripts/verification-change-classifier.mts while preserving
their existing values and behavior.
- Around line 39-43: Update the repository-document classification regex used by
the verification change classifier so only exact README.md, RELEASING.md,
CONTRIBUTING.md, AGENTS.md, LICENSE, docs/ paths, and package-level README.md
paths match; require the relevant path boundaries or end-of-string so suffixes
such as README.md.ts and README.md.backup remain classified as spine.
In `@scripts/verification-manifest.mts`:
- Line 552: Replace each generic Error construction in
scripts/verification-manifest.mts lines 552-552, 558-558, and 587-587, and
scripts/verification-command.mts lines 31-31 and 42-42 with the appropriate
existing Problem subclass or RFC 7807-compatible custom error, preserving each
message and validation behavior.
- Line 540: Rename the module-level constants to SCREAMING_SNAKE_CASE: in
scripts/verification-manifest.mts:230-230 rename spineOnly to SPINE_ONLY,
516-516 rename publishOnly to PUBLISH_ONLY, and 540-540 rename
prohibitedRootAliases to PROHIBITED_ROOT_ALIASES, updating all references. In
scripts/workflow-verification-contract.mts:14-14 rename
actionsOnlyWorkflowCommandAllowlist to ACTIONS_ONLY_WORKFLOW_COMMAND_ALLOWLIST
and update its default-parameter reference at line 129.
🪄 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: aa8c7736-dac6-4d58-8575-567a1ffb6b59
📒 Files selected for processing (18)
.github/workflows/ci.yml.github/workflows/release.ymlpackage.jsonscripts/provenance-config-check.mtsscripts/release-spine-evidence.mtsscripts/tests/ci-workflow.spec.tsscripts/tests/provenance-config-check.spec.tsscripts/tests/release-spine-evidence.spec.tsscripts/tests/release-workflow.spec.tsscripts/tests/verification-change-classifier.spec.tsscripts/tests/verification-command.spec.tsscripts/tests/verification-manifest.spec.tsscripts/tests/verification-policy.spec.tsscripts/verification-change-classifier.mtsscripts/verification-command.mtsscripts/verification-manifest.mtsscripts/verification-policy.mtsscripts/workflow-verification-contract.mts
95d1c43 to
bc1d7a0
Compare
|
@coderabbitai resolve |
✅ Action performedComments resolved and changes approved. |
7e84d06 to
6307732
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/ci.yml (1)
1-1: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win두 워크플로우 모두 GitHub 컨텍스트 표현식을 셸에 직접 보간합니다 (template-injection).
${{ github.event_name }},${{ github.base_ref }},${{ github.event.before }}를run:스크립트에 직접 삽입하는 동일한 패턴이 반복됩니다. zizmor의 표준 권장 완화책은 값을env:로 전달한 뒤 셸 변수로 참조하는 것입니다.
.github/workflows/ci.yml#L29-44:env:에EVENT_NAME: ${{ github.event_name }},BASE_REF: ${{ github.base_ref }},EVENT_BEFORE: ${{ github.event.before }}를 선언하고 스크립트 내부는"$EVENT_NAME","$BASE_REF","$EVENT_BEFORE"로 참조하도록 변경하세요..github/workflows/release.yml#L33-47: 동일하게EVENT_BEFORE: ${{ github.event.before }}를env:로 선언하고base="${{ github.event.before }}"를base="$EVENT_BEFORE"로 변경하세요.🔒 제안 수정 (ci.yml)
- name: Select verification profile id: verification shell: bash + env: + EVENT_NAME: ${{ github.event_name }} + BASE_REF: ${{ github.base_ref }} + EVENT_BEFORE: ${{ github.event.before }} run: | - if [ "${{ github.event_name }}" = "pull_request" ]; then - base="origin/${{ github.base_ref }}" + if [ "$EVENT_NAME" = "pull_request" ]; then + base="origin/$BASE_REF" else - base="${{ github.event.before }}" + base="$EVENT_BEFORE" if [ -z "$base" ] || ! git cat-file -e "$base^{commit}" 2>/dev/null; then base="HEAD^" fi fi node --experimental-strip-types scripts/verification-change-classifier.mts \ --event "$GITHUB_EVENT_NAME" --workflow ci --base "$base" --head HEAD --github-output "$GITHUB_OUTPUT" echo "base=$base" >> "$GITHUB_OUTPUT"🤖 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 @.github/workflows/ci.yml at line 1, Update the workflow steps containing the run scripts to pass github.event_name, github.base_ref, and github.event.before through step-level env variables named EVENT_NAME, BASE_REF, and EVENT_BEFORE, then reference those quoted shell variables instead of interpolating GitHub expressions directly. Apply the same EVENT_BEFORE env-and-shell-variable change to the corresponding release workflow step.Source: Linters/SAST tools
scripts/release-spine-evidence.mts (1)
746-820: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
dashboardStatus가 매니페스트 명령 순서에 암묵적으로 의존합니다.
spine-bundle-size/spine-promotion에 주입되는PACKAGE_QUALITY_*_STATUS값들은lint,format,build,typecheck,test,provider-certification,production-ready체크가 이 체크보다 먼저 실행 완료되어 있어야 정확합니다. 이 순서 보장은verification-manifest.mts의 배열 구성 순서에만 의존하며, 코드상 명시적인 assert나 주석이 없어 향후 매니페스트가 재배열되면 조용히 잘못된 상태값(예: 항상"skipped")이 주입될 수 있습니다.
assertVerificationManifest(또는 이 파일 내부)에 "이 ID들은spine-bundle-size보다 먼저 나와야 한다"는 검증을 추가하거나, 최소한 주석으로 이 불변조건을 명시하는 것을 권장합니다.🤖 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/release-spine-evidence.mts` around lines 746 - 820, Make the ordering dependency explicit for dashboardStatus and the PACKAGE_QUALITY_*_STATUS values used by spine-bundle-size and spine-promotion. Add validation in assertVerificationManifest, or an equivalent check in this file, ensuring lint, format, build, typecheck, test, provider-certification, and production-ready appear before spine-bundle-size; document the invariant with a concise comment if validation is not added.
🤖 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 @.github/workflows/ci.yml:
- Around line 20-27: Update the Checkout step using actions/checkout@v4 to set
persist-credentials to false alongside fetch-depth: 0, ensuring checkout
credentials are not retained in the local Git configuration.
In `@scripts/core-coverage-warning-check.mts`:
- Around line 140-148: 중복된 verification-command.mts 디스패처 정규식 및 실제 명령 조회 로직을 공유
헬퍼로 추출하고, scripts/core-coverage-warning-check.mts의
resolveCoreCoveragePackageFilters와 scripts/first-success-verify.mts의
runsVerificationScript가 모두 이를 사용하도록 변경하세요. 헬퍼는 기존 디스패처 형식을 동일하게 처리하고,
runsVerificationScript에 필요한 정확 매칭 동작은 옵션으로 지원하세요.
- Around line 149-156: Replace the generic Error throws in the package-filter
failure branches of the core coverage parsing function with VerificationProblem
from ./verification-problem.mts, preserving the existing failure message and
ensuring both branches throw a Problem subclass.
In `@scripts/provenance-config-check.mts`:
- Around line 40-47: Update the main-module check around verifyProvenanceConfig
to use pathToFileURL(process.argv[1] ?? "").href instead of new URL(...,
"file:").href, matching the established release-spine-evidence pattern and
handling special characters and Windows paths consistently.
In `@scripts/release-spine-evidence.mts`:
- Around line 183-196: Update readChangedFiles to catch failures from
execFileSync, including missing base/head refs in cold or shallow clones, and
return undefined instead of propagating the exception. Verify that
runReleaseSpineEvidence and manifest applicability handling treat undefined as
“changed files unavailable” and still produce the expected report safely.
In `@scripts/verification-change-classifier.mts`:
- Line 241: Update the entry-point check in the verification-change-classifier
module to use pathToFileURL(argv[1] ?? "").href, matching the established
pattern in verification-command, instead of constructing the URL with new
URL(..., "file:").
---
Outside diff comments:
In @.github/workflows/ci.yml:
- Line 1: Update the workflow steps containing the run scripts to pass
github.event_name, github.base_ref, and github.event.before through step-level
env variables named EVENT_NAME, BASE_REF, and EVENT_BEFORE, then reference those
quoted shell variables instead of interpolating GitHub expressions directly.
Apply the same EVENT_BEFORE env-and-shell-variable change to the corresponding
release workflow step.
In `@scripts/release-spine-evidence.mts`:
- Around line 746-820: Make the ordering dependency explicit for dashboardStatus
and the PACKAGE_QUALITY_*_STATUS values used by spine-bundle-size and
spine-promotion. Add validation in assertVerificationManifest, or an equivalent
check in this file, ensuring lint, format, build, typecheck, test,
provider-certification, and production-ready appear before spine-bundle-size;
document the invariant with a concise comment if validation is not added.
🪄 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: 96c19349-40a7-46a4-bf38-9ab85339a4ae
📒 Files selected for processing (23)
.github/workflows/ci.yml.github/workflows/release.ymlpackage.jsonscripts/core-coverage-warning-check.mtsscripts/first-success-verify.mtsscripts/provenance-config-check.mtsscripts/release-spine-evidence.mtsscripts/tests/ci-workflow.spec.tsscripts/tests/core-coverage-warning-check.spec.tsscripts/tests/first-success-verify.spec.tsscripts/tests/provenance-config-check.spec.tsscripts/tests/release-spine-evidence.spec.tsscripts/tests/release-workflow.spec.tsscripts/tests/verification-change-classifier.spec.tsscripts/tests/verification-command.spec.tsscripts/tests/verification-manifest.spec.tsscripts/tests/verification-policy.spec.tsscripts/verification-change-classifier.mtsscripts/verification-command.mtsscripts/verification-manifest.mtsscripts/verification-policy.mtsscripts/verification-problem.mtsscripts/workflow-verification-contract.mts
b87edbf to
fc15d82
Compare
fc15d82 to
8383418
Compare
8383418 to
e14c5ba
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
Outcome
Repository, spine, and publish verification now share one typed command manifest with stable command IDs. CI selects exactly one path-aware profile, while Release reruns the complete publish profile as defense in depth; credentials, advisories, summaries, uploads, provenance authority, and Changesets publishing remain Actions-owned.
The architecture-policy runtime prerequisite is explicit in the manifest: a cold clone builds
@croco/problems-corebefore running the architecture policy. Mutation-prone commands are guarded at their authoritative definitions, compatibility aliases resolve through shared command IDs, and workflow contracts reject direct, aliased, conditional, or Release-only verification drift.Verification
pnpm test(226/226 tasks)pnpm typecheck(225/225 tasks)Review notes
repo ⊂ spine ⊂ publish; Release invokes publish rather than duplicating manifest members.Fixes #1333
Summary by CodeRabbit