Skip to content

sync: merge dev/tzx into master#236

Open
TalexDreamSoul wants to merge 10 commits intomasterfrom
dev/tzx
Open

sync: merge dev/tzx into master#236
TalexDreamSoul wants to merge 10 commits intomasterfrom
dev/tzx

Conversation

@TalexDreamSoul
Copy link
Copy Markdown
Contributor

@TalexDreamSoul TalexDreamSoul commented Apr 15, 2026

Summary

Notes

Summary by CodeRabbit

  • New Features

    • Added automatic migration of development data during app startup
    • Enhanced runtime dependency management for packaged releases
  • Bug Fixes

    • Fixed path resolution inconsistencies between development and packaged modes
    • Improved application startup reliability and error handling
  • Improvements

    • Enhanced startup logging for better troubleshooting
    • Optimized dependency imports to prevent worker process failures

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 15, 2026

📝 Walkthrough

Walkthrough

This PR introduces comprehensive runtime dependency management for the core-app build and startup processes. It adds a new dependency resolution script that recursively syncs runtime modules, updates the build pipeline to package dependencies into resources/node_modules, implements dev vs. packaged path resolution with legacy data migration, and adjusts various import paths and async control flows accordingly.

Changes

Cohort / File(s) Summary
Build & Runtime Dependency Management
apps/core-app/scripts/build-target.js, apps/core-app/scripts/ensure-runtime-modules.js
Added new ensure-runtime-modules.js script that recursively resolves transitive runtime dependencies from core-app/package.json, copies missing modules to apps/core-app/node_modules, and exports a resolved module list. Updated build-target.js with copyDirectoryWithoutNestedNodeModules() and syncPackagedRuntimeModules() helpers to mirror runtime modules into resources/node_modules, expanded verification logic to check both app.asar and filesystem entries, and integrated dynamic module resolution into the build flow.
App Root Path Resolution & Migration
apps/core-app/src/main/utils/app-root-path.ts, apps/core-app/src/main/utils/app-root-path.test.ts, apps/core-app/src/main/core/precore.ts
Added new app-root-path utility module exporting resolveRuntimeRootPath(), resolveLegacyDevRootPath(), and migrateLegacyDevDataIfNeeded() with support for packaged vs. dev runtime separation, legacy data migration with marker-file tracking, and comprehensive test coverage. Updated precore.ts to use the new path resolution, register early unhandledRejection handler, conditionally enable deprecation tracing, and perform bootstrap-time data migration.
Module Import Path Refinements
apps/core-app/src/main/db/sqlite-retry.ts, apps/core-app/src/main/modules/box-tool/search-engine/search-logger.ts
Narrowed import paths for sleep and StorageList constants from broad package entries to specific subpaths (@talex-touch/utils/common/utils and @talex-touch/utils/common/storage/constants) to prevent worker bundles from inadvertently requiring Electron-related modules.
Network Service Path Resolution
apps/core-app/src/main/modules/network/network-service.ts
Removed local path-resolution fallbacks and packaged/dev branching logic; delegated resolveAppRootPath() to new resolveRuntimeRootPath(app) from app-root-path utility module.
Plugin Installation Async Control Flow
apps/core-app/src/main/modules/plugin/dev-plugin-installer.ts, apps/core-app/src/main/modules/plugin/plugin-resolver.ts
Changed checkDirWithCreate() invocations from awaited to non-awaited, altering async control flow so directory preparation may proceed concurrently with subsequent operations.
Utility Function & Test Updates
apps/core-app/src/main/utils/common-util.ts, apps/core-app/src/main/utils/common-util.test.ts
Converted checkDirWithCreate() from async to synchronous with explicit true return type; always calls mkdirSync with recursive: true. Added corresponding test coverage for recursive directory creation and idempotency.
Documentation
docs/plan-prd/01-project/CHANGES.md, docs/plan-prd/TODO.md
Documented new build-time runtime module sync/packaging, worker import narrowing, app-root-path hardening, data migration, startup observability improvements, and marked corresponding governance items as completed.

Sequence Diagram(s)

sequenceDiagram
    participant Build as Build Process
    participant Ensure as ensure-runtime-modules.js
    participant Tree as App Dependency Tree
    participant Copy as Copy to node_modules
    participant Sync as syncPackagedRuntimeModules()
    participant Pkg as electron-builder
    participant Verify as verifyPackagedRuntimeModules()
    
    Build->>Ensure: Resolve runtime dependencies
    Ensure->>Tree: Read app package.json
    Ensure->>Tree: Recursively traverse dependencies
    Ensure->>Copy: Copy missing modules
    Copy-->>Ensure: Return copied modules
    Ensure-->>Build: Return resolved modules list
    Build->>Sync: Start packaging sync
    Sync->>Sync: Locate app.asar
    Sync->>Sync: Copy modules → resources/node_modules
    Sync-->>Build: Sync complete
    Build->>Pkg: Run electron-builder
    Pkg-->>Build: Build output
    Build->>Verify: Verify packaged modules
    Verify->>Verify: Check asar entries + filesystem
    Verify-->>Build: Verification complete
Loading
sequenceDiagram
    participant Startup as App Startup
    participant Precore as precore.ts
    participant PathUtil as app-root-path utility
    participant UserData as userData directory
    participant Legacy as Legacy app data
    participant Migration as Migration process
    
    Startup->>Precore: Initialize core
    Precore->>PathUtil: resolveRuntimeRootPath(app)
    PathUtil->>UserData: Determine userData location
    PathUtil-->>Precore: Return runtime root path
    Precore->>PathUtil: migrateLegacyDevDataIfNeeded()
    PathUtil->>PathUtil: Check if packaged
    PathUtil->>PathUtil: Verify migration marker exists
    alt marker not found & dev mode
        PathUtil->>Legacy: Locate legacy source data
        PathUtil->>Migration: Check if source/target differ
        Migration->>UserData: Copy legacy data → userData/tuff-dev
        Migration->>PathUtil: Write migration marker
        PathUtil-->>Precore: Return migrated status
    else marker exists or other condition
        PathUtil-->>Precore: Return skipped/failed status
    end
    Precore->>Precore: Register unhandledRejection handler
    Precore->>Precore: Create innerRootPath directory
    Precore-->>Startup: Bootstrap complete
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

codex

Poem

🐰 Hop by hop, we bundle deps so tight,
Legacy data migrates in the night,
Paths resolve from dev to packaged state,
Startup grows robust—no data to lose or wait!
Resources nested, modules now at rest,
Another rabbit refactors with the best!

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The PR title 'sync: merge dev/tzx into master' is vague and generic, describing only the mechanical merge action rather than the substantive changes (runtime path hardening, module synchronization, dev data migration, packaging fixes). Consider a more descriptive title that highlights the main technical changes, such as 'chore: hardening runtime paths, module sync, and dev data migration' or similar to convey the primary improvements.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dev/tzx

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 and usage tips.

}
// Auto-detect build type from version if beta
// Beta versions always use snapshot build, regardless of explicit type
let finalBuildType = (type || 'release').toLowerCase()
}

const builderBin = resolveBuilderBin()
let runtimeModulesToVerify = ['ms']
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/core-app/scripts/build-target.js`:
- Around line 1038-1040: The catch that currently only logs `Error listing dist
files: ${err.message}` should re-throw the error so the build fails instead of
continuing; update the catch block (the try/catch surrounding the dist file
verification that logs "Error listing dist files") to either `throw err` (or
call `process.exit(1)` after logging) so downstream post-processing and the
final "Build completed successfully." message do not run with a broken or empty
dist.
- Around line 36-41: The SIGTERM handler currently only calls cleanup() and
therefore prevents the process from exiting; update the SIGTERM listener (the
process.once('SIGTERM', ...) handler) to call process.exit(1) after invoking
cleanup()—mirroring the SIGINT handler behavior—so the build terminates
correctly when SIGTERM is received.

In `@apps/core-app/scripts/ensure-runtime-modules.js`:
- Around line 34-52: The current logic flattens nested node_modules by deduping
solely on moduleName and stripping nested node_modules during copy; change
resolveModuleRoot, copyModule filter, and visitModule dedupe to preserve each
module's dependency context: make resolveModuleRoot attempt resolution relative
to the module's own sourceDir (searching candidate paths including
path.join(sourceDir, 'node_modules', moduleName)) and return the actual package
root found, update the copyModule filter to include and preserve any nested
node_modules directories (do not strip or collapse them) so the full dependency
tree is copied, and change visitModule dedupe from a bare moduleName key to a
deterministic key that includes the module's package identity + context (for
example packageName@version plus its parent/source path or package.json resolved
path) so different contextual versions are kept and only exact same source roots
are deduped.

In `@apps/core-app/src/main/utils/app-root-path.test.ts`:
- Around line 158-184: The test and migration logic treat a "failed"
DEV_DATA_MIGRATION_MARKER as terminal which permanently blocks retries; update
migrateLegacyDevDataIfNeeded so that an existing marker with status === 'failed'
does not short-circuit future migrations (either remove/overwrite the failed
marker at start or treat it as absent and allow retry), keep short-circuit only
for 'in-progress' or successful markers, and then change this test's assertions
(in the failing-copy case) to reflect that a transient copy error writes a
non-terminal failure (e.g., either no persistent failed marker or a marker that
allows retry) instead of permanently leaving status 'failed' — reference
migrateLegacyDevDataIfNeeded and DEV_DATA_MIGRATION_MARKER to locate the logic
and update the test expectations accordingly.

In `@docs/plan-prd/01-project/CHANGES.md`:
- Around line 2470-2491: Move the new "2026-03-23" changelog entry into the
chronological 2026-03 section with the other 2026-03-* entries (do not leave it
after the terminal "说明" block), and update the top "更新时间" header to reflect this
edit; ensure the "2026-03-23" heading and its bullet list remain intact and
positioned in date order among existing 2026-03 entries.
🪄 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

Run ID: fcbaff70-822a-4e45-bae3-343586ea35ba

📥 Commits

Reviewing files that changed from the base of the PR and between 38ba616 and 1fad301.

📒 Files selected for processing (14)
  • apps/core-app/scripts/build-target.js
  • apps/core-app/scripts/ensure-runtime-modules.js
  • apps/core-app/src/main/core/precore.ts
  • apps/core-app/src/main/db/sqlite-retry.ts
  • apps/core-app/src/main/modules/box-tool/search-engine/search-logger.ts
  • apps/core-app/src/main/modules/network/network-service.ts
  • apps/core-app/src/main/modules/plugin/dev-plugin-installer.ts
  • apps/core-app/src/main/modules/plugin/plugin-resolver.ts
  • apps/core-app/src/main/utils/app-root-path.test.ts
  • apps/core-app/src/main/utils/app-root-path.ts
  • apps/core-app/src/main/utils/common-util.test.ts
  • apps/core-app/src/main/utils/common-util.ts
  • docs/plan-prd/01-project/CHANGES.md
  • docs/plan-prd/TODO.md

Comment on lines +36 to +41
process.once('exit', cleanup)
process.once('SIGINT', () => {
cleanup();
process.exit(1);
});
process.once('SIGTERM', cleanup);
cleanup()
process.exit(1)
})
process.once('SIGTERM', cleanup)
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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the build script around the mentioned lines
fd -t f build-target.js

Repository: talex-touch/tuff

Length of output: 97


🏁 Script executed:

# Search for the file and read it
find . -name "build-target.js" -type f | head -5

Repository: talex-touch/tuff

Length of output: 99


🌐 Web query:

Node.js SIGTERM listener process termination behavior default exit

💡 Result:

By default, on non-Windows platforms, a Node.js process receiving SIGTERM without any listener installed will have its default handler trigger: it resets the terminal mode and exits immediately with exit code 128 + 15 = 143 (since SIGTERM is signal 15). This terminates the process synchronously, dropping pending asynchronous operations without waiting for the event loop to empty or executing 'exit' event listeners. If a listener is installed via process.on('SIGTERM', handler) or process.once('SIGTERM', handler), the default handler is removed. The process does not exit automatically. The handler runs on the next event loop tick, and the process continues running until explicitly terminated (e.g., by calling process.exit in the handler) or naturally when no more work remains on the event loop. This allows for graceful shutdown if implemented in the handler, such as closing servers and databases. On Windows, SIGTERM is not supported natively but can be listened for; sending it has no default termination effect. This behavior is consistent across Node.js versions, including latest v25.x, as per official documentation.

Citations:


🏁 Script executed:

wc -l ./apps/core-app/scripts/build-target.js

Repository: talex-touch/tuff

Length of output: 104


🏁 Script executed:

sed -n '1,50p' ./apps/core-app/scripts/build-target.js

Repository: talex-touch/tuff

Length of output: 1550


🏁 Script executed:

sed -n '30,45p' ./apps/core-app/scripts/build-target.js

Repository: talex-touch/tuff

Length of output: 355


🏁 Script executed:

# Let's see what happens after the function definition and how it's used
sed -n '46,100p' ./apps/core-app/scripts/build-target.js

Repository: talex-touch/tuff

Length of output: 1981


🏁 Script executed:

# Search for where ensureLocalPnpmLockfile is called
rg "ensureLocalPnpmLockfile" ./apps/core-app/scripts/build-target.js -A 5

Repository: talex-touch/tuff

Length of output: 587


🏁 Script executed:

# Let's see the main execution flow and what happens with the cleanup function
sed -n '950,1063p' ./apps/core-app/scripts/build-target.js

Repository: talex-touch/tuff

Length of output: 4976


🏁 Script executed:

# Let's check the broader context of the build function to understand execution flow
sed -n '1000,1025p' ./apps/core-app/scripts/build-target.js

Repository: talex-touch/tuff

Length of output: 1370


SIGTERM no longer terminates the build.

Once Node has a SIGTERM listener, the default exit behavior is removed. Currently, the handler only runs cleanup() without exiting, leaving the process alive. This breaks CI behavior where external orchestrators send SIGTERM expecting the process to terminate. The SIGINT handler properly exits after cleanup; SIGTERM should do the same.

Suggested fix
-  process.once('SIGTERM', cleanup)
+  process.once('SIGTERM', () => {
+    cleanup()
+    process.exit(143)
+  })
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/core-app/scripts/build-target.js` around lines 36 - 41, The SIGTERM
handler currently only calls cleanup() and therefore prevents the process from
exiting; update the SIGTERM listener (the process.once('SIGTERM', ...) handler)
to call process.exit(1) after invoking cleanup()—mirroring the SIGINT handler
behavior—so the build terminates correctly when SIGTERM is received.

Comment on lines +1038 to 1040
} catch (err) {
console.error(`Error listing dist files: ${err.message}`)
}
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.

⚠️ Potential issue | 🔴 Critical

Fail the build when artifact verification throws.

This catch only logs and then continues, so an empty or malformed dist can still flow into post-processing, runtime-module sync, and finally "Build completed successfully." Re-throw here; otherwise the release job can publish a bad package.

Suggested fix
         } catch (err) {
           console.error(`Error listing dist files: ${err.message}`)
+          throw err
         }
📝 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.

Suggested change
} catch (err) {
console.error(`Error listing dist files: ${err.message}`)
}
} catch (err) {
console.error(`Error listing dist files: ${err.message}`)
throw err
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/core-app/scripts/build-target.js` around lines 1038 - 1040, The catch
that currently only logs `Error listing dist files: ${err.message}` should
re-throw the error so the build fails instead of continuing; update the catch
block (the try/catch surrounding the dist file verification that logs "Error
listing dist files") to either `throw err` (or call `process.exit(1)` after
logging) so downstream post-processing and the final "Build completed
successfully." message do not run with a broken or empty dist.

Comment on lines +34 to +52
function resolveModuleRoot(moduleName) {
const directCandidates = [
path.join(targetNodeModules, moduleName),
path.join(workspaceNodeModules, moduleName)
]

for (const candidate of directCandidates) {
if (fs.existsSync(candidate)) {
return candidate
}
}

try {
const pkgPath = require.resolve(path.join(moduleName, 'package.json'), {
paths: [projectRoot, workspaceRoot, targetNodeModules, workspaceNodeModules]
})
return path.dirname(pkgPath)
} catch {
return null
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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n apps/core-app/scripts/ensure-runtime-modules.js

Repository: talex-touch/tuff

Length of output: 5846


🏁 Script executed:

# Search for usage of this script
rg "ensure-runtime-modules" --type js --type json

# Check package.json files to see if there are version conflicts
fd package.json | head -20 | xargs -I {} sh -c 'echo "=== {} ===" && rg "dependencies|optionalDependencies" {} -A 20' | head -100

Repository: talex-touch/tuff

Length of output: 4801


🏁 Script executed:

# Check build-target.js for context
sed -n '1,50p' apps/core-app/scripts/build-target.js

# Search for any issue tracking or comments about dependency conflicts
rg "version.*conflict|transitive|flatten|hoisted" --type js -i

# Check if there's yarn.lock or package-lock.json that shows actual structure
ls -la | grep -E "lock|pnpm"

Repository: talex-touch/tuff

Length of output: 2771


🏁 Script executed:

# Search for any issues or comments about dependency resolution problems
rg "node_modules.*copy|hoist|conflict" --type js -i -B 2 -A 2

# Check if there are any tests for the ensure-runtime-modules script
find . -name "*.test.js" -o -name "*.spec.js" | xargs rg -l "ensure-runtime-modules" 2>/dev/null

# Check git history for any issues with this script
git log --oneline --all -- apps/core-app/scripts/ensure-runtime-modules.js | head -20

Repository: talex-touch/tuff

Length of output: 6026


🏁 Script executed:

# Check if there are any version conflicts in the dependency tree
# Look at what modules are listed in core-app's dependencies
sed -n '/dependencies/,/optionalDependencies/p' apps/core-app/package.json | head -40

# Check if any of those dependencies also have nested dependencies on the same packages with different versions
rg "@langchain|sqlite|compressing" --type json -l | head -10 | xargs -I {} sh -c 'echo "=== {} ===" && cat {}'

Repository: talex-touch/tuff

Length of output: 50372


🏁 Script executed:

# Check if any package appears in multiple dependency trees with different versions
# Look for common transitive dependencies that might conflict
rg "dependencies.*@langchain|dependencies.*drizzle|dependencies.*esbuild" --type json -l | head -5

# Check if there are any known issues or TODOs about this
rg "TODO|FIXME|BUG|ISSUE.*depend|ISSUE.*module" apps/core-app/scripts/ -i

# Check if the script has been recently modified or if there are comments about limitations
git log --all -p --follow -- apps/core-app/scripts/ensure-runtime-modules.js | head -100

Repository: talex-touch/tuff

Length of output: 3117


🏁 Script executed:

# Search for any comments or discussions about this dependency resolution strategy
rg "hoisted|flatten|version.*conflict" --type js -i apps/core-app/scripts/

# Check if electron-builder or other build tools have specific requirements
rg "electron-builder" --type json -l

# Look for any documentation about the script's design
fd README -o CONTRIBUTING | xargs cat 2>/dev/null | grep -A 5 -B 5 "ensure-runtime"

Repository: talex-touch/tuff

Length of output: 388


🏁 Script executed:

# Check what electron-builder does with node_modules
rg "electron-builder|asar|packageManager" apps/core-app --type json | head -20

# Check if there's a package-lock or actual node_modules structure that shows version management
ls -la apps/core-app/ | grep -E "lock|node_modules"

# Verify if the script's intention is to flatten for packaging purposes
grep -r "ensure-runtime-modules" apps/core-app/package.json

Repository: talex-touch/tuff

Length of output: 481


🏁 Script executed:

# Check if there are any issues, error reports, or failed builds related to this
rg "module.*not found|dependency.*missing|pack.*failed" --type js -i apps/core-app/scripts/build-target.js -A 3 -B 3

# Look at what the electron-builder actually does with this script
sed -n '1,100p' apps/core-app/scripts/build-target.js | tail -80

# Check if pnpm-lock.yaml shows any version conflicts for transitive deps
rg "resolution:" pnpm-lock.yaml | head -20

Repository: talex-touch/tuff

Length of output: 5025


🏁 Script executed:

# Check the actual purpose and context more carefully
grep -B 5 -A 10 "ensure-runtime-modules" apps/core-app/scripts/build-target.js

# See if there are any actual dependency conflicts in the pnpm-lock.yaml
# by checking if any package depends on the same transitive dep with different versions
rg "dependencies:" pnpm-lock.yaml | wc -l

# Check if the script was added with any specific justification or issue reference
git log --all --grep="ensure-runtime" --oneline

Repository: talex-touch/tuff

Length of output: 828


Preserve nested node_modules dependency trees when copying modules.

The algorithm dedupes only by moduleName and strips nested node_modules during copy, which flattens packages that carry version-specific transitive dependencies. If two dependencies require different versions of the same transitive package, the first-resolved version wins globally, potentially breaking the second dependency at runtime despite all module names being "present."

Resolve each module's dependencies relative to its own sourceDir and preserve their complete node_modules hierarchies rather than flattening by bare package name. This maintains the proper dependency resolution context that pnpm/npm would normally provide.

Affected code segments

Lines 34–52 (resolveModuleRoot), 56–68 (copyModule filter), 85–130 (visitModule dedup logic)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/core-app/scripts/ensure-runtime-modules.js` around lines 34 - 52, The
current logic flattens nested node_modules by deduping solely on moduleName and
stripping nested node_modules during copy; change resolveModuleRoot, copyModule
filter, and visitModule dedupe to preserve each module's dependency context:
make resolveModuleRoot attempt resolution relative to the module's own sourceDir
(searching candidate paths including path.join(sourceDir, 'node_modules',
moduleName)) and return the actual package root found, update the copyModule
filter to include and preserve any nested node_modules directories (do not strip
or collapse them) so the full dependency tree is copied, and change visitModule
dedupe from a bare moduleName key to a deterministic key that includes the
module's package identity + context (for example packageName@version plus its
parent/source path or package.json resolved path) so different contextual
versions are kept and only exact same source roots are deduped.

Comment on lines +158 to +184
it('returns failed and writes marker when copy throws', () => {
const root = createTempRoot()
const userDataPath = path.join(root, 'user-data')
const appPath = path.join(root, 'app-root')
const sourceRoot = path.join(appPath, APP_FOLDER_NAME)
const targetRoot = path.join(userDataPath, DEV_APP_FOLDER_NAME)
fse.ensureDirSync(sourceRoot)
fse.writeFileSync(path.join(sourceRoot, 'legacy.txt'), 'legacy')

vi.spyOn(fse, 'copySync').mockImplementation(() => {
throw new Error('copy-failed-for-test')
})

const appLike = createAppLike({
isPackaged: false,
userDataPath,
appPath
})

const result = migrateLegacyDevDataIfNeeded(appLike)
const markerPath = path.join(targetRoot, DEV_DATA_MIGRATION_MARKER)

expect(result.status).toBe('failed')
expect(result.error).toContain('copy-failed-for-test')
expect(fse.pathExistsSync(markerPath)).toBe(true)
expect(fse.readJSONSync(markerPath).status).toBe('failed')
})
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.

⚠️ Potential issue | 🟠 Major

Don't make migration failures terminal.

This test locks in a failed marker as the final state for copy errors. In migrateLegacyDevDataIfNeeded(), later boots short-circuit on marker existence, so one transient filesystem failure permanently suppresses future retries and can strand all legacy dev data. Please change the migration contract to retry after failed and update this assertion accordingly.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/core-app/src/main/utils/app-root-path.test.ts` around lines 158 - 184,
The test and migration logic treat a "failed" DEV_DATA_MIGRATION_MARKER as
terminal which permanently blocks retries; update migrateLegacyDevDataIfNeeded
so that an existing marker with status === 'failed' does not short-circuit
future migrations (either remove/overwrite the failed marker at start or treat
it as absent and allow retry), keep short-circuit only for 'in-progress' or
successful markers, and then change this test's assertions (in the failing-copy
case) to reflect that a transient copy error writes a non-terminal failure
(e.g., either no persistent failed marker or a marker that allows retry) instead
of permanently leaving status 'failed' — reference migrateLegacyDevDataIfNeeded
and DEV_DATA_MIGRATION_MARKER to locate the logic and update the test
expectations accordingly.

Comment on lines +2470 to +2491
## 2026-03-23

### fix(core-dev-startup): root path hardening + one-time dev data migration

- Unified runtime root-path policy:
- `app.isPackaged === true` -> `userData/tuff`
- `app.isPackaged === false` -> `userData/tuff-dev`
- Removed dev-mode writes to `app.getAppPath()/tuff` as active root to avoid workspace pollution and path instability.
- Added one-time best-effort migration for dev data:
- Source: `app.getAppPath()/tuff`
- Target: `app.getPath('userData')/tuff-dev`
- Marker: `.dev-data-migration.json` (records migrated / skipped / failed reason to avoid repeated attempts).
- Hardened startup directory initialization order in precore:
- Ensure root first, then `root/logs`, then bind `crashDumps`.
- Hardened `checkDirWithCreate` to synchronous recursive mkdir and aligned call-sites by removing unnecessary `await`.
- Startup observability improvements:
- Corrected single-instance warning semantics to “quitting new instance”.
- Added early `unhandledRejection` logging in precore.
- Added optional deprecation trace switch via `TUFF_TRACE_DEPRECATION=1`.
- Added targeted tests:
- `src/main/utils/app-root-path.test.ts`
- `src/main/utils/common-util.test.ts`
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.

⚠️ Potential issue | 🟡 Minor

Keep the new changelog entry inside the chronological section.

Appending a new dated entry after the terminal 说明 block makes it easy to miss and leaves the file header stale for this edit. Move this section up with the other 2026-03-* entries and bump the top 更新时间 in the same change.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/plan-prd/01-project/CHANGES.md` around lines 2470 - 2491, Move the new
"2026-03-23" changelog entry into the chronological 2026-03 section with the
other 2026-03-* entries (do not leave it after the terminal "说明" block), and
update the top "更新时间" header to reflect this edit; ensure the "2026-03-23"
heading and its bullet list remain intact and positioned in date order among
existing 2026-03 entries.

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages bot commented Apr 15, 2026

Deploying tuff with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1fad301
Status: ✅  Deploy successful!
Preview URL: https://889912ff.tuff-dso.pages.dev
Branch Preview URL: https://dev-tzx.tuff-dso.pages.dev

View logs

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.

2 participants