From d1f3532bb1c518564c67a031cfbdacca73a92b94 Mon Sep 17 00:00:00 2001 From: OrcaWin Date: Tue, 27 Oct 2026 01:07:06 -0700 Subject: [PATCH] docs: restore AGENTS.md reference docs under docs/agents-md-refs (#34322) #34008 removed docs still linked from AGENTS.md (test anti-patterns, cross-platform support, git workflow, component fixtures) plus the macOS daemon TCC doc that cross-platform-support links to. Restore them in a dedicated folder whose name marks them as AGENTS.md inputs. Co-authored-by: OrcaWin <293788423+OrcaWin@users.noreply.github.com> --- .gitignore | 7 +- AGENTS.md | 8 +- docs/agents-md-refs/component-fixtures.md | 57 +++++++ docs/agents-md-refs/cross-platform-support.md | 18 +++ docs/agents-md-refs/git-workflow.md | 31 ++++ .../macos-daemon-tcc-attribution.md | 142 ++++++++++++++++++ docs/agents-md-refs/test-antipatterns.md | 25 +++ 7 files changed, 279 insertions(+), 9 deletions(-) create mode 100644 docs/agents-md-refs/component-fixtures.md create mode 100644 docs/agents-md-refs/cross-platform-support.md create mode 100644 docs/agents-md-refs/git-workflow.md create mode 100644 docs/agents-md-refs/macos-daemon-tcc-attribution.md create mode 100644 docs/agents-md-refs/test-antipatterns.md diff --git a/.gitignore b/.gitignore index 0c9b1cb2c55..457da36b88d 100644 --- a/.gitignore +++ b/.gitignore @@ -97,11 +97,8 @@ docs/** !docs/STYLEGUIDE.md !docs/styleguide/ !docs/styleguide/** -!docs/reference/ -!docs/reference/cross-platform-support.md -!docs/reference/git-workflow.md -!docs/reference/test-antipatterns.md -!docs/reference/component-fixtures.md +!docs/agents-md-refs/ +!docs/agents-md-refs/** !docs/agent-skill-sharing-implementation-checklist.md !docs/mobile-terminal-shortcut-bar.md diff --git a/AGENTS.md b/AGENTS.md index 0faf67ff84f..a12c05d769e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -17,7 +17,7 @@ Never steal monitor focus or reveal test windows: no `show()`, `showInactive()`, Rebuild modified launch-policy code before running an app; stale build wrappers are not safe. Use the `$electron` skill and Playwright CDP for rendered Orca UI checks. Do not use computer-use for Orca UI validation. -Screenshot presentational components without Electron: `pnpm fixtures:shot `. +Screenshot presentational components without Electron: `pnpm fixtures:shot ` ([fixtures](docs/agents-md-refs/component-fixtures.md)). # Code Style @@ -44,7 +44,7 @@ Every change must consider these areas: ## Cross-Platform Support -Support macOS, Linux, and Windows. Guard platform-specific behavior at runtime; use platform-aware shortcuts, paths, and existing process wrappers. +Support macOS, Linux, and Windows. Guard platform-specific behavior at runtime; use platform-aware shortcuts, paths, and existing process wrappers. Before changing platform-specific code, processes, shells, shortcuts, paths, or native modules, read [`docs/agents-md-refs/cross-platform-support.md`](./docs/agents-md-refs/cross-platform-support.md). ## Remote Use Case @@ -60,10 +60,10 @@ All changes must consider folder workspaces as well as git worktrees. Don't assu # Git / GitHub -If changing code around `git`, `gh`, source-control, or review behavior, follow the repository's existing conventions. +If changing code around `git`, `gh`, source-control, or review behavior read [`docs/agents-md-refs/git-workflow.md`](./docs/agents-md-refs/git-workflow.md). For a failed GitHub Actions run, inspect the failed jobs and retry with `gh run rerun --failed --repo stablyai/orca`. Preserve successful jobs; use a full rerun only when the failure requires revalidating them. A code fix needs a new push and fresh checks, not a retry of the old SHA. # Tests -Avoid adding low-value tests. These might be fine for development but offer low value when checked in. +Avoid adding low-value tests. These might be fine for development but offer low value when checked in. [Anti-patterns](./docs/agents-md-refs/test-antipatterns.md) diff --git a/docs/agents-md-refs/component-fixtures.md b/docs/agents-md-refs/component-fixtures.md new file mode 100644 index 00000000000..21136883b38 --- /dev/null +++ b/docs/agents-md-refs/component-fixtures.md @@ -0,0 +1,57 @@ +# Component Fixtures + +Render one renderer component in isolation and screenshot it headlessly in about a second, without booting Electron. Use it as the fast verify loop for presentational UI changes. Use the `$electron` skill when the change depends on real app state, IPC, terminals, or window chrome. + +## Writing a fixture + +Put a `*.fixture.tsx` file next to the component. Each named export built with `defineFixture` is one fixture: + +```tsx +import { defineFixture } from '../../component-fixtures/component-fixture' +import { DeleteWorktreeDirtyChangeHint } from './DeleteWorktreeDirtyChangeHint' + +export const dirty = defineFixture({ + width: 360, + render: () => +}) +``` + +Optional fields: + +- `store`: partial `AppState` merged into the store before render. The store starts from slice defaults plus `getDefaultSettings()`. +- `setup()`: runs before render, for example to override a `window.api` method. +- `interact(frame)`: runs after first paint, for example clicking a trigger so an open popover is captured. Content portaled to `` (popovers, menus, dialogs) is included in the screenshot. +- `width`: frame width in CSS pixels. The height always fits the content. + +Fixture ids are the file path under `src/renderer/src/` with `components/` and `.fixture.tsx` removed, followed by the export name. For example, `sidebar/DeleteWorktreeDirtyChangeHint:dirty`. + +Every fixture renders inside `I18nProvider`, `TooltipProvider`, and the theme class. `window.api` is the web client's preload bridge (`installWebPreloadApi`) with no runtime paired. Calls that need a runtime reject, and namespaces without a web implementation return the web fallback defaults. The web-client flag is cleared, so components render their desktop branches. + +Fixtures are dev-only. They load through `import.meta.glob` in the fixture explorer, which is not a production build input. Oxlint rejects static imports of `*.fixture` modules. + +## Screenshots + +```bash +pnpm fixtures:shot sidebar/DeleteWorktreeDirtyChangeHint:dirty # one fixture +pnpm fixtures:shot sidebar/DeleteWorktreeDirtyChangeHint --theme both # every fixture in a file, both themes +pnpm fixtures:shot 'ui/*' --scale 2 # glob, at 2x +pnpm fixtures:shot --list +pnpm fixtures:shot --stop +``` + +PNGs go to `.cache/fixtures/` (override with `--out`), and the command prints their paths. Read them with your image viewer tool. Render errors and page console errors are printed for each fixture, and any failure makes the command exit non-zero. + +The first call starts a detached server: Vite (`config/vite.fixtures.config.ts`, which extends `vite.web.config.ts`) plus one headless Chromium. The first call takes 10–20 seconds. After that, each shot takes about 100 ms. The first shot after a source edit takes 1–2 seconds because the page reloads. The server restarts itself when the fixture scripts change and exits after 20 idle minutes. + +Chromium resolution order: `ORCA_FIXTURES_BROWSER` (an executable path), then the Playwright-managed `chromium-headless-shell`, then an installed Chrome or Edge. Every option runs headless, so screenshots never open a window or take focus. + +## Browsing + +`pnpm dev:fixtures` serves the explorer for humans. Open `/fixtures` for the index, or `/fixtures?id=sidebar/DeleteWorktreeDirtyChangeHint:dirty&theme=light` for a single fixture. + +## MCP + +`.mcp.json` registers `orca-component-fixtures`, a stdio server at `config/scripts/component-fixtures/fixtures-mcp-server.mjs`. It has two tools: + +- `list_fixtures({ filter? })` +- `screenshot_fixture({ id, theme?, scale? })`: returns the PNGs inline (up to 6) along with their paths. diff --git a/docs/agents-md-refs/cross-platform-support.md b/docs/agents-md-refs/cross-platform-support.md new file mode 100644 index 00000000000..0eff0738b61 --- /dev/null +++ b/docs/agents-md-refs/cross-platform-support.md @@ -0,0 +1,18 @@ +# Cross-Platform Support + +Orca targets macOS, Linux, and Windows. Keep all platform-dependent behavior behind runtime checks: + +- **Keyboard shortcuts**: Never hardcode `e.metaKey`. Use a platform check (`navigator.userAgent.includes('Mac')`) to pick `metaKey` on Mac and `ctrlKey` on Linux/Windows. Electron menu accelerators should use `CmdOrCtrl`. +- **Shortcut labels in UI**: Display `⌘` / `⇧` on Mac and `Ctrl+` / `Shift+` on other platforms. +- **File paths**: Use `path.join` or Electron/Node path utilities — never assume `/` or `\`. +- **Windows terminal shells**: `--shell` picks the shell a terminal _is_; `--command` is typed into whatever shell the host spawned, so a shell choice routed through `command` silently becomes a child process. +- **Windows setup scripts**: the setup/issue-command runner is a `.cmd` batch file unless the script starts with a `#!` line — never derive that from the user's terminal-shell preference, and never launch a `.cmd` runner with a bare `cmd.exe /c` from a Git Bash pane (MSYS rewrites the `/c`). +- **Windows child processes**: start them through `runProcess`/`spawnProcess` from `@orca/process-host` — never `child_process` directly. It pins `windowsHide`, refuses `shell: true`, and encodes `.cmd`/`.bat` arguments so neither `CommandLineToArgvW` nor `cmd.exe` mangles them. Recognised npm/pnpm `.cmd` shims are resolved to their real target so the spawn skips `cmd.exe` entirely. +- **Ripgrep**: Orca bundles `rg` for every platform, WSL, and SSH remotes. Spawn it through `spawnBundledRipgrep` (main) or `resolveRelayRipgrepCommand` (relay), never a bare `'rg'` — Windows resolves a bare name in the spawn cwd before PATH. Don't add git/readdir fallbacks locally; the relay's chain exists only for hosts an upload never reached. +- **Windows process enumeration**: read the table through `src/main/windows/windows-process-table.ts`, never by forking `powershell.exe`. +- **Windows MSYS/Git Bash panes**: their children break away from the per-PTY job unless it is created without `JOB_OBJECT_LIMIT_BREAKAWAY_OK`, and a `conpty.node` built before that fix passes every existing gate. +- **Windows daemon-host relocation**: the terminal daemon runs from a copy of the app runtime under `%LOCALAPPDATA%`, which is what survives an auto-update. +- **macOS daemon launch**: packaged macOS runs the terminal daemon from a copy of the signed terminal host helper through launchd, so an update cannot sever its privacy grants. Before touching that copy, its cleanup, or the fork fallback, read [`macos-daemon-tcc-attribution.md`](./macos-daemon-tcc-attribution.md). +- **Windows EDR signal**: don't add `-ExecutionPolicy Bypass`, `-EncodedCommand`, `cmd.exe /c` with escaped free text, per-operation interpreter spawning, or runtime `Add-Type` compilation without reviewing the EDR impact first — behavioural EDR scores each of those, and being signed does not clear them. +- **WSL commands**: build argv with `buildWslExecArgs` (always `--exec` — under `--`, `wsl.exe` expands `$name` in every argument and silently rewrites the script), and fence anything whose stdout you parse with `buildWslCapturedLoginShellCommand`, because the interactive login shell prints the distro banner to stdout. +- **Linux native modules**: keep the glibc floor at Ubuntu 20.04 / glibc 2.31. A module compiled from source on a newer runner can reference symbol versions absent on the floor and crash the app on startup. Packaging fails if a bundled native binary needs newer glibc. diff --git a/docs/agents-md-refs/git-workflow.md b/docs/agents-md-refs/git-workflow.md new file mode 100644 index 00000000000..a59e63c22ae --- /dev/null +++ b/docs/agents-md-refs/git-workflow.md @@ -0,0 +1,31 @@ +# Git and Review Work + +## Git Binary Compatibility + +Orca runs the user's Git binary on native, WSL, and SSH hosts, which may all have different versions. Treat Git 2.25 as the core-workflow baseline. + +## Git Scan Safety + +- Never enumerate every ref and then run `git ls-tree -r` or `git show` once per ref. That ref × tree fan-out can retain gigabytes of output before a downstream `sort -u` or search can make progress. +- Prefer `rg` over the checked-out files for source searches. For history or refs, use a named ref, an explicit namespace/path, `--max-count`, and a bounded output; do not use an unqualified `--all` scan as a first diagnostic. +- Keep repository-wide commands targeted to the current repository and worktree. If an unbounded scan is genuinely required, measure the ref count first, explain the cost, and get confirmation before running it. + +## Git Provider Compatibility + +Source-control and review changes must consider GitLab and other supported git providers, not only GitHub. Keep provider-specific behavior behind explicit checks, and avoid GitHub-only naming for generic review concepts. + +## GitHub CLI Usage + +Be mindful of the user's `gh` CLI API rate limit — batch requests where possible and avoid unnecessary calls. All code, commands, and scripts must be compatible with macOS, Linux, and Windows. + +## Merging Pull Requests + +Before merging, verify that the required checks and aggregate gate in the full PR workflow have pass. + +## Retrying GitHub Actions + +Inspect a failed run with `gh run view --log-failed --repo stablyai/orca`, then retry failed jobs with `gh run rerun --failed --repo stablyai/orca`. GitHub also reruns jobs that depend on the failed jobs. In GitHub's UI, choose **Re-run failed jobs**; Orca offers **Rerun failed checks**. + +Keep successful jobs unless there is a concrete reason to revalidate them, such as investigating runner-dependent flakiness. A full rerun restarts passing unit shards, packages, and platform checks too. If fixing the failure changes code, push the fix and validate the new commit; rerunning an older run still tests its original commit. + +On a partial retry, the unit-selection plan or full set of shard artifacts may be absent for that attempt. Unit execution falls back to the full test inventory; incomplete selection evidence is advisory and cannot justify enabling selection. Inspect artifact requirements before retrying release or publishing jobs. diff --git a/docs/agents-md-refs/macos-daemon-tcc-attribution.md b/docs/agents-md-refs/macos-daemon-tcc-attribution.md new file mode 100644 index 00000000000..fe3d9b61ad7 --- /dev/null +++ b/docs/agents-md-refs/macos-daemon-tcc-attribution.md @@ -0,0 +1,142 @@ +# macOS terminal permissions across updates + +Read this before changing how the packaged macOS terminal daemon is launched, copied or cleaned +up, or before treating a missing executable as a permission denial. Background: issue +[#13921](https://github.com/stablyai/orca/issues/13921); the signed native evidence is in PR #25848. + +## Why a signed copy started by launchd + +A daemon forked from the app runs as `Orca Helper` and macOS charges its privacy access to the app +at its original path. While an update replaces that path, a daemon that first touches a protected +file is denied, and the denial persists after the new bundle arrives even though System Settings +still shows the grant. A missing executable alone does not cause it: a daemon that already read +protected files keeps working after its old executable is deleted. + +So a packaged macOS GUI app starts its daemon from a private copy that the updater never touches: + +- Each packaged macOS app ships a signed helper, `Contents/Helpers/Orca Terminal Host.app` (about + 124 MiB): the pinned Node from `src/shared/node-runtime-pin.ts` as its main executable, the + daemon's `out/main` JS and that slice's node-pty. `config/scripts/macos-terminal-host-bundle.cjs` + builds and signs it in `afterPack`, boots it through one PTY round trip, and fails the build on any + layout, Info.plist, OS floor, architecture or signature drift. +- `macos-daemon-bundle.ts` keeps one copy per helper build, named by the helper's CDHash, at + `userData/daemon-host/macos/terminal-host-/app.noindex/Orca Terminal Host.app`. The helper + is read from the bundle the running main process executes from, never the whole app. A present + copy is reused as is. Otherwise the helper is cloned (APFS clone, regular copy otherwise) into a + `.staging--*` sibling, verified once with + `codesign --verify --strict -R "="`, and published with one `rename`; + losing that rename to another publisher is fine. An app without the helper falls back to the + fork. +- `macos-daemon-launchd.ts` runs the copy's Node on the copy's `daemon-entry.js` as a + non-persistent launchd job, so the daemon has Orca's own identity rather than the UI process's + replaceable path. The label is fixed per profile and protocol + (`com.stablyai.orca.terminal..v`). Before bootstrap a stopped job with + that label is booted out. A running job gets a bounded wait for shutdown and launchd retirement; + a job still running afterward gets the retired-service recovery below for this protocol's label + only, and stays fatal unless that recovery releases it; an unreadable job is fatal. `--entry-path` stays the installed + app's entry, which the replacement preflight compares. The job file is 0600 and deleted right + after bootstrap, because the environment may hold credentials. It sets + `AssociatedBundleIdentifiers` to `com.stablyai.orca`, which is how a launchd job not installed + through SMAppService names the app it belongs to for Local Network access. Readiness is the + normal authenticated handshake, fenced by the launch nonce. + +Node servers, SSH hosts, unpackaged builds, Linux and Windows keep the fork launcher. Adopting an +existing daemon never copies. Telemetry reports a daemon started from a copy as +`spawner_path_class=stable-copy`. + +Only Developer ID builds exercise the helper path in practice. A local ad hoc build's designated +requirement is a cdhash, which the helper (a different binary) cannot match, so it falls back to the +fork. + +## Why the helper is `com.stablyai.orca` + +TCC records grants against the designated requirement, which for Orca is the identifier plus team. +Signing the helper with the app's own identifier lets it inherit every existing grant, including Full +Disk Access, which a user can only add by hand. A distinct identifier would be a new client: new +folder prompts naming it, and a new Full Disk Access row for every user. + +## macOS floor + +The helper's Node declares `LC_BUILD_VERSION minos 13.5`, which is also the helper's +`LSMinimumSystemVersion` and `NODE_RUNTIME_DARWIN_MINIMUM_OS`. Packaging fails if a Node pin bump +changes it. Below 13.5 the launcher returns before copying and the fork runs, so those Macs keep the +pre-#25848 behaviour; Orca itself still supports macOS 12.0. + +## First launch + +macOS takes about 1 s the first time a binary runs from a new path, so the first daemon of each +build starts that much slower; later daemons reuse the copy and pay nothing. During that first exec +the daemon's `/usr/bin/login` preflight can time out, and that one shell is spawned directly. + +## Security + +The helper is a signed "run any JS as Orca" binary: it honours `NODE_OPTIONS` and any script path, +and Electron's fuses do not apply to it. Orca's own binary already allows the same, because its +RunAsNode fuse is enabled, so the helper adds no new exposure. + +## Fallback states + +`launchMacDaemonFromStableBundle` returns null, and the launcher forks, only when no job from the +attempt can still claim the endpoint. It logs one `[daemon] macOS terminal host fallback reason=<…>` +line with `MacStableLaunchFallbackReason`: `prepare-failed` (helper, copy, signature or job file), +`deadline`, `bootstrap-refused` (launchd refused the job and it is absent), `exited-before-ready` +(the job stopped before answering), or `endpoint-taken` (another daemon owns the endpoint, which +the fork path then adopts normally). After `bootstrap-refused` or `exited-before-ready` the copy is +deleted if unused, so the next launch re-copies it. A timed-out bootstrap, or a job still running or +unreadable, throws: forking beside it could split the endpoint. + +The stable attempt must hand off by `MAC_STABLE_LAUNCH_HANDOFF_MS` after the launch starts, so the +fork's own readiness wait and adapter install still fit the 60 s startup PTY gate. Preparation +shares one abort signal that ends early enough for bootstrap and the job's readiness wait to fit +too. A fallback after the handoff deadline throws instead of forking a daemon that would arrive +after the app has moved to local PTYs. + +## Cleanup rule + +After publishing or reusing the current copy, each launch starts one background collection (one at +a time per process). It deletes every other entry under `daemon-host/macos/`, older copies and +crashed staging dirs, once `lsof +D` over it reports no process. `lsof +D` exits 1 even when it +lists users, so only empty stdout counts as unused; any timeout, stderr, truncated output or other +exit code keeps the copy. Nothing else can run a non-current copy: only this app launches daemons, +and only from the current copy. Staging dirs named with this process's pid are skipped, since they +may still be copying. + +## LaunchServices and Spotlight + +The helper declares no links, documents or types, has a lower `CFBundleVersion` than the app and +`LSUIElement`, and was not registered in the signed spike, nested or copied. Copies sit in a +`.noindex` folder, which Spotlight skips, and Orca never registers them. Starting the daemon through +launchd leaves no record, but another process running the copy's executable directly can make +macOS register it, so retirement runs `lsregister -u` on the bundle before deleting it. + +The helper's main-executable UUID is Node's, shared by every official Node 24.21.0 binary of that +architecture, so it is not unique to Orca. Local Network access still passed in the signed spike: +macOS matched the client by its code signature, and the job's `AssociatedBundleIdentifiers` ties it +to Orca. Do not register copies explicitly (`lsregister -f` or `LSRegisterURL`). + +## Rollback + +To turn this off, make `launchMacDaemonFromStableBundle` (`macos-daemon-launchd.ts`) return `null` +right after its platform checks: every launch then takes the fork path. Copy-launched daemons that +are still running are adopted as usual. Copies are only deleted by the collection, so keep it running +there (`retireOtherMacDaemonCopies(root, '')`) until no copy-launched daemon can remain; each copy +keeps about 124 MiB. + +## Existing sessions and status + +RPC and signal shutdown share a five-second process-exit deadline, armed before terminal teardown. +A stalled teardown must not leave the endpoint retired while the launchd job stays alive forever. + +An explicit, untargeted Restart also recovers signed-helper jobs left by older builds after they +retired their PID record and endpoint but failed to exit. Recovery uses exact launchd labels and +arguments for this profile, ignores newer protocols, and preserves live or unreadable endpoints +and services still publishing a PID record. It verifies descendant exits before killing the old +root and releasing its label. A failed recovery is logged without blocking the current daemon's +restart. A launch runs the same recovery only for the current protocol's label, and only once that +label still blocks bootstrap after the retirement wait (#29361); otherwise the label refused every +new terminal until a manual Restart. Node/SSH hosts do not perform GUI-service recovery. + +Running old daemons keep their terminals; settings reports app and terminal-host Full Disk Access +separately, and a denied terminal is recovered by closing it or restarting the daemon. The packaging +hook also gives the main bundle and every Helper the Desktop, Documents and Downloads descriptions, +because the fork fallback still hosts terminals in `Orca Helper`. diff --git a/docs/agents-md-refs/test-antipatterns.md b/docs/agents-md-refs/test-antipatterns.md new file mode 100644 index 00000000000..e122e7ea59b --- /dev/null +++ b/docs/agents-md-refs/test-antipatterns.md @@ -0,0 +1,25 @@ +# Test anti-patterns + +Tests that: + +- Reimplement production logic instead of exercising it. +- Have no meaningful assertions, such as `expect(true).toBe(true)`. +- Assert only mock calls or values supplied by their own mocks. +- Promise inputs or behavior their setup never exercises. +- Duplicate coverage without faster feedback or clearer failures, including private predicates and provider replays. +- Copy inventories, manifests, exports, imports, or strings instead of proving behavior. +- Check persistence at a path the code never writes. +- Assert capability flags instead of delivery or acknowledgement. +- Pass negative cases without exercising the intended rejection path. +- Wait on real time or networks instead of fake clocks and deterministic boundaries. + +Mocks and fixtures that: + +- Hide incorrect tables, fields, joins, filters, or arguments behind mocked internal chains. +- Contradict real producers or source contracts. +- Implement the behavior under test or treat distinct APIs as equivalent. +- Supply receipts, admission decisions, callback ordering, or other outputs production code should generate. + +Production code that: + +- Adds exports, globals, wrappers, or code paths solely for tests.