Review noted that the option whitelist still let generic short flags
escape: `rm -rf -v /` and `rm -rf -i /` were not classified critical,
which is the same separator class the change set out to close.
Pin only the recursive/force flag and skip any other options on either
side of it, which also removes the need to enumerate long options. An
absolute target is still required, so `rm -rf -- ./build`,
`rm --recursive --force ./dist` and `rm -v /tmp/scratch` remain benign
and are asserted.
`CRITICAL_BASH_PATTERNS` required the target to follow one short flag
cluster directly, so anything in between escaped the check:
rm -rf / matched
rm -rf -- / missed
rm --recursive --force / missed
rm -rf --no-preserve-root / missed
rm --no-preserve-root -rf / missed
The last two matter most. GNU coreutils already refuses `rm -rf /` with
"it is dangerous to operate recursively on '/'" and names
`--no-preserve-root` as the override, so the pattern matched the form
that fails safe and missed the form that does not.
Repeat the option separator instead of assuming the path follows one
cluster, and treat `--no-preserve-root` as critical wherever it appears.
Absolute targets are still required for the first pattern, so
`rm -rf -- ./build` and `rm --recursive --force ./dist` stay benign;
both are asserted in the test.
PR #5270 reported the `--` and long-option forms in July and was closed
unmerged by the contributor-vouch bot rather than on merit. This keeps
its cases, credits them in the tests, and adds the `--no-preserve-root`
forms that patch did not cover.
- Added live tracking and stale status warnings for agent activity snapshots.
- Fixed text wrapping with ANSI escape sequences to defer style open sequences after whitespace.
- Added VirtualRenderScheduler for deterministic virtual-clock rendering tests.
On macOS the shared headless browser daemon launched from the system Google Chrome app bundle, running as a com.google.Chrome instance. macOS LaunchServices could then deliver the user's open-URL Apple Events to the daemon instead of their own Chrome, silently swallowing link clicks.
ensureChromiumExecutable now prefers the isolated Chrome for Testing binary (com.google.chrome.for.testing) on macOS, falling back to system Chrome only when Chrome for Testing cannot be obtained. Other platforms keep the download-avoiding system Chrome preference.
Fixes#8673
Mirrored registry status from pre-wire session run-state transitions and required live session corroboration before a peer can sustain bare hub waits.
Fixes#8634
Added Chromium no-startup-window to the broker-owned browser launch so no unowned foreground page survives session tab cleanup.
Covered the resolved Chromium argv and documented the Windows regression.
Fixes#8615
The shared-browser CDP liveness probes (probeEndpoint, waitForCdp, probeCdpAt) used a bare fetch() against the loopback DevTools endpoint. Bun's fetch honors HTTP_PROXY/HTTPS_PROXY and forwards even 127.0.0.1 requests to the proxy unless NO_PROXY covers them, so a local proxy (e.g. Clash) that 502s internal addresses made a healthy daemon look dead and ensureSharedBrowser tore it down.
Replace the three probes with probeCdpStatus(), a raw-TCP HTTP/1.1 GET that never routes through a proxy and resolves to the response status (or null on unreachable/aborted/timeout).
Fixes#8567
Review raised that `apply_patch`'s non-overwrite contract for `create` and
a rename destination is decided with `Bun.file(dst).exists()`, which
reports `false` when the parent hides the target's metadata rather than
distinguishing "absent" from "unknown" — so a privileged handler could
be asked to write over a protected file it was told not to touch.
Reproduced all three shapes: no handler is consulted in any of them. A
hidden-metadata destination is refused by the path resolver, because the
same denied `lstat` that fools the existence check also leaves the final
component unproven, and an unverifiable destination is never brokered. A
destination that is a symlink onto a protected file is caught earlier —
`exists()` follows the link and reports `true`. A plainly visible
existing file is caught by the same check.
So the contract holds, but it holds through two independent guards in two
files. Pinned that with a regression test asserting the premise (the
existence check cannot see the file), that no handler is consulted, and
that the file is intact; deleting the resolver's symlink proof makes it
fail. Recorded the coupling in the module header too, since relaxing the
refusal to broker unverifiable paths would silently break exclusivity
and needs an explicit intent field first.
Review on #8052 found three problems in the write/delete fallback seam.
A symlink guard that only `lstat`'d the final component let the same
escape through a symlinked ancestor: `ws/link/file` under a
`ws/link -> /outside` link reached a handler as a lexically innocent
path, so a helper's prefix allowlist passed while the bytes landed
outside. Refusing every symlinked component is not available, since
`/var` and `/tmp` are links on macOS and every path under `os.tmpdir()`
traverses one. So `req.dst` is now the path the failed syscall itself
acted on, via `resolveSyscallTarget` beside `confineToWorkspace`: fully
resolved for a write, resolved up to the last component for a delete,
because `unlink` removes a link rather than following it. Resolving also
closes the TOCTOU window a refusal left open. A path that cannot be
canonicalized — a dangling final link, or an ancestor whose own
resolution is denied — is not brokered at all.
Per-handler throw isolation lived outside the per-extension trampoline,
so a throw from one extension's first handler advanced the registry to
the next extension and skipped every later handler that one had
registered. Each handler call is now wrapped individually.
The registry stays process-wide. A subagent spawned with restricted
tools gets `preloadedExtensionPaths: []` and loads no extensions of its
own, so scoping resolution to the originating session would turn its
brokered writes into hard failures, and a host that registers once in
its top-level session expects its subagents covered. The request names
its origin instead: `req.sessionId` against the handler's own
`ctx.sessionManager.getSessionId()`, entered by `ExtensionToolWrapper`,
which `sdk.ts` already puts around the whole tool registry whenever a
runner exists. Both registries are also walked over a snapshot, so a
concurrent session shutdown cannot make another session's walk skip
whichever handler shifted into the hole.
Unit tests go 30 -> 35 and integration 6 -> 7, covering the resolved
target, a symlinked ancestor on both seams, a dangling link, a target
whose own metadata is behind the boundary, same-extension handler
ordering after a throw, and `req.sessionId` matching the handler's own
session end to end.
A host that runs omp inside an OS sandbox can grant a path mid-session but cannot
apply that grant to an in-process write: `write` and `edit` do their I/O in the
agent process, so an out-of-workspace write fails and stays failed until the
process restarts under a wider profile.
Nothing available today closes that. A `tool_call` handler can block and a
`tool_result` handler can rewrite content, but neither can re-run a tool.
`ctx.invokeTool` delegates execution, but the delegated native tool runs in the
same process under the same restrictions. And the failure lands AT the write
syscall - after the tool computed the final content, before it returned - so the
bytes are gone with the throw, and reconstructing them means reimplementing
`edit`'s hashline protocol and the snapshot bookkeeping.
The byte-write that `write`, `edit` and `apply_patch` perform on an ordinary file
path already funnels through one two-line primitive
(`file ? file.write(content) : Bun.write(dst, content)`) at four call sites.
Routing that primitive through `writeFileWithFallback` gives an embedder a single
seam to intercept a permission-denied write: the native tool still records its own
snapshot under the real destination path once a handler reports success, so a
follow-up hashline `edit` on that path keeps working.
Only a permission boundary diverts - `EPERM`/`EACCES`/`EROFS`. Two cases needed
more than that:
- `Bun.write` creates missing parents itself, and when that `mkdir` is the denied
operation it reports the subsequent `open()`'s `ENOENT` instead of the denial -
making a sandboxed write into a new out-of-tree directory indistinguishable from
an ordinary bad path. Redoing the `mkdir` explicitly recovers the real errno, and
because it runs through the same enforcement path as the write it also sees
kernel-level denials (Seatbelt, LSM) that a `stat`/`access` probe reports as
writable. If no handler takes the write, the original `ENOENT` is still what
propagates, with the recovered denial attached as its `cause`.
- `apply_patch` creates the parent as a separate step before writing, so a denial
there threw before the seam was ever reached. That `mkdir` now tolerates a
permission denial when a fallback is registered, letting the write report it.
A denial reached through a SYMLINK is never brokered. The in-process write follows
the link, so the kernel denied the link's TARGET, but a handler receives `dst` and
a privileged helper opening it with ordinary follow semantics would land the bytes
wherever the link points. That also defeats the obvious helper-side defence, since
a prefix allowlist passes when the link sits inside the allowed root while its
target does not. omp cannot vouch for the destination, so it refuses rather than
hand the ambiguity to a privileged writer - the same answer `confineToWorkspace`
already gives an unresolvable link.
Removing a file is a different primitive, so it gets its own seam
(`deleteFileWithFallback`, `registerFileDeleteFallback`) covering `edit`'s `REM`,
a hashline `MV`'s source unlink, and `apply_patch`'s delete op. Two differences
from the write path: `ENOENT` is never diverted, since nothing is created on the
way to an unlink and `REM` needs it to become a not-found error; and the seam
refuses a target it can confirm is a directory, because `unlink` on a directory
reports `EPERM` on Darwin and is otherwise indistinguishable from a sandbox
denial. That check cannot always run - a sandbox denying the unlink usually denies
the target's metadata too - so the request carries `confirmedFile`, and a handler
is required to use a plain unlink rather than resolving or recursing.
The two registries are deliberately separate. A write handler brokers `content` to
`dst`, so a delete request reaching it with no content invites brokering an empty
write and truncating the file it was asked to remove.
With nothing registered both seams are inert: the primitives run exactly as
before, a failure rethrows from the same place, and no extra syscalls are
performed.
Scope is deliberately narrow. Archive-member and SQLite writes are unchanged -
neither is a byte-write to a path, so brokering them needs a different request
shape - along with the ACP bridge's `writeTextFile`, the `lsp` tool's own
workspace-edit and formatter writes, and directory removal.
- Add `renderPdfPageScreenshot` to render PDF pages via headless Chromium.
- Update `ReadTool` to intercept legacy PDF image paths and return page screenshots.
- Ensure daemon clients are closed on read command exit.
- Replaced the custom MuPDF-WASM PDF extraction and rendering pipeline with the new `pdfToMarkdown` native function from `@oh-my-pi/pi-natives`.
- Removed legacy MuPDF extraction modules, WASM embedding scripts, and PDF image extraction tools.
- Added OCR warnings and browser/text redirection for unsupported PDF image reads.
- Updated native package definitions, documentation, and test suites for the new PDF inspection capability.
Move the rewind instruction out of the permanent checkpoint tool result into a transient post-checkpoint notice that is branch-cut away on rewind; rewrite the rewind-report prompt forward-looking (no negation); collapse the rewind tool description to one line. Closes#8499.
formatReadHashlineHeader collapsed every relative in-workspace path to its
basename, so reading a nested file (e.g. src/settings.json) emitted
[settings.json#tag]. When a same-basename file existed at the session cwd,
a verbatim follow-up edit resolved against the cwd file; Patcher.prepare only
runs snapshot-tag path recovery when the authored path is missing, so the
valid edit was deterministically rejected with "hash is not from this
session". Keep the workspace-relative path, which names the file uniquely and
stays directly resolvable against cwd. Out-of-workspace absolute paths remain
shortened; root-level files are unchanged.
Fixes#8482
The executable version probe added in ecb22957 ("validate Linux browser
executables") replaced the file-only check in resolveSystemChromium with
isChromiumExecutable, which spawns the candidate `--version` for every
platform. On Windows chrome.exe is a GUI-subsystem binary: `--version`
does not print to a detached stdout and can hand off to a running
instance, opening/activating the user's normal browser window, after
which the probe rejects the candidate and falls back to cached Chrome
for Testing.
Gate the spawn probe on process.platform === "linux" (its intended
platform, where non-Chromium PATH wrappers are the real risk) and trust
the executable-file check on Windows and macOS.
Fixes#8445
- Multi-question dialogs now advance on Enter in multi-select mode
instead of submitting every question at once (roboomp blocking)
- Ask tool accepts an empty multi-select submission as a "select none"
answer instead of aborting as a cancellation (codex P2)
- Footer hint reflects advance vs submit for multi-question dialogs
- Move the #8252 changelog entry to Unreleased (codex P2)
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Added shared auth storage and model registry for concurrent and eager compaction tests.
- Replaced per-test auth storage creation and cleanup routines with shared lifecycle management.
- Removed local auth storage variables and individual cleanup hooks from test harness instances.
- Added `splitAddressableFileLines` to strip terminal newlines from line addressability without removing genuine blank lines.
- Updated coding-agent read tool context parsing to use addressable file lines.
- Added archive and member size assertion limits along with path byte-length checks for PAX and GNU metadata targets.
- Added support for global PAX attributes, old-GNU name records, and signed GNU base-256 numeric header fields.
- Updated archive reading in WriteTool to accept a filesystem path instead of buffered bytes.
- Added test coverage for signed GNU base-256 values, PAX extensions, overlong path rejections, and oversized archives.
- Bound PAX sparse record memory overhead by caching sparse markers and specific keys.
- Update system prompt phrasing and tests for tool inventory and date displays.
- Added Google provider thinking configuration parameters and force-reasoning-off controls.
- Implemented MCP SSE stream resumption using Last-Event-ID and `SSEResumeError`.
- Added support for TAR old-GNU sparse extension blocks, path length checks, and archive entry overrides.
- Restricted external thinking support to specific models and added semver fallback parsing.
- Added the `--external-thinking` CLI flag alongside model capability checks to gate external thinking tool availability.
- Updated Anthropic and Google transports to honor `forceReasoningOff` for native thinking-off controls.
- Renamed the `thoughts` property and parameter to `notes` across think fixtures, tools, and tests.
- Updated system prompt instructions and test suites to verify transport-specific thinking and tool activation.
- Added support for external thinking and forced reasoning disablement across AI provider options and request transformers.
- Implemented the private scratchpad think tool along with its renderer, system prompt rules, and schema configuration.
- Updated agent session management and SDK tools to support dynamic runtime activation of the think tool via the externalThinking setting.
- Added comprehensive unit tests covering reasoning fallbacks, tool activation, and rendering behavior.
- Define a centralized `USER_AGENT` constant in `@oh-my-pi/pi-utils` formatted as `omp/<version>`.
- Replace hardcoded and platform-specific user agent strings across AI providers, catalog scrapers, tools, and search providers with the unified `USER_AGENT`.
- Add unit tests for update-cli binary release distribution gating.
- Updated user agent and openrouter titles in packages/ai from Oh-My-Pi to omp.
- Integrated getOpenRouterHeaders into image generation tool requests in packages/coding-agent.
- Updated provider documentation and test assertions to reflect the rebrand.
can1357