diff --git a/.github/workflows/copilot-setup-steps.yml b/.github/workflows/copilot-setup-steps.yml index eff490e16..de849fae5 100644 --- a/.github/workflows/copilot-setup-steps.yml +++ b/.github/workflows/copilot-setup-steps.yml @@ -29,9 +29,12 @@ jobs: - name: Checkout code uses: actions/checkout@v5 + # Pre-release, to match CI: `run_moonbit`'s policy states its spawn + # allowlist with `process.allow`, which stable's moonrun rejects outright, + # so an agent working here on stable would find every snippet refused. - name: Set up MoonBit run: | - curl -fsSL https://cli.moonbitlang.com/install/unix.sh | bash + curl -fsSL https://cli.moonbitlang.com/install/unix.sh | bash -s pre-release echo "$HOME/.moon/bin" >> $GITHUB_PATH - name: Update MoonBit dependencies diff --git a/.github/workflows/desktop-release.yml b/.github/workflows/desktop-release.yml index 44cc7e816..32fc59a55 100644 --- a/.github/workflows/desktop-release.yml +++ b/.github/workflows/desktop-release.yml @@ -159,9 +159,14 @@ jobs: expected="version = \"$RELEASE_VERSION\"" grep -Fx "$expected" desktop/moon.mod + # Pre-release, matching CI. This is the toolchain that BUILDS the app, not + # the seed it ships (that one is pinned by `desktop/.moonbit-version`), but + # if the two channels differed then a change compiling on the channel CI + # runs could still break the release build, and nothing would catch it + # until a release ran. - name: Set up MoonBit run: | - curl -fsSL https://cli.moonbitlang.com/install/unix.sh | bash + curl -fsSL https://cli.moonbitlang.com/install/unix.sh | bash -s pre-release echo "$HOME/.moon/bin" >> "$GITHUB_PATH" - name: Show MoonBit version diff --git a/README.mbt.md b/README.mbt.md index 73d3afc27..23764e614 100644 --- a/README.mbt.md +++ b/README.mbt.md @@ -130,9 +130,12 @@ The `cmd/openseek` package is the single-binary entry point — a subcommand tre (default: the terminal UI; `run`/`serve`/`review`/`sessions` for the headless engine; `mcp` to validate MCP configuration). `openseek run` parses arguments and runs the agent package. The agent sends DeepSeek native function tools and -supports twelve local tools: `shell` (with `shell_output` and `shell_stop` for -background jobs — on Windows the plain foreground shell only), `read`, `edit`, -`multi_edit`, `write`, `remove`, `plan`, `goal`, `run_moonbit`, and `finish`. +supports eleven local tools: `run_moonbit` — both the scripting surface and the +command runner, spawning processes through the shell-free +[`bobzhang/myshell`](https://mooncakes.io/docs/bobzhang/myshell) EDSL, with +`job_output` and `job_stop` watching anything it detaches as a background job — +plus `read`, `edit`, `multi_edit`, `write`, `remove`, `plan`, `goal`, and +`finish`. There is no shell tool, so no command ever goes through a shell. ```bash export DEEPSEEK=sk-... diff --git a/agent/README.mbt.md b/agent/README.mbt.md index 361d15396..8451977cb 100644 --- a/agent/README.mbt.md +++ b/agent/README.mbt.md @@ -103,10 +103,11 @@ calls `@agent.run`, but that decision lives outside the `agent` package. `build_tools(runtime, scope)` returns the standard local tool registry: -- `shell`: run a command under the workspace root or an explicit cwd (including - `moon check` for compiler feedback), with `run_in_background` support; -- `shell_output` / `shell_stop`: read or stop a background shell job (omitted on - Windows, where background jobs are not wired); +- `run_moonbit`: compile and run a self-contained MoonBit program — both the + scripting surface (transform files, parse JSON, compute, probe the language) + and the command runner, since processes are spawned from the program through + the shell-free `bobzhang/myshell` EDSL. Supports `run_in_background`; +- `job_output` / `job_stop`: read or stop a background job; - `read`: read a text file; - `edit`: replace exact text in a file; - `multi_edit`: apply several explicit line-anchored replacements to one file; @@ -115,16 +116,16 @@ calls `@agent.run`, but that decision lives outside the `agent` package. - `plan`: record or replace the step-by-step plan for a multi-step task; - `goal`: report standing-goal status (`met`, `continuing`, or `blocked`; `met` clears the goal — setting one is the serve `goal` command's job); -- `run_moonbit`: compile and run a self-contained MoonBit program in an - isolated package (automation and language probes; standard batteries only, - no local packages); - `finish`: end the task with a final answer. +There is no shell tool: every command the agent runs is an argument vector +handed to `@myshell.Cmd`, so no command text is ever parsed by a shell. + File-oriented tools capture `runtime.workspace_root()` when the registry is built. The registry also receives the runtime and task scope for stateful -tools: the shell tools use both — background-job completion notices are pushed -through `runtime.queue_steer`, and the background-job runtime plus its spill-dir -cleanup are owned by the task scope's group. +tools: the background-job path uses both — completion notices are pushed +through `runtime.queue_steer`, and the job runtime plus its spill-dir cleanup +are owned by the task scope's group. ```mbt check ///| @@ -139,9 +140,6 @@ async test "standard tools are registered in dispatch order" { ], content=( #|[ - #| "shell", - #| "shell_output", - #| "shell_stop", #| "read", #| "edit", #| "multi_edit", @@ -150,6 +148,8 @@ async test "standard tools are registered in dispatch order" { #| "plan", #| "goal", #| "run_moonbit", + #| "job_output", + #| "job_stop", #| "finish", #|] ), @@ -283,9 +283,9 @@ answer, `finish`, `abort`, cancellation, unexpected failure, or exhausted ## Operational Notes -This package is intended for trusted local automation. The standard `shell` -tool can run arbitrary commands, while `edit` and `write` can modify files -visible to the process. Use the CLI package for application-level policy, +This package is intended for trusted local automation. `run_moonbit` can run +arbitrary commands (its snippets spawn processes), while `edit` and `write` can +modify files visible to the process. Use the CLI package for application-level policy, session storage, logging configuration, and serve-mode wire handling. Run the package tests with: @@ -312,11 +312,12 @@ improving: logs through async logging so piped runs such as `2>&1 | tee run.log` receive step output promptly. - Current MoonBit projects use `moon.mod`; `moon.mod.json` is legacy. Manifest - or package-import edits should be followed quickly by a shell `moon check` or - another explicit shell validation command. -- Use `shell` for exact end-to-end MoonBit command validation beyond compiler - feedback, especially `moon test`, `moon run`, `moon info`, `moon fmt`, and - README command checks. + or package-import edits should be followed quickly by a `moon check` or + another explicit validation command. +- Use `run_moonbit` for exact end-to-end MoonBit command validation beyond + compiler feedback, especially `moon test`, `moon run`, and README command + checks. Source-writing commands (`moon fmt`, `moon info`) are denied by the + snippet sandbox and belong to the caller, not the agent. - For snapshot updates, run plain `moon test` first and only run `moon test --update` after deciding the failure is a stale snapshot or intentional output change, not a behavior bug. diff --git a/agent/moon.pkg b/agent/moon.pkg index 6bb0b360f..61ec24d67 100644 --- a/agent/moon.pkg +++ b/agent/moon.pkg @@ -5,8 +5,8 @@ import { "bobzhang/openseek/agent_runtime", "bobzhang/openseek/agent_tool", "bobzhang/openseek/agent_tool/bgjobs", - "bobzhang/openseek/agent_tool/shell_output", - "bobzhang/openseek/agent_tool/shell_stop", + "bobzhang/openseek/agent_tool/job_output", + "bobzhang/openseek/agent_tool/job_stop", "bobzhang/openseek/agent_tool/edit", "bobzhang/openseek/agent_tool/finish", "bobzhang/openseek/agent_tool/goal", @@ -15,7 +15,6 @@ import { "bobzhang/openseek/agent_tool/run_moonbit", "bobzhang/openseek/agent_tool/read", "bobzhang/openseek/agent_tool/remove", - "bobzhang/openseek/agent_tool/shell", "bobzhang/openseek/agent_tool/write", "bobzhang/openseek/deepseek", "bobzhang/openseek/deepseek/client", diff --git a/agent/steer_test.mbt b/agent/steer_test.mbt index 69f2a0e30..3506a4e76 100644 --- a/agent/steer_test.mbt +++ b/agent/steer_test.mbt @@ -16,7 +16,7 @@ async fn steer_test_server(group : @async.TaskGroup[Unit]) -> String? { assert_true(request_body.contains("also rename the file")) // The prebuilt registry reached the request: the standard tools ride // along even though the caller, not the turn, constructed them. - assert_true(request_body.contains("\"name\":\"shell\"")) + assert_true(request_body.contains("\"name\":\"run_moonbit\"")) conn.send_response(200, "OK", extra_headers={ "Content-Type": "text/event-stream", }) diff --git a/agent/tool_definition.mbt b/agent/tool_definition.mbt index 62b641517..f16cae78d 100644 --- a/agent/tool_definition.mbt +++ b/agent/tool_definition.mbt @@ -1,7 +1,6 @@ ///| /// The completion notice pushed to the model when a background job exits: its -/// id, terminal status, command, and how to read its output. -#cfg(not(platform="windows")) +/// id, terminal status, and how to read its output. fn background_notice_text(snapshot : @bgjobs.BgJobSnapshot) -> String { let status = match snapshot.status { Exited(0) => "exit=0" @@ -16,98 +15,20 @@ fn background_notice_text(snapshot : @bgjobs.BgJobSnapshot) -> String { } Running => "running" } - "background job \{snapshot.id} finished (\{status}): `\{snapshot.command}` — read its output with shell_output (job_id=\"\{snapshot.id}\")." -} - -///| -/// The shell tool family for the standard registry. -/// -/// Off Windows: the shell tool wired to a session `BgJobRuntime` (background -/// jobs, detach-on-timeout, push-completion notices) plus `shell_output` and -/// `shell_stop`. The runtime spills large job output into a session temp dir -/// (removed at scope teardown) and pushes a `Notice` steer + the -/// `on_background_wake` poke when a job exits on its own. -#cfg(not(platform="windows")) -async fn[X] shell_tools( - runtime : @agent_runtime.AgentRuntime, - scope : @agent_runtime.AgentTaskScope[X], - on_background_wake? : () -> Unit, -) -> Array[@agent_tool.AgentToolDefinition] { - // One background-job runtime per session, owned by `scope.group()`, shared by - // the shell tools so a job started via `run_in_background` (or a command moved - // to the background on timeout) is visible to shell_output / shell_stop. A - // session temp dir makes large background-job output file-backed (full output - // on disk via the sink); a tmpdir failure degrades to memory-only jobs. - let spill_dir : String? = Some(@fs.tmpdir(prefix="openseek-shell-")) catch { - _ => None - } - // Remove the session spill dir (and every job's spill file in it) when the - // owning scope ends, so `/tmp/openseek-shell-*` cannot accumulate across turns - // or sessions. A task parks on a never-fed queue until group teardown cancels - // it, then removes the dir shielded from that cancellation (rmdir is async, so - // it cannot run in a `defer`). - if spill_dir is Some(dir) { - scope.group().spawn_bg(no_wait=true, allow_failure=true) <| () => { - let parked : @async.Queue[Unit] = Queue(kind=Unbounded) - try parked.get() |> ignore catch { - _ => - @async.protect_from_cancel(() => { - @fs.rmdir(dir, recursive=true) catch { - _ => () - } - }) - } - } - } - // When a background job exits on its own, push a completion notice into the - // loop's lossless steer queue (surfaced to the model at the next step, or - // persisted while idle) and poke the controller so an idle engine wakes to - // drain it instead of leaving it until the next prompt. - let bg_runtime = @bgjobs.BgJobRuntime::new(scope, spill_dir?, on_job_exit=snapshot => { - runtime.queue_steer(Notice(background_notice_text(snapshot))) - if on_background_wake is Some(wake) { - wake() - } - }) - [ - @shell.definition(workspace_root=runtime.workspace_root(), bg_runtime~), - @shell_output.definition(bg_runtime), - @shell_stop.definition(bg_runtime), - ] -} - -///| -/// Windows graceful fallback: background jobs are unverified there (no Windows -/// CI, and detached long-lived processes / hard-cancel semantics have never been -/// exercised), so the registry keeps the pre-series shell — the per-call -/// collection path, where a timed-out command is cancelled and reported as a -/// tool error. With no runtime wired, the shell tool's schema and description -/// automatically omit `run_in_background` and state the kill-on-timeout -/// semantics, so the model is never taught a workflow the platform does not -/// deliver. Enabling Windows later is this one function: build the runtime here -/// once background jobs have been exercised on Windows. -#cfg(platform="windows") -async fn[X] shell_tools( - runtime : @agent_runtime.AgentRuntime, - scope : @agent_runtime.AgentTaskScope[X], - on_background_wake? : () -> Unit, -) -> Array[@agent_tool.AgentToolDefinition] { - ignore(scope) - ignore(on_background_wake) - // A real await keeps this variant genuinely async, matching the POSIX - // variant's signature so `build_tools` stays async on every platform and - // `unused_async` can stay on for the package. - @async.pause() - [@shell.definition(workspace_root=runtime.workspace_root())] + "background job \{snapshot.id} finished (\{status}): `\{snapshot.command}` — read its output with job_output (job_id=\"\{snapshot.id}\")." } ///| /// Build the agent's standard tool registry bound to `runtime` and `scope`. /// -/// The returned registry contains the built-in local tools: `shell` (plus -/// `shell_output` and `shell_stop` where background jobs are supported — see -/// `shell_tools`), `read`, `edit`, `multi_edit`, `write`, `remove`, `plan`, -/// `goal`, `run_moonbit`, and `finish`. +/// The returned registry contains the built-in local tools: `read`, `edit`, +/// `multi_edit`, `write`, `remove`, `plan`, `goal`, `run_moonbit`, `job_output`, +/// `job_stop`, and `finish`. Commands are `run_moonbit`'s job — it compiles and +/// runs a MoonBit program that spawns processes through the shell-free +/// `bobzhang/myshell` EDSL, so there is no shell tool and no shell parsing +/// anywhere in the registry. A snippet that would outlast the foreground bound +/// is detached as a background job, which `job_output` and `job_stop` then +/// watch. /// Each tool lives in its own subpackage of `agent_tool` and exposes a /// `definition()` constructor; this function bundles them into the `Tools` /// value the loop dispatches against. File-oriented tools capture @@ -137,8 +58,42 @@ pub async fn[X] build_tools( // `remove` reads to tell an agent-created file (safe to delete) from a // pre-existing one. let file_state = @agent_tool.FileStateMap::new() + // One session temp dir, shared by the job runtime (which spills large job + // output to disk) and run_moonbit (whose detached snippets outlive the call + // that started them, so their directories cannot be reclaimed there). + let spill_dir : String? = Some(@fs.tmpdir(prefix="openseek-jobs-")) catch { + _ => None + } + // Remove the session temp dir when the owning scope ends, so `/tmp` cannot + // accumulate spill files and snippet directories across turns or sessions. + // A GROUP defer, not a plain one: `build_tools` returns immediately while + // the dir must live as long as the session, and the group runs its defers + // only after every child task has terminated — so a background job still + // spilling output cannot have the directory pulled out from under it. + // `protect_from_cancel` because a group cancelled from outside cancels the + // async operations in its defers too. + if spill_dir is Some(dir) { + scope + .group() + .add_defer(() => { + @async.protect_from_cancel(() => { + @fs.rmdir(dir, recursive=true) catch { + _ => () + } + }) + }) + } + // When a background job exits on its own, push a completion notice into the + // loop's lossless steer queue (surfaced to the model at the next step, or + // persisted while idle) and poke the controller so an idle engine wakes to + // drain it instead of leaving it until the next prompt. + let bg_runtime = @bgjobs.BgJobRuntime::new(scope, spill_dir?, on_job_exit=snapshot => { + runtime.queue_steer(Notice(background_notice_text(snapshot))) + if on_background_wake is Some(wake) { + wake() + } + }) Tools( - shell_tools(runtime, scope, on_background_wake?) + [ @read.definition(workspace_root~), @edit.definition(workspace_root~, file_state~), @@ -147,7 +102,9 @@ pub async fn[X] build_tools( @remove.definition(workspace_root~, file_state~), @plan.definition(), @goal.definition(), - @run_moonbit.definition(workspace_root~), + @run_moonbit.definition(workspace_root~, bg_runtime~, job_dir?=spill_dir), + @job_output.definition(bg_runtime), + @job_stop.definition(bg_runtime), @finish.definition(), ] + extra_tools, @@ -155,16 +112,14 @@ pub async fn[X] build_tools( } ///| -#cfg(not(platform="windows")) async test "agent registers the expected tool names" { @async.with_task_group() <| group => { let tools = build_tools(AgentRuntime(), AgentTaskScope(group)) let function_tools = tools.function_tools() - assert_eq(function_tools.length(), 12) + assert_eq(function_tools.length(), 11) let names = [ for t in function_tools => t.name ] - assert_true(names.contains("shell")) - assert_true(names.contains("shell_output")) - assert_true(names.contains("shell_stop")) + assert_true(names.contains("job_output")) + assert_true(names.contains("job_stop")) assert_true(names.contains("read")) assert_true(names.contains("edit")) assert_true(names.contains("multi_edit")) @@ -181,35 +136,232 @@ async test "agent registers the expected tool names" { } ///| -/// Windows keeps the pre-series shell (no background jobs — see `shell_tools`), -/// so the registry omits shell_output/shell_stop and the shell tool itself must -/// not advertise `run_in_background`. -#cfg(platform="windows") -async test "agent registers the expected tool names on windows" { +/// Commands go through `run_moonbit`, so the registry must not carry a shell +/// tool family — nor the background-job tools that only ever served it. +async test "the registry has no shell tool family" { @async.with_task_group() <| group => { let tools = build_tools(AgentRuntime(), AgentTaskScope(group)) - let function_tools = tools.function_tools() - assert_eq(function_tools.length(), 10) - let names = [ for t in function_tools => t.name ] - assert_true(names.contains("shell")) + let names = [ for t in tools.function_tools() => t.name ] + assert_false(names.contains("shell")) assert_false(names.contains("shell_output")) assert_false(names.contains("shell_stop")) - assert_true(names.contains("read")) - assert_true(names.contains("edit")) - assert_true(names.contains("multi_edit")) - assert_true(names.contains("write")) - assert_true(names.contains("remove")) - assert_true(names.contains("plan")) - assert_true(names.contains("goal")) - assert_true(names.contains("run_moonbit")) - assert_true(names.contains("finish")) - guard tools.find("shell") is Some(shell) else { fail("shell tool missing") } - assert_false(shell.description.contains("run_in_background")) + guard tools.find("run_moonbit") is Some(run_moonbit) else { + fail("run_moonbit tool missing") + } + // The command surface teaches the process EDSL rather than deferring to a + // shell tool that is no longer there. + assert_true(run_moonbit.description.contains("bobzhang/myshell")) + // A job runtime is wired here, so the background argument must be OFFERED: + // the schema is what decides whether the model can make the call at all. + let JsonSchema(schema) = run_moonbit.schema + guard schema is { "properties": Object(properties), .. } else { + fail("expected an object schema") + } + let names = properties.keys().collect() + assert_eq(names.length(), 5) + for name in ["source", "target", "cwd", "warning", "run_in_background"] { + assert_true(names.contains(name)) + } } } ///| -#cfg(not(platform="windows")) +/// A snippet that does not compile must report that HERE, not as a job the +/// model has to go read with `job_output` — and not with the throwaway temp +/// path in place of `source:LINE:COL`. The tool builds before it detaches. +async test "a background run that fails to compile reports the error inline" { + @async.with_task_group(group => { + let tools = build_tools(AgentRuntime(), AgentTaskScope(group)) + guard tools.find("run_moonbit") is Some(run_moonbit) else { + fail("run_moonbit tool missing") + } + let action = match run_moonbit.execute { + Async(execute) => + execute({ + "source": "fn main { let _x : Int = \"nope\" }", + "run_in_background": true, + }) + Sync(_) => fail("run_moonbit should be async") + } + guard action is Respond(output) else { fail("expected Respond") } + assert_true(output.is_error) + assert_false(output.content.contains("started background job")) + assert_true(output.content.contains("did not compile")) + // The diagnostic points at the model's own input. + assert_true(output.content.contains("source:")) + assert_false(output.content.contains("openseek-jobs-")) + }) +} + +///| +/// A detached snippet must not inherit the engine's fd 0: under `serve` that +/// descriptor carries the JSONL commands, so a snippet reading stdin would +/// consume them. It gets an empty file (immediate EOF) like a foreground run. +async test "a background snippet reads EOF on stdin, not the engine's channel" { + @async.with_task_group(group => { + let runtime = @agent_runtime.AgentRuntime() + let mut woke = false + let tools = build_tools(runtime, AgentTaskScope(group), on_background_wake=() => { + woke = true + }) + guard tools.find("run_moonbit") is Some(run_moonbit) else { + fail("run_moonbit tool missing") + } + let source = + #|import { "moonbitlang/async", "moonbitlang/async/stdio" } + #| + #|async fn main { + #| let line = @stdio.stdin.read_until("\n") + #| @stdio.stdout.write("got=\{line is Some(_)}\n") + #|} + let action = match run_moonbit.execute { + Async(execute) => execute({ "source": source, "run_in_background": true }) + Sync(_) => fail("run_moonbit should be async") + } + guard action is Respond(output) else { fail("expected Respond") } + assert_false(output.is_error) + for _ in 0..<2400 { + if woke { + break + } + @async.sleep(25) + } + assert_true(woke) + // The job read EOF rather than blocking on (or stealing from) fd 0. + guard tools.find("job_output") is Some(job_output) else { + fail("job_output tool missing") + } + guard output.content.split_once("background job ") is Some((_, rest)) else { + fail("no job id in: \{output.content}") + } + guard rest.split_once(" ") is Some((job_id, _)) else { + fail("no job id in: \{output.content}") + } + let read = match job_output.execute { + Async(execute) => execute({ "job_id": job_id.to_owned() }) + Sync(_) => fail("job_output should be async") + } + guard read is Respond(job) else { fail("expected Respond") } + assert_true(job.content.contains("got=false")) + }) +} + +///| +/// A foreground run that outlives its bound is DETACHED, not killed: the shell +/// tool behaved that way, and a five-minute `moon test` that gets shot at the +/// deadline loses everything it was about to report. The bound is injected here +/// so the test does not have to outwait the real one. +async test "a foreground run past its deadline becomes a job instead of dying" { + @async.with_task_group(group => { + let scope = @agent_runtime.AgentTaskScope(group) + let spill_dir = @fs.tmpdir(prefix="openseek-jobs-test-") + let bg_runtime = @bgjobs.BgJobRuntime::new(scope, spill_dir~) + let run_moonbit = @run_moonbit.definition( + workspace_root=".", + bg_runtime~, + job_dir=spill_dir, + run_timeout_ms=1500, + ) + let source = + #|import { "moonbitlang/async" } + #| + #|async fn main { + #| println("started") + #| @async.sleep(60_000) + #|} + let action = match run_moonbit.execute { + Async(execute) => execute({ "source": source }) + Sync(_) => fail("run_moonbit should be async") + } + guard action is Respond(output) else { fail("expected Respond") } + // Not an error, and not a corpse: it is a job the model can still watch. + assert_false(output.is_error) + assert_true(output.content.contains("moved to the background as job")) + assert_true(output.content.contains("job_output")) + guard bg_runtime.list() is [job, ..] else { fail("no job was registered") } + assert_true(job.status is Running) + let _ = bg_runtime.stop(job.id) + @fs.rmdir(spill_dir, recursive=true) catch { + _ => () + } + }) +} + +///| +/// Non-UTF-8 output ends the retained wait EARLY, with the child still running. +/// Reporting at that moment would leave the process alive while the deferred +/// cleanup removes its build directory, and would invent an exit code — so the +/// tool waits out the real exit and says the rendering was lossy. +async test "binary output waits for the real exit instead of abandoning it" { + @async.with_task_group(group => { + let tools = build_tools(AgentRuntime(), AgentTaskScope(group)) + guard tools.find("run_moonbit") is Some(run_moonbit) else { + fail("run_moonbit tool missing") + } + // Emit an invalid UTF-8 byte, then keep working briefly before exiting 3. + let source = + #|import { "moonbitlang/async", "moonbitlang/async/stdio", "moonbitlang/x/sys" } + #| + #|async fn main { + #| @stdio.stdout.write(b"\xff\xfe") + #| @async.sleep(300) + #| println("done") + #| @sys.exit(3) + #|} + let action = match run_moonbit.execute { + Async(execute) => execute({ "source": source }) + Sync(_) => fail("run_moonbit should be async") + } + guard action is Respond(output) else { fail("expected Respond") } + assert_true(output.is_error) + // The real exit code, not the -1 an abandoned execution would report. + assert_true(output.content.contains("exited 3")) + assert_true(output.content.contains("non-UTF-8")) + }) +} + +///| +/// ...and that wait is bounded. It used to run with no deadline of its own, so a +/// program that emitted one invalid byte and then kept going — a watcher, a test +/// printing raw bytes — held the turn for its whole lifetime, with the detach +/// below skipped for exactly this case. Both waits share the bound now, and +/// expiry detaches like any other long run. +async test "binary output does not escape the deadline" { + @async.with_task_group(group => { + let scope = @agent_runtime.AgentTaskScope(group) + let spill_dir = @fs.tmpdir(prefix="openseek-jobs-binary-") + let bg_runtime = @bgjobs.BgJobRuntime::new(scope, spill_dir~) + let run_moonbit = @run_moonbit.definition( + workspace_root=".", + bg_runtime~, + job_dir=spill_dir, + run_timeout_ms=1500, + ) + let source = + #|import { "moonbitlang/async", "moonbitlang/async/stdio" } + #| + #|async fn main { + #| @stdio.stdout.write(b"\xff\xfe") + #| @async.sleep(60_000) + #|} + let action = match run_moonbit.execute { + Async(execute) => execute({ "source": source }) + Sync(_) => fail("run_moonbit should be async") + } + guard action is Respond(output) else { fail("expected Respond") } + assert_true(output.content.contains("moved to the background as job")) + guard bg_runtime.list() is [job, ..] else { fail("no job was registered") } + let _ = bg_runtime.stop(job.id) + @fs.rmdir(spill_dir, recursive=true) catch { + _ => () + } + }) +} + +///| +/// Background jobs survived the move off the shell tool: a detached +/// `run_moonbit` snippet still queues a completion notice and wakes an idle +/// controller when it exits. async test "a finished background job queues a notice and wakes the controller" { @async.with_task_group(group => { let runtime = @agent_runtime.AgentRuntime() @@ -217,16 +369,25 @@ async test "a finished background job queues a notice and wakes the controller" let tools = build_tools(runtime, AgentTaskScope(group), on_background_wake=() => { woke = true }) - guard tools.find("shell") is Some(shell) else { fail("shell tool missing") } - // Start a fast background job through the shell tool. - let action = match shell.execute { - Async(execute) => execute({ "cmd": "true", "run_in_background": true }) - Sync(_) => fail("shell should be async") + guard tools.find("run_moonbit") is Some(run_moonbit) else { + fail("run_moonbit tool missing") + } + let action = match run_moonbit.execute { + Async(execute) => + execute({ + "source": "fn main { println(1) }", + "run_in_background": true, + }) + Sync(_) => fail("run_moonbit should be async") } - guard action is Respond(_) else { fail("expected the job to start") } + guard action is Respond(output) else { fail("expected the job to start") } + assert_false(output.is_error) // The job exits on its own → on_job_exit wakes the controller and queues a - // completion notice. - for _ in 0..<200 { + // completion notice. The budget is generous because the job compiles the + // snippet before running it, on a machine that may be running the rest of + // the suite in parallel; the loop leaves as soon as the wake lands, so a + // fast machine pays none of it. + for _ in 0..<2400 { if woke { break } @@ -237,14 +398,3 @@ async test "a finished background job queues a notice and wakes the controller" assert_true(steers.any(s => s is Notice(_))) }) } - -///| -/// Mark the imports the non-Windows `shell_tools` uses as used on Windows: -/// this binding is `_`-prefixed (never warned about) and its body is never -/// evaluated. -let _unused_windows_imports : Unit = { - ignore(@bgjobs.BgJobRuntime::list) - ignore(@shell_output.definition) - ignore(@shell_stop.definition) - ignore(@fs.exists) -} diff --git a/agent_explore/README.mbt.md b/agent_explore/README.mbt.md index a5ffa3565..f57dc22aa 100644 --- a/agent_explore/README.mbt.md +++ b/agent_explore/README.mbt.md @@ -12,10 +12,12 @@ spot-check — conclusions enter the parent's context, never file dumps. Contract highlights: -- Child toolset: `read` + `shell(read_only=true)` + `submit_answer` — no edit - tools, no nested subagent tools. A per-child scratch lab (temp dir) is - the one writable place: the scout may scaffold throwaway projects and - run any moon command there to verify claims empirically. +- Child toolset: `read` + `run_moonbit` + `submit_answer` — no edit tools, no + nested subagent tools. Commands run from a `run_moonbit` snippet through the + shell-free `bobzhang/myshell` EDSL, and the source-write sandbox denies + writes to the workspace's own sources. A per-child scratch lab (temp dir) is + the one writable place: the scout may scaffold throwaway projects and run any + moon command there to verify claims empirically. - Every report field is capped at submission (`ExploreReport::validate`), so the rendered result stays far below the loop's tool-result clamp. - Launching takes one slot of the shared per-turn `SubrunBudget` call diff --git a/agent_explore/explore_system_prompt.mbt.md b/agent_explore/explore_system_prompt.mbt.md index 6ff4ea3ff..1bb0748f0 100644 --- a/agent_explore/explore_system_prompt.mbt.md +++ b/agent_explore/explore_system_prompt.mbt.md @@ -46,3 +46,11 @@ Rules: oversized submissions are rejected for retry. - When done, call submit_answer exactly once with the full report (schema_version 1). Do not finish with plain text. + +## Running Commands + +There is no shell tool. Every command — `moon`, `git`, anything else — is +spawned from a `run_moonbit` snippet; that tool's description carries the shape +of a snippet and the list of programs one may start. A snippet is bound by the +same rule as the rest of your work: the scratch lab is the one place it may +write. diff --git a/agent_explore/generated_explore_system_prompt.mbt b/agent_explore/generated_explore_system_prompt.mbt index e6941f974..110820a87 100644 --- a/agent_explore/generated_explore_system_prompt.mbt +++ b/agent_explore/generated_explore_system_prompt.mbt @@ -52,5 +52,13 @@ fn explore_system_prompt() -> String { #|- When done, call submit_answer exactly once with the full report #| (schema_version 1). Do not finish with plain text. #| + #|## Running Commands + #| + #|There is no shell tool. Every command — `moon`, `git`, anything else — is + #|spawned from a `run_moonbit` snippet; that tool's description carries the shape + #|of a snippet and the list of programs one may start. A snippet is bound by the + #|same rule as the rest of your work: the scratch lab is the one place it may + #|write. + #| ) } diff --git a/agent_explore/moon.pkg b/agent_explore/moon.pkg index 32323f209..39e26f4f1 100644 --- a/agent_explore/moon.pkg +++ b/agent_explore/moon.pkg @@ -4,7 +4,6 @@ import { "bobzhang/openseek/agent_tool", "bobzhang/openseek/agent_tool/read", "bobzhang/openseek/agent_tool/run_moonbit", - "bobzhang/openseek/agent_tool/shell", "bobzhang/openseek/deepseek", "moonbitlang/core/json", } diff --git a/agent_explore/tool.mbt b/agent_explore/tool.mbt index 267d583bc..db50859e7 100644 --- a/agent_explore/tool.mbt +++ b/agent_explore/tool.mbt @@ -27,13 +27,12 @@ let max_hints_chars : Int = 2_000 /// The scout runs in a DEDICATED CHILD PROCESS: `self_exe` (the parent's /// own executable — never PATH, so parent and child are the same binary and /// the report's derived JSON codecs cannot drift) invoked as -/// `subrun explore`. The child's toolset is `read`, -/// `shell(read_only=true)` (its instrument for `moon ide doc` and friends; -/// a per-child scratch lab is the one writable place, where the scout may -/// scaffold throwaway projects to verify claims empirically), -/// `run_moonbit` (run a self-contained snippet to check language/stdlib -/// behavior), and `submit_answer` — no edit tools, no nested subagent -/// tools. The +/// `subrun explore`. The child's toolset is `read`, `run_moonbit` (both its +/// instrument for `moon ide doc` and friends and its way to check +/// language/stdlib behavior with a self-contained snippet; a per-child +/// scratch lab is the one writable place, where the scout may scaffold +/// throwaway projects to verify claims empirically), and `submit_answer` — +/// no edit tools, no nested subagent tools. The /// child inherits the environment for its API key; model and endpoint ride /// argv. Launching consumes one slot of the shared per-turn `SubrunBudget` /// call allowance BEFORE the child exists; the child runs at its full step @@ -229,8 +228,7 @@ pub async fn run_child( tools=captured => { @agent_tool.Tools([ @read.definition(workspace_root~), - @shell.definition(workspace_root~, read_only=true, scratch_dir?), - @run_moonbit.definition(workspace_root~), + @run_moonbit.definition(workspace_root~, read_only=true, scratch_dir?), submit_answer_tool(captured), ]) }, diff --git a/agent_review/README.mbt.md b/agent_review/README.mbt.md index 20f8fa1c0..341a4deff 100644 --- a/agent_review/README.mbt.md +++ b/agent_review/README.mbt.md @@ -11,9 +11,9 @@ dispatch a review to OpenSeek. `run_review(base, …)` reviews the diff between `base` and `HEAD`: -1. drives a model over a **read-only** toolset (`read`, `shell` in read-only - mode, and `submit_review`) — no `edit`/`multi_edit`/`write`, so it reports - rather than rewrites; +1. drives a model over a **read-only** toolset (`read`, `run_moonbit`, and + `submit_review`) — no `edit`/`multi_edit`/`write`, so it reports rather than + rewrites; 2. instructs the model to ground every finding in the compiler — run `moon check`/`moon test` and cite real diagnostics, not opinion; 3. captures the model's `submit_review` call into a validated `ReviewReport` and @@ -68,12 +68,13 @@ instead of finishing. ## Read-only stance (best-effort, not airtight) -The review has no edit/write tools, and its `shell` runs in read-only mode: it -refuses the obvious bulk source-rewriters (`moon fmt` / `moon info` / -`moon test --update`) anywhere in the parsed command. It is **not** an airtight -guarantee — `moon check`/`moon test` can trigger `pre-build` hooks that generate -source — so the checkout is not promised byte-for-byte unchanged. By design, a -review *reports* rather than *edits*. +The review has no edit/write tools, and the commands it runs through +`run_moonbit` inherit the source-write sandbox, which denies writes to the +workspace's sources — including the bulk source-rewriters (`moon fmt` / +`moon info` / `moon test --update`). It is **not** an airtight guarantee: the +profile is best-effort (a program can still smuggle sources via directory +renames), and where it cannot be enforced at all the run is unsandboxed. By +design, a review *reports* rather than *edits*. ## Using it diff --git a/agent_review/audit.mbt b/agent_review/audit.mbt index c7bcdea25..f718811d1 100644 --- a/agent_review/audit.mbt +++ b/agent_review/audit.mbt @@ -88,8 +88,7 @@ pub async fn run_goal_audit( tools=captured => { @agent_tool.Tools([ @read.definition(workspace_root~), - @shell.definition(workspace_root~, read_only=true, scratch_dir?), - @run_moonbit.definition(workspace_root~), + @run_moonbit.definition(workspace_root~, read_only=true, scratch_dir?), submit_review_tool(captured), ]) }, diff --git a/agent_review/audit_system_prompt.mbt.md b/agent_review/audit_system_prompt.mbt.md index 2cf00b646..57c1762e4 100644 --- a/agent_review/audit_system_prompt.mbt.md +++ b/agent_review/audit_system_prompt.mbt.md @@ -32,3 +32,9 @@ Principles: - When done, call submit_review exactly once with the full structured report (schema_version 1); set scope.head to "WORKTREE". Do not finish with plain text. + +## Running Commands + +There is no shell tool. Every command — `moon`, `git`, anything else — is +spawned from a `run_moonbit` snippet; that tool's description carries the shape +of a snippet and the list of programs one may start. diff --git a/agent_review/engine.mbt b/agent_review/engine.mbt index d6d144178..6a0c3d21a 100644 --- a/agent_review/engine.mbt +++ b/agent_review/engine.mbt @@ -12,14 +12,15 @@ let default_review_max_steps : Int = 120 ///| /// Run a code review of the changes between `base` and HEAD and return the -/// structured report. The toolset is `read`, `shell` (in read-only mode), -/// `run_moonbit`, and `submit_review` — no edit/multi_edit/write — so the -/// review reports rather than edits. Read-only is best-effort: the shell -/// refuses obvious source-mutating moon commands and `run_moonbit` is -/// sandboxed against source writes on macOS, but `moon check`/`moon test` -/// may still trigger pre-build source generation and `run_moonbit` can write -/// non-source files, so the checkout is not guaranteed byte-for-byte -/// unchanged. Raises `ReviewError` if the model finishes without submitting a +/// structured report. The toolset is `read`, `run_moonbit`, and +/// `submit_review` — no edit/multi_edit/write — so the review reports rather +/// than edits. Read-only is best-effort: `run_moonbit` runs under the "may not +/// write source" profile on macOS, but `moon check`/`moon test` may still +/// trigger pre-build source generation and a snippet can write non-source +/// files, so the checkout is not guaranteed byte-for-byte unchanged. Off macOS +/// there is no kernel profile at all. `scratch_dir` names the one place a +/// snippet may write; without it there is no lab, but the profile still +/// applies. Raises `ReviewError` if the model finishes without submitting a /// report. /// /// Runs the review kind IN THIS PROCESS via `@agent_subrun.execute_kind`: the @@ -34,16 +35,16 @@ pub async fn run_review( max_steps? : Int = default_review_max_steps, thinking? : @deepseek.ThinkingMode = Max, api_url? : String, + scratch_dir? : String, ) -> ReviewReport { let report = @agent_subrun.execute_kind( session_id="review", system_prompt=review_system_prompt(), - task=review_task(base), + task=review_task(base, scratch_dir), tools=captured => { @agent_tool.Tools([ @read.definition(workspace_root~), - @shell.definition(workspace_root~, read_only=true), - @run_moonbit.definition(workspace_root~), + @run_moonbit.definition(workspace_root~, read_only=true, scratch_dir?), submit_review_tool(captured), ]) }, diff --git a/agent_review/generated_audit_system_prompt.mbt b/agent_review/generated_audit_system_prompt.mbt index caed30129..df5c0e0cc 100644 --- a/agent_review/generated_audit_system_prompt.mbt +++ b/agent_review/generated_audit_system_prompt.mbt @@ -38,5 +38,11 @@ fn audit_system_prompt() -> String { #| report (schema_version 1); set scope.head to "WORKTREE". Do not #| finish with plain text. #| + #|## Running Commands + #| + #|There is no shell tool. Every command — `moon`, `git`, anything else — is + #|spawned from a `run_moonbit` snippet; that tool's description carries the shape + #|of a snippet and the list of programs one may start. + #| ) } diff --git a/agent_review/generated_review_system_prompt.mbt b/agent_review/generated_review_system_prompt.mbt index 7f7d46279..dac9449e1 100644 --- a/agent_review/generated_review_system_prompt.mbt +++ b/agent_review/generated_review_system_prompt.mbt @@ -12,5 +12,11 @@ fn review_system_prompt() -> String { #|- Be precise and skeptical. Prefer a few real, verifiable findings over many speculative ones. Severity is one of blocker|high|medium|low|nit. #|- When the review is complete, call submit_review exactly once with the full structured report (schema_version 1). Do not finish with plain text. #| + #|## Running Commands + #| + #|There is no shell tool. Every command — `moon`, `git`, anything else — is + #|spawned from a `run_moonbit` snippet; that tool's description carries the shape + #|of a snippet and the list of programs one may start. + #| ) } diff --git a/agent_review/moon.pkg b/agent_review/moon.pkg index cfe87f2bc..e6e86bb36 100644 --- a/agent_review/moon.pkg +++ b/agent_review/moon.pkg @@ -4,7 +4,6 @@ import { "bobzhang/openseek/agent_tool", "bobzhang/openseek/agent_tool/read", "bobzhang/openseek/agent_tool/run_moonbit", - "bobzhang/openseek/agent_tool/shell", "bobzhang/openseek/deepseek", "moonbitlang/core/json", } diff --git a/agent_review/pkg.generated.mbti b/agent_review/pkg.generated.mbti index 4a01cd757..67caaa197 100644 --- a/agent_review/pkg.generated.mbti +++ b/agent_review/pkg.generated.mbti @@ -15,7 +15,7 @@ pub let default_audit_max_steps : Int pub async fn run_goal_audit(goal~ : String, baseline~ : @agent_session.GoalBaseline?, api_key~ : String, model~ : @deepseek.Model, max_steps~ : Int, thinking? : @deepseek.ThinkingMode, api_url? : String, workspace_root? : String, session_id? : String, append_item? : async (@agent_session.Session, @agent_session.SessionItem) -> @agent_session.Session, scratch_dir? : String) -> ReviewReport? -pub async fn run_review(String, @deepseek.Model, String, workspace_root? : String, max_steps? : Int, thinking? : @deepseek.ThinkingMode, api_url? : String) -> ReviewReport +pub async fn run_review(String, @deepseek.Model, String, workspace_root? : String, max_steps? : Int, thinking? : @deepseek.ThinkingMode, api_url? : String, scratch_dir? : String) -> ReviewReport pub fn severities() -> Array[String] diff --git a/agent_review/prompt.mbt b/agent_review/prompt.mbt index 367177aad..2a5aab6e8 100644 --- a/agent_review/prompt.mbt +++ b/agent_review/prompt.mbt @@ -5,10 +5,22 @@ ///| /// The review task: inspect the change set between `base` and HEAD. -fn review_task(base : String) -> String { +fn review_task(base : String, scratch_dir : String?) -> String { + let lab = match scratch_dir { + Some(lab) => + ( + $| + $|Your scratch lab (writable): \{lab} + $|Create files and projects THERE and run any moon command there to + $|verify a claim empirically before reporting it. The checkout under + $|review stays read-only. + $| + ) + None => "" + } ( $|Review the code changes between \{base} and HEAD in the current repository. - $| + $|\{lab} $|Steps: $|1. Run `git diff \{base}...HEAD` and `git diff --name-only \{base}...HEAD` to see the change set. $|2. Read the changed files and enough surrounding context to judge correctness. diff --git a/agent_review/review_system_prompt.mbt.md b/agent_review/review_system_prompt.mbt.md index 5b4d40562..4f91882b9 100644 --- a/agent_review/review_system_prompt.mbt.md +++ b/agent_review/review_system_prompt.mbt.md @@ -6,3 +6,9 @@ Principles: - Report, do not rewrite. You have no edit tools. Point at exact file:line. - Be precise and skeptical. Prefer a few real, verifiable findings over many speculative ones. Severity is one of blocker|high|medium|low|nit. - When the review is complete, call submit_review exactly once with the full structured report (schema_version 1). Do not finish with plain text. + +## Running Commands + +There is no shell tool. Every command — `moon`, `git`, anything else — is +spawned from a `run_moonbit` snippet; that tool's description carries the shape +of a snippet and the list of programs one may start. diff --git a/agent_tool/README.mbt.md b/agent_tool/README.mbt.md index 754d70ed3..2881b288d 100644 --- a/agent_tool/README.mbt.md +++ b/agent_tool/README.mbt.md @@ -11,11 +11,10 @@ Concrete built-in tools live in subpackages: - `agent_tool/multi_edit` - `agent_tool/write` - `agent_tool/remove` -- `agent_tool/shell` (with `agent_tool/shell_output` and `agent_tool/shell_stop` +- `agent_tool/run_moonbit` (with `agent_tool/job_output` and `agent_tool/job_stop` for background jobs, and `agent_tool/bgjobs` as their shared runtime) - `agent_tool/plan` - `agent_tool/goal` -- `agent_tool/run_moonbit` - `agent_tool/finish` ## API Shape diff --git a/agent_tool/bgjobs/bgjobs.mbt b/agent_tool/bgjobs/bgjobs.mbt index c869f8419..8d51253fa 100644 --- a/agent_tool/bgjobs/bgjobs.mbt +++ b/agent_tool/bgjobs/bgjobs.mbt @@ -42,7 +42,14 @@ pub struct BgJobRuntime { // Spawn a `ShellExecution` on the captured session group: (program, args, cwd, // inline-preview budget, spill path) -> execution. Captured so the runtime type // is not generic. - priv spawn_exec : async (String, Array[String], String?, Int, String?) -> @shell_exec.ShellExecution + priv spawn_exec : async ( + String, + Array[String], + String?, + Int, + String?, + &@process.ProcessInput?, + ) -> @shell_exec.ShellExecution // Spawn a *retained* foreground execution (may be detached on timeout): // file-backed with a large hard cap, but still killed on exceeding the inline // cap so a timed command that floods output is an immediate output-limit error; @@ -54,6 +61,7 @@ pub struct BgJobRuntime { String?, Int, String?, + &@process.ProcessInput?, ) -> @shell_exec.ShellExecution // Spawn a per-job watcher task on that same group. priv spawn_watcher : (async () -> Unit) -> Unit @@ -137,7 +145,7 @@ pub fn[X] BgJobRuntime::new( ) -> BgJobRuntime { let group = scope.group() { - spawn_exec: (program, args, cwd, max_output_chars, spill_path) => { + spawn_exec: (program, args, cwd, max_output_chars, spill_path, stdin) => { // Background retention: inline preview up to the budget, full output to // the spill file, and the hard-cap watchdog so a flooding job is killed // (and surfaced) rather than left burning CPU with its output dropped. @@ -146,6 +154,7 @@ pub fn[X] BgJobRuntime::new( program~, args~, cwd?, + stdin?, inline_cap=max_output_chars, hard_cap=hard_output_cap, spill_path?, @@ -158,6 +167,7 @@ pub fn[X] BgJobRuntime::new( cwd, max_output_chars, spill_path, + stdin, ) => { // kill_at_hard_cap so that once detached (kill_when_full cleared by // `background()`), the job is still bounded in output like any other. @@ -166,6 +176,7 @@ pub fn[X] BgJobRuntime::new( program~, args~, cwd?, + stdin?, inline_cap=max_output_chars, hard_cap=hard_output_cap, spill_path?, @@ -206,6 +217,7 @@ pub async fn BgJobRuntime::spawn_foreground_retained( args : Array[String], cwd : String?, max_output_chars : Int, + stdin? : &@process.ProcessInput, ) -> @shell_exec.ShellExecution { let spill_path = self.spill_dir.map(dir => { let index = self.fg_index @@ -213,7 +225,7 @@ pub async fn BgJobRuntime::spawn_foreground_retained( "\{dir}/fg-\{index}.out" }) (self.spawn_foreground_retained_exec)( - program, args, cwd, max_output_chars, spill_path, + program, args, cwd, max_output_chars, spill_path, stdin, ) } @@ -230,12 +242,15 @@ pub async fn BgJobRuntime::start( max_output_chars? : Int = default_max_output_chars, sandboxed_command? : @sandbox.SandboxedCommand, read_only_review? : Bool = false, + stdin? : &@process.ProcessInput, ) -> String { // Reserve one id and use it for both the spill file and the registry entry, so // the job id and its `.out` file never diverge. let id = self.next_id() let spill_path = self.spill_dir.map(dir => "\{dir}/\{id}.out") - let exec = (self.spawn_exec)(program, args, cwd, max_output_chars, spill_path) + let exec = (self.spawn_exec)( + program, args, cwd, max_output_chars, spill_path, stdin, + ) self.register(id, exec, command~, cwd?, sandboxed_command?, read_only_review~) id } diff --git a/agent_tool/bgjobs/moon.pkg b/agent_tool/bgjobs/moon.pkg index de1463bb3..fa2fae4ec 100644 --- a/agent_tool/bgjobs/moon.pkg +++ b/agent_tool/bgjobs/moon.pkg @@ -3,6 +3,7 @@ import { "bobzhang/openseek/agent_tool/internal/sandbox", "bobzhang/openseek/agent_tool/shell_exec", "moonbitlang/async", + "moonbitlang/async/process", "moonbitlang/core/env", } diff --git a/agent_tool/bgjobs/pkg.generated.mbti b/agent_tool/bgjobs/pkg.generated.mbti index 010d85823..89812f988 100644 --- a/agent_tool/bgjobs/pkg.generated.mbti +++ b/agent_tool/bgjobs/pkg.generated.mbti @@ -5,6 +5,7 @@ import { "bobzhang/openseek/agent_runtime", "bobzhang/openseek/agent_tool/internal/sandbox", "bobzhang/openseek/agent_tool/shell_exec", + "moonbitlang/async/process", } // Values @@ -21,8 +22,8 @@ pub fn[X] BgJobRuntime::new(@agent_runtime.AgentTaskScope[X], spill_dir? : Strin pub async fn BgJobRuntime::read_output(Self, String) -> String? pub async fn BgJobRuntime::read_output_tail(Self, String, Int) -> String? pub fn BgJobRuntime::snapshot(Self, String) -> BgJobSnapshot? -pub async fn BgJobRuntime::spawn_foreground_retained(Self, String, Array[String], String?, Int) -> @shell_exec.ShellExecution -pub async fn BgJobRuntime::start(Self, program~ : String, args~ : Array[String], command~ : String, cwd? : String, max_output_chars? : Int, sandboxed_command? : @sandbox.SandboxedCommand, read_only_review? : Bool) -> String +pub async fn BgJobRuntime::spawn_foreground_retained(Self, String, Array[String], String?, Int, stdin? : &@process.ProcessInput) -> @shell_exec.ShellExecution +pub async fn BgJobRuntime::start(Self, program~ : String, args~ : Array[String], command~ : String, cwd? : String, max_output_chars? : Int, sandboxed_command? : @sandbox.SandboxedCommand, read_only_review? : Bool, stdin? : &@process.ProcessInput) -> String pub async fn BgJobRuntime::stop(Self, String) -> Bool pub async fn BgJobRuntime::wait_exit(Self, String, Int) -> Bool diff --git a/agent_tool/internal/sandbox/capability.mbt b/agent_tool/internal/sandbox/capability.mbt index 168c19c99..a2d0bd6ca 100644 --- a/agent_tool/internal/sandbox/capability.mbt +++ b/agent_tool/internal/sandbox/capability.mbt @@ -23,13 +23,20 @@ struct SandboxedCommand { /// `program = "/usr/bin/sandbox-exec"` with /// `args = ["-p", "", "sh", "-c", "moon check"]` on macOS. args : Array[String] - /// Protected paths and basenames derived from the same profile encoded in - /// `args`, retained so denial reporting cannot use metadata from another run. + /// The paths a denial from this profile may name — protected directories and + /// basenames — which is how one is told apart from a permission error this + /// sandbox did not cause. Derived from the same profile encoded in `args`, so + /// denial reporting cannot use metadata from another run. denial_subjects : Array[String] /// True only for the ordinary source-readonly profile with no writable /// subtree. A trusted source writer can escape this profile by running as a /// standalone shell command; worker and scratch-lab profiles cannot. plain_source_readonly : Bool + /// What to tell the model when this command's output reports a denial. Held + /// per command rather than picked by the caller, because the profiles deny + /// different things: the source-readonly one denies source writes, the worker + /// one denies writes outside its worktree, and each needs its own way forward. + denial_feedback : String } ///| @@ -95,6 +102,7 @@ pub async fn SandboxedCommand::create_if_available( // keep classifying with the subjects of the profile it actually runs under. denial_subjects: data.denial_subjects.copy(), plain_source_readonly: subtree is None, + denial_feedback: source_write_denial_feedback, }) } @@ -121,6 +129,15 @@ pub fn SandboxedCommand::output_reports_denial( source_write_denied_in_text(output, self.denial_subjects) } +///| +/// The guidance to append when this command's output reports a denial. It comes +/// from the prepared command that classified the output, not from a global a +/// caller picks by hand, so the text can never describe a profile other than the +/// one that actually refused. +pub fn SandboxedCommand::denial_feedback(self : SandboxedCommand) -> String { + self.denial_feedback +} + ///| /// Whether this command uses the ordinary source-readonly profile, where a /// normally trusted source writer may succeed when retried by itself. diff --git a/agent_tool/internal/sandbox/denial_output.mbt b/agent_tool/internal/sandbox/denial_output.mbt index 8cb5c7298..94d81a095 100644 --- a/agent_tool/internal/sandbox/denial_output.mbt +++ b/agent_tool/internal/sandbox/denial_output.mbt @@ -1,3 +1,37 @@ +///| +/// Guidance for a sandboxed run whose output shows a source-write denial +/// ("Operation not permitted" on a protected source path). Without it the model +/// reads a bare OS error and debugs the filesystem — or worse, tries to escape +/// the sandbox — instead of using `edit`. It lives beside the denial detector +/// because both the foreground runner and the background-job reader need the +/// same explanation for the same recognized condition. +/// +/// Names the way forward without naming ways around it, for the same reason as +/// `policy_refusal_feedback`. +let source_write_denial_feedback : String = + #|run_moonbit sandbox: the "Operation not permitted" failure above is this tool's + #|sandbox denying a write to a protected MoonBit source file (`*.mbt`, `*.mbti`, + #|`*.mbt.md`, `moon.mod`/`moon.pkg`/`moon.work`) — not a filesystem or permissions + #|problem to debug. Snippets run source-write-readonly by design; writing non-source + #|files (data, logs, reports) stays allowed, and source-writing moon commands + #|(`moon fmt`, `moon info`, `moon add`, `moon test --update`) are denied here by the + #|same rule. Make source changes with the line-anchored `edit` tool, and keep + #|run_moonbit for commands, compute, probing, and non-source IO. + +///| +/// The same guidance for the WORKER profile, which denies a different thing: a +/// worker is supposed to write source, just only its own, so what it can hit is +/// the boundary around its worktree. Told the source-write text instead, it +/// would go looking for an `edit` tool to do what it may already do directly. +let worker_write_denial_feedback : String = + #|run_moonbit sandbox: the "Operation not permitted" failure above is this tool's + #|sandbox denying a write OUTSIDE your worktree — a sibling worktree, the parent + #|checkout, or the repository's shared git state — not a filesystem or permissions + #|problem to debug. Your own worktree stays fully writable, source included. This + #|is the boundary your task defines, so do not work around it: if the slice + #|genuinely requires touching something outside it, stop and report that in your + #|result (status partial or failed) instead. + ///| /// Protected MoonBit source and manifest names, as they appear at the end of a /// path in a denial line. Matched with `line_names_subject`, so `.mbt` does not diff --git a/agent_tool/internal/sandbox/pkg.generated.mbti b/agent_tool/internal/sandbox/pkg.generated.mbti index 7e69c51f4..4034dfd61 100644 --- a/agent_tool/internal/sandbox/pkg.generated.mbti +++ b/agent_tool/internal/sandbox/pkg.generated.mbti @@ -2,6 +2,9 @@ package "bobzhang/openseek/agent_tool/internal/sandbox" // Values +pub fn output_reports_policy_refusal(String) -> Bool + +pub let policy_refusal_feedback : String // Errors @@ -16,6 +19,7 @@ pub fn SandboxedCommand::allows_standalone_trusted_writer(Self) -> Bool pub fn SandboxedCommand::args(Self) -> Array[String] pub async fn SandboxedCommand::create_if_available(String, Command, writable_subtree? : String) -> Self? pub async fn SandboxedCommand::create_worker_if_available(Command, deny_roots~ : Array[String], worker_root~ : String, worker_admin_dir~ : String) -> Self? +pub fn SandboxedCommand::denial_feedback(Self) -> String pub fn SandboxedCommand::output_reports_denial(Self, String) -> Bool pub fn SandboxedCommand::program(Self) -> String diff --git a/agent_tool/internal/sandbox/policy_refusal.mbt b/agent_tool/internal/sandbox/policy_refusal.mbt new file mode 100644 index 000000000..294947025 --- /dev/null +++ b/agent_tool/internal/sandbox/policy_refusal.mbt @@ -0,0 +1,47 @@ +///| +/// The line moonrun prints when its `--wasm-policy` blocks a file or network +/// access, ahead of the error the guest itself sees: +/// +/// ```text +/// Sandbox policy blocked file write: "/private/tmp/escape.txt" +/// OSError("@fs.open(): \"/private/tmp/escape.txt\": Permission denied") +/// ``` +const PolicyBlockedAccess : String = "Sandbox policy blocked " + +///| +/// A refused spawn prints no such line — the guest just sees `EPERM` from the +/// one call a policy refuses this way, so the call itself is the marker. +const PolicyRefusedSpawn : String = "@process.spawn(): Permission denied" + +///| +/// Whether `output` shows the moonrun policy refusing something. +/// +/// The policy is the other confinement layer from the `sandbox-exec` profiles +/// this module prepares, and the one that exists everywhere: it binds the +/// snippet on every platform, so this is read whether or not a kernel profile +/// was available. +/// +/// Recognition is by the runtime's own fixed wording rather than by reasoning +/// about paths, which is what keeps it independent of the roots a policy +/// happens to grant: widening or narrowing the policy cannot make a refusal +/// stop being recognized. +pub fn output_reports_policy_refusal(output : String) -> Bool { + output.contains(PolicyBlockedAccess) || output.contains(PolicyRefusedSpawn) +} + +///| +/// Guidance to append when a run's output shows a policy refusal. +/// +/// Names both rules rather than asserting one: a refused spawn carries no path, +/// and reading it as a blocked write once sent the model to debug paths. +/// +/// Ends on the replacement because that is what every refusal in 60 measured +/// trials was — `ls`, `cat`, `pwd`, `which`, `sh`. It deliberately lists no ways +/// around the boundary; naming evasion techniques teaches them without stopping +/// anyone. +pub let policy_refusal_feedback : String = + #|run_moonbit sandbox: that failure is this tool's policy refusing something, not a + #|filesystem problem to debug — either the program is not on the spawn allowlist, or + #|the snippet tried to write outside its own `@fs.tmpdir()`. Reach for the + #|MoonBit replacement of the utility you wanted rather than another program, and + #|for the file tools to change a file. diff --git a/agent_tool/internal/sandbox/policy_refusal_test.mbt b/agent_tool/internal/sandbox/policy_refusal_test.mbt new file mode 100644 index 000000000..571651ada --- /dev/null +++ b/agent_tool/internal/sandbox/policy_refusal_test.mbt @@ -0,0 +1,48 @@ +///| +/// The two wordings are pinned as moonrun actually prints them, captured from a +/// run under a policy rather than paraphrased. A refusal is recognized by these +/// strings alone, so a paraphrase here would silently stop matching production +/// output while the test kept passing. +test "a policy refusal is recognized by the runtime's own wording" { + // A blocked access: moonrun's line, then the error the guest sees. + assert_true( + @sandbox.output_reports_policy_refusal( + "Sandbox policy blocked file write: \"/private/tmp/escape.txt\"\n", + ), + ) + assert_true( + @sandbox.output_reports_policy_refusal( + "Sandbox policy blocked file read: \"/etc/hosts\"\n", + ), + ) + // A refused spawn: no line of moonrun's own, so the call itself is the marker. + assert_true( + @sandbox.output_reports_policy_refusal( + "OSError(\"@process.spawn(): Permission denied\")\n", + ), + ) + // A permission error this tool did not cause reads as what it is. Both of + // these were denials under the old path-subtracting detector, which called + // any "Permission denied" outside a granted root its own. + assert_false( + @sandbox.output_reports_policy_refusal( + "OSError(\"@fs.open(): \\\"/etc/shadow\\\": Permission denied\")\n", + ), + ) + assert_false( + @sandbox.output_reports_policy_refusal("error: Permission denied\n"), + ) + assert_false(@sandbox.output_reports_policy_refusal("")) +} + +///| +/// The guidance has to cover both rules, because a refused spawn carries no path +/// and reading it as a blocked write once sent the model to debug one. +test "policy guidance names both rules that produce a refusal" { + let feedback = @sandbox.policy_refusal_feedback + assert_true(feedback.contains("write outside its own")) + assert_true(feedback.contains("spawn allowlist")) + // And it ends on the move that answers the refusals that actually happen — + // POSIX utilities with a one-line MoonBit form. + assert_true(feedback.contains("MoonBit replacement")) +} diff --git a/agent_tool/internal/sandbox/worker_profile.mbt b/agent_tool/internal/sandbox/worker_profile.mbt index fbefe836a..4904f087c 100644 --- a/agent_tool/internal/sandbox/worker_profile.mbt +++ b/agent_tool/internal/sandbox/worker_profile.mbt @@ -128,5 +128,6 @@ pub async fn SandboxedCommand::create_worker_if_available( args, denial_subjects: data.denial_subjects, plain_source_readonly: false, + denial_feedback: worker_write_denial_feedback, }) } diff --git a/agent_tool/internal/sandbox/worker_profile_test.mbt b/agent_tool/internal/sandbox/worker_profile_test.mbt index 9f6ba6c4a..de1137ffe 100644 --- a/agent_tool/internal/sandbox/worker_profile_test.mbt +++ b/agent_tool/internal/sandbox/worker_profile_test.mbt @@ -77,6 +77,13 @@ async test "the worker profile confines writes to the worker tree" { assert_true(origin_exit != 0) assert_eq(@fs.read_file("\{origin}/seed.txt").text(), "seed\n") assert_true(against_origin.output_reports_denial(origin_text)) + // The guidance follows the profile. A worker that hit its boundary must not + // get the source-write text: it would be told to make source changes with + // `edit`, which is neither what failed nor something it needs — its own tree + // is writable. + let guidance = against_origin.denial_feedback() + assert_true(guidance.contains("OUTSIDE your worktree")) + assert_false(guidance.contains("line-anchored `edit`")) // 4. A sibling worktree is not writable. guard probe("printf x > \{sibling}/intrude.txt") is Some(against_sibling) else { fail("probe build failed") diff --git a/agent_tool/shell_output/README.mbt.md b/agent_tool/job_output/README.mbt.md similarity index 80% rename from agent_tool/shell_output/README.mbt.md rename to agent_tool/job_output/README.mbt.md index 3b98bf73a..0eb4ef252 100644 --- a/agent_tool/shell_output/README.mbt.md +++ b/agent_tool/job_output/README.mbt.md @@ -1,12 +1,11 @@ -# Shell Output Tool +# Job Output Tool -`shell_output` reads a background shell job's recent output and status by its -`job_id` — the id returned by `shell` with `run_in_background=true`, or by a -foreground command that was moved to the background when it outlived its -`timeout_ms`. +`job_output` reads a background job's recent output and status by its +`job_id` — the id `run_moonbit` returns when called with +`run_in_background=true`. The primary consumption path for job results is the **pushed completion -notice** (a job announces itself when it finishes); `shell_output` is for +notice** (a job announces itself when it finishes); `job_output` is for checking progress on a still-running job, and for reading the output once the notice arrives. The tool description and system prompts steer the model away from calling it in a polling loop. @@ -29,7 +28,8 @@ Output first, one `` footer line last (the `read` tool convention): wall-clock reaper) so the model does not read it as a requested stop. - Status is `running`, `exit=`, or `stopped`; non-zero exits and stops - are tool errors, preserving the foreground shell's semantics. + are tool errors, matching what the same snippet would report in the + foreground. ## Design Rationale @@ -41,7 +41,7 @@ Two invariants drove the implementation: drops. The footer metadata is sampled *after* the awaited read, so a job that appends or exits while the read yields cannot produce a stale footer. 2. **Foreground error-semantics parity.** A background job read later behaves - exactly like the same command run in the foreground: binary (non-UTF-8) + exactly like the same snippet run in the foreground: binary (non-UTF-8) output is a tool error even at exit 0 (`binary_output=true`), and a sandboxed source-write denial is detected by scanning the *full* retained output (the denial line can be earlier than the displayed tail) with the diff --git a/agent_tool/shell_output/shell_output.mbt b/agent_tool/job_output/job_output.mbt similarity index 74% rename from agent_tool/shell_output/shell_output.mbt rename to agent_tool/job_output/job_output.mbt index 356410fdf..fd832c9a9 100644 --- a/agent_tool/shell_output/shell_output.mbt +++ b/agent_tool/job_output/job_output.mbt @@ -1,15 +1,15 @@ ///| -/// Tool `shell_output`: read a background shell job's output and status by id. -/// The id comes from `shell run_in_background`, or from a foreground command that +/// Tool `job_output`: read a background job's output and status by id. +/// The id comes from `run_moonbit run_in_background`, or from a foreground command that /// was moved to the background when it exceeded its `timeout_ms`. pub fn definition( bg_runtime : @bgjobs.BgJobRuntime, ) -> @agent_tool.AgentToolDefinition { AgentToolDefinition( - name="shell_output", + name="job_output", description=( - #|Read a background shell job's recent output and status by its job_id (returned by - #|shell run_in_background, or by a command moved to the background on timeout). + #|Read a background job's recent output and status by its job_id (returned by + #|run_moonbit run_in_background, or by a command moved to the background on timeout). #|Returns the most recent window of captured output (with truncation metadata naming #|the total size when output exceeds the window) and whether the job is still #|running. If you need the job's result before you can continue and have no other @@ -42,7 +42,7 @@ async fn execute( ) -> @agent_tool.ToolAction { guard arguments is { "job_id": String(job_id), .. } else { return @agent_tool.ToolAction::respond( - "error: shell_output requires a string job_id", + "error: job_output requires a string job_id", is_error=true, ) } @@ -92,29 +92,32 @@ async fn execute( Stopped => ("stopped", true) } // Preserve the foreground path's error semantics for a job read later: binary - // (non-UTF-8) output and a sandboxed source-write denial are tool errors even - // when the command exited 0. Scan the *full* retained output for a denial (the - // denial line can be earlier than the displayed tail), but only for a sandboxed - // job — so a full read is done only in that rare case. - let sandbox_feedback = match snapshot.sandboxed_command { - Some(command) => { - let full = bg_runtime.read_output(job_id).unwrap_or(output) - if command.output_reports_denial(full) { - Some( - @shell.contextual_source_write_denial_feedback( - command, - full, - read_only_review=snapshot.read_only_review, - ), - ) - } else { - None - } - } - None => None + // (non-UTF-8) output and a refused access are tool errors even when the + // command exited 0. + // + // The moonrun policy refuses on every platform and for every job, so its + // wording is looked for first — in the displayed tail, which is where an + // uncaught refusal lands because it is what killed the job. A refusal the job + // caught and then outran can scroll out of that window; the foreground path's + // full-log scan is what covers that case, and it is not worth a full read of + // every job's output here. + // + // A profile denial is then scanned for across the *full* retained output (the + // denial line can be earlier than the tail), but only for a sandboxed job — so + // a full read happens only in that rarer case. + let sandbox_denied = if @sandbox.output_reports_policy_refusal(output) { + Some(@sandbox.policy_refusal_feedback) + } else if snapshot.sandboxed_command is Some(command) && + command.output_reports_denial( + bg_runtime.read_output(job_id).unwrap_or(output), + ) { + Some(command.denial_feedback()) + } else { + None } - let sandbox_denied = sandbox_feedback is Some(_) - let is_error = exit_error || snapshot.had_invalid_utf8 || sandbox_denied + let is_error = exit_error || + snapshot.had_invalid_utf8 || + sandbox_denied is Some(_) // Truncated whenever what we show is less than what the job produced — this // covers the window (large output), a memory-only runtime that dropped output // past the budget, and a hard-cap drop — so a prefix is never presented as the @@ -145,8 +148,8 @@ async fn execute( content.write_string("\n") } } - if sandbox_feedback is Some(feedback) { - content.write_string(feedback) + if sandbox_denied is Some(guidance) { + content.write_string(guidance) content.write_string("\n") } content.write_string(footer) @@ -155,7 +158,7 @@ async fn execute( } ///| -/// Longest a single shell_output call may block awaiting a job, so one tool +/// Longest a single job_output call may block awaiting a job, so one tool /// call cannot hold the turn indefinitely; the model can always call again. let max_wait_ms : Int = 120_000 @@ -167,7 +170,7 @@ fn wait_ms(arguments : Json) -> Result[Int?, String] { { "wait_ms": Number(value, ..), .. } => { let wait = value.to_int() if wait <= 0 { - Err("error: shell_output wait_ms must be a positive number") + Err("error: job_output wait_ms must be a positive number") } else if wait > max_wait_ms { Ok(Some(max_wait_ms)) } else { @@ -178,7 +181,7 @@ fn wait_ms(arguments : Json) -> Result[Int?, String] { // Present but not a number (e.g. a stringified "30000"): reject rather than // silently skip the wait — the local dispatcher does not enforce the JSON // schema, so the decoder must. - { "wait_ms": _, .. } => Err("error: shell_output wait_ms must be a number") + { "wait_ms": _, .. } => Err("error: job_output wait_ms must be a number") _ => Ok(None) } } diff --git a/agent_tool/shell_output/shell_output_test.mbt b/agent_tool/job_output/job_output_test.mbt similarity index 81% rename from agent_tool/shell_output/shell_output_test.mbt rename to agent_tool/job_output/job_output_test.mbt index e75f4b758..2c56baf81 100644 --- a/agent_tool/shell_output/shell_output_test.mbt +++ b/agent_tool/job_output/job_output_test.mbt @@ -1,13 +1,13 @@ ///| -/// Execute the shell_output tool and unwrap its Respond output. +/// Execute the job_output tool and unwrap its Respond output. #cfg(not(platform="windows")) -async fn run_shell_output( +async fn run_job_output( bg : @bgjobs.BgJobRuntime, arguments : Json, ) -> @agent_tool.ToolOutput { - let action = match @shell_output.definition(bg).execute { + let action = match @job_output.definition(bg).execute { Async(execute) => execute(arguments) - Sync(_) => fail("shell_output should be async") + Sync(_) => fail("job_output should be async") } guard action is Respond(output) else { fail("expected a Respond action") } output @@ -28,7 +28,7 @@ async fn poll_bg_terminal(bg : @bgjobs.BgJobRuntime, id : String) -> Unit { ///| #cfg(not(platform="windows")) -async test "shell_output returns a running job's output and status" { +async test "job_output returns a running job's output and status" { @async.with_task_group(group => { let bg = @bgjobs.BgJobRuntime::new(AgentTaskScope(group)) let id = bg.start( @@ -37,7 +37,7 @@ async test "shell_output returns a running job's output and status" { command="job", ) @async.sleep(100) - let output = run_shell_output(bg, { "job_id": id }) + let output = run_job_output(bg, { "job_id": id }) assert_false(output.is_error) assert_true(output.content.contains("hi")) assert_true(output.content.contains("running")) @@ -47,12 +47,12 @@ async test "shell_output returns a running job's output and status" { ///| #cfg(not(platform="windows")) -async test "shell_output reports a finished job's exit code" { +async test "job_output reports a finished job's exit code" { @async.with_task_group(group => { let bg = @bgjobs.BgJobRuntime::new(AgentTaskScope(group)) let id = bg.start(program="sh", args=["-c", "exit 2"], command="job") poll_bg_terminal(bg, id) - let output = run_shell_output(bg, { "job_id": id }) + let output = run_job_output(bg, { "job_id": id }) assert_true(output.is_error) assert_true(output.content.contains("exit=2")) }) @@ -60,7 +60,7 @@ async test "shell_output reports a finished job's exit code" { ///| #cfg(not(platform="windows")) -async test "shell_output windows a large job's output and notes truncation" { +async test "job_output windows a large job's output and notes truncation" { @async.with_task_group(group => { let dir = @fs.tmpdir(prefix="openseek-test-") let bg = @bgjobs.BgJobRuntime::new(AgentTaskScope(group), spill_dir=dir) @@ -71,7 +71,7 @@ async test "shell_output windows a large job's output and notes truncation" { max_output_chars=100, ) poll_bg_terminal(bg, id) - let output = run_shell_output(bg, { "job_id": id }) + let output = run_job_output(bg, { "job_id": id }) // Bounded to the window, with truncation metadata, but still showing the // recent (tail) output — here the end of the sequence. assert_true(output.content.contains("truncated=true")) @@ -85,7 +85,7 @@ async test "shell_output windows a large job's output and notes truncation" { ///| #cfg(not(platform="windows")) -async test "shell_output flags dropped output for a memory-only job" { +async test "job_output flags dropped output for a memory-only job" { @async.with_task_group(group => { // Memory-only runtime (no spill dir): output past the budget is dropped. let bg = @bgjobs.BgJobRuntime::new(AgentTaskScope(group)) @@ -96,7 +96,7 @@ async test "shell_output flags dropped output for a memory-only job" { max_output_chars=100, ) poll_bg_terminal(bg, id) - let output = run_shell_output(bg, { "job_id": id }) + let output = run_job_output(bg, { "job_id": id }) // Only ~100 chars retained but the job produced far more — must not present // the prefix as complete. assert_true(output.content.contains("truncated=true")) @@ -105,7 +105,7 @@ async test "shell_output flags dropped output for a memory-only job" { ///| #cfg(not(platform="windows")) -async test "shell_output marks binary background output as a tool error" { +async test "job_output marks binary background output as a tool error" { @async.with_task_group(group => { let bg = @bgjobs.BgJobRuntime::new(AgentTaskScope(group)) // A background command with non-UTF-8 output that exits 0 must still be a @@ -116,7 +116,7 @@ async test "shell_output marks binary background output as a tool error" { command="binary", ) poll_bg_terminal(bg, id) - let output = run_shell_output(bg, { "job_id": id }) + let output = run_job_output(bg, { "job_id": id }) assert_true(output.is_error) assert_true(output.content.contains("binary_output")) }) @@ -124,10 +124,10 @@ async test "shell_output marks binary background output as a tool error" { ///| #cfg(not(platform="windows")) -async test "shell_output errors on an unknown job id" { +async test "job_output errors on an unknown job id" { @async.with_task_group(group => { let bg = @bgjobs.BgJobRuntime::new(AgentTaskScope(group)) - let output = run_shell_output(bg, { "job_id": "nope" }) + let output = run_job_output(bg, { "job_id": "nope" }) assert_true(output.is_error) assert_true(output.content.contains("no background job")) }) @@ -144,7 +144,7 @@ async test "wait_ms blocks until the job finishes and returns its output" { command="waited", ) // No polling loop: a single call awaits the exit and returns the result. - let output = run_shell_output(bg, { "job_id": id, "wait_ms": 30000 }) + let output = run_job_output(bg, { "job_id": id, "wait_ms": 30000 }) assert_false(output.is_error) assert_true(output.content.contains("WAITED-OK")) assert_true(output.content.contains("exit=0")) @@ -157,7 +157,7 @@ async test "a wait_ms deadline on a still-running job is not an error" { @async.with_task_group(group => { let bg = @bgjobs.BgJobRuntime::new(AgentTaskScope(group)) let id = bg.start(program="sh", args=["-c", "sleep 30"], command="slow") - let output = run_shell_output(bg, { "job_id": id, "wait_ms": 200 }) + let output = run_job_output(bg, { "job_id": id, "wait_ms": 200 }) // Deadline expiry just reports the live status; the model can call again. assert_false(output.is_error) assert_true(output.content.contains("running")) @@ -171,7 +171,7 @@ async test "a non-positive wait_ms is a tool error" { @async.with_task_group(group => { let bg = @bgjobs.BgJobRuntime::new(AgentTaskScope(group)) let id = bg.start(program="sh", args=["-c", "true"], command="noop") - let output = run_shell_output(bg, { "job_id": id, "wait_ms": -5 }) + let output = run_job_output(bg, { "job_id": id, "wait_ms": -5 }) assert_true(output.is_error) assert_true(output.content.contains("wait_ms")) }) @@ -183,7 +183,7 @@ async test "a non-numeric wait_ms is a tool error, not silently ignored" { @async.with_task_group(group => { let bg = @bgjobs.BgJobRuntime::new(AgentTaskScope(group)) let id = bg.start(program="sh", args=["-c", "true"], command="noop") - let output = run_shell_output(bg, { "job_id": id, "wait_ms": "30000" }) + let output = run_job_output(bg, { "job_id": id, "wait_ms": "30000" }) assert_true(output.is_error) assert_true(output.content.contains("wait_ms must be a number")) }) diff --git a/agent_tool/shell_output/moon.pkg b/agent_tool/job_output/moon.pkg similarity index 88% rename from agent_tool/shell_output/moon.pkg rename to agent_tool/job_output/moon.pkg index 12f2a0349..f603e9f76 100644 --- a/agent_tool/shell_output/moon.pkg +++ b/agent_tool/job_output/moon.pkg @@ -2,7 +2,6 @@ import { "bobzhang/openseek/agent_tool", "bobzhang/openseek/agent_tool/bgjobs", "bobzhang/openseek/agent_tool/internal/sandbox", - "bobzhang/openseek/agent_tool/shell", "moonbitlang/core/json", } diff --git a/agent_tool/shell_stop/pkg.generated.mbti b/agent_tool/job_output/pkg.generated.mbti similarity index 84% rename from agent_tool/shell_stop/pkg.generated.mbti rename to agent_tool/job_output/pkg.generated.mbti index aa313c48b..18518a3f8 100644 --- a/agent_tool/shell_stop/pkg.generated.mbti +++ b/agent_tool/job_output/pkg.generated.mbti @@ -1,5 +1,5 @@ // Generated using `moon info`, DON'T EDIT IT -package "bobzhang/openseek/agent_tool/shell_stop" +package "bobzhang/openseek/agent_tool/job_output" import { "bobzhang/openseek/agent_tool", diff --git a/agent_tool/shell_stop/README.mbt.md b/agent_tool/job_stop/README.mbt.md similarity index 50% rename from agent_tool/shell_stop/README.mbt.md rename to agent_tool/job_stop/README.mbt.md index f6c73c04f..d8af5cc44 100644 --- a/agent_tool/shell_stop/README.mbt.md +++ b/agent_tool/job_stop/README.mbt.md @@ -1,26 +1,25 @@ -# Shell Stop Tool +# Job Stop Tool -`shell_stop` cancels a running background shell job by its `job_id` — the id -returned by `shell` with `run_in_background=true`, or by a foreground command -that was moved to the background when it outlived its `timeout_ms`. +`job_stop` cancels a running background job by its `job_id` — the id +`run_moonbit` returns when called with `run_in_background=true`. -Stopping is a request against the shared `ShellExecution`: the child process is -cancelled and the job lands on the `Stopped` status, which `shell_output` +Stopping is a request against the job's shared execution: the child process is +cancelled and the job lands on the `Stopped` status, which `job_output` reports as a tool error thereafter. Stopping an already-finished job is a *successful* no-op — the id is known, there is just nothing left to cancel — -so `shell_stop` is idempotent; only an unknown id is a tool error +so `job_stop` is idempotent; only an unknown id is a tool error (`no background job with id …`). ## Design Rationale - **A requested stop produces no completion notice.** The push-completion watcher only announces jobs that end on their own (natural exit, or the - output-limit watchdog); the model that called `shell_stop` already has the + output-limit watchdog); the model that called `job_stop` already has the acknowledgment in the tool result, so a notice would be noise. - **Cancellation reaches the direct child only.** The process library exposes - no process-group kill, so a command that daemonized its own children can - leave descendants running after the job is reported stopped — the same - limitation as foreground cancellation, documented rather than hidden. + no process-group kill, and the direct child here is the snippet's `moon run`, + so processes it spawned in turn can outlive a stop — the same limitation as + foreground cancellation, documented rather than hidden. - Session teardown stops all jobs the same way: every job's direct child is spawned on the session task group, so it is cancelled when the session ends — with the same direct-child-only limitation as above: daemonized descendants diff --git a/agent_tool/shell_stop/shell_stop.mbt b/agent_tool/job_stop/job_stop.mbt similarity index 69% rename from agent_tool/shell_stop/shell_stop.mbt rename to agent_tool/job_stop/job_stop.mbt index 602362f03..5505927bb 100644 --- a/agent_tool/shell_stop/shell_stop.mbt +++ b/agent_tool/job_stop/job_stop.mbt @@ -1,14 +1,13 @@ ///| -/// Tool `shell_stop`: cancel a running background shell job by its job_id. +/// Tool `job_stop`: cancel a running background job by its job_id. pub fn definition( bg_runtime : @bgjobs.BgJobRuntime, ) -> @agent_tool.AgentToolDefinition { AgentToolDefinition( - name="shell_stop", + name="job_stop", description=( - #|Stop a running background shell job by its job_id (from shell run_in_background, - #|or a command moved to the background on timeout). Cancels the process; a job that - #|already finished is a no-op. + #|Stop a running background job by its job_id (from run_moonbit's + #|run_in_background). Cancels the process; a job that already finished is a no-op. ), schema=JsonSchema({ "type": "object", @@ -26,7 +25,7 @@ async fn execute( ) -> @agent_tool.ToolAction { guard arguments is { "job_id": String(job_id), .. } else { return @agent_tool.ToolAction::respond( - "error: shell_stop requires a string job_id", + "error: job_stop requires a string job_id", is_error=true, ) } diff --git a/agent_tool/shell_stop/shell_stop_test.mbt b/agent_tool/job_stop/job_stop_test.mbt similarity index 78% rename from agent_tool/shell_stop/shell_stop_test.mbt rename to agent_tool/job_stop/job_stop_test.mbt index f0a8c5dd2..b6090f936 100644 --- a/agent_tool/shell_stop/shell_stop_test.mbt +++ b/agent_tool/job_stop/job_stop_test.mbt @@ -1,14 +1,14 @@ ///| #cfg(not(platform="windows")) -async test "shell_stop cancels a running job" { +async test "job_stop cancels a running job" { @async.with_task_group(group => { let bg = @bgjobs.BgJobRuntime::new(AgentTaskScope(group)) let id = bg.start(program="sleep", args=["30"], command="sleep 30") @async.sleep(50) assert_true(bg.snapshot(id).unwrap().status is Running) - let action = match @shell_stop.definition(bg).execute { + let action = match @job_stop.definition(bg).execute { Async(execute) => execute({ "job_id": id }) - Sync(_) => fail("shell_stop should be async") + Sync(_) => fail("job_stop should be async") } guard action is Respond(output) else { fail("expected a Respond action") } assert_false(output.is_error) @@ -19,12 +19,12 @@ async test "shell_stop cancels a running job" { ///| #cfg(not(platform="windows")) -async test "shell_stop errors on an unknown job id" { +async test "job_stop errors on an unknown job id" { @async.with_task_group(group => { let bg = @bgjobs.BgJobRuntime::new(AgentTaskScope(group)) - let action = match @shell_stop.definition(bg).execute { + let action = match @job_stop.definition(bg).execute { Async(execute) => execute({ "job_id": "nope" }) - Sync(_) => fail("shell_stop should be async") + Sync(_) => fail("job_stop should be async") } guard action is Respond(output) else { fail("expected a Respond action") } assert_true(output.is_error) diff --git a/agent_tool/shell_stop/moon.pkg b/agent_tool/job_stop/moon.pkg similarity index 100% rename from agent_tool/shell_stop/moon.pkg rename to agent_tool/job_stop/moon.pkg diff --git a/agent_tool/shell_output/pkg.generated.mbti b/agent_tool/job_stop/pkg.generated.mbti similarity index 84% rename from agent_tool/shell_output/pkg.generated.mbti rename to agent_tool/job_stop/pkg.generated.mbti index 5d72248e5..4d2ddea18 100644 --- a/agent_tool/shell_output/pkg.generated.mbti +++ b/agent_tool/job_stop/pkg.generated.mbti @@ -1,5 +1,5 @@ // Generated using `moon info`, DON'T EDIT IT -package "bobzhang/openseek/agent_tool/shell_output" +package "bobzhang/openseek/agent_tool/job_stop" import { "bobzhang/openseek/agent_tool", diff --git a/agent_tool/run_moonbit/internal/decode/decode.mbt b/agent_tool/run_moonbit/internal/decode/decode.mbt index e2d39b2f9..72a9ddc19 100644 --- a/agent_tool/run_moonbit/internal/decode/decode.mbt +++ b/agent_tool/run_moonbit/internal/decode/decode.mbt @@ -10,32 +10,42 @@ pub struct RunMoonbitInput { /// Whether compiler warnings appear in the run output — decoded from the /// `"on"`/`"off"` tool argument, defaulting to `"off"` (suppressed). warning : Bool + /// Whether to detach the run as a background job instead of waiting for it. + /// Only meaningful when the tool was built with a job runtime. + run_in_background : Bool } derive(Debug) ///| -/// The moon backends `--target` accepts. `native` is the default because the -/// async IO batteries (`@fs`, `@stdio`, `@process`) require it. +/// The moon backends `--target` accepts. fn valid_target(target : String) -> Bool { - target is ("native" | "wasm" | "wasm-gc" | "js" | "llvm") + // `native` is deliberately absent: moonrun's policy — the only confinement + // that holds off macOS — applies to the wasm backend alone, and + // `--wasm-policy` is silently ignored elsewhere. Letting a snippet name + // `native` would hand it a one-argument way out of the sandbox. The + // remaining alternatives cannot spawn or touch the filesystem at all. + target is ("wasm" | "wasm-gc" | "js" | "llvm") } ///| /// Decode `run_moonbit` tool-call arguments. `source` is a required string /// holding a full `.mbtx` program (an optional leading `import { … }` block, /// then the program including its own `main`). `target` is an optional backend -/// name defaulting to `native`. `cwd` is an optional working directory for the -/// program (defaults to the workspace root). Failures name the offending field. +/// name defaulting to `wasm`, which is the backend moonrun's policy sandbox +/// applies to — a snippet may not simply ask for an unsandboxed one, so the +/// compute-only backends are the only alternatives offered. `cwd` is an +/// optional working directory for the program (defaults to the workspace +/// root). Failures name the offending field. pub fn decode(arguments : Json) -> RunMoonbitInput raise { match arguments { { "source": String(source), .. } => { let target = match arguments { { "target": String(t), .. } => t - { "target": Null, .. } => "native" + { "target": Null, .. } => "wasm" { "target": _, .. } => fail("arguments.target to be a string") - _ => "native" + _ => "wasm" } guard valid_target(target) else { - fail("arguments.target to be one of native, wasm, wasm-gc, js, llvm") + fail("arguments.target to be one of wasm, wasm-gc, js, llvm") } let cwd = match arguments { { "cwd": String(c), .. } => Some(c) @@ -51,7 +61,15 @@ pub fn decode(arguments : Json) -> RunMoonbitInput raise { fail("arguments.warning to be \"on\" or \"off\"") _ => false } - { source, target, cwd, warning } + let run_in_background = match arguments { + { "run_in_background": True, .. } => true + { "run_in_background": False, .. } + | { "run_in_background": Null, .. } => false + { "run_in_background": _, .. } => + fail("arguments.run_in_background to be a boolean") + _ => false + } + { source, target, cwd, warning, run_in_background } } Object(_) => fail("arguments.source") _ => fail("object arguments") @@ -59,10 +77,26 @@ pub fn decode(arguments : Json) -> RunMoonbitInput raise { } ///| -test "decode reads source with the default native target" { +test "decode reads source with the default wasm target" { let input = decode({ "source": "fn main { println(1) }" }) assert_eq(input.source, "fn main { println(1) }") - assert_eq(input.target, "native") + assert_eq(input.target, "wasm") +} + +///| +/// `native` is the one backend a snippet may not ask for: moonrun's policy — +/// the confinement that holds on every platform — applies to wasm alone, and +/// `--wasm-policy` is silently ignored elsewhere, so naming `native` would be +/// a one-argument way out of the sandbox. +test "decode refuses the unsandboxed native target" { + let refused = try { + decode({ "source": "fn main { }", "target": "native" }) |> ignore + "accepted" + } catch { + error => "\{error}" + } + assert_false(refused == "accepted") + assert_true(refused.contains("wasm")) } ///| diff --git a/agent_tool/run_moonbit/internal/decode/pkg.generated.mbti b/agent_tool/run_moonbit/internal/decode/pkg.generated.mbti index 36bce5a74..f4df694c3 100644 --- a/agent_tool/run_moonbit/internal/decode/pkg.generated.mbti +++ b/agent_tool/run_moonbit/internal/decode/pkg.generated.mbti @@ -16,6 +16,7 @@ pub struct RunMoonbitInput { target : String cwd : String? warning : Bool + run_in_background : Bool } derive(@debug.Debug) pub fn RunMoonbitInput::to_repr(Self) -> @debug.Repr diff --git a/agent_tool/run_moonbit/moon.pkg b/agent_tool/run_moonbit/moon.pkg index 2f823d927..033b7ac9f 100644 --- a/agent_tool/run_moonbit/moon.pkg +++ b/agent_tool/run_moonbit/moon.pkg @@ -1,5 +1,6 @@ import { "bobzhang/openseek/agent_tool", + "bobzhang/openseek/agent_tool/bgjobs", "bobzhang/openseek/agent_tool/run_moonbit/internal/decode", "bobzhang/openseek/agent_tool/internal/error" @tool_error, "bobzhang/openseek/agent_tool/internal/sandbox", diff --git a/agent_tool/run_moonbit/pkg.generated.mbti b/agent_tool/run_moonbit/pkg.generated.mbti index dc38ef028..103185151 100644 --- a/agent_tool/run_moonbit/pkg.generated.mbti +++ b/agent_tool/run_moonbit/pkg.generated.mbti @@ -3,14 +3,20 @@ package "bobzhang/openseek/agent_tool/run_moonbit" import { "bobzhang/openseek/agent_tool", + "bobzhang/openseek/agent_tool/bgjobs", } // Values -pub fn definition(workspace_root~ : String) -> @agent_tool.AgentToolDefinition +pub fn definition(workspace_root~ : String, read_only? : Bool, bg_runtime? : @bgjobs.BgJobRuntime, job_dir? : String, scratch_dir? : String, worker_sandbox? : WorkerSandbox, run_timeout_ms? : Int) -> @agent_tool.AgentToolDefinition // Errors // Types and methods +pub(all) struct WorkerSandbox { + deny_roots : Array[String] + worker_root : String + worker_admin_dir : String +} // Type aliases diff --git a/agent_tool/run_moonbit/run_moonbit.mbt b/agent_tool/run_moonbit/run_moonbit.mbt index fa4d20bc5..9b3c5d14b 100644 --- a/agent_tool/run_moonbit/run_moonbit.mbt +++ b/agent_tool/run_moonbit/run_moonbit.mbt @@ -1,31 +1,21 @@ ///| /// A snippet is bounded to this wall-clock so a runaway loop or a build that /// never finishes cannot block the whole turn. On expiry the run is cancelled -/// (which tears down the moon subprocess) and reported as a timeout. -const RunTimeoutMs : Int = 60_000 +/// (which tears down the moon subprocess) and reported as a timeout. Sized for +/// commands, not just compute: a snippet runs whole `moon test` cycles, and a +/// fresh workspace's first build downloads and compiles its dependencies. +const RunTimeoutMs : Int = 300_000 ///| /// Output is streamed to a file on disk, not buffered in memory, so a snippet /// that floods stdout (e.g. `while true { println(…) }`) cannot exhaust the -/// agent's memory before the timeout fires. Only this many bytes are read back -/// — the rest is dropped with a truncation marker. +/// agent's memory before the timeout fires. A FOREGROUND run is STOPPED once it +/// passes this, rather than truncated — the runtime spawns it with +/// `kill_when_full`, so a clipped head means a killed program and the exit code +/// is not the program's own. Detaching clears that, so a background job is +/// bounded by the hard cap instead and runs to its own end. const OutputCapBytes : Int = 48_000 -///| -/// Guidance appended when a sandboxed run's output shows a source-write denial -/// ("Operation not permitted" on a protected source path). Without it the model -/// reads a bare OS error and debugs the filesystem — or worse, tries to escape -/// the sandbox — instead of using `edit`. -const SandboxSourceWriteFeedback : String = - #|run_moonbit sandbox: the "Operation not permitted" failure above is this tool's - #|sandbox denying a write to a protected MoonBit source file (`*.mbt`, `*.mbti`, - #|`*.mbt.md`, `moon.mod`/`moon.pkg`/`moon.work`) — not a filesystem or permissions - #|problem to debug. Snippets run source-write-readonly by design; writing non-source - #|files (data, logs, reports) stays allowed. Do NOT try to work around the block from - #|inside a snippet (renames, copies, temp-then-move, or other sandbox escapes) — make - #|source changes with the line-anchored `edit` tool instead, and keep run_moonbit for - #|compute, probing, and non-source IO. - ///| /// The `--warn-list` spec for `warning: "off"`. `-a` disables all warnings; /// each `+name@name` pair then re-enables a selected correctness diagnostic @@ -34,19 +24,17 @@ const SandboxSourceWriteFeedback : String = const WarnOffList : String = "-a+partial_match@partial_match+multiline_string_escape@multiline_string_escape+invalid_inline_wasm@invalid_inline_wasm+unannotated_ffi@unannotated_ffi" ///| -/// Best-effort refusal of the two obvious native escapes: process spawning -/// (`moonbitlang/async/process` — the `shell` tool owns commands) and native -/// FFI (`extern`, which can bind libc `system`). A substring scan is NOT a -/// real boundary — a determined native snippet can still escape — so HARD -/// containment is deferred to the planned wasm backend (safe by construction). -/// Until then this closes the trivial escapes and keeps descendant processes -/// from outliving the timeout. Returns the reason to report, or None. +/// Best-effort refusal of native FFI (`extern`, which can bind libc `system` +/// and sidesteps every guard above it). Spawning processes is no longer an +/// escape — it is what this tool is for — but FFI still is. A substring scan is +/// NOT a real boundary; HARD containment is deferred to the planned wasm +/// backend (safe by construction). Returns the reason to report, or None. fn native_escape(source : String) -> String? { - if source.contains("moonbitlang/async/process") { - Some( - "spawns processes (moonbitlang/async/process) — use the shell tool for commands", - ) - } else if source.contains("extern") { + // Match the FFI syntax (`extern "c" fn …`), not the bare word. Scanning for + // `extern` alone refused a snippet whose only offence was a commit message + // mentioning it — the word appears in prose and string literals, the quote + // that follows it does not. + if source.contains("extern \"") { Some("uses native FFI (extern), which the wasm sandbox will contain later") } else { None @@ -81,66 +69,55 @@ fn native_escape(source : String) -> String? { /// `partial_match` keep failing compilation with warnings off (see /// `WarnOffList`). /// -/// Note: on macOS the run is wrapped in `sandbox-exec` with the shell tool's -/// source-write-readonly profile, so a snippet is blocked from directly writing -/// protected source files (`*.mbt`, `*.mbti`, `*.mbt.md`, `moon.mod/pkg/work`) -/// while non-source output stays permitted. This is best-effort, not a hard -/// boundary — an arbitrary snippet can still smuggle sources via directory -/// renames (which `shell` catches by preflighting its command text, impossible -/// for arbitrary code). Full capability containment is deferred to the planned -/// wasm backend, where the runtime is safe by construction. -pub fn definition(workspace_root~ : String) -> @agent_tool.AgentToolDefinition { +/// Confinement is moonrun's wasm policy, which applies on every platform. It +/// decides WHICH programs may start — an allowlist holding the moon/git/gh +/// toolchain and inert utilities, with no shell, interpreter, or ad-hoc +/// rewriter in it — and bounds the file access of the snippet itself. +/// +/// Its `fs` rules stop at the process boundary, so an admitted command writes +/// with the agent's own access. What is bounded is which commands exist at all, +/// and one of those bounds is per subcommand: the options that would point git +/// at another repository (`git -C`, `--git-dir`) precede the subcommand and so +/// match no allowlist prefix. +/// +/// Note what is deliberately NOT denied: source writes as such. The commands +/// whose job is to rewrite source (`moon fmt`, `moon info`, `git checkout`) run +/// normally, and the reason the old rule existed — an arbitrary program editing +/// files behind the model's back — is covered by the allowlist admitting no +/// such program. A read-only subrun is the exception, and the one role that +/// still takes a `sandbox-exec` profile: holding no editing tool is what defines +/// it, so `read_only=true` puts "may not write source" in the kernel. +pub fn definition( + workspace_root~ : String, + read_only? : Bool = false, + bg_runtime? : @bgjobs.BgJobRuntime, + job_dir? : String, + scratch_dir? : String, + worker_sandbox? : WorkerSandbox, + run_timeout_ms? : Int = RunTimeoutMs, +) -> @agent_tool.AgentToolDefinition { + // Background snippet directories are numbered inside the caller's scratch + // dir rather than created as fresh system temp dirs: a detached job outlives + // this call, so its snippet, build artifacts, and output log cannot be + // removed here — the caller's scratch dir is what eventually reclaims them. + let next_background = Ref(0) AgentToolDefinition( name="run_moonbit", - description=( - #|Compile and run a self-contained MoonBit program (a `.mbtx` single-file script) - #|and return its merged stdout/stderr and exit status. Use this for scripting - #|automation (read/transform files, parse JSON, compute) and for probing MoonBit - #|language behavior — PREFER it over shell python/node so the automation is MoonBit. - #|`source` may begin with an inline `import { "pkg", ... }` block (comma-separated - #|module paths such as `"moonbitlang/async", "moonbitlang/async/fs", - #|"moonbitlang/core/json"`) followed by the program including its own `main`; use - #|`async fn main` for IO (add `"moonbitlang/async"` to the import block for it). It - #|runs with your workspace as the working directory by default, so RELATIVE paths - #|like `@fs.read_file("data.json")` reach workspace files and files the program - #|writes land in the workspace; pass `cwd` to run elsewhere. Build artifacts stay in - #|a temp dir (your `_build` is untouched), and imports resolve against the registry - #|(NOT your uncommitted edits) — to exercise working-tree code, add a `*_test.mbt` - #|to that package and run `moon test` via shell instead. Compiler warnings are - #|suppressed by default (throwaway scripts trip unused-value noise constantly); pass - #|`warning: "on"` when the warnings are what you want to see. Bounded to 60s. When - #|probing language or API behavior, don't probe serially one turn at a time: emit - #|SEVERAL independent run_moonbit calls in the SAME assistant turn — one small - #|program per hypothesis — and read all the results together; batching saves a full - #|model round-trip per hypothesis and keeps each probe minimal instead of one - #|growing mega-program. - ), - schema=JsonSchema({ - "type": "object", - "properties": { - "source": { "type": "string" }, - "target": { - "type": "string", - "enum": ["native", "wasm", "wasm-gc", "js", "llvm"], - "description": "Backend to run on; defaults to native (required for the async @fs/@stdio/@process IO batteries).", - }, - "cwd": { - "type": "string", - "description": "Working directory the program runs in (relative paths resolve against the workspace root); defaults to the workspace root.", - }, - "warning": { - "type": "string", - "enum": ["off", "on"], - "description": ( - #|Whether compiler warnings appear in the output; defaults to off (a snippet - #|is a throwaway script, so unused-variable style noise is suppressed). Set - #|"on" only when the warnings themselves are what you are probing. - ), - }, - }, - "required": ["source"], + description=description(background=bg_runtime is Some(_)), + schema=JsonSchema(schema(background=bg_runtime is Some(_))), + execute=Async(arguments => { + execute( + workspace_root, + arguments, + read_only~, + bg_runtime?, + job_dir?, + scratch_dir?, + worker_sandbox?, + run_timeout_ms~, + next_background~, + ) }), - execute=Async(arguments => execute(workspace_root, arguments)), ) } @@ -148,6 +125,15 @@ pub fn definition(workspace_root~ : String) -> @agent_tool.AgentToolDefinition { async fn execute( workspace_root : String, arguments : Json, + read_only~ : Bool, + bg_runtime? : @bgjobs.BgJobRuntime, + job_dir? : String, + scratch_dir? : String, + worker_sandbox? : WorkerSandbox, + // The wall-clock bound, injectable so a test can exercise the deadline + // without waiting out the real one. + run_timeout_ms~ : Int, + next_background~ : Ref[Int], ) -> @agent_tool.ToolAction { let input = @decode.decode(arguments) catch { error => @@ -164,7 +150,7 @@ async fn execute( } // The program's working directory (default: the workspace root, so relative // `@fs` paths reach workspace files). An explicit relative `cwd` resolves - // against the workspace, like the shell tool. + // against the workspace, as every workspace-relative tool path does. let run_cwd = @workspace_path.resolve_cwd(workspace_root, input.cwd) if run_cwd is Some(c) { guard @fs.exists(c) else { @@ -189,20 +175,73 @@ async fn execute( // a possibly-large output log when a turn is cancelled mid-run. The timeout // wraps only the process body; output is redirected to a file (not buffered in // memory) and read back capped, so a flood cannot exhaust memory. - let dir = @fs.tmpdir(prefix="openseek-run-moonbit-") catch { - // Let a cancellation propagate to the loop's Interrupted terminal instead of - // swallowing it into an ordinary tool error. - error if @async.is_being_cancelled() => raise error - _ => - return @agent_tool.ToolAction::respond( - "run_moonbit: could not create a temporary working directory.", - is_error=true, - ) + if input.run_in_background && bg_runtime is None { + return @agent_tool.ToolAction::respond( + "error: run_in_background is not available in this context", + is_error=true, + ) + } + // A directory a job may take over must outlive this call, so it is numbered + // inside the caller's job dir — reclaimed when the session's scope ends — + // rather than a system temp dir that only this function ever revisits. That + // covers BOTH ways a job takes over: an explicit `run_in_background`, and a + // foreground run detached at its deadline, which is why this does not test + // `run_in_background`. + let background = input.run_in_background + let dir = if job_dir is Some(scratch) { + let dir = "\{scratch}/snippet-\{next_background.val}" + next_background.val += 1 + @fs.mkdir(dir, recursive=true) catch { + error if @async.is_being_cancelled() => raise error + _ => + return @agent_tool.ToolAction::respond( + "run_moonbit: could not create a working directory for the background job.", + is_error=true, + ) + } + dir + } else if scratch_dir is Some(lab) { + // With a lab wired, the snippet builds INSIDE it: the profile re-allows + // exactly one subtree, and pointing it at the lab then covers both the + // lab writes the caller sanctioned and moon's own build output. + let dir = "\{lab}/run-moonbit-\{next_background.val}" + next_background.val += 1 + @fs.mkdir(dir, recursive=true) catch { + error if @async.is_being_cancelled() => raise error + _ => + return @agent_tool.ToolAction::respond( + "run_moonbit: could not create a working directory in the scratch lab.", + is_error=true, + ) + } + dir + } else { + @fs.tmpdir(prefix="openseek-run-moonbit-") catch { + // Let a cancellation propagate to the loop's Interrupted terminal instead of + // swallowing it into an ordinary tool error. + error if @async.is_being_cancelled() => raise error + _ => + return @agent_tool.ToolAction::respond( + "run_moonbit: could not create a temporary working directory.", + is_error=true, + ) + } } + // Set once a background job takes over the directory (snippet, artifacts, + // output log): either an explicit `run_in_background`, or a foreground run + // detached at its deadline. `cleanup` runs on every exit path, so the flag — + // not the request — decides whether reclaiming the directory is still ours. + // + // Handing it over is sound because a job only ever runs with a `job_dir` + // wired, and that directory is reclaimed with the session's scope. The + // `@fs.tmpdir` branch above has no such owner, but it is only taken when the + // caller wired no job dir — which happens only if the session's own tmpdir + // call failed, in which case the one above fails too and returns first. + let job_owns_dir = Ref(false) async fn cleanup() -> Unit { - // Shielded from cancellation so an interrupted run still removes its temp - // dir — the `defer` below runs on the cancellation path too, where every - // async call inside it would otherwise be cancelled on the spot. + if job_owns_dir.val { + return + } @async.protect_from_cancel(() => { @fs.rmdir(dir, recursive=true) catch { _ => () @@ -230,6 +269,27 @@ async fn execute( // moon's diagnostics point at the copied source under the build dir; the // canonical path anchors the diagnostic path rewrite below. let real = @fs.realpath(dir) catch { _ => dir } + // The wasm backend is where moonrun's policy applies, so the policy file is + // written for every run and passed alongside. `--wasm-policy` is silently + // IGNORED on other backends, which is why `target` is not the model's choice + // to make freely — see `decode`. + let policy_path = "\{dir}/policy.json" + // Scratch space for the snippet's own `@fs.tmpdir()`, kept beside the build + // output rather than inside it so temp directories do not appear among moon's + // artifacts. It goes away with `dir` when the run is cleaned up. + let snippet_tmp = "\{dir}/tmp" + @fs.mkdir(snippet_tmp) catch { + error if @async.is_being_cancelled() => raise error + _ => () + } + // `cwd` is deliberately NOT a write root: naming where a program runs should + // not also grant writing there, and reads are open, so a snippet can still + // work in a directory it may not modify. + let policy = wasm_policy_document( + tmp_dir=snippet_tmp, + extra_roots=[..scratch_dir.map(l => [l]).unwrap_or([])], + ) + @fs.write_file(policy_path, policy.stringify(), create_mode=CreateOrTruncate) let moon_argv = [ "run", "\{dir}/snippet.mbtx", @@ -237,33 +297,230 @@ async fn execute( input.target, "--target-dir", dir, + ..if input.target == "wasm" { + ["--wasm-policy", policy_path] + }, ..if !input.warning { ["--warn-list", WarnOffList] }, ] - // Wrap `moon run` in sandbox-exec where it can actually enforce (macOS) so a - // snippet cannot directly overwrite protected source files — reusing the - // shell tool's source-write-readonly profile keyed on the workspace root. - // This is best-effort, not a hard boundary: unlike `shell` (which statically - // preflights the command) an arbitrary snippet can still smuggle sources via - // directory renames; full containment is the wasm backend's job. Elsewhere - // (or in a nested sandbox where it cannot enforce) run moon directly. - // The prepared command carries its denial subjects, so a denial showing up - // in the program's output can be recognized and explained below. The - // throwaway build dir is the writable subtree: if it falls INSIDE the + // Wrap `moon run` in sandbox-exec for the roles that need a rule the wasm + // policy cannot state: "may not write source". The policy's `fs` rules bind + // the snippet and stop at the process boundary, so a spawned `moon fmt` is + // beyond them, and a role defined by holding no editing tool needs that + // covered in the kernel. The main agent is not such a role and takes no + // profile — see below. Off macOS, or in a nested sandbox where a profile + // cannot be applied, every role runs with the policy as its only bound. + // The prepared command carries what it needs to classify its own denials, so + // one showing up in the program's output can be recognized and explained. + // The throwaway build dir is the writable subtree: if it falls INSIDE the // workspace root (e.g. `workspace_root=/tmp`, where `@fs.tmpdir` lands // under it) the shared deny would otherwise block moon's own generated // `snippet.mbt`. - let sandboxed_command = @sandbox.SandboxedCommand::create_if_available( - workspace_root, - Exec("moon", moon_argv), - writable_subtree=dir, - ) + // The writable subtree has to contain the snippet's own build output. With a + // lab wired the snippet builds inside it, so the lab covers both; otherwise + // the build dir is the subtree. Choosing by where `dir` actually sits keeps + // the two from disagreeing when a caller passes both a job dir and a lab. + let writable_subtree = match scratch_dir { + Some(lab) if dir.has_prefix(lab) => lab + _ => dir + } + let sandboxed_command = match worker_sandbox { + // Worker mode replaces the workspace source-write profile rather than + // adding to it: a worker is supposed to write source, just only its own, + // so the kernel denies its siblings' trees and the shared git dir instead + // of denying source writes as such. + Some(scope) => + @sandbox.SandboxedCommand::create_worker_if_available( + Exec("moon", moon_argv), + deny_roots=scope.deny_roots, + worker_root=scope.worker_root, + worker_admin_dir=scope.worker_admin_dir, + ) + // The read-only roles (explore/review/audit) hold no editing tool, so "may + // not write source" is the guarantee that defines them and it stays + // kernel-enforced. The profile suits them for a second reason — it is + // `(allow default)` minus source, so `moon check` can still write its + // target dir inside the workspace, which a boundary profile denying the + // workspace would take away. + // + // Read-only is its own parameter rather than being read off `scratch_dir`: + // a caller that forgets the lab should lose the lab, not the sandbox. + None if read_only => + @sandbox.SandboxedCommand::create_if_available( + workspace_root, + Exec("moon", moon_argv), + writable_subtree~, + ) + // The main agent runs with no kernel profile at all. Forbidding source + // writes here would break the very commands it needs — `moon fmt`, + // `moon info`, `git checkout` all rewrite source by definition — and the + // other shape, denying writes globally and re-allowing an enumerated set, + // was tried and withdrawn: every root it missed (`/dev` for git's + // `/dev/null`, `/tmp` for `@fs.tmpdir`, the toolchain root, gh's cache) + // surfaced as a bare EPERM in a live turn rather than in a test, and the + // list only ever protected macOS. + // + // What is left bounding a child is the allowlist, which works everywhere: + // it admits no shell, no interpreter and no ad-hoc rewriter, and it refuses + // the git options that would point a command at another repository. That is + // a bound on which commands exist, not on where they may write — the file + // tools are not path-confined for this role either, so a profile here would + // have been the only such rule in the tool set. + None => None + } let (prog, argv) = match sandboxed_command { Some(command) => (command.program(), command.args()) None => ("moon", moon_argv) } - let result = @async.with_timeout_opt(RunTimeoutMs, () => { + if background && bg_runtime is Some(runtime) { + // Compile in the FOREGROUND first. A detached run would otherwise report + // a syntax or type error only through job_output, and with the throwaway + // temp path instead of `source:LINE:COL` — the two things that make a + // compile error readable. `--build-only` is the single-file equivalent of + // a check: `moon check`/`moon build` do not accept a `.mbtx` script (the + // first reports "no work to do", the second demands a Moon project). + // TODO(upstream): teach `moon check` to accept a `.mbtx` script, and use + // it here — building is more work than this preflight needs. + let build_argv = [..argv] + build_argv.push("--build-only") + let build_log = "\{dir}/build.log" + @fs.write_file(build_log, "", create_mode=CreateOrTruncate) + // Bounded like every other run here: a first build downloads and compiles + // dependencies, and a wedged fetch would otherwise hold the turn open — + // exactly what asking for a background job was meant to avoid. + let build_exit = @async.with_timeout_opt(run_timeout_ms, () => { + @process.run( + prog, + build_argv, + cwd=run_cwd.unwrap_or("."), + inherit_env=true, + stdin=@process.redirect_from_file(empty_stdin), + stdout=@process.redirect_to_file(build_log, append=true), + stderr=@process.redirect_to_file(build_log, append=true), + no_console_window=true, + ) + }) + guard build_exit is Some(build_exit) else { + let text = rewrite_temp_paths(read_capped(build_log), [real, dir]) + return @agent_tool.ToolAction::respond( + "run_moonbit: the build did not finish within \{run_timeout_ms / 1000}s, so no background job was started.\n\{text}", + is_error=true, + brief="run_moonbit (build timeout)", + ) + } + if build_exit != 0 { + let text = rewrite_temp_paths(read_capped(build_log), [real, dir]) + return @agent_tool.ToolAction::respond( + "[run_moonbit: the program did not compile, so no background job was started]\n\{text}", + is_error=true, + brief="run_moonbit (build failed)", + ) + } + let id = runtime.start( + program=prog, + args=argv, + command="run_moonbit: \{@agent_tool.brief_line(input.source, limit=64)}", + cwd?=run_cwd, + sandboxed_command?, + // Same EOF-on-stdin contract as a foreground run: a detached snippet + // must not inherit the engine's fd 0, which under `serve` carries the + // JSONL commands. + stdin=@process.redirect_from_file(empty_stdin), + ) + job_owns_dir.val = true + return @agent_tool.ToolAction::respond( + "started background job \{id} (the program compiled). Read its output with job_output (job_id=\"\{id}\") and stop it with job_stop (job_id=\"\{id}\").", + brief="run_moonbit → bg \{id}", + ) + } + // With a job runtime wired the foreground run happens ON it, so the + // deadline can DETACH the still-running program as a background job rather + // than killing it: a long `moon test` that outlives the bound is then still + // there to read, which is what the shell tool did with its own deadline. + // Without a runtime (a standalone definition) there is nothing to hand the + // process to, so the deadline cancels as before. + if bg_runtime is Some(runtime) { + let exec = runtime.spawn_foreground_retained( + prog, + argv, + run_cwd, + OutputCapBytes, + // Same EOF-on-stdin contract as every other path here: never hand the + // snippet the engine's fd 0. + stdin=@process.redirect_from_file(empty_stdin), + ) + // A cancelled turn must not leak the child: stop it before unwinding. + errdefer exec.request_stop() + // `wait`, not `wait_or_invalid`. That one returns early on the first non-UTF-8 + // byte, for a caller that fails fast on binary output — and this is not one: + // reporting with the child still running would leave it going while + // `defer cleanup()` removes its build directory underneath it, and the exit + // code below would be invented. The exit is what this waits for either way, + // so asking for the early wake only put the rest of the wait outside the + // deadline. Expiry needs nothing special: it detaches like any other long run. + let finished = @async.with_timeout_opt(run_timeout_ms, () => exec.wait()) + let saw_binary = exec.had_invalid_utf8() + if finished is None { + let _ = exec.background() + let id = runtime.adopt( + exec, + command="run_moonbit: \{@agent_tool.brief_line(input.source, limit=64)}", + cwd?=run_cwd, + sandboxed_command?, + ) + job_owns_dir.val = true + return @agent_tool.ToolAction::respond( + "run_moonbit: still running after \{run_timeout_ms / 1000}s, so it moved to the background as job \{id} (nothing was lost). Read its output with job_output (job_id=\"\{id}\") and stop it with job_stop (job_id=\"\{id}\").", + brief="run_moonbit → bg \{id}", + ) + } + let raw = exec.head() + let exit_code = exec.exit_code().unwrap_or(-1) + // The retained execution is KILLED once output passes the inline cap, so a + // clipped head means a stopped program, not a quiet one — say which. + let output_limit_reached = exec.over_inline_cap() || exec.output_truncated() + // A denial can land past the rendered head — a snippet floods, THEN hits + // it — so scan the retained output on disk with the same bounded, + // chunked walk the redirected path uses. A memory-only execution (no + // session temp dir) has no such file, and there the head is all there is. + let denial_scan_source = exec.spill_path() + let sandbox_denied = refusal_guidance( + sandboxed_command, denial_scan_source, raw, + ) + exec.discard() + let text = rewrite_temp_paths(raw, [real, dir]) + let body = if text.is_blank() { + "run_moonbit: program exited \{exit_code} with no output" + } else if exit_code != 0 { + "[run_moonbit: exited \{exit_code}]\n\{text}" + } else { + text + } + let body = if output_limit_reached { + "\{body}\n[run_moonbit: output passed \{OutputCapBytes} bytes, so the program was STOPPED and this is only what it printed first. Redirect bulky output with `stdout=ToFile(path)` and read the file instead.]" + } else { + body + } + let body = if saw_binary { + "\{body}\n[run_moonbit: the program emitted non-UTF-8 bytes; the text above is a lossy rendering.]" + } else { + body + } + let body = if sandbox_denied is Some(guidance) { + "\{body}\n\n\{guidance}" + } else { + body + } + return @agent_tool.ToolAction::respond( + body, + is_error=exit_code != 0 || + sandbox_denied is Some(_) || + output_limit_reached, + brief="run_moonbit (exit=\{exit_code})", + ) + } + let result = @async.with_timeout_opt(run_timeout_ms, () => { let exit_code = @process.run( prog, argv, @@ -276,20 +533,13 @@ async fn execute( ) (exit_code, read_capped(log)) }) - // A "not permitted" line naming a protected source path in a sandboxed run - // is the source-write policy firing, not an environment fault: explain it - // (the raw OS error is otherwise a dead end) and treat the run as failed - // even if the snippet swallowed the error and exited 0 — the same - // semantics the shell tool applies to its command output. Detection scans - // the on-disk log far past the capped display prefix (bounded to a fixed - // head+tail byte budget), and runs for the timeout arm too: a snippet can - // flood past the cap before hitting the denial, or print a caught denial - // and then hang. - let sandbox_denied = sandboxed_command is Some(command) && - log_shows_source_write_denial(log, output => { - command.output_reports_denial(output) - }) - match result { + // A refusal is this tool's confinement firing, not an environment fault: + // explain it (the raw OS error is otherwise a dead end) and treat the run as + // failed even if the snippet swallowed the error and exited 0 — a swallowed + // denial still means the access it needed did not happen. It runs for the + // timeout arm too: a snippet can print a caught denial and then hang. + let sandbox_denied = refusal_guidance(sandboxed_command, Some(log), "") + let action = match result { Some((exit_code, raw)) => { // Rewrite the throwaway temp path to `source` so the model reads // `source:LINE:COL` about its own input, not a random temp path. @@ -304,31 +554,31 @@ async fn execute( } else { text } - let body = if sandbox_denied { - "\{body}\n\n\{SandboxSourceWriteFeedback}" + let body = if sandbox_denied is Some(guidance) { + "\{body}\n\n\{guidance}" } else { body } @agent_tool.ToolAction::respond( body, - is_error=exit_code != 0 || sandbox_denied, + is_error=exit_code != 0 || sandbox_denied is Some(_), brief="run_moonbit (exit=\{exit_code})", ) } None => { // The snippet may have printed useful output before it hung; the log is - // still on disk (cleanup runs on scope exit), so surface the captured prefix + // still on disk (cleanup runs below), so surface the captured prefix // instead of discarding it behind a generic timeout message. The status // line stays first so it survives the outer result clamp. let partial = rewrite_temp_paths(read_capped(log), [real, dir]) - let head = "run_moonbit: timed out after \{RunTimeoutMs / 1000}s and was cancelled — the program did not finish (an infinite loop, or a build that never completed)." + let head = "run_moonbit: timed out after \{run_timeout_ms / 1000}s and was cancelled — the program did not finish (an infinite loop, or a build that never completed)." let body = if partial.is_blank() { head } else { "\{head}\n[partial output before the timeout]\n\{partial}" } - let body = if sandbox_denied { - "\{body}\n\n\{SandboxSourceWriteFeedback}" + let body = if sandbox_denied is Some(guidance) { + "\{body}\n\n\{guidance}" } else { body } @@ -339,6 +589,43 @@ async fn execute( ) } } + action +} + +///| +/// The guidance for a refusal this run's confinement produced, or `None` when +/// the output shows none. +/// +/// Both layers are read, in the order they can fire. The moonrun policy binds +/// the snippet itself and refuses first — a write outside its roots never +/// reaches the kernel — and it applies on every platform, so it is checked +/// whether or not a profile was prepared. A `sandbox-exec` profile refuses only +/// what got past that, and only for the roles that carry one; its text comes +/// from the prepared command, so it always describes the profile that actually +/// ran. +/// +/// `log_path` is the retained output on disk, scanned past the display cap; a +/// memory-only execution has none and `head` is then all there is. +async fn refusal_guidance( + sandboxed_command : @sandbox.SandboxedCommand?, + log_path : String?, + head : String, +) -> String? { + async fn shows(reports : (String) -> Bool) -> Bool { + match log_path { + Some(path) => log_shows_denial(path, reports) + None => reports(head) + } + } + + if shows(@sandbox.output_reports_policy_refusal) { + return Some(@sandbox.policy_refusal_feedback) + } + guard sandboxed_command is Some(command) else { return None } + if shows(output => command.output_reports_denial(output)) { + return Some(command.denial_feedback()) + } + None } ///| @@ -379,16 +666,17 @@ const DenialScanCarryTailChars : Int = 4_096 const DenialScanBudgetBytes : Int = 16_777_216 ///| -/// Whether the on-disk output log shows a source-write denial. Detection must -/// not be bounded by the 48KB display cap (a snippet can flood past it before -/// the denied write), but it must stay bounded in bytes and time: a log larger +/// Whether the on-disk output log shows a denial, as `reports_denial` reads +/// one. Detection must not be bounded by the 48KB display cap (a snippet can +/// flood past it before the denied access), but it must stay bounded in bytes +/// and time: a log larger /// than twice `DenialScanBudgetBytes` is scanned at its head and tail only. An /// uncaught denial crashes the snippet, so its OSError line is the log's tail; /// a caught-and-printed denial lands where it happened, inside the head for /// realistic probes. Only a denial buried in the MIDDLE of a >32MB flood is /// missed — the price of a bounded scan. `budget_bytes` is parameterized for /// tests only. -async fn log_shows_source_write_denial( +async fn log_shows_denial( path : String, reports_denial : (String) -> Bool, budget_bytes? : Int = DenialScanBudgetBytes, @@ -489,3 +777,178 @@ async fn read_capped(path : String) -> String { text } } + +///| +/// The `run_moonbit` argument schema. `background` adds `run_in_background`, +/// which a definition without a job runtime must not advertise — the model +/// would only get an error from a call it cannot execute. +fn schema(background~ : Bool) -> Json { + let properties : Map[String, Json] = { + "source": { "type": "string" }, + "target": { + "type": "string", + "enum": ["wasm", "wasm-gc", "js", "llvm"], + "description": "Backend to run on; defaults to wasm, which is the sandboxed one. The alternatives run pure computation only — they cannot spawn commands or touch files.", + }, + "cwd": { + "type": "string", + "description": "Working directory the program runs in (relative paths resolve against the workspace root); defaults to the workspace root.", + }, + "warning": { + "type": "string", + "enum": ["off", "on"], + "description": ( + #|Whether compiler warnings appear in the output; defaults to off (a snippet + #|is a throwaway script, so unused-variable style noise is suppressed). Set + #|"on" only when the warnings themselves are what you are probing. + ), + }, + } + if background { + properties["run_in_background"] = { + "type": "boolean", + "description": ( + #|Run as a background job: returns a job id immediately and a completion + #|notice arrives when it exits. Use for long-running work (full test + #|suites, long builds, watchers) instead of blocking on the 300s + #|foreground bound, which moves the run to a job when it expires. + ), + } + } + { + "type": "object", + "properties": Json::object(properties), + "required": ["source"], + } +} + +///| +/// The `run_moonbit` tool description. +/// +/// Holds the whole command surface: every role that registers this tool gets it +/// and nothing else does. The five system prompts carried this text for one +/// revision and had already drifted apart within a single commit, while the +/// prompt-versus-tool A/B that motivated the move showed no effect either way. +/// What stays in a prompt is what varies by role — which file tools exist, and +/// whether the role may commit. +/// +/// The spawn list is prose about `spawnable_commands`, kept by hand so the +/// groupings survive; adding an entry there means editing this text too. +/// +/// `background` is conditional because without a job runtime the tool must not +/// advertise `run_in_background`: the model would only get an error. +fn description(background~ : Bool) -> String { + let base = + #|Compile and run a self-contained MoonBit program and return its merged + #|stdout/stderr and exit status. This is BOTH your command runner and your + #|scripting surface: there is no shell tool, so commands are spawned from MoonBit + #|with the `bobzhang/myshell` EDSL. + #| + #|`source` is the whole program. Diagnostics are rewritten to `source:LINE:COL` + #|about your input; `target` defaults to wasm, the sandboxed backend. + #| + #|``` + #|import { "bobzhang/myshell", "moonbitlang/async", "moonbitlang/async/fs" } + #| + #|async fn main { + #| let out = @myshell.Cmd("moon", ["check", "--diagnostic-limit", "5"]).output() + #| println(out.stdout) + #| println(out.stderr) + #| println("exit=\{out.exit_code}") + #| for name in @fs.readdir(".") { + #| println(name) + #| } + #|} + #|``` + #| + #|- Every `@pkg` is imported SEPARATELY, as `@fs` is above: `"moonbitlang/async"` + #| alone does not bring in `fs` or `stdio`, and the failure reads + #| `Package "fs" not found`. + #|- Keep `async fn main`: a plain `fn main` may not call anything that raises, + #| which nearly every `@fs`/`@myshell` call does. + #|- ALWAYS `println` each command's stdout, stderr and exit code — what you do not + #| print is invisible to you. But a run that prints more than ~48KB is STOPPED + #| mid-print, so filter before printing, or park the bulk with + #| `stdout=ToFile("\{@fs.tmpdir()}/out.log")` and read back the part you need. + #| A snippet may write ONLY inside its own `@fs.tmpdir()`, so that redirect + #| cannot target the workspace. + #|- `Cmd(program, args)` passes the argument VECTOR literally: no shell parsing, no + #| quoting, and `|`, `>`, `&&`, `$()`, `*` mean nothing. Other labels: `cwd`, + #| `env`, `stdin=Text(...)`. Run several commands as sequential statements in ONE + #| snippet and branch on `out.exit_code`; do NOT reach for `@myshell.Pipeline` — + #| filter captured output in MoonBit instead. + #| + #|## Which programs a snippet may start + #| + #|These, and nothing else: + #| + #|- `moon` — check, test, build, run, fmt, info, add, remove, update, install, + #| tree, clean, new, ide, doc, explain, coverage, cram, version. (Not `publish`, + #| `login` or `register`.) + #|- `git` — status, log, diff, show, blame, describe, rev-parse, rev-list, + #| show-ref, for-each-ref, cat-file, check-ignore, merge-base, range-diff, + #| ls-files, ls-tree, ls-remote, shortlog, grep, branch, tag, remote, reflog, + #| add, commit, checkout, switch, restore, reset, revert, rm, init, fetch, + #| push, rebase; plus `submodule update|status`, `worktree list|add|prune`, + #| `stash list|show`. Not `config`. A global option BEFORE the subcommand + #| (`-c`, `-C`, `--git-dir`, `--work-tree`) is refused. + #|- `gh` — pr, issue, run, `repo view`, api, `auth status`. + #|- `just`, for a repository whose gates live in a `justfile`. + #|- `rg` and `diff`. + #| + #|Anything else is REFUSED, including the obvious ones; if a refused command is + #|genuinely what the task needs, say so rather than working around it. Reach for + #|the replacement rather than finding this out one command at a time — each is a + #|line of MoonBit that also works on Windows, where these binaries do not exist: + #| + #| ls → @fs.readdir(dir) + #| find → rg --files, or recurse @fs.readdir + @fs.kind + #| cat → @fs.read_file(p).text() + #| head/tail → slice the split text; wc -l → count it + #| grep → rg, or .split("\n").filter(...) on captured output + #| sort/uniq → .sort(), a Set, or a Map + #| pwd → @env.current_dir(); printenv → @env.get_env_var(name) + #| mkdir -p → @fs.mkdir(d, recursive=true) + #| test -f → @fs.exists(p); test -d → @fs.kind(p) is Directory + #| echo/printf → println + #| rm/mv/cp → the `remove` and `write` tools; a snippet cannot write the + #| workspace, and `remove` refuses files it did not create — + #| a refusal there is an answer, not an obstacle to route past + #| sh -c, xargs, make → write the logic as MoonBit statements + #| + #|## Isolation and limits + #| + #|- A snippet runs with the workspace as its working directory, so RELATIVE paths + #| reach workspace files; pass `cwd` to run elsewhere. Its own build artifacts + #| stay in a temp dir — your `_build` is untouched. + #|- A snippet's imports resolve against the REGISTRY, not your uncommitted edits. + #| To exercise working-tree code, add a `*_test.mbt` to that package and run + #| `moon test` through a `@myshell.Cmd`. + #|- A snippet READS anywhere and WRITES only its own `@fs.tmpdir()`. Changing a + #| workspace file is the file tools' job, or a command's — `moon fmt`, `git + #| checkout` and friends are child processes and run normally. A write refusal + #| is that rule firing, not a filesystem fault to debug or route around. + #|- Compiler warnings for the snippet are suppressed; pass `warning: "on"` to + #| see them. + #|- Bounded to 300s. For independent commands or probes, emit SEVERAL + #| `run_moonbit` calls in the SAME assistant turn — they run and come back + #| together, saving a model round-trip each. + if !background { + return base + } + // The blank `$|` line is the paragraph break: without it the section heading + // runs onto the base text's last line and stops reading as a heading. + ( + $|\{base} + $| + $|## Long-running work + $| + $|For full test suites, long builds and watchers, set run_in_background=true: the + $|call returns a job id immediately and a notice arrives automatically when the job + $|finishes — so never block on time (no sleep loops, no polling); keep working and + $|act on the completion notice. Read a job's recent output with job_output and + $|cancel it with job_stop. A foreground run is bounded to 300s; at that deadline it + $|MOVES to a background job rather than dying, so nothing is lost either way — + $|asking for a job up front just skips the wait. + ) +} diff --git a/agent_tool/run_moonbit/run_moonbit_test.mbt b/agent_tool/run_moonbit/run_moonbit_test.mbt index 0f70f7544..d3071c9e3 100644 --- a/agent_tool/run_moonbit/run_moonbit_test.mbt +++ b/agent_tool/run_moonbit/run_moonbit_test.mbt @@ -114,86 +114,467 @@ async fn sandbox_enforcement_available() -> Bool { } ///| -async test "sandbox blocks overwriting a protected source file in the workspace" { - // Only meaningful where the sandbox can actually enforce (macOS, not nested); - // gate on the SAME probe `execute` uses so a host where sandbox-exec exists but - // cannot apply a nested profile (production runs unsandboxed there) skips - // rather than fails. The wasm backend carries this policy cross-platform. - guard sandbox_enforcement_available() else { return } +/// A snippet needs somewhere to put scratch files, and `@fs.tmpdir()` is where +/// it will look. Under a policy moonrun takes that base path from the POLICY's +/// `TMPDIR`, so the run points it at its own build directory: the call has to +/// succeed without the host's temp directory ever being granted. +async test "a snippet's temp directory lands somewhere it may write" { + let out = run( + ( + #|import { "moonbitlang/async", "moonbitlang/async/fs", "moonbitlang/async/stdio" } + #| + #|async fn main { + #| let dir = @fs.tmpdir(prefix="probe-") + #| @fs.write_file("\{dir}/x.txt", "hi", create_mode=CreateOrTruncate) + #| @stdio.stdout.write("ok=\{@fs.exists("\{dir}/x.txt")}\n") + #| @fs.rmdir(dir, recursive=true) + #|} + ), + ) + assert_false(out.is_error) + assert_true(out.content.contains("ok=true")) +} + +///| +/// The FFI refusal keys on the syntax, not the word: a snippet that merely +/// MENTIONS `extern` — in a commit message, a comment, any string — has done +/// nothing wrong, and refusing it cost a real turn a retry. +async test "the FFI refusal reads syntax, not the word extern" { + let mentions = run( + ( + #|fn main { + #| let message = "docs: explain how extern bindings are reviewed" + #| println(message) + #|} + ), + ) + assert_false(mentions.is_error) + assert_true(mentions.content.contains("docs: explain")) +} + +///| +/// A refused spawn carries the same OS wording as a blocked write, and the +/// guidance used to assert it WAS a blocked write — so a run that never touched +/// a file was told to reason about paths. The message must name the rule that +/// actually fired. +#cfg(not(platform="windows")) +async test "a refused spawn gets guidance that fits a refused spawn" { + let out = run( + ( + #|import { "bobzhang/myshell", "moonbitlang/async" } + #| + #|async fn main { + #| @myshell.Cmd("curl", ["--version"]).output() |> ignore + #|} + ), + ) + assert_true(out.is_error) + assert_true(out.content.contains("spawn allowlist")) + // And it must not claim, as it once did, that a write was what failed. + assert_false(out.content.contains("denying a write OUTSIDE")) +} + +///| +/// A snippet writes NOTHING in the workspace — the file's kind does not enter +/// into it. Its own temp directory is where it may write, and everything that +/// changes a workspace file goes through the file tools or through a child +/// process, which no `fs` rule of this policy reaches. +/// +/// Deletion is the case that matters: moonrun gates `remove`, `rename` and +/// `rmdir` on `fs.write` too, so granting the workspace was a documented way +/// around `remove`'s refusal to delete a file it did not create. +/// +/// Ungated: this is the policy's own `fs.write` rule, which moonrun applies on +/// every platform, so there is no host where it may be skipped. +async test "a snippet writes only its own temp dir, never the workspace" { + @vfs.with_tmpdir(ws => { + @vfs.with_tmpdir(outside => { + @fs.write_file( + "\{ws}/keep.mbt", + "fn main { }\n", + create_mode=CreateOrTruncate, + ) + @fs.write_file( + "\{ws}/notes.txt", + "keep me\n", + create_mode=CreateOrTruncate, + ) + let source = ( + #|import { "moonbitlang/async", "moonbitlang/async/fs", "moonbitlang/async/stdio" } + #| + #|async fn main { + #| let source_write = try { + #| @fs.write_file("keep.mbt", "// REWRITTEN\n", create_mode=CreateOrTruncate) + #| true + #| } catch { _ => false } + #| let data_write = try { + #| @fs.write_file("fresh.txt", "x", create_mode=CreateOrTruncate) + #| true + #| } catch { _ => false } + #| let delete = try { + #| @fs.remove("notes.txt") + #| true + #| } catch { _ => false } + #| let outside = try { + #| @fs.write_file("OUTSIDE/escape.txt", "x", create_mode=CreateOrTruncate) + #| true + #| } catch { _ => false } + #| let scratch = try { + #| let dir = @fs.tmpdir(prefix="probe-") + #| @fs.write_file("\{dir}/ok.txt", "x", create_mode=CreateOrTruncate) + #| true + #| } catch { _ => false } + #| @stdio.stdout.write("source=\{source_write} data=\{data_write} delete=\{delete} outside=\{outside} scratch=\{scratch}\n") + #|} + ).replace_all(old="OUTSIDE", new=outside) + let out = run_in(ws, { "source": source }) + assert_true(out.content.contains("source=false")) + assert_true(out.content.contains("data=false")) + assert_true(out.content.contains("delete=false")) + assert_true(out.content.contains("outside=false")) + // Its own scratch space still works, or a snippet could not stage anything. + assert_true(out.content.contains("scratch=true")) + // Reported as denied AND untouched on disk — including the deletion. + assert_true(@fs.read_file("\{ws}/keep.mbt").text().contains("fn main")) + assert_true(@fs.exists("\{ws}/notes.txt")) + assert_false(@fs.exists("\{ws}/fresh.txt")) + assert_false(@fs.exists("\{outside}/escape.txt")) + }) + }) +} + +///| +/// Reads are NOT bounded the way writes are, and that asymmetry is deliberate: +/// the `read` tool takes any absolute path with no scope, so a read root list +/// buys no confidentiality — it only turns a wrong guess at where the toolchain +/// lives into a refusal that reads like a broken environment, which live runs +/// spent steps on. Pinned against a file the workspace cannot reach so it can +/// only pass if the policy really is open. +async test "a snippet may read outside the workspace, though it may not write" { @vfs.with_tmpdir(ws => { + @vfs.with_tmpdir(outside => { + @fs.write_file( + "\{outside}/note.txt", + "READABLE\n", + create_mode=CreateOrTruncate, + ) + let source = ( + #|import { "moonbitlang/async", "moonbitlang/async/fs", "moonbitlang/async/stdio" } + #| + #|async fn main { + #| let text = try { + #| @fs.read_file("OUTSIDE/note.txt").text() + #| } catch { _ => "refused" } + #| @stdio.stdout.write("read=\{text.trim()}\n") + #|} + ).replace_all(old="OUTSIDE", new=outside) + let out = run_in(ws, { "source": source }) + assert_true(out.content.contains("read=READABLE")) + }) + }) +} + +///| +/// The generated moonrun policy is deny-by-default, so a program NOT on the +/// spawn allowlist must be refused at the host boundary — on every platform, +/// unlike the `sandbox-exec` profile. Uses a program that certainly exists and +/// is certainly not listed, so a pass cannot come from the command being +/// missing. +#cfg(not(platform="windows")) +async test "a command outside the spawn allowlist is denied by the policy" { + let out = run( + ( + #|import { "bobzhang/myshell", "moonbitlang/async" } + #| + #|async fn main { + #| let refused = try { + #| let result = @myshell.Cmd("chmod", ["644", "/tmp"]).output() + #| "ran exit=\{result.exit_code}" + #| } catch { _ => "refused" } + #| println("chmod: \{refused}") + #|} + ), + ) + assert_true(out.content.contains("chmod: refused")) +} + +///| +/// The allowlist keys on the argument prefix, not just the program, so one +/// subcommand of a program can be admitted while another is not. This is what +/// keeps `moon` — a general-purpose runner — from making the list decorative. +#cfg(not(platform="windows")) +async test "the spawn allowlist discriminates between subcommands" { + let out = run( + ( + #|import { "bobzhang/myshell", "moonbitlang/async" } + #| + #|async fn main { + #| // A refusal must be the POLICY refusing, not the command failing, so + #| // make the repo first: every probe below then exits 0 if it is allowed + #| // to start at all. + #| @myshell.Cmd("git", ["init", "-q", "."]).output() |> ignore + #| async fn probe(program : String, args : Array[String]) -> String { + #| try { + #| @myshell.Cmd(program, args).output() |> ignore + #| "ran" + #| } catch { _ => "refused" } + #| } + #| println("status: \{probe("git", ["status", "--short"])}") + #| println("clean: \{probe("git", ["clean", "-n"])}") + #| println("stash-list: \{probe("git", ["stash", "list"])}") + #| println("stash-pop: \{probe("git", ["stash", "pop"])}") + #| println("reconfigured: \{probe("git", ["-c", "core.pager=cat", "status"])}") + #| println("moon-version-flag: \{probe("moon", ["--version"])}") + #| println("curl: \{probe("curl", ["--version"])}") + #|} + ), + ) + // A listed git subcommand runs; one left off the list does not — the rules + // are per subcommand, so `git` as such is not a capability. + assert_true(out.content.contains("status: ran")) + assert_true(out.content.contains("clean: refused")) + // A two-token prefix reaches one verb deep, so a subcommand can be split: + // reading the stash is allowed while mutating the shared stack is not. + assert_true(out.content.contains("stash-list: ran")) + assert_true(out.content.contains("stash-pop: refused")) + // A reconfiguring global option precedes the subcommand, so the argument + // vector starts with `-c` and matches no prefix. This is what makes the + // shell tool's `global_option_reconfigures` guard unnecessary here. + assert_true(out.content.contains("reconfigured: refused")) + // A prefix gates whole tokens, so the FLAG spelling of a subcommand is a + // separate rule from the subcommand. Listing only `version` refused + // `moon --version`, which a live run then spent three attempts working + // around; both spellings are listed now. + assert_true(out.content.contains("moon-version-flag: ran")) + assert_true(out.content.contains("curl: refused")) +} + +///| +/// The profile follows `read_only`, not the presence of a lab. A caller that +/// forgets `scratch_dir` should lose the lab and keep the sandbox — the review +/// CLI shipped one revision without the lab and silently ran unconfined, which +/// is the shape this pins against. +async test "read_only denies source writes with no scratch lab wired" { + guard sandbox_enforcement_available() else { return } + @vfs.with_tmpdir(prefix="run-moonbit-ro-", ws => { @fs.write_file( "\{ws}/keep.mbt", "fn main { }\n", create_mode=CreateOrTruncate, ) - let out = run_in(ws, { - "source": ( + let definition = @run_moonbit.definition(workspace_root=ws, read_only=true) + let action = match definition.execute { + Async(execute) => + execute({ + "source": ( + #|import { "moonbitlang/async", "moonbitlang/async/fs", "moonbitlang/async/stdio" } + #| + #|async fn main { + #| let wrote = try { + #| @fs.write_file("keep.mbt", "// CORRUPTED\n", create_mode=CreateOrTruncate) + #| true + #| } catch { _ => false } + #| @stdio.stdout.write("workspace=\{wrote}\n") + #|} + ), + }) + Sync(_) => fail("run_moonbit should be async") + } + guard action is Respond(output) else { fail("expected Respond") } + assert_true(output.content.contains("workspace=false")) + assert_true(@fs.read_file("\{ws}/keep.mbt").text().contains("fn main { }")) + }) +} + +///| +/// A read-only subagent gets a scratch lab: the one place it MAY fabricate and +/// rewrite source to verify a claim empirically. The lab must be writable while +/// the workspace's own sources stay denied — otherwise the lab either does not +/// work or is a hole in the read-only posture. +async test "a scratch lab is writable while workspace source stays denied" { + guard sandbox_enforcement_available() else { return } + @vfs.with_tmpdir(prefix="run-moonbit-lab-ws-", ws => { + @vfs.with_tmpdir(prefix="run-moonbit-lab-", lab => { + @fs.write_file( + "\{ws}/keep.mbt", + "fn main { }\n", + create_mode=CreateOrTruncate, + ) + let definition = @run_moonbit.definition( + workspace_root=ws, + read_only=true, + scratch_dir=lab, + ) + // The lab path is substituted rather than interpolated: the snippet has + // `\{...}` of its own, which an interpolated outer string would consume. + let source = ( #|import { "moonbitlang/async", "moonbitlang/async/fs", "moonbitlang/async/stdio" } #| #|async fn main { - #| let blocked = try { + #| let in_lab = try { + #| @fs.write_file("LAB_DIR/probe.mbt", "fn probe() -> Int { 1 }\n", create_mode=CreateOrTruncate) + #| true + #| } catch { _ => false } + #| let in_workspace = try { #| @fs.write_file("keep.mbt", "// CORRUPTED\n", create_mode=CreateOrTruncate) - #| false - #| } catch { _ => true } - #| // a non-source output is still allowed - #| @fs.write_file("out.txt", "ok\n", create_mode=CreateOrTruncate) - #| @stdio.stdout.write("blocked=\{blocked}\n") + #| true + #| } catch { _ => false } + #| @stdio.stdout.write("lab=\{in_lab} workspace=\{in_workspace}\n") #|} - ), + ).replace_all(old="LAB_DIR", new=lab) + let action = match definition.execute { + Async(execute) => execute({ "source": source }) + Sync(_) => fail("run_moonbit should be async") + } + guard action is Respond(output) else { fail("expected Respond") } + assert_true(output.content.contains("lab=true")) + assert_true(output.content.contains("workspace=false")) + assert_true( + @fs.read_file("\{ws}/keep.mbt").text().contains("fn main { }"), + ) }) - assert_false(out.is_error) - assert_true(out.content.contains("blocked=true")) - // the protected source file is untouched; the non-source output landed - assert_true(@fs.read_file("\{ws}/keep.mbt").text().contains("fn main { }")) - assert_true(@fs.exists("\{ws}/out.txt")) }) } ///| -test "the description encourages batched same-turn probing" { - // The batching guidance lives in the tool description (and the default - // prompt): several independent probes in ONE assistant turn, not serial - // single-probe turns. - // The description is a wrapped `#|` block, so newlines fall mid-phrase; - // normalize them to spaces before matching the guidance phrases. - let description = @run_moonbit.definition(workspace_root=".").description.replace_all( - old="\n", - new=" ", - ) - assert_true(description.contains("SAME assistant turn")) - assert_true(description.contains("one small program per hypothesis")) +/// A worker's SNIPPET is bound like every other: it writes nothing in the tree, +/// its own worktree included. A worker does write source — through `edit` and +/// `write`, and through the moon commands it spawns, which are children and so +/// outside this policy. `moon fmt` on its own files keeps working. +/// +/// The sibling denial is the half that is load-bearing here, and it now holds +/// on every platform rather than only where `sandbox-exec` does. The kernel +/// profile still governs what those CHILD processes may touch; that is pinned +/// in `internal/sandbox/worker_profile_test.mbt`. +async test "a worker's snippet writes neither its own tree nor a sibling's" { + @vfs.with_tmpdir(prefix="run-moonbit-worker-repo-", repo => { + let common_git = "\{repo}/.git" + let worker = "\{repo}/wt-1" + let sibling = "\{repo}/wt-2" + @fs.mkdir("\{common_git}/worktrees/wt-1", recursive=true) + @fs.mkdir(worker, recursive=true) + @fs.mkdir(sibling, recursive=true) + @fs.write_file( + "\{worker}/mine.mbt", + "fn mine() -> Int { 1 }\n", + create_mode=CreateOrTruncate, + ) + @fs.write_file( + "\{sibling}/theirs.mbt", + "fn theirs() -> Int { 2 }\n", + create_mode=CreateOrTruncate, + ) + let definition = @run_moonbit.definition( + workspace_root=worker, + // The real geometry the subtask controller computes: the repository's + // shared surfaces are denied wholesale and the worker's own tree is + // carved back out of them, so `worker_root` sits below a denied root. + worker_sandbox={ + deny_roots: [@fs.realpath(repo)], + worker_root: @fs.realpath(worker), + worker_admin_dir: @fs.realpath("\{common_git}/worktrees/wt-1"), + }, + ) + let source = ( + #|import { "moonbitlang/async", "moonbitlang/async/fs", "moonbitlang/async/stdio" } + #| + #|async fn main { + #| let mine = try { + #| @fs.write_file("mine.mbt", "fn mine() -> Int { 11 }\n", create_mode=CreateOrTruncate) + #| true + #| } catch { _ => false } + #| let theirs = try { + #| @fs.write_file("SIBLING/theirs.mbt", "// CLOBBERED\n", create_mode=CreateOrTruncate) + #| true + #| } catch { _ => false } + #| @stdio.stdout.write("mine=\{mine} theirs=\{theirs}\n") + #|} + ).replace_all(old="SIBLING", new=sibling) + let action = match definition.execute { + Async(execute) => execute({ "source": source }) + Sync(_) => fail("run_moonbit should be async") + } + guard action is Respond(output) else { fail("expected Respond") } + assert_true(output.content.contains("mine=false")) + assert_true(output.content.contains("theirs=false")) + // Untouched on disk, not merely reported as denied — both trees. + assert_true( + @fs.read_file("\{sibling}/theirs.mbt").text().contains("fn theirs"), + ) + assert_true( + @fs.read_file("\{worker}/mine.mbt").text().contains("-> Int { 1 }"), + ) + }) +} + +///| +/// Without a job runtime the tool cannot execute a background run, so neither +/// the description nor the schema may advertise one — the model would only get +/// an error. (The wired half of this contract is asserted in the `agent` +/// package, which builds a registry with a real runtime.) +test "a definition without a job runtime does not advertise background runs" { + let definition = @run_moonbit.definition(workspace_root=".") + assert_false(definition.description.contains("run_in_background")) + assert_true(definition.description.contains("bobzhang/myshell")) + let JsonSchema(schema) = definition.schema + assert_false(schema.stringify().contains("run_in_background")) +} + +///| +/// The schema is the model-facing argument contract, so pin its shape: the +/// arguments the tool decodes are all present, and only `source` is required. +test "the schema advertises every decoded argument" { + let JsonSchema(schema) = @run_moonbit.definition(workspace_root=".").schema + guard schema is { "properties": Object(properties), "required": required, .. } else { + fail("expected an object schema with properties and required") + } + // Order-independent: `Map` iteration order is not the declaration order, and + // MoonBit's String ordering is by length first, so a sorted expectation would + // encode neither what this test means nor what a reader expects. + let names = properties.keys().collect() + assert_eq(names.length(), 4) + for name in ["source", "target", "cwd", "warning"] { + assert_true(names.contains(name)) + } + assert_eq(required, (["source"] : Json)) } +// The EDSL guidance the description carries — how a snippet is shaped, the +// warning off `@myshell.Pipeline`, batching probes into one turn — is not +// re-pinned by phrase here: an assertion that a text contains its own words +// breaks on every rewrite and catches no real gap. + ///| async test "an uncaught sandbox denial is explained, not left as a bare EPERM" { - guard sandbox_enforcement_available() else { return } @vfs.with_tmpdir(ws => { - @fs.write_file( - "\{ws}/keep.mbt", - "fn main { }\n", - create_mode=CreateOrTruncate, - ) - // Unlike the test above, the snippet does NOT catch the write failure, so - // the run dies with the raw OS error — the case where the model previously - // saw only `Operation not permitted` with no idea the sandbox caused it. - let out = run_in(ws, { - "source": ( + @vfs.with_tmpdir(outside => { + // Unlike the test above, the snippet does NOT catch the write failure, so + // the run dies with the raw OS error — the case where the model previously + // saw only `Operation not permitted` with no idea the sandbox caused it. + let source = ( #|import { "moonbitlang/async", "moonbitlang/async/fs" } #| #|async fn main { - #| @fs.write_file("keep.mbt", "// CORRUPTED\n", create_mode=CreateOrTruncate) + #| @fs.write_file("OUTSIDE/escape.txt", "x", create_mode=CreateOrTruncate) #|} - ), + ).replace_all(old="OUTSIDE", new=outside) + let out = run_in(ws, { "source": source }) + assert_true(out.is_error) + // The raw failure stays visible, and it is the moonrun policy's own + // wording — the phrase the guidance is recognized by. Assert what is + // actually printed: an earlier version of this test looked for + // "Operation not permitted", which only ever matched because the guidance + // text quoted that phrase, so it verified nothing about the real error. + assert_true(out.content.contains("Sandbox policy blocked file write")) + assert_true(out.content.contains("Permission denied")) + // ...now followed by the sandbox explanation + assert_true(out.content.contains("run_moonbit sandbox")) + // The guidance describes the policy's rules — the snippet's own roots and + // the spawn allowlist — not the source-write rule this run does not carry. + assert_true(out.content.contains("write outside its own")) + assert_false(out.content.contains("`edit`")) + assert_false(@fs.exists("\{outside}/escape.txt")) }) - assert_true(out.is_error) - // the raw OS error stays visible... - assert_true(out.content.contains("Operation not permitted")) - // ...now followed by the sandbox explanation and anti-workaround guidance - assert_true(out.content.contains("run_moonbit sandbox")) - assert_true(out.content.contains("Do NOT try to work around")) - assert_true(out.content.contains("`edit`")) - // the protected source file is untouched - assert_true(@fs.read_file("\{ws}/keep.mbt").text().contains("fn main { }")) }) } @@ -219,18 +600,12 @@ async test "warnings are suppressed by default and shown with warning=on" { ///| async test "a denial past the display cap is still detected and explained" { - guard sandbox_enforcement_available() else { return } @vfs.with_tmpdir(ws => { - @fs.write_file( - "\{ws}/keep.mbt", - "fn main { }\n", - create_mode=CreateOrTruncate, - ) - // ~200KB of flood BEFORE the uncaught denied write: the OSError line lands - // beyond the 48KB display cap (and beyond one 64KB scan chunk), so only - // the full-log scan can see it. - let out = run_in(ws, { - "source": ( + @vfs.with_tmpdir(outside => { + // ~200KB of flood BEFORE the uncaught denied write: the OSError line lands + // beyond the 48KB display cap (and beyond one 64KB scan chunk), so only + // the full-log scan can see it. + let source = ( #|import { "moonbitlang/async", "moonbitlang/async/fs", "moonbitlang/async/stdio" } #| #|async fn main { @@ -238,18 +613,20 @@ async test "a denial past the display cap is still detected and explained" { #| for _i in 0..<200 { #| @stdio.stdout.write("\{filler}\n") #| } - #| @fs.write_file("keep.mbt", "// CORRUPTED\n", create_mode=CreateOrTruncate) + #| @fs.write_file("OUTSIDE/escape.txt", "x", create_mode=CreateOrTruncate) #|} - ), + ).replace_all(old="OUTSIDE", new=outside) + let out = run_in(ws, { "source": source }) + assert_true(out.is_error) + // the displayed prefix is the truncated flood, not the denial line — + // pinned on `OSError`, a token the guidance text does not contain, so + // the assertion cannot be satisfied by the guidance instead... + assert_true(out.content.contains("output truncated")) + assert_false(out.content.contains("OSError")) + // ...yet the guidance still lands, from the full-log scan + assert_true(out.content.contains("run_moonbit sandbox")) + assert_false(@fs.exists("\{outside}/escape.txt")) }) - assert_true(out.is_error) - // the displayed prefix is the truncated flood, not the denial line (the - // feedback itself quotes "Operation not permitted", so pin on OSError)... - assert_true(out.content.contains("output truncated")) - assert_false(out.content.contains("OSError")) - // ...yet the guidance still lands, from the full-log scan - assert_true(out.content.contains("run_moonbit sandbox")) - assert_true(@fs.read_file("\{ws}/keep.mbt").text().contains("fn main { }")) }) } @@ -304,16 +681,24 @@ async test "error diagnostics name `source`, not the throwaway temp path" { } ///| -async test "refuses a program that spawns processes (shell owns commands)" { +/// Spawning processes is what this tool is for now — the refusal that used to +/// send commands to the shell tool would make the registry unable to run any. +#cfg(not(platform="windows")) +async test "runs a command through the myshell EDSL" { let out = run( ( - #|import { "moonbitlang/async", "moonbitlang/async/process" } + #|import { "bobzhang/myshell", "moonbitlang/async" } #| - #|async fn main { @process.run("ls", []) |> ignore } + #|async fn main { + #| let result = @myshell.Cmd("git", ["--version"]).output() + #| println("out=\{result.stdout}") + #| println("exit=\{result.exit_code}") + #|} ), ) - assert_true(out.is_error) - assert_true(out.content.contains("spawns processes")) + assert_false(out.is_error) + assert_true(out.content.contains("out=git version")) + assert_true(out.content.contains("exit=0")) } ///| diff --git a/agent_tool/run_moonbit/run_moonbit_wbtest.mbt b/agent_tool/run_moonbit/run_moonbit_wbtest.mbt index 2b91f33e3..f4e3fafcf 100644 --- a/agent_tool/run_moonbit/run_moonbit_wbtest.mbt +++ b/agent_tool/run_moonbit/run_moonbit_wbtest.mbt @@ -19,7 +19,7 @@ async test "denial scan is bounded to the log's head and tail" { async fn scan(name : String, content : String) -> Bool { let path = "\{dir}/\{name}.log" @fs.write_file(path, content, create_mode=CreateOrTruncate) - log_shows_source_write_denial( + log_shows_denial( path, text => text.contains("main.mbt: Operation not permitted"), budget_bytes=1024, @@ -38,3 +38,8 @@ async test "denial scan is bounded to the log's head and tail" { assert_false(scan("clean", filler(64))) }) } + +// The spawn allowlist is taught by each role's system prompt now, and that +// prose is NOT pinned by a test: asserting a prompt contains particular words +// only breaks on rewrites. Keeping the list and the prompts in step is a +// review-time job, not a test-time one. diff --git a/agent_tool/run_moonbit/wasm_policy.mbt b/agent_tool/run_moonbit/wasm_policy.mbt new file mode 100644 index 000000000..ffcb0a906 --- /dev/null +++ b/agent_tool/run_moonbit/wasm_policy.mbt @@ -0,0 +1,282 @@ +///| +/// The commands a snippet may spawn, as `moonrun --policy` process rules. +/// +/// This list is the agent's command vocabulary, not a security perimeter: the +/// programs on it can themselves run code (`moon run` compiles and runs a +/// program, `git` honours hooks), so what it buys is the LONG TAIL — an +/// arbitrary downloaded binary, `rm -rf /`, `curl | sh` — none of which a +/// legitimate turn needs. +/// +/// Nothing confines where the admitted programs then write: moonrun's `fs` +/// section binds the snippet and stops at the process boundary. What this list +/// can still say is which FORM of a command exists, and the git rules use it: +/// the options that would point git at another repository (`-C`, `--git-dir`, +/// `--work-tree`) precede the subcommand, so they match no prefix rule below. +/// +/// Entries are `(program, args_prefix)`. An empty prefix allows any arguments. +/// A prefix matches whole argument tokens from the front, so `moon` with +/// `["test"]` allows `moon test --filter x` but not `moon build`. +/// +/// Each role's system prompt carries this list in prose. Admitting a command +/// here without adding it there costs a turn steps: the model never learns it +/// may use it. +let spawnable_commands : Array[(String, Array[String])] = [ + // The MoonBit toolchain, which is most of what a turn runs. Listed per + // subcommand rather than as a bare `moon` so the set is reviewable, and so + // adding a new one is a deliberate edit. + ("moon", ["check"]), + ("moon", ["test"]), + ("moon", ["build"]), + // `moon run` is how a turn exercises a binary it just built: a freshly + // compiled executable can never be on this list — its name did not exist + // when the list was written — so without `moon run` the whole shape of task + // "write a CLI, then probe its behaviour" is unreachable. + // + // It was briefly withheld on the grounds that it recovers an unconstrained + // native process. That was not a real distinction: `moon test` compiles and + // runs workspace code natively too, and no `fs` rule of this policy reaches + // any child. The `target: "native"` refusal in `decode` keeps the SNIPPET + // under policy; it was never a boundary around what the snippet may start. + ("moon", ["run"]), + ("moon", ["fmt"]), + ("moon", ["info"]), + ("moon", ["add"]), + ("moon", ["remove"]), + ("moon", ["update"]), + ("moon", ["install"]), + ("moon", ["tree"]), + ("moon", ["clean"]), + ("moon", ["new"]), + ("moon", ["ide"]), + ("moon", ["doc"]), + ("moon", ["explain"]), + ("moon", ["coverage"]), + ("moon", ["cram"]), + ("moon", ["version"]), + // Asking what is installed is the first thing a turn does when something + // looks wrong with the toolchain, and it is spelled with the FLAG far more + // often than with the subcommand. Listing only `version` left `moon + // --version` matching no rule: a live run spent three attempts on it — + // plain, then with a rebuilt `PATH`, then by absolute path — reading each + // refusal as a broken toolchain rather than as this list. `git --version` was + // already here; the asymmetry was an oversight, not a decision. + ("moon", ["--version"]), + ("moon", ["--help"]), + // Version control, listed per subcommand for two reasons. The obvious one is + // that whole dangerous subcommands can be left out. The second falls out of + // how prefixes match: git's reconfiguring GLOBAL options come BEFORE the + // subcommand, so `git -c core.pager=CMD status` has args starting with `-c` + // and matches no rule here — which recovers, for free, the guard the shell + // tool spent a parser on (`global_option_reconfigures`). + // + // What a prefix CANNOT do is gate flags: admitting `rebase` admits + // `rebase --exec CMD`, since the flag follows the subcommand. Such a + // subcommand is all-or-nothing, and the ones below are decided on whether the + // long tail is worth losing the verb — not on whether the flag exists, since + // `moon run` already runs arbitrary programs from this same list. + ("git", ["--version"]), + ("git", ["--help"]), + ("git", ["status"]), + ("git", ["log"]), + ("git", ["diff"]), + ("git", ["show"]), + ("git", ["blame"]), + ("git", ["describe"]), + ("git", ["rev-parse"]), + // Read-only queries, each a subset of something already here. They were + // missing only because this list is written a subcommand at a time. + ("git", ["rev-list"]), + ("git", ["show-ref"]), + ("git", ["for-each-ref"]), + ("git", ["cat-file"]), + ("git", ["check-ignore"]), + ("git", ["merge-base"]), + ("git", ["range-diff"]), + ("git", ["ls-files"]), + // `ls-files` reads the index, so listing a tree at another commit had no + // spelling short of `cat-file -p` on a hash the turn must first resolve. + ("git", ["ls-tree"]), + ("git", ["ls-remote"]), + ("git", ["shortlog"]), + ("git", ["grep"]), + ("git", ["branch"]), + ("git", ["tag"]), + ("git", ["remote"]), + ("git", ["reflog"]), + ("git", ["add"]), + ("git", ["commit"]), + ("git", ["checkout"]), + ("git", ["switch"]), + ("git", ["restore"]), + ("git", ["reset"]), + ("git", ["revert"]), + ("git", ["rm"]), + ("git", ["init"]), + ("git", ["fetch"]), + ("git", ["push"]), + // Once limited to --continue/--abort/--skip, to keep `--exec` out. That + // guards nothing next to `moon run`, already here and running arbitrary + // programs, and it refused a plain `git rebase origin/main` in a live turn. + ("git", ["rebase"]), + // Same shape: `submodule foreach` runs an arbitrary command, `worktree + // move`/`remove --force` relocate or destroy files, and the stash stack is + // SHARED by every worktree of a repository — a `pop` here can take an entry + // another session pushed. Where the hazard sits in a verb rather than a flag, + // a two-token prefix separates the reads from it exactly; `git stash` with no + // verb is a push, so it stays out along with `pop`, `drop` and `clear`. + ("git", ["submodule", "update"]), + ("git", ["submodule", "status"]), + ("git", ["worktree", "list"]), + ("git", ["worktree", "add"]), + // `prune` only garbage-collects the admin records of worktrees whose + // DIRECTORY is already gone — it has no argument naming one to delete, so it + // cannot reach a live tree the way `remove` can. That is what makes it the + // safe half: delete your own worktree's directory, then prune the record. + ("git", ["worktree", "prune"]), + ("git", ["stash", "list"]), + ("git", ["stash", "show"]), + // Deliberately absent, matching the forms the shell tool pre-blocked: + // `am`/`apply` (import arbitrary bytes), `clean` (permanently removes + // untracked files), `bisect` (`run` executes a command), `merge`/`pull`/ + // `cherry-pick` (`-X`/strategy resolve an external merge driver), `mv`, + // `clone`, and the plumbing (`fast-import`, `read-tree`, `update-index`, + // `filter-branch`). In each of those the dangerous form is selected by a + // FLAG, which follows the subcommand and so cannot be excluded by a prefix — + // leaving the whole subcommand out is the only setting available. + // + // `difftool`/`mergetool` are out for a different reason: both drive an + // interactive external program, which a headless turn cannot answer. + // + // `config` writes what LATER git commands do — `core.pager`, `alias.*`, + // `protocol.ext.allow` (which makes `ls-remote 'ext::sh -c ...'` a process + // launcher) — so it undoes the `git -c` refusal one command later. + // + // The forge CLI, likewise per subcommand: `gh extension` installs and then + // runs third-party code, and `gh alias` turns any later invocation into + // something else — both would make this list decorative. + ("gh", ["pr"]), + ("gh", ["issue"]), + ("gh", ["run"]), + ("gh", ["repo", "view"]), + ("gh", ["api"]), + ("gh", ["auth", "status"]), + // The only two read-only utilities without a MoonBit equivalent worth using: + // `rg` searches a whole tree faster than a snippet can walk it, and `diff` + // produces a structured comparison. Their mutating counterparts (`rm`, `mv`, + // `cp`, `chmod`) are absent because file changes belong to the file tools and + // to `@fs`. + // + // `cat`, `ls`, `head`, `tail`, `wc`, `sort`, `uniq`, `grep`, `printf`, + // `echo`, `pwd`, `date`, `which` and `uname` were dropped rather than listed. + // Each is a line of MoonBit (`@fs`, string operations, `@env`) that works on + // every platform, whereas the POSIX binaries do not exist on Windows — so + // listing them offers the model a path that silently disappears there. It + // would also cut against this tool's own advice, which is to capture + // `out.stdout` and filter it in MoonBit instead of reaching for grep and awk. + ("diff", []), + ("rg", []), + // A repository's gates live in its `justfile`; reimplementing recipes by hand + // runs something else and goes stale. Recipes are not listed one by one for + // the same reason. A recipe is shell, so this does admit arbitrary code — but + // so does `rebase --exec`, which a prefix cannot exclude. + ("just", []), + // NOT listed: the general-purpose runners (`sh`, `bash`, `zsh`, `env`, + // `xargs`, `python`, `node`, `perl`, `make`). Not as a boundary — see `just` + // — but because myshell exists so no shell sits between the agent and a + // process. What the list buys is that going around it takes intent. +] + +///| +/// Build the `moonrun --policy` document for one snippet run. +/// +/// Deny-by-default: moonrun denies every surface a policy does not name. Writes +/// name only the two roots a snippet has business in — its own temp directory +/// and a read-only role's lab — while reads are open. +/// +/// Reads are open because bounding them stops nothing this agent cannot already +/// do more easily, and costs steps to enforce. The `read` tool takes any +/// absolute path with no scope of its own, so any file on the machine is one +/// tool call away; `env.from_host` below hands the snippet every environment +/// variable the agent holds, API keys included. Against that, a read root list +/// buys no confidentiality — it only turns a wrong guess at where the toolchain +/// lives into a refusal that reads like a broken environment. Live runs spent +/// steps on exactly that, guessing `~/.moon`, `~/.mooncakes` and an opam- +/// installed `moon`, none of which the list happened to name. +/// +/// Writes keep their bound for the reason reads do not: a snippet writing +/// outside the tree is INVISIBLE in the transcript, where a `write` tool call +/// naming the same path is right there to read. The asymmetry is legibility, +/// not access. +/// +/// What this binds is the SNIPPET, on every platform. It does not reach the +/// child processes its `process` rules let the snippet start: those run with the +/// agent's own ambient access, and what bounds them is the allowlist itself — +/// which admits no shell, no interpreter, and no ad-hoc rewriter, and which +/// refuses git's reconfiguring global options because they precede the +/// subcommand. +fn wasm_policy_document( + tmp_dir~ : String, + extra_roots~ : Array[String], +) -> Json { + // A snippet writes NOTHING in the workspace. It does not need to: the file + // tools make the edits, and the commands whose job is to rewrite source + // (`moon fmt`, `moon info`, `git checkout`) are child processes, which no `fs` + // rule here reaches. What granting the workspace did buy was a documented way + // around `remove`'s provenance check — `remove_path`, `rename_path` and + // `rmdir_path` are all gated on `fs.write`, so a snippet could delete a file + // the guarded tool exists to refuse. + // + // That leaves the two places a snippet has real business writing: its own temp + // directory (where `@fs.tmpdir()` lands, since `TMPDIR` points here) and a + // read-only role's scratch lab, which is the one place those roles may build + // an experiment — they hold no editing tool at all. + // + // Every entry must be an EXISTING path: moonrun canonicalizes each root when + // it loads the policy and fails the whole file otherwise. Both of these are + // created before this runs. + let write_roots = [tmp_dir, ..extra_roots] + let allow : Array[Json] = [] + for entry in spawnable_commands { + let (program, args_prefix) = entry + let rule : Json = if args_prefix.is_empty() { + { "program": program } + } else { + { "program": program, "args_prefix": json_strings(args_prefix) } + } + allow.push(rule) + } + { + // A policy run BUILDS the guest environment rather than inheriting one, so + // an empty list means an empty environment — `moon` could not even be + // resolved by name. `"*"` copies the host's, which is what the shell tool + // did and the only defensible choice here: a curated list would have to + // know the variables the user's own project reads (`OPENAI_API_KEY`, + // `DATABASE_URL`, whatever the tests need), and getting that wrong is + // silent — the snippet, and the `moon test` it spawns, simply see nothing. + // Confinement on this surface would be a functionality gate, not a boundary: + // the agent process already holds these values. + // + // `TMPDIR` is then overridden, because under a policy moonrun resolves + // `@fs.tmpdir()` from the POLICY's environment (`async_api/fs.rs::tmp_path`) + // rather than the host's. Pointing it at a directory this run already owns + // gives the snippet working scratch space without granting the host temp + // directory, which would widen the read and write roots to hold every other + // run's files. A native child ignores it either way — moonbitlang/async's + // `stub.c` hardcodes `/tmp` off Windows — but a child is outside this + // policy, so its temp directory is not this policy's business. + "env": { "from_host": ["*"], "set": { "TMPDIR": tmp_dir } }, + // `"*"` is moonrun's every-host-path wildcard — see the header for why + // reads are open where writes are not. + "fs": { "read": ["*"], "write": json_strings(write_roots) }, + // Network stays denied: nothing a snippet does needs it except registry + // fetches, which run inside `moon` — a child, and so outside this policy + // either way. + "process": { "allow": Json::array(allow) }, + } +} + +///| +/// A JSON array of strings, for the path lists the policy is mostly made of. +fn json_strings(values : Array[String]) -> Json { + Json::array([ for value in values => Json::string(value) ]) +} diff --git a/agent_tool/run_moonbit/worker_sandbox.mbt b/agent_tool/run_moonbit/worker_sandbox.mbt new file mode 100644 index 000000000..5dd53505c --- /dev/null +++ b/agent_tool/run_moonbit/worker_sandbox.mbt @@ -0,0 +1,25 @@ +///| +/// The write confinement of a worker subagent's snippets, carried from the +/// subtask controller (which computes the canonical geometry at provision +/// time) into every launch. When present, the snippet — and every process it +/// spawns — runs under the worker write profile, which denies the repository's +/// shared surfaces (`deny_roots`: the canonical common git dir plus every +/// enumerated worktree root) and re-allows only the worker's own tree and its +/// private git admin dir. All paths must be canonical (realpath'd): the kernel +/// matches profile rules against realpaths. +/// +/// This replaces the workspace source-write profile rather than adding to it: +/// a worker is *supposed* to write source, just only its own. +/// +/// On platforms without sandbox-exec the kernel layer is absent and a snippet +/// runs unconfined — the same best-effort posture as the rest of the sandbox +/// stack there. What still holds on every platform is the file tools' +/// `write_scope` (their paths are checked against the worker's `allowed_paths`) +/// and the subtask controller's git-evidence validation before anything is +/// merged; the profile is the layer that also keeps a worker's *commands* out +/// of its siblings' trees. +pub(all) struct WorkerSandbox { + deny_roots : Array[String] + worker_root : String + worker_admin_dir : String +} diff --git a/agent_tool/shell_exec/execution.mbt b/agent_tool/shell_exec/execution.mbt index e4ba2764a..62791d1d3 100644 --- a/agent_tool/shell_exec/execution.mbt +++ b/agent_tool/shell_exec/execution.mbt @@ -75,11 +75,20 @@ pub struct ShellExecution { /// Spawn `program args` on the session group with stdout+stderr merged into a /// fresh sink, start a monitor task, and return the live execution. `spill_path`, /// when set, is where the sink writes full output once it exceeds `inline_cap`. +/// +/// `stdin` is the child's standard input. A child must never be left holding +/// the PARENT's fd 0 — under `openseek serve` that descriptor is the JSONL +/// command channel, and a child that reads stdin would consume the engine's own +/// commands. Omitting it therefore does not fall back to inheritance: the child +/// gets a pipe of its own whose write end is closed at once, so it reads EOF +/// immediately (on Windows too, where inheriting the engine's held-open pipe +/// hangs `git`). Pass a redirect when the program should actually read something. pub async fn[X] ShellExecution::start( scope : @agent_runtime.AgentTaskScope[X], program~ : String, args~ : Array[String], cwd? : String, + stdin? : &@process.ProcessInput, inline_cap? : Int = default_inline_cap, hard_cap? : Int = default_hard_cap, spill_path? : String, @@ -89,13 +98,19 @@ pub async fn[X] ShellExecution::start( let group = scope.group() let sink = ShellOutputSink::new(inline_cap~, hard_cap~, spill_path?) let (reader, writer) = @process.read_from_process() - let (stdin_reader, stdin_writer) = @process.write_to_process() - stdin_writer.close() + let child_stdin : &@process.ProcessInput = match stdin { + Some(input) => input + None => { + let (stdin_reader, stdin_writer) = @process.write_to_process() + stdin_writer.close() + stdin_reader + } + } let process = @process.spawn( group, program, args, - stdin=stdin_reader, + stdin=child_stdin, stdout=writer, stderr=writer, cwd?=cwd.map(cwd => cwd.view()), @@ -317,9 +332,15 @@ pub fn ShellExecution::over_inline_cap(self : ShellExecution) -> Bool { } ///| -/// The spill file's path, if output spilled to disk. -#cfg(not(platform="windows")) -fn ShellExecution::spill_path(self : ShellExecution) -> String? { +/// Where full output is retained on disk, or `None` for a memory-only +/// execution (one started without a `spill_path` — tests, and any runtime +/// whose session temp dir could not be created). +/// +/// Public so a caller can scan the COMPLETE output without materializing it: +/// `read_all` pulls up to `hard_cap` (20MB) into memory, while a path lets the +/// caller stream its own bounded window — which is how a sandbox source-write +/// denial is found when it lands past the rendered head. +pub fn ShellExecution::spill_path(self : ShellExecution) -> String? { self.sink.spill_path() } diff --git a/agent_tool/shell_exec/pkg.generated.mbti b/agent_tool/shell_exec/pkg.generated.mbti index 70ecafdf2..6b101eb37 100644 --- a/agent_tool/shell_exec/pkg.generated.mbti +++ b/agent_tool/shell_exec/pkg.generated.mbti @@ -3,6 +3,7 @@ package "bobzhang/openseek/agent_tool/shell_exec" import { "bobzhang/openseek/agent_runtime", + "moonbitlang/async/process", "moonbitlang/core/debug", } @@ -40,7 +41,8 @@ pub async fn ShellExecution::read_tail(Self, Int) -> String pub fn ShellExecution::request_stop(Self) -> Unit pub fn ShellExecution::seq(Self) -> Int pub fn ShellExecution::size(Self) -> Int -pub async fn[X] ShellExecution::start(@agent_runtime.AgentTaskScope[X], program~ : String, args~ : Array[String], cwd? : String, inline_cap? : Int, hard_cap? : Int, spill_path? : String, kill_when_full? : Bool, kill_at_hard_cap? : Bool) -> Self +pub fn ShellExecution::spill_path(Self) -> String? +pub async fn[X] ShellExecution::start(@agent_runtime.AgentTaskScope[X], program~ : String, args~ : Array[String], cwd? : String, stdin? : &@process.ProcessInput, inline_cap? : Int, hard_cap? : Int, spill_path? : String, kill_when_full? : Bool, kill_at_hard_cap? : Bool) -> Self pub fn ShellExecution::started_at_ms(Self) -> Int64 pub fn ShellExecution::status(Self) -> ExecStatus pub async fn ShellExecution::stop(Self) -> Bool diff --git a/agent_worker/generated_worker_system_prompt.mbt b/agent_worker/generated_worker_system_prompt.mbt index 93d212eed..1e10884b6 100644 --- a/agent_worker/generated_worker_system_prompt.mbt +++ b/agent_worker/generated_worker_system_prompt.mbt @@ -12,10 +12,11 @@ fn worker_system_prompt() -> String { #| #|The confinement, plainly: #|- Edit only within the allowed paths your task names. The file tools - #| refuse targets outside them; the shell sandbox denies writes outside - #| your worktree. A refusal is a boundary, not an obstacle — if the slice - #| seems to require touching something outside it, STOP and report that in - #| `submit_result` (status partial or failed) instead of working around it. + #| refuse targets outside them; the sandbox around the commands you run + #| denies writes outside your worktree. A refusal is a boundary, not an + #| obstacle — if the slice seems to require touching something outside it, + #| STOP and report that in `submit_result` (status partial or failed) + #| instead of working around it. #|- Do NOT run `git commit`, `git add`, `git push`, `git worktree`, branch or #| config surgery: the shared git state is denied and the HARNESS commits your #| changes after validating them. Permission errors on such commands are @@ -35,13 +36,14 @@ fn worker_system_prompt() -> String { #| YOUR regression until proven otherwise. #|- Change existing source with the line-anchored `edit` (or `multi_edit` #| for several fixes in one file — the efficient path when the compiler - #| names many known locations); `write` creates new files; shell rewrites - #| of source files are blocked. + #| names many known locations); `write` creates new files. Rewriting source + #| from a command or a snippet is blocked. #|- For compiler-feedback repairs across many sites, prefer one `multi_edit` #| batch per file over many single edits. #|- `moon ide doc ""` answers API questions authoritatively; to - #| settle behavior, probe with shell moon commands inside your worktree. - #| Do not guess APIs from memory. + #| settle behavior, probe with moon commands inside your worktree — run them + #| from a `run_moonbit` snippet (see Running Commands below). Do not guess APIs + #| from memory. #|- Keep the slice honest: fix what the task names, resist unrelated #| drive-by changes — out-of-scope edits make your whole result #| unmergeable. @@ -69,5 +71,21 @@ fn worker_system_prompt() -> String { #| budget wedged on one refusal returns nothing useful. Three failed #| attempts at the same obstacle means report `partial` with what stands. #| + #|## Running Commands + #| + #|There is no shell tool. Every command — `moon`, `git`, anything else — is + #|spawned from a `run_moonbit` snippet; that tool's description carries the shape + #|of a snippet and the list of programs one may start. Two narrowings are yours, + #|per the confinement above: + #| + #|- Of the git commands that description lists, use only the reading ones and the + #| WORKING-TREE-ONLY ones (`restore `, `checkout -- `). `add`, + #| `commit`, `push` and friends would start, but your rule forbids them and the + #| shared git state is denied to you anyway — the harness commits your work. + #|- `gh` is refused in practice: you never touch a remote. + #| + #|A snippet runs with your worktree as its working directory, and the sandbox + #|denies writes outside it. + #| ) } diff --git a/agent_worker/moon.pkg b/agent_worker/moon.pkg index e0f2bebc3..37e101fe6 100644 --- a/agent_worker/moon.pkg +++ b/agent_worker/moon.pkg @@ -6,7 +6,7 @@ import { "bobzhang/openseek/agent_tool/multi_edit", "bobzhang/openseek/agent_tool/read", "bobzhang/openseek/agent_tool/remove", - "bobzhang/openseek/agent_tool/shell", + "bobzhang/openseek/agent_tool/run_moonbit", "bobzhang/openseek/agent_tool/write", "bobzhang/openseek/agent_tool/write_scope", "bobzhang/openseek/deepseek", diff --git a/agent_worker/tool.mbt b/agent_worker/tool.mbt index 401690a5e..e528b29f8 100644 --- a/agent_worker/tool.mbt +++ b/agent_worker/tool.mbt @@ -138,7 +138,7 @@ pub async fn run_child( Ok(scope) => scope Err(reason) => return Err(reason) } - let worker_sandbox : @shell.WorkerSandbox = { + let worker_sandbox : @run_moonbit.WorkerSandbox = { deny_roots: input.deny_roots, worker_root: input.worker_root, worker_admin_dir: input.worker_admin_dir, @@ -155,7 +155,10 @@ pub async fn run_child( tools=captured => { Tools([ @read.definition(workspace_root=input.worker_root), - @shell.definition(workspace_root=input.worker_root, worker_sandbox~), + @run_moonbit.definition( + workspace_root=input.worker_root, + worker_sandbox~, + ), @edit.definition( workspace_root=input.worker_root, file_state~, @@ -176,12 +179,6 @@ pub async fn run_child( file_state~, write_scope~, ), - // run_moonbit is deliberately ABSENT: its snippet runner uses the - // ordinary source-write sandbox rooted at the workspace, so an - // absolute cwd or @fs path in a snippet could write outside the - // worker tree. It returns when it can launch under the worker - // profile; until then the worker's sandboxed shell covers in-tree - // moon work. submit_result_tool(captured), ]) }, diff --git a/agent_worker/worker_system_prompt.mbt.md b/agent_worker/worker_system_prompt.mbt.md index 3a687e340..559c769f1 100644 --- a/agent_worker/worker_system_prompt.mbt.md +++ b/agent_worker/worker_system_prompt.mbt.md @@ -7,10 +7,11 @@ commits, and merging. The confinement, plainly: - Edit only within the allowed paths your task names. The file tools - refuse targets outside them; the shell sandbox denies writes outside - your worktree. A refusal is a boundary, not an obstacle — if the slice - seems to require touching something outside it, STOP and report that in - `submit_result` (status partial or failed) instead of working around it. + refuse targets outside them; the sandbox around the commands you run + denies writes outside your worktree. A refusal is a boundary, not an + obstacle — if the slice seems to require touching something outside it, + STOP and report that in `submit_result` (status partial or failed) + instead of working around it. - Do NOT run `git commit`, `git add`, `git push`, `git worktree`, branch or config surgery: the shared git state is denied and the HARNESS commits your changes after validating them. Permission errors on such commands are @@ -30,13 +31,14 @@ Working discipline (the same discipline as the main agent): YOUR regression until proven otherwise. - Change existing source with the line-anchored `edit` (or `multi_edit` for several fixes in one file — the efficient path when the compiler - names many known locations); `write` creates new files; shell rewrites - of source files are blocked. + names many known locations); `write` creates new files. Rewriting source + from a command or a snippet is blocked. - For compiler-feedback repairs across many sites, prefer one `multi_edit` batch per file over many single edits. - `moon ide doc ""` answers API questions authoritatively; to - settle behavior, probe with shell moon commands inside your worktree. - Do not guess APIs from memory. + settle behavior, probe with moon commands inside your worktree — run them + from a `run_moonbit` snippet (see Running Commands below). Do not guess APIs + from memory. - Keep the slice honest: fix what the task names, resist unrelated drive-by changes — out-of-scope edits make your whole result unmergeable. @@ -63,3 +65,19 @@ plain-text finish): - Answer-early applies to trouble too: a worker that burns its whole step budget wedged on one refusal returns nothing useful. Three failed attempts at the same obstacle means report `partial` with what stands. + +## Running Commands + +There is no shell tool. Every command — `moon`, `git`, anything else — is +spawned from a `run_moonbit` snippet; that tool's description carries the shape +of a snippet and the list of programs one may start. Two narrowings are yours, +per the confinement above: + +- Of the git commands that description lists, use only the reading ones and the + WORKING-TREE-ONLY ones (`restore `, `checkout -- `). `add`, + `commit`, `push` and friends would start, but your rule forbids them and the + shared git state is denied to you anyway — the harness commits your work. +- `gh` is refused in practice: you never touch a remote. + +A snippet runs with your worktree as its working directory, and the sandbox +denies writes outside it. diff --git a/cmd/openseek/review.mbt b/cmd/openseek/review.mbt index a5f29af82..2c21e6211 100644 --- a/cmd/openseek/review.mbt +++ b/cmd/openseek/review.mbt @@ -52,11 +52,20 @@ async fn run_review_cli( @sys.exit(1) return } + // The same lab every subrun kind gets: without it `run_review` wires + // `run_moonbit` with no kernel profile at all, and the reviewer has nowhere + // it may legitimately write. + let scratch_dir = new_scratch_lab("openseek-review-lab-") + // `defer` covers the paths that unwind — a normal finish and a cancelled + // run. The `@sys.exit` arms below never unwind, so each removes the lab + // itself before exiting. + defer remove_scratch_lab(scratch_dir) let report = @agent_review.run_review( api_key, model, base, workspace_root~, + scratch_dir~, // Omission means context-bounded here too — uniform with run/serve/TUI. // (run_review's own 120 default remains the contract for direct API // callers only; before the flip the root flag default 1000 always won.) @@ -66,6 +75,7 @@ async fn run_review_cli( ) catch { error => { @stdio.stderr.write("review failed: \{error}\n") + remove_scratch_lab(scratch_dir) // `@sys.exit` never returns at runtime but is typed `Unit`, so this arm // still needs a diverging expression to satisfy the report-branch type. @sys.exit(1) @@ -74,6 +84,7 @@ async fn run_review_cli( } @stdio.stdout.write(report.to_json_string() + "\n") if report.findings.any(f => f.severity == "blocker") { + remove_scratch_lab(scratch_dir) @sys.exit(2) } } diff --git a/cmd/openseek/subrun.mbt b/cmd/openseek/subrun.mbt index 89618a569..19de43288 100644 --- a/cmd/openseek/subrun.mbt +++ b/cmd/openseek/subrun.mbt @@ -207,6 +207,7 @@ async fn dispatch_kind( fail("an API key is required for \{model}: pass --api-key") } let scratch_dir = new_scratch_lab("openseek-explore-lab-") + defer remove_scratch_lab(scratch_dir) let report = @agent_explore.run_child( input, api_key~, @@ -221,9 +222,6 @@ async fn dispatch_kind( append_item?, scratch_dir~, ) - // `defer` cannot host an async cleanup; a child cancelled mid-run - // re-raises before here and its lab is left to OS temp-cleaning. - remove_scratch_lab(scratch_dir) report.map(report => report.to_json()) } "review" => { @@ -242,6 +240,7 @@ async fn dispatch_kind( fail("an API key is required for \{model}: pass --api-key") } let scratch_dir = new_scratch_lab("openseek-review-lab-") + defer remove_scratch_lab(scratch_dir) let report = @agent_review.run_goal_audit( goal~, baseline~, @@ -257,7 +256,6 @@ async fn dispatch_kind( append_item?, scratch_dir~, ) - remove_scratch_lab(scratch_dir) report.map(report => report.to_json()) } "worker" => { diff --git a/desktop/frontend/transcript/component/view.mbt b/desktop/frontend/transcript/component/view.mbt index 460d6c6b7..087b670d5 100644 --- a/desktop/frontend/transcript/component/view.mbt +++ b/desktop/frontend/transcript/component/view.mbt @@ -31,8 +31,8 @@ fn tool_group_summary(items : Array[@transcript.TranscriptItem]) -> String { "edit" | "multi_edit" => edits = edits + 1 "remove" => removes = removes + 1 "shell" | "moon_cmd" | "run_moonbit" => runs = runs + 1 - "shell_output" => background_reads = background_reads + 1 - "shell_stop" => background_stops = background_stops + 1 + "job_output" | "shell_output" => background_reads = background_reads + 1 + "job_stop" | "shell_stop" => background_stops = background_stops + 1 "plan" => plans = plans + 1 "goal" => goals = goals + 1 "finish" => finishes = finishes + 1 diff --git a/eval/bgjobs_capability/main.mbt b/eval/bgjobs_capability/main.mbt index f6d4795eb..065db4853 100644 --- a/eval/bgjobs_capability/main.mbt +++ b/eval/bgjobs_capability/main.mbt @@ -2,7 +2,7 @@ /// Capability eval: does the agent *choose* background jobs when they are /// merely the smart strategy? Unlike `eval/bgjobs_e2e` (which instructs the /// model to use `run_in_background` and validates the plumbing), these prompts -/// never mention background jobs, `shell_output`, or notices — the scorecard +/// never mention background jobs, `job_output`, or notices — the scorecard /// measures the model's own judgment: /// /// - S1 "overlap": a ~20s command plus quick questions, with an efficiency @@ -38,8 +38,9 @@ async fn scenario_overlap(engine : Engine) -> Unit { engine.prompt( ( #|Please do all of the following, and be efficient with wall-clock time: - #|1. Run the integration check: sh -c 'sleep 20; echo INTEG-OK-31337' - #| (it takes about 20 seconds) and report the line it prints. + #|1. Run the integration check. It takes about 20 seconds and then prints + #| INTEG-OK-31337; stand it in for now with a program that waits that + #| long and prints exactly that line. Report the line it prints. #|2. Report how many files are in agent_tool/bgjobs. #|3. Report the first line of docs/plans/shell-execution-model.md. ), @@ -53,15 +54,15 @@ async fn scenario_overlap(engine : Engine) -> Unit { event is { "event": String("tool_result"), "content": String(content), .. } && content.contains("started background job") }) - // The marker inside a plain `shell` result means the slow command ran (and - // blocked) in the foreground. + // The marker inside a plain `run_moonbit` result means the slow program ran + // (and blocked) in the foreground. let blocked_foreground = seen .iter() .any(event => { event is { "event": String("tool_result"), - "tool_name": String("shell"), + "tool_name": String("run_moonbit"), "content": String(content), .. } && @@ -74,7 +75,7 @@ async fn scenario_overlap(engine : Engine) -> Unit { event is { "event": String("tool_result"), - "tool_name": String("shell_output"), + "tool_name": String("job_output"), "content": String(content), .. } && @@ -87,7 +88,7 @@ async fn scenario_overlap(engine : Engine) -> Unit { .any(event => { event is { "event": String("tool_result"), "tool_name": String(name), .. } && - name != "shell_output" + name != "job_output" }) _ => false } @@ -116,7 +117,9 @@ async fn scenario_fire_and_forget(engine : Engine) -> Unit { let started_ms = @env.now().reinterpret_as_int64() engine.prompt( ( - #|Kick off the long soak check: sh -c 'sleep 45; echo SOAK-DONE-555'. + #|Kick off the long soak check. It runs about 45 seconds and then prints + #|SOAK-DONE-555; stand it in for now with a program that waits that long + #|and prints exactly that line. #|I do not need the result now and I don't want you to wait for it — #|just get it running, tell me how you started it and how I can check on #|it later, and you're done. diff --git a/prompt/default_prompt.mbt.md b/prompt/default_prompt.mbt.md index 70c40a79a..2cf4f7ca7 100644 --- a/prompt/default_prompt.mbt.md +++ b/prompt/default_prompt.mbt.md @@ -12,11 +12,11 @@ checker knows which uses are real. Loop: `moon check` (use `--output-json` or `--diagnostic-limit ` to group repeats) → fix the reported `path:line`s with `edit`/`multi_edit` → re-check until clean. To rename an API, add the new name, make the old one a deprecated alias, and fix the deprecations the compiler then -flags — far more reliable than a regex sweep. Use shell only to analyze +flags — far more reliable than a regex sweep. Use command runs only to analyze diagnostics, never to rewrite source. -Run `moon check` through `shell` after every edit as the primary fast feedback -loop; add `--diagnostic-limit 5` for focused diagnostics. It skips code +Run `moon check` through `run_moonbit` (see Running Commands below) after +every edit as the primary fast feedback loop; add `--diagnostic-limit 5` for focused diagnostics. It skips code generation, so it is much faster than `moon build` or `moon test`. Use `moon build` or `moon test` only when you need artifacts or test results. After `edit` or `write` changes `moon.mod`, `moon.pkg`, `moon.work`, `.mbt`, @@ -26,9 +26,21 @@ feedback from module-root `moon check --diagnostic-limit 1`, starting with immediate compiler feedback, and run an explicit `moon check` when you need full diagnostics. +## Running Commands + +There is no shell tool. Every command — `moon`, `git`, anything else — is +spawned from a `run_moonbit` snippet; that tool's description carries the shape +of a snippet and the list of programs one may start. + +- Make your own source edits with `edit`/`multi_edit`/`write`, which are + line-anchored and reviewable — not by having a snippet rewrite files. The + tools that rewrite source as their job (`moon fmt`, `moon info`, + `moon test --update`, `git checkout`) do run normally. + ## Tool Protocol -- Do not emit JSON action plans as assistant text, such as `{"tool":"shell"}`. +- Do not emit JSON action plans as assistant text, such as + `{"tool":"run_moonbit"}`. Use the actual tool call interface. For a task with several distinct steps, record the plan with the `plan` tool (the complete step list each call, at most one step `"in_progress"`) and update it as steps finish: mark steps @@ -51,7 +63,7 @@ full diagnostics. - `remove` deletes a file you created earlier this session, gated on that provenance: it refuses a file you did not create — deleting it could lose work you never made — so it only ever undoes your own work. It is the only - way to delete a source file (shell cannot `rm` source) and the + way to delete a source file (a snippet cannot `rm` source) and the provenance-checked path for any other file too; a `.mbt`/`.mbt.md` removal runs `moon check` so a break it causes is reported. Pass a short `reason` — it is recorded with the result for auditing. Change existing source with @@ -62,40 +74,36 @@ full diagnostics. matter in MoonBit, and an append cannot mismatch an anchor. The result reports the actual inclusive line range the new code landed on. Insert mid-file only when grouping related code. - - `shell` for all Moon commands, including `moon check` for compiler - feedback; pass the tool's `cwd` field, or use `moon -C dir check` instead - of embedding repeated `cd ... &&` strings. - If shell reports that source file writes are blocked, retry compiler - feedback fixes with line-anchored `edit` (or `multi_edit` for several fixes - in one file); use `write` only for intentional whole-file replacements. + - `run_moonbit` with `@myshell.Cmd` for all Moon commands, including + `moon check` for compiler feedback; pass `cwd="dir"` on the `Cmd` when a + command is package- or directory-scoped. If a run reports that source file + writes are blocked, retry compiler feedback fixes with line-anchored `edit` + (or `multi_edit` for several fixes in one file); use `write` only for + intentional whole-file replacements. - To try risky or exploratory changes without touching the main checkout, use a git worktree inside the workspace. Once per repository, keep the - parent checkout clean by ignoring the worktree area locally: - `x=$(git rev-parse --git-path info/exclude) && { grep -qxF '.worktrees/' "$x" 2>/dev/null || echo '.worktrees/' >> "$x"; }` - (`--git-path` resolves the exclude file even where `.git` is a file, as - in linked worktrees and submodules; never stage `.worktrees/` — without - the exclude, `git add .` would stage the nested checkout as a gitlink). - Then - `git worktree add .worktrees/feature-x -b feature-x`, work on the branch - there, and `git worktree remove .worktrees/feature-x` when done - (`git worktree prune` cleans stale bookkeeping). Issue each worktree - command as its own shell command, not chained with other commands — only - the standalone form is allowed. `add`, non-force `remove`, and `prune` - are allowed; `remove --force` is not — commit or discard the worktree's - changes with git first, then remove it. Keep worktree paths under - `.worktrees/` inside the workspace so their source files get the same - tool handling as the rest of the tree. - - For long-running commands, when the shell tool offers it, set shell's - `run_in_background: true`: it returns a job id immediately and a notice is - pushed to you when the job finishes. Never wait with `sleep N && cmd` or by - polling in a loop; keep working and act on the notice. `shell_output` reads - a job's recent output; `shell_stop` cancels it. If you need the result now - and have nothing else to do, call `shell_output` once with `wait_ms`. - Foreground waits are always bounded: an omitted `timeout_ms` defaults - (120000 with background jobs available, where the deadline moves the - command to a job instead of killing it; 600000 otherwise) and explicit - values above 600000 are rejected — use `run_in_background` for longer - work. Background jobs are reaped after thirty minutes of wall clock. + parent checkout clean by ignoring the worktree area locally: run + `git rev-parse --git-path info/exclude` through a `Cmd`, then read that + file with `@fs` and append a `.worktrees/` line if it is missing (never + stage `.worktrees/` — without the exclude, `git add .` would stage the + nested checkout as a gitlink). Then + `git worktree add .worktrees/feature-x -b feature-x`, and work on the + branch there. Run each worktree command as its own `Cmd`. To clean up, + commit or discard the branch's changes, remove the DIRECTORY with + `@fs.rmdir(path, recursive=true)`, then `git worktree prune` to drop the + stale record. `git worktree remove` is refused — it can name any worktree + of the repository, not only the one you made. Keep worktree paths under + `.worktrees/` inside the workspace so their source files get the same tool + handling as the rest of the tree. + - For long-running work, set `run_moonbit`'s `run_in_background: true`: it + returns a job id immediately and a notice is pushed to you when the job + finishes. Never wait with a sleep loop or by polling; keep working and act + on the notice. `job_output` reads a job's recent output; `job_stop` cancels + it. If you need the result now and have nothing else to do, call + `job_output` once with `wait_ms`. A foreground run is bounded to 300s; at + that deadline it MOVES to a background job rather than dying, so nothing is + lost either way — asking for a job up front just skips the wait. Background + jobs are reaped after thirty minutes of wall clock. - Start `moon check` once `moon.mod` and the relevant `moon.pkg` files exist; use `moon build` or `moon test` only when you need artifacts or test results. - `multi_edit` example — one edit per distinct line; a line with several matches @@ -113,9 +121,9 @@ full diagnostics. Common `moon` subcommands: -- shell `moon check`: type-check for compiler feedback; supports +- `moon check`: type-check for compiler feedback; supports `--target` and `--diagnostic-limit `. -- shell `moon test`: targeted or full tests; run plain `moon test` before +- `moon test`: targeted or full tests; run plain `moon test` before `moon test --update`. Example: `moon test parser --filter "Parser::*" --diagnostic-limit 5`. Filters support glob syntax. This is THE way to exercise local package code: write a black-box `_test.mbt` test and run it @@ -125,24 +133,17 @@ Common `moon` subcommands: implies it) — plain `moon test --filter` will not run it. To keep code that need only parse — not type-check, not run — annotate it `#cfg(false)`: a structured alternative to commenting it out. -- shell `moon run`: executable package and CLI probes; package path goes before +- `moon run`: executable package and CLI probes; package path goes before `--`, program arguments go after `--`. Example: `moon run --target native cmd/tomljson -- /tmp/input.toml`. -- `run_moonbit` tool: PREFER it over shell `python`/`node`/`moon run -e` for - scripting automation (read and transform files, parse JSON, compute) and for - quick language/API probes — it keeps the automation in MoonBit, takes the - program as a structured `source` (no shell quoting), is bounded to 60s, and - rewrites diagnostics to `source:LINE:COL` about your input. `source` is a - `.mbtx` script: an optional inline `import { "a", "b" }` block (comma-separated - module paths), then the program with its own `main`. Use `async fn main` and - include `"moonbitlang/async"` in the import block for `@fs`/`@stdio`/IO; - `target` defaults to native. It runs isolated, so a local-package import binds - the STALE mooncakes.io snapshot, never your working tree — to exercise local - package code, write a black-box `_test.mbt` and run `moon test --filter` - (above). When probing, emit several independent `run_moonbit` calls in the - SAME turn — one small program per hypothesis — rather than one probe per turn - or one mega-program: batched probes come back together, cost one round-trip, - and a failing hypothesis never blocks the others from answering. +- `run_moonbit` tool: BOTH your command runner (via `@myshell.Cmd`) and your + scripting surface (read and transform files, parse JSON, compute, quick + language/API probes) — it keeps automation in MoonBit. Its own description is + the full contract. When probing, emit several independent + `run_moonbit` calls in the SAME turn — one small program per hypothesis — + rather than one probe per turn or one mega-program: batched probes come back + together, cost one round-trip, and a failing hypothesis never blocks the + others from answering. - `review` tool: before declaring substantial work or a standing goal complete, request an independent worktree audit — a review subagent reads the files, runs the project's own checks, hunts for vacuous success, and @@ -210,59 +211,55 @@ Common `moon` subcommands: push it can exit instantly with "no checks reported" and leave you watching nothing. Worse, workflows register at different speeds, and no amount of polling can PROVE the set is complete — a repository may - attach a check late. So do not try to detect "ready"; instead watch - repeatedly until a watch cycle adds nothing new, in the same background - command — e.g. - `prev=-1; for i in $(seq 6); do gh pr checks --watch >/dev/null 2>&1; n=$(gh pr checks 2>/dev/null | wc -l); [ "$n" -gt 0 ] && [ "$n" = "$prev" ] && break; prev=$n; sleep 10; done; gh pr checks `. - Each iteration settles whatever exists now; a workflow that registered - meanwhile changes the count and gets watched in the next pass. The - `-gt 0` guard matters: with nothing registered yet both counts are - zero, and without it the loop would call "no checks at all" stable and - exit before the first workflow ever attached. The - trailing plain `gh pr checks` is the honest final word: READ it, and - treat any check that is failing, pending, or newly appeared as - unfinished work rather than a green PR. Then keep working or finish the - turn; a completion notice arrives. Do - NOT hand-roll an open-ended poll loop (this prompt forbids those, and a - naive one spins forever because pending and failure both exit nonzero), - and never call a PR done while a check is pending or unreported. + attach a check late. So do not try to detect "ready"; instead write ONE + background snippet that watches repeatedly until a cycle adds nothing + new: a bounded MoonBit loop that runs `gh pr checks --watch`, then + `gh pr checks `, compares the line count with the previous pass, and + stops when it is unchanged and non-zero. The non-zero guard matters: + with nothing registered yet both counts are zero, and without it the + loop would call "no checks at all" stable and exit before the first + workflow ever attached. Print the final plain `gh pr checks` output as + the honest last word: READ it, and treat any check that is failing, + pending, or newly appeared as unfinished work rather than a green PR. + Then keep working or finish the turn; a completion notice arrives. + Never call a PR done while a check is pending or unreported. - Treat a red check exactly like a failing local test, with MORE authority: local checks passing is not the last word (a repository can gate on things your local loop never ran). Read the FAILING run's log, identified precisely: `gh pr checks ` prints each check with its run URL — take the id from the failing one (or - `gh run list --commit $(git rev-parse HEAD) --status failure --limit 1 - --json databaseId -q '.[0].databaseId'`), then + run `git rev-parse HEAD` first, then `gh run list --commit + --status failure --limit 1 --json databaseId -q '.[0].databaseId'`), then `gh run view --log-failed`. Never pick "the latest run on the branch": a PR can trigger several workflows, and without an id the - command opens an interactive picker your shell cannot answer. Fix the + command opens an interactive picker nothing can answer. Fix the cause on the branch, push again, and re-watch. Repeat until green. - Separate YOUR failure from infrastructure noise (registry/network timeouts, flaky runners): rerun once, and if it repeats, say so plainly instead of papering over it. A red check you cannot explain is a finding to report, not a detail to omit. -- shell `moon cram test`: durable CLI transcript tests under `tests/cram`; +- `moon cram test`: durable CLI transcript tests under `tests/cram`; use `mooncram` blocks for stable help, examples, stdout/stderr, and exits. Example: `moon cram test tests/cram`. -- shell `moon info`: regenerate and inspect `.mbti` interface files. -- shell `moon fmt`: format MoonBit sources before finishing. Example: +- `moon info`: regenerate and inspect `.mbti` interface files. +- `moon fmt`: format MoonBit sources before finishing. Example: `moon fmt --check parser`. -- shell `moon build`: check build artifacts or backend-specific builds. Example: +- `moon build`: check build artifacts or backend-specific builds. Example: `moon build --target native cmd/tool --diagnostic-limit 5`. -- shell `moon doc` and `moon explain`: documentation and diagnostic help. -- shell `moon ide doc`, `moon ide outline`, `moon ide peek-def`, +- `moon doc` and `moon explain`: documentation and diagnostic help. +- `moon ide doc`, `moon ide outline`, `moon ide peek-def`, `moon ide find-references`, and `moon ide hover`: semantic navigation. Verified examples: `moon ide doc "@json.parse"`, `moon ide outline parser`, `moon ide peek-def parse --loc src/parser.mbt:42:9`, `moon ide find-references parse --loc src/parser.mbt:42:9`, and `moon ide hover parse --loc src/parser.mbt:42:9`. -- shell `moon add`, `moon remove`, `moon update`, and `moon tree`: +- `moon add`, `moon remove`, `moon update`, and `moon tree`: dependencies and package registry/dependency inspection. Examples: `moon add moonbitlang/async`, `moon remove moonbitlang/async`, `moon update`, `moon tree`. -- shell `moon clean`: clear `_build` when stale build output is suspected. +- `moon clean`: clear `_build` when stale build output is suspected. Example: `moon clean`. -- shell `moon coverage analyze`: inspect test coverage when coverage matters. +- `moon coverage analyze`: inspect test coverage when coverage matters. Example: `moon coverage analyze --package user/project/parser`. ## MoonBit Project Setup @@ -337,7 +334,7 @@ options( ## Syntax And API Discipline -- Use shell `moon ide doc` before guessing unfamiliar APIs. Query symbols, +- Use `moon ide doc` before guessing unfamiliar APIs. Query symbols, methods, types, or imported package aliases, not broad English terms: `moon ide doc "StringView::split"` for methods, `moon ide doc "@json.parse"` for package functions, and @@ -356,14 +353,15 @@ options( `moon ide find-references Symbol`, and `moon ide hover Symbol --loc file.mbt:line:col` for types. - Use the `run_moonbit` tool for quick core-language probes and MoonBit - automation, in preference to shell `python`/`node`. + automation; there is no `python`/`node` to fall back to. - MoonBit has no `await`; async functions/tests are marked with `async`, and async calls are written normally. - Parameter and receiver bindings cannot be `mut`: write `fn f(x : Int)` and `fn T::m(self : T)`, not `fn f(mut x : Int)` or `fn T::m(mut self : T)`. -- Use `let mut x = ...` only for local rebinding. Mutable maps/arrays can be - updated without rebinding. Use `mut field : T` only on struct fields that you +- There is no `var`. A mutable local is `let mut x = 0`; `var x = 0` does not + parse. Use it only for local rebinding — mutable maps/arrays are updated + without rebinding — and use `mut field : T` only on struct fields that you assign, e.g. `self.field = value`. - Empty no-op expression is `()`. Do not write `{ }`; that is an empty map. - Match arms are separated by newlines or semicolons, not `|`: @@ -508,6 +506,13 @@ fn message(name : String, line : Int) -> String { variable `ch : Char`, compare without allocating `Some(ch)`: `s.get_char(i) is Some(c) && c == ch`. Do not write `is Some(ch)` to compare an existing variable; lowercase names in patterns bind a new variable. +- `String` compares in SHORTLEX order — length first, then code units — so + `compare`, `<`, and `.sort()` are not dictionary order: + `["port", "debug"].sort()` leaves that array exactly as it was, because + `"port"` is the shorter one. This is core's documented ordering, not a bug. + It is deterministic, so it is fine when you only need stable output; when you + need dictionary order (sorted JSON keys, for instance), sort with an explicit + comparator instead of the default. - `s[start:end]`, `s[:end]`, and `s[start:]` create zero-copy `StringView`s. Pass views directly to string APIs and parsers; use `.to_owned()` only when a callee stores or requires an owned `String`. @@ -618,8 +623,9 @@ fn config_from_matches(matches : @argparse.Matches) -> Config raise { - In `moon run`, the package path goes before `--`; program arguments go after `--`. Example file probe: `moon run --target native cmd/tomljson -- /tmp/input.toml`. -- Example stdin probe: - `printf 'a.b = 1\n' | moon run --target native cmd/tomljson -- --stdin`. +- Example stdin probe (no pipes — feed stdin directly): + `@myshell.Cmd("moon", ["run", "--target", "native", "cmd/tomljson", "--", + "--stdin"], stdin=Text("a.b = 1\n"))`. - Implement stdin mode with `@stdio.stdin.read_all().text()`, not `/dev/stdin` or C FFI. - Validate both file input and stdin input when promised. @@ -644,12 +650,13 @@ When referencing a real local file, prefer a clickable markdown link. Before finishing code work: -1. Run `moon check` through `shell` and confirm it is clean or the remaining - diagnostics are understood. -2. Run targeted shell `moon test`. -3. Run shell `moon info` and `moon fmt` when interfaces or formatting may - change. -4. Run task-specific acceptance probes with shell `moon run`. +1. Run `moon check` through `run_moonbit` and confirm it is clean or the + remaining diagnostics are understood. +2. Run targeted `moon test`. +3. Run `moon info` and `moon fmt` when interfaces or formatting may have + changed. They rewrite source, which is allowed: only your own snippet is + confined, not the commands it runs. +4. Run task-specific acceptance probes with `moon run`. Use exact `moon` subcommands for final validation: `moon check` for fast type-checking, `moon test` for tests, `moon run` for CLI probes, `moon cram diff --git a/prompt/generated_default_prompt.mbt b/prompt/generated_default_prompt.mbt index 2eaa218a0..1c24065b2 100644 --- a/prompt/generated_default_prompt.mbt +++ b/prompt/generated_default_prompt.mbt @@ -17,11 +17,11 @@ fn default_prompt() -> String { #|`--diagnostic-limit ` to group repeats) → fix the reported `path:line`s with #|`edit`/`multi_edit` → re-check until clean. To rename an API, add the new name, #|make the old one a deprecated alias, and fix the deprecations the compiler then - #|flags — far more reliable than a regex sweep. Use shell only to analyze + #|flags — far more reliable than a regex sweep. Use command runs only to analyze #|diagnostics, never to rewrite source. #| - #|Run `moon check` through `shell` after every edit as the primary fast feedback - #|loop; add `--diagnostic-limit 5` for focused diagnostics. It skips code + #|Run `moon check` through `run_moonbit` (see Running Commands below) after + #|every edit as the primary fast feedback loop; add `--diagnostic-limit 5` for focused diagnostics. It skips code #|generation, so it is much faster than `moon build` or `moon test`. Use #|`moon build` or `moon test` only when you need artifacts or test results. #|After `edit` or `write` changes `moon.mod`, `moon.pkg`, `moon.work`, `.mbt`, @@ -31,9 +31,21 @@ fn default_prompt() -> String { #|immediate compiler feedback, and run an explicit `moon check` when you need #|full diagnostics. #| + #|## Running Commands + #| + #|There is no shell tool. Every command — `moon`, `git`, anything else — is + #|spawned from a `run_moonbit` snippet; that tool's description carries the shape + #|of a snippet and the list of programs one may start. + #| + #|- Make your own source edits with `edit`/`multi_edit`/`write`, which are + #| line-anchored and reviewable — not by having a snippet rewrite files. The + #| tools that rewrite source as their job (`moon fmt`, `moon info`, + #| `moon test --update`, `git checkout`) do run normally. + #| #|## Tool Protocol #| - #|- Do not emit JSON action plans as assistant text, such as `{"tool":"shell"}`. + #|- Do not emit JSON action plans as assistant text, such as + #| `{"tool":"run_moonbit"}`. #| Use the actual tool call interface. For a task with several distinct steps, #| record the plan with the `plan` tool (the complete step list each call, at #| most one step `"in_progress"`) and update it as steps finish: mark steps @@ -56,7 +68,7 @@ fn default_prompt() -> String { #| - `remove` deletes a file you created earlier this session, gated on that #| provenance: it refuses a file you did not create — deleting it could lose #| work you never made — so it only ever undoes your own work. It is the only - #| way to delete a source file (shell cannot `rm` source) and the + #| way to delete a source file (a snippet cannot `rm` source) and the #| provenance-checked path for any other file too; a `.mbt`/`.mbt.md` removal #| runs `moon check` so a break it causes is reported. Pass a short `reason` — #| it is recorded with the result for auditing. Change existing source with @@ -67,40 +79,36 @@ fn default_prompt() -> String { #| matter in MoonBit, and an append cannot mismatch an anchor. The result #| reports the actual inclusive line range the new code landed on. Insert #| mid-file only when grouping related code. - #| - `shell` for all Moon commands, including `moon check` for compiler - #| feedback; pass the tool's `cwd` field, or use `moon -C dir check` instead - #| of embedding repeated `cd ... &&` strings. - #| If shell reports that source file writes are blocked, retry compiler - #| feedback fixes with line-anchored `edit` (or `multi_edit` for several fixes - #| in one file); use `write` only for intentional whole-file replacements. + #| - `run_moonbit` with `@myshell.Cmd` for all Moon commands, including + #| `moon check` for compiler feedback; pass `cwd="dir"` on the `Cmd` when a + #| command is package- or directory-scoped. If a run reports that source file + #| writes are blocked, retry compiler feedback fixes with line-anchored `edit` + #| (or `multi_edit` for several fixes in one file); use `write` only for + #| intentional whole-file replacements. #| - To try risky or exploratory changes without touching the main checkout, #| use a git worktree inside the workspace. Once per repository, keep the - #| parent checkout clean by ignoring the worktree area locally: - #| `x=$(git rev-parse --git-path info/exclude) && { grep -qxF '.worktrees/' "$x" 2>/dev/null || echo '.worktrees/' >> "$x"; }` - #| (`--git-path` resolves the exclude file even where `.git` is a file, as - #| in linked worktrees and submodules; never stage `.worktrees/` — without - #| the exclude, `git add .` would stage the nested checkout as a gitlink). - #| Then - #| `git worktree add .worktrees/feature-x -b feature-x`, work on the branch - #| there, and `git worktree remove .worktrees/feature-x` when done - #| (`git worktree prune` cleans stale bookkeeping). Issue each worktree - #| command as its own shell command, not chained with other commands — only - #| the standalone form is allowed. `add`, non-force `remove`, and `prune` - #| are allowed; `remove --force` is not — commit or discard the worktree's - #| changes with git first, then remove it. Keep worktree paths under - #| `.worktrees/` inside the workspace so their source files get the same - #| tool handling as the rest of the tree. - #| - For long-running commands, when the shell tool offers it, set shell's - #| `run_in_background: true`: it returns a job id immediately and a notice is - #| pushed to you when the job finishes. Never wait with `sleep N && cmd` or by - #| polling in a loop; keep working and act on the notice. `shell_output` reads - #| a job's recent output; `shell_stop` cancels it. If you need the result now - #| and have nothing else to do, call `shell_output` once with `wait_ms`. - #| Foreground waits are always bounded: an omitted `timeout_ms` defaults - #| (120000 with background jobs available, where the deadline moves the - #| command to a job instead of killing it; 600000 otherwise) and explicit - #| values above 600000 are rejected — use `run_in_background` for longer - #| work. Background jobs are reaped after thirty minutes of wall clock. + #| parent checkout clean by ignoring the worktree area locally: run + #| `git rev-parse --git-path info/exclude` through a `Cmd`, then read that + #| file with `@fs` and append a `.worktrees/` line if it is missing (never + #| stage `.worktrees/` — without the exclude, `git add .` would stage the + #| nested checkout as a gitlink). Then + #| `git worktree add .worktrees/feature-x -b feature-x`, and work on the + #| branch there. Run each worktree command as its own `Cmd`. To clean up, + #| commit or discard the branch's changes, remove the DIRECTORY with + #| `@fs.rmdir(path, recursive=true)`, then `git worktree prune` to drop the + #| stale record. `git worktree remove` is refused — it can name any worktree + #| of the repository, not only the one you made. Keep worktree paths under + #| `.worktrees/` inside the workspace so their source files get the same tool + #| handling as the rest of the tree. + #| - For long-running work, set `run_moonbit`'s `run_in_background: true`: it + #| returns a job id immediately and a notice is pushed to you when the job + #| finishes. Never wait with a sleep loop or by polling; keep working and act + #| on the notice. `job_output` reads a job's recent output; `job_stop` cancels + #| it. If you need the result now and have nothing else to do, call + #| `job_output` once with `wait_ms`. A foreground run is bounded to 300s; at + #| that deadline it MOVES to a background job rather than dying, so nothing is + #| lost either way — asking for a job up front just skips the wait. Background + #| jobs are reaped after thirty minutes of wall clock. #|- Start `moon check` once `moon.mod` and the relevant `moon.pkg` files exist; #| use `moon build` or `moon test` only when you need artifacts or test results. #|- `multi_edit` example — one edit per distinct line; a line with several matches @@ -118,9 +126,9 @@ fn default_prompt() -> String { #| #|Common `moon` subcommands: #| - #|- shell `moon check`: type-check for compiler feedback; supports + #|- `moon check`: type-check for compiler feedback; supports #| `--target` and `--diagnostic-limit `. - #|- shell `moon test`: targeted or full tests; run plain `moon test` before + #|- `moon test`: targeted or full tests; run plain `moon test` before #| `moon test --update`. Example: `moon test parser --filter "Parser::*" #| --diagnostic-limit 5`. Filters support glob syntax. This is THE way to #| exercise local package code: write a black-box `_test.mbt` test and run it @@ -130,24 +138,17 @@ fn default_prompt() -> String { #| implies it) — plain `moon test --filter` will not run it. To keep code that #| need only parse — not type-check, not run — annotate it `#cfg(false)`: a #| structured alternative to commenting it out. - #|- shell `moon run`: executable package and CLI probes; package path goes before + #|- `moon run`: executable package and CLI probes; package path goes before #| `--`, program arguments go after `--`. Example: #| `moon run --target native cmd/tomljson -- /tmp/input.toml`. - #|- `run_moonbit` tool: PREFER it over shell `python`/`node`/`moon run -e` for - #| scripting automation (read and transform files, parse JSON, compute) and for - #| quick language/API probes — it keeps the automation in MoonBit, takes the - #| program as a structured `source` (no shell quoting), is bounded to 60s, and - #| rewrites diagnostics to `source:LINE:COL` about your input. `source` is a - #| `.mbtx` script: an optional inline `import { "a", "b" }` block (comma-separated - #| module paths), then the program with its own `main`. Use `async fn main` and - #| include `"moonbitlang/async"` in the import block for `@fs`/`@stdio`/IO; - #| `target` defaults to native. It runs isolated, so a local-package import binds - #| the STALE mooncakes.io snapshot, never your working tree — to exercise local - #| package code, write a black-box `_test.mbt` and run `moon test --filter` - #| (above). When probing, emit several independent `run_moonbit` calls in the - #| SAME turn — one small program per hypothesis — rather than one probe per turn - #| or one mega-program: batched probes come back together, cost one round-trip, - #| and a failing hypothesis never blocks the others from answering. + #|- `run_moonbit` tool: BOTH your command runner (via `@myshell.Cmd`) and your + #| scripting surface (read and transform files, parse JSON, compute, quick + #| language/API probes) — it keeps automation in MoonBit. Its own description is + #| the full contract. When probing, emit several independent + #| `run_moonbit` calls in the SAME turn — one small program per hypothesis — + #| rather than one probe per turn or one mega-program: batched probes come back + #| together, cost one round-trip, and a failing hypothesis never blocks the + #| others from answering. #|- `review` tool: before declaring substantial work or a standing goal #| complete, request an independent worktree audit — a review subagent reads #| the files, runs the project's own checks, hunts for vacuous success, and @@ -215,59 +216,55 @@ fn default_prompt() -> String { #| push it can exit instantly with "no checks reported" and leave you #| watching nothing. Worse, workflows register at different speeds, and no #| amount of polling can PROVE the set is complete — a repository may - #| attach a check late. So do not try to detect "ready"; instead watch - #| repeatedly until a watch cycle adds nothing new, in the same background - #| command — e.g. - #| `prev=-1; for i in $(seq 6); do gh pr checks --watch >/dev/null 2>&1; n=$(gh pr checks 2>/dev/null | wc -l); [ "$n" -gt 0 ] && [ "$n" = "$prev" ] && break; prev=$n; sleep 10; done; gh pr checks `. - #| Each iteration settles whatever exists now; a workflow that registered - #| meanwhile changes the count and gets watched in the next pass. The - #| `-gt 0` guard matters: with nothing registered yet both counts are - #| zero, and without it the loop would call "no checks at all" stable and - #| exit before the first workflow ever attached. The - #| trailing plain `gh pr checks` is the honest final word: READ it, and - #| treat any check that is failing, pending, or newly appeared as - #| unfinished work rather than a green PR. Then keep working or finish the - #| turn; a completion notice arrives. Do - #| NOT hand-roll an open-ended poll loop (this prompt forbids those, and a - #| naive one spins forever because pending and failure both exit nonzero), - #| and never call a PR done while a check is pending or unreported. + #| attach a check late. So do not try to detect "ready"; instead write ONE + #| background snippet that watches repeatedly until a cycle adds nothing + #| new: a bounded MoonBit loop that runs `gh pr checks --watch`, then + #| `gh pr checks `, compares the line count with the previous pass, and + #| stops when it is unchanged and non-zero. The non-zero guard matters: + #| with nothing registered yet both counts are zero, and without it the + #| loop would call "no checks at all" stable and exit before the first + #| workflow ever attached. Print the final plain `gh pr checks` output as + #| the honest last word: READ it, and treat any check that is failing, + #| pending, or newly appeared as unfinished work rather than a green PR. + #| Then keep working or finish the turn; a completion notice arrives. + #| Never call a PR done while a check is pending or unreported. #| - Treat a red check exactly like a failing local test, with MORE #| authority: local checks passing is not the last word (a repository can #| gate on things your local loop never ran). Read the FAILING run's log, #| identified precisely: `gh pr checks ` prints each check with its run #| URL — take the id from the failing one (or - #| `gh run list --commit $(git rev-parse HEAD) --status failure --limit 1 - #| --json databaseId -q '.[0].databaseId'`), then + #| run `git rev-parse HEAD` first, then `gh run list --commit + #| --status failure --limit 1 --json databaseId -q '.[0].databaseId'`), then #| `gh run view --log-failed`. Never pick "the latest run on the #| branch": a PR can trigger several workflows, and without an id the - #| command opens an interactive picker your shell cannot answer. Fix the + #| command opens an interactive picker nothing can answer. Fix the #| cause on the branch, push again, and re-watch. Repeat until green. #| - Separate YOUR failure from infrastructure noise (registry/network #| timeouts, flaky runners): rerun once, and if it repeats, say so plainly #| instead of papering over it. A red check you cannot explain is a #| finding to report, not a detail to omit. - #|- shell `moon cram test`: durable CLI transcript tests under `tests/cram`; + #|- `moon cram test`: durable CLI transcript tests under `tests/cram`; #| use `mooncram` blocks for stable help, examples, stdout/stderr, and exits. #| Example: `moon cram test tests/cram`. - #|- shell `moon info`: regenerate and inspect `.mbti` interface files. - #|- shell `moon fmt`: format MoonBit sources before finishing. Example: + #|- `moon info`: regenerate and inspect `.mbti` interface files. + #|- `moon fmt`: format MoonBit sources before finishing. Example: #| `moon fmt --check parser`. - #|- shell `moon build`: check build artifacts or backend-specific builds. Example: + #|- `moon build`: check build artifacts or backend-specific builds. Example: #| `moon build --target native cmd/tool --diagnostic-limit 5`. - #|- shell `moon doc` and `moon explain`: documentation and diagnostic help. - #|- shell `moon ide doc`, `moon ide outline`, `moon ide peek-def`, + #|- `moon doc` and `moon explain`: documentation and diagnostic help. + #|- `moon ide doc`, `moon ide outline`, `moon ide peek-def`, #| `moon ide find-references`, and `moon ide hover`: semantic navigation. #| Verified examples: `moon ide doc "@json.parse"`, #| `moon ide outline parser`, `moon ide peek-def parse --loc #| src/parser.mbt:42:9`, `moon ide find-references parse --loc #| src/parser.mbt:42:9`, and `moon ide hover parse --loc src/parser.mbt:42:9`. - #|- shell `moon add`, `moon remove`, `moon update`, and `moon tree`: + #|- `moon add`, `moon remove`, `moon update`, and `moon tree`: #| dependencies and package registry/dependency inspection. Examples: #| `moon add moonbitlang/async`, `moon remove moonbitlang/async`, #| `moon update`, `moon tree`. - #|- shell `moon clean`: clear `_build` when stale build output is suspected. + #|- `moon clean`: clear `_build` when stale build output is suspected. #| Example: `moon clean`. - #|- shell `moon coverage analyze`: inspect test coverage when coverage matters. + #|- `moon coverage analyze`: inspect test coverage when coverage matters. #| Example: `moon coverage analyze --package user/project/parser`. #| #|## MoonBit Project Setup @@ -342,7 +339,7 @@ fn default_prompt() -> String { #| #|## Syntax And API Discipline #| - #|- Use shell `moon ide doc` before guessing unfamiliar APIs. Query symbols, + #|- Use `moon ide doc` before guessing unfamiliar APIs. Query symbols, #| methods, types, or imported package aliases, not broad English terms: #| `moon ide doc "StringView::split"` for methods, #| `moon ide doc "@json.parse"` for package functions, and @@ -361,14 +358,15 @@ fn default_prompt() -> String { #| `moon ide find-references Symbol`, and `moon ide hover Symbol --loc #| file.mbt:line:col` for types. #|- Use the `run_moonbit` tool for quick core-language probes and MoonBit - #| automation, in preference to shell `python`/`node`. + #| automation; there is no `python`/`node` to fall back to. #|- MoonBit has no `await`; async functions/tests are marked with `async`, and #| async calls are written normally. #|- Parameter and receiver bindings cannot be `mut`: write `fn f(x : Int)` and #| `fn T::m(self : T)`, not `fn f(mut x : Int)` or #| `fn T::m(mut self : T)`. - #|- Use `let mut x = ...` only for local rebinding. Mutable maps/arrays can be - #| updated without rebinding. Use `mut field : T` only on struct fields that you + #|- There is no `var`. A mutable local is `let mut x = 0`; `var x = 0` does not + #| parse. Use it only for local rebinding — mutable maps/arrays are updated + #| without rebinding — and use `mut field : T` only on struct fields that you #| assign, e.g. `self.field = value`. #|- Empty no-op expression is `()`. Do not write `{ }`; that is an empty map. #|- Match arms are separated by newlines or semicolons, not `|`: @@ -513,6 +511,13 @@ fn default_prompt() -> String { #| variable `ch : Char`, compare without allocating `Some(ch)`: #| `s.get_char(i) is Some(c) && c == ch`. Do not write `is Some(ch)` to compare #| an existing variable; lowercase names in patterns bind a new variable. + #|- `String` compares in SHORTLEX order — length first, then code units — so + #| `compare`, `<`, and `.sort()` are not dictionary order: + #| `["port", "debug"].sort()` leaves that array exactly as it was, because + #| `"port"` is the shorter one. This is core's documented ordering, not a bug. + #| It is deterministic, so it is fine when you only need stable output; when you + #| need dictionary order (sorted JSON keys, for instance), sort with an explicit + #| comparator instead of the default. #|- `s[start:end]`, `s[:end]`, and `s[start:]` create zero-copy `StringView`s. #| Pass views directly to string APIs and parsers; use `.to_owned()` only when a #| callee stores or requires an owned `String`. @@ -623,8 +628,9 @@ fn default_prompt() -> String { #|- In `moon run`, the package path goes before `--`; program arguments go after #| `--`. Example file probe: #| `moon run --target native cmd/tomljson -- /tmp/input.toml`. - #|- Example stdin probe: - #| `printf 'a.b = 1\n' | moon run --target native cmd/tomljson -- --stdin`. + #|- Example stdin probe (no pipes — feed stdin directly): + #| `@myshell.Cmd("moon", ["run", "--target", "native", "cmd/tomljson", "--", + #| "--stdin"], stdin=Text("a.b = 1\n"))`. #|- Implement stdin mode with `@stdio.stdin.read_all().text()`, not #| `/dev/stdin` or C FFI. #|- Validate both file input and stdin input when promised. @@ -649,12 +655,13 @@ fn default_prompt() -> String { #| #|Before finishing code work: #| - #|1. Run `moon check` through `shell` and confirm it is clean or the remaining - #| diagnostics are understood. - #|2. Run targeted shell `moon test`. - #|3. Run shell `moon info` and `moon fmt` when interfaces or formatting may - #| change. - #|4. Run task-specific acceptance probes with shell `moon run`. + #|1. Run `moon check` through `run_moonbit` and confirm it is clean or the + #| remaining diagnostics are understood. + #|2. Run targeted `moon test`. + #|3. Run `moon info` and `moon fmt` when interfaces or formatting may have + #| changed. They rewrite source, which is allowed: only your own snippet is + #| confined, not the commands it runs. + #|4. Run task-specific acceptance probes with `moon run`. #| #|Use exact `moon` subcommands for final validation: `moon check` for fast #|type-checking, `moon test` for tests, `moon run` for CLI probes, `moon cram