docs: add Astro Starlight documentation site deployed to Cloudflare Pages#20
Conversation
…ages Modeled on the upstream vercel-labs/emulate docs app (apps/web): guides (getting started, configuration, programmatic API, authentication), reference (architecture, emulator conventions), and 13 service pages generated from skills/*/SKILL.md via website/scripts/sync-services.ts. Deploys to the emulate-docs Cloudflare Pages project from CI.
|
Warning Review limit reached
Next review available in: 52 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds an Astro Starlight docs site under ChangesDocumentation website
Portless HTTPS support
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 101 |
| Duplication | 2 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Greptile SummaryThis PR adds an Astro Starlight documentation site under
Confidence Score: 4/5Safe to merge with one fix needed in the portless alias registration error path. The docs site and CI workflow are solid. The portless feature introduces a gap: packages/emulate/src/commands/start.ts and packages/emulate/src/portless.ts — the Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[startCommand] --> B[defaultBasePort]
A --> C[loadSeedConfig]
A --> D[resolveServices]
A --> E[validateBaseUrlOptions]
E -->|error| F[process.exit 1]
E -->|ok| G{portless?}
G -->|yes| H[setupPortless]
H --> I[ensurePortless]
I -->|not installed, non-TTY| F
I -->|not installed, TTY| J[prompt install]
J -->|declined| F
J -->|accepted| K[npm i -g portless]
K -->|fail| F
I -->|installed| L{proxy running?}
L -->|no| F
L -->|yes| M[buildAliases]
M --> N[registerAliases]
N -->|spawnSync fails| O[throws Error - no catch in setupPortless]
N -->|ok| P[process.on exit cleanup]
G -->|no| Q[createEmulator loop]
P --> Q
Q --> R[resolveBaseUrl per service]
R -->|1 seedBaseUrl| S[use seed URL]
R -->|2 baseUrl flag| T[use flag URL]
R -->|3 EMULATE_BASE_URL| U[use env URL]
R -->|4 PORTLESS_URL| V[use portless env]
R -->|5 fallback| W[http://localhost:port]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[startCommand] --> B[defaultBasePort]
A --> C[loadSeedConfig]
A --> D[resolveServices]
A --> E[validateBaseUrlOptions]
E -->|error| F[process.exit 1]
E -->|ok| G{portless?}
G -->|yes| H[setupPortless]
H --> I[ensurePortless]
I -->|not installed, non-TTY| F
I -->|not installed, TTY| J[prompt install]
J -->|declined| F
J -->|accepted| K[npm i -g portless]
K -->|fail| F
I -->|installed| L{proxy running?}
L -->|no| F
L -->|yes| M[buildAliases]
M --> N[registerAliases]
N -->|spawnSync fails| O[throws Error - no catch in setupPortless]
N -->|ok| P[process.on exit cleanup]
G -->|no| Q[createEmulator loop]
P --> Q
Q --> R[resolveBaseUrl per service]
R -->|1 seedBaseUrl| S[use seed URL]
R -->|2 baseUrl flag| T[use flag URL]
R -->|3 EMULATE_BASE_URL| U[use env URL]
R -->|4 PORTLESS_URL| V[use portless env]
R -->|5 fallback| W[http://localhost:port]
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
packages/emulate/src/commands/start.ts:103-108
**Unhandled throw from `registerAliases` surfaces as a raw exception**
`registerAliases` in `portless.ts` throws `new Error('Failed to register portless alias: …')` when any alias fails. Since `setupPortless` doesn't wrap it in try/catch, the error propagates as a rejected promise through `startCommand`. Commander.js will print the error, but the output is a raw `Error:` stack trace rather than the clean `console.error + process.exit(1)` pattern used everywhere else in the CLI (including `ensurePortless` itself). A user who hits a registration failure on their first `--portless` run will see a confusing stack trace instead of an actionable message.
Reviews (9): Last reviewed commit: "chore: move EMULATE_PORT/PORT fallback i..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Ignore website content markdown (like docs/**) and fix curly/regexp lint errors in sync-services.ts.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
4 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="website/src/content/docs/guides/configuration.md">
<violation number="1" location="website/src/content/docs/guides/configuration.md:87">
P2: The `EMULATE_PORT` and `PORT` environment variables are claimed as an alternative for setting the base port, but no code in the repository reads these env vars. The CLI only accepts `--port` (default 4000). Either remove this section or implement the env-var fallback in the CLI.</violation>
</file>
<file name=".github/workflows/docs.yml">
<violation number="1" location=".github/workflows/docs.yml:21">
P2: Missing concurrency group to cancel in-progress runs on duplicate PR pushes. All other workflows in this repo (`ci.yml`, `release.yml`) use a concurrency group with `cancel-in-progress: true` for PRs — without it, rapid pushes to a PR will queue instead of cancelling the stale run, wasting CI minutes.</violation>
</file>
<file name="website/src/content/docs/services/linear.md">
<violation number="1" location="website/src/content/docs/services/linear.md:35">
P2: The Start section is missing the "(linear's slot is \`4006\`)" parenthetical that the Asana and Firebase docs include. Without it, users reading this page in isolation don't know which port Linear occupies in a multi-service setup. The `-p 4012` example is also misleading since 4012 belongs to X (the 13th service). Either add the slot mention as the other services do, or change the example to `-p 4012` only if the intent is to demonstrate choosing a completely unrelated port — otherwise prefer `-p 4006` or omit the specific number. Both nits trace back to the SKILL.md source in skills/linear/SKILL.md and can be fixed there before re-syncing.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
- pin bun via packageManager in website/package.json (unambiguous bun-version-file) - extract duplicated toss-payments package-name mapping into a variable - fail CI when generated services/*.md drift from skills/
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
.github/workflows/docs.yml (2)
22-24: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winBun setup: caching enabled by default and unpinned tag reference.
Two separate static-analysis findings on this step:
oven-sh/setup-bun'sno-cacheinput defaults tofalse, so the downloaded Bun executable is cached by default — flagged by zizmor as a cache-poisoning risk since a poisoned cache entry could persist across runs.- The action is referenced by mutable tag (
@v2) rather than a pinned full-length commit SHA, so its behavior can change without review (also flagged forcloudflare/wrangler-action@v3at line 30).🔒 Proposed fix (pin + disable cache)
- - uses: oven-sh/setup-bun@v2 + - uses: oven-sh/setup-bun@<pinned-sha> # v2.x.x with: bun-version-file: package.json + no-cache: true🤖 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/docs.yml around lines 22 - 24, The `setup-bun` workflow step is using a mutable action tag and leaving Bun caching enabled by default. Update the `oven-sh/setup-bun` reference to a pinned full-length commit SHA, and set its `no-cache` input to disable executable caching. Apply the same pinning approach to any other action references in this workflow that still use version tags, such as the `wrangler` step.Source: Linters/SAST tools
30-30: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valuePin
cloudflare/wrangler-actionto a commit SHA.Same third-party-action-pinning concern as the setup-bun step: tag references (
@v3) are mutable and not immutable releases.🤖 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/docs.yml at line 30, The workflow step using cloudflare/wrangler-action is pinned to a mutable tag, so update the uses reference to a specific commit SHA instead of `@v3`. Locate the wrangler-action step in the docs workflow and replace the tag with an immutable commit hash, matching the pinning approach used for other third-party actions in this file.Source: Linters/SAST tools
🤖 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/docs.yml:
- Line 21: The docs workflow checkout step is leaving the default GITHUB_TOKEN
persisted in git config, which exposes credentials to later steps like bun
install and postinstall scripts. Update the actions/checkout usage in the docs
workflow to disable credential persistence by setting persist-credentials to
false, and keep the deploy step relying on its own Cloudflare token. Locate the
change at the checkout step in the docs workflow.
- Around line 28-35: The Cloudflare Pages deploy step in the docs workflow only
excludes pull requests, so manual runs can still deploy from non-main branches.
Update the deploy job’s guard on the Deploy to Cloudflare Pages step to also
require the main branch for workflow_dispatch, using the existing workflow
context around github.event_name and github.ref so only main can trigger
production deployment.
In `@website/package.json`:
- Line 11: The website package’s deploy workflow is missing the service-doc sync
step, so `deploy` can publish stale generated docs. Update the `deploy` script
in `package.json` to run the same `sync-services` step used by CI before `astro
build`, so `bun run deploy` always rebuilds from fresh `skills/*/SKILL.md`
content. Keep the existing `wrangler pages deploy dist` step after the build and
preserve the current script naming/entrypoint.
In `@website/src/content/docs/guides/configuration.md`:
- Around line 34-56: Normalize the YAML examples in the configuration docs so
they match the repo’s YAML style: update the tokens example and the supabase
mapping to use the preferred flow/block formatting consistently. Keep the
examples under the tokens, kakao, and supabase keys readable and stylistically
aligned with the rest of the documentation, avoiding mixed inline/flow styles
where the repo expects block style.
In `@website/src/content/docs/reference/conventions.md`:
- Around line 148-170: Normalize the example in the docs snippet to match the
repository style by updating the test setup around makeApp and the token test to
use single quotes, remove semicolons, and delete the unused beforeEach import
from bun:test. Keep the rest of the example intact while ensuring the import
list and string literals in the shown code follow the same markdown conventions
as nearby examples.
In `@website/src/content/docs/services/asana.md`:
- Around line 53-60: The token seeding docs in the Asana page are overstating
how login matching works. Update the `tokens` example and surrounding text to
describe only the actual `tokens[token] -> { login, scopes }` contract, and
remove any claim that `login` can match by email, gid, or name unless that
behavior is confirmed elsewhere. Keep the guidance aligned with the existing
`tokens` config shape in the docs content.
In `@website/src/content/docs/services/naver.md`:
- Around line 40-41: The JSON example in the Naver docs is malformed because the
error payload in the text uses comma-separated keys instead of key/value pairs.
Update the example in the Naver service documentation to show a valid JSON
object for the OAuth/token error response, keeping the same meaning but fixing
the punctuation so the payload shape is syntactically correct.
---
Nitpick comments:
In @.github/workflows/docs.yml:
- Around line 22-24: The `setup-bun` workflow step is using a mutable action tag
and leaving Bun caching enabled by default. Update the `oven-sh/setup-bun`
reference to a pinned full-length commit SHA, and set its `no-cache` input to
disable executable caching. Apply the same pinning approach to any other action
references in this workflow that still use version tags, such as the `wrangler`
step.
- Line 30: The workflow step using cloudflare/wrangler-action is pinned to a
mutable tag, so update the uses reference to a specific commit SHA instead of
`@v3`. Locate the wrangler-action step in the docs workflow and replace the tag
with an immutable commit hash, matching the pinning approach used for other
third-party actions in this file.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8daed626-e16a-481f-a2e0-220cab79d38a
⛔ Files ignored due to path filters (2)
website/bun.lockis excluded by!**/*.lockwebsite/public/favicon.svgis excluded by!**/*.svg
📒 Files selected for processing (29)
.github/workflows/docs.ymlwebsite/.gitignorewebsite/README.mdwebsite/astro.config.mjswebsite/package.jsonwebsite/scripts/sync-services.tswebsite/src/content.config.tswebsite/src/content/docs/guides/authentication.mdwebsite/src/content/docs/guides/configuration.mdwebsite/src/content/docs/guides/getting-started.mdwebsite/src/content/docs/guides/programmatic-api.mdwebsite/src/content/docs/index.mdxwebsite/src/content/docs/reference/architecture.mdwebsite/src/content/docs/reference/conventions.mdwebsite/src/content/docs/services/asana.mdwebsite/src/content/docs/services/autumn.mdwebsite/src/content/docs/services/firebase.mdwebsite/src/content/docs/services/gitlab.mdwebsite/src/content/docs/services/kakao.mdwebsite/src/content/docs/services/linear.mdwebsite/src/content/docs/services/naver.mdwebsite/src/content/docs/services/posthog.mdwebsite/src/content/docs/services/spotify.mdwebsite/src/content/docs/services/supabase.mdwebsite/src/content/docs/services/tosspayments.mdwebsite/src/content/docs/services/workos.mdwebsite/src/content/docs/services/x.mdwebsite/tsconfig.jsonwebsite/wrangler.jsonc
- pin docs workflow actions to full commit SHAs (repo convention), using mise-action to provision bun like ci.yml - iterate the service map instead of readdir in sync-services.ts
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/docs.yml">
<violation number="1" location=".github/workflows/docs.yml:27">
P2: The staleness check only validates tracked file diffs, so untracked generated service pages can slip through CI. `git diff --exit-code` reports changes for tracked files only; it exits 0 when only untracked files are present. If `bun scripts/sync-services.ts` creates a new `.md` file for a newly added skill that hasn't been committed yet, this step passes silently and the generated docs remain incomplete on `main`. Consider expanding the check to also detect untracked files, for example by using `git status --short -- src/content/docs/services`.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Covers using vercel-labs/portless as a local HTTPS proxy in front of the emulators via --base-url / per-service seed baseUrl, and notes that the upstream --portless flag is not implemented in this fork.
Ports vercel-labs/emulate portless integration: --portless auto-registers
<service>.emulate aliases (removed on shutdown, prompts install when
missing), and resolveBaseUrl gains the upstream fallback chain — seed
baseUrl > --base-url > EMULATE_BASE_URL > PORTLESS_URL > localhost — with
{service} interpolation. --base-url now works with multiple services when
it contains a {service} placeholder.
Docs: portless guide now documents the flag; README and getting-started
options updated.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
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 `@packages/emulate/src/__tests__/base-url.test.ts`:
- Around line 8-22: The resolveBaseUrl tests are only cleaning EMULATE_BASE_URL
and PORTLESS_URL in afterEach, so the first case can still read ambient
environment values before it runs. Update the base-url.test.ts setup around
describe('resolveBaseUrl') to clear both env vars before each test, while
keeping the existing restore logic in afterEach, so the defaults behavior is
isolated for the first test and any later cases.
In `@packages/emulate/src/commands/start.ts`:
- Line 112: The cleanup arrow in start.ts currently uses a shorthand body that
returns the void result of removeAliases, which triggers the void-expression
lint rule. Update the cleanup symbol to use a block body and call
removeAliases(aliases) inside it so the function explicitly returns nothing
while preserving the same cleanup behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b0e99ba1-ee6c-4fc7-96d0-ffc53ecf79de
📒 Files selected for processing (13)
.github/workflows/docs.ymlREADME.mdeslint.config.tspackages/emulate/src/__tests__/base-url.test.tspackages/emulate/src/base-url.tspackages/emulate/src/commands/start.tspackages/emulate/src/index.tspackages/emulate/src/portless.tswebsite/astro.config.mjswebsite/package.jsonwebsite/scripts/sync-services.tswebsite/src/content/docs/guides/getting-started.mdwebsite/src/content/docs/guides/portless.md
✅ Files skipped from review due to trivial changes (2)
- website/package.json
- README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- website/astro.config.mjs
- website/scripts/sync-services.ts
- website/src/content/docs/guides/getting-started.md
There was a problem hiding this comment.
2 issues found across 9 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/emulate/src/portless.ts">
<violation number="1" location="packages/emulate/src/portless.ts:71">
P2: `registerAliases` uses `portless alias … --force`, which overwrites any existing alias with the same name. If a later alias in the same batch fails, the rollback loop calls `removeAliases(registered)` and deletes every successfully processed alias name outright—including ones that existed before the call—instead of restoring their prior mappings. This breaks atomicity and can silently destroy pre-existing local portless configuration.
A concrete failure chain: alias `foo` already maps to port 9000; `registerAliases` overwrites it to port 3000 successfully; the next alias fails; rollback removes `foo` entirely rather than restoring `foo -> 9000`.
Consider checking whether an alias already exists before forcing the overwrite, and restoring the original mapping (or at least skipping removal of overwritten aliases) during rollback.</violation>
</file>
<file name="packages/emulate/src/__tests__/base-url.test.ts">
<violation number="1" location="packages/emulate/src/__tests__/base-url.test.ts:8">
P2: The `afterEach` block restores env vars after each test, but there's no `beforeEach` to clear them before the first test runs. If `EMULATE_BASE_URL` or `PORTLESS_URL` happen to be set in the ambient environment (e.g., in CI or a developer's shell), the first test case (`'defaults to localhost with the port'`) will resolve to the env value instead of the expected default, causing a flaky failure.
Adding a `beforeEach` that deletes both variables ensures consistent isolation from the first test onward.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Addresses the SonarCloud quality gate: extracts resolveServices / validateBaseUrlOptions / setupPortless from startCommand (cognitive complexity), moves alias construction into buildAliases, adds tests for alias registration/rollback/removal with a mocked child_process, and marks the intentional PATH-based portless invocations with NOSONAR.
… gate fixes - docs workflow now deploys every run: main goes to production, PR branches get a Cloudflare Pages preview (fork PRs skipped), following the wrangler-action recommended setup with gitHubToken deployment tracking; a sticky PR comment posts the preview + branch alias URLs - cover ensurePortless fully (mocked child_process/readline/process.exit) and fix the misplaced NOSONAR on the multi-line spawnSync call - exclude CLI wiring (index.ts, commands/) from Sonar coverage like the existing openapi.ts exclusion; add braces to a void-returning arrow (Codacy)
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name=".github/workflows/docs.yml">
<violation number="1" location=".github/workflows/docs.yml:21">
P2: Missing concurrency group to cancel in-progress runs on duplicate PR pushes. All other workflows in this repo (`ci.yml`, `release.yml`) use a concurrency group with `cancel-in-progress: true` for PRs — without it, rapid pushes to a PR will queue instead of cancelling the stale run, wasting CI minutes.</violation>
</file>
<file name="packages/emulate/src/portless.ts">
<violation number="1" location="packages/emulate/src/portless.ts:71">
P2: `registerAliases` uses `portless alias … --force`, which overwrites any existing alias with the same name. If a later alias in the same batch fails, the rollback loop calls `removeAliases(registered)` and deletes every successfully processed alias name outright—including ones that existed before the call—instead of restoring their prior mappings. This breaks atomicity and can silently destroy pre-existing local portless configuration.
A concrete failure chain: alias `foo` already maps to port 9000; `registerAliases` overwrites it to port 3000 successfully; the next alias fails; rollback removes `foo` entirely rather than restoring `foo -> 9000`.
Consider checking whether an alias already exists before forcing the overwrite, and restoring the original mapping (or at least skipping removal of overwritten aliases) during rollback.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
- linear/naver skill docs: add port-slot note, fix invalid JSON example (regenerated website service pages) - implement EMULATE_PORT/PORT env fallback for the base port so the documented behavior is real (start-options.ts, tested) - move validateBaseUrlOptions out of commands/ so it stays under coverage gates - base-url.test.ts: clear env in beforeEach so ambient vars can't pollute the first test - website deploy script: sync service pages before building - docs.yml: staleness check now catches untracked generated pages; deploy step skips (instead of failing) when Cloudflare secrets are absent - normalize the conventions test example to repo code style
There was a problem hiding this comment.
1 issue found across 12 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
- docs.yml: add concurrency group (repo convention) and persist-credentials: false on checkout - clarify linear port-slot wording (positional assignment) - avoid dynamically computed env keys (Codacy object-injection)
Add braces to a void-returning arrow in the readline mock and hoist the defaultBasePort() call to a const (Codacy misreads the inline call as an HTML-context sink).
Codacy misflags any variable near the '<port>' help text as an HTML sink; resolving the default inside startCommand keeps index.ts free of the pattern and the behavior identical (flag > env > 4000).
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/docs.yml (1)
33-33: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valuemise-action caching flagged for cache-poisoning by static analysis.
jdx/mise-actioncaches tool installs by default (cache: true). Since this job also deploys to production (Cloudflare Pages) on pushes tomain, a poisoned cache entry could theoretically be restored in a privileged run. Risk is low-confidence per zizmor (this isn't a tag/release-triggered publish workflow), but consider pinning a scopedcache_keyor disabling caching if it's not needed for a docs-only build.Optional: disable or scope caching
- uses: jdx/mise-action@1648a7812b9aeae629881980618f079932869151 # v4.0.1 + with: + cache: false🤖 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/docs.yml at line 33, The docs workflow uses mise-action with default caching, which can be risky in a production-deploying job. Update the mise-action step in the docs workflow to either disable caching or provide a scoped cache_key so cached tool installs cannot be reused across privileged runs. Keep the change focused on the jdx/mise-action step and preserve the existing docs deploy behavior.Source: Linters/SAST tools
🤖 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/emulate/src/__tests__/base-url.test.ts`:
- Around line 6-28: The cleanup in `afterAll` only restores `savedEnv` entries
that were originally defined, so any env var that started unset (like
`EMULATE_PORT` or `PORT`) can remain polluted after the tests. Update the
`afterAll` restore logic in `base-url.test.ts` to explicitly delete
`process.env` keys whose saved value is `undefined`, and restore only the
defined ones, using the existing `savedEnv` object and env var names already
captured at the top of the test file.
In `@packages/emulate/src/commands/start.ts`:
- Around line 92-119: The CLI entrypoint is still using a synchronous parse, so
async command failures from startCommand can surface as unhandled rejections.
Update the program.parse() call in the index entrypoint to use
program.parseAsync(process.argv), and add rejection handling there so errors are
reported cleanly to the user instead of bubbling uncaught.
---
Nitpick comments:
In @.github/workflows/docs.yml:
- Line 33: The docs workflow uses mise-action with default caching, which can be
risky in a production-deploying job. Update the mise-action step in the docs
workflow to either disable caching or provide a scoped cache_key so cached tool
installs cannot be reused across privileged runs. Keep the change focused on the
jdx/mise-action step and preserve the existing docs deploy behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fa11c019-ac26-40cd-a097-747498125866
📒 Files selected for processing (17)
.github/workflows/docs.ymlREADME.mddocs/EMULATOR-CONVENTIONS.mdpackages/emulate/src/__tests__/base-url.test.tspackages/emulate/src/__tests__/portless.test.tspackages/emulate/src/commands/start.tspackages/emulate/src/index.tspackages/emulate/src/portless.tspackages/emulate/src/start-options.tsskills/linear/SKILL.mdskills/naver/SKILL.mdsonar-project.propertieswebsite/README.mdwebsite/package.jsonwebsite/src/content/docs/reference/conventions.mdwebsite/src/content/docs/services/linear.mdwebsite/src/content/docs/services/naver.md
✅ Files skipped from review due to trivial changes (6)
- sonar-project.properties
- docs/EMULATOR-CONVENTIONS.md
- website/src/content/docs/services/linear.md
- website/src/content/docs/reference/conventions.md
- website/README.md
- website/src/content/docs/services/naver.md
🚧 Files skipped from review as they are similar to previous changes (2)
- README.md
- website/package.json
| const savedEnv = { | ||
| EMULATE_BASE_URL: process.env.EMULATE_BASE_URL, | ||
| PORTLESS_URL: process.env.PORTLESS_URL, | ||
| EMULATE_PORT: process.env.EMULATE_PORT, | ||
| PORT: process.env.PORT, | ||
| } | ||
|
|
||
| // Clear before each test (not just after) so ambient shell/CI env vars | ||
| // cannot leak into the first test case. | ||
| beforeEach(() => { | ||
| delete process.env.EMULATE_BASE_URL | ||
| delete process.env.PORTLESS_URL | ||
| delete process.env.EMULATE_PORT | ||
| delete process.env.PORT | ||
| }) | ||
|
|
||
| afterAll(() => { | ||
| for (const [name, value] of Object.entries(savedEnv)) { | ||
| if (value !== undefined) { | ||
| Object.assign(process.env, { [name]: value }) | ||
| } | ||
| } | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
afterAll doesn't clear env vars that were originally unset.
Only vars with an original defined value get restored; vars that started undefined (e.g. EMULATE_PORT/PORT in most shells) are left with whatever the last test set them to, since nothing calls delete for the undefined case. This can leak stray PORT/EMULATE_PORT values into other test files sharing the same worker process.
💚 Proposed fix
afterAll(() => {
for (const [name, value] of Object.entries(savedEnv)) {
if (value !== undefined) {
Object.assign(process.env, { [name]: value })
}
+ else {
+ delete process.env[name]
+ }
}
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const savedEnv = { | |
| EMULATE_BASE_URL: process.env.EMULATE_BASE_URL, | |
| PORTLESS_URL: process.env.PORTLESS_URL, | |
| EMULATE_PORT: process.env.EMULATE_PORT, | |
| PORT: process.env.PORT, | |
| } | |
| // Clear before each test (not just after) so ambient shell/CI env vars | |
| // cannot leak into the first test case. | |
| beforeEach(() => { | |
| delete process.env.EMULATE_BASE_URL | |
| delete process.env.PORTLESS_URL | |
| delete process.env.EMULATE_PORT | |
| delete process.env.PORT | |
| }) | |
| afterAll(() => { | |
| for (const [name, value] of Object.entries(savedEnv)) { | |
| if (value !== undefined) { | |
| Object.assign(process.env, { [name]: value }) | |
| } | |
| } | |
| }) | |
| const savedEnv = { | |
| EMULATE_BASE_URL: process.env.EMULATE_BASE_URL, | |
| PORTLESS_URL: process.env.PORTLESS_URL, | |
| EMULATE_PORT: process.env.EMULATE_PORT, | |
| PORT: process.env.PORT, | |
| } | |
| // Clear before each test (not just after) so ambient shell/CI env vars | |
| // cannot leak into the first test case. | |
| beforeEach(() => { | |
| delete process.env.EMULATE_BASE_URL | |
| delete process.env.PORTLESS_URL | |
| delete process.env.EMULATE_PORT | |
| delete process.env.PORT | |
| }) | |
| afterAll(() => { | |
| for (const [name, value] of Object.entries(savedEnv)) { | |
| if (value !== undefined) { | |
| Object.assign(process.env, { [name]: value }) | |
| } else { | |
| delete process.env[name] | |
| } | |
| } | |
| }) |
🤖 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/emulate/src/__tests__/base-url.test.ts` around lines 6 - 28, The
cleanup in `afterAll` only restores `savedEnv` entries that were originally
defined, so any env var that started unset (like `EMULATE_PORT` or `PORT`) can
remain polluted after the tests. Update the `afterAll` restore logic in
`base-url.test.ts` to explicitly delete `process.env` keys whose saved value is
`undefined`, and restore only the defined ones, using the existing `savedEnv`
object and env var names already captured at the top of the test file.
| async function setupPortless(services: ServiceName[], basePort: number): Promise<void> { | ||
| await ensurePortless() | ||
| const aliases: PortlessAlias[] = buildAliases(services, basePort) | ||
| registerAliases(aliases) | ||
| process.on('exit', () => { | ||
| removeAliases(aliases) | ||
| }) | ||
| } | ||
|
|
||
| if (options.baseUrl && services.length > 1) { | ||
| console.error('--base-url can only be used with a single service (--service).') | ||
| export async function startCommand(options: StartOptions): Promise<void> { | ||
| const { port: basePort } = options | ||
|
|
||
| const loaded = loadSeedConfig(options.seed) | ||
| const seedConfig = loaded?.config ?? null | ||
| const configSource = loaded?.source ?? null | ||
|
|
||
| const services = resolveServices(options, seedConfig) | ||
|
|
||
| const optionError = validateBaseUrlOptions(options, services.length) | ||
| if (optionError) { | ||
| console.error(optionError) | ||
| process.exit(1) | ||
| } | ||
|
|
||
| if (options.portless) { | ||
| await setupPortless(services, basePort) | ||
| } | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how startCommand() is invoked and whether rejections are caught
rg -n -B5 -A10 'startCommand\(' packages/emulate/src/index.tsRepository: pleaseai/emulate
Length of output: 154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== files ==\n'
git ls-files 'packages/emulate/src/**' | sed -n '1,200p'
printf '\n== startCommand references ==\n'
rg -n --hidden --glob '!**/node_modules/**' 'startCommand\s*\(' packages/emulate/src || true
printf '\n== portless alias implementation ==\n'
rg -n --hidden --glob '!**/node_modules/**' 'registerAliases|removeAliases|ensurePortless|buildAliases' packages/emulate/src || trueRepository: pleaseai/emulate
Length of output: 3267
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== locate likely entrypoints ==\n'
fd -a 'index.ts' packages/emulate/src packages/emulate || true
fd -a 'main.ts' packages/emulate/src packages/emulate || true
fd -a 'cli.ts' packages/emulate/src packages/emulate || true
printf '\n== call sites for startCommand ==\n'
rg -n --hidden --glob '!**/node_modules/**' 'startCommand' packages/emulate || true
printf '\n== start.ts context ==\n'
sed -n '1,220p' packages/emulate/src/commands/start.tsRepository: pleaseai/emulate
Length of output: 5435
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== all references to startCommand ==\n'
rg -n --hidden --glob '!**/node_modules/**' 'startCommand' .
printf '\n== portless helpers ==\n'
rg -n --hidden --glob '!**/node_modules/**' 'function (setupPortless|registerAliases|removeAliases|ensurePortless|buildAliases)|const (setupPortless|registerAliases|removeAliases|ensurePortless|buildAliases)|export (async )?function (setupPortless|registerAliases|removeAliases|ensurePortless|buildAliases)' packages/emulate/src
printf '\n== package files ==\n'
git ls-files 'packages/emulate/**' | sed -n '1,300p'Repository: pleaseai/emulate
Length of output: 1599
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== find likely CLI entrypoints ==\n'
fd -a -I 'index.ts' . || true
fd -a -I 'cli.ts' . || true
fd -a -I 'main.ts' . || true
printf '\n== search for emulator start wiring ==\n'
rg -n --hidden --glob '!**/node_modules/**' 'startCommand|setupPortless|validateBaseUrlOptions|resolveServices' .
printf '\n== package.json bin/scripts for emulate ==\n'
rg -n --hidden --glob '!**/node_modules/**' '"bin"|emulate|startCommand' packages/emulate package.json .github . || trueRepository: pleaseai/emulate
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n --hidden --glob '!**/node_modules/**' 'startCommand\(.*\)\.catch|catch\(\s*(err|error)|await startCommand|startCommand\(' packages/emulate .Repository: pleaseai/emulate
Length of output: 392
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' packages/emulate/src/index.ts
printf '\n== package version ==\n'
sed -n '1,120p' packages/emulate/package.jsonRepository: pleaseai/emulate
Length of output: 3713
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' packages/emulate/src/portless.ts
printf '\n== portless tests ==\n'
sed -n '55,95p' packages/emulate/src/__tests__/portless.test.tsRepository: pleaseai/emulate
Length of output: 4697
🌐 Web query:
Commander.js async action handler rejection parse() parseAsync error handling
💡 Result:
When working with asynchronous action handlers in Commander.js, you must use.parseAsync instead of.parse to ensure that the command's promise-based lifecycle is managed correctly [1][2]. Error Handling and parseAsync Because.parseAsync returns a promise, errors occurring within async action handlers (or during parsing) will result in a promise rejection [3]. If these rejections are not handled, they can lead to unhandled promise rejections, which may not consistently terminate the process with a non-zero exit code depending on your Node.js environment [4]. To implement robust error handling: 1. Always use await or.catch with.parseAsync: Wrap your.parseAsync call in an async function and use try/catch or promise chaining to handle errors [5][3]. async function main { try { await program.parseAsync(process.argv); } catch (err) { console.error('Error:', err.message); process.exit(1); } } main; 2. Use.exitOverride: By default, Commander calls process.exit when it encounters errors (like missing arguments). To prevent this and allow your own error-handling logic to catch these as exceptions instead, call.exitOverride before parsing [1][2]. program.exitOverride.parseAsync(process.argv).catch(err => { // Handle Commander-specific errors (e.g., help, version, validation errors) if (err.code === 'commander.help') process.exit(0); console.error('CLI Error:', err.message); process.exit(1); }); Important Considerations - If any action handler or hook is asynchronous, you should use.parseAsync [1][6][3]. While.parse might sometimes appear to work with async code, it is not supported for this use case and can lead to race conditions or silent failures [7]. - CommanderError: When.exitOverride is active, Commander throws a CommanderError for standard CLI events (like help or version display) and validation failures. You can check the error code or exitCode property to distinguish between these cases [2]. - Async Limitations: Note that the callback provided to.exitOverride is currently synchronous. If you need to perform asynchronous cleanup before exiting, you should rely on catching the error from the.parseAsync promise chain rather than the.exitOverride callback [8].
Citations:
- 1: https://github.com/tj/commander.js/tree/master?tab=readme-ov-file
- 2: https://github.com/tj/commander.js/blob/v15.0.0/Readme.md
- 3: Add parseAsync tj/commander.js#1118
- 4: Exit status when exception is thrown tj/commander.js#1414
- 5: Better error handling for parseAsync() tj/commander.js#1144
- 6: https://github.com/tj/commander.js/blob/master/Readme.md
- 7: parse() works for async action handler in subcommand tj/commander.js#1681
- 8: Feat: async
exitOverridetj/commander.js#2446
Use parseAsync() for the CLI entrypoint
packages/emulate/src/index.ts:23 calls program.parse(), so rejections from async actions like startCommand() can escape as an unhandled promise rejection. Switch this to program.parseAsync(process.argv) and handle the rejection to print a clean CLI 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 `@packages/emulate/src/commands/start.ts` around lines 92 - 119, The CLI
entrypoint is still using a synchronous parse, so async command failures from
startCommand can surface as unhandled rejections. Update the program.parse()
call in the index entrypoint to use program.parseAsync(process.argv), and add
rejection handling there so errors are reported cleanly to the user instead of
bubbling uncaught.
|
|
||
| const loaded = loadSeedConfig(options.seed) | ||
| const seedConfig = loaded?.config ?? null | ||
| const configSource = loaded?.source ?? null | ||
|
|
||
| const services = resolveServices(options, seedConfig) |
There was a problem hiding this comment.
registerAliases surfaces as a raw exception
registerAliases in portless.ts throws new Error('Failed to register portless alias: …') when any alias fails. Since setupPortless doesn't wrap it in try/catch, the error propagates as a rejected promise through startCommand. Commander.js will print the error, but the output is a raw Error: stack trace rather than the clean console.error + process.exit(1) pattern used everywhere else in the CLI (including ensurePortless itself). A user who hits a registration failure on their first --portless run will see a confusing stack trace instead of an actionable message.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/emulate/src/commands/start.ts
Line: 103-108
Comment:
**Unhandled throw from `registerAliases` surfaces as a raw exception**
`registerAliases` in `portless.ts` throws `new Error('Failed to register portless alias: …')` when any alias fails. Since `setupPortless` doesn't wrap it in try/catch, the error propagates as a rejected promise through `startCommand`. Commander.js will print the error, but the output is a raw `Error:` stack trace rather than the clean `console.error + process.exit(1)` pattern used everywhere else in the CLI (including `ensurePortless` itself). A user who hits a registration failure on their first `--portless` run will see a confusing stack trace instead of an actionable message.
How can I resolve this? If you propose a fix, please make it concise.
2 participants
Summary
Adds a documentation website under
website/, built with Astro Starlight and deployed to Cloudflare Pages. The information architecture mirrors the original vercel-labs/emulate docs app (apps/web), adapted to this fork's 13 services.emulate.config.yaml,tokens, seeding rules), Programmatic API (createEmulator/reset/close), Authentication (OAuth login page,?user_id=CI auto-approve, per-service schemes)skills/*/SKILL.mdviawebsite/scripts/sync-services.ts(bun run sync:services); skills stay the single source of truthdocs/EMULATOR-CONVENTIONS.md)Deployment
website/wrangler.jsonctargets a Cloudflare Pages projectemulate-docs(pages_build_output_dir: dist).github/workflows/docs.ymlbuilds on PRs and deploys on pushes tomaintouchingwebsite/,skills/, ordocs/CLOUDFLARE_API_TOKEN/CLOUDFLARE_ACCOUNT_IDrepository secrets before the deploy step can runTest plan
bun run build— 21 pages built, Pagefind search index generatedastro previewsmoke test —/, guides, service pages, reference all return 200; unknown path returns 404Summary by cubic
Adds a docs site under
website/using@astrojs/starlight, deployed to Cloudflare Pages with PR preview comments. Ports--portlessHTTPS and improved base URL handling to the CLI; adds base-port env fallback and resolves lint/Codacy nits.New Features
website/scripts/sync-services.ts; CI builds on every run, deploysmainto production, PRs get preview URLs with a sticky comment; stale generated pages fail the build; workflow uses a concurrency group, pins actions by SHA, and skips deploy when Cloudflare secrets are missing. README links the live docs at https://emulate.pleaseai.dev.--portlessauto-registers<service>.emulatealiases (prompts install; requiresportless proxy start; cleans up on exit) and advertises HTTPS URLs; base URL precedence is seedbaseUrl→--base-url(supports{service}) →EMULATE_BASE_URL→PORTLESS_URL→http://localhost:<port>; default base port falls back toEMULATE_PORTorPORT; options validation rejects--portlesswith--base-urland requires{service}for multi-service--base-url; tests cover interpolation, env fallbacks, and alias registration/rollback.Migration
emulate-docs(outputdist) and add repo secretsCLOUDFLARE_API_TOKENandCLOUDFLARE_ACCOUNT_ID.cd website && bun install && bun run dev. Optional HTTPS:npm i -g portlessandportless proxy startbefore using--portless.Written for commit b645fdb. Summary will update on new commits.
Summary by CodeRabbit
{service}interpolation and clearer precedence across seed/config/env values.--portless.