Skip to content

refactor(cli): migrate car assets onto shared request execution - #517

Merged
justinmakaila merged 5 commits into
mainfrom
refactor/cli-assets-car
Oct 19, 2026
Merged

justinmakaila merged 5 commits into
mainfrom
refactor/cli-assets-car

Conversation

@justinmakaila

@justinmakaila justinmakaila commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Migrate iracing-data assets car to the schema → Commander adapter → opinionated command service → shared authenticated request/output executor established by #516. First layer of the three-PR assets stack.

Parent: #516 (codex/issue-515-docs-executor).
Next layers: #518 (series) and #519 (track). Actual branch bases are #516 → #517 → #518 → #519.

Scope

  • Examples or documentation
  • Dependencies or maintenance

Execution boundaries

  • Previously: Commander → per-invocation car scope → credential resolver + generated client → locally duplicated request error handling → returned HTTPS data link → locally duplicated JSON stdout → diagnostics.
  • Now: Commander validates local RawOptionsSchema / OptionsSchema, then invokes makeCarAssetsService. The service selects IRacingAPI.withAccessToken(token).car.getCarAssets(). The shared executor resolves credentials, maps only the generated API request through DataApiRequestError, optionally materializes the returned link outside that catch boundary, invokes the shared document writer, and reports success.
  • Car's scope.ts and its nested per-invocation capabilities are removed.
  • The shared executor gains an optional second-stage materialize callback and the existing writer accepts an Assets label. No new CLI flags; assets are always pretty JSON to stdout with a trailing newline.
  • The materializer is injected once through the registrar, stays HTTPS-only, does not forward Authorization, and retains existing redacted response handling and exactly-once link fetching. Series and track remain on their existing scopes in this layer.

Generated Files

None.

Package and Release Notes

None. Private CLI; no generated SDK/OAuth changes or public option changes.

Validation

The local repository was unavailable in this execution environment; verification below is from GitHub Actions, not local command execution.

  • pnpm --filter @iracing-data/cli test — not runnable in this execution environment (repository/dependencies unavailable locally).
  • pnpm verify:js — same limitation.
  • pnpm verify:repo — same limitation.
  • GitHub Actions CI run #39167954569 — passed repository verification, lint, style, CLI/unit tests, generated-client checks and Rust checks; Docker offline recovery also passed.

Changed tests: car Commander/schema/service/executor behavior, credential and generated-request failure boundaries, materializer security cases and OpenAPI parity. Existing docs, series and track tests remain in the stack.

Notes for Reviewers

Inspect src/services/request.ts first: generated Data API errors are translated, returned-link failures are not. Examine the security assertions in test/assets.test.cjs. This PR is intentionally independently reviewable against #516, before subsequent assets migrations.

Related: #515, #516.

Summary by CodeRabbit

  • New Features
    • Added the assets car command to retrieve car assets and output them as JSON.
  • Bug Fixes
    • Improved handling of car asset requests, including credential failures and errors while processing returned asset links.

@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: b739e3e7-6670-4a28-9d0d-af58b2f7d1c9
📥 Commits

Reviewing files that changed from the base of the PR and between 54c85d8 and 78f8c41.

📒 Files selected for processing: 16
  • apps/iracing-data-cli/src/api.ts
  • apps/iracing-data-cli/src/commands/assets/README.md
  • apps/iracing-data-cli/src/commands/assets/car/command.ts
  • apps/iracing-data-cli/src/commands/assets/car/index.ts
  • apps/iracing-data-cli/src/commands/assets/car/schema.ts
  • apps/iracing-data-cli/src/commands/assets/car/scope.ts
  • apps/iracing-data-cli/src/commands/assets/car/service.ts
  • apps/iracing-data-cli/src/commands/assets/index.ts
  • apps/iracing-data-cli/src/commands/assets/returned-data-api-link.ts
  • apps/iracing-data-cli/src/program.ts
  • apps/iracing-data-cli/src/services/request.ts
  • apps/iracing-data-cli/src/token-output.ts
  • apps/iracing-data-cli/test/assets-parity.test.cjs
  • apps/iracing-data-cli/test/assets.test.cjs
  • apps/iracing-data-cli/test/series-assets.test.cjs
  • apps/iracing-data-cli/test/track-assets.test.cjs
💤 Files with no reviewable changes: 1
  • apps/iracing-data-cli/src/commands/assets/car/scope.ts

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📜 Recent review details
🧰 Additional context used
📚 Code guidelines: 1
AGENTS.md — auto-discovered
📓 Path-based instructions: 2
Applicability reference: apps/iracing-data-mcp/AGENTS.md: # iracing-data-mcp guidance Inherits [root guidance](../../AGENTS.md).

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • apps/iracing-data-cli/test/series-assets.test.cjs
  • apps/iracing-data-cli/src/token-output.ts
  • apps/iracing-data-cli/test/track-assets.test.cjs
  • apps/iracing-data-cli/src/api.ts
  • apps/iracing-data-cli/src/commands/assets/returned-data-api-link.ts
  • apps/iracing-data-cli/src/program.ts
  • apps/iracing-data-cli/src/commands/assets/car/schema.ts
  • apps/iracing-data-cli/src/commands/assets/car/service.ts
  • apps/iracing-data-cli/src/commands/assets/README.md
  • apps/iracing-data-cli/src/commands/assets/index.ts
  • apps/iracing-data-cli/src/commands/assets/car/index.ts
  • apps/iracing-data-cli/test/assets-parity.test.cjs
  • apps/iracing-data-cli/src/services/request.ts
  • apps/iracing-data-cli/src/commands/assets/car/command.ts
  • apps/iracing-data-cli/test/assets.test.cjs
Applicability reference: apps/iracing-data-mcp/AGENTS.md: # iracing-data-mcp guidance Inherits [root guidance](../../AGENTS.md).

📄 CodeRabbit inference engine (AGENTS.md)

Files:

  • apps/iracing-data-cli/src/token-output.ts
  • apps/iracing-data-cli/src/commands/assets/returned-data-api-link.ts
  • apps/iracing-data-cli/src/program.ts
  • apps/iracing-data-cli/src/commands/assets/index.ts
  • apps/iracing-data-cli/src/commands/assets/car/index.ts
  • apps/iracing-data-cli/src/commands/assets/car/command.ts
🔇 Additional comments: 15
apps/iracing-data-cli/src/api.ts: 1

1-1: LGTM!

Also applies to: 6-6, 27-36

apps/iracing-data-cli/src/services/request.ts: 1

24-24: LGTM!

Also applies to: 42-48, 121-126

apps/iracing-data-cli/src/token-output.ts: 1

60-60: LGTM!

apps/iracing-data-cli/src/commands/assets/car/schema.ts: 1

1-16: LGTM!

apps/iracing-data-cli/src/commands/assets/returned-data-api-link.ts: 1

1-7: LGTM!

apps/iracing-data-cli/src/commands/assets/car/service.ts: 1

1-41: LGTM!

apps/iracing-data-cli/src/commands/assets/car/command.ts: 1

1-1: LGTM!

Also applies to: 3-4, 7-9, 11-11, 18-18

apps/iracing-data-cli/src/commands/assets/car/index.ts: 1

1-6: LGTM!

apps/iracing-data-cli/test/assets.test.cjs: 1

22-267: LGTM!

apps/iracing-data-cli/src/commands/assets/README.md: 1

3-3: LGTM!

apps/iracing-data-cli/src/commands/assets/index.ts: 1

1-3: LGTM!

Also applies to: 14-27, 29-48

apps/iracing-data-cli/src/program.ts: 1

2-4: LGTM!

Also applies to: 15-16, 29-37

apps/iracing-data-cli/test/assets-parity.test.cjs: 1

52-59: LGTM!

apps/iracing-data-cli/test/series-assets.test.cjs: 1

43-50: LGTM!

apps/iracing-data-cli/test/track-assets.test.cjs: 1

43-50: LGTM!

📝 Walkthrough

The car-assets command now uses a service backed by shared request execution. It validates credentials, calls the generated car API, materializes the returned link, and publishes the asset document as JSON.

Changes: Car-assets service migration

Layer / File(s) Summary
Shared request capabilities
apps/iracing-data-cli/src/api.ts, apps/iracing-data-cli/src/services/request.ts, apps/iracing-data-cli/src/token-output.ts
The API facade adds a lazily cached CarApi client. The request service can materialize a response before output, and output labels now include Assets.
Car service and command
apps/iracing-data-cli/src/commands/assets/car/*, apps/iracing-data-cli/src/commands/assets/returned-data-api-link.ts
The car command validates credential-only options with fixed JSON output and delegates execution to CarAssetsService. The service calls getCarAssets(), materializes its returned link, and uses the shared request lifecycle. The previous car-assets scope implementation is removed.
CLI composition and validation
apps/iracing-data-cli/src/commands/assets/README.md, apps/iracing-data-cli/src/commands/assets/index.ts, apps/iracing-data-cli/src/program.ts, apps/iracing-data-cli/test/assets*.test.cjs, apps/iracing-data-cli/test/series-assets.test.cjs, apps/iracing-data-cli/test/track-assets.test.cjs
createProgram composes the car service with the shared request service and link materializer. Tests cover service execution, error handling, registration, and help output.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Refactor

Merge Risk: ⚪ Minimal · up to 78f8c

The car-assets migration has no identified merge-blocking issue after normal checks.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly identifies the main change: migrating car assets to the shared request execution flow.
Description check Passed The description includes the required summary, scope, generated files, release notes, validation, and reviewer notes. It also explains unavailable local checks and provides successful CI validation.
Docstring Coverage Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 14 files. (1 skipped: 1 …
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 14 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

A rabbit packed a carrot snack
Then watched the car API call back
A link became JSON, neat and bright
The shared request service got it right
The bunny hopped into the night

Comment @coderabbitai help to get the list of available commands.

@justinmakaila
justinmakaila force-pushed the refactor/cli-assets-car branch from d513d48 to 78f8c41 Compare October 19, 2026 03:19
@justinmakaila
justinmakaila merged commit 966bee7 into main Oct 19, 2026
7 checks passed
@justinmakaila
justinmakaila deleted the refactor/cli-assets-car branch October 19, 2026 17:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant