Skip to content

portless worktree and local settings modes#261

Open
jasonmorais wants to merge 14 commits into
mainfrom
jason/portless-worktrees
Open

portless worktree and local settings modes#261
jasonmorais wants to merge 14 commits into
mainfrom
jason/portless-worktrees

Conversation

@jasonmorais
Copy link
Copy Markdown
Contributor

@jasonmorais jasonmorais commented May 19, 2026

Summary by Sourcery

Introduce worktree-aware local and test infrastructure for MongoDB, Azurite, and portless-proxied dev servers, and centralize verification seed data and environment handling.

New Features:

  • Add worktree-scoped port and hostname utilities for local dev and e2e runs, including portless hostname helpers and deterministic MongoDB/Azurite port allocation.
  • Introduce reusable MongoDB seed data and context abstractions for verification tests, including deterministic end-user seed documents.
  • Add dedicated start scripts for API, UI apps, OAuth2 mock, and Mongo memory mock that apply worktree-aware overrides and standardized child-process exit handling.
  • Add an Azurite test server implementation and wire it into the e2e shared infrastructure alongside existing API, OAuth2, and Vite servers.

Enhancements:

  • Refactor MongoDB test server to always run against an in-memory replica set with centralized seeding and optional Mongoose attachment, simplifying reachability logic.
  • Simplify and harden e2e server management by centralizing spawn environment handling, portless executable resolution, and readiness probing for portless-backed servers.
  • Derive e2e OIDC and API URLs from shared portless hostname configuration and .env.e2e files, reducing duplication and divergence between local dev and tests.
  • Make OAuth2 mock portal discovery prefer process.env for client IDs and redirect URIs, allowing worktree-specific overrides.
  • Standardize dev child-process exit handling across apps via a shared helper and reuse it in API and docs dev runners.

Build:

  • Extend app and workspace package scripts with worktree-specific dev and verify commands, including worktree-aware dev:worktree and test:e2e:worktree flows.
  • Replace inline Azurite startup and local.settings copying with dedicated scripts that generate e2e-focused local settings and start Azurite with worktree-specific ports.
  • Adjust pnpm workspace overrides to pin additional transitive dependencies and address security or compatibility issues.

@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented May 19, 2026

Reviewer's Guide

Implements worktree-aware local dev and e2e infrastructure by introducing deterministic per-worktree ports/hostnames, adding Azurite + Mongo seeding support, and refactoring test servers and scripts to run API/UI/OAuth2/dev tools through portless in a worktree-scoped way.

Sequence diagram for starting a worktree-scoped UI dev server

sequenceDiagram
  actor Dev
  participant RootPkg as root package.json
  participant Portless as portless proxy
  participant UiDev as ui-community start-dev.mjs
  participant Hostnames as portless-hostnames.mjs
  participant ViteArgs as vite-dev-args.mjs
  participant Vite as vite dev server

  Dev->>RootPkg: run "pnpm dev:worktree"
  RootPkg->>Portless: proxy start --https -p 1355
  RootPkg->>UiDev: run "dev:worktree" (WORKTREE_NAME set)

  UiDev->>Hostnames: getHostnames()
  Hostnames-->>UiDev: worktree hostnames
  UiDev->>UiDev: set VITE_* env via buildPortlessUrl()

  UiDev->>ViteArgs: buildViteArgs({ host, port, env })
  ViteArgs-->>UiDev: Vite CLI args (mode e2e when E2E/E2E_VITE_MODE)

  UiDev->>Vite: spawn('vite', args, { env })
  Vite-->>UiDev: exit(code, signal)
  UiDev->>UiDev: forwardChildExit(child)
Loading

File-Level Changes

Change Details Files
Refactor MongoDBTestServer to own the in-memory replica set lifecycle and use a shared seeding API with deterministic end-user documents.
  • Replace reliance on apiSettings/local Cosmos config with explicit MongoMemoryReplicaSetConfig including default db/port and optional binaryVersion.
  • Update startMongoMemoryReplicaSet to return a connection string and always start its own replica set, storing the disposer and seeding via a new seed(context) helper.
  • Introduce resetForScenario that clears collections via MongoDBSeedContext and reseeds test data, and expose attachMongoose/clearMongooseModels internals.
  • Remove legacy seedOwnerCommunityReferenceData and apiSettings-based reachability logic in favor of the shared seedDatabase entry point and new clearDatabase helper.
packages/ocom-verification/verification-shared/src/servers/test-mongodb-server.ts
packages/cellix/server-mongodb-memory-mock-seedwork/src/index.ts
packages/ocom-verification/verification-shared/src/test-data/seed/seed.ts
packages/ocom-verification/verification-shared/src/test-data/seed/end-users.ts
packages/ocom-verification/verification-shared/src/test-data/index.ts
packages/ocom-verification/acceptance-api/src/shared/support/shared-infrastructure.ts
Introduce worktree-aware port and hostname utilities and wire them into dev and test runners for MongoDB, Azurite, Vite, API, and OAuth2 mock servers.
  • Add scripts/local-dev/worktree-ports.mjs and a parallel TS helper for e2e tests to compute per-worktree Mongo/Azurite ports and build Azurite/Mongo connection strings.
  • Add scripts/local-dev/portless-hostnames.mjs and verification-shared portless-settings.ts to derive base hostnames from UI .env and apply WORKTREE_NAME suffixes, plus buildPortlessUrl helpers.
  • Update apps/api/ui-community/ui-staff/server-oauth2-mock/docs dev flows to use new start-dev.mjs wrappers, worktree-specific dev:worktree scripts, and a shared dev-process-exit helper.
  • Add TestAzuriteServer in e2e support that starts Azurite via the API app’s start-azurite.mjs using worktree ports and treats EADDRINUSE as an already-running instance.
  • Update test-environment/test-environment helpers to start the portless proxy in-process, expose getHostnames/buildPortlessUrl from shared settings, and consume .env.e2e files for UI defaults.
scripts/local-dev/worktree-ports.mjs
scripts/local-dev/portless-hostnames.mjs
scripts/local-dev/dev-process-exit.mjs
scripts/local-dev/vite-dev-args.mjs
apps/api/start-dev.mjs
apps/api/start-azurite.mjs
apps/api/scripts/sync-local-settings.mjs
apps/ui-community/start-dev.mjs
apps/ui-staff/start-dev.mjs
apps/server-oauth2-mock/start-dev.mjs
apps/server-mongodb-memory-mock/start-mongo.mjs
apps/docs/start-dev.mjs
packages/ocom-verification/verification-shared/src/settings/portless-settings.ts
packages/ocom-verification/e2e-tests/src/shared/support/servers/worktree-ports.ts
packages/ocom-verification/e2e-tests/src/shared/support/servers/test-azurite-server.ts
packages/ocom-verification/e2e-tests/src/shared/support/servers/app-paths.ts
packages/ocom-verification/e2e-tests/src/shared/support/servers/dev-script.ts
packages/ocom-verification/e2e-tests/src/shared/support/servers/child-process-env.ts
packages/ocom-verification/e2e-tests/src/shared/support/servers/resolve-portless.ts
packages/ocom-verification/e2e-tests/src/shared/support/servers/test-environment.ts
Rework e2e server abstractions (PortlessServer, TestApiServer, Vite servers, OAuth2 server) to use shared worktree-aware env wiring and portless paths.
  • Simplify PortlessServer to spawn using getPortlessPath and spawnEnv, add startupTimeoutMs override, and tighten lifecycle/error handling, including guarding exit handler when already ready.
  • Change TestApiServer/TestCommunityViteServer/TestStaffViteServer/TestOAuth2Server to derive hostnames from getHostnames, use getPortlessDevScript to pick dev vs dev:worktree, and inject env via e2eEnv().
  • Stop running API predev in TestApiServer directly and instead rely on the app’s own scripts, while keeping GraphQL health probe logic intact.
  • Expose new helpers and servers from the e2e servers index and adjust shared-infrastructure to start MongoDB, Azurite, OAuth2, API, and Vite servers in phases with worktree-aware ports and URLs, disabling parallel Cucumber workers.
  • Ensure test-environment initializes the portless proxy, exposes mockOidc URLs using shared hostnames, and loads .env.e2e defaults before tests run.
packages/ocom-verification/e2e-tests/src/shared/support/servers/portless-server.ts
packages/ocom-verification/e2e-tests/src/shared/support/servers/test-api-server.ts
packages/ocom-verification/e2e-tests/src/shared/support/servers/test-community-vite-server.ts
packages/ocom-verification/e2e-tests/src/shared/support/servers/test-staff-vite-server.ts
packages/ocom-verification/e2e-tests/src/shared/support/servers/test-oauth2-server.ts
packages/ocom-verification/e2e-tests/src/shared/support/servers/index.ts
packages/ocom-verification/e2e-tests/src/shared/support/shared-infrastructure.ts
packages/ocom-verification/e2e-tests/src/shared/support/test-environment.ts
packages/ocom-verification/e2e-tests/cucumber.js
Align OAuth2 mock and e2e login with real test actors and support worktree-specific redirect URIs.
  • Update oauth2-login helpers to wait for the mock /login page and fill the CommunityOwner actor email instead of a hardcoded test address.
  • Modify server-oauth2-mock portal discovery to let process.env override client IDs and redirect URIs so worktree-specific values injected by dev scripts are honored.
  • Introduce mock-oidc.users.json and .env.e2e placeholders for UI apps to define test actors and OIDC config used in e2e mode.
packages/ocom-verification/e2e-tests/src/shared/support/oauth2-login.ts
apps/server-oauth2-mock/src/portal-discovery.ts
apps/ui-community/mock-oidc.users.json
apps/ui-community/.env.e2e
apps/ui-staff/.env.e2e
Update workspace scripts and dependency overrides to support verify and worktree-aware e2e flows.
  • Add root dev:worktree, proxy:ensure, and test:e2e:worktree commands that set WORKTREE_NAME from the current directory and reuse the shared portless proxy.
  • Switch verify to run test:e2e:worktree and narrow test:acceptance to specific verification packages; add app-level dev:worktree scripts for API, UI, docs, Mongo mock, and OAuth2 apps.
  • Move Azurite dependency from root into apps/api, add coverage/CPD exclusions for new local-dev scripts, and extend pnpm overrides for Azurite, uuid, ws, shell-quote, and OpenTelemetry packages.
  • Tidy various configs and readme text (e.g., Docusaurus phrasing) without changing behavior.
package.json
apps/api/package.json
apps/ui-community/package.json
apps/ui-staff/package.json
apps/docs/package.json
apps/server-oauth2-mock/package.json
apps/server-mongodb-memory-mock/package.json
pnpm-workspace.yaml
sonar-project.properties
readme.md

Possibly linked issues

  • #0: PR implements portless proxy orchestration, ownercommunity hostnames, app dev/dev:worktree scripts, and mock service wiring requested by issue.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown
Contributor Author

@jasonmorais jasonmorais left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments left by myself - the main thing being to try and clean up some of the rough edges of this implementation - and look at the work from a wholistic view.

Comment thread apps/ui-community/mock-oidc.users.json
Comment thread packages/ocom-verification/archunit-tests/src/validate-env-names.cjs Outdated
Comment thread packages/ocom-verification/e2e-tests/src/shared/support/servers/port-ready.ts Outdated
Comment thread packages/ocom-verification/e2e-tests/turbo.json
Comment thread packages/ocom-verification/verification-shared/src/settings/portless-settings.ts Outdated
Comment thread scripts/local-dev/dev-process-exit.mjs
Comment thread scripts/local-dev/port-ready.mjs Outdated
Comment thread scripts/local-dev/portless-hostnames.mjs
@jasonmorais jasonmorais marked this pull request as ready for review May 28, 2026 17:45
@jasonmorais jasonmorais requested a review from a team May 28, 2026 17:45
@jasonmorais jasonmorais requested a review from a team as a code owner May 28, 2026 17:45
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 4 issues, and left some high level feedback:

  • The worktree port offset logic is duplicated between scripts/local-dev/worktree-ports.mjs and e2e-tests/.../worktree-ports.ts; consider sharing a single implementation (or at least a common hashing helper) to avoid drift if the algorithm changes.
  • Hostname derivation is now implemented in both scripts/local-dev/portless-hostnames.mjs (from .env files) and verification-shared/src/settings/portless-settings.ts (from environment variables); it may be worth consolidating or clearly separating responsibilities to avoid subtle mismatches between local-dev and test/e2e environments.
  • In several places you rely on env vars like VITE_* being present at runtime (e.g. portless-settings.getHostnames() throws if they are missing); if these modules are imported in non-e2e/non-dev contexts, consider lazy evaluation or more defensive error messages so that missing configuration fails closer to the actual entrypoints that require it.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The worktree port offset logic is duplicated between `scripts/local-dev/worktree-ports.mjs` and `e2e-tests/.../worktree-ports.ts`; consider sharing a single implementation (or at least a common hashing helper) to avoid drift if the algorithm changes.
- Hostname derivation is now implemented in both `scripts/local-dev/portless-hostnames.mjs` (from .env files) and `verification-shared/src/settings/portless-settings.ts` (from environment variables); it may be worth consolidating or clearly separating responsibilities to avoid subtle mismatches between local-dev and test/e2e environments.
- In several places you rely on env vars like `VITE_*` being present at runtime (e.g. `portless-settings.getHostnames()` throws if they are missing); if these modules are imported in non-e2e/non-dev contexts, consider lazy evaluation or more defensive error messages so that missing configuration fails closer to the actual entrypoints that require it.

## Individual Comments

### Comment 1
<location path="apps/ui-community/start-dev.mjs" line_range="16" />
<code_context>
+	childEnv.VITE_APP_UI_COMMUNITY_BASE_URL = buildPortlessUrl(hostnames.uiCommunity);
+}
+
+const viteArgs = ['--port', process.env.PORT, '--host', process.env.HOST ?? '127.0.0.1'];
+const viteMode = process.env.E2E_VITE_MODE ?? (isE2E() || process.env.TF_BUILD ? 'e2e' : undefined);
+if (viteMode) {
</code_context>
<issue_to_address>
**issue (bug_risk):** Passing `process.env.PORT` directly can result in Vite receiving the string `'undefined'` as a port.

If `PORT` is unset, this array will pass the literal string `'undefined'` as the port, which can cause Vite to fail or bind incorrectly. Either skip `--port` when `PORT` is missing, or derive a safe fallback first (e.g. `const port = process.env.PORT ?? '5173';`) and use that value.
</issue_to_address>

### Comment 2
<location path="apps/ui-staff/start-dev.mjs" line_range="14" />
<code_context>
+	childEnv.VITE_APP_UI_COMMUNITY_BASE_URL = buildPortlessUrl(hostnames.uiCommunity);
+}
+
+const viteArgs = ['--port', process.env.PORT, '--host', process.env.HOST ?? '127.0.0.1'];
+const viteMode = process.env.E2E_VITE_MODE ?? (isE2E() || process.env.TF_BUILD ? 'e2e' : undefined);
+if (viteMode) {
</code_context>
<issue_to_address>
**issue (bug_risk):** Same `PORT` handling issue here may cause Vite to be started with an invalid `'undefined'` port.

When `PORT` is unset this results in `--port undefined` being passed to Vite. Consider either providing a default value or only adding the `--port` flag when `PORT` is defined to avoid starting Vite with an invalid port.
</issue_to_address>

### Comment 3
<location path="apps/server-mongodb-memory-mock/start-mongo.mjs" line_range="13-14" />
<code_context>
+	stdio: 'inherit',
+	env: { ...process.env, PORT: String(MONGO_PORT) },
+});
+child.on('exit', (code) => {
+	process.exit(code ?? 1);
+});
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Child exit handling does not treat interrupt signals as graceful, unlike other dev runners.

Other dev runners call `forwardChildExit`, which maps SIGINT/SIGTERM/SIGQUIT to `exitCode = 0` so Turbo treats these as graceful. Here, an interrupt from `tsx` that returns non‑zero will be propagated and can cause watch pipelines to fail. Please reuse `forwardChildExit` (and `isGracefulInterruptExit`) here for consistent, graceful handling of interrupts.

Suggested implementation:

```javascript
import { forwardChildExit } from 'PATH_TO_FORWARD_CHILD_EXIT';

const MONGO_PORT = getMongoPort();

```

```javascript
forwardChildExit(child);

```

You will need to:

1. Replace `PATH_TO_FORWARD_CHILD_EXIT` with the actual module path where `forwardChildExit` (and `isGracefulInterruptExit`) are defined in your codebase. For example, it might be something like `'../../dev-tools/child-process'` or a similar shared utility module.
2. Ensure that the signature `forwardChildExit(child)` matches the existing helper; if the helper requires additional arguments (e.g. logger, options), supply them accordingly here.
3. If your linter requires that all imported symbols be used and `isGracefulInterruptExit` is not used directly here, do not import it explicitly—use only `forwardChildExit`, which should already encapsulate the graceful exit behavior.
</issue_to_address>

### Comment 4
<location path="scripts/local-dev/worktree-ports.mjs" line_range="127" />
<code_context>
+	export function getMongoConnectionString() {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Mongo connection string rewriting assumes `127.0.0.1` and will silently fail for other hosts.

The regex only matches `127.0.0.1:<port>`, so if `COSMOSDB_CONNECTION_STRING` uses `localhost`, another hostname, or a different IP, the `replace` is a no‑op and the original port is kept. Consider also handling `localhost` and other common hosts, or parsing the connection string structurally via a dedicated parser instead of a regex.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread apps/ui-community/start-dev.mjs Outdated
Comment thread apps/ui-staff/start-dev.mjs Outdated
Comment thread apps/server-mongodb-memory-mock/start-mongo.mjs Outdated
Comment thread scripts/local-dev/worktree-ports.mjs Outdated
@jasonmorais
Copy link
Copy Markdown
Contributor Author

@sourcery-ai review

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 4 issues, and left some high level feedback:

  • The worktree port calculation and Azurite/Mongo port helpers are now implemented in both scripts/local-dev/worktree-ports.mjs and e2e-tests/.../worktree-ports.ts; consider centralizing this logic in a single shared module to avoid divergence between local dev and e2e behavior.
  • Hostname and URL derivation for portless (e.g., portless-settings.ts, portless-hostnames.mjs, and loadE2EEnvDefaults in test-environment.ts) are doing similar .env/env parsing in multiple places; it would be more robust to extract a shared helper so that hostname resolution rules stay consistent across environments.
  • The process-kill and shutdown patterns in PortlessServer and TestAzuriteServer are very similar (signals, detached process groups, timeouts); factoring these into a shared utility would reduce duplication and make future changes to shutdown behavior easier to apply consistently.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The worktree port calculation and Azurite/Mongo port helpers are now implemented in both `scripts/local-dev/worktree-ports.mjs` and `e2e-tests/.../worktree-ports.ts`; consider centralizing this logic in a single shared module to avoid divergence between local dev and e2e behavior.
- Hostname and URL derivation for portless (e.g., `portless-settings.ts`, `portless-hostnames.mjs`, and `loadE2EEnvDefaults` in `test-environment.ts`) are doing similar `.env`/env parsing in multiple places; it would be more robust to extract a shared helper so that hostname resolution rules stay consistent across environments.
- The process-kill and shutdown patterns in `PortlessServer` and `TestAzuriteServer` are very similar (signals, detached process groups, timeouts); factoring these into a shared utility would reduce duplication and make future changes to shutdown behavior easier to apply consistently.

## Individual Comments

### Comment 1
<location path="apps/api/start-azurite.mjs" line_range="1-10" />
<code_context>
+import { spawn } from 'node:child_process';
</code_context>
<issue_to_address>
**issue:** Handle child process 'error' events for Azurite to avoid silent failures when the binaries are missing or not executable.

If `spawn('azurite-blob' | 'azurite-queue' | 'azurite-table')` fails (e.g. binaries missing or not on `PATH`), Node emits an `error` event rather than `exit`, and the script will continue without any clear failure signal. Please add `proc.on('error', …)` for each spawned process (or via a helper) to log a descriptive message and exit with a non‑zero code.
</issue_to_address>

### Comment 2
<location path="apps/ui-community/start-dev.mjs" line_range="32-33" />
<code_context>
+	settings.Values = values;
+}
+
+function isE2E() {
+	return ['1', 'true', 'yes'].includes((process.env.E2E ?? '').toLowerCase());
+}
</code_context>
<issue_to_address>
**suggestion:** The isE2E helper is duplicated between ui-community and ui-staff; consider sharing to keep behavior in sync.

The same `isE2E()` helper and related `viteArgs`/mode handling appear in both `apps/ui-community/start-dev.mjs` and `apps/ui-staff/start-dev.mjs`. Extracting a shared utility under `scripts/local-dev` for mode detection and argument building would avoid duplication and prevent the two apps from drifting in behavior over time.

Suggested implementation:

```javascript
const viteArgs = buildViteArgs({
	host: process.env.HOST ?? '127.0.0.1',
	port: process.env.PORT,
	env: process.env,
});

```

To fully implement the refactor and avoid duplication, you should also:

1. Create a shared helper, e.g. `scripts/local-dev/devEnvironment.mjs`, that exports both `isE2E(env)` and `buildViteArgs({ host, port, env })`:

   - `isE2E(env)` should implement the existing logic used here:
     ```js
     export function isE2E(env = process.env) {
       return ['1', 'true', 'yes'].includes((env.E2E ?? '').toLowerCase());
     }
     ```
   - `buildViteArgs({ host, port, env })` should encapsulate the current argument/mode handling:
     ```js
     export function buildViteArgs({ host = '127.0.0.1', port, env = process.env } = {}) {
       const args = ['--host', host];

       if (port) {
         args.push('--port', port);
       }

       const viteMode =
         env.E2E_VITE_MODE ??
         (isE2E(env) || env.TF_BUILD ? 'e2e' : undefined);

       if (viteMode) {
         args.push('--mode', viteMode);
       }

       return args;
     }
     ```

2. In `apps/ui-community/start-dev.mjs`, add an import at the top:
   ```js
   import { buildViteArgs } from '../../scripts/local-dev/devEnvironment.mjs';
   ```
   and remove the local `isE2E()` function definition from this file.

3. In `apps/ui-staff/start-dev.mjs`, make the same changes:
   - Import `buildViteArgs` from the shared helper.
   - Replace its local `viteArgs`/`viteMode` construction with a call to `buildViteArgs({ host, port, env: process.env })`.
   - Remove its local `isE2E()` helper so that both apps rely on the same shared implementation.

4. If the repository uses `.cjs`/`.js` instead of `.mjs` for shared scripts, or a different `scripts/local-dev` structure, adjust the file extension and import path accordingly to match the existing conventions.
</issue_to_address>

### Comment 3
<location path="package.json" line_range="16" />
<code_context>
 		"test": "turbo run test",
 		"lint": "turbo run lint",
 		"dev": "pnpm proxy:stop && pnpm proxy:start && turbo watch azurite dev --filter='./apps/*' --filter='./packages/*'",
+		"dev:worktree": "WORKTREE_NAME=$(basename $PWD) pnpm proxy:ensure && WORKTREE_NAME=$(basename $PWD) turbo watch azurite dev:worktree --filter='./apps/*' --filter='./packages/*'",
 		"start": "turbo run build && concurrently --kill-others-on-fail \"pnpm run start:api\" \"pnpm run start:ui-community\"",
 		"proxy:stop": "pnpm exec portless proxy stop || true",
</code_context>
<issue_to_address>
**issue:** The dev:worktree script is POSIX-specific and will not run on Windows shells.

This relies on `$(basename $PWD)` and a POSIX shell, so it fails in `cmd`/PowerShell and is inconsistent with other `:windows` variants (e.g. `sonar:pr-windows`). To support Windows devs, either add a `dev:worktree-windows` script or move WORKTREE_NAME resolution into a small Node helper so `dev:worktree` is cross‑platform.
</issue_to_address>

### Comment 4
<location path="packages/ocom-verification/e2e-tests/src/shared/support/servers/worktree-ports.ts" line_range="7-16" />
<code_context>
+	table: number;
+}
+
+export function getWorktreePortOffset(): number {
+	const name = process.env['WORKTREE_NAME'];
+	if (!name) return 0;
+	let hash = 0;
+	for (const c of name) hash = ((hash << 5) - hash + c.charCodeAt(0)) | 0;
+	return ((Math.abs(hash) % 49) + 1) * 100;
+}
+
+export function getAzuritePorts(): AzuritePorts {
+	const offset = getWorktreePortOffset();
+	return {
+		blob: 10000 + offset,
+		queue: 10001 + offset,
+		table: 10002 + offset,
+	};
+}
+
+export function getMongoPort(): number {
+	return 50000 + getWorktreePortOffset();
+}
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests to validate deterministic worktree port offset calculations used by e2e servers

These functions are now central to coordinating MongoDB and Azurite ports across worktrees and are assumed to be deterministic and non‑colliding. It would be valuable to add small pure-function tests that:

- Assert base values when WORKTREE_NAME is unset (offset 0; Mongo 50000; Azurite 10000/10001/10002).
- Verify determinism and stability for a given WORKTREE_NAME (same name → same offset; different names → typically different offsets).
- Confirm the offset range guarantees the default/unnamed worktree cannot collide with any named worktree.

This will help prevent regressions that could break isolation between parallel worktrees or local setups.

Suggested implementation:

```typescript
interface AzuritePorts {
	blob: number;
	queue: number;
	table: number;
}

/**
 * Pure helper to compute the worktree port offset from a given worktree name.
 * Exposed to enable deterministic unit tests without mutating process.env.
 */
export function getWorktreePortOffsetFromName(name: string | undefined | null): number {
	if (!name) return 0;

	let hash = 0;
	for (const c of name) {
		hash = ((hash << 5) - hash + c.charCodeAt(0)) | 0;
	}

	// Offset in [100, 5000], so unnamed/default worktree (offset 0) cannot collide.
	return ((Math.abs(hash) % 49) + 1) * 100;
}

export function getWorktreePortOffset(): number {
	return getWorktreePortOffsetFromName(process.env['WORKTREE_NAME']);
}

export function getAzuritePorts(): AzuritePorts {

```

To fully implement the test suggestion, add a new test file alongside this module, using your existing test framework (e.g. Jest/Vitest):

1. Create `packages/ocom-verification/e2e-tests/src/shared/support/servers/worktree-ports.test.ts` (or follow the repo's test naming/location conventions).
2. In that test file, import:
   - `getWorktreePortOffsetFromName`
   - `getWorktreePortOffset`
   - `getAzuritePorts`
   - `getMongoPort`
3. Add tests such as:
   - **Base values when WORKTREE_NAME is unset**:
     - `getWorktreePortOffsetFromName(undefined)` and `getWorktreePortOffsetFromName(null)` return `0`.
     - With `WORKTREE_NAME` unset, `getWorktreePortOffset()` returns `0`.
     - `getMongoPort()` returns `50000`.
     - `getAzuritePorts()` returns `{ blob: 10000, queue: 10001, table: 10002 }`.
   - **Determinism/stability**:
     - For a fixed name (e.g. `"feature/my-branch"`), repeated calls to `getWorktreePortOffsetFromName("feature/my-branch")` return the same value.
     - Different names (e.g. `"feature/a"` vs `"feature/b"`, `"main"` vs `"develop"`) typically yield different offsets.
   - **Range/non-collision with unnamed worktree**:
     - For several sample names, assert that `getWorktreePortOffsetFromName(name)` is in `[100, 5000]`.
     - Assert that `getWorktreePortOffsetFromName(name) !== 0` for any non-empty name, so the unnamed/default worktree (offset `0`) cannot collide with any named worktree.
4. If desired, add a small test helper in the test file to temporarily set/restore `process.env.WORKTREE_NAME` for tests that call `getWorktreePortOffset()`, ensuring environment isolation between tests.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread apps/api/start-azurite.mjs
Comment thread apps/ui-community/start-dev.mjs Outdated
Comment thread package.json
@jasonmorais
Copy link
Copy Markdown
Contributor Author

@sourcery-ai review

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 2 issues, and left some high level feedback:

  • In apps/api/start-azurite.mjs, the procs array is referenced inside its own initializer (in the proc.on('error') handler), which will throw due to the temporal dead zone; capture the array indirectly (e.g., define const procs = [] first and push into it) or avoid referencing procs from inside its initialization.
  • The worktree port offset logic is duplicated between scripts/local-dev/worktree-ports.mjs and packages/ocom-verification/e2e-tests/src/shared/support/servers/worktree-ports.ts; consider centralizing this in a single module to keep the hashing and ranges consistent across local dev and e2e.
  • Hostname and URL derivation is split between scripts/local-dev/portless-hostnames.mjs, verification-shared/src/settings/portless-settings.ts, and e2e test-environment helpers; it may be worth consolidating these to one shared implementation to reduce the risk of the different environments drifting apart.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In apps/api/start-azurite.mjs, the `procs` array is referenced inside its own initializer (in the `proc.on('error')` handler), which will throw due to the temporal dead zone; capture the array indirectly (e.g., define `const procs = []` first and push into it) or avoid referencing `procs` from inside its initialization.
- The worktree port offset logic is duplicated between scripts/local-dev/worktree-ports.mjs and packages/ocom-verification/e2e-tests/src/shared/support/servers/worktree-ports.ts; consider centralizing this in a single module to keep the hashing and ranges consistent across local dev and e2e.
- Hostname and URL derivation is split between scripts/local-dev/portless-hostnames.mjs, verification-shared/src/settings/portless-settings.ts, and e2e test-environment helpers; it may be worth consolidating these to one shared implementation to reduce the risk of the different environments drifting apart.

## Individual Comments

### Comment 1
<location path="package.json" line_range="16-23" />
<code_context>
 		"test": "turbo run test",
 		"lint": "turbo run lint",
 		"dev": "pnpm proxy:stop && pnpm proxy:start && turbo watch azurite dev --filter='./apps/*' --filter='./packages/*'",
+		"dev:worktree": "WORKTREE_NAME=$(basename $PWD) pnpm proxy:ensure && WORKTREE_NAME=$(basename $PWD) turbo watch azurite dev:worktree --filter='./apps/*' --filter='./packages/*'",
 		"start": "turbo run build && concurrently --kill-others-on-fail \"pnpm run start:api\" \"pnpm run start:ui-community\"",
 		"proxy:stop": "pnpm exec portless proxy stop || true",
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The dev:worktree and test:e2e:worktree scripts are POSIX-specific and will not work on Windows shells.

These scripts depend on `WORKTREE_NAME=$(basename $PWD)`, which only works in POSIX shells. In default Windows environments (`cmd.exe`, PowerShell), this will fail and break worktree flows for those contributors. To make this portable, you could resolve `WORKTREE_NAME` in a small Node script (e.g. `node scripts/resolve-worktree-name.mjs`) and call that from package.json, or use a tool like `cross-env` to set the variable in a cross‑shell‑compatible way.
</issue_to_address>

### Comment 2
<location path="readme.md" line_range="9" />
<code_context>

+
 [Getting Started](https://developers.cellixjs.org/docs/intro):
 Our Docusaurus website will help you get started in running and contributing to CellixJS

</code_context>
<issue_to_address>
**nitpick (typo):** Consider smoothing the phrasing "get started in running" to more natural wording.

For example: “get started running and contributing to CellixJS” or “get started with running and contributing to CellixJS” would read more naturally.

```suggestion
Our Docusaurus website will help you get started with running and contributing to CellixJS
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread package.json
Comment thread readme.md Outdated
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
@jasonmorais
Copy link
Copy Markdown
Contributor Author

@sourcery-ai review

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • The worktree port offset logic is duplicated in scripts/local-dev/worktree-ports.mjs and e2e-tests/src/shared/support/servers/worktree-ports.ts; consider centralizing this in a single module to avoid future drift between dev and test environments.
  • portless-hostnames.mjs and verification-shared/src/settings/portless-settings.ts both derive hostnames but from different sources (.env files vs process.env); it may be worth unifying these so that all consumers share a single, consistent hostname resolution strategy.
  • In TestAzuriteServer, treating any EADDRINUSE on the Azurite ports as an implicit success could mask conflicts with unrelated processes; you might want to add a simple health probe or PID check to confirm the bound service is actually Azurite before assuming it is reusable.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The worktree port offset logic is duplicated in `scripts/local-dev/worktree-ports.mjs` and `e2e-tests/src/shared/support/servers/worktree-ports.ts`; consider centralizing this in a single module to avoid future drift between dev and test environments.
- `portless-hostnames.mjs` and `verification-shared/src/settings/portless-settings.ts` both derive hostnames but from different sources (.env files vs process.env); it may be worth unifying these so that all consumers share a single, consistent hostname resolution strategy.
- In `TestAzuriteServer`, treating any `EADDRINUSE` on the Azurite ports as an implicit success could mask conflicts with unrelated processes; you might want to add a simple health probe or PID check to confirm the bound service is actually Azurite before assuming it is reusable.

## Individual Comments

### Comment 1
<location path="package.json" line_range="16" />
<code_context>
 		"test": "turbo run test",
 		"lint": "turbo run lint",
 		"dev": "pnpm proxy:stop && pnpm proxy:start && turbo watch azurite dev --filter='./apps/*' --filter='./packages/*'",
+		"dev:worktree": "WORKTREE_NAME=$(basename $PWD) pnpm proxy:ensure && WORKTREE_NAME=$(basename $PWD) turbo watch azurite dev:worktree --filter='./apps/*' --filter='./packages/*'",
 		"start": "turbo run build && concurrently --kill-others-on-fail \"pnpm run start:api\" \"pnpm run start:ui-community\"",
 		"proxy:stop": "pnpm exec portless proxy stop || true",
</code_context>
<issue_to_address>
**issue (bug_risk):** The `dev:worktree` script is POSIX-specific and won’t work on Windows shells.

`WORKTREE_NAME=$(basename $PWD)` and the use of `&&` make this script POSIX-only, so `pnpm run dev:worktree` will fail in cmd/PowerShell. If Windows support is desired, move the WORKTREE_NAME derivation into a small Node script or use a cross-platform helper (e.g. `cross-env-shell`) so the command works across shells.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread package.json
@jasonmorais jasonmorais changed the title initial commit for verify portless worktree and local settings mdoes May 29, 2026
@jasonmorais jasonmorais changed the title portless worktree and local settings mdoes portless worktree and local settings modes May 29, 2026
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