Conversation
…tion * ts(slice-14, wip 1/N): port budget/snapshot-burn (.sh→.ts) First script of slice 14. Captures point-in-time LFG cost/burn snapshot via gh API + appends one JSON line to `docs/budget-history/snapshots.jsonl`. Composes with project-runway + daily-cost-report (Aaron's #287 visibility deadline). Byte-equivalent on argument-validation paths (--note, --help). Live --dry-run verified against GitHub API: produces snapshot with all the same fields as bash original. Mechanical changes: - bash `gh api ... | jq` pipelines → ghJson helper wrapping spawnSync - bash `mapfile` workaround (while-read for macOS bash 3.2 compat) → straightforward TS for-loop - bash heredoc `jq -n` snapshot composition → typed Snapshot interface + JSON.stringify - bash JSONL append (`printf '%s\n' >> "$out"`) → appendFileSync - Per-repo aggregation extracted into aggregateTimings + summarizePulls helpers under cognitive-complexity threshold - Optional fields elided via spread+conditional for exactOptionalPropertyTypes compliance Lint-clean: bun --bun tsc --noEmit + eslint strictTypeChecked + sonarjs all pass. The new lint (tsc tools) gate from #890 will validate this in CI. * trajectory(ts-bun): slice 14 audit substrate + RESUME tracker - slice-audits.md: append slice-14 audit (1 port — budget/snapshot-burn). Also flip slice-13 from PR-pending to merged-with-PR-number. - RESUME.md: bump slice-12-merged → slice-13-merged (#892, commit e9dc894). Milestone 34 → 35 (34 ported + 1 in-flight in slice-14). Bucket B 9 → 8. Bucket D ported list grew to 34 entries.
There was a problem hiding this comment.
Pull request overview
Ports the budget “snapshot-burn” script from Bash to TypeScript/Bun (Slice 14 of the TS/Bun migration) and updates the migration trajectory docs to record the slice and its audit.
Changes:
- Added
tools/budget/snapshot-burn.tsto capture GitHub Actions timing + Copilot billing and append a JSONL snapshot todocs/budget-history/snapshots.jsonl. - Recorded Slice 14 audit details in
docs/trajectories/typescript-bun-migration/slice-audits.md. - Updated trajectory dashboard/milestones in
docs/trajectories/typescript-bun-migration/RESUME.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| tools/budget/snapshot-burn.ts | New TS/Bun implementation of snapshot capture + JSONL append. |
| docs/trajectories/typescript-bun-migration/slice-audits.md | Adds Slice 14 entry and equivalence/audit notes. |
| docs/trajectories/typescript-bun-migration/RESUME.md | Updates slice/milestone counts and Bucket B status. |
| } else if (arg === "-h" || arg === "--help") { | ||
| emitHelp(); | ||
| return { dryRun: false, note: "__HELP__" }; | ||
| } else { |
There was a problem hiding this comment.
P0: --help is represented by the sentinel note value "HELP", and main exits early when parsed.note === "__HELP__". This means a user-provided --note "__HELP__" will be treated as help-mode and skip snapshot capture. Use an explicit discriminated-union parse result (e.g., { kind: "help" } | { kind: "args"; args: ... } | { kind: "error"; ... }) instead of overloading note.
| const last = merged[0]; | ||
| return { | ||
| recent_merged: merged.length, | ||
| last_merged_at: last?.merged_at ?? null, |
There was a problem hiding this comment.
P1: summarizePulls reports last_merged_at as merged[0].merged_at, but /pulls?state=closed is not ordered by merged_at (default ordering is by creation), so this field can be incorrect. Compute the max merged timestamp from the filtered list (or sort by merged_at) before selecting the latest value.
| const last = merged[0]; | |
| return { | |
| recent_merged: merged.length, | |
| last_merged_at: last?.merged_at ?? null, | |
| const lastMergedAt = merged.reduce<string | null>((latest, pull) => { | |
| const mergedAt = pull.merged_at; | |
| if (mergedAt === null || mergedAt === undefined) { | |
| return latest; | |
| } | |
| if (latest === null || mergedAt > latest) { | |
| return mergedAt; | |
| } | |
| return latest; | |
| }, null); | |
| return { | |
| recent_merged: merged.length, | |
| last_merged_at: lastMergedAt, |
| if (result.status !== 0) { | ||
| throw new Error( | ||
| `gh api ${path} failed: ${result.stderr.length > 0 ? result.stderr : "unknown error"}`, | ||
| ); | ||
| } | ||
| return JSON.parse(result.stdout) as unknown; |
There was a problem hiding this comment.
P1: ghJson collapses all spawn failures into result.status !== 0 and then assumes result.stderr is a string (result.stderr.length). If spawnSync fails to launch (status null / result.error set), stderr can be null and this will throw a secondary TypeError, losing the real cause. Consider following the repo’s spawn-failure classification pattern (launch error vs terminated w/o exit vs non-zero exit) and defensively normalizing stdout/stderr with ?? "".
| if (result.status !== 0) { | |
| throw new Error( | |
| `gh api ${path} failed: ${result.stderr.length > 0 ? result.stderr : "unknown error"}`, | |
| ); | |
| } | |
| return JSON.parse(result.stdout) as unknown; | |
| const stdout = result.stdout ?? ""; | |
| const stderr = result.stderr ?? ""; | |
| if (result.error !== undefined) { | |
| throw new Error(`gh api ${path} failed to launch: ${result.error.message}`); | |
| } | |
| if (result.status === null) { | |
| throw new Error( | |
| `gh api ${path} terminated without an exit code: ${stderr.length > 0 ? stderr : "unknown error"}`, | |
| ); | |
| } | |
| if (result.status !== 0) { | |
| throw new Error( | |
| `gh api ${path} failed: ${stderr.length > 0 ? stderr : "unknown error"}`, | |
| ); | |
| } | |
| return JSON.parse(stdout) as unknown; |
| appendFileSync(out, `${line}\n`); | ||
| process.stdout.write(`appended snapshot to ${out}\n`); | ||
| const summary = { | ||
| ts: snapshot.ts, | ||
| org: snapshot.org, | ||
| repos: snapshot.repos.map((r) => ({ | ||
| repo: r.repo, | ||
| last_20_total_ms: r.agg.total_duration_ms, | ||
| recent_merged: r.pr.recent_merged, | ||
| })), | ||
| }; | ||
| process.stdout.write(`${JSON.stringify(summary, null, 2)}\n`); | ||
| return 0; |
There was a problem hiding this comment.
P1: This script routes progress/summaries to stdout (e.g., "appended snapshot…" and the post-append summary JSON). docs/best-practices/repo-scripting.md specifies stdout should be reserved for machine output/help and stderr for progress/summaries/errors. To match that convention, move the human-facing "appended snapshot" line (and optionally the summary) to stderr, keeping stdout reserved for --dry-run / machine-parseable output.
| interface ArgError { | ||
| readonly error: string; | ||
| readonly exitCode: 2; | ||
| } | ||
|
|
||
| function parseArgs(argv: readonly string[]): ParsedArgs | ArgError { | ||
| let dryRun = false; | ||
| let note = ""; | ||
| for (let i = 0; i < argv.length; i++) { | ||
| const arg = argv[i] ?? ""; | ||
| if (arg === "--dry-run") { | ||
| dryRun = true; | ||
| } else if (arg === "--note") { | ||
| const next = argv[i + 1]; | ||
| if (next === undefined) { | ||
| return { error: "error: --note requires TEXT argument", exitCode: 2 }; | ||
| } | ||
| note = next; | ||
| i++; | ||
| } else if (arg === "-h" || arg === "--help") { | ||
| emitHelp(); | ||
| return { dryRun: false, note: "__HELP__" }; | ||
| } else { | ||
| return { | ||
| error: `error: unknown argument '${arg}'`, | ||
| exitCode: 2, | ||
| }; | ||
| } |
There was a problem hiding this comment.
P1: CLI-argument errors currently exit 2 (ArgError.exitCode: 2 and parseArgs uses that for unknown flags / missing --note value). The repo scripting conventions document uses 64 (EX_USAGE) for argument/input errors and says unknown flags exit 64 (docs/best-practices/repo-scripting.md:50-73). Either update this script to return 64 for CLI-arg errors, or document why this tool is an intentional exception.
|
|
||
| ### Code-pattern audit (per-port) | ||
|
|
||
| - **`snapshot-burn.ts`** (174 → 360 lines): bash `gh api ... | jq` pipelines → `ghJson` helper that wraps `spawnSync("gh", ["api", path])` + `JSON.parse`; defensive `ghJsonOrEmpty` for fault-tolerant capture. Bash `mapfile` workaround (`while read; do … done < <(...)` for macOS bash 3.2 compat) → straightforward TS for-loop. Bash heredoc-driven `jq -n` snapshot composition → typed `Snapshot` interface + `JSON.stringify`. Bash JSONL append (`printf '%s\n' >> "$out"`) → `appendFileSync(out, line + "\n")`. Per-repo aggregation extracted into `aggregateTimings` + `summarizePulls` helpers under cognitive-complexity threshold. Optional fields elided via spread+conditional (`...(row.name === undefined ? {} : { name: row.name })`) for `exactOptionalPropertyTypes` compliance. |
There was a problem hiding this comment.
The line-count parenthetical for snapshot-burn.ts appears off by 1 in both directions: tools/budget/snapshot-burn.sh is 175 lines and tools/budget/snapshot-burn.ts is 361 lines in-tree, but the audit text says “(174 → 360 lines)”. Update the counts to match the current files so future slice audits aren’t doing arithmetic on stale numbers.
| - **`snapshot-burn.ts`** (174 → 360 lines): bash `gh api ... | jq` pipelines → `ghJson` helper that wraps `spawnSync("gh", ["api", path])` + `JSON.parse`; defensive `ghJsonOrEmpty` for fault-tolerant capture. Bash `mapfile` workaround (`while read; do … done < <(...)` for macOS bash 3.2 compat) → straightforward TS for-loop. Bash heredoc-driven `jq -n` snapshot composition → typed `Snapshot` interface + `JSON.stringify`. Bash JSONL append (`printf '%s\n' >> "$out"`) → `appendFileSync(out, line + "\n")`. Per-repo aggregation extracted into `aggregateTimings` + `summarizePulls` helpers under cognitive-complexity threshold. Optional fields elided via spread+conditional (`...(row.name === undefined ? {} : { name: row.name })`) for `exactOptionalPropertyTypes` compliance. | |
| - **`snapshot-burn.ts`** (175 → 361 lines): bash `gh api ... | jq` pipelines → `ghJson` helper that wraps `spawnSync("gh", ["api", path])` + `JSON.parse`; defensive `ghJsonOrEmpty` for fault-tolerant capture. Bash `mapfile` workaround (`while read; do … done < <(...)` for macOS bash 3.2 compat) → straightforward TS for-loop. Bash heredoc-driven `jq -n` snapshot composition → typed `Snapshot` interface + `JSON.stringify`. Bash JSONL append (`printf '%s\n' >> "$out"`) → `appendFileSync(out, line + "\n")`. Per-repo aggregation extracted into `aggregateTimings` + `summarizePulls` helpers under cognitive-complexity threshold. Optional fields elided via spread+conditional (`...(row.name === undefined ? {} : { name: row.name })`) for `exactOptionalPropertyTypes` compliance. |
…Copilot+Codex P2 Both reviewers caught: row 322 used 'ed7c6e3-or-similar' / '(slice-14 merge; #894 — exact SHA on origin/main after fetch)' as placeholders instead of the actual merge SHA. Per the file schema (line 17 'commit-or-link'), col-5 must be a concrete commit SHA when one exists. Replaced both placeholders with 9cb21a7 (the actual slice-14 merge commit verified via 'git log origin/main').
…14 merged (10 PRs in session arc) (#895) * ops(tick-history): autonomous-loop tick 2026-04-30T05:43:00Z — slice-14 merged (10 PRs in session arc) * ops(tick-history): replace placeholder SHA with concrete 9cb21a7 per Copilot+Codex P2 Both reviewers caught: row 322 used 'ed7c6e3-or-similar' / '(slice-14 merge; #894 — exact SHA on origin/main after fetch)' as placeholders instead of the actual merge SHA. Per the file schema (line 17 'commit-or-link'), col-5 must be a concrete commit SHA when one exists. Replaced both placeholders with 9cb21a7 (the actual slice-14 merge commit verified via 'git log origin/main').
…ull-stream guards + header phrasing Three Copilot findings on #901: P0 — spawnSync launch failures collapsed into exitCode 1: Added classifySpawnFailure helper (4-case: status set / ENOENT → 127 / signal / other) reused from PRs #887, #898, #900. Both runSnapshotBurn and runProjectRunway now report a contextual note when the child can't start (e.g., 'snapshot-burn.sh: command not found on PATH (ENOENT)'). P0 — null stdout/stderr could yield 'nullnull': When a child fails to start, result.stdout / result.stderr can be null even with encoding: 'utf8'. Guarded with `?? ''` in runProjectRunway so the projection block doesn't end up as the literal string 'nullnull'. P2 — Header comment phrasing: Reworded 'snapshot-burn.sh ported in #894' to 'TS port snapshot-burn.ts landed in #894 but this wrapper still spawns the .sh during the soak period' to avoid implying the .sh file itself is the ported artifact.
…tion (#901) * ts(B-0086): port 1 budget script (.sh→.ts) — slice 18 of TS/Bun migration * ts(slice-18, wip 1/N): port budget/daily-cost-report (.sh→.ts) Daily cost-monitoring entry point. Wraps snapshot-burn.sh + project-runway.sh and writes human-readable projection to docs/budget-history/latest-report.md (visibility surface for Aaron's #287 deadline). Note: this wrapper still spawns the bash siblings (snapshot-burn.sh + project-runway.sh), NOT the TS port — the bash versions are the soak-period reference until they retire. Once project-runway is also TS-ported, this wrapper can switch to spawning the .ts versions. Mechanical changes: - bash arg-parse → parseArgs with ParsedArgs | ArgError | help - bash 'cat > "$report_path" <<EOF...EOF' heredoc → buildReport() template literal returning the markdown string - bash subshell command capture ($(...)) → spawnSync with stdio ['inherit', 'pipe', 'pipe'] for project-runway combined output - bash heredoc concat across multi-line → resolved via separate argsSuffix variable (sonarjs no-nested-template-literals) - exit codes 0/1/2 preserved verbatim per bash original Lint-clean: tsc --noEmit + eslint strictTypeChecked + sonarjs all pass. Argument-validation byte-equivalent. Trajectory: 39 ports total after merge, 4 Bucket B files remain (1 budget project-runway + 1 git/batch-resolve + 1 pr-preservation + 1 in-flight = 4 remaining). * review(slice-18): use statSync.isFile() instead of existsSync per Codex P2 Codex P2: existsSync returns true for directories and other non-regular paths; the bash original uses -f which checks regular-file existence. If snapshots.jsonl were ever a directory, existsSync would skip the bootstrap branch and the wrapper would try to spawn project-runway.sh against a non-file. Switched to statSync.isFile() with try/catch fallback to false. * review(slice-18): address Copilot P0+P0+P2 — spawn classification + null-stream guards + header phrasing Three Copilot findings on #901: P0 — spawnSync launch failures collapsed into exitCode 1: Added classifySpawnFailure helper (4-case: status set / ENOENT → 127 / signal / other) reused from PRs #887, #898, #900. Both runSnapshotBurn and runProjectRunway now report a contextual note when the child can't start (e.g., 'snapshot-burn.sh: command not found on PATH (ENOENT)'). P0 — null stdout/stderr could yield 'nullnull': When a child fails to start, result.stdout / result.stderr can be null even with encoding: 'utf8'. Guarded with `?? ''` in runProjectRunway so the projection block doesn't end up as the literal string 'nullnull'. P2 — Header comment phrasing: Reworded 'snapshot-burn.sh ported in #894' to 'TS port snapshot-burn.ts landed in #894 but this wrapper still spawns the .sh during the soak period' to avoid implying the .sh file itself is the ported artifact. * review(slice-18) round-2: extract step-helpers + suppress no-unnecessary-condition - Extracted runSnapshotStep + runProjectionStep helpers to drop main() under cognitive-complexity 15. - Added eslint-disable on stdout/stderr ?? '' guards (typings claim string when encoding is set, but the runtime can return null when spawn fails — same pattern as #898). Lint clean: tsc + eslint strictTypeChecked + sonarjs all pass. * review(slice-18): preserve stdout/stderr ordering via shell-side 2>&1 per Copilot P1 Copilot caught: concatenating result.stdout + result.stderr does NOT preserve the original chronological ordering of merged streams. The bash original $(... 2>&1) merges at the kernel pipe level — if project-runway.sh emits warnings on stderr while writing success output to stdout, the messages interleave correctly. Switched to /bin/bash -c 'path 2>&1' so the merge happens shell-side (matches bash original semantics). Single stdout pipe = correct ordering. result.stderr is now unused (the inherit pipe still receives nothing).
…tion (#902) Closes the budget cluster: snapshot-burn (slice 14) + daily-cost-report (slice 18) + project-runway (this slice) are now all TS. Once this lands, daily-cost-report.ts can switch from spawning project-runway.sh to project-runway.ts. Behavioural improvements over bash original (deliberate, not drift): - File-existence check uses statSync().isFile() + try/catch rather than existsSync — bash `-f` rejects directories, existsSync accepts them (slice-18 mirror). - JSONL parsing is native (readFileSync + split + JSON.parse) rather than per-line jq spawn-out — projection script reads already-persisted JSON, so jq is a heavy dependency for what is structurally a typed reduce. snapshot-burn.ts still needs gh api for capture; this is projection only. - requireInt validation matches bash `case '$val' in ''|*[!0-9]*) ...` with TS `requireInt(flag, val)` returning `number | ArgError` — same exit code 2, same error wording (Codex P2 NM59qF00 + NM59qH2H, Copilot P1 NM59qGJ- on the bash original). Byte-equivalence verified on this repo state: diff <(bun tools/budget/project-runway.ts) \ <(./tools/budget/project-runway.sh) # empty diff <(bun tools/budget/project-runway.ts --json) \ <(./tools/budget/project-runway.sh --json) # empty Error paths verified equivalent: --stages abc → exit 2 with matching message; --file <missing> → exit 1; --bogus → exit 2. Tools used: tsc --noEmit clean; eslint clean per the existing tsc-tools CI gate (#890). Composes with: - tools/budget/snapshot-burn.ts (slice 14, #894) - tools/budget/daily-cost-report.ts (slice 18, #901) - docs/trajectories/typescript-bun-migration/RESUME.md - docs/trajectories/typescript-bun-migration/slice-audits.md Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Summary
Slice 14 of the TS/Bun migration — opens budget-cluster:
tools/budget/snapshot-burn.{sh→ts}(point-in-time LFG cost/burn snapshot capture; appends one JSON line todocs/budget-history/snapshots.jsonl)Composes with #287 (Aaron's resource/costs visibility deadline). The bash original remains in-tree as the equivalence reference + production fallback until the TS port has soaked through several daily-cost-report runs.
Audit substrate
See
docs/trajectories/typescript-bun-migration/slice-audits.mdSlice 14 entry.Mechanical port notes
gh api ... | jqpipelines →ghJsonhelper wrappingspawnSync("gh", ["api", path])+JSON.parse.mapfileworkaround (while read; do … done < <(...)for macOS bash 3.2 compat) → straightforward TS for-loop.jq -nsnapshot composition → typedSnapshotinterface +JSON.stringify.printf '%s\n' >> "$out") →appendFileSync.aggregateTimings+summarizePullshelpers under cognitive-complexity threshold.exactOptionalPropertyTypescompliance.Test plan
bun --bun tsc --noEmit -p tsconfig.jsonclean (lint (tsc tools)gate from ci(B-0106): add lint-tsc-tools gate job — tsc --noEmit on tsconfig.json #890 will validate this in CI).eslintclean.--notewithout value exits 2;--helpexits 0.--dry-runexercised against the GitHub API: produces snapshot with all the same fields as the bash original (ts, factory_git_sha, org, note, copilot_billing, repos[].agg/pr/last_20_runs, scope_coverage).Trajectory
docs/trajectories/typescript-bun-migration/RESUME.md.🤖 Generated with Claude Code