Port from Go to TypeScript on Effect v4 + Bun by bmdavis419 · Pull Request #2 · davis7dotsh/open-yt-cli

bmdavis419 · 2026-07-25T09:57:43Z

Ports oytc from Go to TypeScript on Effect v4 beta, built and shipped with Bun, compiled with TypeScript 7.

The Go implementation stayed in the tree for the entire port so it could act as the behavioral oracle, and is deleted in the final commit. Nearly every bug listed below was found by differential-testing against a binary built from it, not by reading the source.

Stack


Runtime / package manager	Bun 1.3.14
Framework	`effect@4.0.0-beta.101`
Platform	`@effect/platform-bun@4.0.0-beta.101`
Compiler	`typescript@7.0.2` (native, GA)
Tests	`bun:test` — 2141 passing

Runtime dependencies: two. The CLI framework (effect/unstable/cli), HTTP client (effect/unstable/http), and Schema all live in Effect core — @effect/cli and @effect/platform have no v4 release and are not used. No googleapis, no arg parser, no HTTP library, no schema library, no test runner.

Scope

Full parity: all 30 leaf commands across 12 groups, config and credential storage, OAuth 2.0 with PKCE, self-update, skills install, plus installer scripts, site, and CI migrated to Bun.

Distribution

Five native binaries via bun build --compile: linux x64/arm64, darwin x64/arm64, windows x64.

windows/arm64 is dropped — Bun has no such compile target. install.ps1 now installs the amd64 build on ARM64 Windows (it runs under emulation) with a note, rather than hard-failing, which would strand existing ARM64 users on an un-updatable binary. Asset names are unchanged, so already-installed clients can still self-update.

Intentional deviations

Everything else is exact parity. These four are deliberate, and each has tests pinning the new behavior.

D1b — consistent error-reason normalization. Go applied two different normalizations to the same reasons string in one decision table: the auth test stripped _/-, the quota test did not. So userRateLimitExceeded correctly exited 5 while RATE_LIMIT_EXCEEDED fell through to 6. Google returns SCREAMING_SNAKE reasons in newer API surfaces, so the miss was real. One normalization now applies to every test.

D2 — no misleading resume token. Go set nextPageToken to the last fetched page's token even when --limit discarded items from it, so resuming skipped data. A truncated page now reports "". Exact-limit boundaries still keep the token.

D3 — --all is bounded. The loop terminated only on an empty nextPageToken, trusting the server completely. A harness returning a constant token consumed ~59 GB of RSS before being killed. Two guards now apply: loop detection (a token already followed can only repeat a page) and a 10,000-request ceiling. Neither is reachable on a well-behaved server; --limit still terminates first.

Missing arity checks — the one that could destroy your binary. oytc update <extra-arg> printed nothing, downloaded the last release, replaced its own executable, and exited 0. Command.make("update", …) declared no positional argument, so the framework silently discarded extras and no arity check ran. A typo like oytc update latest destroyed the installed binary. It fired during the audit, overwriting the test binary with the Go one — which briefly made the two agree perfectly, since both were the same program. Eleven other commands had the same gap: logout X deleted credentials, analytics video A B silently answered for A and discarded B.

Bugs caught by differential testing

Unit tests passed on every one of these:

JSON codec. A 15,000-document differential against Go 1.26.5 encoding/json found the spec was wrong about \b/\f escaping, and that lone surrogates yield one U+FFFD rather than three. Numeric literals survive byte-for-byte — including 9007199254740993123, which JSON.parse silently corrupts.
httpCore JSON prefix scanner mishandled a leading zero: a body of 0.5x truncated to 0. 536 diffs before the fix, 0 after.
Error envelope was case-sensitive. Go's decoder falls back to case-insensitive field matching, so {"Error":{"CODE":403}} decoded to an empty envelope — losing the message and every reason, and with them the exit code.
login never validated the key you typed. AppLayer did not expose HttpClient in its output, so Effect.serviceOption always saw None and fell back to the ambient client. Unit tests provided their own client, so only the compiled binary revealed it.
main.ts printed nothing to stderr. Every tagged error sets Runtime.errorReported = false to suppress the multi-line reporter, but the single-line printer meant to replace it existed only in a comment. Exit codes were right; users saw silent failures.
Analytics dropped a column named literally __proto__, and an object cell there replaced the row's prototype.
The file lock could not be interrupted while waiting, so Ctrl-C would not kill a blocked process.

Verified against the Go binary

9/9 validation errors match exactly — message text and exit code
status byte-identical in all four formats, including the quirk where scopes render as ["https://…"] with brackets rather than comma-joined, because Go reaches them through a different code path
Zero secret leaks in any format, with or without --check, tested with distinctive fixture values for the access token, refresh token, and client secret
update bogus-arg exits 2 with the binary's SHA-256 unchanged
Cross-process credential locking proven with real spawned subprocesses — Bun has no flock, so this uses an O_EXCL lockfile plus an in-process semaphore and a staleness steal. A same-process test would pass even if the lockfile were entirely broken.
make release-check passes: tests, all 5 compile targets, site validation

Known gaps

Two upstream issues in effect/unstable/cli, documented but not worked around:

Parse errors dump help to stdout where Go writes nothing, so oytc search foo --typo | jq . gets fed help text.
-- drops the operand for subcommands (video get -- ABC sees zero args). Traced to a one-line bug in the framework's parser; patch location documented.

Help text differs from cobra's layout — parity there was explicitly out of scope.

🤖 Generated with Claude Code

Note

Port oytc CLI from Go to TypeScript using Effect v4 and Bun

Rewrites the entire oytc CLI from Go to TypeScript, preserving Go-compatible wire semantics (JSON key ordering, URL encoding, tabwriter output, duration parsing, semver comparison, OAuth flows, etc.) across all commands.
Replaces Go build tooling with Bun: bun build --compile produces binaries for linux/amd64, linux/arm64, darwin/amd64, darwin/arm64, and windows/amd64; windows/arm64 is dropped (x64 emulation recommended).
All service implementations (HTTP transport, credential store, file locking, OAuth, analytics, YouTube Data API, renderers) are built with Effect v4 layers and tags, enabling dependency injection and structured error propagation.
CLI commands (channel, video, playlist, search, comment, live-chat, subscription, analytics, auth, skills, version, update, catalog) are re-implemented in src/cli/ with validation order and output format parity against the Go binary.
JSON encoding (src/json/encode.ts) preserves numeric literals as RawNumber and sorts object keys by UTF-8 byte order to match encoding/json.
Risk: the Go source (internal/update/update.go and peers) is deleted; the Go toolchain is no longer used anywhere in CI or release.

^{Macroscope summarized a2bf316.}

Begin the TypeScript port. This lands the pieces every other module depends on, verified against the Go implementation before anything is built on top of them. - src/effect.ts: mandated barrel for effect/unstable/* imports, so a pre-4.0-final rename is a one-file fix - src/json/: number-preserving parse (Go UseNumber equivalent, via the ES2025 `source` reviver) and a Go-compatible encoder - src/util/gostring.ts: UTF-8 byte-order key comparison and rune counting JSON.stringify cannot be used for output: it does not escape U+2028/9, emits \udXXX for lone surrogates, and does not sort keys. It is banned outside src/json/encode.ts. Verified with a differential harness against Go 1.26.5 encoding/json: 15,000 randomized documents across 3 seeds, zero mismatches in both compact and pretty output. That harness caught two errors that the hand-written tests passed: - The spec claimed Go emits � / for backspace and form feed; byte-level checking shows Go uses the short \b and \f escapes, matching JSON.stringify. - Lone surrogates yield ONE U+FFFD, not three. Go's decoder collapses a \uD800 escape at decode time; the three-U+FFFD behavior only applies to raw WTF-8 bytes, which never enter this pipeline. Numeric literals survive byte-for-byte, including 9007199254740993123 from the Go test suite, which JSON.parse silently corrupts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Everything the parallel implementation packages code against. Signatures here are frozen: changing one invalidates work in flight elsewhere. - domain/errors.ts: the tagged error ADT, each variant carrying its exit code via Runtime.errorExitCode (verified to drive process exit through a compiled binary) - domain/listResult.ts: the list envelope, with Go's two simultaneous key orders (fixed struct order outside, sorted maps within) - schema/: deliberately loose Data API decoding plus the seven narrow accessors that cover every nested read the Go code performs - services/index.ts: 15 service tags and interfaces - cli/root.ts: the root command and global-flag resolution - impl/processEnv.ts: the only place process.* is touched - stubs for every impl/, output/, and cli/ module so parallel packages own strictly disjoint files Applies DEVIATIONS.md D1b: one reason normalization is used for every test in the exit-code table, so RATE_LIMIT_EXCEEDED and QUOTA_EXCEEDED now classify as quota (5) instead of falling through to 6. Ordering is preserved, so a quota-carrying 403 still exits 5 and a bare 403 exits 4. Per-resource schemas are deliberately absent: --parts and --fields let users request arbitrary field subsets, so any fixed Video/Channel schema would reject responses the Go client accepts. 124 tests pass; TypeScript 7.0.2 typechecks clean under strict and exactOptionalPropertyTypes; `bun run src/main.ts --help` works. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Eight packages implemented in parallel over disjoint file sets, each followed by an independent verifier that re-checked the work against the Go source rather than trusting the implementer's report. - impl/httpCore, youtubeApi, resolveChannel, schema/errorEnvelope: transport and Data API client - impl/analyticsApi: Analytics reports client - output/{columns,table,tsv,jsonOut} + impl/renderer: a real port of Go's text/tabwriter state machine, not a pad-to-width approximation - impl/{credentialStore,fileLock,atomicWrite} + schema/authfile: credential storage with cross-process locking - impl/{oauth,oauthServer,tokenSource,browserOpener}: PKCE S256 loopback - impl/{updater,archive,semver,platformMatrix}: checksum-verified update - impl/{skillInstaller,versionInfo,prompts} + src/skills: embedded skill - scripts/, .depot/, site/, Makefile: Bun packaging for 5 targets The verifiers found defects the implementers' own tests missed, all caught by differential fuzzing against Go 1.26.5 rather than by reading: - httpCore's JSON prefix scanner mishandled a leading zero, so a body of `0.5x` silently truncated to `0` (536 diffs before the fix, 0 after) - the error envelope was case-sensitive, but Go's encoding/json falls back to a case-insensitive field match, so `{"Error":{"CODE":403}}` decoded to an empty envelope — losing the message and every reason, and with them the exit code - analytics dropped a column whose header was literally `__proto__`, and an object cell there replaced the row's prototype - the file lock could not be interrupted while waiting Integration fixes in this commit: - statusText now carries Go's full net/http table (61 entries, generated from Go 1.26.5) instead of the 14 common codes - layers.ts is wired for real. ProcessEnv and VersionInfo are dependencies of four other layers, not siblings: Effect does not let members of a mergeAll satisfy each other's requirements, so each consumer receives them via Layer.provide - TokenSource and HttpCoreShape now carry OAuthError. It must propagate unchanged because it holds its own exit code (3 re-login, 5 on 429, 6 on 5xx); wrapping it would flatten every case to 6 1303 tests pass, TypeScript 7 typechecks clean, and the CLI runs with the full layer graph. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

All 30 leaf commands across 12 groups, wired into root.ts. Three packages implemented in parallel, each independently verified against the real Go binary rather than against the spec. - cli/search, channel, video + the shared fields/validate/render helpers - cli/playlist, comment, subscription, catalog - cli/livechat, analyticsCmd, auth, versionUpdate, skillsCmd Fixes the highest-priority bug the verifiers found: main.ts printed NOTHING to stderr on failure. Every tagged error carries `Runtime.errorReported = false` to suppress the runtime's multi-line reporter, but the single-line printer that was supposed to replace it was only ever described in a comment. Exit codes were right and the message was absent, so a user saw a silent failure. main.ts now installs that printer and translates the CLI framework's parse errors into Go's wording and exit code (the framework exits 1 and uses different phrasing). Two framework details worth recording: its unknown-subcommand tag is spelled `UnknownSubcomand` with one "m", and `--limit -1` lexes as two flags rather than a negative value, so the underlying range error is reported instead of the lexer artifact. Also corrects `--timeout must be greater than zero` to Go's `--timeout must be positive`, and updates the two tests that pinned the old wording. Verified against /tmp/oytc-ref: - 9 of 9 validation errors match exactly, message and exit code - `status` is byte-identical in all four formats, including the G1 quirk where scopes render as ["https://..."] with brackets rather than comma-joined, because Go reaches them through a different code path - `status` leaks no secret in any format, checked with distinctive fixture values for the access token, refresh token and client secret Verifier findings fixed in the packages themselves include `video trainability` mishandling every non-object response body: a `null` body succeeds in Go (exit 0), and other scalars produce an OperationalError at exit 6, where the port returned exit 4 with invented text. The pre-existing test asserted the buggy behavior. 2136 tests pass; TypeScript 7 typechecks clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

A smoke-test pass ran the compiled binary against the Go one across all 32 leaf commands, ~150 validation invocations, every output format, and config handling. Unit tests had not caught any of these. CRITICAL — `oytc update <extra-arg>` performed a real self-update. `Command.make("update", ...)` declared no positional argument, so the framework silently discarded extras and no arity check ever ran. The command printed nothing, downloaded the last Go release from GitHub, replaced its own executable and exited 0. A typo like `oytc update latest` destroyed the installed binary. It fired during the audit itself, overwriting the test binary with the Go one — which briefly made the two "agree" perfectly, since both were the same program. The same missing-arity bug affected 11 more commands: `logout X` deleted credentials, `skills install X` reached the overwrite prompt, and `analytics video A B` silently ran the report for A while discarding B, returning a plausible but wrong answer. HIGH — `login` never validated the key you just typed. `keyScopedApi` resolves an `HttpClient` via `Effect.serviceOption`, but AppLayer did not expose one in its output, so it always saw None and fell back to the ambient client. `login` probed an empty store and reported "no API key configured"; `status --check` validated the API key using OAuth credentials and reported key failures through OAuth error text. Unit tests provided their own HttpClient, so only the compiled binary showed it — a genuine test-versus-production gap. HIGH — the `-f` and `-q` shorthands were missing, breaking any script using `-f json`, and `--no-color` was rejected outright. Go accepts and ignores it, and scripts pass it. Also: leading-dash values now name the flag the user actually wrote rather than always blaming --limit, and a valueless flag reports pflag's "flag needs an argument" instead of "Help requested". Verified after the fix: `update bogus-arg` exits 2 with the binary's SHA-256 unchanged; all 12 arity cases match Go's message and exit code; `status --check` is byte-identical to Go; no secret leaks in any of the four formats with or without --check. 2136 tests pass, tsc clean. Two known issues remain, both upstream in the CLI framework and documented in /tmp/go-spec/SMOKE_TEST.md: parse errors dump help to stdout where Go writes nothing, and `--` drops the operand for subcommands. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

## `--all` could run forever (DEVIATIONS.md D3) The pagination loop terminated only when the server returned an empty `nextPageToken`, which trusts the server completely. A server that repeats a token — a bug, a caching proxy, a hostile endpoint — made `--all` loop forever, accumulating every page in memory with no ceiling and never flushing output. Not hypothetical: a test harness whose fake server returned a constant token ran `search --all` and consumed ~59 GB of RSS across two processes before being killed, taking the machine to 127 GB used with 44 GB in the compressor. A verifier had already seen the hang and recorded "verified both Go and TS hang identically", treating the match as faithful. It was faithful. Faithfully reproducing an unbounded loop is still an unbounded loop. Two guards, neither reachable on a well-behaved server: - Loop detection: a token already followed can only return a page already fetched, so stop and report `nextPageToken = ""` — the same "no valid resume point" signal D2 uses. - Request ceiling: MAX_PAGES = 10000 as a backstop for a server emitting distinct tokens forever, which the loop check cannot catch. It fails with an OperationalError naming `--limit` as the remedy. At the largest page size any endpoint accepts (2000, live chat) the ceiling allows 20M items; every other endpoint caps at 50 or 100 per page. `--limit` still terminates before either guard, so a bounded request against a repeating server stops at the limit. Verified against the exact failing scenario: a local server returning a constant token now terminates in 11ms after 2 requests at 104 MB RSS. ## Go implementation removed Deletes cmd/, internal/, go.mod, go.sum, and skills/oytc/embed.go, and updates the README's build instructions for Bun. The Go source stayed in place for the whole port so it could serve as the behavioral oracle — the differential tests that caught the JSON codec bugs, the error envelope's case-insensitivity, and the `video trainability` body handling all ran against a binary built from it. 2141 tests pass; TypeScript 7 typechecks clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

greptile-apps · 2026-07-25T09:57:46Z

Too many files changed for review. (162 files found, 100 file limit)

Bypass the limit by tagging @greptile-apps to review.

coderabbitai · 2026-07-25T09:58:06Z

📝 Walkthrough

Walkthrough

This PR replaces the entire Go implementation of the oytc CLI with a Bun/TypeScript rewrite. It removes Go source, tooling, and dependencies, and adds new TypeScript modules for CLI commands, HTTP/OAuth/credential services, JSON/output formatting, schema decoding, and updated CI/release workflows, build scripts, and documentation.

Changes

Bun/TypeScript CLI rewrite

Layer / File(s)	Summary
CI, release workflows, and build tooling `.depot/workflows/.yml`, `Makefile`, `dev`, `scripts/package.sh`, `.gitignore`, `package.json`, `tsconfig.json`, `README.md`, `docs/releasing.md`, `site/`	CI/release pipelines, Makefile targets, packaging script, installer docs/scripts, and project config are migrated from Go toolchain steps to Bun install/typecheck/test/build/compile, with the release matrix dropping `windows/arm64`.
Go implementation removal `internal/config/lock_unix.go`, `internal/config/rename_unix.go`, `internal/update/update_test.go`	Remaining Go build-tag files and superseded test files are deleted as part of decommissioning the Go codebase.
Domain errors, list envelope, service tags, and app entrypoint `src/domain/errors.ts`, `src/domain/listResult.ts`, `src/effect.ts`, `src/layers.ts`, `src/main.ts`, `src/services/index.ts`	Introduces the tagged error hierarchy and exit-code mapping, the paginated list envelope, the Effect-service Context tags, the composed application Layer graph, and the CLI entrypoint with Go-compatible error/help/exit-code handling.
Go-compatible JSON encode/parse/value model `src/json/*.ts`	Adds a number-preserving `RawNumber` JSON value model, a strict JSON parser, and Go-style JSON string/value encoders.
Shared utility helpers `src/impl/semver.ts`, `src/impl/platformMatrix.ts`, `src/util/goduration.ts`, `src/util/gostring.ts`	Adds Go-compatible version comparison, the release platform/asset matrix, duration parsing, and UTF-8 string helpers.
Credential storage, file locking, OAuth, browser opener, and prompts `src/impl/credentialStore.ts`, `src/impl/fileLock.ts`, `src/impl/processEnv.ts`, `src/impl/oauth.ts`, `src/impl/tokenSource.ts`, `src/impl/browserOpener.ts`, `src/impl/prompts.ts`	Implements secure credential persistence with cross-process locking, the OAuth login/refresh/revoke flow with a loopback server, cached token sourcing, browser launching, and interactive prompts.
HTTP transport and API/service implementations `src/impl/httpCore.ts`, `src/impl/youtubeApi.ts`, `src/impl/analyticsApi.ts`, `src/impl/resolveChannel.ts`, `src/impl/archive.ts`, `src/impl/atomicWrite.ts`, `src/impl/skillInstaller.ts`, `src/impl/updater.ts`, `src/impl/versionInfo.ts`, `src/impl/renderer.ts`	Implements the retrying/authenticated HTTP transport, YouTube Data/Analytics API clients, channel resolution, archive extraction, atomic file writes, skill installation, self-update logic, version info, and stdout rendering.
Schema decoding `src/schema/*.ts`	Adds Effect Schema definitions and tolerant decoders for the Data API response, Analytics response, Google error envelope, and on-disk `auth.json`.
Output formatting `src/output/*.ts`	Implements column resolution/cell rendering plus JSON, JSONL, table, and TSV renderers shared by all commands.
Shared CLI infrastructure `src/cli/flags.ts`, `src/cli/globals.ts`, `src/cli/validate.ts`, `src/cli/render.ts`, `src/cli/root.ts`, `src/cli/fields.ts`, `src/cli/*Harness.testutil.ts`	Adds global flags/options resolution, shared validation helpers, shared render helpers, the composed root command, the `--fields` selector parser, and CLI test harnesses.
CLI command modules `src/cli/analyticsCmd.ts`, `src/cli/auth.ts`, `src/cli/catalog.ts`, `src/cli/channel.ts`, `src/cli/comment.ts`, `src/cli/livechat.ts`, `src/cli/playlist.ts`, `src/cli/search.ts`, `src/cli/skillsCmd.ts`, `src/cli/subscription.ts`, `src/cli/versionUpdate.ts`, `src/cli/video.ts`	Implements every oytc subcommand (analytics, auth, catalog, channel, comment, live-chat, playlist, search, skills, subscription, version/update, video) with associated tests.
Documentation and bundled agent-skill content `src/skills/SKILL.md`, `src/skills/references/*.md`, `src/skills/bundle.ts`	Adds the oytc agent skill documentation and the embedded skill bundle used by `oytc skills install`.

Possibly related PRs

davis7dotsh/open-yt-cli#1: Removes the Go OAuth/login implementation (cmd/oytc/main.go, internal/cli/auth.go, internal/oauth/oauth.go) that this PR's TypeScript OAuth/login rewrite (src/impl/oauth.ts, src/cli/auth.ts) directly replaces.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main change: porting the project from Go to TypeScript on Effect v4 and Bun.
Description check	✅ Passed	The description is strongly related to the changeset and accurately describes the Go-to-TypeScript/Bun port.

_{Comment @coderabbitai help to get the list of available commands.}

macroscopeapp · 2026-07-25T09:59:21Z

Macroscope skipped reviewing this pull request. Per-review cost limit exceeded (workspace setting).

This review would cost an estimated $28.85, which exceeds your per-review limit of $10.00.

The top 3 files driving up this estimate:

File	Size	Estimate
`src/impl/oauth.ts`	38.88KB	$1.94
`src/impl/updater.ts`	28.76KB	$1.44
`src/cli/playlist.ts`	23.73KB	$1.19

Tip

To get this pull request reviewed, you can:

Comment @macroscope-app on this PR to request a manual review (monthly spend limits still apply).
Exclude the file(s) above from review by adding a pattern to your .macroscope/ignore.md — note that creating this file replaces Macroscope's built-in default ignores rather than extending them.
Raise your cost limit in your workspace billing settings.

^{Turn off this reminder going forward}

coderabbitai

Actionable comments posted: 19

🧹 Nitpick comments (26)

site/install.ps1 (1)
23-31: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Unhandled failure path if Get-CimInstance is unavailable.

$ErrorActionPreference = 'Stop' (line 16) means a WMI/CIM failure here (e.g. locked-down or minimal Windows environments where the CIM service isn't reachable) surfaces as a raw, unfriendly CIM exception instead of a clear installer error. Consider wrapping the detection in try/catch with a graceful fallback (e.g., assume amd64 and proceed, matching this script's existing bias toward always installing amd64).
🛡️ Suggested fix
-$processorArchitecture = @(Get-CimInstance Win32_Processor)[0].Architecture
+try {
+    $processorArchitecture = @(Get-CimInstance Win32_Processor -ErrorAction Stop)[0].Architecture
+} catch {
+    Write-Host 'Could not query CPU architecture via WMI; assuming amd64.'
+    $processorArchitecture = 9
+}
 if ($processorArchitecture -eq 12) {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@site/install.ps1` around lines 23 - 31, Wrap the Win32_Processor query in the
architecture-detection block around $processorArchitecture with try/catch so
CIM/WMI failures do not surface as raw exceptions. On failure, gracefully retain
the existing amd64 default and continue installation, while preserving the ARM64
and 32-bit validation behavior when detection succeeds.
.depot/workflows/ci.yml (1)
15-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Bun version pinned in two places with only a comment enforcing sync. ci.yml defines the canonical BUN_VERSION, but release.yml hardcodes the same value as a separate literal, relying on a human remembering to update both on every bump.

.depot/workflows/ci.yml#L15-L18: keep as the single source of truth; consider promoting it to a repository/organization variable (vars.BUN_VERSION) so other workflows can reference it directly instead of a workflow-local env.

.depot/workflows/release.yml#L55-L58: replace the hardcoded bun-version: "1.3.14" with a reference to the shared value (repository variable, or a small composite action/reusable workflow shared with ci.yml) instead of a second literal plus a "keep in sync" comment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.depot/workflows/ci.yml around lines 15 - 18, Use a shared Bun version
source instead of duplicating the literal: update .depot/workflows/ci.yml lines
15-18 to expose BUN_VERSION as a repository or organization variable (or
equivalent shared workflow source), and update .depot/workflows/release.yml
lines 55-58 to reference that shared value for bun-version. Remove any
synchronization-only comment or hardcoded duplicate while preserving the pinned
version.
package.json (1)
13-13: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Pin @types/bun to the tested Bun version.

latest is the only unbounded toolchain dependency. Pin it to the Bun version used by BUN_VERSION (or a tested compatible range) so future lock refreshes cannot introduce an unreviewed type-surface change.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@package.json` at line 13, Update the `@types/bun` dependency in package.json to
a fixed version matching BUN_VERSION, or use a tested compatible version range
instead of latest. Keep the dependency aligned with the Bun version used by the
project so lockfile refreshes remain controlled.
src/impl/atomicWrite.ts (1)
29-29: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value

Prefer a CSPRNG for the temp-file infix.

Math.random() is predictable, so a co-located attacker can pre-create .auth-<n>.tmp names. The wx flag keeps this safe (the write fails rather than clobbering), but it turns into a denial-of-service on credential saves. crypto.randomUUID()/randomBytes removes the guessing game for free.
🔒 Suggested change
-const tempName = (): string => `.auth-${Math.floor(Math.random() * 0xffffffff)}.tmp`
+const tempName = (): string => `.auth-${randomBytes(8).toString("hex")}.tmp`
Add the import:
+import { randomBytes } from "node:crypto"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/impl/atomicWrite.ts` at line 29, Update tempName in atomicWrite.ts to use
a cryptographically secure random value from the crypto module instead of
Math.random(), while preserving the existing .auth-… .tmp filename format and
return type.
src/impl/credentialStore.worker.ts (1)
42-42: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Make worker failures self-diagnosing.

A non-numeric iterations yields NaN, the loop body never runs, and the worker exits 0 — a silently vacuous test run. Also String(exit.cause) loses the failure detail that the test's empty-stderr assertion is meant to surface.
♻️ Suggested changes
-const iterations = Number.parseInt(iterationsText, 10)
+const iterations = Number.parseInt(iterationsText, 10)
+if (!Number.isInteger(iterations) || iterations <= 0) {
+  process.stderr.write(`invalid iterations: ${iterationsText}\n`)
+  process.exit(1)
+}
Outside the selected range, use Cause.pretty for the failure report:
-  process.stderr.write(`worker ${mode} failed: ${String(exit.cause)}\n`)
+  process.stderr.write(`worker ${mode} failed: ${Cause.pretty(exit.cause)}\n`)
(requires adding Cause to the effect import.)
Also applies to: 101-104
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/impl/credentialStore.worker.ts` at line 42, Validate the result of
Number.parseInt in the worker iteration setup and fail explicitly when
iterations is non-numeric, preventing a vacuous successful run. In the worker
failure-reporting path, replace String(exit.cause) with Cause.pretty and add
Cause to the existing effect import so the full failure detail is surfaced.
src/impl/archive.test.ts (1)
325-339: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Prefer mkdtemp over a hand-rolled random /tmp path.

fs.promises.mkdtemp(path.join(os.tmpdir(), "oytc-archive-test-")) gives a collision-free directory, works if these tests ever run on Windows, and drops the two Bun.$ shell-outs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/impl/archive.test.ts` around lines 325 - 339, Update withTempFile to
create its temporary directory using fs.promises.mkdtemp with os.tmpdir() and
the existing “oytc-archive-test-” prefix. Replace the Bun.$ mkdir and rm
shell-outs while preserving file creation, callback execution, and guaranteed
cleanup in the finally block.
src/impl/oauth.ts (1)
253-254: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Body limit is applied after the whole body is buffered.

response.arrayBuffer materializes the entire response before decodeBody truncates, so the 1<<20 cap documented at Line 61 doesn't actually bound memory the way Go's io.LimitReader does. Low risk in practice (Google endpoints), but streaming-and-truncating would restore the invariant.

Also applies to: 376-381
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/impl/oauth.ts` around lines 253 - 254, Update the response-body handling
around decodeBody and the code paths at the referenced response processing sites
so the body is read through a streaming, byte-limited mechanism before buffering
or decoding. Enforce BODY_LIMIT_BYTES during reads, preserving truncation at the
limit and avoiding response.arrayBuffer() materializing the full response first.
src/impl/youtubeApi.ts (1)
48-53: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

setParam does not collapse duplicate keys, unlike the url.Values.Set it documents.

When params already contains the key more than once, the map branch rewrites every occurrence, so the request carries key=value repeated instead of a single entry. Collapsing to the first position matches Go and is strictly safer:
♻️ Proposed change
 const setParam = (params: Params, key: string, value: string): Params => {
-  const kept = params.filter(([k]) => k !== key)
-  return kept.length === params.length
-    ? [...params, [key, value] as const]
-    : params.map((entry) => (entry[0] === key ? ([key, value] as const) : entry))
+  const first = params.findIndex(([k]) => k === key)
+  if (first < 0) return [...params, [key, value] as const]
+  return params.flatMap((entry, index) =>
+    entry[0] !== key ? [entry] : index === first ? [[key, value] as const] : []
+  )
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/impl/youtubeApi.ts` around lines 48 - 53, Update setParam to collapse
duplicate entries for the target key: preserve the first occurrence with the new
value and remove all later occurrences, while continuing to append the key when
it is absent. Keep other parameter entries and their order unchanged.
src/impl/skillInstaller.test.ts (1)
238-250: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Restore the directory mode in a finally.

If any assertion above throws, chmodSync(locked, 0o700) never runs and the afterEach rmSync cannot remove the 0500 directory, turning one failure into a leaked temp tree plus a confusing teardown error.
♻️ Proposed change
-    const exit = await Effect.runPromise(
-      installSkill(target).pipe(Effect.provide(BunServices.layer), Effect.exit)
-    )
-    expect(exit._tag).toBe("Failure")
-    expect(operationalMessage(exit)).toStartWith("create skills directory: ")
-    fsSync.chmodSync(locked, 0o700)
+    try {
+      const exit = await Effect.runPromise(
+        installSkill(target).pipe(Effect.provide(BunServices.layer), Effect.exit)
+      )
+      expect(exit._tag).toBe("Failure")
+      expect(operationalMessage(exit)).toStartWith("create skills directory: ")
+    } finally {
+      fsSync.chmodSync(locked, 0o700)
+    }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/impl/skillInstaller.test.ts` around lines 238 - 250, Update the test
around installSkill so the locked directory’s chmodSync restoration runs in a
finally block, regardless of assertion or installation failures. Preserve the
existing assertions and restore mode to 0o700 before teardown.
src/impl/httpCore.test.ts (1)
916-922: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

This test asserts nothing about the behaviour it claims to guard.

It builds a layer and checks typeof layer === "object" — it never runs makeHttpCore/getJson against it, so an implementation that bypassed the HttpClient layer and called fetch directly would still pass. Either run a request through the dying layer and assert the defect, or drop the test.
♻️ Sketch
-test("the HttpClient service is what actually issues the request", async () => {
-  // Guards against the impl bypassing the layer and calling fetch directly.
-  const layer = Layer.succeed(HttpClient.HttpClient, {
-    execute: () => Effect.die("should not be reached")
-  } as unknown as HttpClient.HttpClient)
-  expect(typeof layer).toBe("object")
-})
+test("the HttpClient service is what actually issues the request", async () => {
+  const layer = Layer.succeed(HttpClient.HttpClient, {
+    execute: () => Effect.die("reached")
+  } as unknown as HttpClient.HttpClient)
+  const exit = await Effect.runPromise(
+    Effect.gen(function* () {
+      const core = yield* makeHttpCore({ apiKey: "k", tokenSource: undefined, maxRetries: 0 })
+      return yield* core.getJson(base())
+    }).pipe(Effect.provide(layer), Effect.exit)
+  )
+  expect(Exit.isFailure(exit)).toBe(true)
+})
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/impl/httpCore.test.ts` around lines 916 - 922, Replace the non-behavioral
assertion in the “the HttpClient service is what actually issues the request”
test with an execution of makeHttpCore/getJson using the mocked HttpClient
layer. Assert that the request fails with the layer’s “should not be reached”
defect, proving the implementation uses HttpClient rather than calling fetch
directly; otherwise remove the test.
src/skills/bundle.ts (1)
21-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

An ambient *.md declaration would retire the three @ts-ignores. A one-line declare module "*.md" { const content: string; export default content } in a .d.ts gives typed imports and drops the as string casts below; @ts-ignore also hides unrelated errors on those lines.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/skills/bundle.ts` around lines 21 - 26, Add an ambient "*.md" module
declaration in an appropriate TypeScript declaration file, defining the default
export as a string. Remove the three `@ts-ignore` directives from the markdown
imports in the bundle module and remove the related as string casts, relying on
the declaration for typing.
src/cli/analyticsCmd.ts (2)
81-107: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Duplicated renderResult in src/cli/analyticsCmd.ts and src/cli/livechat.ts. Both copies exist because src/cli/render.ts was a stub when they were written; it is a real module in this PR and search.ts/video.ts already import renderResult from it, so the local copies are now drift risk for the stderr summary format.

src/cli/analyticsCmd.ts#L81-L107: delete the local renderResult and import it from ./render.ts, passing the preset default columns.

src/cli/livechat.ts#L214-L247: delete the local writeErr/renderResult pair and use the shared helper for live-chat list.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/analyticsCmd.ts` around lines 81 - 107, The local renderResult in
src/cli/analyticsCmd.ts#L81-L107 should be removed and replaced with an import
from ./render.ts, passing the preset default columns. In
src/cli/livechat.ts#L214-L247, remove the local writeErr/renderResult pair and
use the shared renderResult helper for live-chat list, preserving the existing
options and default-column behavior.
370-375: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

analyticsVideoColumns doubles as the metrics list. They happen to coincide today, but a column-only change (e.g. adding a dimension column) would silently alter the API request. Consider an explicit analyticsVideoMetrics constant, as overview already does with analyticsOverviewMetrics.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/analyticsCmd.ts` around lines 370 - 375, Define a separate
analyticsVideoMetrics constant for the metrics list and use it in the
runAnalytics call, while retaining analyticsVideoColumns for columns. Follow the
existing analyticsOverviewMetrics pattern so future column-only changes do not
alter the requested metrics.
src/cli/livechat.ts (1)
434-441: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

seen grows without bound for the lifetime of a stream. A busy chat polled for hours accumulates every message id in memory. Go had the same shape, so this is parity, but a bounded structure (e.g. capped insertion-ordered set) would make long sessions safe.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/livechat.ts` around lines 434 - 441, The live chat polling loop’s
seen set grows indefinitely during long-running streams. Replace the unbounded
seen structure in the stream flow with a bounded insertion-ordered structure
that evicts the oldest message IDs at a defined capacity, while preserving
duplicate suppression for retained IDs and the existing emitted/request
behavior.
src/cli/analyticsCmd.test.ts (1)
292-302: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Two brittle assertions in this suite.

Line 292-295 recomputes "yesterday" at assertion time while DEFAULT_RANGE was materialized at module import; a run crossing UTC midnight fails spuriously. Line 297-302 compares a spread copy of DEFAULT_RANGE to itself, so it always passes and pins nothing about construction-time materialization — a formatDateOnly-based recomputation comparison would actually test it.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/analyticsCmd.test.ts` around lines 292 - 302, Make the tests
deterministic by capturing the expected UTC date relative to the same
construction-time reference used by DEFAULT_RANGE instead of calling new Date()
during assertion. Replace the self-comparison in the materialization test with a
formatDateOnly-based recomputation comparison that would differ if DEFAULT_RANGE
were evaluated per access, while preserving the existing range assertions.
src/cli/livechat.test.ts (1)
488-495: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Two low-value assertions worth tightening. Line 493 (expect(await exit).toBeDefined()) can never fail. Line 923's title says "prints help and exits 0" while the body asserts Exit.isFailure — rename to reflect the framework's actual help-as-failure behaviour so the test isn't read as a contradiction.

Also applies to: 923-927
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/livechat.test.ts` around lines 488 - 495, Remove the redundant
expect(await exit).toBeDefined() assertion from the offlineAt test, retaining
the meaningful success and API-call checks. Rename the help test around the
referenced lines so its title accurately describes printing help and exiting
with failure, matching its Exit.isFailure assertion.
src/cli/auth.test.ts (1)
60-71: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider a fixture key that won't trip secret scanners.

SECRET_API_KEY uses the real AIzaSy GCP key prefix, which secret-scanning tooling flags (Betterleaks gcp-api-key). A neutral prefix keeps the greppable-fixture property without generating recurring findings.
♻️ Suggested fixture change
-const SECRET_API_KEY = "AIzaSyTESTKEY1234567890abcdefghijklmnop"
+const SECRET_API_KEY = "FAKE-API-KEY-do-not-print-1234567890"
Note: GOLDEN_FINGERPRINT is a stubbed constant in this harness, so changing the fixture value does not affect the fingerprint assertions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/auth.test.ts` around lines 60 - 71, Update the SECRET_API_KEY fixture
to use a neutral, non-provider-specific prefix while keeping it distinctive and
greppable. Preserve the FORBIDDEN collection and all related assertions
unchanged.
Source: Linters/SAST tools
src/cli/auth.ts (2)
84-99: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the raw Stdio writer into one shared helper. Both files re-implement the same Stream.run(Stream.make(text), stdio.stdout()) plumbing with identical OperationalError({ message: "could not write output" }) wrapping; any future change (flush semantics, error text) has to be made twice.

src/cli/auth.ts#L84-L99: move write/writeOut/writeErr into a shared module (e.g. src/cli/stdioWrite.ts) and import them here.

src/cli/versionUpdate.ts#L54-L62: delete the local writeOut and import the shared one.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/auth.ts` around lines 84 - 99, The raw Stdio writing logic is
duplicated across both CLI files. In src/cli/auth.ts lines 84-99, move write,
writeOut, and writeErr into a shared module and import them; in
src/cli/versionUpdate.ts lines 54-62, remove the local writeOut implementation
and import the shared helper, preserving the existing Stream.run behavior and
OperationalError wrapping.
64-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Rename the array-arity helper playlist.ts exports exactArgs with a different contract than validate.ts’s exactArgs, and auth.ts/versionUpdate.ts import the former. A distinct name like exactPositionals would avoid the collision.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/auth.ts` at line 64, Rename the playlist.ts-exported exactArgs helper
to a distinct name such as exactPositionals, then update its imports and usages
in auth.ts and versionUpdate.ts while leaving validate.ts’s exactArgs unchanged.
src/cli/video.test.ts (1)
24-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Type the script parameter as ApiScript.

script = {} is inferred as {}, so a mistyped script key (e.g. page instead of pages) type-checks and silently produces an empty script. src/cli/channel.test.ts already annotates it.
♻️ Suggested change
-const get = (argv: ReadonlyArray<string>, script = {}): Promise<RunResult> =>
+const get = (argv: ReadonlyArray<string>, script: ApiScript = {}): Promise<RunResult> =>
   runCli(cmd(videoGetCommand), argv, { script })
plus the same for stats, popular, trainability, and add ApiScript to the type import at Line 12.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/video.test.ts` around lines 24 - 34, Type the script parameter as
ApiScript in the get, stats, popular, and trainability helpers, and add
ApiScript to the existing type import. Preserve the default empty script while
ensuring invalid keys are rejected by type checking.
src/cli/playlist.ts (1)
6-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Header rationale is already stale, and the helper home is now load-bearing for four files.

Line 39 imports goTrimSpace from ./validate.ts, so that module is no longer an "empty stub"; meanwhile comment.ts, subscription.ts, catalog.ts and skillsCmd.ts (which only needs exactArgs) all import through playlist.ts, dragging playlist commands and column definitions into unrelated modules. Consider completing the planned move of the shared helpers into validate.ts/fields.ts/render.ts and keeping playlist.ts command-only, or at minimum updating this header so the next reader doesn't act on an outdated premise.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/playlist.ts` around lines 6 - 19, Move the shared helpers from
playlist.ts into their owning modules validate.ts, fields.ts, and render.ts,
then update comment.ts, subscription.ts, catalog.ts, and skillsCmd.ts to import
them directly so playlist.ts remains command-only. At minimum, revise the header
rationale to reflect that these modules are no longer empty stubs and that
helper exports are load-bearing across four consumers.
src/impl/updater.ts (1)
337-340: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low value

Interpolate the tag URL-encoded. options.targetVersion reaches the path unescaped, so a value containing /, ? or # silently retargets the request to a different API path rather than failing as an unknown tag. encodeURIComponent keeps ordinary tags byte-identical.
♻️ Suggested change
-        : `${base}/repos/${repoOf(config)}/releases/tags/${tag.startsWith("v") ? tag : `v${tag}`}`
+        : `${base}/repos/${repoOf(config)}/releases/tags/${encodeURIComponent(
+            tag.startsWith("v") ? tag : `v${tag}`
+          )}`
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/impl/updater.ts` around lines 337 - 340, Update the tagged-release URL
construction in the endpoint expression to apply encodeURIComponent to the
normalized tag value before interpolation. Preserve the existing v-prefix
behavior and leave the latest-release path unchanged, ensuring ordinary tags
remain equivalent while reserved characters are escaped.
src/impl/updater.test.ts (1)
1013-1031: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Global /tmp scan can flake. The assertion fails if any unrelated or stale oytc-update-* directory exists (killed prior run, concurrent CI job on the same host). Snapshotting the pre-existing entries and asserting no new ones would keep the intent without the shared-state dependency.
♻️ Suggested tightening
     try {
+      const before = new Set(
+        (await Bun.$`ls -A /tmp`.quiet()).stdout.toString().split("\n")
+      )
       value(await run(runUpdate(f.config, noOptions)))
       const stale = (await Bun.$`ls -A /tmp`.quiet()).stdout
         .toString()
         .split("\n")
-        .filter((name) => name.startsWith("oytc-update-"))
+        .filter((name) => name.startsWith("oytc-update-") && !before.has(name))
       expect(stale).toEqual([])
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/impl/updater.test.ts` around lines 1013 - 1031, Update the “no temporary
archive survives a successful run” test to snapshot existing /tmp entries
matching “oytc-update-” before invoking run, then assert afterward that no new
matching entries were created. Keep unrelated or pre-existing directories out of
the failure condition while preserving the cleanup assertion.
src/cli/harness.testutil.ts (1)
226-264: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Extract the shared harness plumbing into one module. Both harnesses independently re-implement the error-tag set, isOytcError, frameworkExitCode, mountRoot, summaryLine, the JSON literal helpers and paramsToObject; the root cause is the absence of a common test-support module, which lets the tag list drift from src/domain/errors.ts and silently misclassify new errors as framework errors.

src/cli/harness.testutil.ts#L226-L264: move OYTC_TAGS/isOytcError/frameworkExitCode/mountRoot/summaryLine into a shared src/cli/harnessCore.testutil.ts and derive the tag set from the OytcError ADT instead of a literal list.

src/cli/p8aHarness.testutil.ts#L342-L373: delete the local copies and import them from that shared module, keeping only the P8a-specific HttpCore fake and runList.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/harness.testutil.ts` around lines 226 - 264, Extract the shared
harness utilities from src/cli/harness.testutil.ts lines 226-264 into
src/cli/harnessCore.testutil.ts, including OYTC_TAGS, isOytcError,
frameworkExitCode, mountRoot, and summaryLine; derive the error tags from the
OytcError ADT rather than maintaining a literal list. In
src/cli/p8aHarness.testutil.ts lines 342-373, remove the duplicate
implementations and import these utilities from the shared module, retaining
only the P8a-specific HttpCore fake and runList.
src/cli/validate.test.ts (1)
407-416: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add coverage for validateRequestedItems and setValues/batch.

The suite exhaustively covers the string-level checks but skips the branchiest exported helper in validate.ts: validateRequestedItems (dedupe of requested ids, the returned.size === 0 && items.length === uniqueRequested.length escape hatch, and the NotFoundError message/exit-4 path). setValues (empty-value skipping) and batch (empty input yields no chunks) are also untested here.

Want me to draft those cases?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/cli/validate.test.ts` around lines 407 - 416, Add tests in the validation
test suite for exported validateRequestedItems, covering requested-ID
deduplication, the returned.size === 0 && items.length ===
uniqueRequested.length escape hatch, and NotFoundError details including exit
code 4. Also add setValues coverage for skipping empty values and batch coverage
confirming empty input produces no chunks.
src/domain/errors.ts (1)
257-281: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

exitCodeFor duplicates exit codes already carried by each error class.

Every OytcError variant already exposes [Runtime.errorExitCode] (fixed literal or getter). This switch re-hardcodes the same numbers, so a future change to e.g. NotFoundError's exit code silently desyncs from this function unless both sites are updated.
♻️ Proposed simplification
-export const exitCodeFor = (e: OytcError): number => {
-  switch (e._tag) {
-    case "UsageError":
-      return 2
-    case "MissingKeyError":
-    case "MissingOAuthError":
-    case "AuthHintError":
-      return 3
-    case "OAuthError":
-      return e[Runtime.errorExitCode]
-    case "ApiError":
-      return apiExitCode(e)
-    case "NotFoundError":
-      return 4
-    case "CancelledError":
-      return 130
-    case "OperationalError":
-      return 6
-  }
-}
+export const exitCodeFor = (e: OytcError): number => e[Runtime.errorExitCode]
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/domain/errors.ts` around lines 257 - 281, Update exitCodeFor to return
the existing Runtime.errorExitCode value from the supplied OytcError directly,
removing the duplicated _tag switch and apiExitCode classification while
preserving each error class’s configured exit code.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/cli/auth.ts`:
- Around line 346-350: Update WrappedError so it preserves the wrapped cause’s
exit code instead of inheriting OperationalError’s hardcoded code 6. Add the
appropriate Runtime.errorExitCode override or equivalent logic to return
args.cause’s exit code, while retaining the existing prefixed message and cause
chaining.

In `@src/cli/livechat.ts`:
- Around line 328-335: Update formatFlagProvided to recognize bundled
single-dash short-flag clusters such as -qf, while preserving existing --format,
-f, and option-terminator handling. Add coverage for the clustered form through
the relevant live-chat formatting tests.

In `@src/cli/p8aHarness.testutil.ts`:
- Around line 159-185: Update the loop in runList to cap repeated page requests
when --all is enabled with no limit, preventing a scripted final page with a
persistent nextPageToken from looping forever. Preserve normal pagination and
limit-based termination, and make the harness fail or stop once the configured
request bound is exceeded.
- Around line 203-209: Update the command parameter in runCli and the matching
mountRoot wrapper so they accept actual command values instead of never. Mirror
the typing approach in harness.testutil.ts by using an appropriate generic
command parameter or any with the required eslint suppression, while preserving
the existing withSubcommands behavior.

In `@src/cli/skillsCmd.test.ts`:
- Around line 95-107: The test’s stderr assertion is coupled to the
Prompts.confirm stub because it pushes the prompt block directly into err.
Update the confirm stub in the skills command tests, including the analogous
block around the later assertion, to stop writing to err; let impl/prompts.ts
perform stderr output and assert the command’s actual stderr behavior, or limit
the test to blocks when stderr is not under test.

In `@src/cli/skillsCmd.ts`:
- Line 95: Update the variadic extra argument definition in the skills command
to use the conventional placeholder name “ARG” instead of an empty string,
matching the other commands and ensuring usage/help text displays the argument
name.
- Around line 67-82: Update the error handling around the fs.stat pipeline to
compare error.reason directly with "NotFound" rather than accessing
error.reason._tag. Preserve the existing readLink fallback for missing
destinations and the OperationalError path for other failures.

In `@src/cli/video.ts`:
- Around line 57-62: Update goJsonKind to detect RawNumber before the fallback
return and classify it as "number"; ensure RawNumber is excluded from the
object-rendering path so scalar JSON numbers do not reach renderObject, while
preserving existing array and string handling.

In `@src/impl/fileLock.ts`:
- Around line 83-129: Update the lock ownership flow around acquire and release
so an active lock cannot become stale during a long critical section. Refresh
the lockfile mtime periodically while held, or implement owner-PID validation
with an enforced critical-section bound; ensure release cannot remove a lockfile
that has been replaced by a thief. Preserve interruption behavior and the
existing stale-steal retry limits.

In `@src/impl/semver.ts`:
- Around line 57-62: Update atoi and the compareVersions flow to preserve
validated int64 components as bigint instead of converting them to number,
ensuring distinct values beyond JavaScript’s safe-integer range remain distinct
during comparison. Adjust related types and comparisons consistently, and add a
regression test covering adjacent accepted values such as 9007199254740992 and
9007199254740993.

In `@src/impl/skillInstaller.ts`:
- Around line 152-171: Make the swap sequence around the staged install renames
interruption-safe: ensure that if execution exits after the `fs.rename(target,
backup)` call but before `fs.rename(stage, target)`, the existing installation
is restored from `backup` to `target` and the backup is not stranded. Update the
surrounding cleanup/rollback logic, including the `Effect.onExit` handling and
the `fs.rename` failure path, while preserving normal backup removal after a
successful swap.

In `@src/impl/updater.test.ts`:
- Around line 1255-1282: Guard the “a genuinely unwritable directory gets the
privileges/install-script advice” test by skipping it when process.getuid?.()
=== 0, since root can bypass chmod 500. Keep the existing permission assertions
and cleanup behavior unchanged for non-root environments.

In `@src/impl/updater.ts`:
- Around line 285-315: Add a five-minute wall-clock timeout to the updater’s
HTTP request handling, covering both the streaming response path and
downloadVerified so stalled or endless connections terminate. Apply the timeout
at the shared HttpClient/request layer rather than only around size validation,
and propagate timeout failures through the existing error handling.

In `@src/json/encode.ts`:
- Around line 41-49: Update the control-character row in the encode
documentation table to reflect the short \b and \f escapes emitted by the
implementation, removing the contradictory “NOT \b / \f” wording and documenting
the current Go-compatible behavior. Do not change the encoding logic.

In `@src/json/value.ts`:
- Around line 14-24: Make the RawNumber brand unforgeable by defining and using
a module-level Symbol key (such as RawNumberKey) instead of the "$rawNumber"
string property. Update rawNumber and isRawNumber in value.ts, along with the
RawNumber property type and all encoder accesses, including encode.ts, to read
that symbol; preserve the existing single-key validation semantics while
ensuring JSON-parsed objects cannot satisfy isRawNumber.

In `@src/main.ts`:
- Around line 1-20: Update the entrypoint docstring to refer to Effect.tapCause
instead of the obsolete Effect.tapErrorCause name; change only this
documentation reference and leave the existing implementation unchanged.

In `@src/skills/references/commands.md`:
- Around line 19-21: Narrow the pagination statement in the command
documentation so it excludes the catalog commands category list, language list,
and region list, which do not register pagination flags. Preserve the existing
pagination guidance for supported public list commands and retain the separate
live-chat exception.

In `@src/util/goduration.ts`:
- Around line 64-68: Update the duration parsing logic around Number(numText) to
accumulate each component as truncated integer nanoseconds, rejecting values
that exceed signed int64 limits before adding them to the total and also
rejecting total overflow. Convert the final integer nanosecond total to
milliseconds only after parsing completes, preserving Go’s fractional truncation
and overflow semantics.

In `@src/util/gostring.ts`:
- Around line 15-25: Update compareUtf8 to compare UTF-8 encoded byte arrays
rather than raw Unicode code points, ensuring lone surrogates follow
TextEncoder’s U+FFFD replacement behavior. Preserve lexicographic ordering and
length-based tie-breaking after comparing encoded bytes.

---

Nitpick comments:
In @.depot/workflows/ci.yml:
- Around line 15-18: Use a shared Bun version source instead of duplicating the
literal: update .depot/workflows/ci.yml lines 15-18 to expose BUN_VERSION as a
repository or organization variable (or equivalent shared workflow source), and
update .depot/workflows/release.yml lines 55-58 to reference that shared value
for bun-version. Remove any synchronization-only comment or hardcoded duplicate
while preserving the pinned version.

In `@package.json`:
- Line 13: Update the `@types/bun` dependency in package.json to a fixed version
matching BUN_VERSION, or use a tested compatible version range instead of
latest. Keep the dependency aligned with the Bun version used by the project so
lockfile refreshes remain controlled.

In `@site/install.ps1`:
- Around line 23-31: Wrap the Win32_Processor query in the
architecture-detection block around $processorArchitecture with try/catch so
CIM/WMI failures do not surface as raw exceptions. On failure, gracefully retain
the existing amd64 default and continue installation, while preserving the ARM64
and 32-bit validation behavior when detection succeeds.

In `@src/cli/analyticsCmd.test.ts`:
- Around line 292-302: Make the tests deterministic by capturing the expected
UTC date relative to the same construction-time reference used by DEFAULT_RANGE
instead of calling new Date() during assertion. Replace the self-comparison in
the materialization test with a formatDateOnly-based recomputation comparison
that would differ if DEFAULT_RANGE were evaluated per access, while preserving
the existing range assertions.

In `@src/cli/analyticsCmd.ts`:
- Around line 81-107: The local renderResult in src/cli/analyticsCmd.ts#L81-L107
should be removed and replaced with an import from ./render.ts, passing the
preset default columns. In src/cli/livechat.ts#L214-L247, remove the local
writeErr/renderResult pair and use the shared renderResult helper for live-chat
list, preserving the existing options and default-column behavior.
- Around line 370-375: Define a separate analyticsVideoMetrics constant for the
metrics list and use it in the runAnalytics call, while retaining
analyticsVideoColumns for columns. Follow the existing analyticsOverviewMetrics
pattern so future column-only changes do not alter the requested metrics.

In `@src/cli/auth.test.ts`:
- Around line 60-71: Update the SECRET_API_KEY fixture to use a neutral,
non-provider-specific prefix while keeping it distinctive and greppable.
Preserve the FORBIDDEN collection and all related assertions unchanged.

In `@src/cli/auth.ts`:
- Around line 84-99: The raw Stdio writing logic is duplicated across both CLI
files. In src/cli/auth.ts lines 84-99, move write, writeOut, and writeErr into a
shared module and import them; in src/cli/versionUpdate.ts lines 54-62, remove
the local writeOut implementation and import the shared helper, preserving the
existing Stream.run behavior and OperationalError wrapping.
- Line 64: Rename the playlist.ts-exported exactArgs helper to a distinct name
such as exactPositionals, then update its imports and usages in auth.ts and
versionUpdate.ts while leaving validate.ts’s exactArgs unchanged.

In `@src/cli/harness.testutil.ts`:
- Around line 226-264: Extract the shared harness utilities from
src/cli/harness.testutil.ts lines 226-264 into src/cli/harnessCore.testutil.ts,
including OYTC_TAGS, isOytcError, frameworkExitCode, mountRoot, and summaryLine;
derive the error tags from the OytcError ADT rather than maintaining a literal
list. In src/cli/p8aHarness.testutil.ts lines 342-373, remove the duplicate
implementations and import these utilities from the shared module, retaining
only the P8a-specific HttpCore fake and runList.

In `@src/cli/livechat.test.ts`:
- Around line 488-495: Remove the redundant expect(await exit).toBeDefined()
assertion from the offlineAt test, retaining the meaningful success and API-call
checks. Rename the help test around the referenced lines so its title accurately
describes printing help and exiting with failure, matching its Exit.isFailure
assertion.

In `@src/cli/livechat.ts`:
- Around line 434-441: The live chat polling loop’s seen set grows indefinitely
during long-running streams. Replace the unbounded seen structure in the stream
flow with a bounded insertion-ordered structure that evicts the oldest message
IDs at a defined capacity, while preserving duplicate suppression for retained
IDs and the existing emitted/request behavior.

In `@src/cli/playlist.ts`:
- Around line 6-19: Move the shared helpers from playlist.ts into their owning
modules validate.ts, fields.ts, and render.ts, then update comment.ts,
subscription.ts, catalog.ts, and skillsCmd.ts to import them directly so
playlist.ts remains command-only. At minimum, revise the header rationale to
reflect that these modules are no longer empty stubs and that helper exports are
load-bearing across four consumers.

In `@src/cli/validate.test.ts`:
- Around line 407-416: Add tests in the validation test suite for exported
validateRequestedItems, covering requested-ID deduplication, the returned.size
=== 0 && items.length === uniqueRequested.length escape hatch, and NotFoundError
details including exit code 4. Also add setValues coverage for skipping empty
values and batch coverage confirming empty input produces no chunks.

In `@src/cli/video.test.ts`:
- Around line 24-34: Type the script parameter as ApiScript in the get, stats,
popular, and trainability helpers, and add ApiScript to the existing type
import. Preserve the default empty script while ensuring invalid keys are
rejected by type checking.

In `@src/domain/errors.ts`:
- Around line 257-281: Update exitCodeFor to return the existing
Runtime.errorExitCode value from the supplied OytcError directly, removing the
duplicated _tag switch and apiExitCode classification while preserving each
error class’s configured exit code.

In `@src/impl/archive.test.ts`:
- Around line 325-339: Update withTempFile to create its temporary directory
using fs.promises.mkdtemp with os.tmpdir() and the existing “oytc-archive-test-”
prefix. Replace the Bun.$ mkdir and rm shell-outs while preserving file
creation, callback execution, and guaranteed cleanup in the finally block.

In `@src/impl/atomicWrite.ts`:
- Line 29: Update tempName in atomicWrite.ts to use a cryptographically secure
random value from the crypto module instead of Math.random(), while preserving
the existing .auth-… .tmp filename format and return type.

In `@src/impl/credentialStore.worker.ts`:
- Line 42: Validate the result of Number.parseInt in the worker iteration setup
and fail explicitly when iterations is non-numeric, preventing a vacuous
successful run. In the worker failure-reporting path, replace String(exit.cause)
with Cause.pretty and add Cause to the existing effect import so the full
failure detail is surfaced.

In `@src/impl/httpCore.test.ts`:
- Around line 916-922: Replace the non-behavioral assertion in the “the
HttpClient service is what actually issues the request” test with an execution
of makeHttpCore/getJson using the mocked HttpClient layer. Assert that the
request fails with the layer’s “should not be reached” defect, proving the
implementation uses HttpClient rather than calling fetch directly; otherwise
remove the test.

In `@src/impl/oauth.ts`:
- Around line 253-254: Update the response-body handling around decodeBody and
the code paths at the referenced response processing sites so the body is read
through a streaming, byte-limited mechanism before buffering or decoding.
Enforce BODY_LIMIT_BYTES during reads, preserving truncation at the limit and
avoiding response.arrayBuffer() materializing the full response first.

In `@src/impl/skillInstaller.test.ts`:
- Around line 238-250: Update the test around installSkill so the locked
directory’s chmodSync restoration runs in a finally block, regardless of
assertion or installation failures. Preserve the existing assertions and restore
mode to 0o700 before teardown.

In `@src/impl/updater.test.ts`:
- Around line 1013-1031: Update the “no temporary archive survives a successful
run” test to snapshot existing /tmp entries matching “oytc-update-” before
invoking run, then assert afterward that no new matching entries were created.
Keep unrelated or pre-existing directories out of the failure condition while
preserving the cleanup assertion.

In `@src/impl/updater.ts`:
- Around line 337-340: Update the tagged-release URL construction in the
endpoint expression to apply encodeURIComponent to the normalized tag value
before interpolation. Preserve the existing v-prefix behavior and leave the
latest-release path unchanged, ensuring ordinary tags remain equivalent while
reserved characters are escaped.

In `@src/impl/youtubeApi.ts`:
- Around line 48-53: Update setParam to collapse duplicate entries for the
target key: preserve the first occurrence with the new value and remove all
later occurrences, while continuing to append the key when it is absent. Keep
other parameter entries and their order unchanged.

In `@src/skills/bundle.ts`:
- Around line 21-26: Add an ambient "*.md" module declaration in an appropriate
TypeScript declaration file, defining the default export as a string. Remove the
three `@ts-ignore` directives from the markdown imports in the bundle module and
remove the related as string casts, relying on the declaration for typing.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1f5625c3-9eb2-4db3-a0be-e9c746832998

📥 Commits

Reviewing files that changed from the base of the PR and between d4f2d24 and a2bf316.

⛔ Files ignored due to path filters (2)

bun.lock is excluded by !**/*.lock
go.sum is excluded by !**/*.sum

📒 Files selected for processing (160)

.depot/workflows/ci.yml
.depot/workflows/release.yml
.gitignore
Makefile
README.md
cmd/oytc/main.go
cmd/oytc/main_test.go
dev
docs/releasing.md
go.mod
internal/analytics/client.go
internal/analytics/client_test.go
internal/cli/analytics.go
internal/cli/app.go
internal/cli/app_test.go
internal/cli/auth.go
internal/cli/channel_video.go
internal/cli/fields.go
internal/cli/fields_test.go
internal/cli/live_chat.go
internal/cli/resources.go
internal/cli/skills.go
internal/cli/skills_test.go
internal/cli/version_update.go
internal/cli/version_update_test.go
internal/config/config.go
internal/config/config_test.go
internal/config/lock_unix.go
internal/config/lock_windows.go
internal/config/rename_unix.go
internal/config/rename_windows.go
internal/oauth/oauth.go
internal/oauth/oauth_test.go
internal/output/output.go
internal/output/output_test.go
internal/skill/install.go
internal/skill/install_test.go
internal/update/update.go
internal/update/update_test.go
internal/version/version.go
internal/version/version_test.go
internal/youtube/client.go
internal/youtube/client_test.go
internal/youtube/list.go
package.json
scripts/package.sh
site/index.html
site/install.ps1
site/install.sh
skills/oytc/embed.go
src/cli/analyticsCmd.test.ts
src/cli/analyticsCmd.ts
src/cli/auth.test.ts
src/cli/auth.ts
src/cli/catalog.test.ts
src/cli/catalog.ts
src/cli/channel.test.ts
src/cli/channel.ts
src/cli/comment.test.ts
src/cli/comment.ts
src/cli/fields.test.ts
src/cli/fields.ts
src/cli/flags.ts
src/cli/globals.ts
src/cli/harness.testutil.ts
src/cli/livechat.test.ts
src/cli/livechat.ts
src/cli/p8aHarness.testutil.ts
src/cli/playlist.test.ts
src/cli/playlist.ts
src/cli/render.test.ts
src/cli/render.ts
src/cli/root.ts
src/cli/search.test.ts
src/cli/search.ts
src/cli/skillsCmd.test.ts
src/cli/skillsCmd.ts
src/cli/subscription.test.ts
src/cli/subscription.ts
src/cli/validate.test.ts
src/cli/validate.ts
src/cli/versionUpdate.test.ts
src/cli/versionUpdate.ts
src/cli/video.test.ts
src/cli/video.ts
src/domain/errors.test.ts
src/domain/errors.ts
src/domain/listResult.ts
src/effect.ts
src/impl/analyticsApi.test.ts
src/impl/analyticsApi.ts
src/impl/archive.test.ts
src/impl/archive.ts
src/impl/atomicWrite.test.ts
src/impl/atomicWrite.ts
src/impl/browserOpener.test.ts
src/impl/browserOpener.ts
src/impl/credentialStore.test.ts
src/impl/credentialStore.ts
src/impl/credentialStore.worker.ts
src/impl/fileLock.test.ts
src/impl/fileLock.ts
src/impl/httpCore.test.ts
src/impl/httpCore.ts
src/impl/oauth.test.ts
src/impl/oauth.ts
src/impl/oauthServer.test.ts
src/impl/oauthServer.ts
src/impl/platformMatrix.test.ts
src/impl/platformMatrix.ts
src/impl/processEnv.ts
src/impl/prompts.test.ts
src/impl/prompts.ts
src/impl/renderer.test.ts
src/impl/renderer.ts
src/impl/resolveChannel.test.ts
src/impl/resolveChannel.ts
src/impl/semver.test.ts
src/impl/semver.ts
src/impl/skillInstaller.test.ts
src/impl/skillInstaller.ts
src/impl/tokenSource.test.ts
src/impl/tokenSource.ts
src/impl/updater.test.ts
src/impl/updater.ts
src/impl/versionInfo.test.ts
src/impl/versionInfo.ts
src/impl/youtubeApi.test.ts
src/impl/youtubeApi.ts
src/json/codec.test.ts
src/json/encode.ts
src/json/parse.ts
src/json/value.ts
src/layers.ts
src/main.ts
src/output/columns.test.ts
src/output/columns.ts
src/output/jsonOut.test.ts
src/output/jsonOut.ts
src/output/table.test.ts
src/output/table.ts
src/output/tsv.test.ts
src/output/tsv.ts
src/schema/accessors.ts
src/schema/analytics.ts
src/schema/authfile.test.ts
src/schema/authfile.ts
src/schema/dataapi.ts
src/schema/errorEnvelope.test.ts
src/schema/errorEnvelope.ts
src/services/index.ts
src/skills/SKILL.md
src/skills/bundle.test.ts
src/skills/bundle.ts
src/skills/references/commands.md
src/skills/references/recipes.md
src/util/goduration.test.ts
src/util/goduration.ts
src/util/gostring.ts
tsconfig.json

💤 Files with no reviewable changes (38)

go.mod
internal/cli/resources.go
internal/config/lock_windows.go
skills/oytc/embed.go
cmd/oytc/main.go
internal/config/rename_windows.go
internal/version/version.go
internal/cli/skills.go
internal/version/version_test.go
internal/cli/auth.go
internal/cli/version_update_test.go
internal/analytics/client.go
internal/skill/install.go
internal/cli/fields_test.go
internal/cli/app_test.go
internal/cli/version_update.go
internal/analytics/client_test.go
internal/config/lock_unix.go
internal/cli/analytics.go
internal/cli/channel_video.go
internal/oauth/oauth_test.go
internal/cli/fields.go
internal/update/update_test.go
internal/config/rename_unix.go
internal/output/output.go
internal/skill/install_test.go
internal/cli/live_chat.go
internal/cli/skills_test.go
internal/output/output_test.go
internal/config/config_test.go
internal/youtube/client_test.go
internal/oauth/oauth.go
internal/youtube/client.go
cmd/oytc/main_test.go
internal/config/config.go
internal/cli/app.go
internal/update/update.go
internal/youtube/list.go

coderabbitai · 2026-07-25T10:22:13Z

+class WrappedError extends OperationalError {
+  constructor(args: { readonly prefix: string; readonly cause: OytcError }) {
+    super({ message: `${args.prefix}: ${args.cause.message}`, cause: args.cause })
+  }
+}


🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash fd -t f 'errors.ts' src/domain --exec cat -n rg -nP --type=ts 'exitCode' -C4 src

Repository: davis7dotsh/open-yt-cli

Length of output: 50379

WrappedError does not preserve the cause’s exit code
OperationalError is hardcoded to exit 6, so wrapping an ApiError here still exits 6 instead of 3. If this wrapper is meant to forward the cause’s code, it needs its own Runtime.errorExitCode logic rather than only passing cause to OperationalError.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/auth.ts` around lines 346 - 350, Update WrappedError so it preserves the wrapped cause’s exit code instead of inheriting OperationalError’s hardcoded code 6. Add the appropriate Runtime.errorExitCode override or equivalent logic to return args.cause’s exit code, while retaining the existing prefixed message and cause chaining.

coderabbitai · 2026-07-25T10:22:13Z

+export const formatFlagProvided = (argv: ReadonlyArray<string>): boolean => {
+  for (const argument of argv) {
+    if (argument === "--") return false
+    if (argument === "--format" || argument.startsWith("--format=")) return true
+    if (argument === "-f" || argument.startsWith("-f=")) return true
+  }
+  return false
+}


🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash # Does the CLI lexer support clustered short flags like -qf? fd -t f 'lexer' --exec cat -n {} \; | head -200

Repository: davis7dotsh/open-yt-cli

Length of output: 161

🏁 Script executed:

#!/bin/bash set -euo pipefail printf '\n== files ==\n' git ls-files | rg '(^src/cli/livechat\.ts$|lexer|parse|flags|arg)' printf '\n== livechat usage ==\n' rg -n "formatFlagProvided|--format|-f|cluster|short flag|argv" src/cli/livechat.ts src -g '!**/dist/**' printf '\n== candidate parser files ==\n' fd -t f -i 'lexer|arg|flag|parse' src printf '\n== relevant outlines ==\n' for f in $(fd -t f -i 'lexer|arg|flag|parse' src | head -20); do echo "--- $f" ast-grep outline "$f" --view expanded 2>/dev/null | sed -n '1,120p' done

Repository: davis7dotsh/open-yt-cli

Length of output: 40984

🏁 Script executed:

#!/bin/bash set -euo pipefail printf '\n== src/cli/livechat.ts excerpt ==\n' cat -n src/cli/livechat.ts | sed -n '300,360p' printf '\n== search for command-line parsing ==\n' rg -n "process\.argv|argv|yargs|commander|minimist|parseArgs|slice$2$|cluster|short flag" src package.json . -g '!**/dist/**' -g '!**/node_modules/**'

Repository: davis7dotsh/open-yt-cli

Length of output: 18368

🏁 Script executed:

#!/bin/bash set -euo pipefail printf '\n== livechat excerpt ==\n' cat -n src/cli/livechat.ts | sed -n '1,220p' printf '\n== parser/lexer candidates ==\n' fd -t f -i 'lexer|parser|parse|argv|flag' .

Repository: davis7dotsh/open-yt-cli

Length of output: 10558

🏁 Script executed:

#!/bin/bash set -euo pipefail echo '== initial log ==' sed -n '1,250p' /tmp/coderabbit-shell-logs/shell-output-nFX7TU || true

Repository: davis7dotsh/open-yt-cli

Length of output: 179

🏁 Script executed:

#!/bin/bash set -euo pipefail sed -n '1,220p' /tmp/coderabbit-shell-logs/shell-output-nFX7TU

Repository: davis7dotsh/open-yt-cli

Length of output: 161

🏁 Script executed:

#!/bin/bash set -euo pipefail printf '\n== src/cli/flags.ts ==\n' cat -n src/cli/flags.ts | sed -n '1,220p' printf '\n== short-flag handling in cli layer ==\n' rg -n "short|alias|cluster|Flag\.|withAlias|withDescription|withDefault" src/cli src/effect.ts

Repository: davis7dotsh/open-yt-cli

Length of output: 25563

🏁 Script executed:

#!/bin/bash set -euo pipefail printf '\n== references to clustered short flags ==\n' rg -n --hidden --glob '!**/dist/**' --glob '!**/node_modules/**' \ 'cluster|clustered|short flags|short flag|lexed as short flags|`-qf`|`-f` alias|withAlias$"f"$' src docs README.md printf '\n== livechat tests around formatFlagProvided ==\n' cat -n src/cli/livechat.test.ts | sed -n '415,460p' printf '\n== framework flag API surface ==\n' cat -n src/effect.ts | sed -n '1,260p'

Repository: davis7dotsh/open-yt-cli

Length of output: 4045

🌐 Web query:

Effect CLI short flag clustering withAlias bundled flags -qf -f cluster documentation

💡 Result:

The Effect CLI library supports short flag clustering (also known as flag bundling), where multiple short flags can be provided in a single token, such as -abc [1][2]. This functionality is handled automatically by the library's internal parser, which recognizes tokens starting with a single hyphen as potentially containing multiple character-based flags [1][2]. Regarding your specific keywords: 1. Short Flag Clustering: The parser treats a short flag token (e.g., -abc) as multiple distinct flags (e.g., -a, -b, and -c) [1][2]. 2. Aliasing: You can define aliases for any flag using the Flag.withAlias combinator [3][4][5]. For example, a boolean flag created as Flag.boolean("verbose") can be assigned a short alias via.pipe(Flag.withAlias("v")), allowing it to be triggered by either --verbose or -v [3][4]. 3. Bundling: Because the parser automatically decomposes clustered tokens into individual flags, no manual configuration is required to enable this behavior [1][2]. Effect CLI also supports built-in flags (such as --help and --version) which have their own default short aliases (e.g., -h and -v, respectively) [4][6]. These built-ins are processed by the library's internal parsing pipeline, which operates in phases: lexical analysis (including clustering), built-in extraction, and command-specific parsing [1][2]. You can manage flag aliases and definitions using the Flag module within the @effect/cli package [3][7].

Citations:

1: https://github.com/Effect-TS/effect/blob/main/packages/effect/src/unstable/cli/internal/parser.ts

2: https://github.com/Effect-TS/effect/blob/2e9a34ac/packages/effect/src/unstable/cli/internal/parser.ts

3: https://github.com/Effect-TS/effect/blob/main/packages/effect/src/unstable/cli/Flag.ts

4: https://effect-ts-effect-smol.mintlify.app/unstable/cli

5: https://deepwiki.com/Effect-TS/effect/9.1-cli-framework

6: https://github.com/Effect-TS/effect/blob/main/packages/effect/src/unstable/cli/GlobalFlag.ts

7: https://effect-ts.github.io/effect/docs/cli

Clustered -f forms need to be detected here. -qf tsv is accepted as bundled short flags, but formatFlagProvided() only matches exact -f/-f= tokens, so live-chat stream can still fall back to jsonl unexpectedly. Handle single-dash clusters containing f and add a test for that form.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/livechat.ts` around lines 328 - 335, Update formatFlagProvided to recognize bundled single-dash short-flag clusters such as -qf, while preserving existing --format, -f, and option-terminator handling. Add coverage for the clustered form through the relevant live-chat formatting tests.

coderabbitai · 2026-07-25T10:22:13Z

+  for (;;) {
+    const page = pages[Math.min(index, Math.max(pages.length - 1, 0))] ?? { items: [] }
+    onRequest()
+    requests++
+    index++
+
+    let pageItems = [...page.items]
+    // The filter runs BEFORE the limit, so rejected items do not count toward
+    // it and a page can contribute zero items while still consuming a request.
+    if (options.filter !== undefined) pageItems = pageItems.filter(options.filter)
+
+    let truncated = false
+    if (options.limit > 0 && kept.length + pageItems.length > options.limit) {
+      pageItems = pageItems.slice(0, options.limit - kept.length)
+      truncated = true
+    }
+    kept.push(...pageItems)
+    nextPageToken = truncated ? "" : (page.nextPageToken ?? "")
+
+    if (
+      !options.all ||
+      nextPageToken === "" ||
+      (options.limit > 0 && kept.length >= options.limit)
+    ) {
+      break
+    }
+  }


🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

runList can loop forever on a repeating last page.

The page selector clamps to the last scripted page, so with --all, limit === 0 and a last page whose nextPageToken is non-empty, nextPageToken never becomes "" and the limit break never fires — the harness spins instead of failing, hanging the test run. The real impl bounds --all; add a cap here too.

🛡️ Proposed guard

let index = 0 + // Scripts are finite; refuse to re-serve past the end so a last page with a + // resume token cannot spin under `--all`. + const maxRequests = Math.max(pages.length, 1) for (;;) { - const page = pages[Math.min(index, Math.max(pages.length - 1, 0))] ?? { items: [] } + const page = pages[index] ?? { items: [] } onRequest() requests++ index++ @@ if ( !options.all || nextPageToken === "" || + requests >= maxRequests || (options.limit > 0 && kept.length >= options.limit) ) { break }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

for (;;) {

const page = pages[Math.min(index, Math.max(pages.length - 1, 0))] ?? { items: [] }

onRequest()

requests++

index++

let pageItems = [...page.items]

// The filter runs BEFORE the limit, so rejected items do not count toward

// it and a page can contribute zero items while still consuming a request.

if (options.filter !== undefined) pageItems = pageItems.filter(options.filter)

let truncated = false

if (options.limit > 0 && kept.length + pageItems.length > options.limit) {

pageItems = pageItems.slice(0, options.limit - kept.length)

truncated = true

}

kept.push(...pageItems)

nextPageToken = truncated ? "" : (page.nextPageToken ?? "")

if (

!options.all ||

nextPageToken === "" ||

(options.limit > 0 && kept.length >= options.limit)

) {

break

}

}

let index = 0

// Scripts are finite; refuse to re-serve past the end so a last page with a

// resume token cannot spin under `--all`.

const maxRequests = Math.max(pages.length, 1)

for (;;) {

const page = pages[index] ?? { items: [] }

onRequest()

requests++

index++

let pageItems = [...page.items]

// The filter runs BEFORE the limit, so rejected items do not count toward

// it and a page can contribute zero items while still consuming a request.

if (options.filter !== undefined) pageItems = pageItems.filter(options.filter)

let truncated = false

if (options.limit > 0 && kept.length + pageItems.length > options.limit) {

pageItems = pageItems.slice(0, options.limit - kept.length)

truncated = true

}

kept.push(...pageItems)

nextPageToken = truncated ? "" : (page.nextPageToken ?? "")

if (

!options.all ||

nextPageToken === "" ||

requests >= maxRequests ||

(options.limit > 0 && kept.length >= options.limit)

) {

break

}

}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/p8aHarness.testutil.ts` around lines 159 - 185, Update the loop in runList to cap repeated page requests when --all is enabled with no limit, preventing a scripted final page with a persistent nextPageToken from looping forever. Preserve normal pagination and limit-based termination, and make the harness fail or stop once the configured request bound is exceeded.

coderabbitai · 2026-07-25T10:22:13Z

+export const runCli = (
+  // The concrete Command type is a five-parameter generic whose Input differs
+  // per command; the harness only ever passes it to `withSubcommands`.
+  command: never,
+  argv: ReadonlyArray<string>,
+  options: RunOptions = {}
+): Promise<RunResult> => {


🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash # Confirm a typecheck task exists and that call sites pass concrete commands. fd -H -t f 'package.json' -d 2 --exec jq '.scripts' {} fd -H -t f 'tsconfig.json' -d 2 --exec cat {} rg -nP -C2 '\brunCli\s*\(' --iglob '*.test.ts' rg -n 'command: never|command: any' src/cli

Repository: davis7dotsh/open-yt-cli

Length of output: 955

🏁 Script executed:

#!/bin/bash set -euo pipefail # Inspect the harness signatures and nearby call sites. ast-grep outline src/cli/p8aHarness.testutil.ts --view expanded printf '\n---\n' sed -n '180,240p' src/cli/p8aHarness.testutil.ts printf '\n---\n' sed -n '330,390p' src/cli/p8aHarness.testutil.ts printf '\n---\n' rg -n -C2 '\brunCli\s*\(|\bmountRoot\s*\(' src --glob '*.ts' printf '\n---\n' rg -n 'command:\s*(never|any)' src/cli

Repository: davis7dotsh/open-yt-cli

Length of output: 13440

🏁 Script executed:

#!/bin/bash set -euo pipefail # Check the sibling harness pattern the comment mentions. rg -n -C3 'no-explicit-any|command: any|command: never|mountRoot|withSubcommands' src/cli --glob '*testutil.ts'

Repository: davis7dotsh/open-yt-cli

Length of output: 4323

🏁 Script executed:

#!/bin/bash set -euo pipefail # Read the exact signatures and any related types around them. sed -n '1,120p' src/cli/p8aHarness.testutil.ts printf '\n---\n' sed -n '120,240p' src/cli/p8aHarness.testutil.ts printf '\n---\n' sed -n '320,390p' src/cli/p8aHarness.testutil.ts

Repository: davis7dotsh/open-yt-cli

Length of output: 11303

🏁 Script executed:

#!/bin/bash set -euo pipefail # Focus on the exact harness API around the reported lines. nl -ba src/cli/p8aHarness.testutil.ts | sed -n '190,220p' printf '\n---\n' nl -ba src/cli/p8aHarness.testutil.ts | sed -n '350,370p' printf '\n---\n' rg -n -C2 'withSubcommands|Command|never|any' src/cli/p8aHarness.testutil.ts

Repository: davis7dotsh/open-yt-cli

Length of output: 201

command: never makes the harness unusable under tsc
runCli(searchCommand, …) and the matching mountRoot wrapper can’t accept any real command value. Mirror src/cli/harness.testutil.ts with any + // eslint-disable-next-line @typescript-eslint/no-explicit-any``, or introduce a generic command parameter.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/p8aHarness.testutil.ts` around lines 203 - 209, Update the command parameter in runCli and the matching mountRoot wrapper so they accept actual command values instead of never. Mirror the typing approach in harness.testutil.ts by using an appropriate generic command parameter or any with the required eslint suppression, while preserving the existing withSubcommands behavior.

coderabbitai · 2026-07-25T10:22:13Z

+    Layer.succeed(Prompts, {
+      readLine: () => Effect.succeed(""),
+      readSecret: () => Effect.succeed(undefined as never),
+      confirm: (block: string) =>
+        Effect.suspend(() => {
+          blocks.push(block)
+          err.push(block)
+          return options.confirmError === undefined
+            ? Effect.succeed(options.confirmed ?? true)
+            : Effect.fail(options.confirmError)
+        })
+    })
+  )


📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

The stderr assertion tests the stub, not the command. The Prompts.confirm stub itself pushes block into err (Line 101), so "the prompt block goes to STDERR" passes regardless of where the real prompt writes. Let impl/prompts.ts do the writing (or assert on blocks only) so the test has teeth.

Also applies to: 216-226

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/skillsCmd.test.ts` around lines 95 - 107, The test’s stderr assertion is coupled to the Prompts.confirm stub because it pushes the prompt block directly into err. Update the confirm stub in the skills command tests, including the analogous block around the later assertion, to stop writing to err; let impl/prompts.ts perform stderr output and assert the command’s actual stderr behavior, or limit the test to blocks when stderr is not under test.

coderabbitai · 2026-07-25T10:22:14Z

+export interface RawNumber {
+  readonly $rawNumber: string
+}
+
+export const rawNumber = (literal: string): RawNumber => ({ $rawNumber: literal })
+
+export const isRawNumber = (u: unknown): u is RawNumber =>
+  typeof u === "object" &&
+  u !== null &&
+  typeof (u as RawNumber).$rawNumber === "string" &&
+  Object.keys(u as object).length === 1


🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

RawNumber's brand is forgeable by parsed input.

$rawNumber is a plain string-keyed property, so any decoded payload containing {"$rawNumber":"..."} satisfies isRawNumber and src/json/encode.ts line 128 then splices that string into the output unescaped — {"$rawNumber":"1"} renders as bare 1, and a value containing a quote or brace would emit invalid JSON. Unreachable with today's YouTube payloads, but a Symbol key makes it unforgeable via JSON.parse at essentially no cost.

🛡️ Proposed hardening

-export interface RawNumber { - readonly $rawNumber: string -} - -export const rawNumber = (literal: string): RawNumber => ({ $rawNumber: literal }) - -export const isRawNumber = (u: unknown): u is RawNumber => - typeof u === "object" && - u !== null && - typeof (u as RawNumber).$rawNumber === "string" && - Object.keys(u as object).length === 1 +const RawNumberKey: unique symbol = Symbol("oytc/RawNumber") + +export interface RawNumber { + readonly [RawNumberKey]: string +} + +export const rawNumber = (literal: string): RawNumber => ({ [RawNumberKey]: literal }) + +export const isRawNumber = (u: unknown): u is RawNumber => + typeof u === "object" && u !== null && typeof (u as RawNumber)[RawNumberKey] === "string"

encode.ts line 128 and value.ts line 44/53 then read value[RawNumberKey]. Note this also makes Object.keys-based rendering skip the marker, which is what you want.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

export interface RawNumber {

readonly $rawNumber: string

}

export const rawNumber = (literal: string): RawNumber => ({ $rawNumber: literal })

export const isRawNumber = (u: unknown): u is RawNumber =>

typeof u === "object" &&

u !== null &&

typeof (u as RawNumber).$rawNumber === "string" &&

Object.keys(u as object).length === 1

const RawNumberKey: unique symbol = Symbol("oytc/RawNumber")

export interface RawNumber {

readonly [RawNumberKey]: string

}

export const rawNumber = (literal: string): RawNumber => ({ [RawNumberKey]: literal })

export const isRawNumber = (u: unknown): u is RawNumber =>

typeof u === "object" && u !== null && typeof (u as RawNumber)[RawNumberKey] === "string"

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/json/value.ts` around lines 14 - 24, Make the RawNumber brand unforgeable by defining and using a module-level Symbol key (such as RawNumberKey) instead of the "$rawNumber" string property. Update rawNumber and isRawNumber in value.ts, along with the RawNumber property type and all encoder accesses, including encode.ts, to read that symbol; preserve the existing single-key validation semantics while ensuring JSON-parsed objects cannot satisfy isRawNumber.

coderabbitai · 2026-07-25T10:22:14Z

+/**
+ * Entrypoint.
+ *
+ * Every failure is printed as exactly one line — `oytc: <message>` on stderr —
+ * with no usage block, stack trace, or color. Go achieved this by setting
+ * cobra's SilenceErrors/SilenceUsage and routing every error through a single
+ * printer in main().
+ *
+ * Reproducing it here takes two pieces:
+ *
+ *   1. Every tagged error carries `Runtime.errorReported = false`, which stops
+ *      the runtime's default (multi-line, annotated) reporter from firing.
+ *      That alone leaves stderr EMPTY, so...
+ *   2. ...this file installs the printer. `Effect.tapErrorCause` catches the
+ *      failure on its way out, writes the single line, and re-fails so the
+ *      exit code still comes from `Runtime.errorExitCode`.
+ *
+ * The CLI framework's own parse errors need translating: it exits 1 where Go
+ * exits 2, and its messages differ in wording. Both are handled below.
+ */


📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Effect v4 migration Effect.tapErrorCause tapCause rename

💡 Result:

In Effect v4, the API Effect.tapErrorCause has been renamed to Effect.tapCause [1][2]. Similarly, Layer.tapErrorCause has been renamed to Layer.tapCause [1][2]. This change is part of a broader simplification of the Effect API surface in v4, which included renaming several combinators to be more concise [3][4]. Specifically, many APIs involving the Cause type were streamlined to align with the new naming conventions [1]. Summary of relevant renames: - Effect.tapErrorCause -> Effect.tapCause [1] - Layer.tapErrorCause -> Layer.tapCause [1][2] These combinators continue to serve the same purpose as their v3 counterparts: they allow you to run an effectful observer on a failure (specifically accessing the full Cause, including defects and interruptions) while preserving the original error/cause output of the effect [2].

Citations:

1: https://github.com/Effect-TS/effect-smol/blob/main/migration/v3-to-v4.md

2: EFF-705 Add Layer.tap apis Effect-TS/effect-smol#1741

3: https://github.com/Effect-TS/effect-smol/blob/main/migration/error-handling.md

4: https://github.com/Effect-TS/effect-smol/blob/main/MIGRATION.md

🏁 Script executed:

sed -n '190,250p' src/main.ts

Repository: davis7dotsh/open-yt-cli

Length of output: 2230

🏁 Script executed:

rg -n "tapErrorCause|tapCause|catch|findErrorOption|printFailure" src/main.ts

Repository: davis7dotsh/open-yt-cli

Length of output: 501

Update the docstring to Effect.tapCause.
Effect.tapErrorCause is the old name; the code already uses the current API, so only the comment needs updating.

📝 Proposed doc fix

- * 2. ...this file installs the printer. `Effect.tapErrorCause` catches the + * 2. ...this file installs the printer. `Effect.tapCause` catches the

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

/**

* Entrypoint.

*

* Every failure is printed as exactly one line — `oytc: <message>` on stderr —

* with no usage block, stack trace, or color. Go achieved this by setting

* cobra's SilenceErrors/SilenceUsage and routing every error through a single

* printer in main().

*

* Reproducing it here takes two pieces:

*

* 1. Every tagged error carries `Runtime.errorReported = false`, which stops

* the runtime's default (multi-line, annotated) reporter from firing.

* That alone leaves stderr EMPTY, so...

* 2. ...this file installs the printer. `Effect.tapErrorCause` catches the

* failure on its way out, writes the single line, and re-fails so the

* exit code still comes from `Runtime.errorExitCode`.

*

* The CLI framework's own parse errors need translating: it exits 1 where Go

* exits 2, and its messages differ in wording. Both are handled below.

*/

/**

* Entrypoint.

*

* Every failure is printed as exactly one line — `oytc: <message>` on stderr —

* with no usage block, stack trace, or color. Go achieved this by setting

* cobra's SilenceErrors/SilenceUsage and routing every error through a single

* printer in main().

*

* Reproducing it here takes two pieces:

*

* 1. Every tagged error carries `Runtime.errorReported = false`, which stops

* the runtime's default (multi-line, annotated) reporter from firing.

* That alone leaves stderr EMPTY, so...

* 2. ...this file installs the printer. `Effect.tapCause` catches the

* failure on its way out, writes the single line, and re-fails so the

* exit code still comes from `Runtime.errorExitCode`.

*

* The CLI framework's own parse errors need translating: it exits 1 where Go

* exits 2, and its messages differ in wording. Both are handled below.

*/

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main.ts` around lines 1 - 20, Update the entrypoint docstring to refer to Effect.tapCause instead of the obsolete Effect.tapErrorCause name; change only this documentation reference and leave the existing implementation unchanged.

coderabbitai · 2026-07-25T10:22:14Z

+Public list commands use an API key (or fall back to an OAuth grant that includes
+`youtube.readonly`): `--page-size N`, `--page-token T`, `--all`, `--limit N`. Analytics
+commands always require OAuth.


📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Pagination claim is too broad for the catalog commands.

category list, language list and region list expose no pagination flags at all (src/cli/catalog.ts passes a fixed zero page and the flags are not registered), so an agent following this line will emit --page-size/--all and get exit 2. Worth carving them out here the way live-chat list is noted on line 47.

📝 Suggested wording

-Public list commands use an API key (or fall back to an OAuth grant that includes -`youtube.readonly`): `--page-size N`, `--page-token T`, `--all`, `--limit N`. Analytics -commands always require OAuth. +Public list commands use an API key (or fall back to an OAuth grant that includes +`youtube.readonly`): `--page-size N`, `--page-token T`, `--all`, `--limit N`. The catalog +commands (`category`/`language`/`region list`) accept none of these. Analytics commands +always require OAuth.

(The LanguageTool hints on youtube.readonly and "Required input" are false positives — the former is an OAuth scope identifier.)

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

Public list commands use an API key (or fall back to an OAuth grant that includes

`youtube.readonly`): `--page-size N`, `--page-token T`, `--all`, `--limit N`. Analytics

commands always require OAuth.

Public list commands use an API key (or fall back to an OAuth grant that includes

`youtube.readonly`): `--page-size N`, `--page-token T`, `--all`, `--limit N`. The catalog

commands (`category`/`language`/`region list`) accept none of these. Analytics commands

always require OAuth.

🧰 Tools

🪛 LanguageTool

[uncategorized] ~19-~19: The official name of this popular video platform is spelled with a capital “T”.
Context: ...ll back to an OAuth grant that includes youtube.readonly): --page-size N, `--page-to...

(YOUTUBE)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/skills/references/commands.md` around lines 19 - 21, Narrow the pagination statement in the command documentation so it excludes the catalog commands category list, language list, and region list, which do not register pagination flags. Preserve the existing pagination guidance for supported public list commands and retain the separate live-chat exception.

Source: Linters/SAST tools

coderabbitai · 2026-07-25T10:22:14Z

+    const value = Number(numText)
+    if (!Number.isFinite(value)) return fail()
+
+    total += value * UNITS[unit]!
+    matchedAny = true


🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Match Go’s nanosecond range and truncation semantics.

Number arithmetic accepts overflowing inputs such as very large hour counts and returns sub-nanosecond values (for example, 0.1ns) that Go truncates to whole nanoseconds. Parse and accumulate in integer nanoseconds, reject signed-int64 overflow, then convert the final value to milliseconds. Go checks overflow per component and total, and truncates fractional contributions to nanoseconds. (go.dev)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/util/goduration.ts` around lines 64 - 68, Update the duration parsing logic around Number(numText) to accumulate each component as truncated integer nanoseconds, rejecting values that exceed signed int64 limits before adding them to the total and also rejecting total overflow. Convert the final integer nanosecond total to milliseconds only after parsing completes, preserving Go’s fractional truncation and overflow semantics.

coderabbitai · 2026-07-25T10:22:14Z

+export const compareUtf8 = (a: string, b: string): number => {
+  if (a === b) return 0
+  const aCodes = Array.from(a, (c) => c.codePointAt(0) ?? 0)
+  const bCodes = Array.from(b, (c) => c.codePointAt(0) ?? 0)
+  const len = Math.min(aCodes.length, bCodes.length)
+  for (let i = 0; i < len; i++) {
+    const x = aCodes[i]!
+    const y = bCodes[i]!
+    if (x !== y) return x < y ? -1 : 1
+  }
+  return aCodes.length === bCodes.length ? 0 : aCodes.length < bCodes.length ? -1 : 1


🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Compare encoded bytes for lone-surrogate inputs.

A lone surrogate is compared here as 0xD800, but UTF-8 encoding replaces it with U+FFFD; consequently compareUtf8("\uD800", "\uE000") returns the opposite byte order. This can alter query and JSON-key canonicalization. Compare TextEncoder byte arrays, or normalize lone surrogates before comparison. (encoding.spec.whatwg.org)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/util/gostring.ts` around lines 15 - 25, Update compareUtf8 to compare UTF-8 encoded byte arrays rather than raw Unicode code points, ensuring lone surrogates follow TextEncoder’s U+FFFD replacement behavior. Preserve lexicographic ordering and length-based tie-breaking after comparing encoded bytes.

Three fixes, each validated by differential testing during the TypeScript port audit (PR #2) and backported here: - List() no longer reports a nextPageToken when --limit discarded items from the final fetched page. The server's token points past the discarded tail, so resuming from it silently skipped data. A limit landing exactly on a page boundary still keeps the token. - The --all pagination loop is now bounded. Previously it terminated only on an empty nextPageToken, trusting the server completely; a server repeating a token looped forever, accumulating pages in memory without limit. Two guards: a repeated token stops the loop (it can only re-fetch the same page), and a 10,000-request ceiling returns an error. Neither is reachable on a well-behaved server. - exitCode() now applies the same separator-stripping normalization to the quota/rate-limit reason test that the auth test already used, so RATE_LIMIT_EXCEEDED and QUOTA_EXCEEDED (the SCREAMING_SNAKE style newer Google API surfaces return) exit 5 like their camelCase equivalents, instead of falling through to 6. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix: harden pagination and exit-code classification Three fixes, each validated by differential testing during the TypeScript port audit (PR #2) and backported here: - List() no longer reports a nextPageToken when --limit discarded items from the final fetched page. The server's token points past the discarded tail, so resuming from it silently skipped data. A limit landing exactly on a page boundary still keeps the token. - The --all pagination loop is now bounded. Previously it terminated only on an empty nextPageToken, trusting the server completely; a server repeating a token looped forever, accumulating pages in memory without limit. Two guards: a repeated token stops the loop (it can only re-fetch the same page), and a 10,000-request ceiling returns an error. Neither is reachable on a well-behaved server. - exitCode() now applies the same separator-stripping normalization to the quota/rate-limit reason test that the auth test already used, so RATE_LIMIT_EXCEEDED and QUOTA_EXCEEDED (the SCREAMING_SNAKE style newer Google API surfaces return) exit 5 like their camelCase equivalents, instead of falling through to 6. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: seed pagination loop detection with the initial page token A server echoing the caller's --page-token back as nextPageToken is the same loop as any other repeated token, but seenTokens started empty, so the page was fetched and appended twice before detection fired. Seed the set with options.PageToken and cover it with a regression test asserting one request, no duplicated items, and no resume token. Found by CodeRabbit on PR #3. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

bmdavis419 and others added 6 commits July 24, 2026 22:40

coderabbitai Bot reviewed Jul 25, 2026

View reviewed changes

bmdavis419 mentioned this pull request Jul 26, 2026

fix: harden pagination and exit-code classification #3

Merged

Uh oh!

Conversation

bmdavis419 commented Jul 25, 2026 • edited by macroscopeapp Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Stack

Scope

Distribution

Intentional deviations

Bugs caught by differential testing

Verified against the Go binary

Known gaps

Port oytc CLI from Go to TypeScript using Effect v4 and Bun

Uh oh!

greptile-apps Bot commented Jul 25, 2026

Uh oh!

coderabbitai Bot commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

macroscopeapp Bot commented Jul 25, 2026

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

bmdavis419 commented Jul 25, 2026 •

edited by macroscopeapp Bot

Loading

coderabbitai Bot commented Jul 25, 2026 •

edited

Loading