diff --git a/.github/workflows/pr-trusted.yml b/.github/workflows/pr-trusted.yml index 09e6144e041..e0db2a58a2a 100644 --- a/.github/workflows/pr-trusted.yml +++ b/.github/workflows/pr-trusted.yml @@ -416,6 +416,11 @@ jobs: - name: Typecheck workspaces whose build scripts skip TypeScript run: pnpm run typecheck:build-gaps + - name: Typecheck the server in low-memory layers + env: + PAPERCLIP_TYPECHECK_LOW_MEMORY: "1" + run: pnpm --filter @paperclipai/server typecheck + - name: Verify repository-wide CLI guidance safety run: pnpm exec vitest run --project @paperclipai/server server/src/__tests__/cli-invocation-safety.test.ts diff --git a/AGENTS.md b/AGENTS.md index 31c37642966..800d3cc973e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -223,6 +223,8 @@ pnpm test:run pnpm build ``` +If memory is limited, use `node scripts/typecheck-low-memory.mjs` instead of `pnpm -r typecheck`. See `doc/DEVELOPING.md`. + If anything cannot be run, explicitly report what was not run and why. ## 8. API and Auth Expectations diff --git a/Dockerfile b/Dockerfile index 5360d08396a..e392be44845 100644 --- a/Dockerfile +++ b/Dockerfile @@ -146,8 +146,11 @@ RUN pnpm --filter @paperclipai/plugin-sdk build # next RUN as an environment variable. The production stage below declares the # same ARG again for the runtime fallback; an ARG goes out of scope at the # end of its stage. Empty for local `docker build`, which then writes no stamp. +# NODE_OPTIONS covers the Node build steps. tsc is a native binary and ignores it. ENV NODE_OPTIONS=--max-old-space-size=4096 -RUN pnpm --filter @paperclipai/server build +# The image needs no declarations, and CI type-checks this source separately. +# Emitting without either cuts the tsc step from more than 10 GiB to about 2.5 GiB. +RUN PAPERCLIP_TSC_EMIT_ONLY=1 pnpm --filter @paperclipai/server build RUN test -f server/dist/index.js || (echo "ERROR: server build output missing" && exit 1) RUN pnpm --filter @paperclipai/server build:runtime \ && test -f server/dist-runtime/index.mjs || (echo "ERROR: server runtime bundle missing" && exit 1) diff --git a/cli/src/commands/run.ts b/cli/src/commands/run.ts index a141a9a42ce..5a1d9fa7f03 100644 --- a/cli/src/commands/run.ts +++ b/cli/src/commands/run.ts @@ -232,8 +232,11 @@ async function importServerEntry(): Promise { // Production mode: import the published @paperclipai/server package try { - const mod = await import("@paperclipai/server"); - return await startServerFromModule(mod, "@paperclipai/server"); + // A literal specifier would pull the whole server program into the CLI + // typecheck. This import is untyped either way, like the dev import above. + const serverPackage = "@paperclipai/server"; + const mod = await import(serverPackage); + return await startServerFromModule(mod, serverPackage); } catch (err) { const missingSpecifier = getMissingModuleSpecifier(err); const missingServerEntrypoint = !missingSpecifier || missingSpecifier === "@paperclipai/server"; diff --git a/doc/DEVELOPING.md b/doc/DEVELOPING.md index d7608647f99..cb6a7d2a57e 100644 --- a/doc/DEVELOPING.md +++ b/doc/DEVELOPING.md @@ -1132,6 +1132,46 @@ falls back to the fresh path automatically, so the cache can make a suite slower but never fail it or skip it: a migration that cannot apply still fails the suite on the fresh path. +#### Typecheck memory + +TypeScript 7 is a native binary, so `NODE_OPTIONS=--max-old-space-size` does not +limit `tsc`. Its peak memory depends on how much source one process checks and on +the number of checkers, because each checker keeps its own copy of the types. + +`pnpm --filter @paperclipai/server typecheck` does not check one program. +`server/scripts/typecheck-layers.mjs` reads the import graph and runs one `tsc` +process per layer. Each layer reads the earlier layers as declarations: + +| Layer | Files | +| --- | --- | +| `below` | Files the largest import cycle depends on, and unrelated files | +| `cycle` | The largest import cycle | +| `above` | Files that depend on the cycle | +| `tests` | Test files | + +The generated configs and declarations go in +`server/node_modules/.cache/paperclip-typecheck`. You do not maintain a layer list. +`server/tsconfig.json` remains the single program for editors and `pnpm build`. +The server typecheck builds only the Runner's TypeScript output. Use +`pnpm --filter @paperclipai/paperclip-runner build` when a local check needs the +Runner binary. + +If the memory available to the task is limited, use this command: + +```sh +node scripts/typecheck-low-memory.mjs +``` + +It peaks at about 3.8 GiB, against about 5.5 GiB for `pnpm -r typecheck` on 16 +cores. It also takes about twice as long. + +It checks one package at a time and uses one thread for the server layers and the +UI. Do not add `GOMEMLIMIT` to this path. It saves at most 300 MiB here, and it +slows `tsc` by up to 20 times when the live heap goes above the limit. +`pnpm build` still checks the whole server program and needs more than 8 GiB. The +Docker image sets `PAPERCLIP_TSC_EMIT_ONLY=1` for that build, which emits +JavaScript without checks or declarations. + #### Reused Runner build in the server build The server build vendors `packages/paperclip-runner/dist`, so its diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_backend.rs b/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_backend.rs index c7cee722c59..1e445b261d1 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_backend.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_backend.rs @@ -426,7 +426,11 @@ impl AcpxProviderDescriptor { grok_disable_web_search: self.grok_disable_web_search.clone(), permission_mode_pinned: self.permission_mode_pinned, provider_policy: self.provider_policy.clone(), - system_instructions: self.instructions.clone(), + system_instructions: crate::task_memory::instructions_with_memory_note( + &self.instructions, + crate::acpx_provider_session::MAX_SYSTEM_INSTRUCTIONS_BYTES, + ) + .into_owned(), runtime_context: self.runtime_context.clone(), tool_set: provider_tool_set, expected_identity, @@ -3958,6 +3962,26 @@ mod tests { session_config.system_instructions, descriptor_value["instructions"] ); + // The note reaches the provider without changing the durable descriptor. + let task_cgroup = directory.join("tasks").join("task-a"); + fs::create_dir_all(&task_cgroup).unwrap(); + fs::write(directory.join("tasks").join("memory.max"), "6442450944\n").unwrap(); + crate::task_memory::TEST_TASK_CGROUP.with(|path| *path.borrow_mut() = Some(task_cgroup)); + let session_config = refreshed + .session_config( + attached.state.as_ref().unwrap().tool_set.clone(), + attached.state.as_ref().unwrap().identity.clone(), + Some(&launch_profile), + ) + .unwrap(); + crate::task_memory::TEST_TASK_CGROUP.with(|path| *path.borrow_mut() = None); + assert!(session_config + .system_instructions + .starts_with(descriptor_value["instructions"].as_str().unwrap())); + assert!(session_config + .system_instructions + .contains("6 GiB memory limit")); + assert_eq!(refreshed.instructions, descriptor_value["instructions"]); // Both persistence and the sidecar launch config receive the new grant. let persisted: AcpxDurableState = serde_json::from_slice(&fs::read(attached.state_path()).unwrap()).unwrap(); diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_session.rs b/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_session.rs index 943f3b78aa8..d0597a74313 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_session.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/acpx_provider_session.rs @@ -24,7 +24,7 @@ use crate::stable_identity::{is_stable_id, DURABLE_STABLE_ID_CHARS, SHORT_STABLE const MAX_ID_CHARS: usize = 240; const MAX_MODEL_CHARS: usize = 240; -const MAX_SYSTEM_INSTRUCTIONS_BYTES: usize = 1024 * 1024; +pub(crate) const MAX_SYSTEM_INSTRUCTIONS_BYTES: usize = 1024 * 1024; const MAX_JSON_SAFE_INTEGER: u64 = 9_007_199_254_740_991; #[derive(Clone, Copy, Debug, Deserialize, Serialize, PartialEq, Eq)] diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/bin/paperclip-runnerd.rs b/packages/paperclip-runner/runner/crates/runner-core/src/bin/paperclip-runnerd.rs index 13d237e3276..0362b8a554a 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/bin/paperclip-runnerd.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/bin/paperclip-runnerd.rs @@ -495,6 +495,13 @@ fn apply_oom_score_adj(args: &[String]) -> Result<(), LocalRunnerError> { Ok(()) } +fn apply_task_memory_cgroup(args: &[String]) -> Result<(), LocalRunnerError> { + if let Some(path) = optional_value(args, "--task-memory-cgroup")? { + paperclip_runner_core::task_memory::set_task_cgroup(PathBuf::from(path)); + } + Ok(()) +} + fn run(args: &[String]) -> Result<(), LocalRunnerError> { #[cfg(unix)] if matches!( @@ -515,6 +522,7 @@ fn run(args: &[String]) -> Result<(), LocalRunnerError> { return Ok(()); } apply_oom_score_adj(args)?; + apply_task_memory_cgroup(args)?; if args.iter().any(|argument| argument == "--connect-url") || args.iter().any(|argument| argument == "--listen-address") || args.iter().any(|argument| argument == "--listen-port") diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/codex_provider.rs b/packages/paperclip-runner/runner/crates/runner-core/src/codex_provider.rs index 33449c81a66..fefedb0501a 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/codex_provider.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/codex_provider.rs @@ -1254,7 +1254,13 @@ impl CodexProvider { } else { "baseInstructions" }; - params_object.insert(instruction_field.to_owned(), json!(config.instructions)); + params_object.insert( + instruction_field.to_owned(), + json!(crate::task_memory::instructions_with_memory_note( + &config.instructions, + MAX_INSTRUCTIONS_BYTES, + )), + ); if provider.permission_profile == "paperclip-runner-external-sandbox" { // The execution target (for example Daytona) is the OS sandbox. // Codex must not try to create nested user/network namespaces, diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/durable/runner.rs b/packages/paperclip-runner/runner/crates/runner-core/src/durable/runner.rs index 13a61f31271..ddfd4034c94 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/durable/runner.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/durable/runner.rs @@ -476,6 +476,7 @@ pub fn run_durable_runner( #[cfg(unix)] let mut notifications = crate::runtime_notifications::RuntimeNotifications::bind(&config.state_dir).ok(); + let mut task_memory = crate::task_memory::TaskMemoryMonitor::new(); // Bind listener mode or resolve dial mode before processing commands. Dial // reconnects retain the same validated addresses so DNS cannot redirect a // retry after the trust decision. @@ -887,6 +888,9 @@ pub fn run_durable_runner( if let Some(notifications) = notifications.as_mut() { notifications.poll(&config, &mut state, &store); } + if let Some(task_memory) = task_memory.as_mut() { + task_memory.poll(&config, &mut state, &store); + } let now = current_unix_ms()?; if welcome.lease_renewal_version == Some(1) && now >= next_lease_renewal { // A failed write may still have reached the controller. diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/lib.rs b/packages/paperclip-runner/runner/crates/runner-core/src/lib.rs index df2670ec9ea..6f859e098b6 100644 --- a/packages/paperclip-runner/runner/crates/runner-core/src/lib.rs +++ b/packages/paperclip-runner/runner/crates/runner-core/src/lib.rs @@ -38,6 +38,7 @@ pub mod question_response; pub mod replay; mod stable_identity; pub mod stdio_mcp; +pub mod task_memory; use std::error::Error; use std::fmt::{self, Display, Formatter}; diff --git a/packages/paperclip-runner/runner/crates/runner-core/src/task_memory.rs b/packages/paperclip-runner/runner/crates/runner-core/src/task_memory.rs new file mode 100644 index 00000000000..5279f4f3fef --- /dev/null +++ b/packages/paperclip-runner/runner/crates/runner-core/src/task_memory.rs @@ -0,0 +1,468 @@ +//! Memory limit that a launcher applies to the processes Runner supervises. +//! +//! The kernel's cgroup files are the only source of these facts, so an environment +//! that passes no flag gets no note and no notices. + +use crate::durable::{ + DurableRunnerConfig, DurableRunnerError, DurableState, DurableStateStore, EventPriority, +}; +use notify::{RecommendedWatcher, RecursiveMode, Watcher}; +use serde_json::json; +use std::borrow::Cow; +use std::fs; +use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::{Arc, OnceLock}; + +const MAX_NOTICES: u32 = 64; +const BYTES_PER_GIB: u64 = 1 << 30; + +static TASK_CGROUP: OnceLock = OnceLock::new(); + +// Lib tests share a process, so a process-wide path would reach unrelated tests. +#[cfg(test)] +thread_local! { + pub(crate) static TEST_TASK_CGROUP: std::cell::RefCell> = const { + std::cell::RefCell::new(None) + }; +} + +/// Set once at startup from the launch flag. +pub fn set_task_cgroup(path: PathBuf) { + let _ = TASK_CGROUP.set(path); +} + +/// The limit on the parent cgroup, which tasks share. "max" means no limit. +fn read_limit(task_cgroup: &Path) -> Option { + let content = fs::read_to_string(task_cgroup.parent()?.join("memory.max")).ok()?; + content.trim().parse().ok().filter(|limit| *limit > 0) +} + +/// The kernel counts an out-of-memory kill in the victim's cgroup, so this is +/// specific to the task even though the limit is shared. +fn read_kills(task_cgroup: &Path) -> Option { + fs::read_to_string(task_cgroup.join("memory.events")) + .ok()? + .lines() + .find_map(|line| line.strip_prefix("oom_kill "))? + .trim() + .parse() + .ok() +} + +fn format_limit(bytes: u64) -> String { + if bytes.is_multiple_of(BYTES_PER_GIB) { + format!("{} GiB", bytes / BYTES_PER_GIB) + } else { + format!("{:.1} GiB", bytes as f64 / BYTES_PER_GIB as f64) + } +} + +fn memory_note(limit_bytes: u64, task_cgroup: &Path) -> Option { + let shared = task_cgroup.parent()?; + Some(format!( + "All task processes on this computer share a {limit} memory limit, and memory used by a \ + concurrent task counts toward it. Above the limit, the kernel kills a process with \ + SIGKILL, usually the largest, and the shell reports exit code 137. Other causes can also \ + produce that exit code. To check for a memory kill, read the oom_kill count in \ + {task}/memory.events. Read \ + {shared}/memory.current and {shared}/memory.peak for the shared use. After such a kill, \ + the same command will probably fail again, so reduce its memory use or split the work. \ + If the work cannot fit, report that as a blocker.", + limit = format_limit(limit_bytes), + task = task_cgroup.display(), + shared = shared.display(), + )) +} + +fn with_note_from<'a>(task_cgroup: &Path, instructions: &'a str, max_bytes: usize) -> Cow<'a, str> { + let Some(note) = read_limit(task_cgroup).and_then(|limit| memory_note(limit, task_cgroup)) + else { + return Cow::Borrowed(instructions); + }; + let separator = if instructions.is_empty() { "" } else { "\n\n" }; + // Providers reject oversized instructions, and the note is not worth a failed launch. + if instructions.len() + separator.len() + note.len() > max_bytes { + return Cow::Borrowed(instructions); + } + Cow::Owned(format!("{instructions}{separator}{note}")) +} + +/// Adds the limit to instructions for a provider whose tools run under it. +pub fn instructions_with_memory_note(instructions: &str, max_bytes: usize) -> Cow<'_, str> { + #[cfg(test)] + if let Some(path) = TEST_TASK_CGROUP.with(|path| path.borrow().clone()) { + return with_note_from(&path, instructions, max_bytes); + } + match TASK_CGROUP.get() { + Some(path) => with_note_from(path, instructions, max_bytes), + None => Cow::Borrowed(instructions), + } +} + +/// Records a run notice when the kernel reports a new kill in this task. The +/// kernel signals each change to memory.events, so an idle Runner reads nothing. +pub struct TaskMemoryMonitor { + task_cgroup: PathBuf, + reported: u64, + notices: u32, + changed: Arc, + _watcher: RecommendedWatcher, +} + +impl TaskMemoryMonitor { + pub fn new() -> Option { + TASK_CGROUP.get().cloned().and_then(Self::for_cgroup) + } + + /// Runner takes the starting count before it executes any command, so work it + /// starts is counted and a restarted Runner does not report earlier kills. + fn for_cgroup(task_cgroup: PathBuf) -> Option { + let changed = Arc::new(AtomicBool::new(false)); + let signal = Arc::clone(&changed); + let mut watcher = notify::recommended_watcher(move |_| { + signal.store(true, Ordering::Relaxed); + }) + .ok()?; + watcher + .watch( + &task_cgroup.join("memory.events"), + RecursiveMode::NonRecursive, + ) + .ok()?; + Some(Self { + reported: read_kills(&task_cgroup)?, + task_cgroup, + notices: 0, + changed, + _watcher: watcher, + }) + } + + pub fn poll( + &mut self, + config: &DurableRunnerConfig, + state: &mut DurableState, + store: &DurableStateStore, + ) { + // A change stays pending until a run can record it. + if self.notices >= MAX_NOTICES + || !self.changed.load(Ordering::Relaxed) + || state.lifecycle != "ready" + || state.warm_transition.is_some() + || state.run_id != config.run_id + { + return; + } + self.changed.store(false, Ordering::Relaxed); + let Some(kills) = read_kills(&self.task_cgroup) else { + return; + }; + if kills <= self.reported { + return; + } + if self.record(kills, config, state, store).is_ok() { + self.notices += 1; + self.reported = kills; + } else { + self.changed.store(true, Ordering::Relaxed); + } + } + + fn record( + &self, + total: u64, + config: &DurableRunnerConfig, + state: &mut DurableState, + store: &DurableStateStore, + ) -> Result<(), DurableRunnerError> { + let new_kills = total - self.reported; + let killed = if new_kills == 1 { + "a process".to_owned() + } else { + format!("{new_kills} processes") + }; + let mut next = state.clone(); + next.enqueue_event( + config, + "provider.notice.recorded", + EventPriority::P1, + json!({ + "schema": "paperclip.provider.notice.v1", + "noticeId": format!("task-memory-kill-{total}"), + "severity": "warning", + "category": "task_out_of_memory_kill", + "scope": "environment", + "recoverable": true, + "userActionable": false, + // The count also rises for a VM-wide kill, so it does not prove the limit caused it. + "summary": format!( + "The kernel's out-of-memory killer killed {killed} in this task ({total} so far)." + ), + }), + )?; + store.save(&next)?; + *state = next; + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + use std::time::{Duration, Instant}; + use uuid::Uuid; + + struct TestDirectory(PathBuf); + impl TestDirectory { + fn create() -> Self { + let path = std::env::temp_dir().join(format!("task-memory-{}", Uuid::new_v4())); + fs::create_dir_all(path.join("tasks").join("task-a")).unwrap(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + fs::set_permissions(&path, fs::Permissions::from_mode(0o700)).unwrap(); + } + Self(path) + } + fn task_cgroup(&self) -> PathBuf { + self.0.join("tasks").join("task-a") + } + fn write_limit(&self, limit: &str) { + fs::write(self.0.join("tasks").join("memory.max"), limit).unwrap(); + } + fn write_kills(&self, kills: u64) { + fs::write( + self.task_cgroup().join("memory.events"), + format!("low 0\nhigh 0\nmax 7\noom 2\noom_kill {kills}\noom_group_kill 0\n"), + ) + .unwrap(); + } + } + impl Drop for TestDirectory { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.0); + } + } + + fn config(path: PathBuf) -> DurableRunnerConfig { + DurableRunnerConfig { + connect_url: "ws://127.0.0.1/runner".into(), + ca_bundle_path: None, + state_dir: path, + runner_instance_id: "runner-1".into(), + environment_lease_id: "lease-1".into(), + run_id: "run-1".into(), + normalized_session_id: "session-1".into(), + turn_id: "turn-1".into(), + item_id: "item-1".into(), + runner_version: "1".into(), + runner_digest: "sha256:digest".into(), + acpx_launch_profile: None, + opencode_launch_profile: None, + cursor_cloud_launch_profile: None, + max_outbox_bytes: 1024 * 1024, + p0_reserve_bytes: 64 * 1024, + max_frame_bytes: 64 * 1024, + reconnect_delay: Duration::from_millis(1), + reconnect_grace: None, + max_runtime: Duration::ZERO, + } + } + + fn setup(directory: &TestDirectory) -> (DurableRunnerConfig, DurableStateStore, DurableState) { + let config = config(directory.0.clone()); + let store = DurableStateStore::new(&config.state_dir).unwrap(); + let (mut state, _) = store.load_or_create(&config).unwrap(); + state.lifecycle = "ready".into(); + (config, store, state) + } + + fn notices(state: &DurableState) -> Vec { + state + .outbox + .iter() + .filter(|event| { + event + .envelope + .pointer("/payload/eventType") + .and_then(|v| v.as_str()) + == Some("provider.notice.recorded") + }) + .map(|event| event.envelope["payload"]["payload"].clone()) + .collect() + } + + /// Polls until a watcher event has had time to produce the wanted notices. + fn poll_for_notices( + monitor: &mut TaskMemoryMonitor, + config: &DurableRunnerConfig, + state: &mut DurableState, + store: &DurableStateStore, + wanted: usize, + ) { + let deadline = Instant::now() + Duration::from_secs(10); + while notices(state).len() < wanted && Instant::now() < deadline { + monitor.poll(config, state, store); + std::thread::sleep(Duration::from_millis(10)); + } + // Later events must not add a notice. + std::thread::sleep(Duration::from_millis(100)); + monitor.poll(config, state, store); + assert_eq!(notices(state).len(), wanted); + } + + fn monitor(directory: &TestDirectory) -> TaskMemoryMonitor { + directory.write_limit("6442450944\n"); + directory.write_kills(0); + TaskMemoryMonitor::for_cgroup(directory.task_cgroup()).unwrap() + } + + #[test] + fn note_states_the_limit_the_files_and_the_way_out() { + let directory = TestDirectory::create(); + directory.write_limit("6442450944\n"); + let task = directory.task_cgroup(); + let with_note = with_note_from(&task, "Do the work.", 1024 * 1024); + assert!(with_note.starts_with("Do the work.\n\nAll task processes")); + let events = format!("{}/memory.events", task.display()); + let current = format!("{}/memory.current", task.parent().unwrap().display()); + for fact in ["6 GiB", "exit code 137", &events, ¤t, "blocker"] { + assert!(with_note.contains(fact), "missing {fact}"); + } + assert!(with_note_from(&task, "", 1024 * 1024).starts_with("All task processes")); + } + + #[test] + fn instructions_stay_unchanged_without_a_limit() { + let directory = TestDirectory::create(); + let task = directory.task_cgroup(); + assert_eq!(with_note_from(&task, "Do the work.", 4096), "Do the work."); + for limit in ["max\n", "0\n", "not a number"] { + directory.write_limit(limit); + assert_eq!(with_note_from(&task, "Do the work.", 4096), "Do the work."); + } + } + + #[test] + fn note_never_pushes_instructions_over_the_provider_limit() { + let directory = TestDirectory::create(); + directory.write_limit("6442450944\n"); + let task = directory.task_cgroup(); + let note_bytes = with_note_from(&task, "", usize::MAX).len(); + let fits = "x".repeat(10); + assert_eq!( + with_note_from(&task, &fits, 10 + 2 + note_bytes).len(), + 10 + 2 + note_bytes + ); + assert_eq!(with_note_from(&task, &fits, 10 + 2 + note_bytes - 1), fits); + } + + #[test] + fn limit_keeps_a_fraction_when_it_is_not_whole() { + assert_eq!(format_limit(6 << 30), "6 GiB"); + assert_eq!(format_limit(3 << 29), "1.5 GiB"); + } + + #[test] + fn monitor_reports_new_kills_once_with_a_valid_notice() { + let directory = TestDirectory::create(); + let (config, store, mut state) = setup(&directory); + let mut monitor = monitor(&directory); + poll_for_notices(&mut monitor, &config, &mut state, &store, 0); + + directory.write_kills(1); + poll_for_notices(&mut monitor, &config, &mut state, &store, 1); + directory.write_kills(4); + poll_for_notices(&mut monitor, &config, &mut state, &store, 2); + + let notices = notices(&state); + assert_eq!(notices[0]["schema"], "paperclip.provider.notice.v1"); + assert_eq!(notices[0]["noticeId"], "task-memory-kill-1"); + assert_eq!(notices[0]["severity"], "warning"); + assert_eq!(notices[0]["scope"], "environment"); + assert_eq!( + notices[0]["summary"], + "The kernel's out-of-memory killer killed a process in this task (1 so far)." + ); + assert_eq!( + notices[1]["summary"], + "The kernel's out-of-memory killer killed 3 processes in this task (4 so far)." + ); + } + + #[test] + fn monitor_ignores_only_kills_from_before_it_started() { + let directory = TestDirectory::create(); + let (config, store, mut state) = setup(&directory); + directory.write_limit("6442450944\n"); + directory.write_kills(5); + let mut monitor = TaskMemoryMonitor::for_cgroup(directory.task_cgroup()).unwrap(); + // This kill comes after the start but before the first poll. + directory.write_kills(6); + poll_for_notices(&mut monitor, &config, &mut state, &store, 1); + assert_eq!( + notices(&state)[0]["summary"], + "The kernel's out-of-memory killer killed a process in this task (6 so far)." + ); + } + + #[test] + fn monitor_waits_for_an_active_run_and_keeps_the_kill() { + let directory = TestDirectory::create(); + let (config, store, mut state) = setup(&directory); + let mut monitor = monitor(&directory); + directory.write_kills(2); + state.lifecycle = "reconnecting".into(); + poll_for_notices(&mut monitor, &config, &mut state, &store, 0); + state.lifecycle = "ready".into(); + poll_for_notices(&mut monitor, &config, &mut state, &store, 1); + } + + #[test] + fn monitor_reads_nothing_until_the_file_changes() { + let directory = TestDirectory::create(); + let (config, store, mut state) = setup(&directory); + let mut monitor = monitor(&directory); + directory.write_kills(3); + std::thread::sleep(Duration::from_millis(100)); + // A read would now report 3 kills, so only the missing signal can stop it. + monitor.changed.store(false, Ordering::Relaxed); + monitor.poll(&config, &mut state, &store); + assert!(notices(&state).is_empty()); + monitor.changed.store(true, Ordering::Relaxed); + monitor.poll(&config, &mut state, &store); + assert_eq!(notices(&state).len(), 1); + } + + #[test] + fn monitor_bounds_notices() { + let directory = TestDirectory::create(); + let (config, store, mut state) = setup(&directory); + let mut monitor = monitor(&directory); + for count in 1..=u64::from(MAX_NOTICES) { + directory.write_kills(count); + let deadline = Instant::now() + Duration::from_secs(10); + while notices(&state).len() < count as usize && Instant::now() < deadline { + monitor.poll(&config, &mut state, &store); + std::thread::sleep(Duration::from_millis(1)); + } + } + assert_eq!(notices(&state).len(), MAX_NOTICES as usize); + directory.write_kills(u64::from(MAX_NOTICES) + 1); + poll_for_notices( + &mut monitor, + &config, + &mut state, + &store, + MAX_NOTICES as usize, + ); + } + + #[test] + fn monitor_needs_a_cgroup_with_memory_accounting() { + let directory = TestDirectory::create(); + assert!(TaskMemoryMonitor::for_cgroup(directory.task_cgroup()).is_none()); + } +} diff --git a/packages/paperclip-runner/runner/crates/runner-core/tests/task_memory.rs b/packages/paperclip-runner/runner/crates/runner-core/tests/task_memory.rs new file mode 100644 index 00000000000..6d7797495c6 --- /dev/null +++ b/packages/paperclip-runner/runner/crates/runner-core/tests/task_memory.rs @@ -0,0 +1,76 @@ +use std::borrow::Cow; +use std::fs; +use std::path::PathBuf; + +use paperclip_runner_core::codex_provider::{CodexProvider, CodexProviderConfig}; +use paperclip_runner_core::task_memory::{ + instructions_with_memory_note, set_task_cgroup, TaskMemoryMonitor, +}; + +// A separate test binary: the flag's process-wide path starts unset here, and this +// one test owns the order of the before and after states. +#[test] +fn only_the_launch_flag_adds_the_note_and_the_monitor() { + assert!(matches!( + instructions_with_memory_note("Do the work.", 4096), + Cow::Borrowed("Do the work.") + )); + assert!(TaskMemoryMonitor::new().is_none()); + + let directory = std::env::temp_dir().join(format!("runner-memory-{}", uuid::Uuid::new_v4())); + fs::create_dir(&directory).unwrap(); + let task_cgroup: PathBuf = directory.join("task-a"); + fs::create_dir(&task_cgroup).unwrap(); + fs::write(directory.join("memory.max"), "6442450944\n").unwrap(); + fs::write(task_cgroup.join("memory.events"), "oom_kill 0\n").unwrap(); + set_task_cgroup(task_cgroup); + + let instructions = instructions_with_memory_note("Do the work.", 4096); + assert!(instructions.starts_with("Do the work.\n\nAll task processes")); + assert!(instructions.contains("6 GiB")); + assert!(TaskMemoryMonitor::new().is_some()); + + let log = directory.join("requests.ndjson"); + let config = CodexProviderConfig { + provider: "codex".to_owned(), + driver: "codex_app_server".to_owned(), + provider_version: "fake-1".to_owned(), + command: PathBuf::from(env!("CARGO_BIN_EXE_fake-codex-app-server")), + args: vec![ + "--state-file".to_owned(), + directory.join("state.json").display().to_string(), + "--call-log".to_owned(), + directory.join("calls.log").display().to_string(), + "--request-log".to_owned(), + log.display().to_string(), + ], + cwd: std::env::current_dir().unwrap().display().to_string(), + model: Some("test-model".to_owned()), + provider_session_id: None, + instructions: "Do the work.".to_owned(), + approval_policy: "never".to_owned(), + externally_sandboxed: false, + read_only: false, + include_skill_instructions: None, + conversation_mode: None, + }; + let mut provider = CodexProvider::start(&config, None).unwrap(); + let thread_id = provider.thread_id().to_owned(); + provider.shutdown().unwrap(); + let mut resumed = CodexProvider::start(&config, Some(&thread_id)).unwrap(); + resumed.shutdown().unwrap(); + let requests = fs::read_to_string(&log).unwrap(); + for method in ["thread/start", "thread/resume"] { + let request: serde_json::Value = requests + .lines() + .map(|line| serde_json::from_str::(line).unwrap()) + .find(|request| request["method"] == method) + .unwrap(); + assert_eq!( + request["params"]["developerInstructions"].as_str().unwrap(), + instructions.as_ref(), + "{method}" + ); + } + fs::remove_dir_all(directory).unwrap(); +} diff --git a/scripts/run-tsc.mjs b/scripts/run-tsc.mjs new file mode 100644 index 00000000000..143c5f341e7 --- /dev/null +++ b/scripts/run-tsc.mjs @@ -0,0 +1,47 @@ +#!/usr/bin/env node +// Runs the calling package's tsc, honoring the repository's low-memory mode. +// +// TypeScript 7 is a native binary, so Node heap flags do not limit it. Each +// checker keeps its own copy of the types, which makes the checker count the +// most useful control a package has over its peak memory. +// scripts/typecheck-low-memory.mjs sets PAPERCLIP_TYPECHECK_LOW_MEMORY=1 so the +// largest programs check on a single thread. +// +// A build that only needs JavaScript sets PAPERCLIP_TSC_EMIT_ONLY=1. That build +// skips type checks and declarations, which use most of the memory of an emit. +// The Docker image uses this mode. +// +// Usage: node ../scripts/run-tsc.mjs [tsc arguments] + +import { spawnSync } from "node:child_process"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +export const LOW_MEMORY_VARIABLE = "PAPERCLIP_TYPECHECK_LOW_MEMORY"; +export const EMIT_ONLY_VARIABLE = "PAPERCLIP_TSC_EMIT_ONLY"; + +export function lowMemoryTscArgs(env = process.env) { + return env[LOW_MEMORY_VARIABLE] === "1" ? ["--singleThreaded"] : []; +} + +export function emitOnlyTscArgs(env = process.env) { + return env[EMIT_ONLY_VARIABLE] === "1" + ? ["--noCheck", "--declaration", "false", "--declarationMap", "false"] + : []; +} + +export function runTsc(args, { cwd = process.cwd(), env = process.env } = {}) { + const tsc = path.join(cwd, "node_modules", "typescript", "bin", "tsc"); + // tsc only accepts -b as its first argument, so the caller's arguments go first. + const result = spawnSync(process.execPath, [tsc, ...args, ...lowMemoryTscArgs(env), ...emitOnlyTscArgs(env)], { + cwd, + env, + stdio: "inherit", + }); + if (result.error) throw result.error; + return { status: result.status ?? 1, signal: result.signal }; +} + +if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + process.exitCode = runTsc(process.argv.slice(2)).status; +} diff --git a/scripts/typecheck-low-memory.mjs b/scripts/typecheck-low-memory.mjs new file mode 100644 index 00000000000..a4c54dce846 --- /dev/null +++ b/scripts/typecheck-low-memory.mjs @@ -0,0 +1,41 @@ +#!/usr/bin/env node +// Runs the workspace typecheck with less memory, but more slowly. It checks one +// package at a time, and scripts/run-tsc.mjs uses one thread for the largest +// programs. See "Typecheck memory" in doc/DEVELOPING.md. +// +// Usage: node scripts/typecheck-low-memory.mjs + +import { spawnSync } from "node:child_process"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { LOW_MEMORY_VARIABLE } from "./run-tsc.mjs"; + +export const STEPS = [ + ["run", "preflight:workspace-links"], + ["-r", "--workspace-concurrency=1", "typecheck"], +]; + +function runPnpm(args, options) { + const result = spawnSync("pnpm", args, { + ...options, + stdio: "inherit", + // A bare `pnpm` on Windows is pnpm.cmd, which needs the shell to resolve. + shell: process.platform === "win32", + }); + if (result.error) throw result.error; + return result.status ?? 1; +} + +export function typecheckLowMemory({ cwd, env = process.env, run = runPnpm }) { + for (const args of STEPS) { + const status = run(args, { cwd, env: { ...env, [LOW_MEMORY_VARIABLE]: "1" } }); + if (status !== 0) return status; + } + return 0; +} + +if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + process.exitCode = typecheckLowMemory({ + cwd: path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."), + }); +} diff --git a/server/package.json b/server/package.json index 0265e98d04e..acf37f6a691 100644 --- a/server/package.json +++ b/server/package.json @@ -36,14 +36,14 @@ "dev": "tsx src/index.ts", "dev:watch": "cross-env PAPERCLIP_MIGRATION_PROMPT=never PAPERCLIP_MIGRATION_AUTO_APPLY=true tsx ./scripts/dev-watch.ts", "prepare:ui-dist": "bash ../scripts/prepare-server-ui-dist.sh", - "build": "pnpm run prepare:runner-vendor && node scripts/verify-runner-vendor-dependencies.mjs && tsc && mkdir -p dist/onboarding-assets dist/built-ins dist/services/scripts dist/vendor/paperclip-runner && cp -R src/onboarding-assets/. dist/onboarding-assets/ && cp -R src/built-ins/. dist/built-ins/ && cp -R src/services/scripts/. dist/services/scripts/ && cp -Rf ../packages/paperclip-runner/dist/. dist/vendor/paperclip-runner/ && cp ../packages/paperclip-runner/scripts/install-copilot-assets.mjs ../packages/paperclip-runner/scripts/materialize-copilot-binary.mjs ../packages/paperclip-runner/scripts/copilot-inner-distribution.mjs dist/ && node dist/install-copilot-assets.mjs && node scripts/write-build-stamp.mjs", + "build": "pnpm run prepare:runner-vendor && node scripts/verify-runner-vendor-dependencies.mjs && node ../scripts/run-tsc.mjs && mkdir -p dist/onboarding-assets dist/built-ins dist/services/scripts dist/vendor/paperclip-runner && cp -R src/onboarding-assets/. dist/onboarding-assets/ && cp -R src/built-ins/. dist/built-ins/ && cp -R src/services/scripts/. dist/services/scripts/ && cp -Rf ../packages/paperclip-runner/dist/. dist/vendor/paperclip-runner/ && cp ../packages/paperclip-runner/scripts/install-copilot-assets.mjs ../packages/paperclip-runner/scripts/materialize-copilot-binary.mjs ../packages/paperclip-runner/scripts/copilot-inner-distribution.mjs dist/ && node dist/install-copilot-assets.mjs && node scripts/write-build-stamp.mjs", "prepack": "pnpm run prepare:ui-dist && pnpm run build", "postpack": "rm -rf ui-dist", "clean": "rm -rf dist", "build:runtime": "node scripts/bundle-runtime.mjs", "start": "node dist/index.js", "prepare:runner-vendor": "node scripts/prepare-runner-vendor.mjs", - "typecheck": "pnpm run prepare:runner-vendor && pnpm --filter @paperclipai/plugin-sdk ensure-build-deps && tsc --noEmit", + "typecheck": "pnpm --filter @paperclipai/paperclip-runner run build:typescript && pnpm --filter @paperclipai/plugin-sdk ensure-build-deps && node scripts/typecheck-layers.mjs", "postinstall": "node scripts/install-copilot-assets.mjs" }, "dependencies": { diff --git a/server/scripts/prepare-runner-vendor.mjs b/server/scripts/prepare-runner-vendor.mjs index d41e86bd149..f774d440f53 100644 --- a/server/scripts/prepare-runner-vendor.mjs +++ b/server/scripts/prepare-runner-vendor.mjs @@ -1,7 +1,7 @@ // Makes sure packages/paperclip-runner/dist is a current build before the // server build vendors it, rebuilding the Runner only when that is needed. // -// The server build (and typecheck) used to run the Runner's full `build` +// The server build used to run the Runner's full `build` // unconditionally here. That is what lets a standalone // `pnpm --filter @paperclipai/server build` produce a current vendored Runner // on its own, which the Dockerfile relies on. But inside `pnpm -r build` the diff --git a/server/scripts/typecheck-layers.mjs b/server/scripts/typecheck-layers.mjs new file mode 100644 index 00000000000..99cfb37bfb6 --- /dev/null +++ b/server/scripts/typecheck-layers.mjs @@ -0,0 +1,257 @@ +// Typechecks the server in layers so that no single tsc process holds the whole +// program in memory. +// +// tsc keeps every type it creates until it exits, so its peak grows with the +// source it checks. The server is about 1,700 source files and 500 test files, +// and one program over all of them needs more than 8 GiB at any setting. Most +// of those files are not in an import cycle, so they can be checked separately +// and read back as declarations: +// +// below files the largest import cycle depends on, plus unrelated files +// cycle the largest import cycle, reading `below` as declarations +// above files that depend on the cycle, reading both as declarations +// tests test files, reading all three as declarations +// +// The layers come from the import graph on every run, so nothing has to move +// and no list can go stale. The import scan does not have to be perfect: if it +// misses an edge, a file imports one outside its layer's references and tsc +// fails with TS6307 instead of skipping the file. +// +// server/tsconfig.json stays the single program that editors and `pnpm build` +// use. +// +// Usage: node scripts/typecheck-layers.mjs + +import { mkdirSync, readdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { runTsc } from "../../scripts/run-tsc.mjs"; + +const IMPORT_SPECIFIER = /(?:from\s+|import\s*\(\s*|import\s+)["'](\.[^"']+)["']/g; +const OUTPUT_DIRECTORY = path.join("node_modules", ".cache", "paperclip-typecheck"); + +const isTestFile = (file) => /\.test\.tsx?$/.test(file) || file.split("/").includes("__tests__"); +// tsconfig.json excludes this tree. A test reaches its helpers through imports. +const isExcludedFromProgram = (file) => file.startsWith("src/__tests__/"); + +export function listSourceFiles(serverRoot) { + const files = []; + const walk = (directory) => { + for (const entry of readdirSync(path.join(serverRoot, directory), { withFileTypes: true })) { + const relative = `${directory}/${entry.name}`; + if (entry.isDirectory()) walk(relative); + else if (/\.tsx?$/.test(entry.name)) files.push(relative); + } + }; + walk("src"); + return files.sort(); +} + +/** Resolves the relative imports of each file. `files` maps a path to its text. */ +export function readImports(files) { + const imports = new Map(); + for (const [file, text] of files) { + const modules = new Set(); + const json = new Set(); + for (const [, specifier] of text.matchAll(IMPORT_SPECIFIER)) { + const target = path.posix.join(path.posix.dirname(file), specifier); + if (target.endsWith(".json")) { + json.add(target); + continue; + } + const candidates = [ + target, + target.replace(/\.js$/, ".ts"), + target.replace(/\.js$/, ".tsx"), + `${target}.ts`, + `${target}/index.ts`, + ]; + const resolved = candidates.find((candidate) => files.has(candidate)); + if (resolved) modules.add(resolved); + } + imports.set(file, { modules, json }); + } + return imports; +} + +/** Tarjan's algorithm, iterative because import chains can be very deep. */ +function largestCycle(graph) { + const index = new Map(); + const low = new Map(); + const onStack = new Set(); + const stack = []; + let next = 0; + let largest = []; + + for (const root of graph.keys()) { + if (index.has(root)) continue; + const work = [{ node: root, edges: [...graph.get(root)], position: 0 }]; + index.set(root, next); + low.set(root, next); + next += 1; + stack.push(root); + onStack.add(root); + while (work.length > 0) { + const frame = work[work.length - 1]; + if (frame.position < frame.edges.length) { + const edge = frame.edges[frame.position]; + frame.position += 1; + if (!index.has(edge)) { + index.set(edge, next); + low.set(edge, next); + next += 1; + stack.push(edge); + onStack.add(edge); + work.push({ node: edge, edges: [...graph.get(edge)], position: 0 }); + } else if (onStack.has(edge)) { + low.set(frame.node, Math.min(low.get(frame.node), index.get(edge))); + } + continue; + } + work.pop(); + const parent = work[work.length - 1]; + if (parent) low.set(parent.node, Math.min(low.get(parent.node), low.get(frame.node))); + if (low.get(frame.node) === index.get(frame.node)) { + const component = []; + let member; + do { + member = stack.pop(); + onStack.delete(member); + component.push(member); + } while (member !== frame.node); + if (component.length > largest.length) largest = component; + } + } + } + return largest.length > 1 ? new Set(largest) : new Set(); +} + +function reachableFrom(start, graph) { + const seen = new Set(start); + const pending = [...start]; + while (pending.length > 0) { + for (const edge of graph.get(pending.pop()) ?? []) { + if (!seen.has(edge)) { + seen.add(edge); + pending.push(edge); + } + } + } + return seen; +} + +/** + * Sorts every file into the layers, in check order. `files` maps a path to its + * text. Declaration files are ambient, so every layer lists them. + */ +export function planLayers(files) { + const declarations = [...files.keys()].filter((file) => file.endsWith(".d.ts")); + const candidates = new Map([...files].filter(([file]) => !file.endsWith(".d.ts"))); + const imports = readImports(candidates); + + const graph = new Map(); + const reversed = new Map(); + for (const file of candidates.keys()) { + if (!isTestFile(file)) { + graph.set(file, new Set()); + reversed.set(file, new Set()); + } + } + for (const file of graph.keys()) { + for (const target of imports.get(file).modules) { + if (graph.has(target)) { + graph.get(file).add(target); + reversed.get(target).add(file); + } + } + } + + const cycle = largestCycle(graph); + const dependents = reachableFrom(cycle, reversed); + const above = [...dependents].filter((file) => !cycle.has(file)); + const below = [...graph.keys()].filter((file) => !dependents.has(file)); + const tests = [...candidates.keys()].filter((file) => isTestFile(file) && !isExcludedFromProgram(file)); + + const layer = (name, members, references) => { + const sorted = [...members].sort(); + const json = new Set(sorted.flatMap((file) => [...imports.get(file).json])); + return { name, sourceFiles: sorted, files: [...sorted, ...[...json].sort(), ...declarations], references }; + }; + return [ + layer("below", below, []), + layer("cycle", cycle, ["below"]), + layer("above", above, ["below", "cycle"]), + layer("tests", tests, ["below", "cycle", "above"]), + ].filter((candidate) => candidate.sourceFiles.length > 0); +} + +/** Builds the tsconfig for one layer. Paths are relative to the output directory. */ +export function layerConfig(layer, { emptyLayers = new Set() } = {}) { + const fromOutput = (file) => path.posix.join("../../..", file); + const references = layer.references + .filter((name) => !emptyLayers.has(name)) + .map((name) => ({ path: `./tsconfig.${name}.json` })); + const output = + layer.name === "tests" + ? { composite: false, declaration: false, declarationMap: false, sourceMap: false, noEmit: true } + : { + composite: true, + declaration: true, + declarationMap: false, + sourceMap: false, + emitDeclarationOnly: true, + outDir: "./dts", + tsBuildInfoFile: `./${layer.name}.tsbuildinfo`, + }; + return { + extends: fromOutput("tsconfig.json"), + compilerOptions: { ...output, rootDir: fromOutput("src") }, + files: layer.files.map(fromOutput), + include: [], + references, + }; +} + +export function typecheckLayers({ + serverRoot, + run = runTsc, + log = console.log, +}) { + const sourceFiles = listSourceFiles(serverRoot); + const layers = planLayers( + new Map(sourceFiles.map((file) => [file, readFileSync(path.join(serverRoot, file), "utf8")])), + ); + const planned = new Set(layers.map((layer) => layer.name)); + const emptyLayers = new Set(["below", "cycle", "above", "tests"].filter((name) => !planned.has(name))); + + const outputRoot = path.join(serverRoot, OUTPUT_DIRECTORY); + rmSync(outputRoot, { recursive: true, force: true }); + mkdirSync(outputRoot, { recursive: true }); + // Without this, the path under node_modules would make the declarations CommonJS. + writeFileSync(path.join(outputRoot, "package.json"), `${JSON.stringify({ type: "module" })}\n`); + for (const layer of layers) { + writeFileSync( + path.join(outputRoot, `tsconfig.${layer.name}.json`), + `${JSON.stringify(layerConfig(layer, { emptyLayers }), null, 2)}\n`, + ); + } + + let exitCode = 0; + for (const layer of layers) { + log(`typecheck:layers: ${layer.name} (${layer.sourceFiles.length} files)`); + const config = path.join(OUTPUT_DIRECTORY, `tsconfig.${layer.name}.json`); + const { status, signal } = run(["-p", config], { cwd: serverRoot }); + // A killed process leaves no declarations, so later layers would only add noise. + if (signal) return status || 1; + // Later layers still run after type errors. Each file is checked in one + // layer, so this reports everything a single tsc run would. + if (status !== 0) exitCode = status; + } + return exitCode; +} + +if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + process.exitCode = typecheckLayers({ + serverRoot: path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."), + }); +} diff --git a/server/scripts/typecheck-layers.test.mjs b/server/scripts/typecheck-layers.test.mjs new file mode 100644 index 00000000000..eec3f2171e8 --- /dev/null +++ b/server/scripts/typecheck-layers.test.mjs @@ -0,0 +1,471 @@ +import { spawnSync } from "node:child_process"; +import { existsSync, mkdirSync, mkdtempSync, readFileSync, realpathSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +import { afterEach, describe, expect, it } from "vitest"; + +import { emitOnlyTscArgs, lowMemoryTscArgs, runTsc } from "../../scripts/run-tsc.mjs"; +import { STEPS, typecheckLowMemory } from "../../scripts/typecheck-low-memory.mjs"; +import { layerConfig, listSourceFiles, planLayers, readImports, typecheckLayers } from "./typecheck-layers.mjs"; + +const serverRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); +const sourcesOf = (layers, name) => layers.find((layer) => layer.name === name)?.sourceFiles; + +describe("readImports", () => { + it("resolves every import form that TypeScript follows", () => { + const files = new Map([ + [ + "src/a.ts", + [ + `import { b } from "./b.js";`, + `export * from "./c.js";`, + `import "./d.js";`, + `const e = await import("./e.js");`, + `type F = typeof import("./f.js");`, + `import g from "./g/index.js";`, + `import h from "./h.js";`, + `import data from "./data.json" with { type: "json" };`, + `import { z } from "zod";`, + ].join("\n"), + ], + ["src/b.ts", ""], + ["src/c.ts", ""], + ["src/d.ts", ""], + ["src/e.ts", ""], + ["src/f.ts", ""], + ["src/g/index.ts", ""], + ["src/h.tsx", ""], + ]); + + const { modules, json } = readImports(files).get("src/a.ts"); + + expect([...modules].sort()).toEqual([ + "src/b.ts", + "src/c.ts", + "src/d.ts", + "src/e.ts", + "src/f.ts", + "src/g/index.ts", + "src/h.tsx", + ]); + expect([...json]).toEqual(["src/data.json"]); + }); +}); + +describe("planLayers", () => { + it("separates the largest cycle from what it depends on and what depends on it", () => { + const layers = planLayers( + new Map([ + ["src/leaf.ts", ""], + ["src/unrelated.ts", ""], + ["src/one.ts", `import "./two.js"; import "./leaf.js";`], + ["src/two.ts", `import "./one.js";`], + ["src/route.ts", `import "./one.js";`], + ["src/route.test.ts", `import "./route.js";`], + ["src/types/ambient.d.ts", ""], + ]), + ); + + expect(sourcesOf(layers, "below")).toEqual(["src/leaf.ts", "src/unrelated.ts"]); + expect(sourcesOf(layers, "cycle")).toEqual(["src/one.ts", "src/two.ts"]); + expect(sourcesOf(layers, "above")).toEqual(["src/route.ts"]); + expect(sourcesOf(layers, "tests")).toEqual(["src/route.test.ts"]); + for (const layer of layers) expect(layer.files).toContain("src/types/ambient.d.ts"); + }); + + it("keeps test support files in the test layer and leaves src/__tests__ to imports", () => { + const layers = planLayers( + new Map([ + ["src/a.ts", ""], + ["src/services/__tests__/fixtures.ts", ""], + ["src/__tests__/helpers/db.ts", ""], + ["src/__tests__/big.test.ts", ""], + ]), + ); + + expect(sourcesOf(layers, "below")).toEqual(["src/a.ts"]); + expect(sourcesOf(layers, "tests")).toEqual(["src/services/__tests__/fixtures.ts"]); + }); + + it("lists imported JSON in the layer that imports it", () => { + const layers = planLayers( + new Map([ + ["src/a.ts", `import data from "./data.json";`], + ["src/one.ts", `import "./two.js"; import data from "./cycle.json";`], + ["src/two.ts", `import "./one.js";`], + ]), + ); + + expect(layers.find((layer) => layer.name === "below").files).toContain("src/data.json"); + expect(layers.find((layer) => layer.name === "cycle").files).toContain("src/cycle.json"); + }); + + it("omits layers with no files, as when the source has no cycle", () => { + const layers = planLayers(new Map([["src/a.ts", ""], ["src/b.ts", `import "./a.js";`]])); + + expect(layers.map((layer) => layer.name)).toEqual(["below"]); + expect(sourcesOf(layers, "below")).toEqual(["src/a.ts", "src/b.ts"]); + }); + + it("covers the real server: one layer per file, and no import reaches a later layer", () => { + const sourceFiles = listSourceFiles(serverRoot); + const files = new Map( + sourceFiles.map((file) => [file, readFileSync(path.join(serverRoot, file), "utf8")]), + ); + const layers = planLayers(files); + + const planned = layers.flatMap((layer) => layer.sourceFiles); + expect(new Set(planned).size).toBe(planned.length); + expect(planned.sort()).toEqual( + sourceFiles.filter((file) => !file.endsWith(".d.ts") && !file.startsWith("src/__tests__/")).sort(), + ); + + const position = new Map( + layers.flatMap((layer, index) => layer.sourceFiles.map((file) => [file, index])), + ); + const imports = readImports(new Map([...files].filter(([file]) => !file.endsWith(".d.ts")))); + for (const layer of layers.filter((candidate) => candidate.name !== "tests")) { + for (const file of layer.sourceFiles) { + for (const target of imports.get(file).modules) { + if (position.has(target)) expect(position.get(target)).toBeLessThanOrEqual(position.get(file)); + } + } + } + }); +}); + +describe("layerConfig", () => { + const layer = { name: "cycle", files: ["src/a.ts"], references: ["below"] }; + + it("emits declarations for source layers and references earlier ones", () => { + const config = layerConfig(layer); + + expect(config.compilerOptions).toMatchObject({ composite: true, emitDeclarationOnly: true, outDir: "./dts" }); + expect(config.extends).toBe("../../../tsconfig.json"); + expect(config.files).toEqual(["../../../src/a.ts"]); + expect(config.references).toEqual([{ path: "./tsconfig.below.json" }]); + }); + + it("emits nothing for tests and drops references to empty layers", () => { + const config = layerConfig( + { name: "tests", files: [], references: ["below", "cycle"] }, + { emptyLayers: new Set(["cycle"]) }, + ); + + expect(config.compilerOptions).toMatchObject({ composite: false, noEmit: true }); + expect(config.references).toEqual([{ path: "./tsconfig.below.json" }]); + }); +}); + +describe("typecheckLayers", () => { + const roots = []; + + afterEach(() => { + for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); + }); + + function serverFixture() { + const root = mkdtempSync(path.join(tmpdir(), "typecheck-layers-")); + roots.push(root); + mkdirSync(path.join(root, "src")); + writeFileSync(path.join(root, "src", "leaf.ts"), ""); + writeFileSync(path.join(root, "src", "one.ts"), `import "./two.js"; import "./leaf.js";`); + writeFileSync(path.join(root, "src", "two.ts"), `import "./one.js";`); + writeFileSync(path.join(root, "src", "route.ts"), `import "./one.js";`); + writeFileSync(path.join(root, "src", "route.test.ts"), `import "./route.js";`); + return root; + } + + it("runs the layers in order and marks the output directory as ESM", () => { + const root = serverFixture(); + const configs = []; + + const status = typecheckLayers({ + serverRoot: root, + log: () => {}, + run: (args) => { + configs.push(path.basename(args[1])); + return { status: 0, signal: null }; + }, + }); + + expect(status).toBe(0); + expect(configs).toEqual([ + "tsconfig.below.json", + "tsconfig.cycle.json", + "tsconfig.above.json", + "tsconfig.tests.json", + ]); + const output = path.join(root, "node_modules", ".cache", "paperclip-typecheck"); + expect(JSON.parse(readFileSync(path.join(output, "package.json"), "utf8"))).toEqual({ type: "module" }); + }); + + it("reports type errors from every layer", () => { + const root = serverFixture(); + const configs = []; + + const status = typecheckLayers({ + serverRoot: root, + log: () => {}, + run: (args) => { + configs.push(args[1]); + return { status: configs.length === 1 ? 2 : 0, signal: null }; + }, + }); + + expect(status).toBe(2); + expect(configs).toHaveLength(4); + }); + + it("stops when a process is killed, because later layers have no declarations", () => { + const root = serverFixture(); + const configs = []; + + const status = typecheckLayers({ + serverRoot: root, + log: () => {}, + run: (args) => { + configs.push(args[1]); + return { status: 1, signal: "SIGKILL" }; + }, + }); + + expect(status).toBe(1); + expect(configs).toHaveLength(1); + }); +}); + +describe("typecheckLayers with the real compiler", () => { + const tsc = path.join(serverRoot, "node_modules", "typescript", "bin", "tsc"); + const roots = []; + + afterEach(() => { + for (const root of roots.splice(0)) rmSync(root, { recursive: true, force: true }); + }); + + function compilerFixture() { + const root = mkdtempSync(path.join(tmpdir(), "typecheck-layers-real-")); + roots.push(root); + mkdirSync(path.join(root, "src")); + writeFileSync(path.join(root, "package.json"), `${JSON.stringify({ type: "module" })}\n`); + writeFileSync( + path.join(root, "tsconfig.json"), + `${JSON.stringify({ + compilerOptions: { + target: "ES2023", + module: "NodeNext", + moduleResolution: "NodeNext", + strict: true, + types: [], + outDir: "dist", + rootDir: "src", + resolveJsonModule: true, + skipLibCheck: true, + }, + include: ["src"], + })}\n`, + ); + const sources = { + "leaf.ts": `export const leaf = 1;\n`, + "one.ts": `import { two } from "./two.js";\nimport { leaf } from "./leaf.js";\nexport const one = () => leaf + two();\n`, + "two.ts": `import { one } from "./one.js";\nexport const two = (): number => one.length;\n`, + "route.ts": `import { one } from "./one.js";\nexport const route = () => one();\n`, + "route.test.ts": `import { route } from "./route.js";\nexport const checked: number = route();\n`, + }; + for (const [name, text] of Object.entries(sources)) { + writeFileSync(path.join(root, "src", name), text); + } + return root; + } + + function check(root) { + const output = []; + const status = typecheckLayers({ + serverRoot: root, + log: () => {}, + run: (args, { cwd }) => { + const result = spawnSync(process.execPath, [tsc, ...args], { cwd, encoding: "utf8" }); + output.push(result.stdout, result.stderr); + return { status: result.status ?? 1, signal: result.signal }; + }, + }); + return { status, output: output.join("") }; + } + + it("emits declarations between layers and checks every layer", () => { + const root = compilerFixture(); + + const { status, output } = check(root); + + expect(output).toBe(""); + expect(status).toBe(0); + const declarations = path.join(root, "node_modules", ".cache", "paperclip-typecheck", "dts"); + for (const name of ["leaf.d.ts", "one.d.ts", "two.d.ts", "route.d.ts"]) { + expect(existsSync(path.join(declarations, name))).toBe(true); + } + expect(existsSync(path.join(declarations, "route.test.d.ts"))).toBe(false); + }, 60_000); + + it("reports a changed export in the layers that depend on it", () => { + const root = compilerFixture(); + writeFileSync(path.join(root, "src", "leaf.ts"), `export const renamedLeaf = 1;\n`); + + const { status, output } = check(root); + + expect(status).not.toBe(0); + expect(output).toMatch(/src\/one\.ts\(\d+,\d+\): error TS\d+/); + expect(output).toContain("leaf"); + }, 60_000); + + it("reports a type change through declarations in the test layer", () => { + const root = compilerFixture(); + writeFileSync( + path.join(root, "src", "route.ts"), + `import { one } from "./one.js";\nexport const route = (): string => String(one());\n`, + ); + + const { status, output } = check(root); + + expect(status).not.toBe(0); + expect(output).toMatch(/src\/route\.test\.ts\(\d+,\d+\): error TS2322/); + }, 60_000); + + // Emitted declarations keep imports of .d.mts and JSON files, which tsc does not + // copy to the output directory. If those imports stopped resolving, skipLibCheck + // would hide it and the types would become `any`, so these errors would disappear. + it("keeps types from .d.mts and JSON imports in later layers", () => { + const root = compilerFixture(); + mkdirSync(path.join(root, "src", "scripts")); + writeFileSync(path.join(root, "src", "scripts", "helper.mjs"), `export {};\n`); + writeFileSync(path.join(root, "src", "scripts", "helper.d.mts"), `export interface Manifest { version: number }\n`); + writeFileSync(path.join(root, "src", "catalog.json"), `${JSON.stringify({ tools: [{ name: "a" }] })}\n`); + writeFileSync( + path.join(root, "src", "leaf.ts"), + [ + `import catalog from "./catalog.json" with { type: "json" };`, + `import type { Manifest } from "./scripts/helper.mjs";`, + `export type { Manifest };`, + `export type Tool = (typeof catalog.tools)[number];`, + `export const leaf = 1;`, + ``, + ].join("\n"), + ); + const misuse = [ + `import type { Manifest, Tool } from "./leaf.js";`, + `export const manifest: Manifest = { wrong: 1 };`, + `export const tool: Tool = { wrong: 1 };`, + ``, + ].join("\n"); + writeFileSync(path.join(root, "src", "cycle-misuse.ts"), `import { one } from "./one.js";\nexport const used = one;\n${misuse}`); + writeFileSync( + path.join(root, "src", "two.ts"), + `import { one } from "./one.js";\nimport { used } from "./cycle-misuse.js";\nexport const two = (): number => one.length + used.length;\n`, + ); + writeFileSync(path.join(root, "src", "misuse.test.ts"), misuse); + + const { status, output } = check(root); + + expect(status).not.toBe(0); + for (const file of ["cycle-misuse", "misuse\\.test"]) { + expect(output).toMatch(new RegExp(`src/${file}\\.ts\\(\\d+,\\d+\\): error TS2353.*'Manifest'`)); + expect(output).toMatch(new RegExp(`src/${file}\\.ts\\(\\d+,\\d+\\): error TS2353.*'\\{ name: string; \\}'`)); + } + }, 60_000); +}); + +describe("typecheckLowMemory", () => { + it("runs every step in low-memory mode and stops at a failure", () => { + const calls = []; + const run = (args, { env }) => { + calls.push([args, env.PAPERCLIP_TYPECHECK_LOW_MEMORY]); + return calls.length === 1 ? 0 : 3; + }; + + expect(typecheckLowMemory({ cwd: serverRoot, env: {}, run })).toBe(3); + expect(calls).toEqual(STEPS.map((args) => [args, "1"])); + }); + + it("does not run the typecheck after a failed preflight", () => { + const calls = []; + + const status = typecheckLowMemory({ + cwd: serverRoot, + env: {}, + run: (args) => { + calls.push(args); + return 1; + }, + }); + + expect(status).toBe(1); + expect(calls).toHaveLength(1); + }); +}); + +describe("runTsc", () => { + it("emits JavaScript without checks or declarations in emit-only mode", () => { + const root = mkdtempSync(path.join(tmpdir(), "run-tsc-emit-only-")); + try { + mkdirSync(path.join(root, "src")); + mkdirSync(path.join(root, "node_modules")); + symlinkSync( + realpathSync(path.join(serverRoot, "node_modules", "typescript")), + path.join(root, "node_modules", "typescript"), + ); + writeFileSync( + path.join(root, "tsconfig.json"), + `${JSON.stringify({ + compilerOptions: { declaration: true, declarationMap: true, types: [], outDir: "dist", rootDir: "src" }, + include: ["src"], + })}\n`, + ); + writeFileSync(path.join(root, "src", "a.ts"), `export const value: number = "not a number";\n`); + + const { status } = runTsc([], { cwd: root, env: { ...process.env, PAPERCLIP_TSC_EMIT_ONLY: "1" } }); + + expect(status).toBe(0); + expect(existsSync(path.join(root, "dist", "a.js"))).toBe(true); + expect(existsSync(path.join(root, "dist", "a.d.ts"))).toBe(false); + } finally { + rmSync(root, { recursive: true, force: true }); + } + }, 60_000); + + it("accepts build mode with low-memory mode on", () => { + const { status } = runTsc(["-b", "--dry"], { + cwd: serverRoot, + env: { ...process.env, PAPERCLIP_TYPECHECK_LOW_MEMORY: "1" }, + }); + + expect(status).toBe(0); + }); +}); + +describe("emitOnlyTscArgs", () => { + it("removes checks and declarations only in emit-only mode", () => { + expect(emitOnlyTscArgs({ PAPERCLIP_TSC_EMIT_ONLY: "1" })).toEqual([ + "--noCheck", + "--declaration", + "false", + "--declarationMap", + "false", + ]); + expect(emitOnlyTscArgs({})).toEqual([]); + }); + + it("is what the Dockerfile selects for the server build", () => { + const dockerfile = readFileSync(path.join(serverRoot, "..", "Dockerfile"), "utf8"); + + expect(dockerfile).toContain("RUN PAPERCLIP_TSC_EMIT_ONLY=1 pnpm --filter @paperclipai/server build\n"); + }); +}); + +describe("lowMemoryTscArgs", () => { + it("uses one thread only when low-memory mode is on", () => { + expect(lowMemoryTscArgs({ PAPERCLIP_TYPECHECK_LOW_MEMORY: "1" })).toEqual(["--singleThreaded"]); + expect(lowMemoryTscArgs({ PAPERCLIP_TYPECHECK_LOW_MEMORY: "0" })).toEqual([]); + expect(lowMemoryTscArgs({})).toEqual([]); + }); +}); diff --git a/ui/package.json b/ui/package.json index e59f87e405d..28f185fa2e6 100644 --- a/ui/package.json +++ b/ui/package.json @@ -20,7 +20,7 @@ "storybook": "storybook dev -p 6006 -c storybook/.storybook", "build-storybook": "storybook build -c storybook/.storybook -o storybook-static", "preview": "vite preview", - "typecheck": "node ../scripts/storybook-catalog.mjs && tsc -b", + "typecheck": "node ../scripts/storybook-catalog.mjs && node ../scripts/run-tsc.mjs -b", "clean": "rm -rf dist dist-preview storybook-static tsconfig.tsbuildinfo", "prepack": "rm -f package.dev.json && cp package.json package.dev.json && node ../scripts/generate-ui-package-json.mjs", "postpack": "if [ -f package.dev.json ]; then mv package.dev.json package.json; fi"