fix: generate deterministic DI graph manifests - #1218
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDI 그래프 생성 명령, 진단 코드 표준화, usage dashboard 경로 오류 정리, 생성 앱 템플릿 배선, 그리고 관련 문서·테스트·스모크 검증이 함께 변경되었습니다. ChangesDI 그래프와 진단 표준화
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CROCO as croco di graph
participant FS as File System
participant FrameworkContext
User->>CROCO: 실행(--module, --bootstrap, --roots)
CROCO->>CROCO: parseDiGraphArgs()
CROCO->>FrameworkContext: load module / bootstrap / roots
FrameworkContext-->>CROCO: Container or framework context
CROCO->>FS: write di-graph.manifest.json
CROCO-->>User: diagnostics / exitCode
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c667036571
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
📊 Benchmark Results✅ All benchmarks passed
Updated: 2026-07-05T01:39:29.531Z · Commit: 153ddc2 |
c667036 to
ddc15a5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddc15a54c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
6bef31e to
32be5b3
Compare
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/cli/src/commands/diCheck.ts (1)
221-238: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win도달 불가한
legacyCode분기 제거
(stableCode && rawCode ? rawCode : undefined)는 앞선 두??분기와 역할이 겹쳐 실제로 도달하지 않습니다.CLI_LEGACY_DIAGNOSTIC_CODES값도 모두 기존 prefix 규칙을 따르므로, 이 분기를 제거해 로직을 단순화할 수 있습니다.♻️ 제안
const legacyCode = (isUnmappedCliLegacyCode || isMappedDiGraphLegacyCode ? rawCode : rawLegacyCode) ?? - (stableCode && rawCode ? rawCode : undefined) ?? (code === CLI_DIAGNOSTIC_CODES.diCheckDiagnosticUnknown ? CLI_LEGACY_DIAGNOSTIC_CODES.diCheckDiagnosticUnknown : undefined);🤖 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 `@packages/cli/src/commands/diCheck.ts` around lines 221 - 238, The legacyCode computation in diCheck currently contains an unreachable fallback branch because the earlier isUnmappedCliLegacyCode/isMappedDiGraphLegacyCode and stableCode checks already cover the same cases. Simplify the logic in diCheck by removing the redundant “stableCode && rawCode” fallback and keep only the necessary rawCode/rawLegacyCode and unknown-code handling, while preserving the existing behavior for CLI_DIAGNOSTIC_CODES and CLI_LEGACY_DIAGNOSTIC_CODES.packages/framework-context/src/libs/Container.ts (2)
369-378: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
compareTokenDescriptions도입 후에도 동일한 정렬 로직이 중복됨.
createGraphProviders의 최종 정렬(374-377)은 새로 추가된compareTokenDescriptions(505-518)와 동일하게 "1차 label/token 비교 → 동률 시 id 비교" 패턴을 인라인으로 재구현하고 있습니다. 동일 파일 내 새 헬퍼를 도입한 시점에 이 부분도 함께 통합했다면 향후 비교 기준이 어긋날 위험(예: 한쪽만 수정되는 경우)을 줄일 수 있었습니다.♻️ 제안: compareTokenDescriptions 재사용
return Array.from(providers.values()) .map((provider) => ({ ...provider, ...Container.sortProviderDependencies(provider.dependencies), })) - .sort((left, right) => { - const tokenOrder = left.token.localeCompare(right.token); - return tokenOrder === 0 ? left.tokenId.localeCompare(right.tokenId) : tokenOrder; - }); + .sort((left, right) => + Container.compareTokenDescriptions( + { label: left.token, id: left.tokenId }, + { label: right.token, id: right.tokenId }, + ), + );🤖 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 `@packages/framework-context/src/libs/Container.ts` around lines 369 - 378, `createGraphProviders`의 최종 정렬 로직이 `compareTokenDescriptions`와 동일한 비교 규칙을 인라인으로 կրկն복하고 있습니다. `createGraphProviders`의 `.sort(...)`에서 `left.token`/`right.token` 및 `tokenId` 비교를 직접 구현하지 말고, 새 헬퍼 `compareTokenDescriptions`를 재사용하도록 정리해 두 정렬 기준이 항상 동일하게 유지되도록 수정하세요.
397-463: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win진단 스키마 확장 자체는 적절하나, 다운스트림에서
legacyCode가 유실됨.
code/legacyCode분리는 명확하고 각 단계(missing/circular/scope-mismatch/typedi-fallback)에 맞는 매핑도 정확합니다. 다만 이 진단 객체를 소비하는assertNoDependencyGraphDiagnostics(307-325, 본 diff에서는 미변경)는ProblemFactory.internalServerError(errorDiagnostic.code, ...)만 사용하고extensions에legacyCode를 포함하지 않습니다. 신규 stable 코드로 전환하는 과도기에 레거시 코드를 참조하던 호출부(예: 기존 에러 핸들링, 로그 매칭)가 있다면 이 경로에서는 그 정보를 잃게 됩니다.♻️ 제안: extensions에 legacyCode 포함
throw ProblemFactory.internalServerError(errorDiagnostic.code, errorDiagnostic.message, { extensions: { resolution: errorDiagnostic.trace, token: errorDiagnostic.token, tokenId: errorDiagnostic.tokenId, path: errorDiagnostic.path, pathIds: errorDiagnostic.pathIds, + legacyCode: errorDiagnostic.legacyCode, }, });🤖 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 `@packages/framework-context/src/libs/Container.ts` around lines 397 - 463, The diagnostic mapping in Container’s trace handling is correct, but downstream consumers lose the legacyCode value. Update assertNoDependencyGraphDiagnostics to carry each diagnostic’s legacyCode through ProblemFactory.internalServerError by including it in the generated extensions payload (alongside code and other metadata). Keep the existing step-specific diagnostics in Container’s trace loop unchanged, and ensure the legacyCode from the diagnostics pushed via pushDiagnostic remains available to callers relying on older error identifiers.
🤖 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 `@packages/cli/src/commands/diGraph.ts`:
- Around line 100-104: Document the special-case behavior around
loadFrameworkContext in diGraph.ts so callers understand that when
options.loadModule is overridden and options.loadFrameworkContext is omitted, it
is intentionally forced to undefined and readDiGraphContainer falls back to the
CLI Container import. Update the relevant logic around parseDiGraphArgs,
defaultLoadModule, defaultLoadFrameworkContext, and readDiGraphContainer usage
with a concise inline comment or nearby doc note explaining that loadModule
overrides disable framework-context loading unless explicitly provided.
- Around line 133-139: `runDiGraph` returns success based on
`hasErrorDiagnostics(manifest)`, but the manifest `status` from
`Container.createDependencyGraphManifest` can be `failed` even when there are no
error-severity diagnostics, so the exit code can disagree with the reported
manifest state. Update the exit-code logic in `runDiGraph` to key off the
manifest status (or equivalent failed-state check) instead of only
`hasErrorDiagnostics`, and keep `reportDiGraphManifest`/`manifestJson` behavior
aligned with that status. Use `Container.createDependencyGraphManifest`,
`runDiGraph`, and `reportDiGraphManifest` as the anchor points when making the
change.
- Around line 79-90: The diGraph command is only exposing GLOBAL_OPTIONS, so
citty’s default help can win before runDiGraph() handles --help/-h, which hides
the graph-specific flags. Update diGraph in packages/cli/src/commands/diGraph.ts
to expose a custom help/usage entry (or a dedicated help subcommand) that lists
the graph options, and make sure the existing runDiGraph() flow still handles
the command execution path correctly. Reference the diGraph defineCommand setup
and runDiGraph() when wiring the help behavior.
In `@packages/cli/src/tests/diGraph.spec.ts`:
- Around line 1-4: The import block in diGraph.spec.ts is not organized per the
import guidelines. Reorder the imports in the test file so external packages
like vitest come first, then internal `@croco/`* packages such as
`@croco/framework-context`, then relative imports from ../commands/diGraph.js, and
keep type-only imports separated from value imports within their section.
In
`@packages/create-croco-app/templates/ai-saas/apps/api-server/package.json.hbs`:
- Line 15: The di:graph script in package.json.hbs is tied to the internal
`@croco/cli` dist/bin/croco.js output path, unlike the other template scripts that
invoke the package bin entrypoint. Update the di:graph command to call the CLI
by its bin name (consistent with contract:openapi and similar scripts) so the
generated app template does not depend on build output structure. Use the
existing di:graph script as the location to fix and preserve the current flags
and arguments while switching the executable target.
In `@packages/create-croco-app/templates/saas/apps/api-server/src/app.ts`:
- Around line 23-27: `createCrocoDiGraphRoots()` is exposing the shared
`controllers` array by reference, which can let external consumers mutate the
same array used by `createApp({ controllers })`. Update
`createCrocoDiGraphRoots` to return a defensive copy or a readonly-typed array
so callers cannot affect app wiring, and keep the fix centered on the
`controllers` constant and `createCrocoDiGraphRoots` export.
In
`@packages/create-croco-app/templates/spa-be-split/apps/api-server/package.json.hbs`:
- Line 10: The di:graph script is using a hardcoded ../../node_modules path
instead of the workspace-installed croco binary. Update the di:graph entry in
package.json.hbs to match the existing di:check and doctor scripts by invoking
croco directly, so it relies on the PATH resolution provided by the workspace
root. Keep the change scoped to the di:graph script in the api-server template
and preserve the existing arguments passed to the croco di graph command.
In `@packages/create-croco-app/templates/spa-be-split/package.json.hbs`:
- Around line 18-22: The di:assert package script currently embeds a long node
-e manifest check inside package.json.hbs, which is hard to read and maintain.
Move this roots/providers validation out of the inline command into a dedicated
script or helper used by di:assert, and keep di:verify chaining through
di:graph, di:check, di:assert, project-map:write, project-map:check, and doctor
unchanged. Use the existing di:assert and di:verify script names as the
integration points.
In
`@packages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencyGraphDiagnostic.md`:
- Around line 18-21: The public API for DependencyGraphDiagnostic is missing a
re-export for DependencyGraphLegacyDiagnosticCode, so legacyCode renders without
a link. Update packages/framework-context/src/index.ts to re-export
DependencyGraphLegacyDiagnosticCode from the types module, alongside the
existing DependencyGraphDiagnostic exports, so the docs/type alias reference can
resolve like code does.
In `@packages/framework-context/src/libs/types.ts`:
- Around line 58-67: `DependencyGraphDiagnostic.code`의 공개 값이
`framework-context/di-*`에서 `CROCO_DI_*`로 바뀌어 외부 직접 비교가 깨지는 문제입니다.
`DependencyGraphDiagnostic`와 관련 타입 정의에서 `code` 값은 기존
`framework-context/di-missing-provider`,
`framework-context/di-circular-dependency`,
`framework-context/di-scope-mismatch`, `framework-context/di-unknown-provider`를
유지하고, 새 값은 `legacyCode` 같은 보조 필드로만 다루도록 수정하세요. 변경이 꼭 필요하다면
`DependencyGraphLegacyDiagnosticCode`와 함께 호환성 정책에 맞게 major 변경으로 분리하세요.
---
Outside diff comments:
In `@packages/cli/src/commands/diCheck.ts`:
- Around line 221-238: The legacyCode computation in diCheck currently contains
an unreachable fallback branch because the earlier
isUnmappedCliLegacyCode/isMappedDiGraphLegacyCode and stableCode checks already
cover the same cases. Simplify the logic in diCheck by removing the redundant
“stableCode && rawCode” fallback and keep only the necessary
rawCode/rawLegacyCode and unknown-code handling, while preserving the existing
behavior for CLI_DIAGNOSTIC_CODES and CLI_LEGACY_DIAGNOSTIC_CODES.
In `@packages/framework-context/src/libs/Container.ts`:
- Around line 369-378: `createGraphProviders`의 최종 정렬 로직이
`compareTokenDescriptions`와 동일한 비교 규칙을 인라인으로 կրկն복하고 있습니다.
`createGraphProviders`의 `.sort(...)`에서 `left.token`/`right.token` 및 `tokenId`
비교를 직접 구현하지 말고, 새 헬퍼 `compareTokenDescriptions`를 재사용하도록 정리해 두 정렬 기준이 항상 동일하게
유지되도록 수정하세요.
- Around line 397-463: The diagnostic mapping in Container’s trace handling is
correct, but downstream consumers lose the legacyCode value. Update
assertNoDependencyGraphDiagnostics to carry each diagnostic’s legacyCode through
ProblemFactory.internalServerError by including it in the generated extensions
payload (alongside code and other metadata). Keep the existing step-specific
diagnostics in Container’s trace loop unchanged, and ensure the legacyCode from
the diagnostics pushed via pushDiagnostic remains available to callers relying
on older error identifiers.
🪄 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: e041a177-4eb1-4ee7-9f7f-6841d119dab7
⛔ Files ignored due to path filters (1)
packages/problems-core/src/generated/problem-code-registry.tsis excluded by!**/generated/**
📒 Files selected for processing (65)
.changeset/deterministic-di-graph-command.mddocs/problem-code-registry.jsondocs/troubleshooting/diagnostics.mdpackages/cli/README.mdpackages/cli/src/commands/di.tspackages/cli/src/commands/diCheck.tspackages/cli/src/commands/diGraph.tspackages/cli/src/commands/generateUsageDashboard.tspackages/cli/src/index.tspackages/cli/src/libs/codemods/registerController.tspackages/cli/src/libs/diagnosticCodes.tspackages/cli/src/tests/codemods/registerController.spec.tspackages/cli/src/tests/diCheck.spec.tspackages/cli/src/tests/diGraph.spec.tspackages/cli/src/tests/doctor.spec.tspackages/cli/src/tests/generateUsageDashboard.spec.tspackages/cli/src/tests/integration/e2e.spec.tspackages/create-croco-app/src/tests/e2e-generation.spec.tspackages/create-croco-app/src/tests/templates-build.spec.tspackages/create-croco-app/templates/admin-console/README.md.hbspackages/create-croco-app/templates/admin-console/apps/api-server/package.json.hbspackages/create-croco-app/templates/admin-console/apps/api-server/src/app.ts.hbspackages/create-croco-app/templates/admin-console/apps/api-server/src/controllers/AdminController.tspackages/create-croco-app/templates/admin-console/package.json.hbspackages/create-croco-app/templates/ai-saas/README.md.hbspackages/create-croco-app/templates/ai-saas/apps/api-server/package.json.hbspackages/create-croco-app/templates/ai-saas/apps/api-server/src/app.ts.hbspackages/create-croco-app/templates/ai-saas/apps/api-server/src/controllers/AiController.tspackages/create-croco-app/templates/ai-saas/package.json.hbspackages/create-croco-app/templates/saas/README.md.hbspackages/create-croco-app/templates/saas/apps/api-server/package.json.hbspackages/create-croco-app/templates/saas/apps/api-server/src/app.tspackages/create-croco-app/templates/saas/apps/api-server/src/controllers/JobsController.tspackages/create-croco-app/templates/saas/apps/api-server/src/controllers/OperationsController.tspackages/create-croco-app/templates/saas/apps/api-server/src/controllers/SaasController.tspackages/create-croco-app/templates/saas/package.json.hbspackages/create-croco-app/templates/spa-be-split/README.md.hbspackages/create-croco-app/templates/spa-be-split/apps/api-server/package.json.hbspackages/create-croco-app/templates/spa-be-split/apps/api-server/src/app.tspackages/create-croco-app/templates/spa-be-split/apps/api-server/src/controllers/UserController.tspackages/create-croco-app/templates/spa-be-split/package.json.hbspackages/diagnostics-core/src/libs/DiagnosticCodes.tspackages/diagnostics-core/src/tests/DiagnosticCodes.spec.tspackages/docs/scripts/sanitize-typedoc-index.mjspackages/docs/src/content/docs/api/cli/src/functions/parseDiGraphArgs.mdpackages/docs/src/content/docs/api/cli/src/functions/runDiGraph.mdpackages/docs/src/content/docs/api/cli/src/type-aliases/DiGraphFrameworkContextLoader.mdpackages/docs/src/content/docs/api/cli/src/type-aliases/DiGraphIo.mdpackages/docs/src/content/docs/api/cli/src/type-aliases/DiGraphModuleLoader.mdpackages/docs/src/content/docs/api/cli/src/variables/diGraph.mdpackages/docs/src/content/docs/api/diagnostics-core/src/variables/CROCO_DIAGNOSTIC_CODE_DEFINITIONS.mdpackages/docs/src/content/docs/api/framework-context/src/classes/Container.mdpackages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencyGraphDiagnostic.mdpackages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencyGraphDiagnosticCode.mdpackages/docs/src/content/docs/api/framework-context/src/type-aliases/TokenIdentifier.mdpackages/docs/src/content/docs/api/problems-core/src/variables/CROCO_PROBLEM_CODE_REGISTRY.mdpackages/docs/src/content/docs/en/reference/problem-recovery-cookbook.mdpackages/framework-context/src/index.tspackages/framework-context/src/libs/Container.tspackages/framework-context/src/libs/types.tspackages/framework-context/src/tests/DependencyGraphManifest.spec.tspublic-api-surface.snapshot.jsonscripts/create-croco-app-generated-smoke.mtsscripts/public-api-surface.mtsscripts/static-misuse-raw-error-allowlist.json
💤 Files with no reviewable changes (1)
- scripts/static-misuse-raw-error-allowlist.json
32be5b3 to
82ebdbd
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
packages/cli/src/commands/diGraph.ts (1)
81-92: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
croco di graph --help가 citty의 전역--help가로채기에 의해 커스텀 help에 도달하지 못할 가능성.citty 0.1.6의
runMain은run()콜백이 호출되기 전에 최상위에서rawArgs.includes("--help") || rawArgs.includes("-h")를 검사해 자체 usage를 렌더링하고 프로세스를 종료합니다.async function runMain(cmd, opts = {}) { const rawArgs = opts.rawArgs || process.argv.slice(2); ... if (rawArgs.includes("--help") || rawArgs.includes("-h")) { await showUsage$1(...await resolveSubCommand(cmd, rawArgs)); process.exit(0); } else if (...) { ... } else { await runCommand(cmd, { rawArgs }); } }
diGraph의args는GLOBAL_OPTIONS만 포함하므로, 자동 렌더링되는 usage에는--write/--module/--bootstrap/--roots/--json이 나타나지 않고,runDiGraph내부의parseDiGraphArgs/printDiGraphHelp(Line 108-111, 144-147, 434-444)는 실제 CLI 경로(crocobin이runMain으로 실행되는 경우)에서 결코 실행되지 않는 데드 코드일 가능성이 있습니다. 동일한 우려가 과거 리뷰에서 이미 제기되어 "Addressed"로 표시되었으나, 제공된 최종 코드 스냅샷에는 여전히 동일한 패턴이 남아 있습니다.
croco의 실제 진입점(runMain호출 방식)과GLOBAL_OPTIONS정의를 확인해 주세요.🐛 검증 스크립트
#!/bin/bash set -euo pipefail echo "===== bin entrypoint (runMain usage) =====" rg -n "runMain\(" packages/cli/src -g '!*.spec.ts' echo "===== GLOBAL_OPTIONS definition =====" fd -e ts options.ts --path packages/cli/src/commands cat -n packages/cli/src/commands/options.ts 2>/dev/null || trueAlso applies to: 108-147, 434-444
🤖 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 `@packages/cli/src/commands/diGraph.ts` around lines 81 - 92, The custom help path in diGraph is likely bypassed because citty handles --help/-h before runDiGraph runs, so the command’s own parseDiGraphArgs/printDiGraphHelp logic never executes. Update the diGraph command and/or the entrypoint wiring so the graph-specific options are visible in the top-level usage and the custom help output is reached from defineCommand/runMain, using diGraph, runDiGraph, parseDiGraphArgs, and printDiGraphHelp as the key references.
🤖 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 `@docs/problem-code-registry.json`:
- Around line 2675-2704: The troubleshooting diagnostics mapping is missing the
CROCO_CLI_USAGE_DASHBOARD_005 entry, so add it to the table in
docs/troubleshooting/diagnostics.md to match the existing
usageDashboardInvalidRoutePath symbol in
packages/cli/src/libs/diagnosticCodes.ts. Insert the new row alongside the other
CROCO_CLI_USAGE_DASHBOARD codes (001–004) using the same code, category, and
recovery details so lookup and guidance remain consistent.
In `@packages/cli/src/commands/diCheck.ts`:
- Around line 55-60: `getStableDiGraphDiagnosticCodeForLegacyCode`에서
`DI_GRAPH_LEGACY_DIAGNOSTIC_CODES`를 일반 객체로 브래킷 조회하는 부분이 프로토타입 오염에 취약합니다. 임의의 입력
문자열이 `Object.prototype` 속성을 타고 함수 값을 반환하지 않도록,
`DI_GRAPH_LEGACY_DIAGNOSTIC_CODES` 조회를
`Object.prototype.hasOwnProperty.call(...)`로 가드하거나
`LEGACY_TO_STABLE_DIAGNOSTIC_CODES`처럼 `Map` 기반으로 변경하세요. 반환값이 항상 문자열 코드 또는
`undefined`가 되도록 `code`, `stableCode`를 만드는 흐름을 함께 점검해 주세요.
In `@packages/cli/src/commands/diGraph.ts`:
- Around line 15-16: parseDiGraphArgs currently rejects the shared
GLOBAL_OPTIONS flags, so the di graph command is inconsistent with its declared
options. Update the parser in diGraph.ts to accept --cwd, --dryRun, and
--overwrite alongside the existing valueFlags and booleanFlags, ideally by
wiring in GLOBAL_OPTIONS parsing logic where parseDiGraphArgs handles args; if
these options are not meant to be supported, remove GLOBAL_OPTIONS from the
command definition instead so the CLI surface matches the parser behavior.
In
`@packages/create-croco-app/templates/admin-console/apps/api-server/package.json.hbs`:
- Line 11: The di:graph script uses POSIX-only NODE_OPTIONS inline syntax, which
can fail in Windows shells. Update the package.json.hbs template’s di:graph
entry to use a cross-platform approach, and if native Windows shell support is
intended, add and reference cross-env in the generated template so the script
works in cmd.exe and PowerShell as well as POSIX shells.
In
`@packages/create-croco-app/templates/admin-console/scripts/assert-di-graph.mjs`:
- Around line 1-14: Wrap the manifest loading in assert-di-graph.mjs with
try/catch around readFileSync and JSON.parse so missing or corrupted files
produce a friendly error instead of a raw stack trace. Update the error path in
the main script flow to print a clear message that mentions the DI graph
manifest and points users toward the DI graph drift recovery steps, then exit
non-zero. Keep the existing manifest validation for roots/providers after
successful parsing.
In `@packages/create-croco-app/templates/saas/scripts/assert-di-graph.mjs`:
- Around line 1-14: The assert-di-graph.mjs script currently lets readFileSync
and JSON.parse failures bubble up as raw stack traces, so update the script to
catch errors around loading the manifest and print a user-friendly message that
includes the manifestPath and the underlying error details before exiting
nonzero. Use the existing manifestPath/manifest flow in assert-di-graph.mjs and
keep the validation of manifest.roots and manifest.providers unchanged, but
ensure any file read or parse failure is clearly reported for the shared
di:verify script used by all templates.
---
Duplicate comments:
In `@packages/cli/src/commands/diGraph.ts`:
- Around line 81-92: The custom help path in diGraph is likely bypassed because
citty handles --help/-h before runDiGraph runs, so the command’s own
parseDiGraphArgs/printDiGraphHelp logic never executes. Update the diGraph
command and/or the entrypoint wiring so the graph-specific options are visible
in the top-level usage and the custom help output is reached from
defineCommand/runMain, using diGraph, runDiGraph, parseDiGraphArgs, and
printDiGraphHelp as the key references.
🪄 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: 71c08d6b-f7d3-452a-8825-f1d627db80e1
⛔ Files ignored due to path filters (1)
packages/problems-core/src/generated/problem-code-registry.tsis excluded by!**/generated/**
📒 Files selected for processing (70)
.changeset/deterministic-di-graph-command.mddocs/problem-code-registry.jsondocs/troubleshooting/diagnostics.mdpackages/cli/README.mdpackages/cli/src/commands/di.tspackages/cli/src/commands/diCheck.tspackages/cli/src/commands/diGraph.tspackages/cli/src/commands/generateUsageDashboard.tspackages/cli/src/index.tspackages/cli/src/libs/codemods/registerController.tspackages/cli/src/libs/diagnosticCodes.tspackages/cli/src/tests/codemods/registerController.spec.tspackages/cli/src/tests/diCheck.spec.tspackages/cli/src/tests/diGraph.spec.tspackages/cli/src/tests/doctor.spec.tspackages/cli/src/tests/generateUsageDashboard.spec.tspackages/cli/src/tests/integration/e2e.spec.tspackages/create-croco-app/src/tests/e2e-generation.spec.tspackages/create-croco-app/src/tests/templates-build.spec.tspackages/create-croco-app/templates/admin-console/README.md.hbspackages/create-croco-app/templates/admin-console/apps/api-server/package.json.hbspackages/create-croco-app/templates/admin-console/apps/api-server/src/app.ts.hbspackages/create-croco-app/templates/admin-console/apps/api-server/src/controllers/AdminController.tspackages/create-croco-app/templates/admin-console/package.json.hbspackages/create-croco-app/templates/admin-console/scripts/assert-di-graph.mjspackages/create-croco-app/templates/ai-saas/README.md.hbspackages/create-croco-app/templates/ai-saas/apps/api-server/package.json.hbspackages/create-croco-app/templates/ai-saas/apps/api-server/src/app.ts.hbspackages/create-croco-app/templates/ai-saas/apps/api-server/src/controllers/AiController.tspackages/create-croco-app/templates/ai-saas/package.json.hbspackages/create-croco-app/templates/ai-saas/scripts/assert-di-graph.mjspackages/create-croco-app/templates/saas/README.md.hbspackages/create-croco-app/templates/saas/apps/api-server/package.json.hbspackages/create-croco-app/templates/saas/apps/api-server/src/app.tspackages/create-croco-app/templates/saas/apps/api-server/src/controllers/JobsController.tspackages/create-croco-app/templates/saas/apps/api-server/src/controllers/OperationsController.tspackages/create-croco-app/templates/saas/apps/api-server/src/controllers/SaasController.tspackages/create-croco-app/templates/saas/package.json.hbspackages/create-croco-app/templates/saas/scripts/assert-di-graph.mjspackages/create-croco-app/templates/spa-be-split/README.md.hbspackages/create-croco-app/templates/spa-be-split/apps/api-server/package.json.hbspackages/create-croco-app/templates/spa-be-split/apps/api-server/src/app.tspackages/create-croco-app/templates/spa-be-split/apps/api-server/src/controllers/UserController.tspackages/create-croco-app/templates/spa-be-split/package.json.hbspackages/create-croco-app/templates/spa-be-split/scripts/assert-di-graph.mjspackages/diagnostics-core/src/libs/DiagnosticCodes.tspackages/diagnostics-core/src/tests/DiagnosticCodes.spec.tspackages/docs/scripts/sanitize-typedoc-index.mjspackages/docs/src/content/docs/api/cli/src/functions/parseDiGraphArgs.mdpackages/docs/src/content/docs/api/cli/src/functions/runDiGraph.mdpackages/docs/src/content/docs/api/cli/src/type-aliases/DiGraphFrameworkContextLoader.mdpackages/docs/src/content/docs/api/cli/src/type-aliases/DiGraphIo.mdpackages/docs/src/content/docs/api/cli/src/type-aliases/DiGraphModuleLoader.mdpackages/docs/src/content/docs/api/cli/src/variables/diGraph.mdpackages/docs/src/content/docs/api/diagnostics-core/src/variables/CROCO_DIAGNOSTIC_CODE_DEFINITIONS.mdpackages/docs/src/content/docs/api/framework-context/src/classes/Container.mdpackages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencyGraphDiagnostic.mdpackages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencyGraphDiagnosticCode.mdpackages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencyGraphLegacyDiagnosticCode.mdpackages/docs/src/content/docs/api/framework-context/src/type-aliases/TokenIdentifier.mdpackages/docs/src/content/docs/api/problems-core/src/variables/CROCO_PROBLEM_CODE_REGISTRY.mdpackages/docs/src/content/docs/en/reference/problem-recovery-cookbook.mdpackages/framework-context/src/index.tspackages/framework-context/src/libs/Container.tspackages/framework-context/src/libs/types.tspackages/framework-context/src/tests/DependencyGraphManifest.spec.tspublic-api-surface.snapshot.jsonscripts/create-croco-app-generated-smoke.mtsscripts/public-api-surface.mtsscripts/static-misuse-raw-error-allowlist.json
f8bcf0d to
76a409a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (5)
packages/create-croco-app/templates/admin-console/apps/api-server/package.json.hbs (1)
11-11: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winWindows 호환성 이슈 미해결.
이전 리뷰에서 지적된
NODE_OPTIONS=--import=tsx인라인 문법의 Windows(cmd.exe/PowerShell) 비호환 문제가 아직 해결되지 않았습니다.cross-env도입을 권장합니다.♻️ 제안: cross-env 사용
- "di:graph": "NODE_OPTIONS=--import=tsx croco di graph --module src/app.ts --bootstrap createCrocoApp --roots createCrocoDiGraphRoots --write ../../.croco/build/di-graph.manifest.json", + "di:graph": "cross-env NODE_OPTIONS=--import=tsx croco di graph --module src/app.ts --bootstrap createCrocoApp --roots createCrocoDiGraphRoots --write ../../.croco/build/di-graph.manifest.json",🤖 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 `@packages/create-croco-app/templates/admin-console/apps/api-server/package.json.hbs` at line 11, The di:graph script still uses inline NODE_OPTIONS assignment in package.json.hbs, which breaks on Windows shells. Update the template’s api-server package.json script to use cross-env for setting NODE_OPTIONS before running croco di graph, and ensure the existing script name and command structure remain unchanged apart from the environment-variable handling.packages/create-croco-app/templates/spa-be-split/apps/api-server/package.json.hbs (1)
10-10: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winnode_modules 경로 하드코딩 문제는 해결됐지만 Windows 호환성 문제는 남아있음.
이전에 지적된
../../node_modules/@croco/cli/dist/bin/croco.js하드코딩 참조는croco바이너리 직접 호출로 수정되었습니다. 다만NODE_OPTIONS=--import=tsxPOSIX 인라인 문법은 여전히 남아 있어 Windows 네이티브 셸에서 실패할 수 있습니다. 다른 템플릿(admin-console)에서도 동일한 패턴이 지적된 바 있습니다.♻️ 제안: cross-env 사용
- "di:graph": "NODE_OPTIONS=--import=tsx croco di graph --module src/app.ts --bootstrap createCrocoApp --roots createCrocoDiGraphRoots --write ../../.croco/build/di-graph.manifest.json", + "di:graph": "cross-env NODE_OPTIONS=--import=tsx croco di graph --module src/app.ts --bootstrap createCrocoApp --roots createCrocoDiGraphRoots --write ../../.croco/build/di-graph.manifest.json",🤖 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 `@packages/create-croco-app/templates/spa-be-split/apps/api-server/package.json.hbs` at line 10, The di:graph script in the package.json.hbs template still uses POSIX-only inline environment assignment, which breaks on Windows shells. Update the command in the api-server template to use a cross-platform env wrapper such as cross-env for the NODE_OPTIONS=--import=tsx part, matching the approach used in the admin-console template. Keep the croco CLI invocation and existing arguments the same, but make the environment setup shell-agnostic.packages/create-croco-app/templates/saas/apps/api-server/package.json.hbs (1)
17-17: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
NODE_OPTIONS=인라인 문법의 Windows 호환성 확인 필요.
NODE_OPTIONS=--import=tsx croco di graph ...는 POSIX 셸 문법으로, Windows의 cmd.exe/PowerShell 네이티브 셸에서는 그대로 실행되지 않을 수 있습니다.devDependencies에cross-env등 크로스플랫폼 헬퍼가 없어 Windows 사용자가pnpm di:graph를 실행하면 실패할 가능성이 있습니다. 동일한 패턴이 다른 템플릿(admin-console,spa-be-split)에도 반복되고 있으므로, 이 생성 앱 템플릿들이 Windows 네이티브 셸 지원을 목표로 한다면cross-env사용을 권장합니다.♻️ 제안: cross-env 사용
- "di:graph": "NODE_OPTIONS=--import=tsx croco di graph --module src/app.ts --bootstrap createCrocoApp --roots createCrocoDiGraphRoots --write ../../.croco/build/di-graph.manifest.json", + "di:graph": "cross-env NODE_OPTIONS=--import=tsx croco di graph --module src/app.ts --bootstrap createCrocoApp --roots createCrocoDiGraphRoots --write ../../.croco/build/di-graph.manifest.json",🤖 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 `@packages/create-croco-app/templates/saas/apps/api-server/package.json.hbs` at line 17, The di:graph script in the create-croco-app template uses POSIX-only NODE_OPTIONS= inline syntax, which will not run reliably in Windows native shells. Update the template’s package.json.hbs script (and the same script pattern in the other affected templates like admin-console and spa-be-split) to use a cross-platform environment helper such as cross-env, and make sure the needed dependency is added so the generated apps can run pnpm di:graph on Windows as well as Unix shells.packages/create-croco-app/templates/saas/scripts/assert-di-graph.mjs (1)
1-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win파일 읽기/파싱 실패 시 에러 메시지 개선 필요 (기존 지적 사항 유지).
readFileSync/JSON.parse실패 시 raw stack trace만 노출됩니다. 4개 템플릿에 동일 스크립트가 반복되므로 공통 개선이 유효합니다.♻️ 제안 diff
import { readFileSync } from "node:fs"; const manifestPath = process.argv[2] ?? ".croco/build/di-graph.manifest.json"; -const manifest = JSON.parse(readFileSync(manifestPath, "utf8")); +let manifest; +try { + manifest = JSON.parse(readFileSync(manifestPath, "utf8")); +} catch (error) { + console.error(`Failed to read/parse DI graph manifest at '${manifestPath}': ${error.message}`); + process.exit(1); +}🤖 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 `@packages/create-croco-app/templates/saas/scripts/assert-di-graph.mjs` around lines 1 - 14, The manifest load in assert-di-graph.mjs can fail with an unhelpful raw stack trace when readFileSync or JSON.parse throws. Update the script around the manifestPath/readFileSync/JSON.parse flow to catch those failures and log a clear, descriptive error that includes the manifest path and the underlying error details before exiting; apply the same pattern in the shared template copy used across the four saas scripts.packages/create-croco-app/templates/spa-be-split/scripts/assert-di-graph.mjs (1)
1-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win파일 읽기/파싱 실패 시 에러 메시지 개선 필요 (다른 템플릿과 동일한 이슈).
saas템플릿의 동일 스크립트에 이미 지적된 사항과 같습니다.🤖 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 `@packages/create-croco-app/templates/spa-be-split/scripts/assert-di-graph.mjs` around lines 1 - 14, The DI graph assertion script only validates the manifest shape and does not improve diagnostics when reading or parsing fails. Update assert-di-graph.mjs around manifestPath/readFileSync/JSON.parse to catch file read or JSON parse errors and report the actual error details in console.error before exiting, matching the behavior used in the saas template’s equivalent script.
🤖 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 `@packages/framework-context/src/libs/Container.ts`:
- Around line 491-506: `sortProviderDependencies` is duplicating the same
label-then-id comparison already implemented in `compareTokenDescriptions`.
Refactor the `Container` sorting logic to reuse `compareTokenDescriptions`
inside the `sortProviderDependencies` callback, keeping the existing ordering
behavior while centralizing the comparison rule in one place.
---
Duplicate comments:
In
`@packages/create-croco-app/templates/admin-console/apps/api-server/package.json.hbs`:
- Line 11: The di:graph script still uses inline NODE_OPTIONS assignment in
package.json.hbs, which breaks on Windows shells. Update the template’s
api-server package.json script to use cross-env for setting NODE_OPTIONS before
running croco di graph, and ensure the existing script name and command
structure remain unchanged apart from the environment-variable handling.
In `@packages/create-croco-app/templates/saas/apps/api-server/package.json.hbs`:
- Line 17: The di:graph script in the create-croco-app template uses POSIX-only
NODE_OPTIONS= inline syntax, which will not run reliably in Windows native
shells. Update the template’s package.json.hbs script (and the same script
pattern in the other affected templates like admin-console and spa-be-split) to
use a cross-platform environment helper such as cross-env, and make sure the
needed dependency is added so the generated apps can run pnpm di:graph on
Windows as well as Unix shells.
In `@packages/create-croco-app/templates/saas/scripts/assert-di-graph.mjs`:
- Around line 1-14: The manifest load in assert-di-graph.mjs can fail with an
unhelpful raw stack trace when readFileSync or JSON.parse throws. Update the
script around the manifestPath/readFileSync/JSON.parse flow to catch those
failures and log a clear, descriptive error that includes the manifest path and
the underlying error details before exiting; apply the same pattern in the
shared template copy used across the four saas scripts.
In
`@packages/create-croco-app/templates/spa-be-split/apps/api-server/package.json.hbs`:
- Line 10: The di:graph script in the package.json.hbs template still uses
POSIX-only inline environment assignment, which breaks on Windows shells. Update
the command in the api-server template to use a cross-platform env wrapper such
as cross-env for the NODE_OPTIONS=--import=tsx part, matching the approach used
in the admin-console template. Keep the croco CLI invocation and existing
arguments the same, but make the environment setup shell-agnostic.
In
`@packages/create-croco-app/templates/spa-be-split/scripts/assert-di-graph.mjs`:
- Around line 1-14: The DI graph assertion script only validates the manifest
shape and does not improve diagnostics when reading or parsing fails. Update
assert-di-graph.mjs around manifestPath/readFileSync/JSON.parse to catch file
read or JSON parse errors and report the actual error details in console.error
before exiting, matching the behavior used in the saas template’s equivalent
script.
🪄 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: 3c301976-fa81-492e-a569-467961461d3a
⛔ Files ignored due to path filters (1)
packages/problems-core/src/generated/problem-code-registry.tsis excluded by!**/generated/**
📒 Files selected for processing (70)
.changeset/deterministic-di-graph-command.mddocs/problem-code-registry.jsondocs/troubleshooting/diagnostics.mdpackages/cli/README.mdpackages/cli/src/commands/di.tspackages/cli/src/commands/diCheck.tspackages/cli/src/commands/diGraph.tspackages/cli/src/commands/generateUsageDashboard.tspackages/cli/src/index.tspackages/cli/src/libs/codemods/registerController.tspackages/cli/src/libs/diagnosticCodes.tspackages/cli/src/tests/codemods/registerController.spec.tspackages/cli/src/tests/diCheck.spec.tspackages/cli/src/tests/diGraph.spec.tspackages/cli/src/tests/doctor.spec.tspackages/cli/src/tests/generateUsageDashboard.spec.tspackages/cli/src/tests/integration/e2e.spec.tspackages/create-croco-app/src/tests/e2e-generation.spec.tspackages/create-croco-app/src/tests/templates-build.spec.tspackages/create-croco-app/templates/admin-console/README.md.hbspackages/create-croco-app/templates/admin-console/apps/api-server/package.json.hbspackages/create-croco-app/templates/admin-console/apps/api-server/src/app.ts.hbspackages/create-croco-app/templates/admin-console/apps/api-server/src/controllers/AdminController.tspackages/create-croco-app/templates/admin-console/package.json.hbspackages/create-croco-app/templates/admin-console/scripts/assert-di-graph.mjspackages/create-croco-app/templates/ai-saas/README.md.hbspackages/create-croco-app/templates/ai-saas/apps/api-server/package.json.hbspackages/create-croco-app/templates/ai-saas/apps/api-server/src/app.ts.hbspackages/create-croco-app/templates/ai-saas/apps/api-server/src/controllers/AiController.tspackages/create-croco-app/templates/ai-saas/package.json.hbspackages/create-croco-app/templates/ai-saas/scripts/assert-di-graph.mjspackages/create-croco-app/templates/saas/README.md.hbspackages/create-croco-app/templates/saas/apps/api-server/package.json.hbspackages/create-croco-app/templates/saas/apps/api-server/src/app.tspackages/create-croco-app/templates/saas/apps/api-server/src/controllers/JobsController.tspackages/create-croco-app/templates/saas/apps/api-server/src/controllers/OperationsController.tspackages/create-croco-app/templates/saas/apps/api-server/src/controllers/SaasController.tspackages/create-croco-app/templates/saas/package.json.hbspackages/create-croco-app/templates/saas/scripts/assert-di-graph.mjspackages/create-croco-app/templates/spa-be-split/README.md.hbspackages/create-croco-app/templates/spa-be-split/apps/api-server/package.json.hbspackages/create-croco-app/templates/spa-be-split/apps/api-server/src/app.tspackages/create-croco-app/templates/spa-be-split/apps/api-server/src/controllers/UserController.tspackages/create-croco-app/templates/spa-be-split/package.json.hbspackages/create-croco-app/templates/spa-be-split/scripts/assert-di-graph.mjspackages/diagnostics-core/src/libs/DiagnosticCodes.tspackages/diagnostics-core/src/tests/DiagnosticCodes.spec.tspackages/docs/scripts/sanitize-typedoc-index.mjspackages/docs/src/content/docs/api/cli/src/functions/parseDiGraphArgs.mdpackages/docs/src/content/docs/api/cli/src/functions/runDiGraph.mdpackages/docs/src/content/docs/api/cli/src/type-aliases/DiGraphFrameworkContextLoader.mdpackages/docs/src/content/docs/api/cli/src/type-aliases/DiGraphIo.mdpackages/docs/src/content/docs/api/cli/src/type-aliases/DiGraphModuleLoader.mdpackages/docs/src/content/docs/api/cli/src/variables/diGraph.mdpackages/docs/src/content/docs/api/diagnostics-core/src/variables/CROCO_DIAGNOSTIC_CODE_DEFINITIONS.mdpackages/docs/src/content/docs/api/framework-context/src/classes/Container.mdpackages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencyGraphDiagnostic.mdpackages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencyGraphDiagnosticCode.mdpackages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencyGraphLegacyDiagnosticCode.mdpackages/docs/src/content/docs/api/framework-context/src/type-aliases/TokenIdentifier.mdpackages/docs/src/content/docs/api/problems-core/src/variables/CROCO_PROBLEM_CODE_REGISTRY.mdpackages/docs/src/content/docs/en/reference/problem-recovery-cookbook.mdpackages/framework-context/src/index.tspackages/framework-context/src/libs/Container.tspackages/framework-context/src/libs/types.tspackages/framework-context/src/tests/DependencyGraphManifest.spec.tspublic-api-surface.snapshot.jsonscripts/create-croco-app-generated-smoke.mtsscripts/public-api-surface.mtsscripts/static-misuse-raw-error-allowlist.json
💤 Files with no reviewable changes (1)
- scripts/static-misuse-raw-error-allowlist.json
76a409a to
9f39e56
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@packages/cli/src/commands/diGraph.ts`:
- Around line 13-15: Rename the module-level constants in diGraph.ts to
SCREAMING_SNAKE_CASE to match the codebase convention: update
defaultManifestPath, valueFlags, and booleanFlags to uppercase names, and then
update every reference in the diGraph parsing logic accordingly (including any
helpers or exports that use them). Keep the behavior unchanged; this is only a
naming consistency fix.
In `@packages/cli/src/tests/diGraph.spec.ts`:
- Around line 249-337: The test creates a temporary directory with mkdtemp in
diGraph.spec.ts but never cleans it up, so add teardown in the ESM app import
test to remove the created cwd after assertions. Use the existing temp-directory
setup in the it("loads the app package import entry for ESM application
modules") case and ensure cleanup runs even if the test fails, without changing
the runDiGraph behavior or expectations.
In `@packages/create-croco-app/src/tests/templates-build.spec.ts`:
- Line 204: The API template dependency list still pins cross-env to the legacy
7.x major; update the template in templates-build.spec.ts so the generated API
project uses a newer 8+ version instead. Locate the dependency entry in the
template fixture and replace the cross-env version range consistently with the
rest of the Node 22-oriented template setup.
In `@packages/create-croco-app/templates/ai-saas/README.md.hbs`:
- Line 90: Update the di:verify description in the README template to reflect
the full verification chain: di:graph, di:check, di:assert, project-map:write,
project-map:check, and doctor. The current wording around “confirming the file
exists” should be expanded to mention that di:assert also validates manifest
fields via script, so the description accurately matches the behavior of the
di:verify flow.
In
`@packages/create-croco-app/templates/spa-be-split/scripts/assert-di-graph.mjs`:
- Around line 4-13: The DI graph manifest handling in assert-di-graph.mjs should
guard against JSON.parse returning null, because the later manifest.roots access
can throw a TypeError outside the existing friendly error path. Update the
manifest loading flow after the readFileSync/JSON.parse try block to validate
that manifest is a non-null object before any property access, and if it is null
or invalid, print the same recovery-oriented error message and exit through the
existing fail path.
🪄 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: 0b946521-337c-4252-b080-bb0f986b36b2
⛔ Files ignored due to path filters (1)
packages/problems-core/src/generated/problem-code-registry.tsis excluded by!**/generated/**
📒 Files selected for processing (70)
.changeset/deterministic-di-graph-command.mddocs/problem-code-registry.jsondocs/troubleshooting/diagnostics.mdpackages/cli/README.mdpackages/cli/src/commands/di.tspackages/cli/src/commands/diCheck.tspackages/cli/src/commands/diGraph.tspackages/cli/src/commands/generateUsageDashboard.tspackages/cli/src/index.tspackages/cli/src/libs/codemods/registerController.tspackages/cli/src/libs/diagnosticCodes.tspackages/cli/src/tests/codemods/registerController.spec.tspackages/cli/src/tests/diCheck.spec.tspackages/cli/src/tests/diGraph.spec.tspackages/cli/src/tests/doctor.spec.tspackages/cli/src/tests/generateUsageDashboard.spec.tspackages/cli/src/tests/integration/e2e.spec.tspackages/create-croco-app/src/tests/e2e-generation.spec.tspackages/create-croco-app/src/tests/templates-build.spec.tspackages/create-croco-app/templates/admin-console/README.md.hbspackages/create-croco-app/templates/admin-console/apps/api-server/package.json.hbspackages/create-croco-app/templates/admin-console/apps/api-server/src/app.ts.hbspackages/create-croco-app/templates/admin-console/apps/api-server/src/controllers/AdminController.tspackages/create-croco-app/templates/admin-console/package.json.hbspackages/create-croco-app/templates/admin-console/scripts/assert-di-graph.mjspackages/create-croco-app/templates/ai-saas/README.md.hbspackages/create-croco-app/templates/ai-saas/apps/api-server/package.json.hbspackages/create-croco-app/templates/ai-saas/apps/api-server/src/app.ts.hbspackages/create-croco-app/templates/ai-saas/apps/api-server/src/controllers/AiController.tspackages/create-croco-app/templates/ai-saas/package.json.hbspackages/create-croco-app/templates/ai-saas/scripts/assert-di-graph.mjspackages/create-croco-app/templates/saas/README.md.hbspackages/create-croco-app/templates/saas/apps/api-server/package.json.hbspackages/create-croco-app/templates/saas/apps/api-server/src/app.tspackages/create-croco-app/templates/saas/apps/api-server/src/controllers/JobsController.tspackages/create-croco-app/templates/saas/apps/api-server/src/controllers/OperationsController.tspackages/create-croco-app/templates/saas/apps/api-server/src/controllers/SaasController.tspackages/create-croco-app/templates/saas/package.json.hbspackages/create-croco-app/templates/saas/scripts/assert-di-graph.mjspackages/create-croco-app/templates/spa-be-split/README.md.hbspackages/create-croco-app/templates/spa-be-split/apps/api-server/package.json.hbspackages/create-croco-app/templates/spa-be-split/apps/api-server/src/app.tspackages/create-croco-app/templates/spa-be-split/apps/api-server/src/controllers/UserController.tspackages/create-croco-app/templates/spa-be-split/package.json.hbspackages/create-croco-app/templates/spa-be-split/scripts/assert-di-graph.mjspackages/diagnostics-core/src/libs/DiagnosticCodes.tspackages/diagnostics-core/src/tests/DiagnosticCodes.spec.tspackages/docs/scripts/sanitize-typedoc-index.mjspackages/docs/src/content/docs/api/cli/src/functions/parseDiGraphArgs.mdpackages/docs/src/content/docs/api/cli/src/functions/runDiGraph.mdpackages/docs/src/content/docs/api/cli/src/type-aliases/DiGraphFrameworkContextLoader.mdpackages/docs/src/content/docs/api/cli/src/type-aliases/DiGraphIo.mdpackages/docs/src/content/docs/api/cli/src/type-aliases/DiGraphModuleLoader.mdpackages/docs/src/content/docs/api/cli/src/variables/diGraph.mdpackages/docs/src/content/docs/api/diagnostics-core/src/variables/CROCO_DIAGNOSTIC_CODE_DEFINITIONS.mdpackages/docs/src/content/docs/api/framework-context/src/classes/Container.mdpackages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencyGraphDiagnostic.mdpackages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencyGraphDiagnosticCode.mdpackages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencyGraphLegacyDiagnosticCode.mdpackages/docs/src/content/docs/api/framework-context/src/type-aliases/TokenIdentifier.mdpackages/docs/src/content/docs/api/problems-core/src/variables/CROCO_PROBLEM_CODE_REGISTRY.mdpackages/docs/src/content/docs/en/reference/problem-recovery-cookbook.mdpackages/framework-context/src/index.tspackages/framework-context/src/libs/Container.tspackages/framework-context/src/libs/types.tspackages/framework-context/src/tests/DependencyGraphManifest.spec.tspublic-api-surface.snapshot.jsonscripts/create-croco-app-generated-smoke.mtsscripts/public-api-surface.mtsscripts/static-misuse-raw-error-allowlist.json
💤 Files with no reviewable changes (1)
- scripts/static-misuse-raw-error-allowlist.json
3714117 to
df41ed9
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@packages/cli/src/tests/integration/e2e.spec.ts`:
- Around line 368-370: The ambient declaration for Component is too narrow and
only covers the no-argument decorator shape, so update the declare module
'`@croco/framework-context`' stub in e2e.spec.ts to accept the same options form
used by the real API, including Component({ scope: "request" }). Keep the
declaration aligned with the actual decorator signature so the test fixture
reflects both Component() and option-based usage.
In `@packages/create-croco-app/templates/spa-be-split/apps/api-server/src/app.ts`:
- Around line 21-25: The return type in createCrocoDiGraphRoots is using a
verbose indexed-array expression, which should be simplified to the project’s
constructor-based factory pattern. Update the controllers typing around
UserController and the createCrocoDiGraphRoots signature to use a generic
Constructor<T = unknown> constraint (or an alias built from it) instead of
readonly (typeof controllers)[number][]. Keep the function behavior the same,
but make the type reflect a reflection/factory list of constructors more
clearly.
In `@packages/framework-context/src/tests/DependencyGraphManifest.spec.ts`:
- Around line 151-199: The deterministic ordering test in
DependencyGraphManifest.spec is missing a provider uniqueness check, so a
tokenId collision could still pass if both manifests fail the same way. Update
the test around Container.createDependencyGraphManifest to also assert the
expected provider count and that the providers’ tokenId values are unique, using
the existing FirstSharedService and SecondSharedService setup to catch
class.name-based collisions.
🪄 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: 7da23af0-b288-4c00-86dd-133bfd55c376
⛔ Files ignored due to path filters (1)
packages/problems-core/src/generated/problem-code-registry.tsis excluded by!**/generated/**
📒 Files selected for processing (70)
.changeset/deterministic-di-graph-command.mddocs/problem-code-registry.jsondocs/troubleshooting/diagnostics.mdpackages/cli/README.mdpackages/cli/src/commands/di.tspackages/cli/src/commands/diCheck.tspackages/cli/src/commands/diGraph.tspackages/cli/src/commands/generateUsageDashboard.tspackages/cli/src/index.tspackages/cli/src/libs/codemods/registerController.tspackages/cli/src/libs/diagnosticCodes.tspackages/cli/src/tests/codemods/registerController.spec.tspackages/cli/src/tests/diCheck.spec.tspackages/cli/src/tests/diGraph.spec.tspackages/cli/src/tests/doctor.spec.tspackages/cli/src/tests/generateUsageDashboard.spec.tspackages/cli/src/tests/integration/e2e.spec.tspackages/create-croco-app/src/tests/e2e-generation.spec.tspackages/create-croco-app/src/tests/templates-build.spec.tspackages/create-croco-app/templates/admin-console/README.md.hbspackages/create-croco-app/templates/admin-console/apps/api-server/package.json.hbspackages/create-croco-app/templates/admin-console/apps/api-server/src/app.ts.hbspackages/create-croco-app/templates/admin-console/apps/api-server/src/controllers/AdminController.tspackages/create-croco-app/templates/admin-console/package.json.hbspackages/create-croco-app/templates/admin-console/scripts/assert-di-graph.mjspackages/create-croco-app/templates/ai-saas/README.md.hbspackages/create-croco-app/templates/ai-saas/apps/api-server/package.json.hbspackages/create-croco-app/templates/ai-saas/apps/api-server/src/app.ts.hbspackages/create-croco-app/templates/ai-saas/apps/api-server/src/controllers/AiController.tspackages/create-croco-app/templates/ai-saas/package.json.hbspackages/create-croco-app/templates/ai-saas/scripts/assert-di-graph.mjspackages/create-croco-app/templates/saas/README.md.hbspackages/create-croco-app/templates/saas/apps/api-server/package.json.hbspackages/create-croco-app/templates/saas/apps/api-server/src/app.tspackages/create-croco-app/templates/saas/apps/api-server/src/controllers/JobsController.tspackages/create-croco-app/templates/saas/apps/api-server/src/controllers/OperationsController.tspackages/create-croco-app/templates/saas/apps/api-server/src/controllers/SaasController.tspackages/create-croco-app/templates/saas/package.json.hbspackages/create-croco-app/templates/saas/scripts/assert-di-graph.mjspackages/create-croco-app/templates/spa-be-split/README.md.hbspackages/create-croco-app/templates/spa-be-split/apps/api-server/package.json.hbspackages/create-croco-app/templates/spa-be-split/apps/api-server/src/app.tspackages/create-croco-app/templates/spa-be-split/apps/api-server/src/controllers/UserController.tspackages/create-croco-app/templates/spa-be-split/package.json.hbspackages/create-croco-app/templates/spa-be-split/scripts/assert-di-graph.mjspackages/diagnostics-core/src/libs/DiagnosticCodes.tspackages/diagnostics-core/src/tests/DiagnosticCodes.spec.tspackages/docs/scripts/sanitize-typedoc-index.mjspackages/docs/src/content/docs/api/cli/src/functions/parseDiGraphArgs.mdpackages/docs/src/content/docs/api/cli/src/functions/runDiGraph.mdpackages/docs/src/content/docs/api/cli/src/type-aliases/DiGraphFrameworkContextLoader.mdpackages/docs/src/content/docs/api/cli/src/type-aliases/DiGraphIo.mdpackages/docs/src/content/docs/api/cli/src/type-aliases/DiGraphModuleLoader.mdpackages/docs/src/content/docs/api/cli/src/variables/diGraph.mdpackages/docs/src/content/docs/api/diagnostics-core/src/variables/CROCO_DIAGNOSTIC_CODE_DEFINITIONS.mdpackages/docs/src/content/docs/api/framework-context/src/classes/Container.mdpackages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencyGraphDiagnostic.mdpackages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencyGraphDiagnosticCode.mdpackages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencyGraphLegacyDiagnosticCode.mdpackages/docs/src/content/docs/api/framework-context/src/type-aliases/TokenIdentifier.mdpackages/docs/src/content/docs/api/problems-core/src/variables/CROCO_PROBLEM_CODE_REGISTRY.mdpackages/docs/src/content/docs/en/reference/problem-recovery-cookbook.mdpackages/framework-context/src/index.tspackages/framework-context/src/libs/Container.tspackages/framework-context/src/libs/types.tspackages/framework-context/src/tests/DependencyGraphManifest.spec.tspublic-api-surface.snapshot.jsonscripts/create-croco-app-generated-smoke.mtsscripts/public-api-surface.mtsscripts/static-misuse-raw-error-allowlist.json
💤 Files with no reviewable changes (1)
- scripts/static-misuse-raw-error-allowlist.json
df41ed9 to
01b78dd
Compare
Fixes #1183.
Summary
croco di graphso apps can write.croco/build/di-graph.manifest.jsonfrom the framework container, an app module bootstrap, or explicit root exports.CROCO_DI_001throughCROCO_DI_004diagnostic codes.pnpm di:verifythrough graph generation, DI check, manifest assertion, Project Map checks, andcroco doctor.Verification
TURBO_CACHE_DIR=$(mktemp -d /tmp/croco-turbo-cache.XXXXXX) CROCO_GENERATED_SMOKE_CASES=production-app-starter,admin-console-starter,saas-golden-path,ai-saas-golden-path node --experimental-strip-types scripts/create-croco-app-generated-smoke.mts- passed.corepack pnpm --filter @croco/cli exec vitest run src/tests/diGraph.spec.ts src/tests/diCheck.spec.ts src/tests/doctor.spec.ts src/tests/codemods/registerController.spec.ts src/tests/generateUsageDashboard.spec.ts src/tests/diagnosticCodes.spec.ts- passed, 64 tests.corepack pnpm --filter create-croco-app exec vitest run src/tests/templates-build.spec.ts src/tests/e2e-generation.spec.ts- passed, 26 tests.corepack pnpm --filter @croco/framework-context exec vitest run src/tests/DependencyGraphManifest.spec.ts- passed, 7 tests.corepack pnpm --filter @croco/diagnostics-core exec vitest run src/tests/DiagnosticCodes.spec.ts- passed.corepack pnpm --filter @croco/framework-context --filter @croco/diagnostics-core --filter @croco/problems-core --filter @croco/cli --filter create-croco-app typecheck- passed.corepack pnpm --filter @croco/framework-context --filter @croco/diagnostics-core --filter @croco/problems-core --filter @croco/cli --filter create-croco-app build- passed.corepack pnpm static-misuse:check- passed.corepack pnpm problem-registry:check- passed, 413 codes.corepack pnpm public-api:check- passed, 110 packages.corepack pnpm check- passed.corepack pnpm changeset-required:check -- --base origin/trunk --head HEAD- passed.git diff --check HEAD^ HEAD- passed.test- passed, 225/225 Turbo tasks.typecheck- passed, 224/224 Turbo tasks.Self-review gates
Notes
Summary by CodeRabbit
croco di graph로 결정적 DI 그래프 매니페스트(manifest)를 생성하고,croco di check로 검증할 수 있습니다.di:graph → di:check → di:assert → di:verify기반 DI 검증 파이프라인을 추가했습니다.CROCO_DI_001~004로 정규화되며, 기존 레거시 정보는 함께 제공됩니다.