Centralize runtime env aliases and startup thread selection#1481
Centralize runtime env aliases and startup thread selection#1481raulferrodrigues wants to merge 2 commits intopingdotgg:mainfrom
Conversation
- Move shared env alias and home-path logic into `@tero/shared/runtime` - Rename welcome payload ids to `startup*` and use latest thread fallback - Tighten desktop backend resolution for local dev runtime
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
github-actions
bot
added
size:XXL
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| - any place where legacy `t3code` and new `tero` paths/keys may be mixed | ||
| - Avoid launching the app/dev stack casually while collecting evidence; prefer static inspection first. | ||
| - Only after the startup/persistence risk is understood should the rename resume. | ||
| - Update this file as findings and changes accumulate. |
There was a problem hiding this comment.
Investigation log with personal paths accidentally committed
Medium Severity
CURRENT-TASK.md is a 448-line forensic investigation log containing absolute local filesystem paths (e.g. /Users/raulrodrigues/workspace/personal/t3code-ero/...), detailed debugging session notes, incident timelines, and working hypotheses. This is a temporary task-tracking document that exposes developer-specific environment details and doesn't belong in the repository.
| TurnId, | ||
| type OrchestrationEvent, | ||
| } from "@t3tools/contracts"; | ||
| } from "@tero/contracts"; |
There was a problem hiding this comment.
Test fixtures use stale checkpoint ref prefix
Low Severity
CHECKPOINT_REFS_PREFIX was renamed from refs/t3/checkpoints to refs/tero/checkpoints in Utils.ts, but several test files still hardcode the old refs/t3/checkpoints prefix in fixture data. While these tests are internally consistent and still pass, they no longer test the actual production ref format, reducing their value as regression guards for the checkpoint path.
Additional Locations (1)
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| }; | ||
| // Send welcome before adding to broadcast set so publishAll calls | ||
| // cannot reach this client before the welcome arrives. | ||
| void runPromise( |
There was a problem hiding this comment.
🟠 High src/wsServer.ts:1013
Line 1015 calls projectionReadModelQuery.getSnapshot() inside void runPromise(...) with no error handling. If the snapshot query fails (e.g., database error), the rejected promise becomes an unhandled rejection that can crash the Node.js process depending on its --unhandled-rejections mode. Consider adding .catch() or Effect.ignoreCause to the pipeline so failures don't propagate as unhandled rejections.
🤖 Copy this AI Prompt to have your agent fix this:
In file apps/server/src/wsServer.ts around line 1013:
Line 1015 calls `projectionReadModelQuery.getSnapshot()` inside `void runPromise(...)` with no error handling. If the snapshot query fails (e.g., database error), the rejected promise becomes an unhandled rejection that can crash the Node.js process depending on its `--unhandled-rejections` mode. Consider adding `.catch()` or `Effect.ignoreCause` to the pipeline so failures don't propagate as unhandled rejections.
Evidence trail:
apps/server/src/wsServer.ts lines 1013-1040 (REVIEWED_COMMIT): `void runPromise(...)` pipeline with no error handling. Line 736: `const runPromise = Effect.runPromiseWith(runtimeServices);`. Line 1043: contrast shows `Effect.ignoreCause({ log: true })` being used for error handling in similar pattern.
There was a problem hiding this comment.
🟡 Medium
Line 46 in 1682216
Changing PERSISTED_STATE_KEY to "tero:renderer-state:v8" without updating readPersistedState() causes silent data loss for existing users. When an existing user loads the app, readPersistedState() reads only from the new key (line 48), finds nothing, and returns initialState. The user's persisted expanded project states and project order are lost. The old key is then deleted by persistState() (lines 86-90) before its data is ever read. Consider updating readPersistedState() to attempt reading from LEGACY_PERSISTED_STATE_KEYS as a fallback before returning initialState.
Also found in 1 other location(s)
apps/web/src/appSettings.ts:17
Changing
APP_SETTINGS_STORAGE_KEYfrom"t3code:app-settings:v1"to"tero:app-settings:v1"will cause existing users to lose all their saved settings. TheuseAppSettingshook uses this key withuseLocalStorage, so any settings previously stored under the old key will be orphaned and the app will fall back toDEFAULT_APP_SETTINGS. If preserving user settings during the rebrand is intended, a migration from the old key to the new key should be implemented.
🤖 Copy this AI Prompt to have your agent fix this:
In file apps/web/src/store.ts around line 46:
Changing `PERSISTED_STATE_KEY` to `"tero:renderer-state:v8"` without updating `readPersistedState()` causes silent data loss for existing users. When an existing user loads the app, `readPersistedState()` reads only from the new key (line 48), finds nothing, and returns `initialState`. The user's persisted expanded project states and project order are lost. The old key is then deleted by `persistState()` (lines 86-90) before its data is ever read. Consider updating `readPersistedState()` to attempt reading from `LEGACY_PERSISTED_STATE_KEYS` as a fallback before returning `initialState`.
Evidence trail:
apps/web/src/store.ts lines 20-31 (PERSISTED_STATE_KEY and LEGACY_PERSISTED_STATE_KEYS definitions), lines 44-66 (readPersistedState function - only reads from new key at line 47, returns initialState at line 48 if not found, no fallback to legacy keys), lines 70-91 (persistState function - deletes all legacy keys at lines 86-90). Commit: REVIEWED_COMMIT.
Also found in 1 other location(s):
- apps/web/src/appSettings.ts:17 -- Changing `APP_SETTINGS_STORAGE_KEY` from `"t3code:app-settings:v1"` to `"tero:app-settings:v1"` will cause existing users to lose all their saved settings. The `useAppSettings` hook uses this key with `useLocalStorage`, so any settings previously stored under the old key will be orphaned and the app will fall back to `DEFAULT_APP_SETTINGS`. If preserving user settings during the rebrand is intended, a migration from the old key to the new key should be implemented.
| export async function collectWorkspacePackages(repoRoot) { | ||
| const rootPackageJsonPath = join(repoRoot, "package.json"); | ||
| const rootPackageJson = await readJson(rootPackageJsonPath); | ||
| const packagePatterns = rootPackageJson.workspaces?.packages ?? []; |
There was a problem hiding this comment.
🟢 Low lib/workspace-links.mjs:71
When workspaces is specified as a plain array like {"workspaces": ["packages/*"]}, rootPackageJson.workspaces?.packages evaluates to undefined, so packagePatterns becomes [] and the function silently returns zero packages. Should handle both the shorthand array format and the { packages: [...] } object format.
- const packagePatterns = rootPackageJson.workspaces?.packages ?? [];
+ const packagePatterns = Array.isArray(rootPackageJson.workspaces)
+ ? rootPackageJson.workspaces
+ : rootPackageJson.workspaces?.packages ?? [];🤖 Copy this AI Prompt to have your agent fix this:
In file scripts/lib/workspace-links.mjs around line 71:
When `workspaces` is specified as a plain array like `{"workspaces": ["packages/*"]}`, `rootPackageJson.workspaces?.packages` evaluates to `undefined`, so `packagePatterns` becomes `[]` and the function silently returns zero packages. Should handle both the shorthand array format and the `{ packages: [...] }` object format.
Evidence trail:
scripts/lib/workspace-links.mjs lines 65-85 at REVIEWED_COMMIT (line 71 shows `rootPackageJson.workspaces?.packages ?? []`). npm documentation at https://docs.npmjs.com/cli/v11/configuring-npm/package-json/ and https://docs.npmjs.com/cli/v10/using-npm/workspaces/ confirms npm workspaces use array format `"workspaces": ["packages/a"]`, not object format with `packages` property.
|
sorry idk why this went to this repo. |
1 participant
Summary
Validation
Note
Medium Risk
Medium risk because it changes runtime defaults (env var names, home-dir selection, desktop backend spawning, and
server.welcomepayload fields), which can affect startup behavior and persisted state locations across installs/dev.Overview
Renames project/package scope and runtime identifiers from
@t3tools/*/t3/T3CODE_*to@tero/*/tero/TERO_*across docs, build/release config, and the desktop/server/web code paths.Centralizes runtime env aliasing and default home-dir selection via
@tero/shared/runtime, defaults dev runs to~/.tero-dev, and makes desktop backend spawning explicit (requiresTERO_DESKTOP_SERVER_EXECUTABLE, passes--mode desktop --no-browser --home-dir ... --auth-token ...) to avoid Electron/Node confusion and state collisions.Updates the WebSocket
server.welcomecontract to providestartupProjectId/startupThreadId(selected from cwd when auto-bootstrap is enabled, otherwise latest non-deleted thread) and adjusts tests accordingly.Adds a
postinstallworkspace-link sync script (scripts/sync-workspace-links.mjs) to ensurenode_modules/@tero/*symlinks exist for local Node/Electron entrypoints.Written by Cursor Bugbot for commit 1682216. This will update automatically on new commits. Configure here.
Note
Rename all
@t3tools/T3CODE_*/t3identifiers to@tero/TERO_*/teroacross the monorepo@t3tools/*to@tero/*and updates the CLI binary fromt3totero, includingpackage.jsonfiles, imports, and turbo/build configs throughout the monorepo.normalizeEnvAliasesfunction in@tero/shared/runtime, called at startup in the server, desktop, and dev runner to warn on conflicts and backfillTERO_*keys from legacyT3CODE_*values.T3CODE_*runtime env vars toTERO_*(e.g.TERO_PORT,TERO_HOME,TERO_AUTH_TOKEN) in the server CLI, desktop main process, and build scripts.bootstrapProjectId/bootstrapThreadIdtostartupProjectId/startupThreadId, and updates selection logic to use the current read-model snapshot rather than only recent bootstrap operations.t3code:*totero:*(e.g.tero:app-settings:v1,tero:renderer-state:v8) and adds aLEGACY_PERSISTED_STATE_KEYSarray for migration.scripts/lib/workspace-links.mjswithsyncWorkspaceLinksand apostinstallhook to maintainnode_modulessymlinks for workspace packages.TERO_*storage keys, env vars, git refs (refs/tero/checkpoints), checkpoint commit prefixes, and service tag identifiers are changed; existing persisted state under old keys will not be read unless explicitly migrated.📊 Macroscope summarized 1682216. 86 files reviewed, 5 issues evaluated, 1 issue filtered, 3 comments posted
🗂️ Filtered Issues
apps/web/src/appSettings.ts — 0 comments posted, 1 evaluated, 1 filtered
APP_SETTINGS_STORAGE_KEYfrom"t3code:app-settings:v1"to"tero:app-settings:v1"will cause existing users to lose all their saved settings. TheuseAppSettingshook uses this key withuseLocalStorage, so any settings previously stored under the old key will be orphaned and the app will fall back toDEFAULT_APP_SETTINGS. If preserving user settings during the rebrand is intended, a migration from the old key to the new key should be implemented. [ Cross-file consolidated ]