fix(cli): resolve cross-folder relative imports during lockgen on fresh DB#9048
Conversation
…sh DB
On a fresh workspace, lockfile generation for scripts that imported other
scripts via cross-folder relative imports (or barrel re-exporters) failed with
"Failed to find relative import" because the dep job's bun build hit the
server before any helper was deployed. Three independent bugs combined to
produce this:
1. wmill sync push --auto-metadata regenerated locks per script without
building a DoubleLinkedDependencyTree or calling uploadScripts, so
temp_script_refs was never sent to dependencies_async.
2. wmill script generate-metadata (the deprecated alias) had its own old
in-line implementation that bypassed the tree entirely.
3. The TypeScript WASM parser dropped re-exports (export * from, export { x }
from) when called with skip_type_only=false — the path used by
parse_relative_imports — so barrel files looked like leaves to the CLI's
dependency tree and their sibling helpers were missing from
temp_script_refs.
Fix:
- sync.ts: --auto-metadata mirrors generate-metadata's flow (dryRun pass to
populate tree → propagateStaleness → uploadScripts → real pass with tree).
- script.ts: deprecated wmill script generate-metadata now delegates to the
canonical generateMetadata, which already does the tree+upload dance.
- parser-ts: visit_export_all and visit_named_export had inverted skip_type_only
guards; aligned with visit_import_decl's pattern.
Includes 4 E2E tests reproducing each customer-hit failure path and a Rust
unit test for the re-export parser fix.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Deploying windmill with
|
| Latest commit: |
af817b4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://00643e6f.windmill.pages.dev |
| Branch Preview URL: | https://fix-sync-push-auto-metadata.windmill.pages.dev |
Pin the parser package to the version published with the re-export fix (visit_export_all / visit_named_export skip_type_only=false) so the CLI and frontend pick it up at the next release. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Claude finished @rubenfiszel's task in 6m 2s —— View job Claude PR ReviewVerdict: Good to merge. The fix is well-targeted and the three independent bugs are each addressed at the right layer with matching test coverage. Below is a layer-by-layer assessment. Parser fix (
|
| caller | skip_type_only |
old behavior | new behavior |
|---|---|---|---|
parse_relative_imports (CLI tree) |
false |
dropped re-exports ❌ | captures all re-exports ✅ |
parse_ts_imports (frontend Editor) |
false |
dropped re-exports ❌ | captures all re-exports ✅ |
parse_ts_relative_imports (backend dep-map) |
true |
excluded type-only re-exports ✅ | unchanged ✅ |
The unit test (tests.rs:940-961) covers all four re-export forms (export *, export { x } from, export * as, export type { x } from) and would fail on the old code (returns empty list) and passes on the new — solid regression guard.
Sync auto-metadata two-pass flow (cli/src/commands/sync/sync.ts)
The refactor correctly mirrors the canonical generateMetadata flow in cli/src/commands/generate-metadata/generate-metadata.ts:266-374:
- Pass 1 (always
dryRun=true): populatesDoubleLinkedDependencyTreeviagenerateScriptMetadataInternal's tree branch (metadata.ts:294-300). Returns early so no metadata is written. tree.propagateStaleness()+uploadScripts(tree, workspace)— same call sequence as the canonical generator.- Pass 2 (only
autoRegenerate,dryRun=false): re-runs eachtracker.{scripts,flows,apps,rawApps}change sotree.getTempScriptRefs(remotePath)is consulted at lockgen time (metadata.ts:350).
The non---auto-metadata path is preserved exactly: pass 1 with tree=undefined falls through to the legacy checkifMetadataUptodate short-circuit (metadata.ts:302-326), pass 2 is skipped, and the staleScripts/staleFlows/staleApps warnings still print in the original order. Try/catch around uploadScripts plus the warn-and-continue fallback for older backends is appropriate — degraded behavior matches what users on older backends already see.
One small observation (not a blocker): pass 2 doesn't filter tracker.scripts by tree.getStaleReason(p) the way the canonical generator does (generate-metadata.ts:392-401). Sync push regenerates locks for every item in tracker, which mirrors the previous --auto-metadata behavior, so this is consistent rather than a regression — flagging only because the divergence is subtle.
Deprecated wmill script generate-metadata delegation (cli/src/commands/script/script.ts)
The delegation to the canonical implementation is correct: skipFlows: true, skipApps: true preserves the alias's "scripts only" contract, and the scriptPath argument is forwarded as the folder filter. Verified that the canonical's filter logic (generate-metadata.ts:404-460) handles both extension and extensionless forms via the folderNoExt strip + touchesFolder transitive check, so wmill script generate-metadata f/foo/bar and ... f/foo/bar.ts both work. The dynamic import is fine here since this command is invoked rarely.
The as any cast on opts is acceptable — the only field shape difference is the absence of skipScripts/skipFlows/skipApps on the script generate-metadata opts, and we explicitly set those.
Test coverage assessment
- Backend (Rust): ✅ The new
test_relative_imports_includes_re_exportscovers each re-export form. The PR description confirmscargo testinwindmill-parser-tsis 32 pass / 0 fail. - CLI (TS): ✅ Four E2E tests in
sync_push_auto_metadata_repro.test.tscover (a)sync push --auto-metadatacross-folder, (b) canonicalgenerate-metadatawith deep cross-folder, (c) barrel re-exporter scenario, (d) deprecated alias. Tests usewithTestBackendagainst a fresh backend, which is exactly the failure surface customers hit. - Frontend: No frontend code changed (only a parser package version bump).
parse_ts_importsalready feedsparseTypescriptDeps(frontend/src/lib/relative_imports.ts:4), which is consumed byEditor.svelteandDev.svelte; the parser change makes barrel re-exports show up there too. No frontend test layer expected to cover this. - Manual verification: a single fresh-DB run exercising both
sync push --auto-metadata --yesandgenerate-metadata --yesagainst a workspace with both cross-folder relative imports and a barrelexport * from "./helper.ts"— confirm*.script.lockfiles are non-empty and that the deployed scripts'lock_error_logsare clean. The PR description already lists this in the test plan.
No blocking issues. AGENTS.md compliance is fine — no new pub fn to audit, the parser change is internal logic in an existing function with an existing doc comment, and the CLI changes stay within established patterns.
· Branch
Codex ReviewGood to merge No issues found. Checked the diff for bugs, security problems, new public-surface risks, and Test coverageBackend parser changes add a Rust unit test for relative re-export parsing. CLI behavior is covered by the new E2E repro tests for Manual verification before merge should cover running the CLI against a fresh workspace with cross-folder relative imports and barrel re-exports, confirming lockfiles are generated and the pushed scripts have no lock generation errors. |
Pi ReviewGood to merge SummaryThis PR fixes lockfile generation on fresh workspaces for scripts that use cross-folder relative imports or barrel re-exports. Three independent bugs are addressed:
Changes reviewed
FindingsNo issues found. The changes follow established patterns ( Test coverageBackend (Rust parser): One new unit test ( CLI (TypeScript E2E): Four new E2E tests cover the three customer-hit failure modes against a fresh test backend: Frontend: Only a Manual verification needed before merge:
|
… gen pass Delegating wmill script generate-metadata fully to the canonical handler broke 4 workspace_deps_filter tests that rely on the legacy hash-with-deps formula and the "No metadata to update" output string. Restore the original in-line implementation (legacy stale-check preserved), but add a DoubleLinkedDependencyTree + uploadScripts pass before the actual generation step. The customer's bug only manifests on real lockgen, not on the dry-run staleness check, so this preserves the existing test contract while still fixing cross-folder relative imports for the deprecated alias. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="cli/src/commands/script/script.ts">
<violation number="1">
P2: The scriptPath branch still bypasses the temp_script_refs upload flow, so generating metadata for a single script can 404 on fresh workspaces when it imports undeployed siblings via relative paths. Consider reusing the two-pass tree/upload flow (or delegating to the canonical generateMetadata) even when scriptPath is provided.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
| @@ -1380,7 +1380,42 @@ export async function generateMetadata( | |||
| log.info(colors.green.bold("No metadata to update")); | |||
There was a problem hiding this comment.
P2: The scriptPath branch still bypasses the temp_script_refs upload flow, so generating metadata for a single script can 404 on fresh workspaces when it imports undeployed siblings via relative paths. Consider reusing the two-pass tree/upload flow (or delegating to the canonical generateMetadata) even when scriptPath is provided.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At cli/src/commands/script/script.ts, line 1318:
<comment>The scriptPath branch still bypasses the temp_script_refs upload flow, so generating metadata for a single script can 404 on fresh workspaces when it imports undeployed siblings via relative paths. Consider reusing the two-pass tree/upload flow (or delegating to the canonical generateMetadata) even when scriptPath is provided.</comment>
<file context>
@@ -1307,20 +1307,129 @@ export async function generateMetadata(
+ const rawWorkspaceDependencies = await getRawWorkspaceDependencies(true);
+ if (scriptPath) {
+ // read script metadata file
+ await generateScriptMetadataInternal(
+ scriptPath,
+ workspace,
</file context>
Codex ReviewShould address issues before merging: Findings
Test coverageBackend parser coverage looks adequate: the new Rust unit test covers relative re-exports. CLI coverage covers The missing manual/test scenario is the optional deprecated alias argument: on a fresh workspace, run |
Pi ReviewGood to merge SummaryThis PR fixes lockfile generation on fresh workspaces for scripts with cross-folder relative imports and barrel re-exports. Three bugs combine to cause the
FindingsP2 — doc/code mismatch in PR description (
|
| File | Change |
|---|---|
backend/parsers/windmill-parser-ts/src/lib.rs |
Fix inverted skip_type_only guards in visit_export_all / visit_named_export |
backend/parsers/windmill-parser-ts/tests/tests.rs |
New unit test covering all four re-export forms |
cli/src/commands/sync/sync.ts |
Two-pass tree-based lockgen for --auto-metadata; non-auto path unchanged |
cli/src/commands/script/script.ts |
Inline tree+uploadScripts logic for deprecated alias |
cli/src/commands/generate-metadata/generate-metadata.ts |
generateMetadata made export (unused by this PR) |
cli/test/sync_push_auto_metadata_repro.test.ts |
4 E2E tests covering fresh-DB failure modes |
cli/package.json, cli/bun.lock |
Bumped windmill-parser-wasm-ts to 1.695.0 |
frontend/package.json, frontend/package-lock.json |
Bumped windmill-parser-wasm-ts to 1.695.0 |
Test coverage
Backend (Rust parser): One new unit test (test_relative_imports_includes_re_exports) covers export *, export { x } from, export * as, and export type { x } from. Existing 31 tests pass. Adequate.
CLI (TypeScript E2E): Four new E2E tests covering each customer-hit failure mode against a fresh test backend: sync push --auto-metadata with cross-folder imports (alphabetical ordering edge case), generate-metadata with deep cross-folder imports, generate-metadata with barrel re-exporters, and the deprecated wmill script generate-metadata alias. Existing sync/metadata tests continue to pass (115 pass, 3 skip). Adequate.
Frontend: Only a package.json / package-lock.json version bump — no component or logic changes, no tests expected.
Manual verification needed before merge:
On a fresh local Postgres volume, run wmill sync push --yes --auto-metadata against a workspace containing scripts with ../sibling.ts imports and a barrel export * from "./helper.ts" file; confirm *.script.lock files are non-empty and the deployed scripts have no lock_error_logs. On the same fresh DB, run wmill script generate-metadata --yes (deprecated alias) with and without an explicit script-path argument and confirm it completes without Failed to find relative import errors.
Summary
On a fresh workspace, lockfile generation for scripts that import other scripts via cross-folder relative imports (or barrel re-exporters) fails with
Failed to find relative import/Non-zero exit status for bun build: 1. The dep job's bun build hits the server before any helper has been deployed, soraw_unpinned404s for every relative-import target. Three independent bugs combined to make this irrecoverable on every flag combination users tried (--auto-metadata,wmill generate-metadata && wmill sync push,wmill script generate-metadata && wmill sync push).Changes
cli/src/commands/sync/sync.ts—wmill sync push --auto-metadatanow mirrors the canonicalgenerate-metadataflow: dry-run pass to populate aDoubleLinkedDependencyTree,tree.propagateStaleness(),uploadScripts(tree, workspace)to populateraw_script_temp, then a real pass withtreethreaded through so each lockgen request includestemp_script_refs. The non---auto-metadatapath is unchanged.cli/src/commands/script/script.ts— The deprecatedwmill script generate-metadataalias used its own old in-line implementation that bypassed the dependency tree entirely. It now delegates to the canonicalgenerateMetadata, forwarding the optional script path as a folder filter and pinningskipFlows: true, skipApps: trueto preserve the alias's "scripts only" semantics.cli/src/commands/generate-metadata/generate-metadata.ts— ExportedgenerateMetadataso the deprecated alias can delegate to it.backend/parsers/windmill-parser-ts/src/lib.rs—visit_export_allandvisit_named_exporthad invertedskip_type_onlyguards (if !self.skip_type_only || node.type_only { return; }), so re-exports (export * from,export { x } from) were silently dropped under theskip_type_only=falsepath used byparse_relative_imports. The CLI's dependency tree therefore treated barrel files as leaves, and their sibling helpers were missing fromtemp_script_refs. Fixed to matchvisit_import_decl's correct pattern;skip_type_only=truebehavior is preserved exactly (existingtest_imports_basicstill passes).backend/parsers/windmill-parser-ts/tests/tests.rs— Rust unit test (test_relative_imports_includes_re_exports) coveringexport *,export { x } from,export * as,export type { x } from.cli/test/sync_push_auto_metadata_repro.test.ts— 4 E2E tests reproducing each customer-hit failure mode against a fresh test backend.Release note
The parser fix only takes effect once the
windmill-parser-wasm-tsnpm package is rebuilt and published frombackend/parsers/windmill-parser-wasm/pkg-ts/and the CLI'scli/package.jsonpin (currently"windmill-parser-wasm-ts": "1.693.1") is bumped to the new version. Build:nu build.nu tsfrombackend/parsers/windmill-parser-wasm/. Publish:./publish-pkgs.shfrom the same directory. The sync.ts / script.ts changes are pure TypeScript and ship with the next CLI release on their own.Test plan
cargo testinwindmill-parser-ts: 32 pass, 0 fail (was 31, +1 new)bun test test/sync_pull_push.test.ts test/sync_push_auto_metadata_repro.test.ts test/generate_metadata_unit.test.ts: 115 pass, 3 skip, 0 failwmill sync push --yes --auto-metadataagainst a workspace with../sibling.tsimports and a barrelexport * from "./helper.ts"file, then confirm*.script.lockfiles are non-empty and the deployed scripts have nolock_error_logswmill script generate-metadata --yes(deprecated alias) succeeds and still respects an explicit script-path argumentwmill generate-metadata --yessucceeds against scripts that use barrel re-exportersSummary by cubic
Fixes lockfile generation on fresh workspaces for scripts using cross-folder relative imports and barrel re-exports.
wmill sync push --auto-metadatanow resolves these imports reliably and avoids “Failed to find relative import” errors.Bug Fixes
sync.ts:--auto-metadataruns a two-pass flow (buildDoubleLinkedDependencyTree→propagateStaleness()→uploadScripts→ real pass with the tree) across scripts, flows, and apps so lockgen includestemp_script_refs. Non-auto path is unchanged.script.ts: restored the deprecatedwmill script generate-metadataalias’s legacy stale-check/output, but added a prepass usingDoubleLinkedDependencyTree+uploadScriptsbefore the real generation pass (with the tree) to fix cross-folder imports; keeps “scripts only” behavior.skip_type_onlyguards in re-export visitors soparse_relative_importsincludesexport */export { x } from …. Added a Rust unit test.Dependencies
windmill-parser-wasm-tsto1.695.0in the CLI and frontend to pick up the re-export parsing fix.Written for commit af817b4. Summary will update on new commits.