Skip to content

fix(config): preserve production state paths#3864

Closed
PixPMusic wants to merge 2 commits into
pingdotgg:t3code/codex-turn-mappingfrom
PixPMusic:pixpmusic/fix-state-directory-migration
Closed

fix(config): preserve production state paths#3864
PixPMusic wants to merge 2 commits into
pingdotgg:t3code/codex-turn-mappingfrom
PixPMusic:pixpmusic/fix-state-directory-migration

Conversation

@PixPMusic

@PixPMusic PixPMusic commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • keep production server state in the established userdata directory
  • preserve the existing Electron user-data identities so upgrades retain history and settings
  • continue isolating development state under dev and align SSH runtime discovery

Open question

#2829 deliberately introduced userdata-v2 in 3b864044c. Reverting that rename preserves installations that only have the original userdata state, but it can orphan state already written by v2 preview builds. This PR is held as a draft while upstream decides between reverting before release or retaining v2 with an explicit migration/import policy.

As v2 was never released, I'm leaning towards migrating v1 userdata to v2 in place with an explicit backup policy. This would break downgrades potentially--and I suppose that's exactly why the rename was introduced.

Validation

  • vp check
  • vp run typecheck
  • focused server, desktop, and SSH path tests (73 tests)

Addresses review feedback on #2829.

Julius Marminge and others added 2 commits July 7, 2026 16:50
Ports main's features onto the V2 orchestrator and wire protocol:

- HTTP snapshot loading (pingdotgg#3719): new OrchestrationV2ThreadDetailSnapshot
  contract, /api/orchestration/{shell,threads/:threadId} endpoints served
  from the V2 engine (orchestration-v2/http.ts), afterSequence resume on
  the V2 subscribeShell/subscribeThread WS methods, and V2-typed client
  snapshot loaders wired into shell/thread sync with warm-cache resume.
- Thread cache now persists the snapshot sequence (cache schema v3) so
  warm caches resume via afterSequence instead of refetching.
- Mobile: main's react-navigation architecture retained; branch's V2
  surfaces (relationships banner, queue control, activity inspector,
  work-log thread links, fork-from-run) ported off expo-router; thread
  status presentation and awareness diagnostics mapped to V2 runtime
  fields; pending-task start-turn helper gains creationSource "mobile".
- Retired v1-only additions from main (v1 RPC schemas, session/latestTurn
  awareness heuristics, v1 engine test updates) where the branch already
  replaced those systems with V2 equivalents.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 6276c8b3-21d0-472d-9fea-3a247015f52b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions github-actions Bot added size:S 10-29 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list. labels Jul 10, 2026
Comment thread apps/server/src/config.ts
): Effect.fn.Return<ServerDerivedPaths, never, Path.Path> {
const { join } = yield* Path.Path;
const stateDir = join(baseDir, devUrl !== undefined ? "dev" : "userdata-v2");
const stateDir = join(baseDir, devUrl !== undefined ? "dev" : "userdata");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Medium src/config.ts:96

Changing stateDir from "userdata-v2" back to "userdata" makes the server look for production state in join(baseDir, "userdata") instead of join(baseDir, "userdata-v2"), where the current base revision writes it. There is no migration or fallback lookup, so after upgrading the server creates a fresh empty state.sqlite, settings, logs, and environment IDs under userdata, causing existing user history and configuration to appear lost. If this revert is intentional, consider adding a migration or fallback so existing userdata-v2 state is preserved.

Also found in 1 other location(s)

apps/desktop/src/app/DesktopEnvironment.ts:158

Reverting stateDir to path.join(baseDir, &#34;userdata&#34;) strands the desktop's existing production files from the base revision, which wrote desktop-settings.json, client-settings.json, and saved-environments.json under .../userdata-v2. The upgraded app now reads and writes those files in .../userdata instead, so users coming from the current base build lose their saved environments and desktop settings until they manually copy the old directory.

🤖 Copy this AI Prompt to have your agent fix this:
In file @apps/server/src/config.ts around line 96:

Changing `stateDir` from `"userdata-v2"` back to `"userdata"` makes the server look for production state in `join(baseDir, "userdata")` instead of `join(baseDir, "userdata-v2")`, where the current base revision writes it. There is no migration or fallback lookup, so after upgrading the server creates a fresh empty `state.sqlite`, settings, logs, and environment IDs under `userdata`, causing existing user history and configuration to appear lost. If this revert is intentional, consider adding a migration or fallback so existing `userdata-v2` state is preserved.

Also found in 1 other location(s):
- apps/desktop/src/app/DesktopEnvironment.ts:158 -- Reverting `stateDir` to `path.join(baseDir, "userdata")` strands the desktop's existing production files from the base revision, which wrote `desktop-settings.json`, `client-settings.json`, and `saved-environments.json` under `.../userdata-v2`. The upgraded app now reads and writes those files in `.../userdata` instead, so users coming from the current base build lose their saved environments and desktop settings until they manually copy the old directory.

@macroscopeapp

macroscopeapp Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Needs human review

1 blocking correctness issue found. Changes production state directory paths from 'userdata-v2' to 'userdata' without apparent migration logic. An unresolved review comment identifies this could orphan existing user data, causing settings and state to appear lost after upgrade.

You can customize Macroscope's approvability policy. Learn more.

@PixPMusic
PixPMusic marked this pull request as draft July 10, 2026 18:55
@juliusmarminge
juliusmarminge force-pushed the t3code/codex-turn-mapping branch 3 times, most recently from 1e58e65 to a286c60 Compare July 24, 2026 13:37

Copy link
Copy Markdown
Member

Closing after evaluation because this is superseded by #4400 and is unsafe as a standalone migration. The path-only switch from userdata-v2 to userdata can strand existing preview state; #4400 covers the same convergence with explicit legacy import/migration handling. The focused tests passed, but this branch is stale/conflicting and there is no safe reason to land it independently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants