Use local npm run dist for built-in plugins, remove SDK CLI dependency, and improve Tauri build/env handling

[Jordonbc]

Motivation

• Remove the dependency on the external openvcs-sdk cargo CLI and make built-in plugin packaging local to each plugin.
• Ensure deterministic detection of plugin source changes and robust copying of produced .ovcsp archives into the app bundle.
• Improve Tauri build ergonomics by loading local env, normalizing signing keys, and running the build from the Backend directory.

Description

• Replaced calls to the openvcs-sdk CLI with a Node script-based flow by adding Backend/scripts/ensure-built-in-plugins.js improvements and updating the Justfile to run node Backend/scripts/ensure-built-in-plugins.js for plugin builds.
• Enhanced ensure-built-in-plugins.js with latestSourceTime, bundleFileNameForPlugin, and copyPackagedBundles helpers, added detection to skip non-code plugins, switched packaging to run npm run dist inside each plugin, and copy produced .ovcsp files into target/openvcs/built-in-plugins.
• Removed cargo install --locked openvcs-sdk --bin cargo-openvcs steps from the GitHub workflows nightly.yml and publish-stable.yml.
• Improved scripts/tauri-build.js by adding loadLocalEnv, normalizeSigningKey, promptHidden, and resolvePaths, reading .env.tauri.local, supporting TAURI_SIGNING_PRIVATE_KEY_FILE, prompting for the signing password if needed, setting NO_STRIP, and running cargo tauri build from the Backend directory.

Testing

• Executed the repository automated test targets: cargo test --workspace and the frontend checks cd Frontend && npm exec tsc -- -p tsconfig.json --noEmit and cd Frontend && npx vitest run, and they completed successfully.
• Verified that just build plugins now runs node Backend/scripts/ensure-built-in-plugins.js and that the script exits non-zero when plugin packaging fails to produce .ovcsp archives (behavior exercised by unit/manual runs during development).

---

Codex Task

[github-actions]

Code Review Summary

Issues Found

1. Missing directory creation in ensure-built-in-plugins.js

• pluginBundles directory may not exist before copying archives
• Add at the start of copyPackagedBundles: fs.mkdirSync(pluginBundles, { recursive: true });

2. Non-code plugins silently skipped

• Plugins without package.json are now silently skipped (line 215-218)
• If these plugins previously produced .ovcsp bundles via SDK, this is a breaking change
• Should log a warning or verify this behavior is intentional

3. Ambiguous bundle selection in copyPackagedBundles

• When multiple .ovcsp files exist, picks first one arbitrarily (line 181-184)
• No warning about multiple archives or which one was chosen
• Consider: if (archiveEntries.length > 1) console.warn('Multiple archives found, using:', sourceArchive);

4. Missing error context in bundleFileNameForPlugin

• If openvcs.plugin.json is malformed, error message won't indicate which plugin failed
• Add plugin name to error: console.error(\Failed to read manifest for ${name}:`, e);`

5. loadLocalEnv security consideration

• Parses .env.tauri.local which may contain secrets
• Currently echoes to child process - ensure CI environment handles this appropriately

Suggestions

• Add fs.mkdirSync(pluginBundles, { recursive: true }); before file copy operations
• Consider adding a --force flag to rebuild all plugins regardless of timestamps
• The resolvePaths() function could be tested with different working directory scenarios

Positive Aspects

• Clean separation of concerns with new helper functions
• Proper JSDoc documentation added
• Uses process.cwd correctly when running npm in plugin directories
• Good error handling with process.exit(1) on failures
• Robust .env.tauri.local loading with key normalization

Overall: Solid changes with minor improvements needed for robustness. The architectural shift from SDK CLI to local npm is well-executed.

opencode session  |  github run

[github-actions]

Code Review

Critical Issues

1. Missing require statements in tauri-build.js

const fs = require('fs');  // Line 2 - exists
const path = require('path');  // Line 3 - exists

Actually present. Disregard.

2. CI workflows removed SDK install but don't run the new plugin build
The SDK installation was removed from both nightly.yml and publish-stable.yml, but no step was added to run just build plugins or node Backend/scripts/ensure-built-in-plugins.js. Built-in plugins won't be packaged in CI.

3. resolvePaths() may break relative working directory assumptions

// OLD
const repoRoot = process.cwd();

// NEW  
const { repoRoot, backendDir } = resolvePaths();

__dirname is fixed to the script location; process.cwd() respects where the user ran from. If a user runs node scripts/tauri-build.js from a subdirectory expecting it to use that as root, this breaks. Consider preserving process.cwd() for repoRoot while using __dirname for backendDir.

Minor Issues

4. Misleading warning level in bundleFileNameForPlugin

} catch (e) {
  console.warn(`Failed to read manifest for ${name}:`, e);

This is a recovery path (falls through to fallback), so warn is appropriate. However, the message says "Failed" when it's actually handled gracefully—consider console.debug() or rephrasing.

5. Non-code plugin warning is verbose

console.warn(
  `Skipping ${pluginName}: no package.json (non-code plugin; expected to provide prebuilt bundle).`
);

This fires on every build for non-code plugins. Could be console.log once per session or debug level.

Suggestions

1. Add plugin build step to CI workflows before the tauri build
2. Consider adding --force flag to bypass timestamp checks (useful for CI)
3. Add || 1 to process.exit(1) calls that only pass numbers

Summary

The architectural shift is solid. Main concern is the CI workflow gap—built-in plugins won't be packaged without adding the new script invocation. The resolvePaths() change from process.cwd() to __dirname is technically correct but may surprise users who relied on CWD-based behavior.

opencode session  |  github run

[github-actions]

Code Review

Critical Issues

1. resolvePaths() behavior change (scripts/tauri-build.js:92-103)
The change from process.cwd() to __dirname-based resolution may break user workflows. If a user runs node scripts/tauri-build.js from a subdirectory expecting it to use that as the repo root, it will now use the script's location instead. The current implementation attempts to handle this with a fallback check for tauri.conf.json in CWD, but this is a subtle behavioral change that could surprise users.

2. Non-code plugins silently skipped (Backend/scripts/ensure-built-in-plugins.js:228-232)
Plugins without package.json are now silently skipped with only a console.log. If these plugins previously produced .ovcsp bundles via the SDK, this is a breaking change that could go unnoticed. Consider:

console.warn(`Skipping ${pluginName}: no package.json (non-code plugin; expected to provide prebuilt bundle).`);

Minor Issues

3. No --force flag for CI
There's no way to force rebuild all plugins regardless of timestamps. This would be useful for CI scenarios where you want deterministic builds. Consider adding:

if (process.argv.includes('--force')) {
  runDistCommand(outdated.length ? outdated : findAllPlugins());
}

4. Missing error context in bundleFileNameForPlugin (Backend/scripts/ensure-built-in-plugins.js:71-75)
The catch block uses console.debug() which is good, but the fallback to directory name could mask manifest issues. The current implementation is acceptable since it forces a rebuild attempt which surfaces the real error later.

Positive Aspects

• The CI workflows correctly include the plugin build step (nightly.yml:173-174, publish-stable.yml:63-64) - the earlier review comments about missing steps are incorrect.
• copyPackagedBundles properly creates the bundles directory with { recursive: true } (line 190).
• Multiple archive warning is well-implemented (lines 213-217).
• loadLocalEnv correctly preserves existing env vars (line 28).
• normalizeSigningKey handles base64 key formatting correctly.
• Proper error propagation with process.exit(1) on failures.

Summary

The architectural shift from SDK CLI to local npm is well-executed. The main concerns are the subtle behavioral change in resolvePaths() and silent skipping of non-code plugins. Both are manageable but worth documenting for users upgrading.

opencode session  |  github run

[Jordonbc]

/oc can you fix this?

[github-actions]

fatal: could not read Username for 'https://github.com': No such device or address

opencode session  |  github run

[github-actions]

Code Review

Critical Issues

None identified. The previous comments about missing CI steps are incorrect—both workflows correctly invoke node Backend/scripts/ensure-built-in-plugins.js before the Tauri build.

Minor Issues

1. copyPackagedBundles creates pluginBundles directory redundantly (ensure-built-in-plugins.js:190)

   • ensureBundlesDir() is already called at line 252 before any plugin processing. The mkdir in copyPackagedBundles is harmless but unnecessary.

2. promptHidden may deadlock in non-interactive environments (tauri-build.js:59-85)

   • If stdin.isTTY is false (pipe/CI), raw mode isn't enabled but stdin listening still starts. For CI usage where signing keys are pre-set via secrets, this won't trigger, but the function could hang if called without stdin attached.

3. Silent env loading (tauri-build.js:107)

   • loadLocalEnv silently skips missing .env.tauri.local. This is correct behavior, but a debug log would help developers understand why their local env isn't being picked up.

Suggestions

1. Add --force flag to ensure-built-in-plugins.js to bypass timestamp checks (useful for CI determinism):

   if (process.argv.includes('--force')) {
     runDistCommand(findOutdatedPlugins().length ? findOutdatedPlugins() : findAllPlugins());
   }

2. Consider wrapping promptHidden with a timeout for robustness in unexpected environments.

Positive Aspects

• Clean separation with well-documented helper functions
• Proper fallback logic in resolvePaths() preserves CWD-based behavior when possible
• Robust .ovcsp archive handling with multi-archive warning
• Good error propagation with process.exit(1) on failures
• normalizeSigningKey correctly handles minisign file format
• CI workflows correctly integrate the new plugin build step

Summary

Solid PR. The architectural shift from SDK CLI to local npm is well-implemented with proper error handling. The minor issues above are polish suggestions rather than blockers.

opencode session  |  github run

[Jordonbc]

/oc can you fix this?

[opencode-agent]

Plugin Bundles

• Added a --force switch to Backend/scripts/ensure-built-in-plugins.js, which now can rebuild every built-in plugin on demand (useful for CI) while still using timestamp checks normally, always makes sure the target/openvcs/built-in-plugins directory exists, and logs a warning when skipping non-code plugins.
• Documented how to run the helper script and when to use --force in docs/plugins.md so maintainers know how bundled .ovcsp artifacts are regenerated.

Testing

• Not run (not requested).

Next steps:

1. Run node Backend/scripts/ensure-built-in-plugins.js --force locally or in CI to verify the forced rebuild path and confirm all bundles copy over correctly.

opencode session  |  github run

[github-actions]

User opencode-agent[bot] does not have write permissions

github run

[github-actions]

User opencode-agent[bot] does not have write permissions

github run

[github-actions]

Code Review

Critical Issues

1. resolvePaths() breaks user workflow assumptions (scripts/tauri-build.js:92-103)
The function prioritizes process.cwd() but only checks if Backend/tauri.conf.json exists there. If a user runs the script from a subdirectory without a Backend/tauri.conf.json, it silently falls back to __dirname-based paths. This subtle behavior change could confuse users who expect CWD to be respected. The fallback should at least warn:

if (fs.existsSync(cwdTauriConfig)) {
  return { repoRoot: cwdRepoRoot, backendDir: cwdBackendDir };
}
console.warn(`CWD not a repo root (no ${cwdTauriConfig}); using script location.`);

2. Missing --force validation (Backend/scripts/ensure-built-in-plugins.js:271-276)
The script accepts --force but doesn't validate unexpected arguments. A typo like --forc silently ignores the flag. Consider:

const unknownArgs = process.argv.filter(a => !a.startsWith('--') || (a !== '--force'));
if (unknownArgs.length > 1) {
  console.error(`Unknown arguments: ${unknownArgs.join(', ')}`);
  process.exit(1);
}

Minor Issues

3. Redundant ensureBundlesDir() call (ensure-built-in-plugins.js:195)
copyPackagedBundles() calls ensureBundlesDir() at line 196, but it's also called at line 271. This is harmless but unnecessary—the call at line 271 could be removed since copyPackagedBundles already handles it.

4. promptHidden potential deadlock in non-TTY (tauri-build.js:59-85)
When stdin.isTTY is false, raw mode isn't enabled but the promise never resolves if stdin closes. This won't affect CI (signing key pre-set via secrets), but could hang if called unexpectedly. Consider adding a timeout:

const timeout = new Promise((_, reject) => setTimeout(() => reject(new Error('Timeout waiting for input')), 30000));

5. Silent .env.tauri.local skip (tauri-build.js:107)
loadLocalEnv silently skips missing files. A debug log would help developers debug env loading issues:

if (!fs.existsSync(filePath)) {
  console.debug(`No local env file at ${filePath}, skipping.`);
  return;
}

6. process.exit(res.status || 1) loses error detail (ensure-built-in-plugins.js:245)
When res.status is 0 but res.error exists, this correctly exits with 1. However, if res.error is set and res.status is non-zero, the specific status is returned which is fine. Consider also logging res.error on failure:

if (res.error) {
  console.error(`Failed to run npm dist for ${pluginName}:`, res.error);
  process.exit(1);
}

Positive Aspects

• Clean architectural shift from SDK CLI to local npm
• Well-documented helper functions with JSDoc
• Proper fallback logic in resolvePaths() preserves backward compatibility
• Good error propagation with process.exit(1) on failures
• Robust .ovcsp archive handling with multi-archive warning
• CI workflows correctly integrate the plugin build step
• --force flag added for CI determinism

Summary

Solid PR with well-executed architectural changes. The main concern is the silent behavior change in resolvePaths() fallback—adding a warning would help users debug unexpected behavior. The other issues are minor polish suggestions.

opencode session  |  github run