Skip to content

Latest commit

 

History

History
19 lines (16 loc) · 12.1 KB

gotchas.md

File metadata and controls

19 lines (16 loc) · 12.1 KB

Gotchas

  • Don't loosen the sessionId/branch regexes in src/git/worktree-manager.ts. They're a defense-in-depth check against path traversal and git argv-flag smuggling (a session id starting with - would be parsed as a flag). The -- separator on the git command is the second layer; keep both.

  • Hook server is loopback + secret. Any new endpoint added to src/permissions/hook-server.ts must validate the secret header — the PWA-facing server (server.ts) is the only thing exposed on the tailnet.

  • State lives in ~/.outpost/ (override: OUTPOST_RUNTIME_DIR). index.json files use atomic rename for persistence — preserve that pattern in any new persisted store.

  • ~/.outpost/.env is sourced at daemon startup (see env-file.ts). It exists because launchd strips shell env, so this is how secrets like GITHUB_TOKEN reach the daemon and its sessions without baking them into the plist. Plist > .env > defaults. A value starting with op:// is a 1Password reference resolved through the op CLI (resolveOpBin, honoring OUTPOST_OP_BIN) on every load — warnings by default, fatal under OUTPOST_ENV_STRICT=1. Plain values are assigned before references resolve, so a file can configure the loader it's read by; resolved secrets are assigned after, so a failed reference never strands the raw op:// string in process.env. One op read per distinct reference, never logging the value, and the first timeout skips the rest: the reads run synchronously before the server listens, and an unanswered unlock prompt would otherwise cost a full timeout per reference.

  • The daemon's GitHub calls don't need gh when GITHUB_TOKEN is set. Every one goes through runGh(cwd, argv) (src/integrations/gh-cli.ts), which still runs the binary when it's installed (gh reads the same token); only when gh is missing (ENOENT, remembered for the process) does a token send the call to ghViaApi (gh-api-shim.ts), which answers the argv from REST/GraphQL and prints the JSON gh would have, so callers and the test fakes keyed on argv never changed. The binary wins because the shim is narrower: an existing install with both keeps gh's fork-upstream, default-repo and GHE resolution. The shim knows only the shapes the daemon uses — api user, api <path> --paginate, pr list --head, pr view --json, pr create, run view, search prs, issue create — and throws on anything else, including an unlisted --json field, rather than guess. So a new GitHub call is a new shim case plus a test in gh-api-shim.test.ts, not just a new argv. Known approximations: pr view takes the last 100 reviews and comments (gh pages through all), statusCheckRollup the first 100 contexts, pr view <branch> takes that head name's newest open PR (else newest in any state) from any fork, and the repo is always origin's (gh may resolve a fork's upstream). Never spawn gh directly again — that is how five call sites ran blind on a machine without it.

  • $OUTPOST_API_URL is exported into process.env at daemon startup, so every spawned session inherits the loopback base URL (claude-proc.ts merges process.env under its per-session vars). That's how an action step POSTs back to the daemon — it has no other way to learn the port. Actions that use it must allowlist the $OUTPOST_API_URL and 127.0.0.1:<port> spellings; the checker sees the command text, not the expanded value.

  • A blocker always lands in the action's journal — except a denied write draft. WorkEngine.onStepFailed appends a blocked entry unless the session already journaled for that step, so the Library's "Self-reported lessons" is never empty for an action that keeps failing (which is the only signal meta.improve-actions reads). Pass { journal: false } for failures the action didn't cause — a daemon bounce, a workspace that wouldn't provision. A user deny of a write draft is the carve-out: denyDraft (src/work/write-draft-runner.ts) journals a denied outcome against whoever chose to run the action — the controller, or the job's orchestratorAction (meta.orchestrate) — never against the write action itself, because the reason is feedback on the decision to run it, not on how it ran. A separate gated_denied outcome (journalGatedDenial in engine.ts) records a mid-run allowlist/pin miss against the acting action instead; both denied and gated_denied are deliberately excluded from onStepFailed's one-entry-per-step blocked slot so a self-authored lesson is never crowded out by either.

    A skipped outcome is not a deny, and it is filed twice. Skipping every drafted call (settleUnrun) says running the action was right and its payload wasn't, so the lesson goes to the action that drafted the calls. For a briefed draft — a controller or dispatch raiser — a second copy goes to the briefer, because the drafter didn't choose the content: code.reply-pr-comments posts boundNote verbatim and treats anything it adds as unapproved text on a public PR, so blaming it alone files the evidence against the one participant that had no say in what was said. A step raiser wrote its own payload and keeps the single lesson. The lesson quotes the drafted bodies, not just the call labels — it used to carry comment ids alone, which is how nine recorded skips of PR replies that all opened "You're correct" taught nobody that the replies agreed. Bodies come from the call's files map (the command only names a /tmp path), rationed against JournalStore's 400-char cap. Pinned by write-draft-skip-lesson.test.ts. The authoring fix for that particular pattern is in code.triage-pr-comments's rubric, not here — a reply disputes or scopes out, never concedes.

  • Every catalog action must live in the repo's actions/, not just ~/.outpost/actions/. The runtime dir is what the registry reads, but ensureActionsInstalled skips existing real dirs — so a repo-only edit never reaches the daemon, and a runtime-only action doesn't exist on a fresh install. Write both. tests/unit/action-effective-allowlist.test.ts pins the resolved grant of the actions whose group defaults alone don't cover their own SKILL.md.

  • A rule that grants a network or filesystem write must be a positive whitelist pinned to its destination. Anchor it ^…$, name the exact host/path, and enumerate every flag that may appear — anything unrecognised then denies. A prefix rule grants everything after it: ^curl (and ^curl -s , and ^curl -fsS -X POST ) is every method, every body, every URL, plus -o <path> overwriting any local file and --next starting a second request with fresh flags. Never write a rule that tries to forbid a flag — -d, -d=x, "-d", -d"", -d$X, -sd all reach argv as the same flag, so quoted spellings must be refused by the whitelist too (gh api "-X" PUT … is a merge). . doesn't match a newline and a \-continuation stays inside one clause, so .* guards leak across lines. tests/unit/permission-group-pull.test.ts pins this for the pull group.

  • A gated write needs an approved pin, not just an allowlist hit. The registry resolves a second allowlist per action, gated — the subset of its grants that came from a GATED_GROUPS group (currently just push; see resolvePermissions in src/actions/registry.ts). For an action-bound session, the PreToolUse hook checks gated first: a call matching it is allowed only if it also matches an unconsumed PinnedCall on an approved WriteDraft for that session (matchPinnedCall in src/work/write-draft.ts — Bash on exact command text past an outer trim; any other tool name matches on itself plus deep-equal args, which in practice means MCP, since that's the only other shape a draft pins) and independently clears the ordinary allows() check. So an action that "improves" the user's edited payload before running it — reformats the body, adds a flag — gets denied: the pin is for the text the user actually approved, not a paraphrase of it. The pin is consumed at allow time; a PostToolUseFailure releases it (keyed on tool_use_id, exactly-once) if the call failed, so an identical retry needs no second approval — it does not release on a user interrupt, since it's unknown whether the write landed. A bash call whose payload is too big for the command line (--input/--body-file/--notes-file, the short -F, and -f key=@path — see FILE_REFERENCING_FLAGS in src/permissions/file-flags.ts) is additionally pinned by a sha256 digest of the referenced file's bytes (PinnedCall.fileDigests, re-verified before the pin is consumed) — command-text matching alone says nothing about whether the file still holds what was approved; the session drafts that body inline as PinnedCall.files (path → content) instead of writing it itself, the user edits it directly in the approval card, and acceptDraft (src/work/write-draft-runner.ts) writes the approved bytes to disk itself before hashing them, so the digest can never drift from what was actually approved.

    Two ways a write escapes the pin check. (1) The global/project half of this closed with confinement: for an action-bound call, scopesFor (src/permissions/allowlist.ts) no longer touches global or project scope at all — an action's grants are core ∪ its declared groups ∪ its colocated allowlist.json (plus session scope), full stop; global/project only apply to a call with no actionName, where there is no gated set to bypass in the first place. gatedMatch (src/permissions/hook-handler.ts) still consults only the action's static gated set, computed once at registry load, while the allows() fallback right below it consults those confined scopes — the colocated allowlist.json, hot-added per-action overrides in ~/.outpost/actions.json, and session scope — none of which ever populate gated. A new write-shaped rule can no longer be installed into either remaining scope: assertNotWriteShaped (src/permissions/write-shape.ts) is enforced at Allowlist.addRule, ActionsStore.addRule, and the registry's colocated allowlist.json reader (checked on every load, so this one applies to pre-existing files too — see the pull bullet in permissions.md). What's left: a write-shaped rule already persisted in ~/.outpost/actions.json before this lint shipped still loads and still runs ungated — auditPersistedRules (src/storage/actions-store.ts) only logs a startup console.warn naming it, it does not refuse the load. (2) Anything not shaped like the Bash/MCP call the hook can inspect — see the edit-group ceiling in deferred.md.

  • Two daemons can't run on the same ~/.outpost/. Use alternate ports for side-by-side testing, but stop the launchd instance first.

  • A review comment's diff_hunk is not the diff around it. GitHub's field runs from the hunk's START to the COMMENTED LINE — so it holds nothing after the comment (often the half that says what the comment is about), and on an added file the hunk starts at line 1, so a comment on line 302 arrives carrying 302 lines of file body. The card rendered it verbatim, uncapped, until this was fixed. pr-file-patches.ts fetches the real per-file patch on demand so utils/diff-window.js can anchor the comment inside its hunk and take a window with lines on both sides; diff_hunk is now only the fallback, windowed to its tail. Don't "simplify" the fetch away as redundant with a field that's already in the payload — it isn't the same data. Note also that a collapsed <details> still costs its full markup in every repaint's innerHTML, so the expanders render 20 rows, not the rest of the hunk (see project_pwa_subagent_repaint_cost — the same shape caused a timeline stutter before).

  • PWA modules extracted from app.js use a deps-injection pattern. initX({ callbackA, callbackB }) is how the extracted module gets app-side functions it can't cleanly import (would create cycles). app-bridge.js is the other half of this — a shared reserved-keys object for callbacks multiple components need, installed once via installAppBridge() rather than threaded through every initX. Follow one of these two patterns when adding more.