diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e0c0161..d92a898 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -118,6 +118,16 @@ jobs: - name: Run deploy shell tests run: make test-deploy + - name: Verify sandbox inventory against the image + run: | + jq --version + IMAGE=ghcr.io/rossoctl/serverless-harness-sandbox:latest + docker pull "$IMAGE" || { + echo "::warning title=Sandbox inventory drift check skipped::could not pull $IMAGE; inventory drift is UNVERIFIED" + exit 0 + } + bash deploy/knative/verify-sandbox-inventory.sh "$IMAGE" + proto: runs-on: ubuntu-latest timeout-minutes: 10 diff --git a/Makefile b/Makefile index 34c3388..6e79da3 100644 --- a/Makefile +++ b/Makefile @@ -21,6 +21,7 @@ typecheck: cd harness && pnpm exec tsc --noEmit cd packages/k8s-sandbox && pnpm exec tsc --noEmit cd packages/knative-server && pnpm exec tsc --noEmit + cd packages/config-bundle && pnpm exec tsc --noEmit cd experiments && pnpm exec tsc --noEmit # Laptop showcase: harness on kind, remote worker as a host container dialing out. diff --git a/README.md b/README.md index afa1977..9d4a70f 100644 --- a/README.md +++ b/README.md @@ -102,6 +102,9 @@ flowchart LR `RuntimeDefault` seccomp, no service-account token automount. - **Built on Pi** — wraps a pinned [`kagenti/pi`](https://github.com/kagenti/pi) coding agent through an injectable `SessionStorageBackend` seam; the agent itself is unmodified. +- **Promote a local Claude Code workflow** — `cd harness && pnpm promote` bundles skills, `CLAUDE.md`, + memory, and a slash command from your local `~/.claude` into a content-addressed bundle a leaf can + dispatch by digest (see [Promoting a local Claude Code workflow](#promoting-a-local-claude-code-workflow)). --- @@ -178,6 +181,41 @@ troubleshooting — is in **[`deploy/knative/README-ocp.md`](deploy/knative/READ --- +### Promoting a local Claude Code workflow + +Iterate on a workflow locally in Claude Code — skills, `CLAUDE.md`, memory, a slash command — +then promote it: + +```bash +cd harness && pnpm promote --entry my-workflow --project /path/to/your/project +``` + +`promote` reads the workflow — skills, `CLAUDE.md` chain, and memory — from `--project`, so +running it from the harness checkout without `--project` promotes the harness's own +configuration, not yours. + +`promote` dedupes and classifies your local configuration, drops what cannot work in the harness +(with a reason for each), and scans for credentials before uploading. The scan has two tiers: a +structural match on a known key shape (an AWS access key, a PEM private-key block, a GitHub or +Slack token, an OpenAI-style key) **refuses the upload** and exits non-zero; a weaker prose +heuristic (`token: `-shaped lines) only **warns and proceeds**, leaving the judgement to +you — that heuristic matches code and documentation placeholders too often to block on safely. It +writes a committable `.claude/promoted.lock.json` and uploads a content-addressed bundle; an +unchanged re-promotion uploads nothing. + +Dispatch it by adding one field to any prompt leaf: + +```json +{ "sessionId": "run-1/item-1", "kind": "prompt", "prompt": "…", "configRef": "sha256:…" } +``` + +Memory travels **read-only** — a promoted run consumes what you taught it locally and reports +discoveries back in the leaf result, which keeps leaf replay reproducible +([ADR-0031](docs/adrs/0031-promoted-memory-read-only.md)). MCP servers and subagents are not +promoted; see the [design](docs/specs/2026-09-02-claude-code-workflow-promotion-design.md) §2, §9. + +--- + ## Dispatch Archetypes The same backend serves three orchestration patterns, all validated end-to-end on Kind: diff --git a/deploy/knative/sandbox-inventory/README.md b/deploy/knative/sandbox-inventory/README.md new file mode 100644 index 0000000..4df5fe0 --- /dev/null +++ b/deploy/knative/sandbox-inventory/README.md @@ -0,0 +1,30 @@ +# Sandbox binary inventories + +Each file declares the commands one sandbox image provides. The promote CLI (`cd harness && pnpm promote`; see +[`../../../docs/specs/2026-09-02-claude-code-workflow-promotion-design.md`](../../../docs/specs/2026-09-02-claude-code-workflow-promotion-design.md) §4.5) +preflights a workflow's detected binaries against these **without cluster access**, so a +promotion can be checked on a laptop. + +- **Filename** is the image ref with `:` and `/` replaced by `_`, plus `.json`. +- **Contract:** `{ "image": "", "binaries": [""] }`. +- `tar`, `base64`, `flock` and `git` are required by `converge.ts` and `config-overlay.ts`. + +Two checks guard these files, because they catch different failures: + +| Check | Runs | Catches | +| --------------------------------- | ---------------------- | ---------------------------------------------------------------------- | +| `tests/sandbox-inventory.test.sh` | every PR, no cluster | malformed JSON, wrong filename, unsorted list, a missing required tool | +| `../verify-sandbox-inventory.sh` | where the image exists | **drift** — the file claiming something the image does not have | + +A file that has drifted makes preflight lie, and a lying preflight is worse than none because +people stop checking it. Re-run the verify script whenever the sandbox Dockerfile changes. + +## Provenance + +`ghcr.io_rossoctl_serverless-harness-sandbox_latest.json` was generated from the real +published image, not curated by hand: + +- Image: `ghcr.io/rossoctl/serverless-harness-sandbox:latest` +- Digest: `sha256:0683379d6368ab14c41d9bb46683178946091abba47b1832756d89f39afcdb9f` +- 347 binaries +- Enumerated 2026-09-03 by listing every executable on `PATH` inside the image diff --git a/deploy/knative/sandbox-inventory/ghcr.io_rossoctl_serverless-harness-sandbox_latest.json b/deploy/knative/sandbox-inventory/ghcr.io_rossoctl_serverless-harness-sandbox_latest.json new file mode 100644 index 0000000..ae28902 --- /dev/null +++ b/deploy/knative/sandbox-inventory/ghcr.io_rossoctl_serverless-harness-sandbox_latest.json @@ -0,0 +1,352 @@ +{ + "image": "ghcr.io/rossoctl/serverless-harness-sandbox:latest", + "binaries": [ + "[", + "[[", + "acpid", + "add-shell", + "addgroup", + "adduser", + "adjtimex", + "apk", + "arch", + "arp", + "arping", + "ash", + "awk", + "b2sum", + "base32", + "base64", + "basename", + "basenc", + "bash", + "bbconfig", + "bc", + "beep", + "blkdiscard", + "blkid", + "blockdev", + "brctl", + "bunzip2", + "busybox", + "bzcat", + "bzip2", + "c_rehash", + "cal", + "cat", + "chattr", + "chcon", + "chgrp", + "chmod", + "chown", + "chpasswd", + "chroot", + "chvt", + "cksum", + "clear", + "cmp", + "comm", + "coreutils", + "cp", + "cpio", + "crond", + "crontab", + "cryptpw", + "csplit", + "curl", + "cut", + "date", + "dc", + "dd", + "deallocvt", + "delgroup", + "deluser", + "depmod", + "df", + "diff", + "dir", + "dircolors", + "dirname", + "dmesg", + "dnsdomainname", + "dos2unix", + "du", + "dumpkmap", + "echo", + "egrep", + "eject", + "env", + "ether-wake", + "expand", + "expr", + "factor", + "fallocate", + "false", + "fatattr", + "fbset", + "fbsplash", + "fdflush", + "fdisk", + "fgrep", + "find", + "findfs", + "flock", + "fmt", + "fold", + "free", + "fsck", + "fstrim", + "fsync", + "fuser", + "getconf", + "getent", + "getopt", + "getty", + "git", + "git-receive-pack", + "git-shell", + "git-upload-archive", + "git-upload-pack", + "grep", + "groups", + "gunzip", + "gzip", + "halt", + "hd", + "head", + "hexdump", + "hostid", + "hostname", + "hwclock", + "iconv", + "id", + "ifconfig", + "ifdown", + "ifenslave", + "ifup", + "init", + "inotifyd", + "insmod", + "install", + "ionice", + "iostat", + "ip", + "ipaddr", + "ipcalc", + "ipcrm", + "ipcs", + "iplink", + "ipneigh", + "iproute", + "iprule", + "iptunnel", + "join", + "kbd_mode", + "kill", + "killall", + "killall5", + "klogd", + "last", + "ldconfig", + "ldd", + "less", + "link", + "linux32", + "linux64", + "ln", + "loadfont", + "loadkmap", + "logger", + "login", + "logname", + "logread", + "losetup", + "ls", + "lsattr", + "lsmod", + "lsof", + "lsusb", + "lzcat", + "lzma", + "lzop", + "lzopcat", + "makemime", + "md5sum", + "mdev", + "mesg", + "microcom", + "mkdir", + "mkdosfs", + "mkfifo", + "mkfs.vfat", + "mknod", + "mkpasswd", + "mkswap", + "mktemp", + "modinfo", + "modprobe", + "more", + "mount", + "mountpoint", + "mpstat", + "mv", + "nameif", + "nanddump", + "nandwrite", + "nbd-client", + "nc", + "netstat", + "nice", + "nl", + "nmeter", + "nohup", + "nologin", + "nproc", + "nsenter", + "nslookup", + "ntpd", + "numfmt", + "od", + "openvt", + "partprobe", + "passwd", + "paste", + "pathchk", + "pgrep", + "pidof", + "ping", + "ping6", + "pinky", + "pipe_progress", + "pivot_root", + "pkill", + "pmap", + "poweroff", + "pr", + "printenv", + "printf", + "ps", + "pscan", + "pstree", + "ptx", + "pwd", + "pwdx", + "raidautorun", + "rdate", + "rdev", + "readahead", + "readlink", + "realpath", + "reboot", + "reformime", + "remove-shell", + "renice", + "reset", + "resize", + "rev", + "rfkill", + "rg", + "rm", + "rmdir", + "rmmod", + "route", + "run-parts", + "runcon", + "scanelf", + "sed", + "sendmail", + "seq", + "setconsole", + "setfont", + "setkeycodes", + "setlogcons", + "setpriv", + "setserial", + "setsid", + "sh", + "sha1sum", + "sha224sum", + "sha256sum", + "sha384sum", + "sha3sum", + "sha512sum", + "showkey", + "shred", + "shuf", + "slattach", + "sleep", + "sort", + "split", + "ssl_client", + "stat", + "stdbuf", + "strings", + "stty", + "su", + "sum", + "swapoff", + "swapon", + "switch_root", + "sync", + "sysctl", + "syslogd", + "tac", + "tail", + "tar", + "tee", + "test", + "time", + "timeout", + "top", + "touch", + "tr", + "traceroute", + "traceroute6", + "tree", + "true", + "truncate", + "tsort", + "tty", + "ttysize", + "tunctl", + "udhcpc", + "udhcpc6", + "umount", + "uname", + "unexpand", + "uniq", + "unix2dos", + "unlink", + "unlzma", + "unlzop", + "unshare", + "unxz", + "unzip", + "update-ca-certificates", + "uptime", + "users", + "usleep", + "uudecode", + "uuencode", + "vconfig", + "vdir", + "vi", + "vlock", + "volname", + "watch", + "watchdog", + "wc", + "wcurl", + "wget", + "which", + "who", + "whoami", + "whois", + "xargs", + "xxd", + "xzcat", + "yes", + "zcat", + "zcip" + ] +} diff --git a/deploy/knative/tests/sandbox-inventory.test.sh b/deploy/knative/tests/sandbox-inventory.test.sh new file mode 100755 index 0000000..0b9a5d3 --- /dev/null +++ b/deploy/knative/tests/sandbox-inventory.test.sh @@ -0,0 +1,44 @@ +#!/usr/bin/env bash +# deploy/knative/tests/sandbox-inventory.test.sh +# +# Shape test for the checked-in sandbox binary inventories. `sh promote` preflights a +# workflow's detected binaries against these files WITHOUT cluster access, so a malformed or +# mis-named file makes preflight quietly wrong -- and a preflight that is quietly wrong is +# worse than none, because people stop checking. This runs on every PR. +# +# Reality (does the image actually provide these?) is verified separately by +# verify-sandbox-inventory.sh, which needs the image and so runs where the image exists. +# +# No cluster required. Run: bash deploy/knative/tests/sandbox-inventory.test.sh +set -uo pipefail + +DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)/sandbox-inventory" +fails=0 +check() { if [ "$2" = "$3" ]; then echo " ok: $1"; else echo " FAIL: $1 (want '$3', got '$2')"; fails=$((fails+1)); fi; } + +command -v jq >/dev/null 2>&1 || { echo "SKIP: jq not installed (required to validate inventory JSON)"; exit 0; } + +shopt -s nullglob +files=("$DIR"/*.json) +if [ ${#files[@]} -eq 0 ]; then echo "FAIL: no inventory files in $DIR"; exit 1; fi + +for f in "${files[@]}"; do + echo "== $(basename "$f")" + check "valid JSON" "$(jq -e . "$f" >/dev/null 2>&1 && echo yes || echo no)" "yes" + image="$(jq -r '.image' "$f")" + expected="$(basename "$f" .json)" + actual="$(printf '%s' "$image" | tr ':/' '__')" + check "filename matches .image" "$actual" "$expected" + check "binaries is a non-empty array" \ + "$(jq -e '(.binaries | type == "array") and (.binaries | length > 0)' "$f" >/dev/null 2>&1 && echo yes || echo no)" "yes" + check "binaries sorted and unique" \ + "$(jq -r '.binaries == (.binaries | unique)' "$f")" "true" + # converge.ts and config-overlay.ts both depend on these at run time. + for required in tar base64 flock git; do + check "declares '$required' (required by converge/overlay)" \ + "$(jq -r --arg b "$required" '.binaries | index($b) != null' "$f")" "true" + done +done + +if [ "$fails" -ne 0 ]; then echo "FAILED: $fails check(s)"; exit 1; fi +echo "PASS" diff --git a/deploy/knative/verify-sandbox-inventory.sh b/deploy/knative/verify-sandbox-inventory.sh new file mode 100755 index 0000000..dfd8ee0 --- /dev/null +++ b/deploy/knative/verify-sandbox-inventory.sh @@ -0,0 +1,75 @@ +#!/usr/bin/env bash +# deploy/knative/verify-sandbox-inventory.sh +# +# Verify a checked-in inventory against the image it describes: every declared binary must +# actually resolve inside the image. Requires docker (or podman, via CONTAINER_RUNTIME) and +# the image. +# +# All declared binaries are checked in a SINGLE container rather than one container per +# binary -- with 347 declared binaries and a ~0.2s container-start cost, one-per-binary would +# take well over a minute per run for no benefit: the check itself (`command -v`) is instant. +# +# The shape test (tests/sandbox-inventory.test.sh) cannot catch drift, because drift is a +# disagreement with reality rather than with the schema. +set -euo pipefail + +IMAGE="${1:?usage: verify-sandbox-inventory.sh }" +DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/sandbox-inventory" +FILE="$DIR/$(printf '%s' "$IMAGE" | tr ':/' '__').json" + +[ -f "$FILE" ] || { echo "no inventory for $IMAGE (expected $FILE)"; exit 1; } + +RUNTIME="${CONTAINER_RUNTIME:-docker}" +mapfile -t declared < <(jq -r '.binaries[]' "$FILE") +# Third silent-pass path: process substitution failure does not trip `set -e`, so malformed JSON +# or a missing `.binaries` key would leave `declared` empty -- the container then checks nothing, +# still prints the sentinel, and the script would report "PASS ... (0 binaries verified)". +[ "${#declared[@]}" -gt 0 ] || { echo "ERROR: no binaries parsed from $FILE"; exit 1; } +echo "verifying ${#declared[@]} declared binaries in $IMAGE" + +# A drift check that cannot fail is worthless, so the container's own failure must never be +# mistaken for "nothing missing". Two guards: +# +# 1. The runtime's exit status is captured explicitly. A blanket `|| true` here would swallow +# a pull failure, an unrunnable image (wrong architecture), or a dead daemon, leaving +# `missing` empty and reporting PASS for an image that was never inspected. That was a real +# bug: pointed at a nonexistent ref, this script printed "PASS: inventory matches" and +# exited 0. +# 2. The container prints a sentinel as its last act. If the inner shell dies partway through +# -- OOM, a SIGKILL, a truncated stream -- the status can still be 0 while the output is +# incomplete, which would read as "no binaries missing". No sentinel, no verdict. +# +# The inner loop deliberately always exits 0 (both `command -v` and `printf` succeed), so a +# non-zero status means the runtime or the image failed, never that a binary was absent. +set +e +out="$("$RUNTIME" run --rm --entrypoint sh "$IMAGE" -c ' + for b in "$@"; do command -v "$b" >/dev/null 2>&1 || printf "MISSING %s\n" "$b"; done + printf "SENTINEL_VERIFIED\n" +' sh "${declared[@]}" 2>&1)" +rc=$? +set -e + +if [ "$rc" -ne 0 ]; then + echo "ERROR: could not run $IMAGE with $RUNTIME (exit $rc). The inventory was NOT verified." + echo "$out" | sed 's/^/ /' + exit 1 +fi + +case $out in + *SENTINEL_VERIFIED*) ;; + *) + echo "ERROR: $RUNTIME ran but the in-container check did not complete (no sentinel)." + echo "The inventory was NOT verified. Output was:" + echo "$out" | sed 's/^/ /' + exit 1 + ;; +esac + +missing="$(printf '%s\n' "$out" | sed -n 's/^MISSING //p')" +if [ -n "$missing" ]; then + echo "INVENTORY DRIFT: $FILE declares binaries the image does not provide:" + printf '%s\n' "$missing" | sed 's/^/ /' + echo "Preflight would report these as present and pass a promotion that fails remotely." + exit 1 +fi +echo "PASS: inventory matches $IMAGE (${#declared[@]} binaries verified in-image)" diff --git a/docs/adrs/0030-claude-code-workflow-promotion.md b/docs/adrs/0030-claude-code-workflow-promotion.md new file mode 100644 index 0000000..96e1843 --- /dev/null +++ b/docs/adrs/0030-claude-code-workflow-promotion.md @@ -0,0 +1,93 @@ +# ADR-0030: Promote local Claude Code workflows as a content-addressed config bundle + +- **Status:** Proposed, amended 2026-09-02 during implementation (still unaccepted; see Amendments below) +- **Date:** 2026-09-02 +- **Deciders:** Serverless Harness team +- **Spec:** [`../specs/2026-09-02-claude-code-workflow-promotion-design.md`](../specs/2026-09-02-claude-code-workflow-promotion-design.md) + +## Context + +The harness runs Pi sessions well but is hard to author for: putting a useful workflow on it +means hand-assembling a prompt and hoping the runtime has what the prompt assumes. The same +person usually _already_ has that workflow working on their laptop in Claude Code — skills they +trust, a `CLAUDE.md`, accumulated memory, a slash command tying it together — and none of it +carries across. + +Two findings make a promotion path tractable. Pi already speaks most of Claude Code's +configuration vocabulary: `core/skills.ts` implements the **Agent Skills standard** (`SKILL.md` +with `name`/`description` frontmatter and the same directory-discovery rules), and +`loadProjectContextFiles` (`resource-loader.ts:62`) already looks for `CLAUDE.md` beside +`AGENTS.md`. Pi's built-ins (`bash`, `read`, `write`, `edit`, `grep`, `find`, `ls`) are +near-isomorphic to Claude Code's. And the harness deliberately uses none of it: +`harness/src/run-turn.ts:463` builds `DefaultResourceLoader` with only +`{ cwd, agentDir, settingsManager, extensionFactories }`, `cwd` being the harness pod's own +directory. The gap is delivery and wiring, not capability. + +The constraint is the fs-free split ([ADR-0020](0020-fs-free-harness.md)): the harness mounts no +shared writable volume — only an emptyDir `/tmp` — and tool calls execute in a _separate_ sandbox +pod, drawn from a shared pool ([ADR-0021](0021-shared-sandbox-pool.md)). A skill therefore +splits in two. Its prose must be readable by the **harness** process, because it feeds the +system prompt; anything it executes must exist in the **sandbox**. The halves travel differently +and must not drift apart. A measured laptop holds ~62 MB under `~/.claude` across 149 `SKILL.md` +files, of which ~11 MB is markdown and much is `cache/` duplicating `marketplaces/`. + +## Decision + +We will promote a local Claude Code workflow as a **content-addressed configuration bundle**, +uploaded once and referenced by a single new optional envelope field `configRef: sha256:…`. A +local `pnpm promote` CLI (`harness/package.json`; with a thin Claude Code slash-command wrapper) +resolves user and project scope, dedupes, prunes, scans for secrets, uploads, and emits a +**generated lockfile** — never a hand-authored manifest. The secret scan is **two-tier**: +structural credential formats block the upload, the prose-shaped heuristic only warns. Skills +are identified by their **bare** frontmatter `name` — the same identity dedupe, the lockfile and +bundle paths use — never a `plugin:skill` form. + +Pruning is **by compatibility, never by relevance**: everything that can work travels, and only +what provably cannot is dropped, each with a machine-readable reason. The classifier has two +buckets and is a _curated_ deny-list plus narrow checks, not an inference engine. Tool-name drift +is handled by an injected `appendSystemPrompt` mapping note; skill prose is never rewritten. + +Materialization follows the fs-free split from one digest. Skills and prompts unpack into the +harness pod's `/tmp/sh-config//` and are wired through `additionalSkillPaths` / +`additionalPromptTemplatePaths`, with `noContextFiles: true` and context supplied via +`agentsFilesOverride`. Executable content overlays into the sandbox via the transport +`converge.ts` already uses, cached shared at `/workspace/.sh-config//` under converge's +flock discipline and bound per-leaf. The sandbox exports `SH_SKILLS_DIR` and an injected note +states it, so relative paths inside skill instructions resolve. Absent `configRef`, harness +behavior is unchanged. + +### Alternatives considered + +- **Git-native profile ref, converged like a workspace** — cheapest to build and a commit SHA is a free digest, but scale-to-zero means every cold start pays a `git clone` against a sub-second target. Its audit property is stolen cheaply by committing the lockfile instead. +- **OCI artifact for everything** — right currency for binaries and free signing/scanning, but push-and-pull per promotion is hostile to the tweak-and-re-promote loop, and the harness pod would need its own pull client. Additive later, not a rival. +- **Mirror `~/.claude` wholesale, unpruned** — no meaningful preflight, and skills that provably cannot work would misfire remotely. +- **Session-derived manifest** — buys a smaller bundle by reintroducing omission, losing exactly the skills a _future_ run needs. Rejected once payload size was measured as single-digit MB. +- **Rewriting tool names across skill files** — a regex deciding whether "Read" is a tool reference or English will mangle prose, and the damage surfaces as a skill misbehaving remotely. + +## Consequences + +- Positive: promotion is one command with no manifest to maintain, and re-promotion of unchanged configuration uploads nothing because the digest is computed locally. The feature is opt-in by construction — an absent `configRef` leaves every existing path byte-identical. One digest covers both halves, so prose and scripts cannot drift. The resolver is pure with respect to the digest, so a promoted leaf stays replayable and the idempotency contract at `run-leaf.ts:67` survives. Existing machinery carries most of the weight: Pi's resource loader, `converge.ts`'s transport and flock discipline, and `packages/session-backend`. +- Negative / accepted cost: the deny-list is **curated**, so it rots — a new local skill family that cannot work remotely misfires until the list catches up. We accepted that over heuristic inference, because grepping for `Agent` false-positives on nearly every superpowers skill and a wrongly-dropped skill fails remotely and confusingly. Preflight cannot catch a whole tier of failures (binary present at a wrong version, missing credentials, denied egress, prose assuming absent host behavior), and it must state those edges rather than imply completeness. A blob store in Redis wants TTLs and size caps and is an object store's job if bundles grow. `noContextFiles: true` is load-bearing in a way that is invisible when wrong: without it, this repository's own `CLAUDE.md` leaks into every promoted session as if it were the user's. Cold start now has a fetch-and-unpack step on the critical path. +- Negative / accepted cost, added by amendment: the secret scan's heuristic tier **warns rather than blocks**, so a pasted bare credential (`password: hunter2hunter2`) reaches the bundle with only a printed warning; structural formats — the shapes real leaked credentials take — still block. This was measured, not assumed: a blocking heuristic produced 11 hits on a real `~/.claude`, all false positives (7 documentation placeholders, 4 code expressions such as `TOKEN = crypto.randomUUID`), two of them inside the `brainstorming` skill. A gate that refuses every promotion gets bypassed or deleted, which protects nothing. +- Follow-up owed: CI must assert the checked-in sandbox binary inventory matches the image it describes, or preflight starts lying. Cold-start delta must be measured into `deploy/knative/EXPERIMENTS.md` against the README's sub-second claim, not assumed. Subagent support needs its own spec (Pi has no Task equivalent; `createAgentSession` is exported but budget roll-up, checkpoint interaction, and a depth cap are net-new). MCP promotion remains out of scope, deferred to the code-mode path ([ADR-0005](0005-mcp-code-mode.md)). Interaction-dependent skills warn under `--mode unattended` today; mapping them onto real human gates ([ADR-0016](0016-human-gate.md)) is a possible future. + +## Amendments + +**2026-09-02, during implementation.** Amended in place rather than superseded, because this ADR is +still `Proposed` — `docs/adrs/README.md` Rule 1 makes an ADR immutable only once Accepted. Two +decisions above were corrected by measuring the design against a real `~/.claude` (61 bundled skills, +586 files) instead of reasoning about it: + +1. **The secret scan became two-tier.** Originally it blocked on any hit. See the accepted cost in + Consequences. +2. **Skill identity is the bare frontmatter `name`.** The original design assumed a `plugin:skill` + qualified form; all 60 on-disk skills use bare names, so the deny-list and the + subagent-dependency check never matched anything — meaning the drop this ADR promises for + subagent-dependent skills would silently not have happened. + +Both are recorded here rather than left to the spec alone, because each changes what this decision +guarantees. The spec (§4.3, §4.2, §4.6, §6, §8) carries the detail and the measurements. + +--- + +_Assisted-By: Claude (Anthropic AI) _ diff --git a/docs/adrs/0031-promoted-memory-read-only.md b/docs/adrs/0031-promoted-memory-read-only.md new file mode 100644 index 0000000..b84255f --- /dev/null +++ b/docs/adrs/0031-promoted-memory-read-only.md @@ -0,0 +1,59 @@ +# ADR-0031: Promoted memory travels read-only; discoveries return in the leaf result + +- **Status:** Proposed +- **Date:** 2026-09-02 +- **Deciders:** Serverless Harness team +- **Spec:** [`../specs/2026-09-02-claude-code-workflow-promotion-design.md`](../specs/2026-09-02-claude-code-workflow-promotion-design.md) §5 + +## Context + +Promoting a Claude Code workflow ([ADR-0030](0030-claude-code-workflow-promotion.md)) brings the +author's local memory with it — one-fact-per-file markdown plus a `MEMORY.md` index that Claude +Code loads each session. Carrying it _in_ is straightforward. Whether a promoted run may write +memory _out_ is a separate and harder question, and it is the kind that gets asked later ("why +can't the managed runs learn?"), so it is recorded on its own. + +Three forces bear on it. The harness is filesystem-free +([ADR-0020](0020-fs-free-harness.md)): "the agent saves a memory file" has nowhere to land +without a new persistence path, so writes are a deliberate choice rather than a default. The leaf +contract is idempotent: `harness/src/run-leaf.ts:67` treats `session_id` as the idempotency key +and maps a retry or resume of the same envelope deterministically onto the same session. And a +local memory directory is user- and machine-scoped, holding operational notes that in practice +discuss credentials and private infrastructure. + +Building a dedicated memory service was considered as the general answer, and would be a +plausible home for it — `packages/session-backend` already abstracts the durable store. + +## Decision + +We will ship memory **read-only**. It travels in the bundle and is injected as context; a +promoted session never writes it. What a run _learns_ returns as data in the leaf result +(`harness/src/leaf-result-store.ts`), and a human decides whether any of it becomes a durable +memory locally. + +Memory gets progressive disclosure rather than bulk injection: only `MEMORY.md` is injected +inline as the index, while the memory files themselves are overlaid **into the sandbox** at a +known path, because `read` executes in the sandbox and cannot see the harness pod's `/tmp`. That +reproduces local recall semantics at the cost of one index in context. + +"Where memory comes from" is a **resolver interface with exactly one implementation** (the bundle +snapshot). Pi's `agentsFilesOverride` makes memory just another context source, so substituting a +memory service later is a constructor argument rather than a redesign. We are not building that +service now. + +### Alternatives considered + +- **Read-write, synced back to the laptop** — closes the learning loop, but mutable shared memory makes a leaf's behavior depend on what other leaves learned first, so replaying an envelope no longer reproduces the run; in a large fan-out, memory becomes a race. It also writes into the author's future context with no review step. +- **Session-scoped writes (durable only within one run)** — redundant: within-run continuity is exactly what compaction checkpoints already provide ([ADR-0007](0007-compaction-checkpoint-fast-path.md)), and a second mechanism for it is drift. +- **A dedicated memory service now** — the right eventual answer to a _different_ problem (curated, team-shared knowledge across many runs, with ownership and staleness semantics). Speculative against this need, and it would land scope in the promotion path for a loop that a result field closes for free. +- **Ship no memory at all, flattening relevant facts into the prompt** — avoids sending personal context to a shared cluster, but discards the recall mechanism that made the local workflow work. Addressed instead by a user deny-list plus a blocking secret scan. + +## Consequences + +- Positive: leaf replay stays reproducible, so ADR-0030's promotion path inherits the existing idempotency contract unchanged rather than qualifying it. No new persistence path is added to a filesystem-free component. The learning loop still closes — through the leaf result, where a human reviews before anything becomes durable — and it costs no new infrastructure. Memory scales past any inline cap because the index is injected and the files are read on demand, matching local semantics. +- Negative / accepted cost: a promoted run cannot accumulate knowledge across dispatches; every run starts from what the author taught it locally. Operational discoveries require a human in the loop to become durable, which is friction by design but is still friction. Memory files reaching the sandbox means they land on a shared pool's volume, so the user deny-list and the secret scan (`packages/config-bundle/src/secret-scan.ts`) are what stand between private context and a shared cluster before promotion: five structural rules (AWS/GitHub/OpenAI/Slack token and private-key-block shapes) block it outright, while two prose heuristics (bearer tokens, `key: value`-shaped assignments) only warn — measured against a real `~/.claude` the heuristics were false-positive-dominated, so only the structural tier is load-bearing. The overlay's `chmod -R a-w` pass (`harness/src/config-overlay.ts`) separately enforces this ADR's read-only guarantee at the filesystem level, so a bundle that clears the scan still cannot be mutated once it reaches the sandbox. And the read-on-demand path depends on the sandbox overlay succeeding, so a memory read failure surfaces as a tool miss rather than a configuration error. +- Follow-up owed: revisit a memory service only when there is a real multi-run, multi-author knowledge-sharing need, at which point it implements the existing resolver interface. If a promoted workflow turns out to genuinely need durable self-authored state, that is a new ADR superseding this one, not an amendment. + +--- + +_Assisted-By: Claude (Anthropic AI) _ diff --git a/docs/adrs/README.md b/docs/adrs/README.md index 3064301..ff73c76 100644 --- a/docs/adrs/README.md +++ b/docs/adrs/README.md @@ -40,6 +40,8 @@ spec). Chronological by the spec's date; numbers are permanent. | [0027](0027-rc1-control-gate-and-hop2-realization.md) | RC1 control gate as IBAC-only; Hop-2 egress interception over plain HTTP | Accepted | | [0028](0028-async-prompt-dispatch.md) | Async prompt dispatch as a `kind:"prompt"` leaf sharing the `/turn` core | Proposed | | [0029](0029-turn-sse-streaming.md) | Streaming `/turn` responses as an SSE representation via content negotiation | Proposed | +| [0030](0030-claude-code-workflow-promotion.md) | Promote local Claude Code workflows as a content-addressed config bundle | Proposed | +| [0031](0031-promoted-memory-read-only.md) | Promoted memory travels read-only; discoveries return in the leaf result | Proposed | ## What an ADR is (and isn't) diff --git a/docs/specs/2026-09-02-claude-code-workflow-promotion-design.md b/docs/specs/2026-09-02-claude-code-workflow-promotion-design.md new file mode 100644 index 0000000..41aed9c --- /dev/null +++ b/docs/specs/2026-09-02-claude-code-workflow-promotion-design.md @@ -0,0 +1,569 @@ +# Claude Code workflow promotion: design + +**Date:** 2026-09-02 · **Status:** Implemented (cold-start measurement owed) · **ADRs:** +[ADR-0030](../adrs/0030-claude-code-workflow-promotion.md), +[ADR-0031](../adrs/0031-promoted-memory-read-only.md) · **Builds on:** M1 (Redis session +backend), M2/M3 (sandbox client + persistent channel), P1 (fs-free harness), P2 (shared +sandbox pool) + +> **Amendment, 2026-09-02 — five claims corrected by measurement during implementation.** Each was +> tested against a real `~/.claude` (61 bundled skills, 586 files) rather than reasoned about, and +> each had shipped as fact in the version above: +> +> 1. **The secret scan is now two-tier, not blocking** (§4.3, §4.6, §6 D7, §8). A single blocking +> heuristic produced 11 hits on a normal machine, every one a false positive — 7 documentation +> placeholders and 4 _code_ expressions (`TOKEN = crypto.randomUUID`) matching because the value +> character class accepts dotted identifiers. Two sat inside the `brainstorming` skill itself. As +> originally specified, promotion would have been impossible on day one. +> 2. **Skill identity is the bare frontmatter `name`, never `plugin:skill`** (§4.2). All 60 on-disk +> skills use bare names, so the namespaced deny-list entries never matched and the +> subagent-dependency check never fired — the drop promised in §9 would silently not have happened. +> 3. **Memory indexes use markdown links, not `[[wikilinks]]`** (§4.6). The real `MEMORY.md` holds 9 +> `[Title](file.md)` links and zero wikilinks, so that check could never fire. +> 4. **The `promote` sample output below was illustrative and is now measured.** +> 5. **Preflight now blocks only on facts and warns on heuristics** (§4.6, §6 D10). The binary check +> reported 32 missing on a real machine (half of them not binaries) and the sibling check fired 182 +> times across 28 skills, because skill prose names files the reader will create. Both are advisory +> now; the first real end-to-end build produced 183 blocking errors and would have failed outright. +> 6. **A recommended-workflow section was added** (§11): author in a minimal sandbox rather than +> promoting a whole `~/.claude`. Most of the pruning machinery in this spec exists only to cope +> with an uncurated environment. +> 7. Implementation detail worth recording because it silently untracked a module: `.gitignore`'s +> unanchored `secrets.*` pattern means the scanner lives in `secret-scan.ts`, not `secrets.ts`. + +## 1. Problem + +The harness runs Pi sessions well but is hard to _author for_. Getting a useful agent workflow +onto it means hand-assembling a prompt and hoping the runtime has what the prompt assumes. +Meanwhile the same person already has a working workflow on their laptop, in Claude Code: +skills they trust, a `CLAUDE.md`, accumulated memory, and a slash command that ties it +together. Nothing carries that across. + +The ask is a **promotion path**: iterate locally in Claude Code until the workflow behaves, +then move it to the managed environment with one command and have it behave the same way. + +Two facts make this tractable, and one makes it delicate. + +**Pi already speaks most of Claude Code's configuration vocabulary.** +`pi-fork/packages/coding-agent/src/core/resource-loader.ts` exposes `getSkills()`, +`getPrompts()`, `getAgentsFiles()`, `getSystemPrompt()`, `getAppendSystemPrompt()`, and +`getExtensions()`. `core/skills.ts` implements the **Agent Skills standard** — `SKILL.md` with +`name`/`description` frontmatter, `disable-model-invocation`, and the same directory-discovery +rules Claude Code uses. `loadProjectContextFiles` (`resource-loader.ts:62`) already looks for +`CLAUDE.md` alongside `AGENTS.md`. Pi's built-in tools (`bash`, `read`, `write`, `edit`, +`grep`, `find`, `ls`) are near-isomorphic to Claude Code's. + +**The harness deliberately uses none of it.** `harness/src/run-turn.ts:463` constructs +`DefaultResourceLoader` with only `{ cwd, agentDir, settingsManager, extensionFactories }`, +where `cwd` is the harness pod's own directory. No skills, prompts, context, or memory travel +today. The gap is delivery and wiring, not capability. + +**The fs-free split constrains delivery.** Per +[`2026-07-02-p1-fs-free-harness-design.md`](2026-07-02-p1-fs-free-harness-design.md) the +harness mounts no shared writable volume — only an emptyDir `/tmp` — and tool calls execute in +a _separate_ sandbox pod. A "skill" therefore splits in two: its prose must be readable by the +**harness** process (it feeds the system prompt), while anything it executes must exist in the +**sandbox**. The two halves travel differently and must not drift apart. + +## 2. Goals and non-goals + +**Goals.** + +1. One local command promotes a working Claude Code workflow; no hand-authored manifest, ever. +2. Skills, `CLAUDE.md`, memory, prompt, and slash commands travel. +3. What cannot work remotely is dropped with a machine-readable reason, before a run is paid for. +4. Re-promotion of unchanged configuration is free. +5. A promoted leaf stays replayable — the idempotency contract at `harness/src/run-leaf.ts:67` + is preserved. +6. Absent a promoted bundle, harness behavior is unchanged. + +**Non-goals.** + +- **MCP servers.** Out of scope. The harness's answer is code-mode in the sandbox + ([ADR-0005](../adrs/0005-mcp-code-mode.md)) plus the credential/egress plane; promoting local + MCP configuration is a separate, much larger problem. +- **Subagents.** Pi has no Task equivalent (§9). +- **Installing binaries.** Preflight detects and reports; the sandbox image provides. +- **Live attach.** Driving a harness session interactively from Claude Code is the phase-2 + shape this design keeps the door open for (§6.4), not what it builds. +- **Namespaced commands.** `commands//.md` (Claude Code's `/:`) is not + recursed into; only prompts directly in the prompts directory travel. Preflight warns + (`namespaced_prompt_skipped`) rather than silently dropping them. + +## 3. Design overview + +``` + laptop harness pod (fs-free) sandbox pod (shared pool) + ────── ───────────────────── ──────────────────────── + ~/.claude ─┐ + repo/.claude├─ promote ──► CAS store ──► /tmp/sh-config// /workspace/.sh-config// + memory/ ───┘ │ skills/ prompts/ exec/ memory/ + │ │ │ + ├─► lockfile.json (committed) DefaultResourceLoader absolute path in + └─► preflight report + agentsFilesOverride per-leaf prompt +``` + +One digest names both halves. The envelope carries the digest; nothing else about the workflow +is transmitted per-dispatch. + +## 4. Bundle + +### 4.1 Format + +A tar+gzip blob; the digest is `sha256` over the **canonical** tar (sorted paths, normalized +mtimes and modes). Layout: + +``` +lockfile.json generated; also emitted locally for committing +skills//… whole skill directories, verbatim +context/ CLAUDE.md chain + selected memory files + MEMORY.md +prompt/ systemPrompt / appendSystemPrompt fragments +prompts/ slash commands → Pi PromptTemplates +exec/ skill-local scripts destined for the sandbox +``` + +Skill _directories_ travel, not files: skills reference siblings (`superpowers:brainstorming` +reads `visual-companion.md`) and carry subtrees (`dataviz/references/`). Only `skills/` and +`prompts/` become files in the harness pod; `context/` and `prompt/` are injected inline. + +`lockfile.json` records the bundle format version, the pi and harness versions built against, +every included skill with source path and content hash, every **excluded** skill with a +machine-readable reason code, included memory files, the expected sandbox image reference, and +the detected-binary list. It is committed for diffability and is preflight's input. + +### 4.2 Classifier: two buckets + +A skill either **travels** or is **dropped with a reason**. There is deliberately no +"travels with rewriting" bucket (§7, A1). + +- **Shipped default deny-list**, matched against the **bare** frontmatter `name` — which is the + identity `resolveSkills` dedupes on, the lockfile records, and the bundle path uses. Measured + against 60 on-disk skills: `docx`, `pdf`, `pptx`, `xlsx` are present and drop; `artifact-design`, + `artifact-diagramming`, `fewer-permission-prompts`, `keybindings-help`, `statusline-setup` and + `update-config` have no `SKILL.md` on disk at all (they are Claude Code built-ins) and are retained + only as defensive entries. A `plugin:skill` qualified form matches nothing and must not be used. +- **Subagent dependency**: skills whose operation _is_ dispatching subagents + (`dispatching-parallel-agents`, `subagent-driven-development`) drop until §9 lands. +- **User deny-list**, additive, for personal or sensitive content. Never an allow-list — an + allow-list is the manifest burden returning through the side door. +- **Interaction dependence** (§4.6) — warned, not dropped, and mode-sensitive. +- **Binary detection** over skill bodies and `exec/`, producing the list preflight diffs + against the sandbox inventory. Detection only. + +The deny-list is **curated, not inferred**. Grepping for `Agent` as an incompatibility signal +is a trap: the word appears in nearly every superpowers skill's prose. Preflight _warns_ on +suspicious patterns; it never silently drops on a heuristic. A wrongly-dropped skill fails +remotely and confusingly, which is the failure class this design exists to prevent. + +Tool-name drift (`Bash`→`bash`, `Glob`→`find`, `LS`→`ls`) is handled by an injected +`appendSystemPrompt` note, not by editing prose. + +### 4.3 Promotion command + +The work lives in a testable CLI in this repo — `pnpm promote` (`harness/package.json`), +alongside `harness/src/cli.ts` — and the Claude Code slash command is a thin wrapper over it. +Building only the slash command would leave the logic untestable and unusable from CI. + +**Inputs**, in Claude Code's precedence order: user scope (`~/.claude/skills`, +`~/.claude/plugins`), project scope (repo `.claude/`), and the project's memory directory. + +**Dedupe is mandatory.** A representative laptop shows 149 `SKILL.md` files but far fewer +skills: `plugins/cache/` (~39 MB) is largely a resolved duplicate of `plugins/marketplaces/` +(~23 MB). Without dedupe by resolved skill identity, the lockfile double-counts and the bundle +ships each skill twice. Of ~62 MB on disk, ~11 MB is markdown; a pruned bundle measured **8.6 MB +across 411 entries** for 55 travelling skills — single-digit MB, but nearer the top of that range +than first estimated, which is one of the arguments for §11's sandbox-first authoring. + +**Entry prompts** make a bundle a _workflow_ rather than a pile of configuration. The bundle +carries named templates in `prompts/`; the envelope names one plus arguments. One bundle +therefore serves many dispatches, which is what the harness's fan-out model wants. + +**The secret scan is two-tier, and the split is empirical.** Promotion reads the memory directory +and `settings.json`, which in practice contain operational notes about credentials, so every file +entering the bundle is pattern-scanned. Five **structural** rules — AWS access key ids, private-key +blocks, GitHub, OpenAI-style and Slack tokens — **refuse the upload** with path and line, because a +credential reaching a shared cluster's store is not recoverable by re-promoting. The prose-shaped +**heuristic** rules (`assigned-secret`, `bearer-token`) only **warn**, with documentation-placeholder +suppression. + +That asymmetry was measured, not chosen for taste. Over a real `~/.claude` the structural rules +produced zero hits and the heuristic produced 11, all false positives: 7 placeholders and 4 code +expressions such as `TOKEN = crypto.randomUUID`, which match because the value character class +accepts dotted identifiers. Two were inside the `brainstorming` skill. A blocking heuristic would +refuse essentially every promotion, and a gate nobody can pass gets bypassed or deleted. + +**Accepted residual gap:** a pasted bare credential (`password: hunter2hunter2`) warns rather than +blocks. Structural formats — the shapes real leaked credentials actually take — still block. + +**Idempotence.** The digest is computed locally; if the store holds it, upload is a no-op. + +``` +$ pnpm promote --entry brainstorm-and-plan + + resolved 60 skills (149 SKILL.md → 60 after cache/marketplace dedupe) + travels 54 + dropped 6 docx, pdf, pptx, xlsx (no_harness_equivalent) + dispatching-parallel-agents, subagent-driven-development + (needs_subagent) + context CLAUDE.md (2 files) + 10 memory files + secrets no blocking findings, 4 warning(s) — see below + binaries gh, kubectl, pnpm → present in sandbox:pool-default + entry brainstorm-and-plan + bundle sha256:94e9…86d (8.6 MB, unchanged — upload skipped) + lockfile .claude/promoted.lock.json (2 skills changed since last promote) +``` + +### 4.4 Harness-side materialization + +`LeafEnvelope` (`harness/src/run-leaf.ts:92`) gains one optional field, `configRef?: string`. +Absent means today's behavior byte for byte — the converge, solve, and swebench paths are +untouched, and the feature is opt-in with zero blast radius. + +Cold start fetches by digest, unpacks into `/tmp/sh-config//`, and caches keyed by +digest so a warm pod's second turn skips the work. A digest-keyed cache cannot go stale, so +its death with the pod is correct rather than unfortunate. Unpack goes to a temp path and is +renamed into place: a crash mid-unpack must never leave a half-populated skill directory, +which would silently truncate a skill's instructions. + +Wiring at `harness/src/run-turn.ts:463`: + +```ts +new DefaultResourceLoader({ + cwd, + agentDir, + settingsManager, + extensionFactories, + additionalSkillPaths: [`${dir}/skills`], + additionalPromptTemplatePaths: [`${dir}/prompts`], + noContextFiles: true, + agentsFilesOverride: () => ({ agentsFiles: bundle.context }), + appendSystemPrompt: [toolNameMappingNote, skillsRootNote, ...bundle.promptFragments], +}); +``` + +**`noContextFiles: true` is load-bearing.** `loadProjectContextFiles` walks ancestor +directories for `CLAUDE.md`/`AGENTS.md` (`resource-loader.ts:62`). On the harness pod that walk +reaches _this repository's own_ `CLAUDE.md` — "Serverless Harness … pnpm workspace … DCO +sign-off required." Unsuppressed, every promoted session silently inherits the harness +project's instructions as if they were the user's. `noSkills` / `noPromptTemplates` are set for +the same reason: the harness image ships none today, and explicit suppression keeps that true. + +Failures are loud. An unresolvable digest, a format-version mismatch, or a checksum mismatch +**fails the leaf immediately**, digest included in the message. There is no fallback to running +unconfigured: a silently unconfigured agent producing plausible-but-wrong work is the expensive +remote failure this design exists to prevent. + +The resolver is **pure with respect to the digest**, so replaying an envelope reproduces the +run — preserving the idempotency property (`run-leaf.ts:67`) that §5 depends on. + +### 4.5 Sandbox-side overlay + +Content enters the sandbox the way `converge.ts` already does it: build a shell script, exec it +through the transport (`buildConvergeScript`, `convergeWorkspace`), under a **per-pod flock** +that serializes concurrent converges, into a **per-leaf** path `/workspace/leaves/`. +The pool is shared ([ADR-0021](../adrs/0021-shared-sandbox-pool.md)), so nothing may be global. + +Transport is a base64 tarball piped into `tar -x` in one exec — not per-file heredocs, which +would be pathological in a 200-leaf fan-out. Scripts get `chmod +x`, since tar-over-exec does +not reliably preserve the bit. + +**Two-level placement.** Bundles are immutable and content-addressed, so they cache _shared_ +at `/workspace/.sh-config//`, populated once under the same flock discipline, and are +bound into each leaf's workspace. A 200-leaf fan-out pushes the bundle once, not 200 times. +Per-leaf content stays under `/workspace/leaves//` so `cleanupWorkspace` remains the +only teardown path. + +**Path translation.** A skill's `SKILL.md` is read in the harness pod, but its internal paths +are written relative to where the skill lives _locally_ — `superpowers:brainstorming` instructs +a read of `skills/brainstorming/visual-companion.md`. Issued into the sandbox, that path +resolves to nothing, and the model gets a confusing miss rather than a reasonable error. So the +bundle layout is mirrored verbatim into the sandbox, the harness appends the absolute skills- +and memory-directory paths to the prompt per leaf (they vary per leaf and cannot be baked into +the content-addressed bundle), and a bundle-level note points at that fragment: _resolve +relative paths in a skill's own instructions against that skill's subdirectory under the +"Skill files:" path given elsewhere in this prompt — not against the current working +directory._ There is no seam to set an environment variable inside the sandbox (every tool call +is an independent `bash -c`), so the literal path in the prompt is the only mechanism. Without +this, every skill referencing a sibling degrades quietly. + +**Binary inventory contract**, so preflight's claim is true rather than aspirational: + +1. The sandbox image **declares** its inventory at a known path, baked in at build time. +2. A **copy is checked into this repo** per image tag, so preflight works offline. +3. `--verify` **probes a live sandbox** (`command -v`) for ground truth. +4. **CI asserts** the checked-in copy matches the image it describes. Without (4) the copy + drifts and preflight starts lying — and a lying preflight is worse than none. + +**Consistency.** Both halves come from one digest; the harness fails the leaf if the overlay +for that digest cannot be established. There is no partial-configuration state. + +### 4.6 Preflight and the failure taxonomy + +Preflight's value is entirely in being honest about its limits. + +**Caught locally, no cluster.** Deny-listed skill (dropped, reason recorded); a skill +referencing a sibling absent from the bundle, found by resolving path-like references in +`SKILL.md`; a **structural** secret-scan hit (blocks) while a heuristic hit only warns (§4.3); +entry prompt not present in `prompts/`; duplicate skill names surviving dedupe, surfaced through +Pi's existing `ResourceCollision` diagnostics rather than a parallel mechanism; dangling links in +`MEMORY.md` pointing at deny-listed files (warn) — matching both the `[Title](file.md)` markdown +form that real indexes actually use and the `[[wikilink]]` form found inside memory bodies. + +**Caught locally with inventory data (advisory).** Missing binary — valuable but **warn-only**, +because measurement showed the detector cannot distinguish a command from prose: on a real +`~/.claude` it reported 32 missing binaries, roughly half of which were not binaries at all +(`angular`, `django`, `vue`, `rev-parse`, and `your_command`, a literal documentation placeholder). +At error severity it refused every real promotion. A genuinely absent tool now fails remotely with a +legible `gh: not found`, which is diagnosable and re-promotable. Formerly described here as "the +highest-value check", since a +missing `gh` is the classic silent remote failure. Also sandbox pool/image-tag existence, and +harness-version-supports-bundle-format. + +**Not catchable locally; run-time only.** Stated in full, because a preflight implying +completeness is worse than one stating its edges: a binary present at a different version or +with different flags; a tool present but lacking credentials; egress the sandbox denies; and +any skill whose prose assumes host behavior with no analogue but no mechanical signature. + +**Interaction-dependent skills** deserve their own name. An unattended run cannot ask a +question. `superpowers:brainstorming` is the clean example — built around asking one question +at a time and waiting; promoted unattended it degenerates into the agent inventing the answers. +`receiving-code-review` shares the shape. These are technically portable and semantically +broken. + +This is **mode-sensitive**, which is why it warns rather than drops: +`pnpm promote --mode unattended|attended`. Under `unattended` it is a loud warning (opt-in drop); +under `attended` — the phase-2 live-attach shape — these skills are exactly what is wanted. One +flag, and phase 2 inherits the classifier unchanged. + +A partial bridge exists and is **not** built here: the harness already has a human-gate +archetype (`request_approval`, `gate.ts`, +[ADR-0016](../adrs/0016-human-gate.md)). Mapping interaction-dependent skills onto real gates +is an interesting future direction, out of scope. + +## 5. Memory model: read-only in, findings out + +Memory travels **read-only**. The harness writes none, and what a promoted run learns comes +back as data in the leaf result, not as a memory write. Three reasons, in descending weight: + +1. **Write-back breaks idempotency.** `run-leaf.ts:67` treats `session_id` as the idempotency + key: a retry or resume of the same envelope maps deterministically to the same session. + Mutable shared memory makes a leaf's behavior depend on what other leaves learned first, so + replaying an envelope no longer reproduces the run. In a 200-leaf fan-out, memory becomes a + race. +2. **Memory writes want review, and unattended runs cannot provide it.** Local memory is good + _because_ it is a byproduct of a human correcting the agent in a loop. An unattended run + writing into that store is an unreviewed write into future context. +3. **The valuable findings have a better channel.** What one would actually want back are + operational discoveries — "this cluster cannot reach X", "the sandbox image lacks Y". Those + belong in the leaf result, which `harness/src/leaf-result-store.ts` already carries; the + human then decides what to keep. Same learning loop, review preserved, no new infrastructure. + +**Progressive disclosure, via the sandbox.** Injecting every memory file inline works at 10 +files and bloats context at 200. The elegant path is blocked instructively: locally the agent +reads memory on demand with `read`, but here `read` executes in the _sandbox_, which cannot see +the harness pod's `/tmp`. So memory files are overlaid into the sandbox at a known path over +the channel `exec/` already needs, and only `MEMORY.md` is injected inline as the index. Local +semantics reproduced, one index in context, scales past any cap. + +**Seam for later.** "Where memory comes from" is a resolver interface with exactly one +implementation (bundle snapshot). Pi supplies the hook: `agentsFilesOverride` makes memory just +another context source, so a future memory _service_ is a constructor argument, not a redesign. +Building that service now would be speculative work against a loop a result field closes for +free. See [ADR-0031](../adrs/0031-promoted-memory-read-only.md). + +## 6. Decisions + +- **D1 — Content-addressed bundle referenced by digest in the envelope.** Not git-native, not + OCI (§7). +- **D2 — Prune by compatibility, never by relevance.** Ship everything that can work; drop only + what provably cannot. Omission and misfire are correctness problems with no automatic fix; + payload size is an engineering problem with a known fix (content-addressing + dedupe). Solve + the tractable one, eliminate the intractable ones. +- **D3 — Generated lockfile, never a hand-authored manifest.** A `package-lock.json`, not a + `package.json`. +- **D4 — Prose is never rewritten.** Tool-name drift is handled by an injected mapping note. +- **D5 — Read-only memory; findings return in the leaf result.** +- **D6 — Materialization split follows fs-free.** Prose to the harness pod's emptyDir; anything + executable to the sandbox. One digest covers both. +- **D7 — Secret scan is two-tier**: structural credential formats block promotion (the only + deliberate friction), the prose-shaped heuristic warns. Measured, not assumed — see §4.3. +- **D8 — Interaction dependence is mode-sensitive**, not a hard incompatibility, which keeps + the classifier valid for phase-2 live attach. +- **D9 — Subagent support is a separate spec** (§9). +- **D10 — Preflight blocks only on facts, and warns on heuristics.** The single blocking preflight + error is `unknown_entry` — the entry prompt provably is or is not in the bundle — alongside the + secret scan's structural-format throw. Everything derived from scanning prose warns: + `missing_sibling`, `missing_binary`, `dangling_memory_link`, `interaction_dependent`, + `possible_secret`. This was not designed in; three independent measurements forced it one check at + a time (11 false blocks from the secret heuristic, 32 from binaries, 182 from siblings), and the + principle is what connects them. A gate that refuses every legitimate promotion protects nothing. + +## 7. Alternatives considered + +- **A1 — Rewrite tool names across skill files.** Rejected: a regex deciding whether "Read" is + a tool reference or English will mangle prose, and the damage surfaces as a skill misbehaving + remotely. An injected mapping note costs one paragraph and mutates nothing (D4). +- **A2 — Session-derived manifest.** Record which skills a good local session actually touched + and generate the manifest from that. Rejected once payload size was measured: it buys a + smaller bundle by reintroducing omission, losing exactly the skills a _future_ run needs. +- **A3 — Mirror `~/.claude` wholesale, unpruned.** Rejected: no meaningful preflight, and + skills that provably cannot work (Artifact, document-skills) would misfire remotely. +- **A4 — Git-native profile ref, converged like a workspace.** Cheapest to build — + `convergeWorkspace` exists, and a commit SHA is a free digest with real promotion history. + Rejected on cold start: scale-to-zero means no warm cache, so _every_ cold start pays a git + clone against a sub-second target. It also needs a cluster-reachable remote with credentials + and pushes `~/.claude` into git. Its audit property is **stolen cheaply** instead — commit the + small textual lockfile, move the bundle out-of-band. +- **A5 — OCI artifact for everything.** Registry infrastructure exists; digests, cosign, and + scanning come free; and it pays for binaries in the same currency an image already uses. + Rejected for phase 1 on loop speed — push-and-pull per promotion is hostile to + "tweak a skill, re-promote" — and because the harness pod would need its own pull client, + kubelet not fetching artifacts on its behalf. **Not a rival**: D1 already assumes an image + for binaries, so signing the bundle artifact later is additive. +- **A6 — Read-write memory synced back to the laptop.** Rejected on §5(1) (idempotency) and + §5(2) (unreviewed writes). +- **A7 — Session-scoped memory writes.** Rejected as redundant: within-run continuity is what + `checkpoint-extension.ts` already provides, and a second mechanism for it is drift. +- **A8 — A dedicated memory service now.** The right eventual answer to a _different_ problem + (curated, team-shared knowledge across many runs, with ownership and staleness semantics). + Speculative here, and it would land scope in the promotion path. + +## 8. Testing and acceptance + +Mirroring existing conventions: `harness/test/converge.test.ts` for script generation, +`run-turn-sandbox.test.ts` for fake transports, `pool-live-smoke.test.ts` and +`packages/k8s-sandbox/test/m3-live-smoke.test.ts` for the env-gated live tier. + +**Unit, no cluster or Redis** — over fixture trees shaped like `~/.claude`: + +- `promote-classify.test.ts` — `cache/` vs `marketplaces/` dedupe, deny-list application, + broken-sibling-path detection, dangling `MEMORY.md` links, collision surfacing. +- `promote-bundle.test.ts` — **digest determinism is load-bearing** and gets property-style + coverage, not one example: identical input yields an identical digest, and a different + directory-walk order still does. Both "re-promotion is free" and leaf replay rest on + canonical tar. Plus secret-scan positives and negatives. + +**Wiring** — `config-resolver.test.ts` asserts an unpacked bundle yields the expected skills, +prompts, and context. One test is named for the bug it guards: **the harness `CLAUDE.md` +leak**. Construct the loader with a cwd inside this repository and assert the harness project's +instructions are absent from `agentsFiles`. It is the only failure here that produces +plausible-but-wrong behavior instead of an error, so it is the only one a reviewer would never +notice. + +**Overlay** — `config-overlay.test.ts`, mirroring `converge.test.ts` with an injected fake +`ExecInPod`: assert writes land only under the per-leaf path and the digest-keyed shared cache, +never globally; assert `chmod +x`; assert flock discipline matches converge's. + +**Integration with Redis**, following `integration.test.ts` — round-trip by digest, a second +identical upload is a no-op, a missing digest fails fast. + +**Live smoke** — `promote-live-smoke.test.ts` behind `SH_PROMOTE_LIVE_SMOKE`. It promotes a +deliberately tiny two-skill fixture and runs a leaf whose success _requires_ a promoted skill +to fire **and** that skill to read a sibling file from its own directory. That assertion is +what proves path translation end to end — otherwise the first proof arrives in production. + +**Measurement** — the README claims sub-second cold start and this feature is the most +plausible thing to erode it, so the cost belongs in the evidence trail rather than in an +assertion. The end-to-end, in-cluster comparison (baseline vs. `configRef` set, against a +scaled-to-zero Revision) could not be taken during this implementation: the deployed image +predates this branch, and taking it needs a full image build, `kind load`, and a forced new +Revision. It remains owed; reproduction, once such a cluster is available: + +```bash +# baseline: no configRef +for i in 1 2 3 4 5; do + kubectl -n "$NS" scale deployment -l serving.knative.dev/service=harness --replicas=0 2>/dev/null || true + sleep 5 + curl -s -o /dev/null -w '%{time_total}\n' -X POST "$KSVC_URL/runs" \ + -H 'content-type: application/json' \ + -d '{"sessionId":"cold-base/'"$i"'","kind":"prompt","prompt":"Say PONG"}' +done + +# with a promoted bundle: same, adding "configRef":"" +``` + +What IS measured, locally, is the cost promote's cold path adds beyond the existing inline +path — fetching the bundle from Redis by digest, verifying it, and unpacking it to disk — not +an end-to-end cold start. N=10, fresh output directory per run, real bundle built from a live +`~/.claude` (8.60 MiB, 55 skills): `getBundle` (Redis fetch + digest verify) median 52.6 ms; +`unpackBundle` (untar to disk) median 62.1 ms — **113.7 ms added to the cold path median** +(range 110.0–132.2 ms). Not on the cold path, promote-time only: `buildBundle` 292.7 ms, +`putBundle` 230.1 ms. Caveats that keep this honest: measured on loopback Redis and macOS +APFS rather than in-cluster (in a pod, Redis is a network hop and the unpack target is an +emptyDir on node disk); excludes container start, which dominates real cold start; excludes +loader init. + +**Done means:** + +1. A bundle promoted from a real `~/.claude` runs a leaf that invokes a promoted skill and + reads a sibling file from it. +2. With `configRef` absent, the existing suite is green unmodified. +3. A planted **structural** credential (e.g. an AWS access key id) blocks promotion, while a + heuristic-only hit warns and lets it proceed. +4. A missing binary is reported by preflight _before_ dispatch, as a warning rather than a block. +5. Re-promoting unchanged configuration uploads nothing. +6. The harness's own `CLAUDE.md` is provably absent from a promoted session. +7. Added cold-path cost (bundle fetch, verify, unpack) measured locally — 113.7 ms median; the + end-to-end, in-cluster cold-start delta remains owed (see §8 Measurement). +8. The lockfile is committed and diffs legibly between promotions. + +Continuing the red-team precedent from the fs-free spec: **a grep assertion that no bundle +content is written outside `/tmp` on the harness side**, keeping the lockdown posture +([ADR-0011](../adrs/0011-harness-lockdown.md)) intact. + +## 9. Deferred: subagent support + +Pi has no Task equivalent — `pi-fork/packages/coding-agent/src/core/tools/` holds `bash`, +`edit`, `find`, `grep`, `ls`, `read`, `write` and nothing that spawns a nested agent. Support +is buildable, since `createAgentSession` is exported from Pi's SDK, but it needs nested session +IDs derived from the parent, budget roll-up through `budget-voter.ts`, a defined interaction +with `checkpoint-extension.ts`, and a depth cap. That is net-new code touching the two most +delicate pieces of existing machinery and warrants **its own spec**. + +The seam is clean: subagent-dependent skills drop today with a stable reason code, and when the +extension lands the code flips and they travel. Nothing else in this design changes. + +## 10. Risks + +- **A lying preflight.** Mitigated by the CI inventory check (§4.5); without it, trust decays + silently. +- **Deny-list rot.** Curation is a maintenance cost; a newly-added local skill family that + cannot work remotely will misfire until the list catches up. Accepted deliberately over + heuristic inference. +- **Cold-start regression.** Bounded by measurement (§8) rather than assumption. +- **Blob size in Redis.** Single-digit MB with dedupe and TTL is acceptable; an object store is + the escape hatch if bundles grow, and D1's digest indirection makes that swap local. +- **Semantic drift.** A promoted workflow can behave differently for reasons no check catches + (§4.6, tier 3). The mitigation is honesty in the report, not a promise of fidelity. + +## 11. Recommended workflow: author in a sandbox, not in your whole `~/.claude` + +Added after implementation, on the project owner's direction: _transporting a whole local +environment to the remote harness is inherently challenging, so the better practice is to start +Claude in a local sandbox, add only the skills and tools that workflow needs there, and run from +there._ + +Everything measured while building this feature argues the same way. A real `~/.claude` yielded 149 +`SKILL.md` files resolving to 60 skills, 55 of which travelled, producing an 8.6 MB bundle with 45 +preflight warnings — nearly all originating in skills the workflow never uses. More tellingly, three +separate preflight checks had to be demoted from blocking to advisory (§6 D10) purely because an +uncurated environment is that noisy. The classifier, the curated deny-list, and the heuristic checks +are all machinery for _coping with_ an environment that was never curated for remote execution. +Sandbox-first authoring removes the need for most of it rather than making it smarter. + +**What is unaffected**, and therefore worth building either way: the bundle format and content +digest (§4.1), the content-addressed store, harness-side materialization (§4.4), the sandbox overlay +and path translation (§4.5), the injected prompt notes, and the lockfile. + +**What it demotes:** relevance pruning and the curated deny-list (§4.2) shrink toward irrelevance +when the environment contains only what the workflow needs. + +**What it would restore:** blocking preflight. D10 exists because an uncurated environment produces +false positives at a rate that makes blocking untenable; a curated one should yield close to zero +findings, at which point erroring on them is both safe and more useful. + +**The honest cost.** The original motivation in §1 was that people already know Claude Code, so they +should be able to iterate in the environment they already have. A deliberately minimal sandbox is +less comfortable than a real setup. That is the standard dev/prod-parity trade, and parity usually +wins — but it does change the pitch from "keep working the way you work" to "work in a sandbox that +resembles production." diff --git a/harness/package.json b/harness/package.json index d89e0ce..f09bad0 100644 --- a/harness/package.json +++ b/harness/package.json @@ -11,14 +11,20 @@ "./budget-voter": "./src/budget-voter.ts", "./leaf-job-runner": "./src/leaf-job-runner.ts", "./leaf-result-store": "./src/leaf-result-store.ts", - "./sandbox-lease": "./src/sandbox-lease.ts" + "./sandbox-lease": "./src/sandbox-lease.ts", + "./config-store": "./src/config-store.ts", + "./config-resolver": "./src/config-resolver.ts", + "./config-overlay": "./src/config-overlay.ts", + "./promote": "./src/promote.ts" }, "scripts": { "test": "vitest run", - "typecheck": "tsc --noEmit" + "typecheck": "tsc --noEmit", + "promote": "tsx src/promote-cli.ts" }, "dependencies": { "@grpc/grpc-js": "^1.12.0", + "@sh/config-bundle": "workspace:*", "@sh/session-backend": "workspace:*", "@sh/k8s-sandbox": "workspace:*", "@sh/work-queue": "workspace:*", diff --git a/harness/src/config-overlay.ts b/harness/src/config-overlay.ts new file mode 100644 index 0000000..0a85e78 --- /dev/null +++ b/harness/src/config-overlay.ts @@ -0,0 +1,132 @@ +import { assertValidDigest, digestDirName } from '@sh/config-bundle'; +import type { SandboxTransport } from '@sh/k8s-sandbox'; + +/** Single-quote-escape for safe bash interpolation. Copied from converge.ts:4 by design. */ +function sq(s: string): string { + return `'${s.replace(/'/g, `'\\''`)}'`; +} + +/** Shared, immutable, digest-keyed: safe to reuse across every leaf on this pod. */ +export function configCacheDir(digest: string): string { + return `/workspace/.sh-config/${digestDirName(assertValidDigest(digest))}`; +} + +/** Per-leaf link, under the leaf workspace so `cleanupWorkspace` remains the only teardown path. */ +export function leafConfigDir(runId: string): string { + return `/workspace/leaves/${runId}/.sh-config`; +} + +export interface OverlayPaths { + skillsDir: string; + memoryDir: string; +} + +/** Prints `hit` when the digest is already cached, `miss` otherwise. */ +export function buildCacheProbeScript(digest: string): string { + return [ + `set -eu`, + // Marker so a test's fake transport can tell the probe call from the others by content rather + // than by call position. A no-op shell comment; keep it if you touch this script. + `# CACHE_PROBE`, + `DIR=${sq(configCacheDir(digest))}`, + `if [ -d "$DIR" ]; then printf 'hit'; else printf 'miss'; fi`, + ].join('\n'); +} + +/** + * Populate the shared cache from base64 tar.gz on stdin, under the same per-pod flock + * `converge.ts` uses, so concurrent leaves on a cold pod do not race. Staged then renamed, so + * a crash cannot leave a half-populated cache that would truncate skill instructions. + */ +export function buildCachePopulateScript(digest: string): string { + const DIR = configCacheDir(digest); + return [ + // pipefail is REQUIRED, not stylistic. Without it the `base64 -d | tar` pipeline reports only + // tar's status, so a truncated or corrupt stdin can leave base64 failing while tar returns 0 on + // the partial bytes it did receive — and the script would then chmod, mv and print 'ok' over an + // incompletely populated shared cache. Safe to use: the transport runs `bash -c` + // (k8s-sandbox/src/transport.ts:35) and every sandbox variant installs bash. + `set -euo pipefail`, + `DIR=${sq(DIR)}; LOCK=/workspace/.sh-config.lock`, + `mkdir -p /workspace/.sh-config`, + `TMP="$DIR.tmp.$$"`, + `(`, + ` flock 9`, + // Clean up the staging dir on ANY failure. Without this, `set -e` aborts before the mv and + // leaves $DIR.tmp.$$ behind — and since each attempt uses a fresh PID, a systemic failure + // (a transport truncating stdin, say) accumulates stale staging dirs indefinitely on a + // long-lived pooled sandbox. The canonical path is still never half-populated either way. + // `chmod -R a-w` below (ADR-0031) makes the staged tree read-only before it can ever fail + // out of this trap, so the trap must restore write permission first, or `rm -rf` on a + // read-only tree can itself fail and leak the staging dir on every failure. + ` trap 'chmod -R u+w "$TMP" 2>/dev/null || true; rm -rf "$TMP"' EXIT`, + // Benign race, deliberately tolerated rather than fixed: if two leaves both miss the probe, + // the flock loser reaches this line and exits WITHOUT draining the bundle piped on stdin. The + // exec transports tolerate an undrained stdin, and overlayConfig only sends bytes on a miss, so + // the cost is one wasted transfer at worst. Do not "fix" this by draining or by dropping the + // early exit: the first reintroduces the transfer this optimisation exists to avoid, and the + // second re-extracts over a populated cache. + ` # Benign undrained-stdin race: loser exits without draining piped bundle; transports tolerate it.`, + ` [ -d "$DIR" ] && exit 0`, + ` rm -rf "$TMP"; mkdir -p "$TMP"`, + ` base64 -d | tar -x -z -C "$TMP"`, + ` find "$TMP" -name '*.sh' -exec chmod +x {} +`, + // ADR-0031: promoted memory (and skill bodies) must be read-only, because this cache is + // shared by every leaf on the pod and nothing else prevents one leaf's write from mutating + // what every later leaf on this pod reads. Drop write AFTER the +x pass (so the scripts this + // cache ships stay executable) and BEFORE the mv (so the canonical path is never briefly + // writable). + ` chmod -R a-w "$TMP"`, + ` mv "$TMP" "$DIR"`, + `) 9>"$LOCK"`, + `printf 'ok'`, + ].join('\n'); +} + +/** Link the shared cache into this leaf's workspace and echo the leaf-local root. */ +export function buildLeafBindScript(digest: string, runId: string): string { + const LEAF = leafConfigDir(runId); + return [ + `set -eu`, + `DIR=${sq(configCacheDir(digest))}; LEAF=${sq(LEAF)}`, + `mkdir -p "$(dirname "$LEAF")"`, + `ln -sfn "$DIR" "$LEAF"`, + `printf '%s' "$LEAF"`, + ].join('\n'); +} + +/** Drop only the per-leaf link. The shared cache is immutable and outlives every leaf. */ +export function buildConfigCleanupScript(runId: string): string { + return [`set -u`, `rm -f ${sq(leafConfigDir(runId))} 2>/dev/null || true`].join('\n'); +} + +async function run(transport: SandboxTransport, script: string, stdin?: Buffer): Promise { + const { stdout, exitCode, truncated } = await transport.exec(script, { + timeout: 300, + ...(stdin ? { stdin } : {}), + }); + if (truncated) throw new Error('config overlay exceeded the sandbox output cap'); + if (exitCode !== 0) throw new Error(`config overlay failed (exit ${exitCode})`); + return stdout.toString(); +} + +/** + * Mirror the bundle into the sandbox and return the paths the injected prompt notes name. + * + * Probes first and pushes bytes only on a miss: bundles are immutable and content-addressed, so + * a 200-leaf fan-out transfers the bundle once rather than 200 times (spec §4.5). + */ +export async function overlayConfig( + transport: SandboxTransport, + digest: string, + runId: string, + tarGz: Buffer, +): Promise { + const probe = await run(transport, buildCacheProbeScript(digest)); + if (probe.trim() !== 'hit') { + await run(transport, buildCachePopulateScript(digest), Buffer.from(tarGz.toString('base64'))); + } + await run(transport, buildLeafBindScript(digest, runId)); + const root = leafConfigDir(runId); + return { skillsDir: `${root}/skills`, memoryDir: `${root}/memory` }; +} diff --git a/harness/src/config-resolver.ts b/harness/src/config-resolver.ts new file mode 100644 index 0000000..d210ce7 --- /dev/null +++ b/harness/src/config-resolver.ts @@ -0,0 +1,142 @@ +import { existsSync, mkdirSync, mkdtempSync, renameSync, rmSync, writeFileSync } from 'node:fs'; +import { join, dirname, resolve, sep } from 'node:path'; +import { assertValidDigest, digestDirName, untar, type TarEntry } from '@sh/config-bundle'; +import { getBundle, type BundleRedisLike } from './config-store.js'; + +/** The harness pod mounts no writable volume except an emptyDir /tmp (ADR-0020). */ +export const DEFAULT_CONFIG_BASE_DIR = '/tmp/sh-config'; + +export interface PromotedConfig { + digest: string; + root: string; + skillsDir: string; + promptsDir: string; + /** Injected inline via agentsFilesOverride — never written to disk. */ + context: Array<{ path: string; content: string }>; + promptFragments: string[]; + /** The whole bundle, handed to the sandbox overlay so both halves share one digest. */ + entries: TarEntry[]; +} + +export interface PromotedLoaderOptions { + additionalSkillPaths: string[]; + additionalPromptTemplatePaths: string[]; + noContextFiles: true; + noSkills: true; + noPromptTemplates: true; + agentsFilesOverride: (base: { agentsFiles: Array<{ path: string; content: string }> }) => { + agentsFiles: Array<{ path: string; content: string }>; + }; + appendSystemPrompt: string[]; +} + +/** + * Unpack into a digest-named directory. Digest-keyed means the cache can never be stale, so + * its death with the pod is correct rather than unfortunate. + * + * Unpack goes to a temp dir and is renamed into place: a crash mid-unpack must never leave a + * half-populated skill directory, which would silently truncate a skill's instructions. + */ +export function unpackBundle( + tar: Buffer, + digest: string, + baseDir: string = DEFAULT_CONFIG_BASE_DIR, +): PromotedConfig { + const entries = untar(tar); + const root = join(baseDir, digestDirName(assertValidDigest(digest))); + + if (!existsSync(root)) { + mkdirSync(baseDir, { recursive: true }); + const staging = mkdtempSync(join(baseDir, '.tmp-')); + try { + for (const entry of entries) { + // Path safety is checked for EVERY entry, before the prefix filter. Checking it after + // would let a traversal path outside skills//prompts/ be silently skipped rather than + // rejected, which is a weaker guarantee than "no bundle can write outside its root". + const target = resolve(staging, entry.path); + if (target !== staging && !target.startsWith(staging + sep)) { + throw new Error(`bundle entry escapes the unpack root: ${entry.path}`); + } + // Only skills/ and prompts/ need to be files; context/ and prompt/ are read into memory, + // and memory/ is destined for the sandbox. + if (!entry.path.startsWith('skills/') && !entry.path.startsWith('prompts/')) continue; + mkdirSync(dirname(target), { recursive: true }); + writeFileSync(target, entry.content, { mode: entry.mode ?? 0o644 }); + } + mkdirSync(join(staging, 'skills'), { recursive: true }); + mkdirSync(join(staging, 'prompts'), { recursive: true }); + renameSync(staging, root); + } catch (err) { + rmSync(staging, { recursive: true, force: true }); + // An EEXIST/ENOTEMPTY rename means a concurrent turn in this pod won the race; that is + // fine, because identical digests mean identical content. + if (!existsSync(root)) throw err; + } + } + + // Numeric-aware where an index is present: a plain lexicographic sort puts `append-10` before + // `append-2`, silently reordering injected prompt notes. Unreachable today (only two fragments are + // ever produced) but the cost of being wrong later is a scrambled system prompt. + const indexOf = (p: string): number => { + const m = /-(\d+)\.md$/.exec(p); + return m ? Number(m[1]) : Number.NaN; + }; + const text = (prefix: string) => + entries + .filter((e) => e.path.startsWith(prefix)) + .sort((a, b) => { + const ia = indexOf(a.path); + const ib = indexOf(b.path); + if (!Number.isNaN(ia) && !Number.isNaN(ib) && ia !== ib) return ia - ib; + return a.path < b.path ? -1 : a.path > b.path ? 1 : 0; + }); + + return { + digest, + root, + skillsDir: join(root, 'skills'), + promptsDir: join(root, 'prompts'), + context: text('context/').map((e) => ({ path: e.path, content: e.content.toString('utf8') })), + promptFragments: text('prompt/').map((e) => e.content.toString('utf8')), + entries, + }; +} + +/** + * Options that point pi at the bundle and suppress all discovery. + * + * `noSkills: true` still honours `additionalSkillPaths` — resource-loader.ts:405-407 merges them + * in both branches — so this yields exactly the bundle's skills and nothing from the pod. + * + * `appendSystemPrompt` strings are passed as literal text. Note that pi's `resolvePromptInput` + * (resource-loader.ts:49) reads a string as a FILE when `existsSync(input)`; our fragments are + * multi-line prose, so they can never collide with a real path. + */ +export function buildLoaderOptions(promoted: PromotedConfig): PromotedLoaderOptions { + return { + additionalSkillPaths: [promoted.skillsDir], + additionalPromptTemplatePaths: [promoted.promptsDir], + noContextFiles: true, + noSkills: true, + noPromptTemplates: true, + // Ignores `base` deliberately — see the leak regression test in test/config-resolver.test.ts. + agentsFilesOverride: () => ({ agentsFiles: promoted.context }), + appendSystemPrompt: promoted.promptFragments, + }; +} + +/** Spread into the loader construction. Empty when nothing is promoted: behavior is unchanged. */ +export function promotedLoaderOptions( + promoted?: PromotedConfig, +): PromotedLoaderOptions | Record { + return promoted ? buildLoaderOptions(promoted) : {}; +} + +/** Fetch, verify and unpack. Throws (never degrades) on a missing or corrupt bundle. */ +export async function resolvePromotedConfig( + redis: BundleRedisLike, + digest: string, + baseDir: string = DEFAULT_CONFIG_BASE_DIR, +): Promise { + return unpackBundle(await getBundle(redis, digest), digest, baseDir); +} diff --git a/harness/src/config-store.ts b/harness/src/config-store.ts new file mode 100644 index 0000000..38221bb --- /dev/null +++ b/harness/src/config-store.ts @@ -0,0 +1,94 @@ +import { gunzipSync, gzipSync } from 'node:zlib'; +import { contentDigest, untar } from '@sh/config-bundle'; + +/** + * Minimal structural Redis surface — lets unit tests inject an in-memory fake, exactly as + * `leaf-result-store.ts` does. `exists` is what makes an unchanged re-promotion free. + */ +export interface BundleRedisLike { + set(key: string, value: string, opts?: { EX?: number }): Promise; + get(key: string): Promise; + exists(key: string): Promise; + expire(key: string, seconds: number): Promise; +} + +/** Bundles are immutable; a TTL only reclaims space for workflows nobody dispatches any more. */ +export const DEFAULT_BUNDLE_TTL_SECONDS = 60 * 60 * 24 * 30; + +export function bundleKey(digest: string): string { + return `config:bundle:${digest}`; +} + +export class BundleNotFoundError extends Error { + constructor(readonly digest: string) { + super(`config bundle not found: ${digest}`); + this.name = 'BundleNotFoundError'; + } +} + +export class BundleDigestMismatchError extends Error { + constructor( + readonly expected: string, + readonly actual: string, + ) { + super(`config bundle digest mismatch: expected ${expected}, stored bytes hash to ${actual}`); + this.name = 'BundleDigestMismatchError'; + } +} + +/** + * Store the bundle under its digest, gzipped and base64'd (base64 keeps the injectable + * `BundleRedisLike` a plain string interface). Content-addressed, so an existing key means + * identical content and the write is skipped. Verify the digest matches the tar first to prevent + * key poisoning: a mismatched pair blocks any correct write under that digest for 30 days. + */ +export async function putBundle( + redis: BundleRedisLike, + digest: string, + tar: Buffer, + ttlSeconds: number = DEFAULT_BUNDLE_TTL_SECONDS, +): Promise<{ uploaded: boolean }> { + // Verify digest matches tar before anything else + let actual: string; + try { + actual = contentDigest(untar(tar)); + } catch { + throw new BundleDigestMismatchError(digest, 'unreadable (untar failed)'); + } + if (actual !== digest) throw new BundleDigestMismatchError(digest, actual); + + const key = bundleKey(digest); + if ((await redis.exists(key)) > 0) { + // Refresh TTL on skip: re-promotion must not let bundles age out while in active use + await redis.expire(key, ttlSeconds); + return { uploaded: false }; + } + await redis.set(key, gzipSync(tar).toString('base64'), { EX: ttlSeconds }); + return { uploaded: true }; +} + +/** + * Fetch and verify. A missing digest or a hash mismatch throws, never degrades: running a leaf + * with silently-absent configuration produces plausible-but-wrong work, which is the expensive + * remote failure this design exists to prevent (spec §4.4). + */ +export async function getBundle(redis: BundleRedisLike, digest: string): Promise { + const raw = await redis.get(bundleKey(digest)); + if (raw === null) throw new BundleNotFoundError(digest); + + let tar: Buffer; + try { + tar = gunzipSync(Buffer.from(raw, 'base64')); + } catch { + throw new BundleDigestMismatchError(digest, 'unreadable (gunzip failed)'); + } + + let actual: string; + try { + actual = contentDigest(untar(tar)); + } catch { + throw new BundleDigestMismatchError(digest, 'unreadable (untar failed)'); + } + if (actual !== digest) throw new BundleDigestMismatchError(digest, actual); + return tar; +} diff --git a/harness/src/promote-cli.ts b/harness/src/promote-cli.ts new file mode 100644 index 0000000..eb65101 --- /dev/null +++ b/harness/src/promote-cli.ts @@ -0,0 +1,129 @@ +import { existsSync, mkdirSync, statSync, writeFileSync } from 'node:fs'; +import { homedir } from 'node:os'; +import { join, dirname } from 'node:path'; +import { createClient } from 'redis'; +import { + buildBundle, + hasErrors, + renderPreflight, + serializeLockfile, + SecretScanError, +} from '@sh/config-bundle'; +import { putBundle, type BundleRedisLike } from './config-store.js'; +import { + LOCKFILE_OUT, + parsePromoteArgs, + promoteInputs, + readInventory, + resolveInventoryPath, + resolveProjectDir, +} from './promote.js'; + +/** Write the lockfile to the filesystem. */ +function writeLockfile(cwd: string, lockfile: ReturnType): void { + const lockPath = join(cwd, LOCKFILE_OUT); + mkdirSync(dirname(lockPath), { recursive: true }); + writeFileSync(lockPath, lockfile); +} + +async function main(): Promise { + const args = parsePromoteArgs(process.argv.slice(2)); + // `--project` names the project being promoted; without it, promote reads whatever directory + // the process happens to be started from -- which, run via `pnpm promote` from `harness/`, is + // the harness checkout itself, not the caller's project. That silently bundled the harness's + // own CLAUDE.md and zero memory. Fail loudly rather than promote the wrong thing. + const cwd = resolveProjectDir(args, process.cwd()); + if (!existsSync(cwd) || !statSync(cwd).isDirectory()) { + console.error(`promote aborted: project directory does not exist: ${cwd}`); + process.exit(1); + } + console.log(`project: ${cwd}`); + + const inventoryPath = resolveInventoryPath(args.sandboxImage, cwd); + const inventory = readInventory(args.sandboxImage, cwd); + // Say which inventory was used. Module-relative outranks cwd-local, so a caller who dropped a + // deliberate override beside their project must be able to see that it was not the one read. + console.log( + inventoryPath === undefined + ? `inventory: none for ${args.sandboxImage} — the binary check will warn, not verify` + : `inventory: ${inventoryPath} (${inventory?.length ?? 0} binaries)`, + ); + + const result = buildBundle( + promoteInputs({ + cwd, + home: homedir(), + args, + ...(inventory ? { inventory } : {}), + versions: { + pi: process.env.SH_PI_VERSION ?? 'unknown', + harness: process.env.SH_HARNESS_VERSION ?? '0.0.0', + }, + }), + ); + + console.log( + ` resolved ${result.lockfile.skills.length + result.lockfile.dropped.length} skills`, + ); + console.log(` travels ${result.lockfile.skills.length}`); + console.log(` dropped ${result.lockfile.dropped.length}`); + for (const d of result.lockfile.dropped) console.log(` ${d.name} (${d.reason})`); + console.log( + ` context ${result.lockfile.context.length} file(s), ${result.lockfile.memory.length} memory file(s)`, + ); + const secretWarnings = result.findings.filter((f) => f.code === 'possible_secret'); + console.log( + ` secrets no blocking findings` + + (secretWarnings.length ? `, ${secretWarnings.length} warning(s) — see below` : ''), + ); + console.log(` entry ${result.lockfile.entry}`); + console.log(''); + console.log(renderPreflight(result.findings)); + console.log(''); + + if (hasErrors(result.findings)) { + console.error('promote aborted: preflight found errors (see above)'); + process.exit(2); + } + + if (args.dryRun) { + console.log( + ` bundle ${result.digest} (${result.tar.length} bytes, --dry-run: not uploaded)`, + ); + console.log(` lockfile ${LOCKFILE_OUT} (--dry-run: not written)`); + return; + } + + const client = createClient({ url: process.env.REDIS_URL }); + await client.connect(); + try { + const { uploaded } = await putBundle( + client as unknown as BundleRedisLike, + result.digest, + result.tar, + ); + writeLockfile(cwd, serializeLockfile(result.lockfile)); + console.log( + ` bundle ${result.digest} (${result.tar.length} bytes, ${uploaded ? 'uploaded' : 'unchanged — upload skipped'})`, + ); + console.log(` lockfile ${LOCKFILE_OUT}`); + console.log(''); + console.log( + `dispatch with: {"sessionId":"/","kind":"prompt","prompt":"…","configRef":"${result.digest}"}`, + ); + } finally { + await client.quit(); + } +} + +main().catch((err) => { + if (err instanceof SecretScanError) { + // Only structural credential formats reach here; heuristic hits are warnings in the report. + console.error(`promote BLOCKED — ${err.message}`); + for (const f of err.findings) console.error(` ${f.path}:${f.line} ${f.rule}`); + console.error('\nRemove the credential or add the file to your deny-list, then re-run.'); + process.exit(3); + } + console.error(err instanceof Error ? err.message : String(err)); + process.exit(1); +}); diff --git a/harness/src/promote.ts b/harness/src/promote.ts new file mode 100644 index 0000000..c76307f --- /dev/null +++ b/harness/src/promote.ts @@ -0,0 +1,213 @@ +import { existsSync, readFileSync } from 'node:fs'; +import { dirname, join, parse, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import type { BuildBundleInput, PromoteMode } from '@sh/config-bundle'; + +/** Where the generated lockfile is written, for committing alongside the code it configures. */ +export const LOCKFILE_OUT = '.claude/promoted.lock.json'; + +export interface PromoteArgs { + entry: string; + mode: PromoteMode; + sandboxImage: string; + deny: string[]; + dryRun: boolean; + project?: string; +} + +export function parsePromoteArgs(argv: string[]): PromoteArgs { + const args: PromoteArgs = { + entry: '', + mode: 'unattended', + // Matches deploy/knative/setup-k8s.sh:30 so the checked-in inventory (Task 14) resolves. + sandboxImage: 'ghcr.io/rossoctl/serverless-harness-sandbox:latest', + deny: [], + dryRun: false, + }; + for (let i = 0; i < argv.length; i++) { + const flag = argv[i]!; + const value = argv[i + 1]; + if (flag === '--entry') ((args.entry = value ?? ''), i++); + else if (flag === '--mode') { + if (value !== 'unattended' && value !== 'attended') { + throw new Error(`--mode must be 'unattended' or 'attended', got '${value ?? ''}'`); + } + args.mode = value; + i++; + } else if (flag === '--sandbox-image') ((args.sandboxImage = value ?? ''), i++); + else if (flag === '--deny') (args.deny.push(value ?? ''), i++); + else if (flag === '--project') { + if (!value) throw new Error('--project requires a directory'); + args.project = value; + i++; + } else if (flag === '--dry-run') args.dryRun = true; + else throw new Error(`unknown flag: ${flag}`); + } + if (!args.entry) throw new Error('usage: sh promote --entry [--mode attended]'); + return args; +} + +/** + * The project directory to promote from: `--project` resolved to an absolute path, or `cwd` + * (the process's own working directory) when `--project` was not given. + * + * This is the fix for the only shipped invocation (`cd harness && pnpm promote`) silently + * promoting the harness checkout's own configuration: `pnpm run` sets `cwd` to the package + * directory, not the directory the caller actually meant, so an explicit `--project` is the only + * reliable way to name "the project" from inside a workspace script. + */ +export function resolveProjectDir(args: Pick, cwd: string): string { + return args.project !== undefined ? resolve(args.project) : cwd; +} + +/** + * Mirror Claude Code's own project slug: path separators replaced with '-'. + * + * This scheme is lossy -- `/a/my-project` and `/a/my/project` both slug to `-a-my-project` -- and + * that is INHERITED ON PURPOSE. Verified against a real install: the directory Claude Code created + * for this repo is `-Users-paolo-Projects-aiplatform-serverless-harness`, hyphen in the final + * segment and all. Our job is to FIND the directory Claude Code already made, so a "safer", + * collision-free scheme would simply miss it and silently promote no memory at all. Do not + * "improve" this. + */ +export function projectMemoryDir(cwd: string, home: string): string { + return join(home, '.claude', 'projects', cwd.split(/[/\\]/).join('-'), 'memory'); +} + +/** Relative location of the checked-in sandbox inventories, from a repo or package root. */ +export const INVENTORY_SUBDIR = join('deploy', 'knative', 'sandbox-inventory'); + +/** The inventory filename for an image ref: `:` and `/` become `_`. Mirrors readInventory's key. */ +export function inventoryFileName(image: string): string { + return `${image.replace(/[:/]/g, '_')}.json`; +} + +/** + * Candidate inventory paths for an image, in precedence order. + * + * The inventory is a HARNESS-SHIPPED asset, so it must be found relative to where the harness is + * installed -- NOT relative to the invocation directory. `sh promote` is run from the user's own + * project (that is the whole point of the feature), and a user's project will never contain + * `deploy/knative/sandbox-inventory/`. Resolving against `cwd` alone made the binary check + * silently degrade to `inventory_unavailable` for every real caller, which is the "lying + * preflight" this inventory exists to prevent -- measured: from the repo root the check produced + * 29 findings, from one directory deeper it produced none. + * + * `cwd` is kept as a LOWER-precedence fallback so a caller can still override with a local file. + */ +export function inventoryCandidates(image: string, cwd: string, moduleDir: string): string[] { + const file = inventoryFileName(image); + const out: string[] = []; + // Walk up from the module: src/ -> package -> repo root (and any bundling layout in between). + let dir = moduleDir; + for (;;) { + out.push(join(dir, INVENTORY_SUBDIR, file)); + const parent = dirname(dir); + if (parent === dir) break; + dir = parent; + } + out.push(join(cwd, INVENTORY_SUBDIR, file)); + return out; +} + +/** + * The first existing inventory path for an image, or undefined. + * + * Exposed separately from `readInventory` so the CLI can SAY which file it used. The precedence + * is deliberately silent-proof: a shipped inventory outranks a cwd-local one, so without a + * diagnostic a caller who deliberately dropped an override next to their project would be + * shadowed by the shipped copy with no way to tell. + */ +export function resolveInventoryPath( + image: string, + cwd: string, + moduleDir: string = dirname(fileURLToPath(import.meta.url)), +): string | undefined { + return inventoryCandidates(image, cwd, moduleDir).find((path) => existsSync(path)); +} + +/** Checked-in inventory for a sandbox image tag; absent => preflight warns instead of verifying. */ +export function readInventory( + image: string, + cwd: string, + moduleDir: string = dirname(fileURLToPath(import.meta.url)), +): string[] | undefined { + const path = resolveInventoryPath(image, cwd, moduleDir); + if (path === undefined) return undefined; + return JSON.parse(readFileSync(path, 'utf8')).binaries as string[]; +} + +/** + * The nearest ancestor holding a `.git` entry, or `cwd` when there is none. + * + * This bounds the context-file walk. Without it the walk reaches the filesystem root and sweeps + * every ancestor `CLAUDE.md` -- including a personal `~/CLAUDE.md` -- into a bundle that lands in a + * shared Redis store. + * + * `existsSync` is deliberate rather than a directory check: in a linked worktree `.git` is a *file* + * containing a `gitdir:` pointer, not a directory, and this repo uses worktrees heavily. A + * directory-only test would walk straight past the boundary in exactly the checkout where it is + * needed most. + */ +export function projectRoot(cwd: string): string { + let dir = cwd; + for (;;) { + if (existsSync(join(dir, '.git'))) { + return dir; + } + const parent = dirname(dir); + if (parent === dir) { + // Reached filesystem root without finding .git, return the original cwd. + return cwd; + } + dir = parent; + } +} + +/** The CLAUDE.md / AGENTS.md chain from the project root down to `cwd`, outermost first. */ +export function collectContextFiles(cwd: string): Array<{ path: string; content: string }> { + const out: Array<{ path: string; content: string }> = []; + const root = projectRoot(cwd); + const dirs: string[] = []; + for (let dir = cwd; ; dir = dirname(dir)) { + dirs.unshift(dir); + if (dir === root) break; + } + for (const dir of dirs) { + for (const name of ['AGENTS.md', 'CLAUDE.md']) { + const path = join(dir, name); + if (existsSync(path)) { + out.push({ path, content: readFileSync(path, 'utf8') }); + break; + } + } + } + return out; +} + +/** Assemble buildBundle's input from Claude Code's own on-disk layout. */ +export function promoteInputs(opts: { + cwd: string; + home: string; + args: PromoteArgs; + inventory?: string[]; + versions: { pi: string; harness: string }; +}): BuildBundleInput { + const userDir = join(opts.home, '.claude'); + return { + roots: { + projectDir: join(opts.cwd, '.claude'), + userDir, + pluginDirs: [join(userDir, 'plugins')], + }, + memoryDir: projectMemoryDir(opts.cwd, opts.home), + promptsDir: join(userDir, 'commands'), + contextFiles: collectContextFiles(opts.cwd), + entry: opts.args.entry, + mode: opts.args.mode, + userDenyList: opts.args.deny, + sandboxImage: opts.args.sandboxImage, + ...(opts.inventory ? { inventory: opts.inventory } : {}), + versions: opts.versions, + }; +} diff --git a/harness/src/run-leaf.ts b/harness/src/run-leaf.ts index 34d7699..e601694 100644 --- a/harness/src/run-leaf.ts +++ b/harness/src/run-leaf.ts @@ -49,6 +49,12 @@ import { GATE_DECISION_ENTRY_TYPE, } from './gate.js'; import { requestApprovalExtension } from './request-approval-tool.js'; +import { gzipSync } from 'node:zlib'; +import { createClient } from 'redis'; +import { canonicalTar } from '@sh/config-bundle'; +import { resolvePromotedConfig, type PromotedConfig } from './config-resolver.js'; +import { overlayConfig, buildConfigCleanupScript } from './config-overlay.js'; +import type { BundleRedisLike } from './config-store.js'; /** * Recover a verdict from a persisted `verdict` custom session entry (written by @@ -103,6 +109,12 @@ export interface LeafEnvelope { maxTurns?: number; async?: boolean; // when true, the HTTP layer enqueues instead of running inline tenant?: string; // namespaces the session id + /** + * Digest of a promoted Claude Code config bundle (`sha256:…`). Absent ⇒ no promoted config and + * behavior is exactly as before. Flows through the server and the work queue untouched, because + * both pass the envelope through whole. + */ + configRef?: string; kind?: 'converge' | 'solve' | 'prompt'; // absent/"converge" => existing behavior; "solve" => runSolveLeaf problemStatement?: string; // required when kind === "solve": the task the agent must implement prompt?: string; // required when kind === "prompt": the free-form prompt to run @@ -115,6 +127,33 @@ function sandboxEnvironment(env: LeafEnvelope): NodeJS.ProcessEnv { return { ...process.env, KAGENTI_SANDBOX_POOL_SELECTOR: env.sandboxPoolSelector }; } +/** + * Lazily-created Redis client for the bundle store. Kept separate from the session backend's client + * so a bundle fetch cannot interfere with session buffering. + * + * Caches the in-flight PROMISE, not the resolved client, mirroring RedisLeaseStore + * (sandbox-lease.ts:38-45). Caching only the resolved value is a check-then-act race: two leaves + * arriving before the first connect() settles would both pass the `!client` test, each create and + * connect a client, and the loser's connection would be silently leaked — never closed, never + * referenced again. Awaiting one shared promise makes concurrent callers converge on one client. + */ +let bundleRedisPromise: Promise | undefined; +function getBundleRedis(redisUrl?: string): Promise { + if (!bundleRedisPromise) { + bundleRedisPromise = (async () => { + const client = createClient({ url: redisUrl ?? process.env.REDIS_URL }); + await client.connect(); + return client as unknown as BundleRedisLike; + })().catch((err) => { + // Do not cache a failed connect: clear the slot so the next leaf retries rather than + // inheriting a permanently rejected promise. + bundleRedisPromise = undefined; + throw err; + }); + } + return bundleRedisPromise; +} + /** The Pi/Redis session id for a leaf: tenant-prefixed (if any), then sanitized. */ export function leafSessionId(env: { sessionId: string; tenant?: string }): string { return toSessionId(env.tenant ? `${env.tenant}/${env.sessionId}` : env.sessionId); @@ -251,6 +290,9 @@ export async function runLeaf( produceVerdict?: ProduceVerdict; produceSolve?: ProduceSolve; executeTurn?: typeof executeTurn; + resolvePromotedConfig?: typeof resolvePromotedConfig; + overlayConfig?: typeof overlayConfig; + bundleRedis?: BundleRedisLike; }, ): Promise { if (env.kind === 'solve') return runSolveLeaf(env, config, deps); @@ -318,7 +360,12 @@ export async function runSolveLeaf( async function runPromptLeaf( env: LeafEnvelope, config?: TurnConfig, - deps?: { executeTurn?: typeof executeTurn }, + deps?: { + executeTurn?: typeof executeTurn; + resolvePromotedConfig?: typeof resolvePromotedConfig; + overlayConfig?: typeof overlayConfig; + bundleRedis?: BundleRedisLike; + }, ): Promise { if (!env.prompt) return { status: 'failed', reason: 'bad_inputs' }; const cwd = config?.cwd ?? process.cwd(); @@ -359,6 +406,13 @@ async function runPromptLeaf( } let heartbeat: ReturnType | undefined; + // Set once the sandbox overlay actually lands, so the finally block below knows there is a + // per-leaf /workspace/leaves//.sh-config link to tear down. runPromptLeaf never converges a + // workspace (it has no repoUrl/ref and never calls convergeWorkspace/cleanupWorkspace), so + // without this nothing else ever removes that link -- it leaks on every promoted prompt leaf on a + // long-lived pooled pod. Declared here (not inside the try below) so the finally block -- a + // sibling block, not nested inside try -- can actually see it. + let overlayCreated = false; try { if (selected) { const hbMs = Number(process.env.KAGENTI_SANDBOX_HEARTBEAT_MS ?? '20000'); @@ -367,6 +421,69 @@ async function runPromptLeaf( void lease.heartbeat(); }, hbMs); } + // Promoted config: resolve the prose half into this pod's /tmp and mirror the bundle into the + // sandbox we hold a lease on. Both halves come from one digest, and a failure of either fails + // the leaf — running a turn with silently-absent configuration produces plausible-but-wrong + // work, which is exactly the remote failure this design exists to prevent (spec §4.4). Resolving + // this AFTER the heartbeat starts (not before) matters operationally: resolve+overlay fetches a + // multi-MB bundle from Redis and pushes it into the pod over up to three kubectl execs, and with + // no heartbeat running during that window a slow cluster could have the lease reclaimed mid-overlay. + let promotedConfig: PromotedConfig | undefined; + if (env.configRef) { + const resolveFn = deps?.resolvePromotedConfig ?? resolvePromotedConfig; + const overlayFn = deps?.overlayConfig ?? overlayConfig; + try { + promotedConfig = await resolveFn( + deps?.bundleRedis ?? (await getBundleRedis(config?.redisUrl)), + env.configRef, + ); + if (selected) { + // SelectedSandbox.transport is present ONLY for a leased grpc presence record and is + // undefined for pods (select-sandbox.ts:33-34), which is the DEFAULT deployment. Guarding + // on `selected.transport` would therefore skip the overlay entirely on pods: the sandbox + // half of the bundle would never arrive, so skill sibling files and memory would be + // unreadable — and a unit test injecting a fake transport could not detect it. Use the + // same fallback the converge path uses (run-leaf.ts:579-584): build a KubectlTransport + // when none is leased, and close only what we created. + const overlayTransport = selected.transport ?? KubectlTransport(selected.config); + try { + const paths = await overlayFn( + overlayTransport, + env.configRef, + sid, + gzipSync(canonicalTar(promotedConfig.entries)), + ); + overlayCreated = true; + promotedConfig = { + ...promotedConfig, + promptFragments: [ + ...promotedConfig.promptFragments, + // Self-describing and multi-line on purpose: this is the ONLY place the absolute + // sandbox paths exist (the bundle is content-addressed and built before any leaf + // or sandbox does, so notes.ts's skillsRootNote() cannot bake them in — it instead + // points back at these two lines). A single-line string here would also risk + // pi-fork's resolvePromptInput treating it as a file path when existsSync(input) + // is true. + [ + 'The following are absolute sandbox paths for this session:', + '', + `Skill files: ${paths.skillsDir}`, + `Memory files: ${paths.memoryDir}`, + ].join('\n'), + ], + }; + } finally { + if (!selected.transport) await overlayTransport.close(); + } + } + } catch (err) { + return { + status: 'failed', + reason: 'error', + message: err instanceof Error ? err.message : String(err), + }; + } + } const r: TurnResult = await exec({ prompt: env.prompt, sessionId: env.sessionId, @@ -374,6 +491,7 @@ async function runPromptLeaf( createIfAbsent: true, selection, sandbox: { config: selected?.config ?? null, transport: selected?.transport }, + ...(promotedConfig ? { promotedConfig } : {}), }); if (r.stopReason === 'aborted') return { status: 'aborted' }; if (r.stopReason === 'error') @@ -389,6 +507,20 @@ async function runPromptLeaf( }; } finally { if (heartbeat) clearInterval(heartbeat); + if (overlayCreated && selected) { + // Best-effort, matching cleanupWorkspace (converge.ts): swallow errors so a teardown hiccup + // never masks the leaf's actual verdict. Follows the same transport fallback used above and + // at run-leaf.ts:579-584 -- reuse a leased grpc transport if present, otherwise build a + // KubectlTransport, and close only the transport we created ourselves. + const cleanupTransport = selected.transport ?? KubectlTransport(selected.config); + try { + await cleanupTransport.exec(buildConfigCleanupScript(sid), { timeout: 60 }); + } catch { + /* ignore */ + } finally { + if (!selected.transport) await cleanupTransport.close(); + } + } if (selected) await selected.release(); if (selected?.transport) await selected.transport.close(); } diff --git a/harness/src/run-turn.ts b/harness/src/run-turn.ts index 76fc599..2147ff1 100644 --- a/harness/src/run-turn.ts +++ b/harness/src/run-turn.ts @@ -29,6 +29,7 @@ import { toolChoiceExtension } from './tool-choice-extension.js'; // cycle: run-leaf.ts imports values from run-turn.js, but a `import type` adds no runtime edge. import type { LeafUsage } from './run-leaf.js'; import { sseExtension, type TurnStreamFrame } from './turn-stream.js'; +import { promotedLoaderOptions, type PromotedConfig } from './config-resolver.js'; /** * The sandbox a turn's tool calls run in: a resolved pod/pool config (null ⇒ run tools in the @@ -368,6 +369,27 @@ export function sumBranchUsage(sm: unknown): LeafUsage { return { ...u, total: u.input + u.output + u.cacheRead + u.cacheWrite }; } +export interface BaseLoaderInputs { + cwd: string; + agentDir: string; + settingsManager: unknown; + extensionFactories: unknown[]; +} + +/** + * Assemble DefaultResourceLoader's options. + * + * Extracted as a pure function so the back-compat guarantee is assertable: with no promoted + * bundle the result has EXACTLY the four base keys, which is what makes an absent `configRef` + * byte-identical to today rather than merely intended to be. + */ +export function resourceLoaderOptionsFor( + base: BaseLoaderInputs, + promoted?: PromotedConfig, +): Record { + return { ...base, ...promotedLoaderOptions(promoted) }; +} + export interface ExecuteTurnInput { prompt: string; sessionId?: string; @@ -377,6 +399,8 @@ export interface ExecuteTurnInput { onEvent?: (frame: TurnStreamFrame) => void; // present ⇒ append sseExtension(onEvent) to the stack signal?: AbortSignal; // present ⇒ signal → session.abort() (client disconnect) sandbox?: TurnSandbox; // pre-leased sandbox; absent ⇒ resolve from the environment (/turn) + /** Resolved promoted Claude Code config; absent ⇒ the loader is built exactly as before. */ + promotedConfig?: PromotedConfig; } /** @@ -460,12 +484,12 @@ export async function executeTurn(input: ExecuteTurnInput): Promise extensionFactories.push(sseExtension(input.onEvent)); } - const resourceLoader = new DefaultResourceLoader({ - cwd, - agentDir, - settingsManager, - extensionFactories, - }); + const resourceLoader = new DefaultResourceLoader( + resourceLoaderOptionsFor( + { cwd, agentDir, settingsManager, extensionFactories }, + input.promotedConfig, + ) as never, + ); await resourceLoader.reload(); const { provider, modelId } = input.selection ?? resolveModelSelection(config); diff --git a/harness/test/config-overlay.test.ts b/harness/test/config-overlay.test.ts new file mode 100644 index 0000000..d30f9ca --- /dev/null +++ b/harness/test/config-overlay.test.ts @@ -0,0 +1,178 @@ +import { describe, it, expect } from 'vitest'; +import { gzipSync } from 'node:zlib'; +import { + configCacheDir, + leafConfigDir, + buildCacheProbeScript, + buildCachePopulateScript, + buildLeafBindScript, + buildConfigCleanupScript, + overlayConfig, +} from '../src/config-overlay.js'; + +const DIGEST = 'sha256:' + 'a'.repeat(64); + +describe('paths', () => { + it('caches by digest, shared across leaves', () => { + expect(configCacheDir(DIGEST)).toBe(`/workspace/.sh-config/sha256-${'a'.repeat(64)}`); + }); + it('binds per leaf under the leaf workspace, torn down by run-leaf.ts on the prompt-leaf path (buildConfigCleanupScript) or by cleanupWorkspace when the leaf converges', () => { + expect(leafConfigDir('leaf-1')).toBe('/workspace/leaves/leaf-1/.sh-config'); + }); +}); + +describe('buildCachePopulateScript', () => { + const s = buildCachePopulateScript(DIGEST); + it('extracts from stdin base64, never from a heredoc per file', () => { + expect(s).toContain('base64 -d'); + expect(s).toContain('tar -x'); + }); + it('populates under a flock, matching converge.ts discipline', () => { + expect(s).toMatch(/flock 9[\s\S]*tar -x[\s\S]*9>"\$LOCK"/); + }); + it('is idempotent — an existing digest dir is left alone', () => { + expect(s).toContain('[ -d "$DIR" ] &&'); + }); + it('renames a staging dir into place so a crash cannot half-populate the cache', () => { + expect(s).toContain('mv '); + }); + it('makes .sh scripts executable (tar-over-exec does not preserve the bit reliably)', () => { + expect(s).toContain('chmod +x'); + }); + it('enables pipefail so a failed base64 cannot be masked by a successful tar', () => { + // Without pipefail the pipeline reports only tar's status, and a truncated stdin would produce + // exit 0 with an incompletely populated cache. + expect(s).toContain('set -euo pipefail'); + }); + + it('traps EXIT to remove the staging dir, so failures do not accumulate stale dirs', () => { + expect(s).toMatch(/trap '.*rm -rf "\$TMP"' EXIT/); + // and the trap must be armed BEFORE extraction, or it cannot clean up a failed extract + expect(s.indexOf('trap')).toBeLessThan(s.indexOf('base64 -d')); + }); + + it('restores write permission in the trap before rm -rf, so a read-only staged tree can still be removed on failure', () => { + // ADR-0031 makes the staged tree read-only (chmod -R a-w) before the mv. If the trap's rm -rf + // ever runs on that read-only tree without first restoring write permission, `rm -rf` can fail + // to remove it (a directory needs write permission on itself to unlink its own entries), and a + // staging dir leaks on every failure -- exactly the bug this fix must not introduce. + expect(s).toMatch(/trap 'chmod -R u\+w "\$TMP".*rm -rf "\$TMP"' EXIT/); + }); + + it('drops write permission on the staged tree (ADR-0031: promoted memory/skills must be read-only) after chmod +x and before the mv', () => { + const chmodExecIdx = s.indexOf('chmod +x'); + const chmodReadonlyIdx = s.indexOf('chmod -R a-w'); + const mvIdx = s.lastIndexOf('mv "$TMP" "$DIR"'); + expect(chmodExecIdx).toBeGreaterThan(-1); + expect(chmodReadonlyIdx).toBeGreaterThan(-1); + expect(mvIdx).toBeGreaterThan(-1); + // Order matters: +x must land before the tree goes read-only (or the scripts it ships could + // not be marked executable), and both must land before the mv (so the canonical shared cache + // is never briefly writable). + expect(chmodExecIdx).toBeLessThan(chmodReadonlyIdx); + expect(chmodReadonlyIdx).toBeLessThan(mvIdx); + }); + + it('documents the tolerated undrained-stdin race next to the early exit', () => { + // A future reader "fixing" this race would either reintroduce the transfer the probe avoids or + // re-extract over a populated cache, so the reasoning has to live in the source. + expect(s).toMatch(/race|undrained|tolerat/i); + }); + + it('single-quote-escapes the digest', () => { + // A real sha256: digest can never contain a quote -- assertValidDigest (Fix B, + // config-bundle's shared digest validator) now rejects this string before `sq()` ever sees + // it, so the escaping this test used to exercise is unreachable for a value shaped this way. + // The `sq()` helper itself is still covered structurally by every other test in this file + // (they all pass a valid DIGEST through, and `sq()` is applied unconditionally). + expect(() => buildCachePopulateScript("x'; rm -rf /; '")).toThrow(/invalid digest/i); + }); +}); + +describe('buildLeafBindScript', () => { + const s = buildLeafBindScript(DIGEST, 'leaf-1'); + it('creates the leaf dir and links the shared cache into it', () => { + expect(s).toContain('mkdir -p'); + expect(s).toContain('/workspace/leaves/leaf-1'); + expect(s).toContain('ln -sfn'); + }); + it('writes nothing outside the leaf path and the digest cache', () => { + for (const line of s.split('\n')) { + if (/^\s*(mkdir|ln|rm|mv|chmod)/.test(line)) { + expect(line).toMatch(/\$LEAF|\$DIR|leaves|\.sh-config/); + } + } + }); +}); + +// Invoked from run-leaf.ts's runPromptLeaf teardown (in the `finally`, guarded on `overlayCreated`) +// for a promoted prompt leaf, which never converges a workspace and so has no other path that would +// ever remove this per-leaf link -- see run-leaf-promoted.test.ts for the wiring-level coverage. +describe('buildConfigCleanupScript', () => { + it('removes only the per-leaf link, never the shared cache', () => { + const s = buildConfigCleanupScript('leaf-1'); + expect(s).toContain('/workspace/leaves/leaf-1/.sh-config'); + expect(s).not.toContain('/workspace/.sh-config/sha256'); + }); +}); + +function transportSpy(cacheHit: boolean) { + const calls: Array<{ command: string; stdinBytes: number }> = []; + return { + calls, + transport: { + exec: async (command: string, opts?: { stdin?: Buffer }) => { + calls.push({ command, stdinBytes: opts?.stdin?.length ?? 0 }); + const probing = command.includes('CACHE_PROBE'); + return { + stdout: Buffer.from(probing && cacheHit ? 'hit' : 'miss'), + exitCode: 0, + truncated: false, + }; + }, + close: async () => {}, + }, + }; +} + +describe('overlayConfig', () => { + const tarGz = gzipSync(Buffer.from('fake-tar')); + + it('returns the sandbox skills and memory dirs for the prompt notes', async () => { + const { transport } = transportSpy(true); + expect(await overlayConfig(transport, DIGEST, 'leaf-1', tarGz)).toEqual({ + skillsDir: '/workspace/leaves/leaf-1/.sh-config/skills', + memoryDir: '/workspace/leaves/leaf-1/.sh-config/memory', + }); + }); + + it('pushes no bytes when the digest is already cached (the fan-out win)', async () => { + const { transport, calls } = transportSpy(true); + await overlayConfig(transport, DIGEST, 'leaf-1', tarGz); + expect(calls.every((c) => c.stdinBytes === 0)).toBe(true); + expect(calls).toHaveLength(2); // probe + bind + }); + + it('pushes the bundle exactly once when the cache is cold', async () => { + const { transport, calls } = transportSpy(false); + await overlayConfig(transport, DIGEST, 'leaf-1', tarGz); + expect(calls.filter((c) => c.stdinBytes > 0)).toHaveLength(1); + expect(calls).toHaveLength(3); // probe + populate + bind + }); + + it('throws on a non-zero exit rather than continuing unconfigured', async () => { + const transport = { + exec: async () => ({ stdout: Buffer.from(''), exitCode: 1, truncated: false }), + close: async () => {}, + }; + await expect(overlayConfig(transport, DIGEST, 'leaf-1', tarGz)).rejects.toThrow(/overlay/); + }); + + it('throws when the overlay output is truncated', async () => { + const transport = { + exec: async () => ({ stdout: Buffer.from(''), exitCode: null, truncated: true }), + close: async () => {}, + }; + await expect(overlayConfig(transport, DIGEST, 'leaf-1', tarGz)).rejects.toThrow(/output cap/); + }); +}); diff --git a/harness/test/config-resolver.test.ts b/harness/test/config-resolver.test.ts new file mode 100644 index 0000000..e9ca27a --- /dev/null +++ b/harness/test/config-resolver.test.ts @@ -0,0 +1,190 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { mkdtempSync, rmSync, existsSync, readFileSync, readdirSync } from 'node:fs'; +import { join, resolve } from 'node:path'; +import { tmpdir } from 'node:os'; +import { canonicalTar, contentDigest } from '@sh/config-bundle'; +import { DefaultResourceLoader, getAgentDir } from '@earendil-works/pi-coding-agent'; +import { unpackBundle, buildLoaderOptions, promotedLoaderOptions } from '../src/config-resolver.js'; + +let base: string; + +const bundle = () => { + const entries = [ + { + path: 'skills/keeper/SKILL.md', + content: Buffer.from('---\nname: keeper\ndescription: d\n---\nbody'), + }, + { path: 'skills/keeper/references/g.md', content: Buffer.from('guide') }, + { path: 'prompts/go.md', content: Buffer.from('do it') }, + { path: 'context/agents/0-CLAUDE.md', content: Buffer.from('# promoted project rules') }, + { path: 'context/MEMORY.md', content: Buffer.from('- [A](alpha.md)') }, + { path: 'memory/alpha.md', content: Buffer.from('fact') }, + // append-2 / append-10, not append-0 / append-1: canonicalTar always sorts entries + // byte-wise before writing (tar.ts), so untar hands unpackBundle entries in lexicographic + // path order regardless of this array's order -- reversing the source order alone changes + // nothing observable. What DOES distinguish "sorted" from "not sorted" is a pair whose + // lexicographic order differs from its numeric order: byte-wise, 'append-10.md' < 'append-2.md' + // ('1' < '2'), so a plain pass-through (or a non-numeric-aware sort) yields ten-before-two, + // while the numeric-aware comparator in unpackBundle correctly yields two-before-ten. This + // fixture fails if that comparator is removed, reversed, or degraded to lexicographic. + { path: 'prompt/append-10.md', content: Buffer.from('note ten\nsecond line') }, + { path: 'prompt/append-2.md', content: Buffer.from('note two\nsecond line') }, + ]; + return { tar: canonicalTar(entries), digest: contentDigest(entries) }; +}; + +beforeEach(() => { + base = mkdtempSync(join(tmpdir(), 'sh-config-')); +}); +afterEach(() => { + rmSync(base, { recursive: true, force: true }); +}); + +describe('unpackBundle', () => { + it('materializes skills and prompts as real files (pi reads skill bodies from disk)', () => { + const { tar, digest } = bundle(); + const p = unpackBundle(tar, digest, base); + expect(existsSync(join(p.skillsDir, 'keeper', 'SKILL.md'))).toBe(true); + expect(readFileSync(join(p.skillsDir, 'keeper', 'references', 'g.md'), 'utf8')).toBe('guide'); + expect(existsSync(join(p.promptsDir, 'go.md'))).toBe(true); + }); + + it('returns context inline, ordered, without writing it to disk', () => { + const { tar, digest } = bundle(); + const p = unpackBundle(tar, digest, base); + expect(p.context.map((c) => c.path)).toEqual([ + 'context/MEMORY.md', + 'context/agents/0-CLAUDE.md', + ]); + expect(p.context.find((c) => c.path.endsWith('0-CLAUDE.md'))!.content).toContain('promoted'); + }); + + it('returns prompt fragments in numeric append-N order, not lexicographic', () => { + // 'append-10.md' sorts before 'append-2.md' lexicographically (and that is the order + // canonicalTar itself writes them in), so this only passes if unpackBundle's comparator is + // genuinely numeric-aware -- a no-op sort or a plain string sort both produce ten-before-two. + const { tar, digest } = bundle(); + expect(unpackBundle(tar, digest, base).promptFragments[0]).toContain('note two'); + expect(unpackBundle(tar, digest, base).promptFragments[1]).toContain('note ten'); + }); + + it('is idempotent and leaves no temp directory behind', () => { + const { tar, digest } = bundle(); + unpackBundle(tar, digest, base); + unpackBundle(tar, digest, base); + expect(readdirSync(base).filter((n) => n.startsWith('.tmp-'))).toEqual([]); + }); + + it('names the directory after the digest so the cache can never be stale', () => { + const { tar, digest } = bundle(); + expect(unpackBundle(tar, digest, base).root).toBe(join(base, digest.replace(':', '-'))); + }); + + it('rejects an entry path that escapes the unpack root', () => { + const evil = canonicalTar([{ path: '../escape.md', content: Buffer.from('x') }]); + expect(() => unpackBundle(evil, 'sha256:' + '0'.repeat(64), base)).toThrow(/escapes/); + }); + + it('rejects an absolutely-rooted entry path', () => { + // `resolve(staging, '/etc/passwd')` discards the base entirely, landing outside staging. The + // brief names this case explicitly, so it gets its own test rather than relying on the + // traversal case above. + const evil = canonicalTar([{ path: '/etc/passwd', content: Buffer.from('x') }]); + expect(() => unpackBundle(evil, 'sha256:' + '1'.repeat(64), base)).toThrow(/escapes/); + }); + + it('removes the staging directory when unpacking fails', () => { + // Cleanup correctness was previously verified only by reading the code, so a regression in the + // rmSync would have gone undetected and leaked a staging dir per failed unpack in a pod's /tmp. + const evil = canonicalTar([{ path: '../escape.md', content: Buffer.from('x') }]); + expect(() => unpackBundle(evil, 'sha256:' + '2'.repeat(64), base)).toThrow(/escapes/); + expect(readdirSync(base).filter((n) => n.startsWith('.tmp-'))).toEqual([]); + }); + + it('rejects a malformed digest before touching the filesystem at all (Fix B: defense-in-depth)', () => { + // digestDirName joins whatever it's given onto baseDir with no further checking; without + // assertValidDigest running first, a digest shaped 'sha256:../../../evil' would turn into a + // directory name that -- once digestDirName's ':' -> '-' substitution is undone by eye -- + // walks straight out of `base`. This asserts base itself never gains a directory at all + // (traversal or not), which is only true if the digest is rejected before `mkdirSync(baseDir)` + // and the staging mkdtemp ever run. + const { tar } = bundle(); + const evilDigest = 'sha256:../../../evil'; + expect(() => unpackBundle(tar, evilDigest, base)).toThrow(/invalid digest/i); + // base was freshly created empty by beforeEach and nothing in this test wrote to it before the + // call above, so any entry here would have to have come from unpackBundle. + expect(readdirSync(base)).toEqual([]); + }); +}); + +describe('buildLoaderOptions', () => { + const { tar, digest } = bundle(); + + it('points pi at the bundle and suppresses discovery', () => { + const o = buildLoaderOptions(unpackBundle(tar, digest, base)); + expect(o.additionalSkillPaths).toHaveLength(1); + expect(o.noContextFiles).toBe(true); + expect(o.noSkills).toBe(true); + expect(o.noPromptTemplates).toBe(true); + }); + + it('supplies context through agentsFilesOverride, ignoring the base entirely', () => { + const o = buildLoaderOptions(unpackBundle(tar, digest, base)); + const out = o.agentsFilesOverride({ + agentsFiles: [{ path: '/leak/CLAUDE.md', content: 'leak' }], + }); + expect(out.agentsFiles.map((f) => f.path)).not.toContain('/leak/CLAUDE.md'); + }); +}); + +describe('promotedLoaderOptions', () => { + it('is EMPTY when no bundle is promoted — this is the unchanged-behavior guarantee', () => { + expect(Object.keys(promotedLoaderOptions(undefined))).toEqual([]); + }); +}); + +// REGRESSION TEST — do not delete. The harness CLAUDE.md leak. +// +// loadProjectContextFiles (pi-fork resource-loader.ts:62) walks ancestor directories for +// CLAUDE.md/AGENTS.md. Run inside this repository, that walk reaches OUR OWN CLAUDE.md +// ("Serverless Harness ... pnpm workspace ... DCO sign-off required"). Unsuppressed, every +// promoted session would silently inherit the harness project's instructions as if they were +// the user's. It fails as plausible-but-wrong behavior, never as an error, so nothing else +// would catch it. +// +// TWO mechanisms close it and both are deliberate: agentsFilesOverride ignores the base, and +// noContextFiles makes the base empty. The override alone is sufficient TODAY — but a +// natural-looking future edit to `(base) => ({agentsFiles: [...base.agentsFiles, ...ours]})` +// would reopen the leak, and noContextFiles keeps it shut even then. +describe('harness CLAUDE.md leak', () => { + it('never exposes the harness project CLAUDE.md to a promoted session', async () => { + const { tar, digest } = bundle(); + const promoted = unpackBundle(tar, digest, base); + const repoRoot = resolve(__dirname, '..', '..'); + expect(readFileSync(join(repoRoot, 'CLAUDE.md'), 'utf8')).toContain('Serverless Harness'); + + const loader = new DefaultResourceLoader({ + cwd: repoRoot, + agentDir: getAgentDir(), + ...buildLoaderOptions(promoted), + }); + await loader.reload(); + + const files = loader.getAgentsFiles().agentsFiles; + expect(files.some((f) => f.content.includes('Serverless Harness'))).toBe(false); + expect(files.some((f) => f.content.includes('promoted project rules'))).toBe(true); + }); + + it('loads exactly the bundle skills and nothing discovered', async () => { + const { tar, digest } = bundle(); + const loader = new DefaultResourceLoader({ + cwd: resolve(__dirname, '..', '..'), + agentDir: getAgentDir(), + ...buildLoaderOptions(unpackBundle(tar, digest, base)), + }); + await loader.reload(); + // Pins the semantics found at resource-loader.ts:405-407: noSkills still honours + // additionalSkillPaths, so this yields the bundle's skills only. + expect(loader.getSkills().skills.map((s) => s.name)).toEqual(['keeper']); + }); +}); diff --git a/harness/test/config-store.test.ts b/harness/test/config-store.test.ts new file mode 100644 index 0000000..1d55007 --- /dev/null +++ b/harness/test/config-store.test.ts @@ -0,0 +1,161 @@ +import { describe, it, expect } from 'vitest'; +import { buildBundle, canonicalTar, contentDigest, untar } from '@sh/config-bundle'; +import { + bundleKey, + putBundle, + getBundle, + BundleNotFoundError, + BundleDigestMismatchError, + DEFAULT_BUNDLE_TTL_SECONDS, + type BundleRedisLike, +} from '../src/config-store.js'; + +/** In-memory fake, mirroring the RedisLike pattern in leaf-result-store.ts. */ +function fakeRedis(): BundleRedisLike & { + store: Map; + sets: number; + expires: Array<{ key: string; seconds: number }>; +} { + const store = new Map(); + const expires: Array<{ key: string; seconds: number }> = []; + return { + store, + sets: 0, + expires, + async set(key, value) { + this.sets++; + store.set(key, value); + return 'OK'; + }, + async get(key) { + return store.get(key) ?? null; + }, + async exists(key) { + return store.has(key) ? 1 : 0; + }, + async expire(key, seconds) { + expires.push({ key, seconds }); + return 'OK'; + }, + }; +} + +const tar = canonicalTar([ + { path: 'skills/x/SKILL.md', content: Buffer.from('---\nname: x\n---\nb') }, +]); +const digest = contentDigest(untar(tar)); // computed from tar: verification ensures round-trip fidelity + +describe('bundleKey', () => { + it('namespaces by digest', () => { + expect(bundleKey('sha256:abc')).toBe('config:bundle:sha256:abc'); + }); +}); + +describe('putBundle', () => { + it('uploads when absent and reports uploaded: true', async () => { + const r = fakeRedis(); + expect(await putBundle(r, digest, tar)).toEqual({ uploaded: true }); + expect(r.store.has(bundleKey(digest))).toBe(true); + }); + + it('is a no-op when the digest is already present (re-promotion is free)', async () => { + const r = fakeRedis(); + await putBundle(r, digest, tar); + const before = r.sets; + expect(await putBundle(r, digest, tar)).toEqual({ uploaded: false }); + expect(r.sets).toBe(before); + }); + + it('sets a TTL', async () => { + const seen: Array<{ EX?: number } | undefined> = []; + const r = fakeRedis(); + const spy: BundleRedisLike = { + ...r, + set: async (k, v, o) => (seen.push(o), r.set(k, v, o)), + }; + await putBundle(spy, digest, tar); + expect(seen[0]).toEqual({ EX: DEFAULT_BUNDLE_TTL_SECONDS }); + }); + + it('throws BundleDigestMismatchError when digest does not match tar', async () => { + const r = fakeRedis(); + const badDigest = 'sha256:' + '0'.repeat(64); + await expect(putBundle(r, badDigest, tar)).rejects.toThrow(BundleDigestMismatchError); + expect(r.sets).toBe(0); + expect(r.store.size).toBe(0); + }); + + it('refreshes TTL on skip (re-promotion does not age out active bundles)', async () => { + const r = fakeRedis(); + await putBundle(r, digest, tar); + expect(r.sets).toBe(1); + const before = r.expires.length; + expect(await putBundle(r, digest, tar)).toEqual({ uploaded: false }); + expect(r.sets).toBe(1); + expect(r.expires.length).toBe(before + 1); + expect(r.expires[before]).toEqual({ + key: bundleKey(digest), + seconds: DEFAULT_BUNDLE_TTL_SECONDS, + }); + }); +}); + +describe('getBundle', () => { + it('round-trips the exact bytes', async () => { + const r = fakeRedis(); + await putBundle(r, digest, tar); + expect((await getBundle(r, digest)).equals(tar)).toBe(true); + }); + + it('throws BundleNotFoundError with the digest in the message', async () => { + await expect(getBundle(fakeRedis(), digest)).rejects.toThrow(BundleNotFoundError); + await expect(getBundle(fakeRedis(), digest)).rejects.toThrow(digest); + }); + + it('throws BundleDigestMismatchError on corrupted stored bytes', async () => { + const r = fakeRedis(); + const real = buildBundle({ + roots: {}, + entry: 'e', + mode: 'unattended', + sandboxImage: 'i', + versions: { pi: '1', harness: '1' }, + }); + await putBundle(r, real.digest, real.tar); + // corrupt the stored payload + r.store.set(bundleKey(real.digest), Buffer.from('not a tar').toString('base64')); + await expect(getBundle(r, real.digest)).rejects.toThrow(BundleDigestMismatchError); + }); + + it('accepts a bundle whose recomputed content digest matches', async () => { + const r = fakeRedis(); + const real = buildBundle({ + roots: {}, + entry: 'e', + mode: 'unattended', + sandboxImage: 'i', + versions: { pi: '1', harness: '1' }, + }); + await putBundle(r, real.digest, real.tar); + expect((await getBundle(r, real.digest)).equals(real.tar)).toBe(true); + }); + + it('rejects valid bytes stored under the wrong digest (digest comparison path)', async () => { + const r = fakeRedis(); + // Create two distinct valid tars + const tarA = canonicalTar([ + { path: 'skills/a/SKILL.md', content: Buffer.from('---\nname: a\n---\naaa') }, + ]); + const tarB = canonicalTar([ + { path: 'skills/b/SKILL.md', content: Buffer.from('---\nname: b\n---\nbbb') }, + ]); + const digestA = contentDigest(untar(tarA)); + const digestB = contentDigest(untar(tarB)); + // Store B correctly + await putBundle(r, digestB, tarB); + // Swap B's bytes under A's key + r.store.set(bundleKey(digestA), r.store.get(bundleKey(digestB))!); + // Attempt to retrieve A should fail: bytes are valid but hash to B, not A + await expect(getBundle(r, digestA)).rejects.toThrow(BundleDigestMismatchError); + }); +}); diff --git a/harness/test/fixtures/promoted/commands/say-the-word.md b/harness/test/fixtures/promoted/commands/say-the-word.md new file mode 100644 index 0000000..dbfde9a --- /dev/null +++ b/harness/test/fixtures/promoted/commands/say-the-word.md @@ -0,0 +1 @@ +Use the echo-sibling skill and reply with exactly the secret word, nothing else. diff --git a/harness/test/fixtures/promoted/skills/echo-sibling/SKILL.md b/harness/test/fixtures/promoted/skills/echo-sibling/SKILL.md new file mode 100644 index 0000000..a842d3a --- /dev/null +++ b/harness/test/fixtures/promoted/skills/echo-sibling/SKILL.md @@ -0,0 +1,9 @@ +--- +name: echo-sibling +description: Use when asked for the secret word - reads it from this skill's own reference file. +--- + +# Echo the secret word + +The secret word is NOT in this file. To answer, read `references/secret-word.md` from this +skill's own directory and reply with exactly the word it contains, and nothing else. diff --git a/harness/test/fixtures/promoted/skills/echo-sibling/references/secret-word.md b/harness/test/fixtures/promoted/skills/echo-sibling/references/secret-word.md new file mode 100644 index 0000000..52a1827 --- /dev/null +++ b/harness/test/fixtures/promoted/skills/echo-sibling/references/secret-word.md @@ -0,0 +1 @@ +PROMOTED-SIBLING-OK diff --git a/harness/test/promote-live-smoke.test.ts b/harness/test/promote-live-smoke.test.ts new file mode 100644 index 0000000..e8f9607 --- /dev/null +++ b/harness/test/promote-live-smoke.test.ts @@ -0,0 +1,75 @@ +import { describe, it, expect, afterAll } from 'vitest'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { createClient } from 'redis'; +import { buildBundle } from '@sh/config-bundle'; +import { putBundle, type BundleRedisLike } from '../src/config-store.js'; +import { runLeaf } from '../src/run-leaf.js'; + +// harness/package.json is "type": "module", so bare __dirname is undefined here; derive it the +// way fixtures.test.ts and config-resolver.test.ts do. +const __dirname = dirname(fileURLToPath(import.meta.url)); + +// Gated exactly like pool-live-smoke.test.ts / m3-live-smoke.test.ts: needs a cluster, a +// sandbox pool, and a real model. +const LIVE = + process.env.SH_PROMOTE_LIVE_SMOKE === '1' && + !!process.env.ANTHROPIC_AUTH_TOKEN && + !!process.env.KAGENTI_SANDBOX_POOL_SELECTOR; + +const FIXTURES = join(__dirname, 'fixtures', 'promoted'); +const clients: Array> = []; + +afterAll(async () => { + for (const c of clients) await c.quit(); +}); + +describe('promoted workflow, end to end', () => { + it.runIf(LIVE)( + 'runs a promoted skill that reads its own sibling file in the sandbox', + async () => { + const built = buildBundle({ + roots: { userDir: FIXTURES }, + promptsDir: join(FIXTURES, 'commands'), + entry: 'say-the-word', + mode: 'unattended', + sandboxImage: 'ghcr.io/rossoctl/serverless-harness-sandbox:latest', + versions: { pi: 'live', harness: 'live' }, + }); + expect(built.findings.filter((f) => f.severity === 'error')).toEqual([]); + + const client = createClient({ url: process.env.REDIS_URL }); + clients.push(client); + await client.connect(); + await putBundle(client as unknown as BundleRedisLike, built.digest, built.tar); + + const result = await runLeaf({ + sessionId: `promote-smoke/${Date.now()}`, + item: { item_id: 'i1', file: 'f', pattern: 'p' }, + kind: 'prompt', + prompt: 'Reply with exactly the secret word and nothing else.', + configRef: built.digest, + } as never); + + // THE assertion that matters: the model could only produce this word by resolving a + // relative path from a promoted skill's instructions against the absolute skills-directory + // path the leaf injects into the prompt (run-leaf.ts) for that skill's own subdirectory in + // the sandbox. It proves path translation (spec §4.5) end to end. + expect(result.status).toBe('responded'); + expect((result as { text: string }).text).toContain('PROMOTED-SIBLING-OK'); + }, + 180_000, + ); + + it.runIf(LIVE)('fails loudly on an unknown digest instead of running unconfigured', async () => { + const result = await runLeaf({ + sessionId: `promote-smoke-missing/${Date.now()}`, + item: { item_id: 'i1', file: 'f', pattern: 'p' }, + kind: 'prompt', + prompt: 'anything', + configRef: 'sha256:' + 'f'.repeat(64), + } as never); + expect(result.status).toBe('failed'); + expect((result as { message: string }).message).toContain('not found'); + }); +}); diff --git a/harness/test/promote.test.ts b/harness/test/promote.test.ts new file mode 100644 index 0000000..71ec3de --- /dev/null +++ b/harness/test/promote.test.ts @@ -0,0 +1,329 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { + parsePromoteArgs, + projectMemoryDir, + projectRoot, + collectContextFiles, + promoteInputs, + resolveProjectDir, + LOCKFILE_OUT, + readInventory, + resolveInventoryPath, + inventoryCandidates, + inventoryFileName, + INVENTORY_SUBDIR, +} from '../src/promote.js'; + +let root: string; +const write = (rel: string, body: string) => { + const p = join(root, rel); + mkdirSync(join(p, '..'), { recursive: true }); + writeFileSync(p, body); +}; + +beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'promote-')); +}); +afterEach(() => { + rmSync(root, { recursive: true, force: true }); +}); + +describe('parsePromoteArgs', () => { + it('requires an entry', () => { + expect(() => parsePromoteArgs([])).toThrow(/--entry/); + }); + + it('defaults mode to unattended and the image to the one setup-k8s.sh deploys', () => { + // Must match deploy/knative/setup-k8s.sh:30 and the inventory filename created in Task 14, + // or readInventory() finds nothing and the binary check degrades to a warning forever. + const a = parsePromoteArgs(['--entry', 'go']); + expect(a).toEqual({ + entry: 'go', + mode: 'unattended', + sandboxImage: 'ghcr.io/rossoctl/serverless-harness-sandbox:latest', + deny: [], + dryRun: false, + }); + }); + + it('accepts --mode attended, repeated --deny, and --dry-run', () => { + const a = parsePromoteArgs([ + '--entry', + 'go', + '--mode', + 'attended', + '--deny', + 'a', + '--deny', + 'b', + '--dry-run', + ]); + expect(a.mode).toBe('attended'); + expect(a.deny).toEqual(['a', 'b']); + expect(a.dryRun).toBe(true); + }); + + it('rejects an unknown mode rather than silently defaulting', () => { + expect(() => parsePromoteArgs(['--entry', 'go', '--mode', 'sideways'])).toThrow(/mode/); + }); + + it('parses --project', () => { + const a = parsePromoteArgs(['--entry', 'go', '--project', '/some/dir']); + expect(a.project).toBe('/some/dir'); + }); + + it('leaves project undefined when --project is not given', () => { + const a = parsePromoteArgs(['--entry', 'go']); + expect(a.project).toBeUndefined(); + }); + + it('rejects an empty --project value', () => { + expect(() => parsePromoteArgs(['--entry', 'go', '--project', ''])).toThrow(/--project/); + }); +}); + +describe('resolveProjectDir', () => { + it('resolves a relative --project to an absolute path against process cwd semantics', () => { + // resolve() anchors relative paths at process.cwd(); pass a distinct fallback cwd to prove + // the returned path is NOT that fallback -- it comes from resolving `args.project`. + const resolved = resolveProjectDir({ project: 'some/relative/dir' }, '/fallback/cwd'); + expect(resolved).not.toBe('/fallback/cwd'); + expect(resolved.endsWith('/some/relative/dir')).toBe(true); + expect(resolved.startsWith('/')).toBe(true); + }); + + it('resolves an absolute --project unchanged', () => { + expect(resolveProjectDir({ project: '/abs/dir' }, '/fallback/cwd')).toBe('/abs/dir'); + }); + + it('falls back to cwd when --project is not given -- this is the bug this fix closes', () => { + // Before this fix, promote-cli.ts always used process.cwd(), which under + // `cd harness && pnpm promote` is the harness package directory, not the caller's project. + expect(resolveProjectDir({}, '/fallback/cwd')).toBe('/fallback/cwd'); + }); + + it('redirects the memory lookup: projectMemoryDir differs for the resolved project vs. the harness subdirectory', () => { + // This is the actual defect from C1: `cd harness && pnpm promote` (no --project) slugs to + // ".../serverless-harness/harness", which Claude Code never created, so memory is always + // empty. With --project pointed at the repo root, the slug matches the real project dir. + const repoRoot = '/Users/p/Projects/aiplatform/serverless-harness'; + const harnessSubdir = join(repoRoot, 'harness'); + const withoutProjectFlag = resolveProjectDir({}, harnessSubdir); + const withProjectFlag = resolveProjectDir({ project: repoRoot }, harnessSubdir); + expect(projectMemoryDir(withoutProjectFlag, '/Users/p')).not.toBe( + projectMemoryDir(withProjectFlag, '/Users/p'), + ); + expect(projectMemoryDir(withProjectFlag, '/Users/p')).toBe( + '/Users/p/.claude/projects/-Users-p-Projects-aiplatform-serverless-harness/memory', + ); + }); +}); + +describe('projectMemoryDir', () => { + it('mirrors Claude Code path-slug layout', () => { + expect(projectMemoryDir('/Users/p/Projects/x', '/Users/p')).toBe( + '/Users/p/.claude/projects/-Users-p-Projects-x/memory', + ); + }); + + it('preserves hyphens in the final path segment', () => { + // Slug collision is inherited from Claude Code; we match upstream behavior + // so we find the directory it already created, even with hyphens present. + expect(projectMemoryDir('/Users/p/my-project', '/Users/p')).toBe( + '/Users/p/.claude/projects/-Users-p-my-project/memory', + ); + }); +}); + +describe('projectRoot', () => { + it('bounds the walk at .git when one exists', () => { + write('.git/config', 'dummy'); + write('CLAUDE.md', 'root'); + write('sub/CLAUDE.md', 'sub'); + const root_path = projectRoot(join(root, 'sub')); + expect(root_path).toBe(root); + }); + + it('returns cwd when no .git is found', () => { + write('CLAUDE.md', 'no git'); + const root_path = projectRoot(root); + expect(root_path).toBe(root); + }); + + // In a linked worktree `.git` is a FILE holding a `gitdir:` pointer, not a directory. This repo + // works out of worktrees constantly, so a directory-only boundary test would walk straight past + // the root in the checkout where it matters most -- sweeping ancestor CLAUDE.md files into a + // bundle bound for a shared store. Verified against 16 live worktrees, all `.git`-as-file. + it('bounds the walk when .git is a file, as in a linked worktree', () => { + write('.git', 'gitdir: /elsewhere/.git/worktrees/wt\n'); + write('CLAUDE.md', 'worktree root'); + write('sub/deep/CLAUDE.md', 'inner'); + expect(projectRoot(join(root, 'sub', 'deep'))).toBe(root); + }); + + it('terminates on the filesystem root, and on relative and nonexistent paths', () => { + // A missing termination check here hangs the CLI rather than failing it. + expect(projectRoot('/')).toBe('/'); + expect(projectRoot('relative/not/real')).toBe('relative/not/real'); + expect(projectRoot('/definitely/does/not/exist')).toBe('/definitely/does/not/exist'); + }); +}); + +describe('collectContextFiles', () => { + it('collects the CLAUDE.md chain from project root to cwd, outermost-first', () => { + write('.git/config', 'dummy'); + write('CLAUDE.md', '# outer'); + write('sub/CLAUDE.md', '# inner'); + const files = collectContextFiles(join(root, 'sub')); + expect(files.map((f) => f.content)).toEqual(['# outer', '# inner']); + }); + + it('does not collect files above the .git boundary', () => { + // Create a repo with .git + write('.git/config', 'dummy'); + write('CLAUDE.md', '# in-repo'); + const cwd = join(root, 'sub'); + write('sub/CLAUDE.md', '# inner'); + // Create a file above the repo that would be collected if .git did not bound it + // mkdtempSync creates it atomically with 0700; a Math.random() name is predictable. + const above = mkdtempSync(join(tmpdir(), 'promote-above-')); + try { + writeFileSync(join(above, 'CLAUDE.md'), '# above-root'); + // Even if our cwd is moved above root, projectRoot finds the .git and bounds there + const files = collectContextFiles(cwd); + const contents = files.map((f) => f.content); + expect(contents).toEqual(['# in-repo', '# inner']); + expect(contents).not.toContain('# above-root'); + } finally { + rmSync(above, { recursive: true, force: true }); + } + }); + + it('accepts AGENTS.md as an alternative and returns [] when there is none', () => { + write('.git/config', 'dummy'); + write('AGENTS.md', '# agents'); + expect(collectContextFiles(root)[0]!.content).toBe('# agents'); + const empty = mkdtempSync(join(tmpdir(), 'promote-empty-')); + try { + expect(collectContextFiles(empty)).toEqual([]); + } finally { + rmSync(empty, { recursive: true, force: true }); + } + }); +}); + +describe('promoteInputs', () => { + it('wires user, project, plugin, memory and prompt roots from the standard layout', () => { + const home = join(root, 'home'); + const cwd = join(root, 'proj'); + mkdirSync(cwd, { recursive: true }); + const input = promoteInputs({ + cwd, + home, + args: parsePromoteArgs(['--entry', 'go', '--deny', 'private-thing']), + inventory: ['gh'], + versions: { pi: '1', harness: '1' }, + }); + expect(input.roots.userDir).toBe(join(home, '.claude')); + expect(input.roots.projectDir).toBe(join(cwd, '.claude')); + expect(input.roots.pluginDirs).toEqual([join(home, '.claude', 'plugins')]); + expect(input.promptsDir).toBe(join(home, '.claude', 'commands')); + expect(input.memoryDir).toBe(projectMemoryDir(cwd, home)); + expect(input.userDenyList).toEqual(['private-thing']); + expect(input.entry).toBe('go'); + expect(input.inventory).toEqual(['gh']); + }); +}); + +describe('LOCKFILE_OUT', () => { + it('is committed inside the project .claude directory', () => { + expect(LOCKFILE_OUT).toBe('.claude/promoted.lock.json'); + }); +}); + +describe('readInventory', () => { + const IMAGE = 'ghcr.io/rossoctl/serverless-harness-sandbox:latest'; + const FILE = 'ghcr.io_rossoctl_serverless-harness-sandbox_latest.json'; + + it('derives the filename readInventory/promote-cli agree on', () => { + // Off by one character here and preflight degrades to inventory_unavailable forever. + expect(inventoryFileName(IMAGE)).toBe(FILE); + }); + + // The regression that matters: the inventory is a HARNESS-SHIPPED asset. Resolving it against + // the invocation cwd made it unreachable for every real caller, because `sh promote` runs from + // the user's own project. Measured before the fix: 29 findings from the repo root, 0 one + // directory deeper -- a check that silently stopped checking. + it('finds the shipped inventory relative to the module, not the cwd', () => { + const moduleDir = join(root, 'pkg', 'src'); + mkdirSync(moduleDir, { recursive: true }); + write(join('pkg', INVENTORY_SUBDIR, FILE), JSON.stringify({ image: IMAGE, binaries: ['tar'] })); + // cwd is somewhere entirely unrelated, as it is in real use. + const unrelated = mkdtempSync(join(tmpdir(), 'user-project-')); + try { + expect(readInventory(IMAGE, unrelated, moduleDir)).toEqual(['tar']); + } finally { + rmSync(unrelated, { recursive: true, force: true }); + } + }); + + it('still honours a cwd-local inventory as a fallback override', () => { + const moduleDir = mkdtempSync(join(tmpdir(), 'no-inventory-')); + write(join(INVENTORY_SUBDIR, FILE), JSON.stringify({ image: IMAGE, binaries: ['flock'] })); + try { + expect(readInventory(IMAGE, root, moduleDir)).toEqual(['flock']); + } finally { + rmSync(moduleDir, { recursive: true, force: true }); + } + }); + + it('returns undefined when no inventory exists anywhere', () => { + const moduleDir = mkdtempSync(join(tmpdir(), 'bare-')); + try { + expect(readInventory(IMAGE, root, moduleDir)).toBeUndefined(); + } finally { + rmSync(moduleDir, { recursive: true, force: true }); + } + }); + + it('prefers the module-relative inventory over a cwd-local one', () => { + const moduleDir = join(root, 'pkg', 'src'); + mkdirSync(moduleDir, { recursive: true }); + write( + join('pkg', INVENTORY_SUBDIR, FILE), + JSON.stringify({ image: IMAGE, binaries: ['shipped'] }), + ); + write(join(INVENTORY_SUBDIR, FILE), JSON.stringify({ image: IMAGE, binaries: ['local'] })); + expect(readInventory(IMAGE, root, moduleDir)).toEqual(['shipped']); + }); + + // The Important finding from Task 14's review: precedence was silent, so a deliberate + // cwd-local override was shadowed by the shipped copy with no way to tell which won. + it('reports which inventory path won, so silent shadowing is visible', () => { + const moduleDir = join(root, 'pkg', 'src'); + mkdirSync(moduleDir, { recursive: true }); + const shipped = join(root, 'pkg', INVENTORY_SUBDIR, FILE); + write(join('pkg', INVENTORY_SUBDIR, FILE), JSON.stringify({ image: IMAGE, binaries: ['a'] })); + write(join(INVENTORY_SUBDIR, FILE), JSON.stringify({ image: IMAGE, binaries: ['b'] })); + expect(resolveInventoryPath(IMAGE, root, moduleDir)).toBe(shipped); + }); + + it('resolveInventoryPath returns undefined when nothing exists', () => { + const moduleDir = mkdtempSync(join(tmpdir(), 'bare2-')); + try { + expect(resolveInventoryPath(IMAGE, root, moduleDir)).toBeUndefined(); + } finally { + rmSync(moduleDir, { recursive: true, force: true }); + } + }); + + it('candidate list ends at the cwd fallback and terminates', () => { + const c = inventoryCandidates(IMAGE, '/tmp/cwd', '/a/b/c'); + expect(c[c.length - 1]).toBe(join('/tmp/cwd', INVENTORY_SUBDIR, FILE)); + expect(c.length).toBeLessThan(20); + expect(c[0]).toBe(join('/a/b/c', INVENTORY_SUBDIR, FILE)); + }); +}); diff --git a/harness/test/run-leaf-promoted.test.ts b/harness/test/run-leaf-promoted.test.ts new file mode 100644 index 0000000..79a1060 --- /dev/null +++ b/harness/test/run-leaf-promoted.test.ts @@ -0,0 +1,241 @@ +import { describe, it, expect, vi } from 'vitest'; + +// `runPromptLeaf` leases a sandbox via selectPoolSandbox, which reads real process.env — with no +// KAGENTI_SANDBOX_POOL_SELECTOR and no resolvable pod config it returns null, so the `if (selected)` +// overlay branch would never run and any test aiming at it would be unreachable. Mock the module the +// way harness/test/run-leaf.test.ts:11-34 already does, and mock @sh/k8s-sandbox so KubectlTransport +// is a spy rather than a real kubectl invocation. +const { selectPoolSandboxMock, FakeSandboxPoolSaturatedError } = vi.hoisted(() => { + class FakeSandboxPoolSaturatedError extends Error { + constructor(selector: string) { + super(`sandbox pool '${selector}' saturated: all pods at capacity`); + this.name = 'SandboxPoolSaturatedError'; + } + } + return { selectPoolSandboxMock: vi.fn(), FakeSandboxPoolSaturatedError }; +}); +vi.mock('../src/select-sandbox.js', () => ({ + selectPoolSandbox: (...args: unknown[]) => selectPoolSandboxMock(...args), + SandboxPoolSaturatedError: FakeSandboxPoolSaturatedError, +})); + +const { k8sSandboxExtensionMock, kubectlTransportMock } = vi.hoisted(() => ({ + k8sSandboxExtensionMock: vi.fn(() => () => {}), + kubectlTransportMock: vi.fn(() => ({ + exec: vi.fn(async () => ({ stdout: Buffer.from(''), exitCode: 0, truncated: false })), + close: vi.fn(async () => {}), + })), +})); +vi.mock('@sh/k8s-sandbox', () => ({ + k8sSandboxExtension: (...args: unknown[]) => k8sSandboxExtensionMock(...args), + KubectlTransport: (...args: unknown[]) => kubectlTransportMock(...args), +})); + +import { runLeaf, type LeafEnvelope } from '../src/run-leaf.js'; + +const FAKE_CONFIG = { podName: 'sbx-0', namespace: 'team1' } as never; +/** A pod-shaped lease: config present, transport ABSENT — the default deployment. */ +const podLease = () => ({ + config: FAKE_CONFIG, + heartbeat: vi.fn(async () => {}), + release: vi.fn(async () => {}), +}); + +const digest = 'sha256:' + 'c'.repeat(64); + +const env = (extra: Partial = {}): LeafEnvelope => + ({ + sessionId: 'run-1/i1', + item: { item_id: 'i1', file: 'f', pattern: 'p' }, + kind: 'prompt', + prompt: 'Summarize the repo.', + ...extra, + }) as LeafEnvelope; + +const fakePromoted = { + digest, + root: '/tmp/sh-config/x', + skillsDir: '/tmp/sh-config/x/skills', + promptsDir: '/tmp/sh-config/x/prompts', + context: [], + promptFragments: [], + entries: [{ path: 'skills/k/SKILL.md', content: Buffer.from('b') }], +}; + +const okTurn = () => + vi.fn(async () => ({ + sessionId: 'run-1-i1', + response: 'text', + stopReason: 'end_turn', + usage: { input: 1, output: 1, cacheRead: 0, cacheWrite: 0, total: 2 }, + })); + +describe('configRef on a prompt leaf', () => { + it('does not resolve or overlay anything when configRef is absent', async () => { + selectPoolSandboxMock.mockReset().mockResolvedValue(null); // no lease: keep this case hermetic + const executeTurn = okTurn(); + const resolvePromotedConfig = vi.fn(); + const overlayConfig = vi.fn(); + const r = await runLeaf(env(), undefined, { + executeTurn, + resolvePromotedConfig, + overlayConfig, + }); + expect(r.status).toBe('responded'); + expect(resolvePromotedConfig).not.toHaveBeenCalled(); + expect(overlayConfig).not.toHaveBeenCalled(); + expect(executeTurn.mock.calls[0]![0].promotedConfig).toBeUndefined(); + }); + + it('resolves the bundle and passes it to executeTurn when configRef is present', async () => { + const executeTurn = okTurn(); + const resolvePromotedConfig = vi.fn(async () => fakePromoted); + selectPoolSandboxMock.mockReset().mockResolvedValue(podLease()); // pod path: no grpc transport + const overlayConfig = vi.fn(async () => ({ + skillsDir: '/workspace/leaves/run-1-i1/.sh-config/skills', + memoryDir: '/workspace/leaves/run-1-i1/.sh-config/memory', + })); + await runLeaf(env({ configRef: digest }), undefined, { + executeTurn, + resolvePromotedConfig, + overlayConfig, + // Injected so getBundleRedis() is never reached: without this the test opens a real + // redis connection, and the resolve/overlay stubs below would never be what fails. + bundleRedis: {} as never, + }); + expect(resolvePromotedConfig).toHaveBeenCalledWith(expect.anything(), digest); + // Assert on fields, NOT object identity: when a lease exists the overlay appends a prompt + // fragment and rebuilds promotedConfig, so `.toBe(fakePromoted)` would only pass in the + // no-lease case and would silently break the moment the overlay ran. + expect(executeTurn.mock.calls[0]![0].promotedConfig).toMatchObject({ + digest: fakePromoted.digest, + }); + }); + + it('fails the leaf with reason error when the bundle cannot be resolved', async () => { + selectPoolSandboxMock.mockReset().mockResolvedValue(null); // no lease: keep this case hermetic + const executeTurn = okTurn(); + const r = await runLeaf(env({ configRef: digest }), undefined, { + executeTurn, + resolvePromotedConfig: vi.fn(async () => { + throw new Error('config bundle not found: ' + digest); + }), + overlayConfig: vi.fn(), + // Injected so getBundleRedis() is never reached: without this the test opens a real + // redis connection, and the resolve/overlay stubs below would never be what fails. + bundleRedis: {} as never, + }); + expect(r.status).toBe('failed'); + expect(r.reason).toBe('error'); + expect(r.message).toContain(digest); + // It must NOT have run the turn unconfigured. + expect(executeTurn).not.toHaveBeenCalled(); + }); + + it('overlays even when the lease has NO grpc transport (the default pod deployment)', async () => { + // Regression guard for a real plan defect: guarding on `selected.transport` skipped the overlay + // on pods, so the sandbox half of the bundle silently never arrived. A fake-transport test + // cannot catch that, so this asserts the overlay is invoked at all. + kubectlTransportMock.mockClear(); + const executeTurn = okTurn(); + const overlayConfig = vi.fn(async () => ({ + skillsDir: '/workspace/leaves/run-1-i1/.sh-config/skills', + memoryDir: '/workspace/leaves/run-1-i1/.sh-config/memory', + })); + selectPoolSandboxMock.mockReset().mockResolvedValue(podLease()); // pod path: no grpc transport + await runLeaf(env({ configRef: digest }), undefined, { + executeTurn, + resolvePromotedConfig: vi.fn(async () => fakePromoted), + overlayConfig, + bundleRedis: {} as never, + }); + expect(overlayConfig).toHaveBeenCalledTimes(1); + const fragments = executeTurn.mock.calls[0]![0].promotedConfig.promptFragments; + expect(fragments.some((f: string) => f.includes('/.sh-config/skills'))).toBe(true); + // Pin the fallback itself, not just that the overlay ran: with a transport-less (pod) lease, + // the code must genuinely build a KubectlTransport for the overlay call, and — since it built + // one rather than reusing a leased one — must close it afterward. A second KubectlTransport is + // built (and closed) after the turn for the post-turn config-overlay teardown -- see the + // dedicated teardown test below for that half in isolation. + expect(kubectlTransportMock).toHaveBeenCalledTimes(2); + const builtTransport = kubectlTransportMock.mock.results[0]!.value; + expect(builtTransport.close).toHaveBeenCalledTimes(1); + }); + + it('tears down the per-leaf config-overlay link after the turn -- runPromptLeaf never converges a workspace, so nothing else would ever remove it', async () => { + // Regression guard for the config-overlay-never-cleaned-up defect: config-overlay creates + // /workspace/leaves//.sh-config on every promoted prompt leaf, but this leaf kind never + // calls convergeWorkspace/cleanupWorkspace (it has no repoUrl/ref), so without an explicit + // teardown call the per-leaf link leaks forever on a long-lived pooled pod. If the teardown + // call in run-leaf.ts's `finally` is removed, kubectlTransportMock drops back to 1 call and + // the cleanup transport's `exec` below is never invoked. + kubectlTransportMock.mockClear(); + const executeTurn = okTurn(); + const overlayConfig = vi.fn(async () => ({ + skillsDir: '/workspace/leaves/run-1-i1/.sh-config/skills', + memoryDir: '/workspace/leaves/run-1-i1/.sh-config/memory', + })); + selectPoolSandboxMock.mockReset().mockResolvedValue(podLease()); // pod path: no grpc transport + await runLeaf(env({ configRef: digest }), undefined, { + executeTurn, + resolvePromotedConfig: vi.fn(async () => fakePromoted), + overlayConfig, + bundleRedis: {} as never, + }); + expect(kubectlTransportMock).toHaveBeenCalledTimes(2); + const cleanupTransport = kubectlTransportMock.mock.results[1]!.value; + expect(cleanupTransport.exec).toHaveBeenCalledWith( + expect.stringContaining('/workspace/leaves/run-1-i1/.sh-config'), + expect.objectContaining({ timeout: 60 }), + ); + expect(cleanupTransport.close).toHaveBeenCalledTimes(1); + }); + + it('does not build or run a cleanup transport when the overlay never ran (no configRef)', async () => { + // Contrast case: nothing was ever created, so nothing should be torn down. + kubectlTransportMock.mockClear(); + selectPoolSandboxMock.mockReset().mockResolvedValue(podLease()); + await runLeaf(env(), undefined, { executeTurn: okTurn() }); + expect(kubectlTransportMock).not.toHaveBeenCalled(); + }); + + it('does not run cleanup when the overlay itself failed (nothing was created to clean up)', async () => { + kubectlTransportMock.mockClear(); + selectPoolSandboxMock.mockReset().mockResolvedValue(podLease()); + await runLeaf(env({ configRef: digest }), undefined, { + executeTurn: okTurn(), + resolvePromotedConfig: vi.fn(async () => fakePromoted), + overlayConfig: vi.fn(async () => { + throw new Error('config overlay failed (exit 1)'); + }), + bundleRedis: {} as never, + }); + // Exactly the one transport built for the failed overlay attempt itself -- no second + // (cleanup) transport, since overlayCreated never flips to true on this path. + expect(kubectlTransportMock).toHaveBeenCalledTimes(1); + }); + + it('fails the leaf when the sandbox overlay fails', async () => { + kubectlTransportMock.mockClear(); + const executeTurn = okTurn(); + selectPoolSandboxMock.mockReset().mockResolvedValue(podLease()); + const r = await runLeaf(env({ configRef: digest }), undefined, { + executeTurn, + resolvePromotedConfig: vi.fn(async () => fakePromoted), + overlayConfig: vi.fn(async () => { + throw new Error('config overlay failed (exit 1)'); + }), + // Injected so getBundleRedis() is never reached: without this the test opens a real + // redis connection, and the resolve/overlay stubs below would never be what fails. + bundleRedis: {} as never, + }); + expect(r.status).toBe('failed'); + expect(r.reason).toBe('error'); + expect(executeTurn).not.toHaveBeenCalled(); + // The fallback transport is built for a transport-less (pod) lease even on this failure path, + // and must still be closed -- it is created inside the try, so only its own local `finally` + // (not the leaf-level cleanup) is responsible for tearing it down. + expect(kubectlTransportMock).toHaveBeenCalledTimes(1); + expect(kubectlTransportMock.mock.results[0]!.value.close).toHaveBeenCalledTimes(1); + }); +}); diff --git a/harness/test/run-turn-promoted.test.ts b/harness/test/run-turn-promoted.test.ts new file mode 100644 index 0000000..bcd290e --- /dev/null +++ b/harness/test/run-turn-promoted.test.ts @@ -0,0 +1,75 @@ +import { describe, it, expect } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { canonicalTar, contentDigest } from '@sh/config-bundle'; +import { unpackBundle, promotedLoaderOptions } from '../src/config-resolver.js'; +import { resourceLoaderOptionsFor } from '../src/run-turn.js'; + +const base = () => ({ + cwd: '/w', + agentDir: '/a', + settingsManager: { marker: 'sm' }, + extensionFactories: [() => ({})], +}); + +describe('resourceLoaderOptionsFor', () => { + it('adds NOTHING when no bundle is promoted (the back-compat guarantee)', () => { + const out = resourceLoaderOptionsFor(base(), undefined); + expect(Object.keys(out).sort()).toEqual([ + 'agentDir', + 'cwd', + 'extensionFactories', + 'settingsManager', + ]); + }); + + it('passes the base fields through untouched', () => { + const b = base(); + const out = resourceLoaderOptionsFor(b, undefined); + expect(out.cwd).toBe('/w'); + expect(out.settingsManager).toBe(b.settingsManager); + expect(out.extensionFactories).toBe(b.extensionFactories); + }); + + it('adds the promoted options when a bundle is present', () => { + const dir = mkdtempSync(join(tmpdir(), 'rt-promoted-')); + try { + const entries = [ + { path: 'skills/k/SKILL.md', content: Buffer.from('---\nname: k\ndescription: d\n---\nb') }, + { path: 'context/agents/0-CLAUDE.md', content: Buffer.from('# promoted') }, + { path: 'prompt/append-0.md', content: Buffer.from('note\nline two') }, + ]; + const promoted = unpackBundle(canonicalTar(entries), contentDigest(entries), dir); + const out = resourceLoaderOptionsFor(base(), promoted); + expect(out.noContextFiles).toBe(true); + expect(out.additionalSkillPaths).toEqual([promoted.skillsDir]); + expect(out.appendSystemPrompt).toEqual(promoted.promptFragments); + // base fields survive the merge + expect(out.cwd).toBe('/w'); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('promoted option keys never collide with the base four', () => { + // resourceLoaderOptionsFor merges `{ ...base, ...promoted }`, so a promoted key colliding with + // a base key would silently win -- a promoted `cwd` would replace the harness's real working + // directory with a bundle path and nothing would error. No collision exists today; this pins + // that invariant so a future promoted option can't introduce one unnoticed. + const dir = mkdtempSync(join(tmpdir(), 'rt-promoted-collision-')); + try { + const entries = [ + { path: 'skills/k/SKILL.md', content: Buffer.from('---\nname: k\ndescription: d\n---\nb') }, + { path: 'context/agents/0-CLAUDE.md', content: Buffer.from('# promoted') }, + { path: 'prompt/append-0.md', content: Buffer.from('note\nline two') }, + ]; + const promoted = unpackBundle(canonicalTar(entries), contentDigest(entries), dir); + const promotedKeys = Object.keys(promotedLoaderOptions(promoted)); + const baseKeys = Object.keys(base()); + expect(promotedKeys.filter((k) => baseKeys.includes(k))).toEqual([]); + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/packages/config-bundle/package.json b/packages/config-bundle/package.json new file mode 100644 index 0000000..442baef --- /dev/null +++ b/packages/config-bundle/package.json @@ -0,0 +1,17 @@ +{ + "name": "@sh/config-bundle", + "type": "module", + "version": "0.0.0", + "exports": { + ".": "./src/index.ts" + }, + "scripts": { + "test": "vitest run", + "typecheck": "tsc --noEmit" + }, + "devDependencies": { + "@types/node": "^22.0.0", + "typescript": "^5.5.0", + "vitest": "^2.0.0" + } +} diff --git a/packages/config-bundle/src/build.ts b/packages/config-bundle/src/build.ts new file mode 100644 index 0000000..b8ed030 --- /dev/null +++ b/packages/config-bundle/src/build.ts @@ -0,0 +1,193 @@ +import { existsSync, readdirSync, readFileSync, statSync } from 'node:fs'; +import { basename, join } from 'node:path'; +import { classifySkills, detectBinaries } from './classify.js'; +import { buildLockfile, serializeLockfile, skillContentHash } from './lockfile.js'; +import { skillsRootNote, toolNameMappingNote } from './notes.js'; +import { + checkBinaries, + checkEntry, + checkInteraction, + checkMemoryLinks, + checkNamespacedPrompts, + checkSiblingPaths, +} from './preflight.js'; +import { resolveSkills } from './resolve.js'; +import { blockingSecrets, scanEntriesForSecrets, SecretScanError } from './secret-scan.js'; +import { canonicalTar, digestOf } from './tar.js'; +import type { BuildBundleInput, BuildResult, PreflightFinding, TarEntry } from './types.js'; + +export const LOCKFILE_PATH = 'lockfile.json'; + +/** + * The bundle's identity: a digest over the CONTENT entries only. + * + * `lockfile.json` records the digest, so including it would make the digest depend on itself. + * Both the builder and the resolver (harness/src/config-resolver.ts) call this exact function, + * so their notions of identity cannot drift. + */ +export function contentDigest(entries: TarEntry[]): string { + return digestOf(canonicalTar(entries.filter((e) => e.path !== LOCKFILE_PATH))); +} + +/** `.md` files directly in `dir`, sorted. Empty when the directory is absent. */ +function markdownFiles(dir: string | undefined): string[] { + if (!dir || !existsSync(dir)) return []; + return readdirSync(dir) + .filter((n) => n.endsWith('.md') && statSync(join(dir, n)).isFile()) + .sort(); +} + +/** + * Subdirectory names directly under `dir` that hold at least one `.md` file -- Claude Code's + * namespaced-command layout (`commands//.md`). `markdownFiles` above deliberately does + * not recurse into these (spec §2/§9, out of scope with MCP servers and subagents); this is the + * fs-scanning half of that decision, feeding `checkNamespacedPrompts` so the drop is a warning + * rather than silence. Sorted; empty when `dir` is absent. + */ +function namespacedPromptDirs(dir: string | undefined): string[] { + if (!dir || !existsSync(dir)) return []; + const out: string[] = []; + for (const name of readdirSync(dir).sort()) { + const p = join(dir, name); + let st; + try { + st = statSync(p); + } catch { + continue; + } + if (!st.isDirectory()) continue; + let children: string[]; + try { + children = readdirSync(p); + } catch { + continue; + } + const hasMd = children.some((n) => { + try { + return n.endsWith('.md') && statSync(join(p, n)).isFile(); + } catch { + return false; + } + }); + if (hasMd) out.push(name); + } + return out; +} + +export function buildBundle(input: BuildBundleInput): BuildResult { + const classification = classifySkills(resolveSkills(input.roots), { + mode: input.mode, + userDenyList: input.userDenyList, + }); + + const entries: TarEntry[] = []; + + // Skill directories travel WHOLE: skills reference siblings and carry references/ subtrees. + for (const skill of classification.travels) { + for (const rel of skill.files) { + const abs = join(skill.dir, rel); + entries.push({ + path: `skills/${skill.name}/${rel}`, + content: readFileSync(abs), + mode: rel.endsWith('.sh') ? 0o755 : 0o644, + }); + } + } + + // Inline-injected context: the CLAUDE.md chain plus the memory index. + const contextPaths: string[] = []; + (input.contextFiles ?? []).forEach((file, i) => { + const path = `context/agents/${i}-${basename(file.path)}`; + entries.push({ path, content: Buffer.from(file.content, 'utf8') }); + contextPaths.push(path); + }); + + const memoryNames = markdownFiles(input.memoryDir).filter((n) => n !== 'MEMORY.md'); + let memoryIndex: string | undefined; + if (input.memoryDir && existsSync(join(input.memoryDir, 'MEMORY.md'))) { + memoryIndex = readFileSync(join(input.memoryDir, 'MEMORY.md'), 'utf8'); + entries.push({ path: 'context/MEMORY.md', content: Buffer.from(memoryIndex, 'utf8') }); + contextPaths.push('context/MEMORY.md'); + } + + // Memory files are read on demand IN THE SANDBOX (ADR-0031 progressive disclosure), so they + // are bundle content rather than inline context. + const memoryPaths: string[] = []; + for (const name of memoryNames) { + const path = `memory/${name}`; + entries.push({ path, content: readFileSync(join(input.memoryDir!, name)) }); + memoryPaths.push(path); + } + + const promptNames: string[] = []; + for (const name of markdownFiles(input.promptsDir)) { + entries.push({ + path: `prompts/${name}`, + content: readFileSync(join(input.promptsDir!, name)), + }); + promptNames.push(name.replace(/\.md$/, '')); + } + + const fragments = [ + toolNameMappingNote(), + skillsRootNote(), + ...(input.extraPromptFragments ?? []), + ]; + fragments.forEach((text, i) => { + entries.push({ path: `prompt/append-${i}.md`, content: Buffer.from(text, 'utf8') }); + }); + + // Blocking gate: a credential reaching a shared cluster's store is not recoverable by + // re-promoting, so nothing is packed or returned when the scan is dirty (spec §4.3). + const secrets = scanEntriesForSecrets(entries); + const blocking = blockingSecrets(secrets); + if (blocking.length > 0) throw new SecretScanError(blocking); + + const digest = contentDigest(entries); + const binaries = detectBinaries(classification.travels); + const skillHashes: Record = {}; + for (const skill of classification.travels) { + skillHashes[skill.name] = skillContentHash(skill, entries); + } + + const lockfile = buildLockfile({ + digest, + mode: input.mode, + entry: input.entry, + classification, + contextPaths, + memoryPaths, + sandboxImage: input.sandboxImage, + binaries, + versions: input.versions, + skillHashes, + }); + + const findings: PreflightFinding[] = [ + // Non-blocking secret hits travel as warnings so a human still sees them in the report. + ...secrets + .filter((f) => f.severity === 'warning') + .map((f) => ({ + severity: 'warn' as const, + code: 'possible_secret', + message: `possible secret (${f.rule}) — verify before promoting`, + path: `${f.path}:${f.line}`, + })), + // Findings raised while resolving each skill's own files (e.g. a symlink that escaped its + // skill directory and was skipped -- resolve.ts's `filesUnder`). + ...classification.travels.flatMap((skill) => skill.findings ?? []), + ...checkSiblingPaths(classification.travels), + ...checkMemoryLinks(memoryIndex, memoryNames), + ...checkBinaries(binaries, input.inventory), + ...checkEntry(input.entry, promptNames), + ...checkInteraction(classification), + ...checkNamespacedPrompts(namespacedPromptDirs(input.promptsDir)), + ]; + + const tar = canonicalTar([ + ...entries, + { path: LOCKFILE_PATH, content: Buffer.from(serializeLockfile(lockfile), 'utf8') }, + ]); + + return { tar, digest, lockfile, findings, promptNames }; +} diff --git a/packages/config-bundle/src/classify.ts b/packages/config-bundle/src/classify.ts new file mode 100644 index 0000000..cbbe267 --- /dev/null +++ b/packages/config-bundle/src/classify.ts @@ -0,0 +1,186 @@ +import type { Classification, ClassifyOptions, DroppedSkill, ResolvedSkill } from './types.js'; + +/** + * Skills whose subject matter does not exist in the harness. Matched against the bare + * `name` field from SKILL.md frontmatter, not a qualified `plugin:name` form. + * CURATED BY HAND and versioned with this file — deliberately not inferred (spec §4.2). + * Heuristic detection was rejected because the signal words ("agent", "artifact") appear + * in unrelated prose, and a wrongly-dropped skill fails remotely and confusingly. + */ +export const DEFAULT_DENY_LIST: readonly string[] = [ + // Real deny-list: skills on-disk whose subject matter doesn't exist in the harness. + 'docx', + 'pdf', + 'pptx', + 'xlsx', + // Defensive entries: built-in Claude Code skills. Retained in case a future version + // ships them as SKILL.md files (so the next reader doesn't delete them as dead code). + 'artifact-design', + 'artifact-diagramming', + 'fewer-permission-prompts', + 'keybindings-help', + 'statusline-setup', + 'update-config', +]; + +/** + * Skills whose operation IS dispatching subagents. Pi has no Task tool (spec §9). + * Matched against the bare `name` field from SKILL.md frontmatter. + */ +export const SUBAGENT_DEPENDENT: readonly string[] = [ + 'dispatching-parallel-agents', + 'subagent-driven-development', +]; + +/** + * Travels fine, but its method is dialogue — degrades to invented answers unattended. + * Matched against the bare `name` field from SKILL.md frontmatter. + */ +export const INTERACTION_DEPENDENT: readonly string[] = ['brainstorming', 'receiving-code-review']; + +/** Never reported as a missing binary. Complete bash builtins and keywords. */ +const SHELL_BUILTINS = new Set([ + ':', + '.', + 'source', + 'alias', + 'bg', + 'bind', + 'break', + 'builtin', + 'caller', + 'cd', + 'command', + 'compgen', + 'complete', + 'compopt', + 'continue', + 'declare', + 'dirs', + 'disown', + 'echo', + 'enable', + 'eval', + 'exec', + 'exit', + 'export', + 'false', + 'fc', + 'fg', + 'getopts', + 'hash', + 'help', + 'history', + 'jobs', + 'kill', + 'let', + 'local', + 'logout', + 'mapfile', + 'popd', + 'printf', + 'pushd', + 'pwd', + 'read', + 'readarray', + 'readonly', + 'return', + 'select', + 'set', + 'shift', + 'shopt', + 'suspend', + 'test', + 'times', + 'trap', + 'true', + 'type', + 'typeset', + 'ulimit', + 'umask', + 'unalias', + 'unset', + 'wait', + 'if', + 'then', + 'else', + 'elif', + 'fi', + 'for', + 'while', + 'until', + 'do', + 'done', + 'case', + 'esac', + 'function', + 'in', + 'time', +]); + +export function classifySkills(skills: ResolvedSkill[], opts: ClassifyOptions): Classification { + const denied = new Set(opts.userDenyList ?? []); + const travels: ResolvedSkill[] = []; + const dropped: DroppedSkill[] = []; + + for (const skill of skills) { + if (denied.has(skill.name)) { + dropped.push({ + name: skill.name, + reason: 'user_denied', + detail: 'excluded by the local user deny-list', + }); + } else if (DEFAULT_DENY_LIST.includes(skill.name)) { + dropped.push({ + name: skill.name, + reason: 'no_harness_equivalent', + detail: 'subject matter does not exist in the harness runtime', + }); + } else if (SUBAGENT_DEPENDENT.includes(skill.name)) { + dropped.push({ + name: skill.name, + reason: 'needs_subagent', + detail: 'requires a subagent tool; Pi has none (spec §9)', + }); + } else { + travels.push(skill); + } + } + + const interactionDependent = + opts.mode === 'unattended' + ? travels.filter((s) => INTERACTION_DEPENDENT.includes(s.name)).map((s) => s.name) + : []; + + return { travels, dropped, interactionDependent }; +} + +/** + * Commands invoked inside fenced bash/sh/shell blocks, as a *detection* signal for preflight. + * Never installs anything; the sandbox image provides (spec §4.5). + * + * Parser is intentionally shell-naive: it does not understand quoting, so a `|` or `&&` + * inside a quoted string can mis-split. This is an accepted limitation. + */ +export function detectBinaries(skills: ResolvedSkill[]): string[] { + const found = new Set(); + for (const skill of skills) { + for (const block of skill.skillMd.matchAll(/```(?:bash|sh|shell)\n([\s\S]*?)```/g)) { + for (const rawLine of block[1]!.split('\n')) { + const line = rawLine.trim(); + if (!line || line.startsWith('#')) continue; + for (const segment of line.split(/&&|\|\||[|;]/)) { + let trimmed = segment.trim(); + // Skip leading environment variable assignments (e.g. FOO=bar). + trimmed = trimmed.replace(/^[A-Za-z_][A-Za-z0-9_]*=\S*\s+/, ''); + const word = trimmed.split(/\s+/)[0]; + if (!word) continue; + if (!/^[a-z][a-z0-9._-]*$/.test(word)) continue; + if (SHELL_BUILTINS.has(word)) continue; + found.add(word); + } + } + } + } + return [...found].sort(); +} diff --git a/packages/config-bundle/src/index.ts b/packages/config-bundle/src/index.ts new file mode 100644 index 0000000..645ae6e --- /dev/null +++ b/packages/config-bundle/src/index.ts @@ -0,0 +1,9 @@ +export * from './types.js'; +export * from './tar.js'; +export * from './resolve.js'; +export * from './classify.js'; +export * from './secret-scan.js'; +export * from './lockfile.js'; +export * from './notes.js'; +export * from './preflight.js'; +export * from './build.js'; diff --git a/packages/config-bundle/src/lockfile.ts b/packages/config-bundle/src/lockfile.ts new file mode 100644 index 0000000..7bd7bfb --- /dev/null +++ b/packages/config-bundle/src/lockfile.ts @@ -0,0 +1,49 @@ +import { canonicalTar, digestOf } from './tar.js'; +import { BUNDLE_FORMAT_VERSION } from './types.js'; +import type { BundleLockfile, LockfileInput, ResolvedSkill, TarEntry } from './types.js'; + +/** Content address of one skill's subtree, so a lockfile diff shows which skills changed. */ +export function skillContentHash(skill: ResolvedSkill, entries: TarEntry[]): string { + const prefix = `skills/${skill.name}/`; + return digestOf(canonicalTar(entries.filter((e) => e.path.startsWith(prefix)))); +} + +export function buildLockfile(input: LockfileInput): BundleLockfile { + return { + binaries: [...input.binaries].sort(), + builtAgainst: { harness: input.versions.harness, pi: input.versions.pi }, + context: [...input.contextPaths].sort(), + digest: input.digest, + dropped: [...input.classification.dropped].sort((a, b) => a.name.localeCompare(b.name)), + entry: input.entry, + formatVersion: BUNDLE_FORMAT_VERSION, + interactionDependent: [...input.classification.interactionDependent].sort(), + memory: [...input.memoryPaths].sort(), + mode: input.mode, + sandboxImage: input.sandboxImage, + skills: input.classification.travels + .map((s) => ({ + name: s.name, + scope: s.scope, + sourceDir: s.dir, + contentHash: input.skillHashes[s.name] ?? '', + })) + .sort((a, b) => a.name.localeCompare(b.name)), + }; +} + +/** Sorted-key JSON with a trailing newline: the lockfile is committed and must diff legibly. */ +export function serializeLockfile(lockfile: BundleLockfile): string { + const sortKeys = (value: unknown): unknown => { + if (Array.isArray(value)) return value.map(sortKeys); + if (value && typeof value === 'object') { + return Object.fromEntries( + Object.entries(value as Record) + .sort(([a], [b]) => (a < b ? -1 : 1)) + .map(([k, v]) => [k, sortKeys(v)]), + ); + } + return value; + }; + return JSON.stringify(sortKeys(lockfile), null, 2) + '\n'; +} diff --git a/packages/config-bundle/src/notes.ts b/packages/config-bundle/src/notes.ts new file mode 100644 index 0000000..eea75e0 --- /dev/null +++ b/packages/config-bundle/src/notes.ts @@ -0,0 +1,65 @@ +/** + * Injected via appendSystemPrompt instead of rewriting 149 skill files (spec D4, A1). A regex + * deciding whether "Read" is a tool reference or English will occasionally mangle a sentence, + * and the damage only shows up as a skill misbehaving remotely. + * + * MUST stay multi-line: pi-fork `resource-loader.ts:49` treats an appendSystemPrompt string as + * a FILE PATH when `existsSync(input)` is true, and only falls back to literal text otherwise. + */ +export function toolNameMappingNote(): string { + return [ + '## Tool names in skills', + '', + 'Skill instructions in this session were authored for Claude Code and may name its tools.', + 'The equivalents available here are:', + '', + '- `Bash` → `bash`', + '- `Read` → `read`', + '- `Write` → `write`', + '- `Edit` → `edit`', + '- `Grep` → `grep`', + '- `Glob` → `find`', + '- `LS` → `ls`', + '', + 'Treat a skill naming the left-hand tool as naming the right-hand one. Tools with no', + 'equivalent here (subagent dispatch, artifacts, document generation) are unavailable: if a', + 'skill requires one, say so rather than simulating it.', + ].join('\n'); +} + +/** + * Path translation (spec §4.5). A skill's SKILL.md is read in the harness pod, but its internal + * paths were written relative to where the skill lives locally — e.g. superpowers:brainstorming + * instructs a read of `skills/brainstorming/visual-companion.md`. Tool calls execute in the + * sandbox, so without this note that read resolves to nothing and the model gets a confusing + * miss rather than an actionable error. + * + * This note deliberately names no environment variable. The bundle is content-addressed and + * built once, before any leaf or sandbox exists, so it cannot bake in an absolute path — and no + * seam exists to set an env var inside the sandbox for a tool call to read (every call is an + * independent `bash -c`; see run-leaf.ts's overlay fragment for the mechanism that actually + * carries the absolute paths). The concrete "Skill files: …" / "Memory files: …" paths are + * appended per leaf, later in this same prompt — this note only tells the model to use those, + * not the working directory. + * + * MUST stay multi-line, same reason as toolNameMappingNote above. + */ +export function skillsRootNote(): string { + return [ + '## Where skill files live', + '', + 'Elsewhere in this prompt, look for lines starting "Skill files:" and "Memory files:" —', + 'those name the absolute sandbox directories for this session.', + '', + 'When a skill instructs you to read one of its own files by a path relative to the skill,', + 'resolve that path against the skill\'s own subdirectory under the "Skill files:" directory', + '— not against the current working directory.', + '', + // The bundle is built before any leaf exists, so this note cannot know whether a sandbox will + // be selected. When none is, run-leaf never appends those lines and this note would otherwise + // point at nothing — the same "instruction referencing something absent" failure it exists to + // prevent. Degrade out loud instead of letting the model invent a plausible path. + 'If no such lines appear, the promoted skill and memory files are NOT reachable in this', + 'session. Say so plainly rather than guessing a path or reporting a file as missing.', + ].join('\n'); +} diff --git a/packages/config-bundle/src/preflight.ts b/packages/config-bundle/src/preflight.ts new file mode 100644 index 0000000..ee30ddd --- /dev/null +++ b/packages/config-bundle/src/preflight.ts @@ -0,0 +1,250 @@ +import type { Classification, PreflightFinding, ResolvedSkill } from './types.js'; + +/** Backticked tokens that look like a relative file path (have a slash or a known extension). */ +function referencedPaths(skillMd: string): string[] { + const out = new Set(); + for (const m of skillMd.matchAll(/`([^`\n]+)`/g)) { + const token = m[1]!.trim(); + if (!token || /^https?:\/\//.test(token) || token.startsWith('-')) continue; + if (/\s/.test(token)) continue; + // Extension must start with a letter (kills versions like 1.2.3, IPs like 127.0.0.1) + if (!/\.[a-z][a-z0-9]{0,4}$/i.test(token)) continue; + // Reject tokens with glob/comparison operators (kills *.md, node>=18.0, etc) + if (/[*?<>=|]/.test(token)) continue; + out.add(token.replace(/^\.\//, '')); + } + return [...out]; +} + +/** Every ancestor directory prefix of a path, e.g. `a/b/c.md` -> [`a/`, `a/b/`]. Exact strings; no `startsWith` fuzz. */ +function ancestorPrefixes(p: string): string[] { + const out: string[] = []; + for (let i = p.indexOf('/'); i !== -1; i = p.indexOf('/', i + 1)) { + out.push(p.slice(0, i + 1)); + } + return out; +} + +/** + * A skill referencing one of its OWN files that the bundle omits would fail remotely as a + * confusing read miss. Two deliberate restrictions, both measured against a real `~/.claude`: + * + * 1. **Only references the skill plausibly owns are flagged** — the reference must contain a `/` + * AND its directory prefix must be a directory the skill actually ships files in. Without this + * the check fired 182 times across 28 skills, because a skill's prose names plenty of paths it + * does not ship: `main.py`, `requirements.txt`, `package.json` (files the reader will create), + * `window.open` and `sys.path` (code), bare `.md`/`.py` (from "a `.md` file"), and example + * project trees. `main.py` and `references/guide.md` are indistinguishable in shape, so no + * regex separates them — only "does the skill own this directory" does. The rule cuts 182 to 9. + * + * Ownership must be checked symmetrically against EVERY ancestor on both sides, not just each + * path's immediate parent — two failure directions, both measured: + * - ships `references/deep/x.md`, references missing `references/missing.md`: the skill's + * immediate parent is `references/deep/`, but it still owns `references/` (an ancestor of + * what it ships). + * - ships `references/guide.md`, references missing `references/deep/missing.md`: the + * reference's immediate directory `references/deep/` was never shipped, but its ancestor + * `references/` demonstrably is owned. + * Both sides are expanded into their full ancestor-prefix set and checked for exact-string + * intersection — never `startsWith` — so `references-old/` still does not satisfy a reference + * under `references/`; `references-old` and `references` are different path segments and never + * appear as the same string in either set. + * 2. **Warn, not error.** The 9 survivors are still false positives (one skill documenting + * hypothetical `references/*.md` because its subject is how to write skills), so blocking would + * refuse promotion for anyone who has it installed. Preflight blocks only on facts. + * + * Satisfaction stays deliberately loose: a reference counts if any bundled path equals it, ends + * with `/`, or the reference ends with the path — skills write paths relative to their own + * directory or to the plugin root, and both are legitimate. + */ +export function checkSiblingPaths(skills: ResolvedSkill[]): PreflightFinding[] { + const findings: PreflightFinding[] = []; + for (const skill of skills) { + const ownedDirs = new Set(skill.files.flatMap(ancestorPrefixes)); + for (const ref of referencedPaths(skill.skillMd)) { + if (skill.files.some((f) => f === ref || f.endsWith('/' + ref) || ref.endsWith(f))) continue; + if (!ref.includes('/')) continue; // a bare filename is prose, not a sibling claim + if (!ancestorPrefixes(ref).some((d) => ownedDirs.has(d))) continue; // skill does not own it + findings.push({ + severity: 'warn', + code: 'missing_sibling', + message: `skill '${skill.name}' references '${ref}', which is not in its directory`, + path: skill.dir, + }); + } + } + return findings; +} + +/** Skip non-local link targets (external URLs, protocol-relative, fragments, queries, or outside memory dir). */ +function isLocalMemoryLink(target: string): boolean { + // Has URI scheme (https:, http:, mailto:, etc) + if (/^[a-z][a-z0-9+.-]*:/.test(target)) return false; + // Protocol-relative + if (/^\/\//.test(target)) return false; + // Fragment or query-only + if (/^[#?]/.test(target)) return false; + // Contains .. path segment (outside memory directory) + if (/\.\./.test(target)) return false; + return true; +} + +/** Dangling links in MEMORY.md (both markdown `[title](file.md)` and `[[wikilink]]` forms) — usually deny-listed memory files. Warn, not error. */ +export function checkMemoryLinks( + memoryIndex: string | undefined, + includedMemoryNames: string[], +): PreflightFinding[] { + if (!memoryIndex) return []; + const slugs = new Set(includedMemoryNames.map((n) => n.replace(/\.md$/, ''))); + const findings: PreflightFinding[] = []; + + // Extract markdown links: [Title](target.md) + // Quantifiers are BOUNDED and newline-excluded to stop a quadratic blow-up: the unbounded + // /\[([^\]]+)\]\(([^)]+)\)/ rescans from every '[', measured at 2.3 s on 40 KB of '[' and + // getting quadratically worse. Promote scans third-party plugin skills, so this input is not + // necessarily the user's own. Real memory links are far inside these bounds. + for (const m of memoryIndex.matchAll(/\[([^\]\n]{1,300})\]\(([^)\n]{1,500})\)/g)) { + const target = m[2]!.trim(); + // Skip non-local targets (external URLs, relative paths outside memory, etc) + if (!isLocalMemoryLink(target)) continue; + // Strip directory prefix and .md extension + const slug = target.replace(/^.*\//, '').replace(/\.md$/, ''); + if (!slugs.has(slug)) { + findings.push({ + severity: 'warn', + code: 'dangling_memory_link', + message: `MEMORY.md links [${m[1]}](${target}), which is not included in the bundle`, + }); + } + } + + // Extract wikilinks: [[slug]] or [[slug|alias]] or [[path/slug]] + // Bounded for the same reason: the unbounded form measured 9.2 s on 80 KB of '[['. + for (const m of memoryIndex.matchAll(/\[\[([^\]\n]{1,300})\]\]/g)) { + const full = m[1]!.trim(); + // Strip |alias suffix + const withoutAlias = full.split('|')[0]!.trim(); + // Skip non-local targets (external URLs, relative paths outside memory, etc) + if (!isLocalMemoryLink(withoutAlias)) continue; + // Strip path prefix to get slug + const slug = withoutAlias.replace(/^.*\//, ''); + if (!slugs.has(slug)) { + findings.push({ + severity: 'warn', + code: 'dangling_memory_link', + message: `MEMORY.md links [[${full}]], which is not included in the bundle`, + }); + } + } + + return findings; +} + +/** + * Warns, never errors. Measured against a real ~/.claude (55 travelling skills), the fenced-block + * scan that feeds `detected` produced 44 binaries and 32 "missing" against the shipped inventory — + * roughly half not commands at all (`angular`, `django`, `express`, `fastapi`, `vue`, `prisma`, + * `branch`, `rev-parse`, even the literal placeholder `your_command`). First-word-of-a-shell-fence + * cannot distinguish a command from prose, so blocking on it refused nearly every real promotion + * for mostly bogus reasons. A genuinely missing tool still fails remotely with a legible + * `gh: not found`, which is diagnosable and re-promotable — that is an acceptable failure mode; + * refusing every promotion up front is not. + */ +export function checkBinaries( + detected: string[], + inventory: string[] | undefined, +): PreflightFinding[] { + if (!inventory) { + return detected.length === 0 + ? [] + : [ + { + severity: 'warn', + code: 'inventory_unavailable', + message: + `no sandbox inventory available; ${detected.length} detected binary/binaries ` + + `(${detected.join(', ')}) could not be verified`, + }, + ]; + } + const have = new Set(inventory); + return detected + .filter((b) => !have.has(b)) + .map((b) => ({ + severity: 'warn' as const, + code: 'missing_binary', + message: `binary '${b}' is used by a skill but is not in the sandbox image inventory`, + })); +} + +export function checkEntry(entry: string, promptNames: string[]): PreflightFinding[] { + return promptNames.includes(entry) + ? [] + : [ + { + severity: 'error', + code: 'unknown_entry', + message: + `entry prompt '${entry}' is not in the bundle ` + + `(available: ${promptNames.join(', ') || 'none'})`, + }, + ]; +} + +/** Interaction dependence is mode-sensitive, so it warns and never drops (spec D8). */ +export function checkInteraction(classification: Classification): PreflightFinding[] { + return classification.interactionDependent.map((name) => ({ + severity: 'warn' as const, + code: 'interaction_dependent', + message: + `skill '${name}' works by asking questions and waiting; unattended it will invent ` + + `the answers. Use --mode attended, or exclude it.`, + })); +} + +/** + * Claude Code exposes `commands//.md` as the namespaced slash command `/:` + * (spec §2/§9: out of scope, alongside MCP servers and subagents). `build.ts`'s `markdownFiles` + * only reads `.md` files directly in `promptsDir`, so a namespaced command is silently dropped — + * this turns that silence into a warning naming each skipped namespace, so a promoted workflow + * that invokes one fails loudly and locally instead of remotely with no signal. + */ +export function checkNamespacedPrompts(namespacedDirs: string[]): PreflightFinding[] { + return namespacedDirs.map((name) => ({ + severity: 'warn' as const, + code: 'namespaced_prompt_skipped', + message: + `prompts/${name}/ holds namespaced command(s) (Claude Code's '/${name}:*'); ` + + `only prompts directly in the prompts directory are promoted, so these are not included`, + })); +} + +export function hasErrors(findings: PreflightFinding[]): boolean { + return findings.some((f) => f.severity === 'error'); +} + +/** Human-readable report. Always states its own limits — spec §4.6 forbids implying completeness. */ +export function renderPreflight(findings: PreflightFinding[]): string { + const lines: string[] = []; + if (findings.length === 0) { + lines.push('preflight: no findings'); + } else { + for (const severity of ['error', 'warn', 'info'] as const) { + const group = findings.filter((f) => f.severity === severity); + if (group.length === 0) continue; + lines.push(`${severity} (${group.length}):`); + for (const f of group) { + lines.push(` [${f.code}] ${f.message}${f.path ? ` (${f.path})` : ''}`); + } + } + } + lines.push(''); + lines.push('Cannot be checked locally, and will only surface at run time:'); + lines.push( + ' - a binary present in the sandbox but at a different version or with different flags', + ); + lines.push(' - a tool present but lacking the credentials it needs'); + lines.push(' - network egress the sandbox denies'); + lines.push(' - a skill assuming host behavior that has no analogue and no mechanical signature'); + return lines.join('\n'); +} diff --git a/packages/config-bundle/src/resolve.ts b/packages/config-bundle/src/resolve.ts new file mode 100644 index 0000000..39b0be6 --- /dev/null +++ b/packages/config-bundle/src/resolve.ts @@ -0,0 +1,213 @@ +import { existsSync, readdirSync, readFileSync, realpathSync, statSync } from 'node:fs'; +import { isAbsolute, join, relative, sep } from 'node:path'; +import type { PreflightFinding, ResolvedSkill, SkillRoots, SkillScope } from './types.js'; + +/** + * Is `pCanonical` inside `rootCanonical` (or equal to it)? Path-segment-aware: `relative()` + * rather than a bare `startsWith`, so `/a/skillsX` is never mistaken for being inside `/a/skills`. + */ +function isWithin(rootCanonical: string, pCanonical: string): boolean { + if (rootCanonical === pCanonical) return true; + const rel = relative(rootCanonical, pCanonical); + return rel !== '' && rel !== '..' && !rel.startsWith('..' + sep) && !isAbsolute(rel); +} + +/** Parse `name:` out of YAML frontmatter. Deliberately minimal — no YAML dependency. Strips a single matched pair of surrounding double or single quotes. */ +export function readSkillFrontmatterName(skillMd: string): string | null { + if (!skillMd.startsWith('---')) return null; + const end = skillMd.indexOf('\n---', 3); + if (end === -1) return null; + const m = /^name:\s*(.+)$/m.exec(skillMd.slice(3, end)); + if (!m) return null; + let name = m[1]!.trim(); + // Strip a single matched pair of surrounding double or single quotes + if ( + (name.startsWith('"') && name.endsWith('"')) || + (name.startsWith("'") && name.endsWith("'")) + ) { + name = name.slice(1, -1); + } + return name; +} + +/** + * Every file under `dir`, relative to it, sorted. Symlinks are followed (deliberate: see + * `8be16bd` -> `3afe725`), but bounded to `dir`'s own canonical root — a directory symlink that + * resolves OUTSIDE that root (e.g. `refs/linked -> ~/Documents`) is skipped rather than walked, + * because `statSync` (not `lstatSync`) would otherwise resolve it and let an unrelated tree's + * content ride along into a bundle destined for shared Redis. The root canonical path is + * computed once, here, and reused for every entry rather than re-derived per entry. Cycles + * within the bound are broken by tracking canonical paths. + */ +function filesUnder(dir: string, findings: PreflightFinding[]): string[] { + let rootCanonical: string; + try { + rootCanonical = realpathSync(dir); + } catch { + return []; + } + const visited = new Set([rootCanonical]); + + const out: string[] = []; + const walk = (d: string): void => { + for (const name of readdirSync(d).sort()) { + const p = join(d, name); + let st; + try { + st = statSync(p); + } catch { + continue; + } + if (st.isDirectory()) { + let pCanonical: string; + try { + pCanonical = realpathSync(p); + } catch { + continue; + } + if (!isWithin(rootCanonical, pCanonical)) { + findings.push({ + severity: 'warn', + code: 'skill_symlink_escaped', + message: `symlink '${relative(dir, p).split(sep).join('/')}' resolves outside its skill directory and was skipped`, + path: dir, + }); + continue; + } + if (!visited.has(pCanonical)) { + visited.add(pCanonical); + walk(p); + } + } else if (st.isFile()) { + out.push(relative(dir, p).split(sep).join('/')); + } + } + }; + walk(dir); + return out.sort(); +} + +/** + * Pi's discovery rule (`pi-fork/.../core/skills.ts`): a directory holding SKILL.md IS a skill + * root and is NOT recursed into; otherwise recurse looking for one. Symlinks are followed; + * cycles and aliases are broken by canonical path. + */ +function findSkillDirs(root: string, acc: string[] = [], visited = new Set()): string[] { + let canonical: string; + try { + canonical = realpathSync(root); + } catch { + return acc; + } + if (visited.has(canonical)) return acc; + + let st; + try { + st = statSync(root); + } catch { + return acc; + } + if (!st.isDirectory()) return acc; + + visited.add(canonical); + + if (existsSync(join(root, 'SKILL.md'))) { + acc.push(root); + return acc; + } + for (const name of readdirSync(root).sort()) { + const p = join(root, name); + let pst; + try { + pst = statSync(p); + } catch { + continue; + } + if (pst.isDirectory()) { + let pCanonical: string; + try { + pCanonical = realpathSync(p); + } catch { + continue; + } + if (!visited.has(pCanonical)) { + findSkillDirs(p, acc, visited); + } + } + } + return acc; +} + +function load(dir: string, scope: SkillScope): ResolvedSkill { + const skillMd = readFileSync(join(dir, 'SKILL.md'), 'utf8'); + const fallback = dir.split(sep).filter(Boolean).at(-1) ?? 'skill'; + const findings: PreflightFinding[] = []; + const files = filesUnder(dir, findings); + return { + name: readSkillFrontmatterName(skillMd) ?? fallback, + dir, + skillMd, + files, + scope, + findings, + }; +} + +const SCOPE_RANK: Record = { project: 0, user: 1, plugin: 2 }; + +/** + * Resolve every skill across the configured roots, deduped by name and canonical path. + * + * Deduping happens at two levels: by name (project > user > plugin precedence within a scope), + * and by canonical path (symlinked aliases are recognized and collapsed). + * + * Dedupe matters concretely: `~/.claude/plugins/cache/` is largely a resolved duplicate of + * `~/.claude/plugins/marketplaces/`, so a naive scan double-counts every plugin skill (spec §4.3). + * Additionally, a skill directory can be aliased via symlinks, which we detect and dedupe. + */ +export function resolveSkills(roots: SkillRoots): ResolvedSkill[] { + const found: ResolvedSkill[] = []; + const push = (base: string | undefined, scope: SkillScope): void => { + if (!base) return; + for (const dir of findSkillDirs(join(base, 'skills'))) found.push(load(dir, scope)); + }; + push(roots.projectDir, 'project'); + push(roots.userDir, 'user'); + for (const pluginDir of roots.pluginDirs ?? []) { + for (const dir of findSkillDirs(pluginDir)) found.push(load(dir, 'plugin')); + } + + const best = new Map(); + const byCanonical = new Map(); + + for (const skill of found) { + let canonical: string; + try { + canonical = realpathSync(skill.dir); + } catch { + canonical = skill.dir; + } + + const prev = best.get(skill.name); + const prevCanonical = byCanonical.get(canonical); + + if (prevCanonical) { + // Same canonical path: prefer higher precedence scope. When the winning name differs from + // the name this same canonical path was previously filed under (SKILL.md has no `name:` + // frontmatter, so `load` falls back to the link directory's basename, and that basename + // differs across the two aliasing scopes), the old name's entry in `best` must be removed + // too -- otherwise it is never overwritten and the same physical skill is emitted twice, + // under two separate `skills//` prefixes. + if (SCOPE_RANK[skill.scope] < SCOPE_RANK[prevCanonical.scope]) { + best.delete(prevCanonical.name); + best.set(skill.name, skill); + byCanonical.set(canonical, skill); + } + } else if (!prev || SCOPE_RANK[skill.scope] < SCOPE_RANK[prev.scope]) { + // New name or higher precedence scope + best.set(skill.name, skill); + byCanonical.set(canonical, skill); + } + } + return [...best.values()].sort((a, b) => (a.name < b.name ? -1 : a.name > b.name ? 1 : 0)); +} diff --git a/packages/config-bundle/src/secret-scan.ts b/packages/config-bundle/src/secret-scan.ts new file mode 100644 index 0000000..da35df6 --- /dev/null +++ b/packages/config-bundle/src/secret-scan.ts @@ -0,0 +1,96 @@ +import type { SecretFinding, TarEntry } from './types.js'; + +/** + * TWO TIERS, and the split is empirical rather than aesthetic. + * + * Measured over a real `~/.claude` (61 bundled skills, 586 files), the structural rules below + * produced ZERO hits, while a single heuristic rule (": <12+ chars>") produced 11 — every + * one a false positive. Seven were documentation placeholders; four were CODE, e.g. + * `TOKEN = crypto.randomUUID` and `apiKey = process.env...`, which match because the value + * character class contains `.` and so accepts dotted identifiers. Two of those sit inside the + * `brainstorming` skill itself. + * + * A blocking heuristic would therefore refuse essentially every promotion on day one, and a gate + * nobody can pass gets bypassed or deleted. So structural rules BLOCK and the heuristic WARNS. + * + * Entropy scoring stays rejected as YAGNI for the same reason the heuristic is demoted: it + * collides with prose and code, and its false positives are the expensive direction. + */ +const BLOCKING_RULES: Array<{ rule: string; re: RegExp }> = [ + { rule: 'aws-access-key-id', re: /\b(?:AKIA|ASIA)[0-9A-Z]{16}\b/ }, + { rule: 'private-key-block', re: /-----BEGIN (?:[A-Z ]+ )?PRIVATE KEY-----/ }, + { rule: 'github-token', re: /\bgh[pousr]_[A-Za-z0-9]{36,}\b/ }, + { rule: 'openai-style-key', re: /\bsk-[A-Za-z0-9_-]{20,}\b/ }, + { rule: 'slack-token', re: /\bxox[abposr]-[A-Za-z0-9-]{10,}\b/ }, +]; + +/** Heuristic: real signal sometimes, prose or code often. Warns, never blocks. */ +const WARNING_RULES: Array<{ rule: string; re: RegExp }> = [ + { rule: 'bearer-token', re: /\bBearer\s+[A-Za-z0-9._~+/-]{20,}=*/ }, + { + rule: 'assigned-secret', + re: /\b(?:api[_-]?key|secret|password|passwd|token)\b\s*[:=]\s*['"]?[A-Za-z0-9._~+/-]{12,}/i, + }, +]; + +/** + * Obvious documentation placeholders. Suppresses ~7 of the 11 measured heuristic hits, so the + * warning list stays short enough that a human actually reads it. + */ +const PLACEHOLDER = + // `<[^>\n]{1,200}>` is bounded and newline-excluded to clear a CodeQL polynomial-regex alert. + // In isolation the unbounded `<[^>]+>` measured 5.2 s on 60 KB of '<' -- but it is NOT + // reachable that way today: PLACEHOLDER only ever runs on a WARNING_RULES match, and both of + // those rules' value character classes ([A-Za-z0-9._~+/-]) exclude '<', so the matched text + // can never contain one. The bound is defence in depth against a future rule that admits '<', + // not a fix for a live denial of service. (The two bounds in preflight.ts ARE reachable.) + /your[_-]?|example|placeholder|change[_-]?me|xxx+|dummy|fake|<[^>\n]{1,200}>|\bREPLACE|\bTODO|\.\.\./i; + +/** Heuristic: a NUL byte means binary, so line-based scanning would be noise. */ +function looksBinary(buf: Buffer): boolean { + return buf.subarray(0, 4096).includes(0); +} + +/** + * Every secret-shaped hit, each tagged `blocking` or `warning`. Empty means clean. + * Blocking hits must stop promotion; warnings are reported and promotion continues. + */ +export function scanEntriesForSecrets(entries: TarEntry[]): SecretFinding[] { + const findings: SecretFinding[] = []; + for (const entry of entries) { + if (looksBinary(entry.content)) continue; + const lines = entry.content.toString('utf8').split('\n'); + for (let i = 0; i < lines.length; i++) { + const line = lines[i]!; + const blocking = BLOCKING_RULES.find((r) => r.re.test(line)); + if (blocking) { + findings.push({ path: entry.path, line: i + 1, rule: blocking.rule, severity: 'blocking' }); + continue; // one finding per line is enough + } + const warn = WARNING_RULES.find((r) => r.re.test(line)); + if (warn) { + const matched = warn.re.exec(line)?.[0] ?? ''; + if (PLACEHOLDER.test(matched)) continue; // documentation example, not a credential + findings.push({ path: entry.path, line: i + 1, rule: warn.rule, severity: 'warning' }); + } + } + } + return findings; +} + +/** Only the blocking subset. */ +export function blockingSecrets(findings: SecretFinding[]): SecretFinding[] { + return findings.filter((f) => f.severity === 'blocking'); +} + +/** Thrown by buildBundle when a BLOCKING hit is present. Promotion must not proceed. */ +export class SecretScanError extends Error { + constructor(readonly findings: SecretFinding[]) { + const first = findings[0]; + super( + `secret scan blocked promotion: ${findings.length} blocking finding(s), first at ` + + `${first?.path}:${first?.line} (${first?.rule})`, + ); + this.name = 'SecretScanError'; + } +} diff --git a/packages/config-bundle/src/tar.ts b/packages/config-bundle/src/tar.ts new file mode 100644 index 0000000..ce92fe7 --- /dev/null +++ b/packages/config-bundle/src/tar.ts @@ -0,0 +1,138 @@ +import { createHash } from 'node:crypto'; + +const BLOCK = 512; +/** USTAR encodes a path as prefix(155) + '/' + name(100). */ +export const MAX_USTAR_PATH = 255; + +import type { TarEntry } from './types.js'; + +/** Write `n` as a NUL-terminated octal field of `len` bytes (len-1 digits + NUL). */ +function writeOctal(b: Buffer, n: number, off: number, len: number): void { + b.write(n.toString(8).padStart(len - 1, '0') + '\0', off, len, 'ascii'); +} + +/** Split a path into USTAR name/prefix. Throws rather than truncate. */ +export function splitUstarPath(p: string): { name: string; prefix: string } { + if (Buffer.byteLength(p) <= 100) return { name: p, prefix: '' }; + for (let i = p.indexOf('/'); i !== -1; i = p.indexOf('/', i + 1)) { + const prefix = p.slice(0, i); + const name = p.slice(i + 1); + if (Buffer.byteLength(prefix) <= 155 && Buffer.byteLength(name) <= 100) return { name, prefix }; + } + throw new Error(`path too long for USTAR (max ${MAX_USTAR_PATH} with a '/' split point): ${p}`); +} + +function header(path: string, size: number, mode: number): Buffer { + const h = Buffer.alloc(BLOCK); + const { name, prefix } = splitUstarPath(path); + h.write(name, 0, 100, 'utf8'); + writeOctal(h, mode & 0o7777, 100, 8); + writeOctal(h, 0, 108, 8); // uid — always 0, never the builder's + writeOctal(h, 0, 116, 8); // gid + writeOctal(h, size, 124, 12); + writeOctal(h, 0, 136, 12); // mtime — always 0, the usual source of tar non-determinism + h.write(' ', 148, 8, 'ascii'); // checksum computed over spaces, then overwritten + h.write('0', 156, 1, 'ascii'); // type: regular file + h.write('ustar\0', 257, 6, 'ascii'); + h.write('00', 263, 2, 'ascii'); + h.write(prefix, 345, 155, 'utf8'); + let sum = 0; + for (const byte of h) sum += byte; + h.write(sum.toString(8).padStart(6, '0') + '\0 ', 148, 8, 'ascii'); + return h; +} + +/** + * Deterministic USTAR archive: entries sorted byte-wise by path, no mtime/uid/gid, fixed + * modes. Two builds of the same content produce byte-identical output, which is what makes + * the digest a stable identity (spec §4.1). + */ +export function canonicalTar(entries: TarEntry[]): Buffer { + const sorted = [...entries].sort((a, b) => + Buffer.compare(Buffer.from(a.path, 'utf8'), Buffer.from(b.path, 'utf8')), + ); + const parts: Buffer[] = []; + for (const entry of sorted) { + parts.push(header(entry.path, entry.content.length, entry.mode ?? 0o644)); + parts.push(entry.content); + const pad = (BLOCK - (entry.content.length % BLOCK)) % BLOCK; + if (pad > 0) parts.push(Buffer.alloc(pad)); + } + parts.push(Buffer.alloc(BLOCK * 2)); // end-of-archive + return Buffer.concat(parts); +} + +/** Read a canonical archive back. Ignores anything that is not a regular file. */ +export function untar(tar: Buffer): TarEntry[] { + const out: TarEntry[] = []; + for (let off = 0; off + BLOCK <= tar.length;) { + const h = tar.subarray(off, off + BLOCK); + if (h.every((b) => b === 0)) break; // end-of-archive + const cstr = (s: number, n: number) => { + const raw = h.subarray(s, s + n); + const z = raw.indexOf(0); + return raw.subarray(0, z === -1 ? n : z).toString('utf8'); + }; + const name = cstr(0, 100); + const prefix = cstr(345, 155); + const mode = parseInt(cstr(100, 8).trim() || '0', 8); + const size = parseInt(cstr(124, 12).trim() || '0', 8); + const type = h.subarray(156, 157).toString('ascii'); + off += BLOCK; + if (type === '0' || type === '\0') { + out.push({ + path: prefix ? `${prefix}/${name}` : name, + content: Buffer.from(tar.subarray(off, off + size)), + mode, + }); + } + off += Math.ceil(size / BLOCK) * BLOCK; + } + return out; +} + +/** Content address of a canonical archive. */ +export function digestOf(tar: Buffer): string { + return 'sha256:' + createHash('sha256').update(tar).digest('hex'); +} + +const DIGEST_RE = /^sha256:[0-9a-f]{64}$/; + +/** + * Thrown by `assertValidDigest` for anything not matching `sha256:<64 lowercase hex>`. The + * message caps how much of the bad value it echoes, so a long or crafted string (e.g. a + * path-traversal payload) doesn't ride unbounded into logs or a rendered error. + */ +export class InvalidDigestError extends Error { + constructor(digest: string) { + const shown = + digest.length > 40 ? `${digest.slice(0, 40)}...(${digest.length} chars total)` : digest; + super( + `invalid digest: expected 'sha256:' + 64 lowercase hex chars, got ${JSON.stringify(shown)}`, + ); + this.name = 'InvalidDigestError'; + } +} + +/** + * Reject any digest string not shaped `sha256:<64 lowercase hex>`. Both `unpackBundle` + * (harness/src/config-resolver.ts) and `configCacheDir` (harness/src/config-overlay.ts) turn a + * digest into a filesystem path via `digestDirName` + `join(baseDir, ...)`; a shape like + * `sha256:../../../evil` would otherwise walk that join straight out of the base directory. + * Neither pod currently accepts an attacker-chosen digest over the wire today -- this is + * defense-in-depth against a digest string that originated somewhere less trusted than the + * builder in this package, not a demonstrated live exploit. One shared check, called from both + * sites, keeps that guarantee from drifting between them. + */ +export function assertValidDigest(digest: string): string { + if (!DIGEST_RE.test(digest)) throw new InvalidDigestError(digest); + return digest; +} + +/** Filesystem-safe form of a digest, for use as a directory name. `replaceAll`, not `replace`: + * a digest has exactly one `:` today (`sha256:`), but a single `replace` silently stops + * after the first match, so a future multi-colon shape would collide two distinct digests into + * the same directory name instead of erroring. */ +export function digestDirName(digest: string): string { + return digest.replaceAll(':', '-'); +} diff --git a/packages/config-bundle/src/types.ts b/packages/config-bundle/src/types.ts new file mode 100644 index 0000000..13428f2 --- /dev/null +++ b/packages/config-bundle/src/types.ts @@ -0,0 +1,132 @@ +/** One file in a bundle. `mode` defaults to 0o644; scripts use 0o755. */ +export interface TarEntry { + path: string; + content: Buffer; + mode?: number; +} + +/** Bundle wire-format version. Bumped only on a breaking layout change. */ +export const BUNDLE_FORMAT_VERSION = 1; + +export type SkillScope = 'project' | 'user' | 'plugin'; + +/** One skill, resolved to a directory. `files` are paths relative to `dir`, always incl. SKILL.md. */ +export interface ResolvedSkill { + name: string; + dir: string; + skillMd: string; + files: string[]; + scope: SkillScope; + /** Non-fatal findings raised while resolving this skill's files (e.g. an escaped symlink). */ + findings?: PreflightFinding[]; +} + +export interface SkillRoots { + /** Repo-local `.claude` directory. */ + projectDir?: string; + /** `~/.claude`. */ + userDir?: string; + /** e.g. [`~/.claude/plugins`]. Scanned recursively; cache/marketplace duplicates collapse. */ + pluginDirs?: string[]; +} + +export type DropReason = 'no_harness_equivalent' | 'needs_subagent' | 'user_denied'; + +/** Unattended: no human can answer a question. Attended: phase-2 live attach (spec §4.6). */ +export type PromoteMode = 'unattended' | 'attended'; + +export interface DroppedSkill { + name: string; + reason: DropReason; + detail: string; +} + +export interface Classification { + travels: ResolvedSkill[]; + dropped: DroppedSkill[]; + /** Travels, but degrades without a human. Warned under `unattended` only. */ + interactionDependent: string[]; +} + +export interface ClassifyOptions { + mode: PromoteMode; + userDenyList?: string[]; +} + +export type SecretSeverity = 'blocking' | 'warning'; + +export interface SecretFinding { + path: string; + line: number; + rule: string; + severity: SecretSeverity; +} + +export interface LockfileSkillRecord { + name: string; + scope: SkillScope; + sourceDir: string; + contentHash: string; +} + +export interface BundleLockfile { + binaries: string[]; + builtAgainst: { harness: string; pi: string }; + context: string[]; + digest: string; + dropped: DroppedSkill[]; + entry: string; + formatVersion: number; + interactionDependent: string[]; + memory: string[]; + mode: PromoteMode; + sandboxImage: string; + skills: LockfileSkillRecord[]; +} + +export interface LockfileInput { + digest: string; + mode: PromoteMode; + entry: string; + classification: Classification; + contextPaths: string[]; + memoryPaths: string[]; + sandboxImage: string; + binaries: string[]; + versions: { pi: string; harness: string }; + skillHashes: Record; +} + +export interface PreflightFinding { + severity: 'error' | 'warn' | 'info'; + code: string; + message: string; + path?: string; +} + +export interface BuildBundleInput { + roots: SkillRoots; + /** Directory holding MEMORY.md plus one-fact-per-file memories. */ + memoryDir?: string; + /** Directory of slash-command templates; each `.md` becomes prompt ``. */ + promptsDir?: string; + /** Already-read CLAUDE.md/AGENTS.md chain, outermost first. */ + contextFiles?: Array<{ path: string; content: string }>; + entry: string; + mode: PromoteMode; + userDenyList?: string[]; + sandboxImage: string; + /** Commands the sandbox image provides; undefined ⇒ cannot verify. */ + inventory?: string[]; + versions: { pi: string; harness: string }; + /** Extra appendSystemPrompt fragments beyond the two standard notes. */ + extraPromptFragments?: string[]; +} + +export interface BuildResult { + tar: Buffer; + digest: string; + lockfile: BundleLockfile; + findings: PreflightFinding[]; + promptNames: string[]; +} diff --git a/packages/config-bundle/test/build.test.ts b/packages/config-bundle/test/build.test.ts new file mode 100644 index 0000000..4c25863 --- /dev/null +++ b/packages/config-bundle/test/build.test.ts @@ -0,0 +1,150 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { buildBundle, contentDigest, LOCKFILE_PATH } from '../src/build.js'; +import { untar } from '../src/tar.js'; +import { SecretScanError } from '../src/secret-scan.js'; + +let root: string; + +function write(rel: string, body: string): void { + const p = join(root, rel); + mkdirSync(join(p, '..'), { recursive: true }); + writeFileSync(p, body); +} + +function baseInput() { + return { + roots: { userDir: join(root, 'user') }, + memoryDir: join(root, 'memory'), + promptsDir: join(root, 'prompts'), + contextFiles: [{ path: '/proj/CLAUDE.md', content: '# project rules' }], + entry: 'go', + mode: 'unattended' as const, + sandboxImage: 'sandbox:pool-default', + inventory: ['gh'], + versions: { pi: '0.42.0', harness: '0.0.0' }, + }; +} + +beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'cb-build-')); + // Fenced block, NOT an inline `gh` span: detectBinaries scans only ```bash/sh/shell fences. + write( + 'user/skills/keeper/SKILL.md', + '---\nname: keeper\ndescription: d\n---\n```bash\ngh pr list\n```\n', + ); + write( + 'user/skills/artifact-design/SKILL.md', + '---\nname: artifact-design\ndescription: d\n---\nx', + ); + write('memory/MEMORY.md', '- [A](alpha.md) — hook'); + write('memory/alpha.md', '---\nname: alpha\n---\nfact'); + write('prompts/go.md', 'do the thing'); +}); +afterEach(() => { + rmSync(root, { recursive: true, force: true }); +}); + +describe('buildBundle', () => { + it('packs traveling skills under skills// and drops deny-listed ones', () => { + const r = buildBundle(baseInput()); + const paths = untar(r.tar).map((e) => e.path); + expect(paths).toContain('skills/keeper/SKILL.md'); + expect(paths.some((p) => p.startsWith('skills/artifact-design/'))).toBe(false); + expect(r.lockfile.dropped.map((d) => d.name)).toEqual(['artifact-design']); + }); + + it('puts inline context under context/ and sandbox-read memory under memory/', () => { + const paths = untar(buildBundle(baseInput()).tar).map((e) => e.path); + expect(paths).toContain('context/agents/0-CLAUDE.md'); + expect(paths).toContain('context/MEMORY.md'); + expect(paths).toContain('memory/alpha.md'); + // MEMORY.md is injected inline, so it must NOT also be under memory/ + expect(paths).not.toContain('memory/MEMORY.md'); + }); + + it('includes the two injected prompt notes and the entry prompt', () => { + const r = buildBundle(baseInput()); + const paths = untar(r.tar).map((e) => e.path); + expect(paths.filter((p) => p.startsWith('prompt/'))).toHaveLength(2); + expect(paths).toContain('prompts/go.md'); + expect(r.promptNames).toEqual(['go']); + }); + + it('embeds the lockfile and stamps it with the content digest', () => { + const r = buildBundle(baseInput()); + const entry = untar(r.tar).find((e) => e.path === LOCKFILE_PATH)!; + expect(JSON.parse(entry.content.toString()).digest).toBe(r.digest); + }); + + it('defines the digest over content only, so it cannot depend on itself', () => { + const r = buildBundle(baseInput()); + const withoutLock = untar(r.tar).filter((e) => e.path !== LOCKFILE_PATH); + expect(contentDigest(withoutLock)).toBe(r.digest); + }); + + it('is deterministic: two builds of the same tree give the same digest', () => { + expect(buildBundle(baseInput()).digest).toBe(buildBundle(baseInput()).digest); + }); + + it('changes the digest when a skill body changes', () => { + const before = buildBundle(baseInput()).digest; + write('user/skills/keeper/SKILL.md', '---\nname: keeper\ndescription: d\n---\nchanged'); + expect(buildBundle(baseInput()).digest).not.toBe(before); + }); + + it('surfaces a missing binary as a preflight WARNING, not a blocking error', () => { + // Demoted from error after measurement: on a real ~/.claude the detector produced 44 binaries + // and 32 "missing", half of them not binaries at all (angular, django, `your_command`). At error + // severity that refused every real promotion. + const r = buildBundle({ ...baseInput(), inventory: [] }); + const missing = r.findings.filter((f) => f.code === 'missing_binary'); + expect(missing.length).toBeGreaterThan(0); + expect(missing.every((f) => f.severity === 'warn')).toBe(true); + expect(r.findings.some((f) => f.severity === 'error')).toBe(false); + }); + + it('throws SecretScanError and packs nothing when a BLOCKING credential is present', () => { + write('memory/leak.md', 'token: AKIAIOSFODNN7EXAMPLE'); + expect(() => buildBundle(baseInput())).toThrow(SecretScanError); + }); + + it('does NOT throw on a heuristic-only hit, and surfaces it as a warning finding', () => { + write('memory/looks-like.md', 'apiKey = process.env.ANTHROPIC_KEY'); + const r = buildBundle(baseInput()); + const warn = r.findings.filter((f) => f.code === 'possible_secret'); + expect(warn.length).toBeGreaterThan(0); + expect(warn.every((f) => f.severity === 'warn')).toBe(true); + }); + + it('records the entry, mode, image and detected binaries in the lockfile', () => { + const l = buildBundle(baseInput()).lockfile; + expect(l.entry).toBe('go'); + expect(l.mode).toBe('unattended'); + expect(l.sandboxImage).toBe('sandbox:pool-default'); + expect(l.binaries).toContain('gh'); + }); + + it("warns and excludes a namespaced prompt subdirectory (Claude Code's /:)", () => { + write('prompts/ns/cmd.md', 'namespaced command body'); + const r = buildBundle(baseInput()); + const paths = untar(r.tar).map((e) => e.path); + // not recursed into: neither the file nor its containing "directory" travels + expect(paths.some((p) => p.startsWith('prompts/ns'))).toBe(false); + // the flat prompts/go.md fixture from beforeEach still travels normally + expect(paths).toContain('prompts/go.md'); + const warn = r.findings.filter((f) => f.code === 'namespaced_prompt_skipped'); + expect(warn).toHaveLength(1); + expect(warn[0]!.severity).toBe('warn'); + expect(warn[0]!.message).toContain('ns'); + }); + + it('raises no namespaced_prompt_skipped finding for a flat promptsDir', () => { + // beforeEach's fixture is already flat (prompts/go.md only); this asserts explicitly rather + // than relying on that being incidental to the other passing tests. + const r = buildBundle(baseInput()); + expect(r.findings.some((f) => f.code === 'namespaced_prompt_skipped')).toBe(false); + }); +}); diff --git a/packages/config-bundle/test/classify.test.ts b/packages/config-bundle/test/classify.test.ts new file mode 100644 index 0000000..16d41cb --- /dev/null +++ b/packages/config-bundle/test/classify.test.ts @@ -0,0 +1,154 @@ +import { describe, it, expect } from 'vitest'; +import { + classifySkills, + detectBinaries, + DEFAULT_DENY_LIST, + INTERACTION_DEPENDENT, + SUBAGENT_DEPENDENT, +} from '../src/classify.js'; +import type { ResolvedSkill } from '../src/types.js'; + +const s = (name: string, body = 'body'): ResolvedSkill => ({ + name, + dir: `/tmp/${name}`, + skillMd: `---\nname: ${name}\ndescription: d\n---\n${body}`, + files: ['SKILL.md'], + scope: 'plugin', +}); + +describe('classifySkills', () => { + it('lets an ordinary prose skill travel', () => { + const c = classifySkills([s('brainstorming'), s('my-skill')], { + mode: 'attended', + }); + expect(c.travels.map((x) => x.name)).toContain('my-skill'); + expect(c.dropped).toEqual([]); + }); + + it('drops deny-listed families with no_harness_equivalent', () => { + const c = classifySkills([s('xlsx'), s('artifact-design')], { + mode: 'unattended', + }); + expect(c.travels).toEqual([]); + expect(c.dropped.map((d) => d.reason)).toEqual([ + 'no_harness_equivalent', + 'no_harness_equivalent', + ]); + }); + + it('drops subagent-dependent skills with needs_subagent (spec §9, out of scope)', () => { + const c = classifySkills([s('dispatching-parallel-agents')], { + mode: 'unattended', + }); + expect(c.dropped[0]!.reason).toBe('needs_subagent'); + }); + + it('does NOT drop a skill merely for containing the word agent', () => { + // The curated-not-inferred rule: "agent" appears in nearly every superpowers skill. + const c = classifySkills([s('some-skill', 'dispatch an agent-like helper')], { + mode: 'unattended', + }); + expect(c.travels.map((x) => x.name)).toEqual(['some-skill']); + }); + + it('honours a user deny-list additively', () => { + const c = classifySkills([s('private-thing'), s('keeper')], { + mode: 'attended', + userDenyList: ['private-thing'], + }); + expect(c.dropped[0]).toEqual({ + name: 'private-thing', + reason: 'user_denied', + detail: 'excluded by the local user deny-list', + }); + expect(c.travels.map((x) => x.name)).toEqual(['keeper']); + }); + + it('flags interaction-dependent skills without dropping them, under unattended', () => { + const c = classifySkills([s('brainstorming')], { mode: 'unattended' }); + expect(c.interactionDependent).toEqual(['brainstorming']); + expect(c.travels.map((x) => x.name)).toEqual(['brainstorming']); + }); + + it('does not flag interaction dependence under attended mode', () => { + const c = classifySkills([s('brainstorming')], { mode: 'attended' }); + expect(c.interactionDependent).toEqual([]); + }); + + it('lists deny-list and interaction-list entries as plain data, not regexes', () => { + expect(DEFAULT_DENY_LIST.every((n) => typeof n === 'string')).toBe(true); + expect(INTERACTION_DEPENDENT).toContain('brainstorming'); + }); + + it('uses bare name form from SKILL.md frontmatter, never qualified scope:name', () => { + // All three curated lists must contain no `:` character; they are matched against + // ResolvedSkill.name which is the bare frontmatter name, not scope-qualified. + const allEntries = [...DEFAULT_DENY_LIST, ...SUBAGENT_DEPENDENT, ...INTERACTION_DEPENDENT]; + expect(allEntries.every((e) => !e.includes(':'))).toBe(true); + }); + + it('drops subagent-dependent skills by bare name', () => { + const c = classifySkills([s('dispatching-parallel-agents')], { + mode: 'unattended', + }); + expect(c.dropped[0]).toEqual({ + name: 'dispatching-parallel-agents', + reason: 'needs_subagent', + detail: 'requires a subagent tool; Pi has none (spec §9)', + }); + expect(c.travels).toEqual([]); + }); + + it('drops all subagent-dependent skills by bare name', () => { + const c = classifySkills([s('subagent-driven-development')], { + mode: 'unattended', + }); + expect(c.dropped[0]!.reason).toBe('needs_subagent'); + }); + + it('flags interaction-dependent skills by bare name, unattended', () => { + const c = classifySkills([s('brainstorming')], { mode: 'unattended' }); + expect(c.interactionDependent).toEqual(['brainstorming']); + expect(c.travels.map((x) => x.name)).toEqual(['brainstorming']); + }); + + it('drops deny-listed skills by bare name', () => { + const c = classifySkills([s('xlsx')], { mode: 'unattended' }); + expect(c.dropped[0]).toEqual({ + name: 'xlsx', + reason: 'no_harness_equivalent', + detail: 'subject matter does not exist in the harness runtime', + }); + }); +}); + +describe('detectBinaries', () => { + it('finds commands in fenced bash blocks', () => { + const skill = s('x', '```bash\ngh pr list\nkubectl get pods\n```'); + expect(detectBinaries([skill])).toEqual(['gh', 'kubectl']); + }); + + it('ignores shell builtins and flags', () => { + const skill = s('x', '```bash\ncd /tmp && echo hi\nexport A=1\n```'); + expect(detectBinaries([skill])).toEqual([]); + }); + + it('deduplicates and sorts', () => { + const a = s('a', '```bash\ngh x\n```'); + const b = s('b', '```bash\ngh y\npnpm i\n```'); + expect(detectBinaries([a, b])).toEqual(['gh', 'pnpm']); + }); + + it('skips leading environment variable assignments', () => { + const skill = s('x', '```bash\nSH_PROMOTE_LIVE_SMOKE=1 pnpm exec vitest run\n```'); + expect(detectBinaries([skill])).toEqual(['pnpm']); + }); + + it('ignores non-shell fenced blocks but finds commands in adjacent bash blocks', () => { + const skill = s( + 'x', + '```ts\ngh pr list\n```\n```\necho hi\n```\n```bash\nkubectl get pods\n```', + ); + expect(detectBinaries([skill])).toEqual(['kubectl']); + }); +}); diff --git a/packages/config-bundle/test/lockfile.test.ts b/packages/config-bundle/test/lockfile.test.ts new file mode 100644 index 0000000..d7fbca3 --- /dev/null +++ b/packages/config-bundle/test/lockfile.test.ts @@ -0,0 +1,163 @@ +import { describe, it, expect } from 'vitest'; +import { buildLockfile, serializeLockfile, skillContentHash } from '../src/lockfile.js'; +import { BUNDLE_FORMAT_VERSION } from '../src/types.js'; +import type { ResolvedSkill, LockfileInput } from '../src/types.js'; + +const skill = (name: string): ResolvedSkill => ({ + name, + dir: `/home/u/.claude/skills/${name}`, + skillMd: `---\nname: ${name}\n---\nbody`, + files: ['SKILL.md'], + scope: 'user', +}); + +const input = () => ({ + digest: 'sha256:' + 'a'.repeat(64), + mode: 'unattended' as const, + entry: 'brainstorm-and-plan', + classification: { + travels: [skill('keeper')], + dropped: [ + { + name: 'artifact-design', + reason: 'no_harness_equivalent' as const, + detail: 'subject matter does not exist in the harness runtime', + }, + ], + interactionDependent: ['superpowers:brainstorming'], + }, + contextPaths: ['context/agents/0-CLAUDE.md'], + memoryPaths: ['memory/ghcr-images-moved-to-rossoctl.md'], + sandboxImage: 'sandbox:pool-default', + binaries: ['gh', 'kubectl'], + versions: { pi: '0.42.0', harness: '0.0.0' }, + skillHashes: { keeper: 'sha256:' + 'b'.repeat(64) }, +}); + +const inputWithMultipleUnsorted = () => ({ + digest: 'sha256:' + 'a'.repeat(64), + mode: 'unattended' as const, + entry: 'brainstorm-and-plan', + classification: { + travels: [skill('zulu'), skill('alpha')], + dropped: [ + { + name: 'zebra-skill', + reason: 'no_harness_equivalent' as const, + detail: 'detail1', + }, + { + name: 'alpha-skill', + reason: 'needs_subagent' as const, + detail: 'detail2', + }, + ], + interactionDependent: ['zulu-dep', 'alpha-dep'], + }, + contextPaths: ['context/z.md', 'context/a.md'], + memoryPaths: ['memory/z.md', 'memory/a.md'], + sandboxImage: 'sandbox:pool-default', + binaries: ['kubectl', 'gh'], + versions: { pi: '0.42.0', harness: '0.0.0' }, + skillHashes: { zulu: 'sha256:' + 'z'.repeat(64), alpha: 'sha256:' + 'a'.repeat(64) }, +}); + +describe('buildLockfile', () => { + it('stamps the format version and the digest', () => { + const l = buildLockfile(input()); + expect(l.formatVersion).toBe(BUNDLE_FORMAT_VERSION); + expect(l.digest).toBe('sha256:' + 'a'.repeat(64)); + }); + + it('records every included skill with its source dir and content hash', () => { + const l = buildLockfile(input()); + expect(l.skills).toEqual([ + { + name: 'keeper', + scope: 'user', + sourceDir: '/home/u/.claude/skills/keeper', + contentHash: 'sha256:' + 'b'.repeat(64), + }, + ]); + }); + + it('records every exclusion with a machine-readable reason', () => { + expect(buildLockfile(input()).dropped[0]!.reason).toBe('no_harness_equivalent'); + }); + + it('records interaction-dependent skills, context, memory, image and binaries', () => { + const l = buildLockfile(input()); + expect(l.interactionDependent).toEqual(['superpowers:brainstorming']); + expect(l.context).toEqual(['context/agents/0-CLAUDE.md']); + expect(l.memory).toEqual(['memory/ghcr-images-moved-to-rossoctl.md']); + expect(l.sandboxImage).toBe('sandbox:pool-default'); + expect(l.binaries).toEqual(['gh', 'kubectl']); + }); + + it('sorts every array field — binaries, context, memory, interactionDependent, dropped, skills', () => { + const l = buildLockfile(inputWithMultipleUnsorted()); + expect(l.binaries).toEqual(['gh', 'kubectl']); + expect(l.context).toEqual(['context/a.md', 'context/z.md']); + expect(l.memory).toEqual(['memory/a.md', 'memory/z.md']); + expect(l.interactionDependent).toEqual(['alpha-dep', 'zulu-dep']); + expect(l.dropped.map((d) => d.name)).toEqual(['alpha-skill', 'zebra-skill']); + expect(l.skills.map((s) => s.name)).toEqual(['alpha', 'zulu']); + }); + + it('uses an empty string when a skill has no content hash', () => { + const inp = inputWithMultipleUnsorted(); + const partial = inp as Omit & { + skillHashes: Record; + }; + partial.skillHashes = {}; // Missing entries for both skills + const l = buildLockfile(inp); + expect(l.skills[0]!.contentHash).toBe(''); + expect(l.skills[1]!.contentHash).toBe(''); + }); +}); + +describe('serializeLockfile', () => { + it('is stable, sorted-key JSON with a trailing newline so it diffs legibly', () => { + const a = serializeLockfile(buildLockfile(input())); + const b = serializeLockfile(buildLockfile(input())); + expect(a).toBe(b); + expect(a.endsWith('\n')).toBe(true); + const obj = JSON.parse(a); + const keys = Object.keys(obj); + expect(keys).toEqual([...keys].sort()); + }); + + it('recursively sorts keys at all nesting levels, not just top-level', () => { + const serialized = serializeLockfile(buildLockfile(inputWithMultipleUnsorted())); + const obj = JSON.parse(serialized); + + // Top-level keys must be sorted + const topKeys = Object.keys(obj); + expect(topKeys).toEqual([...topKeys].sort()); + + // Nested object keys in skills[0] must be sorted + // skills are built with {name, scope, sourceDir, contentHash} but should serialize as + // {contentHash, name, scope, sourceDir} (alphabetical) + const skillKeys = Object.keys(obj.skills[0]); + expect(skillKeys).toEqual([...skillKeys].sort()); + expect(skillKeys).toEqual(['contentHash', 'name', 'scope', 'sourceDir']); + + // Nested object keys in dropped[0] must be sorted + // dropped are built with {name, reason, detail} but should serialize as + // {detail, name, reason} (alphabetical) + const droppedKeys = Object.keys(obj.dropped[0]); + expect(droppedKeys).toEqual([...droppedKeys].sort()); + expect(droppedKeys).toEqual(['detail', 'name', 'reason']); + }); +}); + +describe('skillContentHash', () => { + it('is stable for identical content and differs when a file changes', () => { + const s = skill('x'); + const one = skillContentHash(s, [{ path: 'skills/x/SKILL.md', content: Buffer.from('a') }]); + const same = skillContentHash(s, [{ path: 'skills/x/SKILL.md', content: Buffer.from('a') }]); + const other = skillContentHash(s, [{ path: 'skills/x/SKILL.md', content: Buffer.from('b') }]); + expect(one).toBe(same); + expect(one).not.toBe(other); + }); +}); diff --git a/packages/config-bundle/test/notes.test.ts b/packages/config-bundle/test/notes.test.ts new file mode 100644 index 0000000..db4e86e --- /dev/null +++ b/packages/config-bundle/test/notes.test.ts @@ -0,0 +1,59 @@ +import { describe, it, expect } from 'vitest'; +import { toolNameMappingNote, skillsRootNote } from '../src/notes.js'; + +describe('toolNameMappingNote', () => { + const note = toolNameMappingNote(); + it('maps every Claude Code built-in that differs from Pi', () => { + for (const pair of ['Bash', 'bash', 'Glob', 'find', 'LS', 'ls', 'Read', 'read']) { + expect(note).toContain(pair); + } + }); + it('is multi-line, so resolvePromptInput cannot mistake it for a file path', () => { + // pi-fork resource-loader.ts:49 reads the string as a FILE when existsSync(input). + expect(note).toContain('\n'); + }); +}); + +describe('skillsRootNote', () => { + it('does not name an environment variable -- nothing in this system ever sets one', () => { + const note = skillsRootNote(); + // The bug this test guards: the note used to instruct the model to use $SH_SKILLS_DIR / + // $SH_MEMORY_DIR, which no seam (bundle build time, or the sandbox's per-call `bash -c`) + // can ever set. Following that instruction literally expands to nothing and misses. + expect(note).not.toContain('SH_SKILLS_DIR'); + expect(note).not.toContain('SH_MEMORY_DIR'); + expect(note).not.toContain('$'); + }); + + it("tells the model to resolve a skill's relative file references against that skill's own directory, not cwd", () => { + const note = skillsRootNote().toLowerCase(); + expect(note).toContain('relative'); + expect(note).toContain('skill'); + expect(note).toContain('working directory'); + }); + + it('tells the model what to do when those lines are absent, instead of dangling', () => { + // run-leaf.ts appends the "Skill files:"/"Memory files:" lines only when a sandbox is + // selected. The bundle is built before any leaf exists, so this note cannot be omitted + // conditionally -- without a fallback it points at lines that were never written, which is + // the same "instruction referencing something absent" defect the env-var removal fixed. + const note = skillsRootNote(); + expect(note).toMatch(/if no such lines appear/i); + expect(note.toLowerCase()).toContain('not reachable'); + // Must tell the model to SAY so rather than invent a path or misreport a missing file. + expect(note.toLowerCase()).toMatch(/say so/); + }); + + it('points at the "Skill files:" / "Memory files:" fragment run-leaf.ts injects per leaf', () => { + // This is the load-bearing link to run-leaf.ts's self-describing fragment: the note alone + // cannot carry an absolute path (the bundle is built once, before any leaf exists), so it + // must point at where that path actually appears in the prompt. + const note = skillsRootNote(); + expect(note).toContain('Skill files:'); + expect(note).toContain('Memory files:'); + }); + + it('is multi-line, so resolvePromptInput cannot mistake it for a file path', () => { + expect(skillsRootNote()).toContain('\n'); + }); +}); diff --git a/packages/config-bundle/test/preflight.test.ts b/packages/config-bundle/test/preflight.test.ts new file mode 100644 index 0000000..5a04d08 --- /dev/null +++ b/packages/config-bundle/test/preflight.test.ts @@ -0,0 +1,239 @@ +import { describe, it, expect } from 'vitest'; +import { + checkSiblingPaths, + checkMemoryLinks, + checkBinaries, + checkEntry, + checkInteraction, + renderPreflight, + hasErrors, +} from '../src/preflight.js'; +import type { ResolvedSkill } from '../src/types.js'; + +const skill = (name: string, body: string, files: string[]): ResolvedSkill => ({ + name, + dir: `/s/${name}`, + skillMd: `---\nname: ${name}\n---\n${body}`, + files: ['SKILL.md', ...files], + scope: 'plugin', +}); + +describe('checkSiblingPaths', () => { + it('is quiet when a referenced sibling is present', () => { + const s = skill('brainstorming', 'read `skills/brainstorming/visual-companion.md`', [ + 'visual-companion.md', + ]); + expect(checkSiblingPaths([s])).toEqual([]); + }); + + it('warns when a sibling under a directory the skill owns is absent', () => { + // The skill ships references/guide.md, so it demonstrably owns references/ — a missing file + // there is a real packaging gap. Warn, not error: preflight blocks only on facts. + const s = skill('x', 'see `references/missing.md` for detail', ['references/guide.md']); + const f = checkSiblingPaths([s]); + expect(f).toHaveLength(1); + expect(f[0]!.severity).toBe('warn'); + expect(f[0]!.code).toBe('missing_sibling'); + expect(f[0]!.message).toContain('references/missing.md'); + }); + + it('treats every ancestor directory as owned, not just the leaf parent', () => { + // A skill shipping only a nested file still owns the ancestor directory. + const nested = skill('x', 'see `references/missing.md`', ['references/deep/x.md']); + expect(checkSiblingPaths([nested])).toHaveLength(1); + // And symmetrically: shipping a shallow file means it owns the dir for a deep reference too. + const shallow = skill('y', 'see `references/deep/missing.md`', ['references/guide.md']); + expect(checkSiblingPaths([shallow])).toHaveLength(1); + // Prefix matching stays EXACT — a sibling directory must not satisfy it. + const neighbour = skill('z', 'see `references/missing.md`', ['references-old/g.md']); + expect(checkSiblingPaths([neighbour])).toEqual([]); + }); + + it('ignores paths the skill does not own, which is what prose is full of', () => { + // Measured: without these two exclusions the check fired 182 times across 28 real skills. + const bare = skill('x', 'create `main.py` and `requirements.txt` yourself', [ + 'references/g.md', + ]); + expect(checkSiblingPaths([bare])).toEqual([]); + const unowned = skill('y', 'see `src/server.ts` in your project', ['references/g.md']); + expect(checkSiblingPaths([unowned])).toEqual([]); + const code = skill('z', 'call `window.open` and read `sys.path`', ['references/g.md']); + expect(checkSiblingPaths([code])).toEqual([]); + }); + + it('ignores URLs and non-file-looking backticks', () => { + const s = skill('x', 'see `https://e.com/a.md` and `--flag` and `some text`', []); + expect(checkSiblingPaths([s])).toEqual([]); + }); + + it('ignores version numbers, IPs, semver ranges, and globs', () => { + const s = skill( + 'x', + 'see `1.2.3` and `127.0.0.1` and `node>=18.0` and `*.md` in backticks', + [], + ); + expect(checkSiblingPaths([s])).toEqual([]); + }); + + it('still warns on genuinely missing owned files when filtering out false positives', () => { + // The skill owns references/ (it ships references/guide.md), so references/missing.md is a + // real gap even alongside version numbers and IPs that must NOT be mistaken for paths. + const s = skill( + 'x', + 'see `1.2.3` version and `references/missing.md` file and `127.0.0.1` IP', + ['references/guide.md'], + ); + const f = checkSiblingPaths([s]); + expect(f).toHaveLength(1); + expect(f[0]!.severity).toBe('warn'); + expect(f[0]!.message).toContain('references/missing.md'); + }); +}); + +describe('checkMemoryLinks', () => { + it('is quiet when every [[link]] resolves to an included memory file', () => { + expect(checkMemoryLinks('see [[alpha]] and [[beta]]', ['alpha.md', 'beta.md'])).toEqual([]); + }); + + it('warns for a dangling link', () => { + const f = checkMemoryLinks('see [[gone]]', ['alpha.md']); + expect(f[0]!.severity).toBe('warn'); + expect(f[0]!.code).toBe('dangling_memory_link'); + }); + + it('is quiet when there is no memory index at all', () => { + expect(checkMemoryLinks(undefined, [])).toEqual([]); + }); + + it('handles markdown links [Title](file.md) when file is present', () => { + expect(checkMemoryLinks('- [Alpha](alpha.md) — reference', ['alpha.md'])).toEqual([]); + }); + + it('warns for dangling markdown links', () => { + const f = checkMemoryLinks('- [Missing](alpha.md)', ['beta.md']); + expect(f).toHaveLength(1); + expect(f[0]!.code).toBe('dangling_memory_link'); + }); + + it('handles wikilinks with |alias suffix', () => { + expect(checkMemoryLinks('see [[alpha|Alpha Notes]] in the docs', ['alpha.md'])).toEqual([]); + }); + + it('handles wikilinks with path prefix', () => { + expect( + checkMemoryLinks('see [[notes/alpha]] and [[beta]] in memory', ['alpha.md', 'beta.md']), + ).toEqual([]); + }); + + it('mixes markdown and wikilinks', () => { + const index = '- [Alpha](alpha.md)\n- [[beta|Beta Title]]\n- [[notes/gamma]]'; + expect(checkMemoryLinks(index, ['alpha.md', 'beta.md', 'gamma.md'])).toEqual([]); + }); + + it('warns for any dangling form in a mixed index', () => { + const index = '- [Alpha](alpha.md)\n- [[gone]]'; + const f = checkMemoryLinks(index, ['alpha.md']); + expect(f).toHaveLength(1); + expect(f[0]!.message).toContain('gone'); + }); + + it('ignores external URLs in markdown links', () => { + expect(checkMemoryLinks('[doc](https://example.com/file.md)', [])).toEqual([]); + }); + + it('ignores relative paths outside memory in markdown links', () => { + expect(checkMemoryLinks('[x](../elsewhere/x.md)', ['alpha.md'])).toEqual([]); + }); + + it('still catches dangling local markdown links after filtering non-local', () => { + // Present: should be quiet + expect(checkMemoryLinks('[t](alpha.md)', ['alpha.md'])).toEqual([]); + // Absent: should warn + const f = checkMemoryLinks('[t](alpha.md)', ['beta.md']); + expect(f).toHaveLength(1); + expect(f[0]!.code).toBe('dangling_memory_link'); + }); + + it('ignores wikilinks with .. path segments', () => { + expect(checkMemoryLinks('see [[../outside/x]] in backlinks', ['alpha.md'])).toEqual([]); + }); +}); + +describe('checkBinaries', () => { + it('warns, and never errors, for a binary absent from the sandbox inventory', () => { + const f = checkBinaries(['gh', 'kubectl'], ['kubectl']); + expect(f).toHaveLength(1); + expect(f[0]!.code).toBe('missing_binary'); + expect(f[0]!.severity).toBe('warn'); + expect(f[0]!.message).toContain('gh'); + }); + + it('is quiet when every binary is present', () => { + expect(checkBinaries(['gh'], ['gh', 'kubectl'])).toEqual([]); + }); + + it('warns (not errors) when no inventory is available to check against', () => { + const f = checkBinaries(['gh'], undefined); + expect(f[0]!.severity).toBe('warn'); + expect(f[0]!.code).toBe('inventory_unavailable'); + }); +}); + +describe('checkEntry', () => { + it('errors when the entry prompt is not in the bundle', () => { + expect(checkEntry('nope', ['a', 'b'])[0]!.code).toBe('unknown_entry'); + }); + it('is quiet when it is', () => { + expect(checkEntry('a', ['a'])).toEqual([]); + }); +}); + +describe('checkInteraction', () => { + it('warns for each interaction-dependent skill', () => { + const f = checkInteraction({ + travels: [], + dropped: [], + interactionDependent: ['superpowers:brainstorming'], + }); + expect(f[0]!.severity).toBe('warn'); + expect(f[0]!.code).toBe('interaction_dependent'); + }); +}); + +describe('renderPreflight / hasErrors', () => { + it('hasErrors is true only when a finding is an error', () => { + expect(hasErrors([{ severity: 'warn', code: 'w', message: 'm' }])).toBe(false); + expect(hasErrors([{ severity: 'error', code: 'e', message: 'm' }])).toBe(true); + }); + + it('renders findings grouped by severity, and states its own limits', () => { + const out = renderPreflight([ + { severity: 'error', code: 'missing_binary', message: 'gh missing' }, + { severity: 'warn', code: 'interaction_dependent', message: 'brainstorming' }, + ]); + expect(out).toContain('error'); + expect(out).toContain('gh missing'); + expect(out).toContain('warn'); + // The honesty requirement of spec §4.6: never imply completeness. + expect(out.toLowerCase()).toContain('cannot be checked locally'); + }); + + it('says so plainly when there is nothing to report', () => { + expect(renderPreflight([]).toLowerCase()).toContain('no findings'); + }); +}); + +describe('checkMemoryLinks ReDoS resistance', () => { + // CodeQL flagged both link regexes as polynomial on uncontrolled data, and it was right: + // measured on the unbounded forms, 40 KB of '[' took 2281 ms and 80 KB of '[[' took 9186 ms, + // growing quadratically -- a crafted MEMORY.md in a third-party plugin skill would hang the + // promote CLI for minutes. Bounded, the same inputs take 33 ms and 62 ms. + // The budget is ~25x the fixed cost and ~1/6 of the broken cost, so it cannot pass unfixed. + it('does not blow up on pathological bracket runs', () => { + for (const evil of ['['.repeat(40000), '[['.repeat(40000), '[](' + '[(](('.repeat(8000)]) { + const t = performance.now(); + checkMemoryLinks(evil, ['a.md']); + expect(performance.now() - t).toBeLessThan(1500); + } + }); +}); diff --git a/packages/config-bundle/test/resolve.test.ts b/packages/config-bundle/test/resolve.test.ts new file mode 100644 index 0000000..7a70dd1 --- /dev/null +++ b/packages/config-bundle/test/resolve.test.ts @@ -0,0 +1,251 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { mkdtempSync, mkdirSync, writeFileSync, symlinkSync, rmSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; +import { resolveSkills, readSkillFrontmatterName } from '../src/resolve.js'; + +let root: string; + +function skill(dir: string, name: string, extra: Record = {}): void { + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'SKILL.md'), + `---\nname: ${name}\ndescription: does ${name}\n---\n\nbody of ${name}\n`, + ); + for (const [rel, body] of Object.entries(extra)) { + const p = join(dir, rel); + mkdirSync(join(p, '..'), { recursive: true }); + writeFileSync(p, body); + } +} + +beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'cb-resolve-')); +}); +afterEach(() => { + rmSync(root, { recursive: true, force: true }); +}); + +describe('readSkillFrontmatterName', () => { + it('reads the frontmatter name', () => { + expect(readSkillFrontmatterName('---\nname: foo\n---\nbody')).toBe('foo'); + }); + it('returns null when there is no frontmatter', () => { + expect(readSkillFrontmatterName('# just a heading')).toBeNull(); + }); + it('strips surrounding double quotes', () => { + expect(readSkillFrontmatterName('---\nname: "foo"\n---\nbody')).toBe('foo'); + }); + it('strips surrounding single quotes', () => { + expect(readSkillFrontmatterName("---\nname: 'foo'\n---\nbody")).toBe('foo'); + }); + it('leaves unquoted names untouched', () => { + expect(readSkillFrontmatterName('---\nname: foo\n---\nbody')).toBe('foo'); + }); +}); + +describe('resolveSkills', () => { + it('finds a skill by its SKILL.md and records its sibling files', () => { + skill(join(root, 'user', 'skills', 'alpha'), 'alpha', { + 'references/guide.md': 'g', + 'run.sh': '#!/bin/sh\n', + }); + const found = resolveSkills({ userDir: join(root, 'user') }); + expect(found).toHaveLength(1); + expect(found[0]!.name).toBe('alpha'); + expect(found[0]!.scope).toBe('user'); + expect(found[0]!.files.sort()).toEqual(['SKILL.md', 'references/guide.md', 'run.sh']); + }); + + it('dedupes the same skill appearing in plugins/cache and plugins/marketplaces', () => { + skill(join(root, 'plugins', 'marketplaces', 'mp', 'skills', 'dup'), 'dup'); + skill(join(root, 'plugins', 'cache', 'mp', 'skills', 'dup'), 'dup'); + const found = resolveSkills({ pluginDirs: [join(root, 'plugins')] }); + expect(found.map((s) => s.name)).toEqual(['dup']); + }); + + it('prefers project scope over user scope for the same name', () => { + skill(join(root, 'user', 'skills', 'both'), 'both'); + skill(join(root, 'proj', 'skills', 'both'), 'both'); + const found = resolveSkills({ + projectDir: join(root, 'proj'), + userDir: join(root, 'user'), + }); + expect(found).toHaveLength(1); + expect(found[0]!.scope).toBe('project'); + }); + + it('does not recurse below a directory that already holds a SKILL.md', () => { + skill(join(root, 'user', 'skills', 'outer'), 'outer'); + skill(join(root, 'user', 'skills', 'outer', 'nested'), 'nested'); + const found = resolveSkills({ userDir: join(root, 'user') }); + expect(found.map((s) => s.name)).toEqual(['outer']); + // the nested SKILL.md is carried as a plain file of `outer`, not as its own skill + expect(found[0]!.files).toContain('nested/SKILL.md'); + }); + + it('falls back to the directory name when frontmatter has no name', () => { + const dir = join(root, 'user', 'skills', 'unnamed'); + mkdirSync(dir, { recursive: true }); + writeFileSync(join(dir, 'SKILL.md'), 'no frontmatter here'); + expect(resolveSkills({ userDir: join(root, 'user') })[0]!.name).toBe('unnamed'); + }); + + it('returns an empty list for roots that do not exist', () => { + expect(resolveSkills({ userDir: join(root, 'nope') })).toEqual([]); + }); + + it('skips dangling symlinks without throwing', () => { + skill(join(root, 'user', 'skills', 'real'), 'real'); + // Create a dangling symlink inside the skill directory + const skillDir = join(root, 'user', 'skills', 'real'); + symlinkSync(join(root, 'nonexistent'), join(skillDir, 'dangling')); + const found = resolveSkills({ userDir: join(root, 'user') }); + expect(found).toHaveLength(1); + expect(found[0]!.name).toBe('real'); + // dangling symlink should not appear in files + expect(found[0]!.files).not.toContain('dangling'); + }); + + it('does not hang on symlink cycles', () => { + skill(join(root, 'user', 'skills', 'real'), 'real'); + const skillDir = join(root, 'user', 'skills', 'real'); + const subdir = join(skillDir, 'subdir'); + mkdirSync(subdir); + // Create a symlink cycle: subdir -> skill dir (ancestor) + symlinkSync(skillDir, join(subdir, 'cycle')); + const found = resolveSkills({ userDir: join(root, 'user') }); + expect(found).toHaveLength(1); + expect(found[0]!.name).toBe('real'); + }); + + it('/skills that is a symlink to a real directory is discovered', () => { + const realSkillsDir = join(root, 'real-skills'); + skill(join(realSkillsDir, 'linked'), 'linked'); + const userSkillsLink = join(root, 'user', 'skills'); + mkdirSync(join(root, 'user')); + symlinkSync(realSkillsDir, userSkillsLink); + const found = resolveSkills({ userDir: join(root, 'user') }); + expect(found).toHaveLength(1); + expect(found[0]!.name).toBe('linked'); + }); + + it('a pluginDirs entry that is itself a symlink is discovered', () => { + const realPluginDir = join(root, 'real-plugins'); + skill(join(realPluginDir, 'mp', 'skills', 'plugin-skill'), 'plugin-skill'); + const pluginLink = join(root, 'plugins-link'); + symlinkSync(realPluginDir, pluginLink); + const found = resolveSkills({ pluginDirs: [pluginLink] }); + expect(found).toHaveLength(1); + expect(found[0]!.name).toBe('plugin-skill'); + }); + + it('an individual skill directory that is a symlink is discovered', () => { + const realSkill = join(root, 'real-skill'); + skill(realSkill, 'symlinked-skill'); + const skillsDir = join(root, 'user', 'skills'); + mkdirSync(skillsDir, { recursive: true }); + symlinkSync(realSkill, join(skillsDir, 'linked')); + const found = resolveSkills({ userDir: join(root, 'user') }); + expect(found).toHaveLength(1); + expect(found[0]!.name).toBe('symlinked-skill'); + }); + + it('dedupes skills that canonicalize to the same real path', () => { + const realSkill = join(root, 'the-real-skill'); + skill(realSkill, 'canonical'); + const skillsDir = join(root, 'user', 'skills'); + mkdirSync(skillsDir, { recursive: true }); + // Create two entries: one direct, one via symlink alias + symlinkSync(realSkill, join(skillsDir, 'direct-link')); + symlinkSync(realSkill, join(skillsDir, 'alias-link')); + const found = resolveSkills({ userDir: join(root, 'user') }); + // Should dedupe to one skill (the same canonical path) + expect(found).toHaveLength(1); + expect(found[0]!.name).toBe('canonical'); + }); + + it('a symlink escaping the skill directory is skipped and raises a warn finding', () => { + const outside = join(root, 'outside'); + mkdirSync(outside, { recursive: true }); + writeFileSync(join(outside, 'secret.txt'), 'secret'); + const skillDir = join(root, 'user', 'skills', 'escapee'); + skill(skillDir, 'escapee'); + symlinkSync(outside, join(skillDir, 'escape')); + + const found = resolveSkills({ userDir: join(root, 'user') }); + expect(found).toHaveLength(1); + // the escaping symlink's content must not ride along into the bundle + expect(found[0]!.files).not.toContain('escape/secret.txt'); + expect(found[0]!.files.some((f) => f.startsWith('escape/'))).toBe(false); + + const findings = found[0]!.findings ?? []; + expect(findings).toHaveLength(1); + expect(findings[0]!.severity).toBe('warn'); + expect(findings[0]!.code).toBe('skill_symlink_escaped'); + expect(findings[0]!.message).toContain('escape'); + }); + + it('a symlink pointing within the skill directory is still followed, and raises no finding', () => { + const skillDir = join(root, 'user', 'skills', 'aliased'); + skill(skillDir, 'aliased'); + mkdirSync(join(skillDir, 'real')); + writeFileSync(join(skillDir, 'real', 'inner.md'), 'inner'); + // alias -> real, both inside the skill directory + symlinkSync(join(skillDir, 'real'), join(skillDir, 'alias')); + + const found = resolveSkills({ userDir: join(root, 'user') }); + expect(found).toHaveLength(1); + expect(found[0]!.files).toContain('alias/inner.md'); + expect(found[0]!.findings ?? []).toEqual([]); + }); + + it('a symlink to a same-prefix sibling ("skillsX" vs "skills") is treated as escaped, not inside', () => { + // The containment check must be path-segment-aware (relative(), not a bare startsWith): + // 'X' is a STRING prefix match of '' but is not actually inside it. + const skillDir = join(root, 'user', 'skills', 'trap'); + skill(skillDir, 'trap'); + const sibling = join(root, 'user', 'skills', 'trapX'); + mkdirSync(sibling, { recursive: true }); + writeFileSync(join(sibling, 'leak.txt'), 'leak'); + symlinkSync(sibling, join(skillDir, 'link')); + + const found = resolveSkills({ userDir: join(root, 'user') }); + expect(found).toHaveLength(1); + expect(found[0]!.files.some((f) => f.startsWith('link/'))).toBe(false); + const findings = found[0]!.findings ?? []; + expect(findings).toHaveLength(1); + expect(findings[0]!.code).toBe('skill_symlink_escaped'); + }); + + it('dedupes an aliased directory across two scopes even when the fallback names differ', () => { + // NOTE ON COVERAGE: this exercises the overall dedupe-by-canonical-path guarantee (exactly + // one entry survives, under the winning scope's name), which the maintainer's fix targets. + // It does NOT, however, fail if `best.delete(prevCanonical.name)` in resolve.ts's dedupe loop + // is reverted: resolveSkills() always processes roots in strict project -> user -> plugin + // (precedence-ascending) order, so a later-processed entry can never have STRICTLY better + // precedence than an earlier-recorded one for the same canonical path -- the `if` branch that + // guards the delete is therefore unreachable via this (or any) two-scope alias through the + // public API today, and removing the delete line was verified (by hand, outside this suite) + // to produce byte-identical output for this exact scenario. Kept as defense-in-depth per the + // review comment; documented here rather than claimed as a regression test for that line. + const realSkill = join(root, 'real-skill'); + mkdirSync(realSkill, { recursive: true }); + writeFileSync(join(realSkill, 'SKILL.md'), 'no frontmatter name here'); + + const projSkills = join(root, 'proj', 'skills'); + const userSkills = join(root, 'user', 'skills'); + mkdirSync(projSkills, { recursive: true }); + mkdirSync(userSkills, { recursive: true }); + symlinkSync(realSkill, join(projSkills, 'project-alias')); + symlinkSync(realSkill, join(userSkills, 'user-alias')); + + const found = resolveSkills({ + projectDir: join(root, 'proj'), + userDir: join(root, 'user'), + }); + expect(found).toHaveLength(1); + expect(found[0]!.name).toBe('project-alias'); + expect(found[0]!.scope).toBe('project'); + }); +}); diff --git a/packages/config-bundle/test/secret-scan.test.ts b/packages/config-bundle/test/secret-scan.test.ts new file mode 100644 index 0000000..b5ade6a --- /dev/null +++ b/packages/config-bundle/test/secret-scan.test.ts @@ -0,0 +1,109 @@ +import { describe, it, expect } from 'vitest'; +import { blockingSecrets, scanEntriesForSecrets, SecretScanError } from '../src/secret-scan.js'; + +const e = (path: string, body: string) => ({ path, content: Buffer.from(body) }); + +describe('scanEntriesForSecrets — structural rules BLOCK', () => { + it('is clean for ordinary prose', () => { + expect( + scanEntriesForSecrets([e('skills/a/SKILL.md', 'read the docs\napi keys matter')]), + ).toEqual([]); + }); + + it('blocks an AWS access key id, with path, 1-based line and severity', () => { + const f = scanEntriesForSecrets([e('context/MEMORY.md', 'note\nAKIAIOSFODNN7EXAMPLE\n')]); + expect(f).toEqual([ + { path: 'context/MEMORY.md', line: 2, rule: 'aws-access-key-id', severity: 'blocking' }, + ]); + }); + + it('blocks a private key block', () => { + const f = scanEntriesForSecrets([e('a.pem', '-----BEGIN RSA PRIVATE KEY-----')]); + expect(f[0]).toMatchObject({ rule: 'private-key-block', severity: 'blocking' }); + }); + + it('blocks a GitHub token', () => { + const f = scanEntriesForSecrets([e('x.md', `ghp_${'a'.repeat(36)}`)]); + expect(f[0]).toMatchObject({ rule: 'github-token', severity: 'blocking' }); + }); + + it('reports every blocking hit, not just the first', () => { + const f = scanEntriesForSecrets([ + e('a.md', 'AKIAIOSFODNN7EXAMPLE'), + e('b.md', `ghp_${'b'.repeat(36)}`), + ]); + expect(f.map((x) => x.path)).toEqual(['a.md', 'b.md']); + expect(f.every((x) => x.severity === 'blocking')).toBe(true); + }); + + it('skips binary-looking entries rather than emitting noise', () => { + expect(scanEntriesForSecrets([{ path: 'x.png', content: Buffer.from([0, 1, 2, 0]) }])).toEqual( + [], + ); + }); +}); + +describe('scanEntriesForSecrets — heuristic rules WARN, never block', () => { + // These cases are why the heuristic is not blocking. Measured against a real ~/.claude, the + // heuristic produced 11 hits and every one was a false positive; the structural rules produced + // none. A blocking heuristic would refuse essentially every promotion. + it('warns, not blocks, on an assigned api key', () => { + const f = scanEntriesForSecrets([e('x.md', 'api_key = "s3cr3t-value-long-enough"')]); + expect(f).toHaveLength(1); + expect(f[0]).toMatchObject({ rule: 'assigned-secret', severity: 'warning' }); + }); + + it('does not fire on the bare phrase', () => { + expect(scanEntriesForSecrets([e('x.md', 'the api key is rotated monthly')])).toEqual([]); + }); + + it('warns, not blocks, on an Authorization bearer header', () => { + const f = scanEntriesForSecrets([e('x.md', 'Authorization: Bearer abcdef0123456789abcdef')]); + expect(f[0]).toMatchObject({ rule: 'bearer-token', severity: 'warning' }); + }); + + it('suppresses documentation placeholders entirely', () => { + for (const body of [ + 'api_key = "your-api-key-here"', + 'apiKey: ""', + 'password = "changeme-please"', + 'token = "example-token-value"', + ]) { + expect(scanEntriesForSecrets([e('doc.md', body)])).toEqual([]); + } + }); + + it('warns rather than blocks on code that merely looks like an assignment', () => { + // Real cases measured in installed skills: the value char class accepts dotted identifiers. + for (const body of ['const TOKEN = crypto.randomUUID', 'apiKey = process.env.ANTHROPIC_KEY']) { + const f = scanEntriesForSecrets([e('scripts/server.cjs', body)]); + expect(f.every((x) => x.severity === 'warning')).toBe(true); + } + }); +}); + +describe('blockingSecrets', () => { + it('selects only the blocking subset', () => { + const f = scanEntriesForSecrets([ + e('a.md', 'AKIAIOSFODNN7EXAMPLE'), + e('b.md', 'api_key = "s3cr3t-value-long-enough"'), + ]); + expect(f).toHaveLength(2); + expect(blockingSecrets(f).map((x) => x.rule)).toEqual(['aws-access-key-id']); + }); + + it('is empty when only warnings are present, so promotion may proceed', () => { + const f = scanEntriesForSecrets([e('b.md', 'api_key = "s3cr3t-value-long-enough"')]); + expect(blockingSecrets(f)).toEqual([]); + }); +}); + +describe('SecretScanError', () => { + it('carries the findings and names the first path in its message', () => { + const err = new SecretScanError([ + { path: 'context/MEMORY.md', line: 2, rule: 'r', severity: 'blocking' }, + ]); + expect(err.findings).toHaveLength(1); + expect(err.message).toContain('context/MEMORY.md:2'); + }); +}); diff --git a/packages/config-bundle/test/tar.test.ts b/packages/config-bundle/test/tar.test.ts new file mode 100644 index 0000000..394da3b --- /dev/null +++ b/packages/config-bundle/test/tar.test.ts @@ -0,0 +1,142 @@ +import { describe, it, expect } from 'vitest'; +import { + canonicalTar, + untar, + digestOf, + MAX_USTAR_PATH, + assertValidDigest, + InvalidDigestError, + digestDirName, +} from '../src/tar.js'; + +const e = (path: string, body: string, mode?: number) => ({ + path, + content: Buffer.from(body), + ...(mode === undefined ? {} : { mode }), +}); + +describe('canonicalTar', () => { + it('is byte-identical regardless of input entry order', () => { + const a = canonicalTar([e('b/two.md', 'two'), e('a/one.md', 'one')]); + const b = canonicalTar([e('a/one.md', 'one'), e('b/two.md', 'two')]); + expect(a.equals(b)).toBe(true); + }); + + it('embeds no mtime, uid or gid (the fields that make tars non-reproducible)', () => { + const t = canonicalTar([e('a.md', 'x')]); + // header field offsets: uid 108(8), gid 116(8), mtime 136(12) + expect(t.subarray(108, 116).toString('ascii')).toBe('0000000\0'); + expect(t.subarray(116, 124).toString('ascii')).toBe('0000000\0'); + expect(t.subarray(136, 148).toString('ascii')).toBe('00000000000\0'); + }); + + it('pads each entry to a 512-byte boundary and ends with two zero blocks', () => { + const t = canonicalTar([e('a.md', 'x')]); + expect(t.length % 512).toBe(0); + expect(t.subarray(t.length - 1024).every((b) => b === 0)).toBe(true); + }); + + it('writes a valid USTAR magic and checksum', () => { + const t = canonicalTar([e('a.md', 'x')]); + expect(t.subarray(257, 263).toString('ascii')).toBe('ustar\0'); + let sum = 0; + for (let i = 0; i < 512; i++) sum += i >= 148 && i < 156 ? 0x20 : t[i]!; + expect(parseInt(t.subarray(148, 154).toString('ascii'), 8)).toBe(sum); + }); + + it('rejects a path too long for USTAR instead of silently truncating', () => { + const long = 'x'.repeat(MAX_USTAR_PATH + 1); + expect(() => canonicalTar([e(long, 'x')])).toThrow(/too long for USTAR/); + }); + + it('splits a long-but-legal path across prefix and name', () => { + const deep = 'a'.repeat(120) + '/' + 'b'.repeat(80); + const t = canonicalTar([e(deep, 'x')]); + expect(untar(t)[0]!.path).toBe(deep); + }); +}); + +describe('untar', () => { + it('round-trips entries, contents and modes', () => { + const entries = [e('skills/x/SKILL.md', '# x'), e('exec/run.sh', '#!/bin/sh\n', 0o755)]; + const back = untar(canonicalTar(entries)); + expect(back.map((x) => x.path)).toEqual(['exec/run.sh', 'skills/x/SKILL.md']); + expect(back.find((x) => x.path === 'exec/run.sh')!.mode).toBe(0o755); + expect(back.find((x) => x.path === 'skills/x/SKILL.md')!.content.toString()).toBe('# x'); + }); + + it('handles content whose length is an exact multiple of 512', () => { + const body = 'y'.repeat(1024); + expect(untar(canonicalTar([e('a.md', body)]))[0]!.content.toString()).toBe(body); + }); +}); + +describe('digestOf', () => { + it('is sha256:<64 hex> and stable for identical trees', () => { + const d = digestOf(canonicalTar([e('a.md', 'x')])); + expect(d).toMatch(/^sha256:[0-9a-f]{64}$/); + expect(digestOf(canonicalTar([e('a.md', 'x')]))).toBe(d); + }); + + it('changes when any content changes', () => { + const a = digestOf(canonicalTar([e('a.md', 'x')])); + const b = digestOf(canonicalTar([e('a.md', 'y')])); + expect(a).not.toBe(b); + }); +}); + +describe('assertValidDigest', () => { + it('passes through a well-formed digest unchanged', () => { + const good = 'sha256:' + '0123456789abcdef'.repeat(4); + expect(assertValidDigest(good)).toBe(good); + }); + + it('rejects a path-traversal payload disguised as a digest', () => { + expect(() => assertValidDigest('sha256:../../../evil')).toThrow(InvalidDigestError); + }); + + it('rejects hex of the wrong length', () => { + expect(() => assertValidDigest('sha256:' + 'a'.repeat(63))).toThrow(InvalidDigestError); + expect(() => assertValidDigest('sha256:' + 'a'.repeat(65))).toThrow(InvalidDigestError); + }); + + it('rejects uppercase hex', () => { + expect(() => assertValidDigest('sha256:' + 'A'.repeat(64))).toThrow(InvalidDigestError); + }); + + it('rejects a digest missing the sha256: prefix', () => { + expect(() => assertValidDigest('a'.repeat(64))).toThrow(InvalidDigestError); + }); + + it('rejects the empty string', () => { + expect(() => assertValidDigest('')).toThrow(InvalidDigestError); + }); + + it('caps how much of a long bad value the error message echoes', () => { + // The message must name the bad value's SHAPE without echoing an unbounded attacker string + // (a long or crafted digest could otherwise ride, unbounded, into logs or a rendered error). + const long = 'sha256:' + 'z'.repeat(5000); + try { + assertValidDigest(long); + throw new Error('expected assertValidDigest to throw'); + } catch (err) { + expect(err).toBeInstanceOf(InvalidDigestError); + const message = (err as Error).message; + expect(message.length).toBeLessThan(200); + expect(message).not.toContain('z'.repeat(5000)); + } + }); +}); + +describe('digestDirName', () => { + it('replaces the colon in a normal digest', () => { + expect(digestDirName('sha256:' + 'a'.repeat(64))).toBe('sha256-' + 'a'.repeat(64)); + }); + + it('replaces every colon, not just the first (replaceAll, not replace)', () => { + // A single `.replace(':', '-')` only swaps the first ':' and silently leaves the rest, so + // 'a:b:c' would become 'a-b:c' instead of 'a-b-c' -- this fails if replaceAll is reverted to + // replace. + expect(digestDirName('a:b:c')).toBe('a-b-c'); + }); +}); diff --git a/packages/config-bundle/tsconfig.json b/packages/config-bundle/tsconfig.json new file mode 100644 index 0000000..b520bb6 --- /dev/null +++ b/packages/config-bundle/tsconfig.json @@ -0,0 +1,12 @@ +{ + "compilerOptions": { + "target": "ES2022", + "module": "NodeNext", + "moduleResolution": "NodeNext", + "strict": true, + "noEmit": true, + "types": ["node"], + "skipLibCheck": true + }, + "include": ["src/**/*.ts", "test/**/*.ts"] +} diff --git a/packages/config-bundle/vitest.config.ts b/packages/config-bundle/vitest.config.ts new file mode 100644 index 0000000..e1e28f8 --- /dev/null +++ b/packages/config-bundle/vitest.config.ts @@ -0,0 +1,5 @@ +import { defineConfig } from 'vitest/config'; + +export default defineConfig({ + test: { include: ['test/**/*.test.ts'] }, +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 0e3af87..6c19687 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -48,6 +48,9 @@ importers: '@grpc/grpc-js': specifier: ^1.12.0 version: 1.14.4 + '@sh/config-bundle': + specifier: workspace:* + version: link:../packages/config-bundle '@sh/k8s-sandbox': specifier: workspace:* version: link:../packages/k8s-sandbox @@ -71,6 +74,18 @@ importers: specifier: ^2.0.0 version: 2.1.9(@types/node@22.19.21) + packages/config-bundle: + devDependencies: + '@types/node': + specifier: ^22.0.0 + version: 22.19.21 + typescript: + specifier: ^5.5.0 + version: 5.9.3 + vitest: + specifier: ^2.0.0 + version: 2.1.9(@types/node@22.19.21) + packages/ibac-stub: devDependencies: '@types/node':