diff --git a/AGENTS.md b/AGENTS.md index 1f0e5951..7e840b3f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -629,7 +629,7 @@ mock.module("./some-module", () => ({ ### Architecture -* **CLI telemetry DSN is public write-only — safe to embed in install script**: The CLI's own Sentry DSN (\`SENTRY\_CLI\_DSN\` in \`src/lib/constants.ts\`) is a public write-only ingest key (\`1188a86f...@o1.ingest.us.sentry.io/4510776311808000\`). It's already baked into every distributed binary. Safe to hardcode in the bash install script for error reporting via the envelope API — no secrets needed. Opt-out via \`SENTRY\_CLI\_NO\_TELEMETRY=1\` (same env var the binary checks). Envelope endpoint: \`https://o1.ingest.us.sentry.io/api/{PROJECT\_ID}/envelope/\` with \`x-sentry-auth\` header containing the public key. +* **CLI telemetry DSN is public write-only — safe to embed in install script**: The CLI's Sentry DSN (\`SENTRY\_CLI\_DSN\` in \`src/lib/constants.ts\`) is a public write-only ingest key already baked into every binary. Safe to hardcode in install scripts. Opt-out: \`SENTRY\_CLI\_NO\_TELEMETRY=1\`. * **cli.sentry.dev is served from gh-pages branch via GitHub Pages**: \`cli.sentry.dev\` is served from gh-pages branch via GitHub Pages. Craft's gh-pages target runs \`git rm -r -f .\` before extracting docs — persist extra files via \`postReleaseCommand\` in \`.craft.yml\`. Install script supports \`--channel nightly\`, downloading from the \`nightly\` release tag directly. version.json is only used by upgrade/version-check flow. @@ -669,13 +669,13 @@ mock.module("./some-module", () => ({ * **Bun binary build requires SENTRY\_CLIENT\_ID env var**: The build script (\`script/bundle.ts\`) requires \`SENTRY\_CLIENT\_ID\` environment variable and exits with code 1 if missing. When building locally, use \`bun run --env-file=.env.local build\` or set the env var explicitly. The binary build (\`bun run build\`) also needs it. Without it you get: \`Error: SENTRY\_CLIENT\_ID environment variable is required.\` -* **GitHub immutable releases prevent rolling nightly tag pattern**: getsentry/cli has immutable GitHub releases — assets can't be modified and tags can NEVER be reused. Nightly uses per-version tags (e.g., \`0.13.0-dev.1772062077\`) with API-based latest discovery; deletes all existing assets before uploading. Craft minVersion >= 2.21.0 with no \`preReleaseCommand\` silently skips \`bump-version.sh\` if the only target is \`github\`. Fix: explicitly set \`preReleaseCommand: bash scripts/bump-version.sh\`. +* **GitHub immutable releases prevent rolling nightly tag pattern**: getsentry/cli has immutable GitHub releases — assets can't be modified and tags can NEVER be reused. Nightly uses per-version tags (e.g., \`0.13.0-dev.1772062077\`) with API-based latest discovery. Craft with no \`preReleaseCommand\` silently skips \`bump-version.sh\` if the only target is \`github\` — must explicitly set it. -* **Install script: BSD sed and awk JSON parsing breaks OCI digest extraction**: The install script parses OCI manifests with awk (no jq dependency). Key trap: \`sed 's/},{/}\n{/g'\` doesn't insert newlines on macOS BSD sed (\`\n\` is literal). Also, the first layer shares a line with the config block after \`\[{\` split. Fix: use a single awk pass tracking last-seen \`"digest"\` value, printing it when \`"org.opencontainers.image.title"\` matches target. Works because \`digest\` always precedes \`annotations\` within each OCI layer object. This avoids sed entirely and handles both GNU/BSD awk. The config digest (\`sha256:44136fa...\`) is a 2-byte \`{}\` blob — downloading it instead of the real binary causes \`gunzip: unexpected end of file\`. The install script now has fire-and-forget Sentry telemetry via \`die()\` + ERR trap, which would catch such failures automatically. +* **Install script: BSD sed and awk JSON parsing breaks OCI digest extraction**: The install script parses OCI manifests with awk (no jq). Key trap: BSD sed \`\n\` is literal, not newline. Fix: single awk pass tracking last-seen \`"digest"\`, printing when \`"org.opencontainers.image.title"\` matches target. The config digest (\`sha256:44136fa...\`) is a 2-byte \`{}\` blob — downloading it instead of the real binary causes \`gunzip: unexpected end of file\`. -* **macOS SIGKILL on MAP\_SHARED mmap of signed Mach-O binaries**: macOS AMFI (code signing enforcement) sends SIGKILL when \`MAP\_SHARED\` with \`PROT\_WRITE\` is used on a code-signed Mach-O binary. \`Bun.mmap()\` defaults to \`{ shared: true }\` (MAP\_SHARED). In \`src/lib/bspatch.ts\`, \`Bun.mmap(process.execPath)\` kills the process on macOS during delta upgrades because the running CLI binary is ad-hoc signed (all Bun binaries are). Fix: pass \`{ shared: false }\` for MAP\_PRIVATE. Since the mapping is read-only in practice, no COW pages are allocated — identical performance. Linux ELF binaries have no such restriction. +* **macOS SIGKILL on MAP\_SHARED mmap of signed Mach-O binaries**: macOS AMFI (code signing enforcement) sends uncatchable SIGKILL when \`Bun.mmap()\` is used on code-signed Mach-O binaries. \`Bun.mmap()\` always requests PROT\_WRITE regardless of the \`shared\` flag, and macOS rejects ANY writable mapping (MAP\_SHARED or MAP\_PRIVATE) on signed Mach-O. PR #339's MAP\_PRIVATE fix was insufficient. Fixed by replacing \`Bun.mmap(oldPath)\` with \`new Uint8Array(await Bun.file(oldPath).arrayBuffer())\` in bspatch.ts. Costs ~100 MB heap for the old binary but avoids the uncatchable SIGKILL entirely. Sentry telemetry confirmed: zero successful macOS upgrade traces from delta-enabled versions, while Linux worked fine. * **Multiple mockFetch calls replace each other — use unified mocks for multi-endpoint tests**: Bun test mocking gotchas: (1) \`mockFetch()\` replaces \`globalThis.fetch\` — calling it twice replaces the first mock. Use a single unified fetch mock dispatching by URL pattern. (2) \`mock.module()\` pollutes the module registry for ALL subsequent test files. Tests using it must live in \`test/isolated/\` and run via \`test:isolated\`. (3) For \`Bun.spawn\`, use direct property assignment in \`beforeEach\`/\`afterEach\`. @@ -688,9 +688,6 @@ mock.module("./some-module", () => ({ ### Pattern - -* **Markdown table structure for marked-terminal: blank header row + separator + data rows**: Markdown tables for marked-terminal: blank header row (\`| | |\`), separator (\`|---|---|\`), then data rows (\`| \*\*Label\*\* | value |\`). Data rows before separator produce malformed output. Escape user content via \`escapeMarkdownCell()\` in \`src/lib/formatters/markdown.ts\` — backslashes first, then pipes. CodeQL flags incomplete escaping as high severity. - * **Org-scoped SDK calls follow getOrgSdkConfig + unwrapResult pattern**: All org-scoped API calls in src/lib/api-client.ts: (1) call \`getOrgSdkConfig(orgSlug)\` for regional URL + SDK config, (2) spread into SDK function: \`{ ...config, path: { organization\_id\_or\_slug: orgSlug, ... } }\`, (3) pass to \`unwrapResult(result, errorContext)\`. Shared helpers \`resolveAllTargets\`/\`resolveOrgAndProject\` must NOT call \`fetchProjectId\` — commands that need it enrich targets themselves. @@ -703,5 +700,5 @@ mock.module("./some-module", () => ({ ### Preference -* **CI scripts: prefer jq/sed over node -e for JSON manipulation**: Reviewer (BYK) prefers using standard Unix tools (\`jq\`, \`sed\`, \`awk\`) over \`node -e\` for simple JSON manipulation in CI workflow scripts. For example, reading/modifying package.json version: \`jq -r .version package.json\` to read, \`jq --arg v "$NEW" '.version = $v' package.json > tmp && mv tmp package.json\` to write. This avoids requiring Node.js to be installed in CI steps that only need basic JSON operations, and is more readable for shell-centric workflows. +* **CI scripts: prefer jq/sed over node -e for JSON manipulation**: Prefer \`jq\`/\`sed\`/\`awk\` over \`node -e\` for JSON manipulation in CI scripts. Example: \`jq -r .version package.json\` to read, \`jq --arg v "$NEW" '.version = $v' package.json > tmp && mv tmp package.json\` to write. diff --git a/src/lib/bspatch.ts b/src/lib/bspatch.ts index 73fa03f9..9497b7a4 100644 --- a/src/lib/bspatch.ts +++ b/src/lib/bspatch.ts @@ -5,12 +5,12 @@ * TRDIFF10 format (produced by zig-bsdiff with `--use-zstd`). Designed for * minimal memory usage during CLI self-upgrades: * - * - Old binary: memory-mapped via `Bun.mmap()` (0 JS heap) + * - Old binary: `Bun.mmap()` on Linux (0 JS heap), `arrayBuffer()` on macOS * - Diff/extra blocks: streamed via `DecompressionStream('zstd')` * - Output: written incrementally to disk via `Bun.file().writer()` * - Integrity: SHA-256 computed inline via `Bun.CryptoHasher` * - * Total heap usage: ~1-2 MB regardless of binary size. + * Total heap usage: ~1-2 MB on Linux, ~100 MB on macOS (old file in memory). * * TRDIFF10 format (from zig-bsdiff): * ``` @@ -210,7 +210,7 @@ function createZstdStreamReader(compressed: Uint8Array): BufferedStreamReader { /** * Apply a TRDIFF10 binary patch with streaming I/O for minimal memory usage. * - * Uses `Bun.mmap()` for the old file (0 heap), `DecompressionStream('zstd')` + * Uses `Bun.mmap()` (Linux) or `arrayBuffer()` (macOS) for the old file, `DecompressionStream('zstd')` * for streaming diff/extra blocks (~16 KB buffers), `Bun.file().writer()` * for disk output, and `Bun.CryptoHasher` for inline SHA-256 verification. * @@ -243,13 +243,16 @@ export async function applyPatch( ); const extraReader = createZstdStreamReader(patchData.subarray(extraStart)); - // Memory-map old file: OS manages pages, 0 JS heap allocation. - // MAP_PRIVATE (shared: false) is required on macOS: the kernel's code - // signing enforcement (AMFI) sends SIGKILL when a MAP_SHARED writable - // mapping is created on a code-signed Mach-O binary (which the running - // CLI is). Since we only read from the mapping, no COW pages are ever - // allocated — identical performance to MAP_SHARED. - const oldFile = Bun.mmap(oldPath, { shared: false }); + // On macOS, Bun.mmap() triggers an uncatchable SIGKILL from AMFI code + // signing enforcement — it always requests PROT_WRITE, and macOS rejects + // ANY writable mapping (MAP_SHARED or MAP_PRIVATE) on signed Mach-O + // binaries. Fall back to reading into memory (~100 MB heap, freed after + // patching). On Linux, mmap with MAP_PRIVATE is safe and avoids heap + // allocation entirely (shared: false avoids ETXTBSY on the running binary). + const oldFile = + process.platform === "darwin" + ? new Uint8Array(await Bun.file(oldPath).arrayBuffer()) + : Bun.mmap(oldPath, { shared: false }); // Streaming output: write directly to disk, no output buffer in memory const writer = Bun.file(destPath).writer();