starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-09-12 16:05:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: unpin only the specifiers in the bundle a bun modules run executes (#11083)

Browse Source 
* fix: keep version pins from imported scripts in bun lockfiles

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WCq4nkUjiZBnPPMuGYCo6w

* fix: strip version pins from the bundle a bun modules run executes

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WCq4nkUjiZBnPPMuGYCo6w

* fix: unpin only module specifiers, not matching text elsewhere in the script

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WCq4nkUjiZBnPPMuGYCo6w

* docs: name the raw endpoint lock generation fetches imports through

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WCq4nkUjiZBnPPMuGYCo6w

* fix: leave require calls alone and skip spans not on a quote pair when unpinning

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WCq4nkUjiZBnPPMuGYCo6w

* test: create the bun bundle cache dir a dependency job saves into

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WCq4nkUjiZBnPPMuGYCo6w

* test: drop the lock test #11082's module test already covers

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WCq4nkUjiZBnPPMuGYCo6w

* fix: narrow the change to a fail-open strip of the modules-run bundle

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WCq4nkUjiZBnPPMuGYCo6w

* fix: unpin only the specifiers in the modules-run bundle, and log a parse fallback

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WCq4nkUjiZBnPPMuGYCo6w

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ruben Fiszel
2026-09-11 13:09:45 +00:00
committed by GitHub
co-authored by Claude Opus 5
parent d539e8674f
commit 30ffdbecc1
6 changed files with 209 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/parsers/windmill-parser-ts/src/lib.rs
+101
View File
@@ -694,6 +694,107 @@ pub fn remove_pinned_imports(code: &str) -> anyhow::Result<String> {
Ok(content)
}
/// Spans of the string literals naming a loaded module: `import`/`export … from` sources and the
/// argument of a dynamic `import()`. A `require()` call is left out: `require` is an ordinary
/// binding a script can shadow, so its argument is not known to be a module.
struct ImportSpecifierSpans(Vec<Span>);
impl Visit for ImportSpecifierSpans {
noop_visit_type!();
fn visit_import_decl(&mut self, n: &swc_ecma_ast::ImportDecl) {
self.0.push(n.src.span);
}
fn visit_export_all(&mut self, n: &swc_ecma_ast::ExportAll) {
self.0.push(n.src.span);
}
fn visit_named_export(&mut self, n: &swc_ecma_ast::NamedExport) {
if let Some(src) = &n.src {
self.0.push(src.span);
}
}
fn visit_call_expr(&mut self, n: &swc_ecma_ast::CallExpr) {
if let (swc_ecma_ast::Callee::Import(_), Some(arg)) = (&n.callee, n.args.first()) {
if let (None, Expr::Lit(Lit::Str(s))) = (arg.spread, &*arg.expr) {
self.0.push(s.span);
}
}
n.visit_children_with(self);
}
}
/// Drops the `@version` from each pinned module specifier (`pkg@1.2.3/sub` -> `pkg/sub`),
/// rewriting only the specifier literals. Unlike [`remove_pinned_imports`], the same text
/// elsewhere, such as a string the script returns, stays as written.
pub fn remove_pinned_import_specifiers(code: &str) -> anyhow::Result<String> {
let cm: Lrc<SourceMap> = Default::default();
let fm = cm.new_source_file(
FileName::Custom("main.d.ts".into()).into(),
code.to_string(),
);
let mut tss = TsSyntax::default();
tss.tsx = true;
tss.no_early_errors = true;
let lexer = Lexer::new(
Syntax::Typescript(tss),
Default::default(),
StringInput::from(&*fm),
None,
);
let module = Parser::new_from(lexer).parse_module().map_err(|e| {
anyhow::anyhow!("Error while parsing code, it is invalid TypeScript: {e:?}")
})?;
let mut specifiers = ImportSpecifierSpans(vec![]);
specifiers.visit_module(&module);
specifiers.0.sort_by_key(|s| s.lo);
// Spans index the parsed source, which the source map stripped of any UTF-8 BOM.
let bom = if code.starts_with('\u{feff}') {
'\u{feff}'.len_utf8()
} else {
0
};
let offset =
|pos: swc_common::BytePos| pos.0.checked_sub(fm.start_pos.0).map(|o| bom + o as usize);
let mut content = String::with_capacity(code.len());
let mut copied = 0;
for span in specifiers.0 {
// A span covers the literal's quotes. One that does not land on a matching pair is left
// as written rather than risk rewriting the wrong bytes.
let (Some(open), Some(close)) = (
offset(span.lo),
offset(span.hi).and_then(|e| e.checked_sub(1)),
) else {
continue;
};
let quote = code.as_bytes().get(open);
if open >= close
|| open < copied
|| !matches!(quote, Some(b'"' | b'\''))
|| code.as_bytes().get(close) != quote
{
continue;
}
let Some(specifier) = code.get(open + 1..close) else {
continue;
};
let unpinned = IMPORTS_VERSION.captures(specifier).and_then(|x| {
x.get(1)
.map(|y| format!("{}{}", y.as_str(), x.get(2).map_or("", |z| z.as_str())))
});
if let Some(unpinned) = unpinned.filter(|u| u != specifier) {
content.push_str(&code[copied..open + 1]);
content.push_str(&unpinned);
copied = close;
}
}
content.push_str(&code[copied..]);
Ok(content)
}
fn resolve_type_ref(type_resolver: &HashMap<String, (Typ, bool)>, typ: &mut Typ) {
let mut visited = std::collections::HashSet::new();
resolve_type_ref_with_visited(type_resolver, typ, &mut visited);
backend/parsers/windmill-parser-ts/tests/tests.rs
+41
View File
@@ -4,6 +4,7 @@ mod tests {
use windmill_parser::{Arg, MainArgSignature, ObjectProperty, ObjectType, Typ};
use windmill_parser_ts::{
parse_deno_signature, parse_expr_for_imports, parse_relative_imports,
remove_pinned_import_specifiers,
};
#[test]
@@ -33,6 +34,46 @@ mod tests {
);
}
#[test]
fn test_remove_pinned_import_specifiers_rewrites_only_specifiers() {
let code = r#"// héllo
import a from "pkg@1.2.3";
import b from "@scope/pkg@^2/sub";
export * from "other@3";
import rel from "./helper";
const c = await import("dyn@4");
const require = (v: string) => v;
const d = require("req@5");
// pkg@1.2.3
export const label = "pkg@1.2.3";
"#;
assert_eq!(
remove_pinned_import_specifiers(code).unwrap(),
r#"// héllo
import a from "pkg";
import b from "@scope/pkg/sub";
export * from "other";
import rel from "./helper";
const c = await import("dyn");
const require = (v: string) => v;
const d = require("req@5");
// pkg@1.2.3
export const label = "pkg@1.2.3";
"#
);
assert_eq!(
remove_pinned_import_specifiers("\u{feff}import a from 'pkg@1';").unwrap(),
"\u{feff}import a from 'pkg';"
);
assert_eq!(
remove_pinned_import_specifiers(
"// a\r\n// b\r\nimport a from \"pkg@1\";\r\nimport b from 'x@2';"
)
.unwrap(),
"// a\r\n// b\r\nimport a from \"pkg\";\r\nimport b from 'x';"
);
}
#[test]
fn test_parse_empty_main_signature() {
let code = r#"
backend/tests/bun_jobs.rs
+51
View File
@@ -939,6 +939,57 @@ export function main() { return midValue(); }"#,
Ok(())
}
/// A run with local modules and no lock executes the bundle its lock generation built, which
/// kept the imported script's pin; the run must still load the one copy in node_modules, and
/// leave the script's own data alone even where it matches the pinned specifier.
#[sqlx::test(fixtures("base"))]
async fn test_bun_modules_run_loads_imported_pin_from_node_modules(
db: Pool<Postgres>,
) -> anyhow::Result<()> {
initialize_tracing().await;
let server = ApiServer::start(db.clone()).await?;
let port = server.addr.port();
insert_deployed_bun_script(
&db,
"f/pinned_import_modules/module",
41240002,
r#"import * as isNumber from "is-number@6.0.0";
export const ns = isNumber;
export const label = "is-number@6.0.0";"#,
)
.await;
let job = JobPayload::Code(RawCode {
content: r#"import * as isNumber from "is-number";
import { ns, label } from "/f/pinned_import_modules/module";
import { local } from "./helper";
export function main() { return [ns === isNumber, label, local()]; }"#
.into(),
path: Some("f/pinned_import_modules/main".into()),
language: ScriptLang::Bun,
modules: Some(std::collections::HashMap::from([(
"helper.ts".to_string(),
windmill_common::scripts::ScriptModule {
content: "export const local = () => 'local';".into(),
language: ScriptLang::Bun,
lock: None,
},
)])),
..RawCode::default()
});
let result = run_job_in_new_worker_until_complete(&db, false, job, port)
.await
.json_result()
.unwrap();
assert_eq!(
result,
serde_json::json!([true, "is-number@6.0.0", "local"])
);
Ok(())
}
#[sqlx::test(fixtures("base", "bun_edge_cases"))]
async fn test_bun_shared_imports_both_styles(db: Pool<Postgres>) -> anyhow::Result<()> {
initialize_tracing().await;
backend/windmill-worker/loader.bun.js
+2
View File
@@ -105,6 +105,8 @@ const p = {
const normalized = (isRelative ? join(dirname(file_path), pathNoExt) : pathNoExt.slice(1)).replace(/\\/g, "/");
const hash = TEMP_SCRIPT_REFS?.[normalized];
// Lock generation substitutes `raw`: the dependency scan reads versions from the
// `pkg@version` specifiers in imported scripts, which `raw_unpinned` strips.
const url = (isRelative
? `${base_internal_url}/api/w/${w_id}/scripts/RAW_GET_ENDPOINT/p/${file_path}/../${args.path}${endExt}`
: `${base_internal_url}/api/w/${w_id}/scripts/RAW_GET_ENDPOINT/p/${args.path}${endExt}`
backend/windmill-worker/src/bun_executor.rs
+13 -2
View File
@@ -13,7 +13,7 @@ use itertools::Itertools;
use serde_json::value::RawValue;
use uuid::Uuid;
use windmill_parser_ts::remove_pinned_imports;
use windmill_parser_ts::{remove_pinned_import_specifiers, remove_pinned_imports};
use windmill_queue::{append_logs, CanceledBy, MiniPulledJob, PrecomputedAgentInfo};
@@ -1784,8 +1784,19 @@ pub async fn handle_bun_job(
if modules.as_ref().is_some_and(|m| !m.is_empty()) {
let bundle_path = std::path::Path::new(job_dir).join("out").join("main.js");
if bundle_path.exists() {
// The lock-generation build kept every `pkg@version` specifier, and bun resolves
// a pinned specifier outside node_modules, loading a second copy of the package.
// The bundle holds the user's code too, so only the specifiers are rewritten, and
// a bundle the parser rejects still runs as built, pins and all.
let bundled = std::fs::read_to_string(&bundle_path)?;
write_file(job_dir, "main.ts", &bundled)?;
let unpinned = remove_pinned_import_specifiers(&bundled).unwrap_or_else(|e| {
tracing::warn!(
job_id = %job.id,
"could not unpin the modules bundle, running it as built: {e:#}"
);
bundled
});
write_file(job_dir, "main.ts", &unpinned)?;
}
}
"\n\n--- BUN CODE EXECUTION ---\n".to_string()
cli/test/sync_push_auto_metadata_repro.test.ts
+1 -1
View File
@@ -168,7 +168,7 @@ test(
// Customer scenario: a barrel file (f/lib/errors/index.ts) re-exports from
// siblings (./types.ts, ./WorkflowError.ts, ...). An importer in a different
// folder imports from the barrel. On a fresh DB, the dep job for the importer
// fetches index.ts via raw_unpinned + temp_script_hash, but bun's resolver
// fetches index.ts via raw + temp_script_hash, but bun's resolver
// then has to resolve the barrel's *sibling* imports — and those need to be
// in TEMP_SCRIPT_REFS too.
test(