diff --git a/plans/260818-2056-migrate-newsletter-scripts-to-go/plan.md b/plans/260818-2056-migrate-newsletter-scripts-to-go/plan.md index 3b07052..a410645 100644 --- a/plans/260818-2056-migrate-newsletter-scripts-to-go/plan.md +++ b/plans/260818-2056-migrate-newsletter-scripts-to-go/plan.md @@ -1,6 +1,7 @@ # Migrate Newsletter Scripts from Node.js to Go **Status:** COMPLETED (2026-08-18) — all 3 phases done; parity report in `plans/reports/parity-260818-newsletter-go-migration-report.md` +**Superseded:** 2026-09-18 — the Go engine this plan delivered was ported back to JavaScript by `plans/260918-0647-migrate-newsletter-engine-to-javascript/`. Body left unchanged; it records the reasoning as it stood. **Created:** 2026-08-18 **Scope:** Port the 8-file shared engine in `scripts/newsletter/` (~740 LoC JS, stdlib-only) to a single Go package with 6 subcommands, verify behavior parity, cut over all skill/doc call sites, delete the JS. diff --git a/plans/260918-0647-migrate-newsletter-engine-to-javascript/phase-01-port-engine-to-javascript.md b/plans/260918-0647-migrate-newsletter-engine-to-javascript/phase-01-port-engine-to-javascript.md new file mode 100644 index 0000000..b6080df --- /dev/null +++ b/plans/260918-0647-migrate-newsletter-engine-to-javascript/phase-01-port-engine-to-javascript.md @@ -0,0 +1,216 @@ +--- +phase: 1 +title: "Port the engine to JavaScript" +status: done +priority: P1 +effort: "1.5d" +dependencies: [] +--- + +# Phase 1: Port the engine to JavaScript + +## Overview + +Translate the 1168-LOC Go engine into Node ESM modules under +`scripts/newsletter/`, one JS file per Go file, replacing hand-rolled HTML/XML +regexes with cheerio. Go and JS coexist at the end of this phase — nothing is +deleted until phase 4. + +## Requirements + +**Functional** +- `node scripts/newsletter [args]` dispatches all seven subcommands from + the repo root, with the same usage text and the same argument shapes. +- Output is byte-identical to the Go engine except where this phase names a + deliberate difference. +- Repo-relative paths (`content/post`) resolve from `process.cwd()`, preserving + the "invoked from the repo root" contract. +- `config/substack-publications.json` stays the editable source of truth, read at + runtime (no `go:embed` equivalent needed), with the same + `["blog.bytebytego.com"]` fallback on read or parse failure. + +**Non-functional** +- ESLint clean under the existing `eslint.config.mjs` (`eqeqeq`, `prefer-const`, + `no-unused-vars` with `^_` escape). +- JSDoc type annotations on every exported function, per repo policy — types + without TypeScript. +- Exactly three runtime dependencies. No dev dependencies. + +## Architecture + +``` +scripts/newsletter/ + index.js subcommand dispatch, usage, printJson, dep-guard + url-utils.js cleanUrl, bareUrl, isSubstackImage, substackImageUuid, + fetchTextOk, checkAccessibility, collectMarkdown, + checkDuplicate, classifyType, boundary helpers + html-text.js stripTags, itemTitle, itemLink, extractCandidates, + captionForUuid, postTitleFromHtml [cheerio] + add-url.js add-url + detect-image-source.js detect-image-source + find-substack-post.js find-substack-post [cheerio] + find-newsletter-number.js find-newsletter-number + list-existing-tags.js list-existing-tags + fetch-via-defuddle.js fetch-via-defuddle [defuddle] + post-stats.js post-stats + config/substack-publications.json (unchanged) +``` + +Each command module exports a single `run*(args)` function; `index.js` owns +dispatch and the missing-dependency guard. + +**Deviation, made during review:** the shared JSON printer moved out of +`index.js` into its own `json-out.js`. Keeping it in `index.js` meant every +JSON-emitting command imported the dispatcher, and `index.js` runs `main()` at +module scope — so importing a command module executed the CLI against the host +argv, and the pure functions this phase exports for testing could not be imported +at all. One extra module removes the cycle outright. + +`printJson(v)` = `process.stdout.write(JSON.stringify(v, null, 2) + "\n")`. +This matches Go's `SetEscapeHTML(false)` + `SetIndent("", " ")` + `Encode` +exactly: JS does not HTML-escape, and `Encode` appends the newline. + +## Related code files + +- Create: `scripts/newsletter/index.js` and the nine modules above +- Modify: `package.json` (dependencies, `test` script) +- Modify: `package-lock.json` (via `npm install`) +- Unchanged: `scripts/newsletter/config/substack-publications.json` +- Untouched this phase: all `scripts/newsletter/*.go` + +## The seven parity traps + +Each is a place where the idiomatic JS differs from what the Go does. These are +the reason phase 2 exists; get them right here. + +**T1 — `cleanUrl` must not re-encode the query.** `url_utils.go` splits +`u.RawQuery` on `&` and rejoins surviving pairs verbatim, with a comment +explaining that Go's `url.Values` would sort keys and that sorted output must not +leak into stored `clean_url` values. JS `URLSearchParams.delete()` preserves +order but *normalizes percent-encoding* (e.g. `%7E` → `~`, `+` → `%20`), which is +the same class of bug. Split `url.search.slice(1)` on `&` and rejoin the kept +pairs as raw strings. Filter rule: drop any key whose lowercase form starts with +`utm_`, or is in the exact-match set (`fbclid gclid msclkid mc_eid aid ref +ref_src ref_url source s ck_subscriber_id igshid yclid vero_id`). + +**T2 — keep `hasBoundaryMatch` as an index loop.** Go hand-rolls it because RE2 +has no lookahead. JS regex *does* have lookahead, so the tempting move is to +collapse it into one pattern. Do not: the byte-offset boundary checks +(`uuidBoundaryOk`, `urlBoundaryOk`, including the optional trailing-slash skip +and the `)]"'?#<>_&,` delimiter set) are what make a prefix URL not a false +duplicate. Port the loop literally. A regex rewrite is a separate, separately +verified change. + +**T3 — `omitempty` fields must be absent, not empty.** Go omits `title`, +`author` (add-url) and `uuid`, `innerUrl` (detect-image-source) when empty. +Build those objects conditionally; never emit `"title": ""`. + +**T4 — the deep-miss `cutoff` is `null`, not omitted.** `find-substack-post +--deep` on a total miss emits `{found, source, scanned, budget, cutoff}` where +`cutoff` is a `*string` — JSON `null` when no sitemap could be fetched. Emit +`null` explicitly. + +**T5 — count runes, not UTF-16 units.** `extractCandidates` filters on +`utf8.RuneCountInString(text) < 6 || > 70`. JS `.length` counts UTF-16 code +units, so any non-BMP character (emoji in a TOC bullet) counts double and can +push a valid title past 70. Use `[...text].length`. + +**T6 — `list-existing-tags` output is `%6d %s`.** Right-align the count in a +6-character field: `String(count).padStart(6) + " " + tag`. It is plain text, not +JSON, and the skill reads it. + +**T7 — `extractInnerUrl` swallows decode errors.** Go's `url.PathUnescape` +returns an error on a malformed `%` sequence and the code falls back to the raw +substring. JS `decodeURIComponent` *throws*. Wrap in try/catch and return the +raw substring on failure. + +Two more that are free in JS but worth confirming: `extractCandidates` must +marshal as `[]` not `null` when empty (natural in JS; Go needed an explicit +non-nil slice), and `URL` already serializes an empty path as `/` (Go had to add +that by hand). + +## Implementation steps + +1. `npm install cheerio defuddle linkedom` — confirm the lockfile records exactly + these three plus their transitive deps, and that `npm ci` works offline + afterward on this ARM64 box. +2. Add to `package.json`: `"test": "node --test scripts/newsletter/*.test.js"`. + Leave `lint` and `projects:refresh` alone. +3. Write `index.js`: usage text (updated to `node scripts/newsletter …`), + `switch` dispatch over `process.argv[2]`, `printJson`, and the R1 dependency + guard — wrap the dynamic import of each command module and translate + `ERR_MODULE_NOT_FOUND` into + `newsletter engine: dependencies missing — run 'npm ci' from the repo root` + on stderr with exit 1. +4. Port `url-utils.js`. No dependencies — pure stdlib (`node:fs`, `node:path`, + `URL`, global `fetch`). Apply T1 and T2. `fetchTextOk` and + `checkAccessibility` use `AbortSignal.timeout(ms)`; `checkAccessibility` + returns the literal string `"000"` on any network error. +5. Port `html-text.js` using cheerio. This is the phase's real work: + - `stripTags` → `cheerio.load(s).text()` then collapse whitespace and trim. + Note cheerio decodes entities, replacing Go's `html.UnescapeString`. + - `itemTitle` / `itemLink` → `$('title').first()` / `$('link').first()` under + `xmlMode: true`, which handles CDATA natively. + - `extractCandidates` → `$('li')` with the T5 rune filter, case-insensitive + dedupe, first-seen order preserved. + - `captionForUuid` → find the element whose serialized subtree contains the + uuid, walk up with `.closest('figure')`, read `figcaption`. This replaces + the `LastIndex("")` string surgery; expect + *better* results on nested figures and record any difference under R3. + - `postTitleFromHtml` → `meta[property="og:title"]` → `h1` → `title`, same + precedence. +6. Port the five straightforward commands: `add-url.js`, `detect-image-source.js` + (T7), `find-newsletter-number.js` (keep the per-year early exit: scan a whole + year's months and days, then break only if that year yielded a number), + `list-existing-tags.js` (T6), `post-stats.js`. +7. Port `find-substack-post.js`: `node:util`'s `parseArgs` for `--uuid` / + `--deep`; cheerio `xmlMode` for RSS `` and sitemap `//`; + the 3-month cutoff; newest-first stable sort; the shared `deepFetchBudget = 40` + spanning all publications; T4 on the miss output. Read the publications JSON + at runtime with the documented fallback. +8. Port `fetch-via-defuddle.js` with the two-tier design from `plan.md`: + local `defuddle/node` first, then `https://defuddle.md/`. Exit 2 on bad + args, 1 on both tiers failing, 0 with the body on stdout. Stderr names which + tier failed and why. +9. `npx eslint scripts/newsletter/` and fix. Add JSDoc to every exported + function. +10. Smoke every subcommand once by hand against a live URL before handing off to + phase 2. + +## Success criteria + +- [x] All seven subcommands run under `node scripts/newsletter ` without a + stack trace. +- [x] `npx eslint scripts/newsletter/` is clean. +- [x] Every exported function carries a JSDoc type annotation. +- [x] `package.json` lists exactly `cheerio`, `defuddle`, `linkedom` as new + dependencies; no new devDependencies. +- [x] Each of T1-T7 is addressed, with a code comment at the site explaining the + constraint (explain the invariant, not the trap number). +- [x] Deleting `node_modules` and running any subcommand prints the `npm ci` + message, not a module-resolution stack trace. +- [x] No `.go` file modified or deleted. + +## Risk assessment + +**Cheerio's whitespace and entity handling differs from `html.UnescapeString` + +regex.** `stripTags` is used for captions and candidate titles that land verbatim +in published posts. +*Signal:* phase-2 fixture mismatches concentrated in `extractCandidates` / +`captionForUuid`. +*Response:* adjust the cheerio text pipeline to match Go on the fixtures. If a +difference is cheerio being correct on malformed markup, accept and record under +R3 — do not contort the parser back into regex behavior. + +**`defuddle/node` may not work on headless ARM64 Linux.** linkedom is pure JS so +it should, but this is unverified. +*Signal:* step 1 or step 8 fails on import or on a real extraction. +*Response:* ship `fetch-via-defuddle` with the `defuddle.md` proxy tier only, +drop `defuddle` and `linkedom` from `package.json`, and note the reduction — the +command keeps exactly its current Go behavior, so nothing regresses. + +**`parseArgs` is stricter than Go's `flag`.** Go's package accepts `-uuid` as +well as `--uuid`; `parseArgs` accepts only `--uuid`. +*Signal:* a skill or a user invoking the single-dash form. +*Response:* accept the narrowing — every documented call site uses `--uuid` — +and state it in the phase-2 report as a deliberate CLI difference. diff --git a/plans/260918-0647-migrate-newsletter-engine-to-javascript/phase-02-parity-test-suite.md b/plans/260918-0647-migrate-newsletter-engine-to-javascript/phase-02-parity-test-suite.md new file mode 100644 index 0000000..dbc6fee --- /dev/null +++ b/plans/260918-0647-migrate-newsletter-engine-to-javascript/phase-02-parity-test-suite.md @@ -0,0 +1,164 @@ +--- +phase: 2 +title: "Golden-output parity suite" +status: done +priority: P1 +effort: "0.5d" +dependencies: [1] +--- + +# Phase 2: Golden-output parity suite + +## Overview + +Capture the Go engine's stdout for a fixed case matrix, commit those outputs as +fixtures, and assert the JS engine reproduces them using `node:test`. This is +both the migration's proof of correctness and the regression net the engine has +never had. It must run while the Go engine still exists. + +## Requirements + +**Functional** +- Every subcommand covered; every parity trap T1-T7 from phase 1 covered by at + least one case that would fail if the trap were mishandled. +- Fixtures are generated from the Go engine, never hand-written — a hand-written + fixture proves nothing about parity. +- Network-dependent cases are separated from deterministic ones so `npm test` + stays runnable offline. + +**Non-functional** +- `node:test` + `node:assert` only. No test framework dependency. +- The suite runs in under 30 seconds offline. + +## Architecture + +``` +scripts/newsletter/ + parity.test.js deterministic cases, offline, runs in npm test + network.test.js live-network cases, skipped unless NEWSLETTER_NET=1 + __fixtures__/ + golden/.json or .txt — captured Go stdout, committed + repo/content/post/ a small synthetic content tree; __fixtures__/repo + stands in as a repo root via the subprocess cwd +``` + +**Two test tiers.** Deterministic cases (URL cleaning, classification, dedup +against a fixture content tree, post-stats, tag listing, newsletter numbering, +HTML/RSS extraction from saved markup) run always. Live-network cases (YouTube +oEmbed, Substack RSS and sitemap, defuddle) run only with `NEWSLETTER_NET=1`, +because upstream content changes and a CI-red-on-someone-else's-outage test +teaches people to ignore red. + +**Fixture content tree.** Dedup and the content-scanning commands must not read +the real `content/post` — it grows every day, so a fixture captured today fails +tomorrow. Build a small synthetic tree under `__fixtures__/repo/content/post/` +covering: +a post with a `Newsletter #N` heading, a post with a stored bare URL, a post with +a stored URL that is a strict *prefix* of the test URL (the T2 false-duplicate +case), a post with a Substack uuid in `
` form, a post with a uuid in +markdown-autolink form, and a post with a frontmatter `tags:` array. + +Tests point the engine at that tree by **setting the subprocess working +directory**, not by adding a config knob. Both engines already resolve +`content/post` relative to the process CWD — that is the documented repo-root +invocation contract — so `execFileSync(…, { cwd: fixtureRoot })` redirects every +content-scanning command with no change to either engine. The fixture tree +therefore lives at `__fixtures__/repo/content/post/…` so that `__fixtures__/repo` +can stand in as a repo root. + +**Saved markup.** RSS and sitemap parsing cases use saved upstream responses in +`__fixtures__/`, captured once. This is what lets cheerio-vs-regex differences be +compared deterministically. + +## Related code files + +- Create: `scripts/newsletter/parity.test.js` +- Create: `scripts/newsletter/network.test.js` +- Create: `scripts/newsletter/__fixtures__/` (golden outputs, `repo/content/post` tree, saved markup) +- Create: `plans/reports/parity-260918-newsletter-js-migration-report.md` +- Modify: `scripts/newsletter/*.js` — only if a fixture mismatch reveals a port bug +- Modify: `.gitignore` if any capture artifact must be excluded (prefer not to) + +## Case matrix + +Capture from Go, assert against JS. Minimum coverage: + +| Subcommand | Cases | +|---|---| +| `add-url` | article; article with `utm_*` + `fbclid` + `ref` params (T1); article with a param that must survive; `youtube.com/watch?v=`; `youtu.be/`; `youtube.com/shorts/`; a YouTube playlist URL (must *not* route youtube); substackcdn image URL; raw S3 substack image; `.pdf`; `.mp4`; `.avif` with query string; a URL that is a prefix of one already stored (T2, expect `duplicate:false`); a URL genuinely stored (expect `duplicate:true`); a non-YouTube URL (T3, expect no `title` key at all) | +| `detect-image-source` | substackcdn `/image/fetch/` wrapper; raw S3; non-Substack image (T3, no `uuid`/`innerUrl` keys); a wrapper with a malformed `%` sequence (T7) | +| `find-substack-post` | saved RSS containing the uuid; saved RSS missing it (`{found:false}`); `--deep` total miss against a saved sitemap (T4, `cutoff` present and `null` when the sitemap fetch fails); a saved RSS item whose TOC bullets include a non-BMP character near the 70-rune limit (T5) | +| `find-newsletter-number` | fixture tree with newsletters; empty tree (expect `1`); a tree where the newest year has no newsletter but an older year does (per-year early exit) | +| `list-existing-tags` | fixture tree — assert the exact `%6d %s` text including alignment (T6); tie-break ordering between two tags with equal counts | +| `post-stats` | a post with articles, all three Bonus subsections, and a `Newsletter #N` heading; a post with an empty Bonus; a post with no Bonus at all | +| `fetch-via-defuddle` | bad args → exit 2; unreachable host → exit 1; success → exit 0 with a body (network tier) | + +## Implementation steps + +1. Build `__fixtures__/repo/content/post/` by hand — six or so small `index.md` + files, each documented with the case it exists for. +2. Confirm both engines resolve content paths from CWD with no env override, so + the same fixture root drives the Go capture and the JS assertion. +3. Save the upstream RSS and sitemap responses used by the extraction cases. +4. Write a capture script in the scratchpad (not the repo) that runs every + deterministic case through `go run ./scripts/newsletter …` — with the + content-scanning cases run from `__fixtures__/repo` as CWD, using an absolute + path to the Go package — and writes + `__fixtures__/golden/`. Record exit codes alongside stdout. +5. Review every captured fixture by eye before committing. A fixture that + encodes a Go bug becomes a requirement the JS must match forever — catch that + now, and if one is found, record it as a known-bug fixture with a comment + rather than silently "fixing" it in the port. +6. Write `parity.test.js`: for each case, invoke the JS engine as a subprocess + (`node:child_process` `execFileSync`) so the test exercises the real CLI + surface including exit codes, passing `cwd: __fixtures__/repo` for the + content-scanning cases, and compare stdout to the golden file. +7. Write `network.test.js` with the same shape, guarded by + `{ skip: process.env.NEWSLETTER_NET !== "1" }`. +8. Run `npm test`. Every mismatch is triaged into exactly one of: **port bug** + (fix the JS), **accepted improvement** (cheerio extracting better text; update + the fixture and record the reason), or **accepted CLI difference** (e.g. + `parseArgs` rejecting `-uuid`). +9. Run the network tier once manually with `NEWSLETTER_NET=1`. +10. Write `plans/reports/parity-260918-newsletter-js-migration-report.md`: the + matrix, pass/fail per case, and every accepted difference with its + justification. Mirror the structure of the existing + `parity-260818-newsletter-go-migration-report.md`. + +## Success criteria + +- [x] `npm test` passes offline from a clean `npm ci`. +- [x] `NEWSLETTER_NET=1 npm test` passes against live upstreams. +- [x] Every subcommand has at least one case; T1-T7 each have a case that fails + if the trap is mishandled (verify by temporarily reintroducing the naive + implementation and watching the test go red). +- [x] Every fixture was generated from the Go engine and eyeballed before commit. +- [x] Zero unexplained differences. Every accepted difference is in the report + with a reason. +- [x] Tests read only `__fixtures__/`, never the real `content/post` — verify by + running the suite with the real `content/post` temporarily renamed. +- [x] Neither engine gained a test-only config knob; redirection is CWD alone. + +## Risk assessment + +**Fixtures that encode Go bugs.** Capturing blindly turns any existing bug into a +permanent contract. +*Signal:* step 5's review finds output that is wrong on its own terms. +*Response:* keep the fixture as-is with a comment naming the wrong behavior, so +the port is provably faithful; fix it as a separate change after cutover. + +**Live-network cases rot.** ByteByteGo's RSS window moves; a uuid that resolves +today will not in a month. +*Mitigation:* the network tier is opt-in and never gates `npm test`. +*Response:* if a network case goes stale, re-capture the saved markup and move +the assertion into the deterministic tier rather than chasing live content. + +**The synthetic content tree misses a real-world markdown shape.** Dedup +boundary handling is the highest-consequence logic in the engine — a false +negative publishes a duplicate, a false positive silently drops a URL the user +asked for. +*Mitigation:* seed the fixture tree by grepping the real `content/post` for the +distinct shapes stored URLs actually take (bare, trailing slash, autolink +`<…>`, inside a markdown link, inside an image) and cover each. +*Response:* if a shape is found that neither engine handles, that is a +pre-existing bug — record it, do not fix it inside the migration. diff --git a/plans/260918-0647-migrate-newsletter-engine-to-javascript/phase-03-skills-restructure-and-cutover.md b/plans/260918-0647-migrate-newsletter-engine-to-javascript/phase-03-skills-restructure-and-cutover.md new file mode 100644 index 0000000..c8bf844 --- /dev/null +++ b/plans/260918-0647-migrate-newsletter-engine-to-javascript/phase-03-skills-restructure-and-cutover.md @@ -0,0 +1,191 @@ +--- +phase: 3 +title: "Skill restructure and call-site cutover" +status: done +priority: P2 +effort: "0.5d" +dependencies: [2] +--- + +# Phase 3: Skill restructure and call-site cutover + +## Overview + +Switch every `go run ./scripts/newsletter` call site to `node scripts/newsletter` +and, in the same pass, fix the three structural problems in the skill layer: +duplicated Codex adapters, a shared reference file owned by one skill, and the +engine's command list spelled out in five different files. + +## Requirements + +**Functional** +- No `go run` anywhere outside `plans/` and `scripts/newsletter/*.go` (the Go + sources document their own invocation in doc comments and are deleted whole in + phase 4; `plans/` are historical records and stay as written). +- The seven engine commands are enumerated in exactly one file. Every other + mention is a link, except a skill inlining the one or two commands it actually + invokes. +- Shared skill prose is owned by no individual skill. +- `.agents/` carries no content that can drift from `.claude/`. +- Setup docs state the `npm ci` prerequisite (R1) once, where a new user will + hit it. + +**Non-functional** +- Both Claude Code and Codex still discover and run all six skills afterward. +- Skill frontmatter `description` fields keep their trigger wording — they are + how the runtimes decide to invoke a skill, and rewording them changes routing + behavior. + +## Architecture + +**Shared prose moves to `docs/newsletter/`,** not into another skill's directory. +`docs/` already exists and is tool-neutral, which is the point: `.claude/` and +`.agents/` both reference it by repo-root-relative path, so nothing breaks under +symlinks and no skill owns another skill's documentation. + +``` +docs/newsletter/ + engine-commands.md the ONE enumeration of the seven subcommands + post-mechanics.md moved from .claude/skills/mt-add-url/references/ +``` + +**`.agents/skills/mt-*/SKILL.md` become symlinks** to their `.claude` +counterparts, eliminating the parallel frontmatter. Verify Codex follows symlinks +before committing; if it does not, keep the current thin pointers (they are +already only 10 lines) and settle for centralizing the command table. + +**Renames — confirmed 2026-09-18.** The scheme is uniform +`mt--`; four of the six already conform, so only two move: +`mt-add-post` → `mt-add-article` (it adds an article *to* a post) and +`mt-webfetch` → `mt-fetch-url`. `mt-add-url`, `mt-add-video`, `mt-add-image` and +`mt-add-tags` keep their names — renaming a conforming skill is churn. Directory +name and frontmatter `name:` change together, and every cross-reference in the +other skills, `AGENTS.md`, and `README.md` follows. + + + +## Related code files + +- Create: `docs/newsletter/engine-commands.md` +- Create: `docs/newsletter/post-mechanics.md` (git mv from + `.claude/skills/mt-add-url/references/newsletter-post-mechanics.md`) +- Delete: `.claude/skills/mt-add-url/references/` (now empty) +- Rename: `.claude/skills/mt-add-post/` → `mt-add-article/`; + `.claude/skills/mt-webfetch/` → `mt-fetch-url/`; same under `.agents/skills/` +- Modify: all six `.claude/skills/*/SKILL.md` +- Replace: all six `.agents/skills/*/SKILL.md` (symlinks) +- Modify: `AGENTS.md` (lines 42-51 and the routing section), `README.md` (line 42), + `docs/multi-tool-usage.md` (lines 3, 13, 26, 27, 29) +- Modify: `opencode.json` — it already allows `node *` and never allowed `go *`; + confirm no change is needed rather than assuming + +## Call-site inventory + +Verified by grep at plan time. Every `go run` occurrence outside `plans/`: + +| File | Lines | Commands | +|---|---|---| +| `AGENTS.md` | 42, 45-51 | all seven + the prose describing the engine as Go | +| `.claude/skills/mt-add-url/SKILL.md` | 8, 23, 65 | `add-url`, `post-stats` | +| `.claude/skills/mt-add-url/references/newsletter-post-mechanics.md` | 4, 29, 91 | `find-newsletter-number`, `post-stats` | +| `.claude/skills/mt-add-post/SKILL.md` | 10, 16 | `add-url` | +| `.claude/skills/mt-add-video/SKILL.md` | 12, 22 | `add-url` | +| `.claude/skills/mt-add-image/SKILL.md` | 10, 24, 30, 40, 44, 71 | `detect-image-source`, `add-url`, `find-substack-post` (+`--deep`) | +| `.claude/skills/mt-add-tags/SKILL.md` | 53 | `list-existing-tags` | +| `.claude/skills/mt-webfetch/SKILL.md` | 31, 82 | `fetch-via-defuddle` | +| `README.md` | 42 | prose: "Go via `go run`" | +| `docs/multi-tool-usage.md` | 3, 13, 26, 27, 29 | prose + the shared-engine invocation | + +## Implementation steps + +1. Re-read the confirmed rename scheme above before touching anything — every + later step bakes the names in. +2. Write `docs/newsletter/engine-commands.md`: the seven commands, their + arguments, their output shapes, exit codes (including + `fetch-via-defuddle`'s now-meaningful 2-vs-1), the repo-root invocation + contract, and the `npm ci` prerequisite. +3. `git mv` the shared reference to `docs/newsletter/post-mechanics.md`. Update + its own line 4 (`All shared scripts live in scripts/newsletter/`) and its two + command blocks. Remove the now-empty `references/` directory. +4. Rename the two skill directories with `git mv`, under both `.claude/` and + `.agents/`, and update each `SKILL.md` frontmatter `name:`. +5. Update all six `.claude` skills: `go run ./scripts/newsletter X` → + `node scripts/newsletter X`; `../mt-add-url/references/newsletter-post-mechanics.md` + → `docs/newsletter/post-mechanics.md`; `mt-add-post` → `mt-add-article` and + `mt-webfetch` → `mt-fetch-url` in every cross-reference; add a link to + `docs/newsletter/engine-commands.md` where a skill currently explains the + engine. Leave the `description:` frontmatter wording alone except for the two + renamed names. +6. Update `mt-fetch-url`'s tier list for the new two-stage tier 1 (local + defuddle, then the `defuddle.md` proxy) from phase 1 — the current text + describes a single defuddle step. The chain becomes: WebFetch → local defuddle + → defuddle.md → reader proxy → report failure. +7. Test the symlink approach: replace one `.agents` adapter with a symlink, run + Codex, confirm it discovers and follows the skill. On success convert the + other five; on failure revert to thin pointers and note the reason in + `docs/multi-tool-usage.md`. +8. Rewrite `AGENTS.md` lines 40-52: describe the engine as Node, replace the + seven-command block with a one-line pointer to + `docs/newsletter/engine-commands.md` plus the `npm ci` prerequisite, and + update the skill names in the routing section and the Codex `$mt-*` list. +9. Update `README.md` line 42 and `docs/multi-tool-usage.md` lines 3, 13, 26, 27, + 29 — Node not Go, link the command reference rather than restating it, and + keep `docs/newsletter/` in the "what you must keep" list in the + pick-one-tool section. +10. Verify: `grep -rn "go run\|mt-add-post\|mt-webfetch" .claude .agents AGENTS.md + README.md docs` returns nothing. Do not include `scripts/` — the Go sources + still exist and reference themselves. +11. Verify the enumeration rule: `grep -rln "find-newsletter-number" .claude + .agents AGENTS.md README.md docs` should show `docs/newsletter/` files and + only those skills that actually invoke that command. +12. Run one real `mt-add-url` flow end to end — an article, a YouTube link, and a + Substack image into a scratch post — under Claude Code, then repeat the + dispatch step under Codex. + +## Success criteria + +- [x] `grep -rn "go run" .claude .agents AGENTS.md README.md docs` → empty + (`scripts/` is excluded until phase 4: the 10 Go sources carry 16 `go run` + occurrences in their own doc comments) +- [x] `grep -rn "mt-add-post\|mt-webfetch" .claude .agents AGENTS.md README.md docs` → empty +- [x] All seven commands enumerated in `docs/newsletter/engine-commands.md` and + nowhere else +- [x] No skill references a path inside another skill's directory +- [x] `.agents/skills/` contains no prose that can drift from `.claude/skills/` + (symlinks), or the fallback is documented with the reason +- [x] Claude Code lists and runs all six skills, two under new names +- [x] All six skill directory names satisfy `mt--` +- [ ] Codex discovers all six skills and `$mt-add-url` dispatches correctly — deferred: the user will verify on their next Codex run (symlinks were chosen over thin pointers at their direction) +- [x] One real end-to-end `mt-add-url` flow produces the expected markdown +- [x] The `npm ci` prerequisite appears in `AGENTS.md`, `README.md`, and + `docs/newsletter/engine-commands.md` + +## Risk assessment + +**Renaming skills breaks the user's muscle memory and any saved invocation.** +`mt-add-post` and `mt-webfetch` are typed by hand today. +*Signal:* the user or a runtime invoking an old name after cutover. +*Response:* the renames are the user's request, so carry them out — but call out +in the final report that `$mt-add-post` and `/mt-webfetch` no longer resolve. +Do not leave alias directories; two names for one skill is the duplication this +phase exists to remove. + +**Frontmatter `description` is routing logic, not documentation.** These strings +are how Claude Code and Codex decide whether to invoke a skill. An edit that +looks like tidying can stop a skill from triggering. +*Mitigation:* change only the skill names inside them; leave every trigger phrase +untouched. +*Signal:* step 12's end-to-end run failing to auto-dispatch. +*Response:* restore the original wording verbatim and change only the name token. + +**Symlinked skills may break Codex discovery, or break on a Windows checkout.** +*Signal:* step 7. +*Response:* thin pointers, as today. This is a nice-to-have; the command-table +centralization is the part that actually reduces future edit cost. + +**Moving `post-mechanics.md` out of `.claude/skills/` could put it outside what a +runtime packages with a skill.** Claude Code reads repo-relative paths fine, but +this changes the file's relationship to the skill. +*Signal:* a skill run that cannot read the referenced file. +*Response:* move it back under `.claude/skills/` in a shared, non-skill +subdirectory and have `.agents` reference it by repo-root-relative path instead. diff --git a/plans/260918-0647-migrate-newsletter-engine-to-javascript/phase-04-retire-go-toolchain.md b/plans/260918-0647-migrate-newsletter-engine-to-javascript/phase-04-retire-go-toolchain.md new file mode 100644 index 0000000..19dc7b5 --- /dev/null +++ b/plans/260918-0647-migrate-newsletter-engine-to-javascript/phase-04-retire-go-toolchain.md @@ -0,0 +1,125 @@ +--- +phase: 4 +title: "Retire the Go toolchain" +status: done +priority: P2 +effort: "2h" +dependencies: [2, 3] +--- + +# Phase 4: Retire the Go toolchain + +## Overview + +Delete the Go engine and remove Go from the repo, GitHub Actions, and Netlify. +The newsletter engine is Go's only consumer here — Hugo vendors its theme in +`themes/` and `config/_default/` has no `[module]` section, so nothing else +needs a Go toolchain. This is the only irreversible phase; it runs last and only +after phase 2 is green. + +## Requirements + +**Functional** +- No `.go` file, no `go.mod`, no Go step in CI, no `GO_VERSION` anywhere. +- The Hugo build still succeeds on GitHub Actions and on Netlify. +- The superseded research recommendation is annotated, not left contradicting + the repo. + +**Non-functional** +- One commit per concern (engine deletion, CI, docs/report) so a bisect can + isolate a Hugo build break to the CI change. + +## Architecture + +Go's footprint, verified by grep at plan time: + +| File | What to remove | +|---|---| +| `scripts/newsletter/*.go` | all 10 files | +| `go.mod` | the whole file (there is no `go.sum` — the engine was stdlib-only) | +| `.github/workflows/hugo.yml` | `GO_VERSION: 1.25.5` (line 23), the `Setup Go` step (lines 33-37), and the Go line in Verify installations (line 64) | +| `netlify.toml` | `GO_VERSION = "1.25.5"` (line 3) | + +Removing the pins also disposes of the latent defect the research report flagged: +`go.mod` declares `go 1.26` while both build environments pin 1.25.5, which only +works because `GOTOOLCHAIN=auto` silently downloads 1.26. Deleting Go resolves it +outright — no version alignment needed. + +## Related code files + +- Delete: `scripts/newsletter/add_url.go`, `detect_image_source.go`, + `fetch_via_defuddle.go`, `find_newsletter_number.go`, `find_substack_post.go`, + `html_text.go`, `list_existing_tags.go`, `main.go`, `post_stats.go`, + `url_utils.go` +- Delete: `go.mod` +- Modify: `.github/workflows/hugo.yml`, `netlify.toml` +- Modify: `.gitignore` — the Go block (`*.o`, `_cgo_*`, `_testmain.go`, `*.test`, + …) is dead weight afterward; remove it and keep `node_modules` +- Modify: `plans/reports/research-260918-1139-script-language-choice.md` +- Modify: `plans/260818-2056-migrate-newsletter-scripts-to-go/plan.md` — status + line only + +## Implementation steps + +1. Re-confirm phase 2 is green and phase 3's end-to-end run passed. Everything + below assumes the JS engine is the working engine. +2. Confirm Go has no other consumer: grep the repo's `*.yml`, `*.toml` and `*.md` + (excluding `node_modules/` and `plans/`) for every Go invocation and CI action + spelling, and check `config/_default/` for a `[module]` section. Both must + come back empty of anything but the sites listed above. +3. `git rm scripts/newsletter/*.go go.mod`. Commit alone. +4. Edit `.github/workflows/hugo.yml`: drop the `GO_VERSION` env entry, the whole + `Setup Go` step, and the Go line from Verify installations. Leave the Node + setup and the conditional `npm ci` — the lockfile exists, so `npm ci` runs, + which is what the engine now needs if CI ever invokes it. +5. Edit `netlify.toml`: drop `GO_VERSION`. Keep `NODE_VERSION = "24.12.0"`. +6. Remove the Go section from `.gitignore`. +7. Push and watch both builds. GitHub Actions must deploy; Netlify must build a + preview. Compare the built output against the previous deploy — the site + should be byte-identical, since Go never touched it. +8. Append a dated note to + `plans/reports/research-260918-1139-script-language-choice.md`, directly under + its recommendation #1, recording that the user overrode it on 2026-09-18, the + reason given, and a link to this plan. Do not delete or rewrite the original + recommendation — the report is a stateful record of what was concluded at the + time, and erasing it destroys the reasoning a future reader needs. +9. Update the old Go plan's status line to note it was superseded by this plan, + with the date. Leave its body untouched for the same reason. +10. Run `ak plan close` on this plan and `/ak:journal` for the migration. + +## Success criteria + +- [x] `find . -name "*.go" -not -path "./node_modules/*" -not -path "./themes/*"` → empty +- [x] `go.mod` gone; no `go.sum` was ever present +- [x] No `GO_VERSION` or Go-setup action reference remains in `.github/` or + `netlify.toml` +- [ ] GitHub Actions `hugo.yml` deploys green — not exercised: nothing pushed +- [ ] Netlify builds green — not exercised: nothing pushed +- [ ] Deployed site output unchanged from the pre-phase-4 deploy — not exercised: nothing pushed +- [x] The research report carries the override note; its original recommendation + is still readable +- [x] The 2026-08-18 Go migration plan is marked superseded +- [x] Every subcommand still works from a fresh clone + `npm ci` + +## Risk assessment + +**Something else needs Go and the grep missed it.** Hugo Modules are the obvious +candidate — a `[module]` block in Hugo config makes `hugo` itself require Go. +*Signal:* the Hugo build failing on Actions or Netlify after step 4 or 5. +*Response:* restore the Go setup step and the `GO_VERSION` pins (the engine stays +deleted — Hugo needing Go is unrelated to the engine) and align the pins to a +single version, which the research report wanted anyway. + +**Netlify caches the Go toolchain and the build passes now but fails on the next +cache eviction.** +*Mitigation:* step 7 checks a real Netlify build, and Netlify builds previews +from a clean environment per deploy. +*Response:* if a cold build fails for a Go-shaped reason, that contradicts step +2 — reopen the investigation rather than re-adding the pin blindly. + +**Irreversibility.** After this phase, the Go engine exists only in git history. +*Mitigation:* the phase ordering — parity proven (2) and skills cut over and +exercised end-to-end (3) before anything is deleted. +*Response:* recovery is `git checkout -- scripts/newsletter +go.mod`, which restores a fully working engine. Record that SHA in the journal +entry so it is findable without archaeology. diff --git a/plans/260918-0647-migrate-newsletter-engine-to-javascript/phase-05-unify-module-extensions.md b/plans/260918-0647-migrate-newsletter-engine-to-javascript/phase-05-unify-module-extensions.md new file mode 100644 index 0000000..4eab166 --- /dev/null +++ b/plans/260918-0647-migrate-newsletter-engine-to-javascript/phase-05-unify-module-extensions.md @@ -0,0 +1,124 @@ +--- +phase: 5 +title: "Unify module extensions on .js" +status: done +priority: P3 +effort: "1h" +dependencies: [3] +--- + +# Phase 5: Unify module extensions on `.js` + +## Overview + +Rename `.github/scripts/*.mjs` to `.js` so the repo has one module extension. +Independent of the engine migration; sequenced after phase 3 only so the +newsletter cutover lands first and a CI break here is unambiguous. + +## Why `.js` and not `.mjs` + +Decided during validation (2026-09-18), on three checks against this repo: + +1. The root `package.json` declares `"type": "module"`, so a `.js` file here is + already ESM. `.mjs` restates what the manifest states. +2. Node's directory resolution requires `index.js` — verified empirically: with + only `eng/index.mjs` present, `node eng` fails with + `Cannot find module '…/eng'`; adding `eng/index.js` makes it resolve. This is + what keeps the engine invocation at `node scripts/newsletter ` rather + than a full file path, so `scripts/newsletter/` must use `.js` regardless. +3. `eslint.config.mjs` already globs `**/*.js` alongside `**/*.mjs`, so no lint + config change is needed. + +`.mjs` earns its place in a package without `"type": "module"`, or one shipping +dual CJS/ESM entry points. Neither applies. `eslint.config.mjs` itself keeps its +extension — ESLint's own convention, and it sits outside the `type` field's +reach in some resolution paths. + +## Requirements + +**Functional** +- No `.mjs` under `.github/scripts/`. +- `npm run projects:refresh` works locally. +- The `update-projects.yml` scheduled workflow runs green. + +**Non-functional** +- Renames use `git mv` so history follows the file. +- One commit, so a revert is clean if the workflow breaks. + +## Architecture + +Three files move; five reference sites follow. All verified by grep at plan time: + +| File | Action | +|---|---| +| `.github/scripts/update-projects-list.mjs` | → `update-projects-list.js` | +| `.github/scripts/lib/github-api.mjs` | → `lib/github-api.js` | +| `.github/scripts/lib/render-projects.mjs` | → `lib/render-projects.js` | + +| Reference site | Line(s) | What changes | +|---|---|---| +| `package.json` | 12 | `projects:refresh` script path | +| `.github/workflows/update-projects.yml` | 41 | `run: node …` path | +| `.github/scripts/update-projects-list.mjs` | 18, 25 | two `import` specifiers | +| `.github/scripts/update-projects-list.mjs` | 36, 37 | two JSDoc `import("…")` types | +| `.github/scripts/lib/render-projects.mjs` | 6, 51, 66, 80 | four JSDoc `import("…")` types | + +The JSDoc `import("./github-api.mjs").Repo` annotations are the easy miss: they +are type references inside comments, so a stale one does not throw at runtime — +it silently degrades to `any` and the repo loses the typing it adopted JSDoc for. + +## Related code files + +- Rename: the three files above +- Modify: `package.json`, `.github/workflows/update-projects.yml` +- Modify: the renamed files' own import specifiers and JSDoc type imports +- Unchanged: `eslint.config.mjs` + +## Implementation steps + +1. `git mv` the three files. +2. Update the two runtime `import` specifiers in `update-projects-list.js`. +3. Update all six JSDoc `import("…")` type references (two in + `update-projects-list.js`, four in `lib/render-projects.js`). +4. Update `package.json` line 12 and `update-projects.yml` line 41. +5. `grep -rn "\.mjs" package.json .github/` → only `eslint.config.mjs` style + matches should remain, and none under `.github/scripts/`. +6. `npm run lint` and `npm run projects:refresh` locally. The refresh needs a + GitHub token; if one is not available locally, confirm the script gets as far + as the auth check rather than failing on module resolution. +7. Push and let `update-projects.yml` run — trigger it manually rather than + waiting for the daily cron. + +## Success criteria + +- [x] `find .github/scripts -name "*.mjs"` → empty +- [x] `grep -rn "\.mjs" .github/ package.json` → empty +- [x] `npm run lint` clean +- [x] `npm run projects:refresh` resolves all modules (auth failure is acceptable + locally; a module-resolution failure is not) +- [ ] `update-projects.yml` completes green on a manual dispatch — not exercised: nothing pushed +- [ ] The projects page content it generates is unchanged — not exercised: needs a GitHub token and a workflow run + +## Risk assessment + +**A stale JSDoc `import("…")` type silently degrades to `any`.** Nothing throws, +lint stays green, and the repo quietly loses type coverage on `Repo` and +`Contribution` — the exact thing the JSDoc-not-TypeScript policy exists to +provide. +*Signal:* step 5's grep, which is why it checks JSDoc strings and not just +`import` statements. +*Response:* fix the reference. If any are found after the commit, add a lint rule +or a grep to CI rather than relying on the next person to remember. + +**`update-projects.yml` breaks and the projects page goes stale silently.** It is +a daily cron, so a failure is easy to miss. +*Mitigation:* step 7 triggers it manually instead of waiting. +*Response:* revert the single commit; the rename is cosmetic and nothing depends +on it. + +**Scope creep into an unrelated CI path.** This phase touches the projects +workflow, which has nothing to do with the newsletter engine. +*Mitigation:* it is its own phase, its own commit, and it is P3 — droppable +without affecting phases 1-4. +*Response:* if phase 4 leaves CI unstable for any reason, drop this phase rather +than stacking a second CI change on top. diff --git a/plans/260918-0647-migrate-newsletter-engine-to-javascript/plan.md b/plans/260918-0647-migrate-newsletter-engine-to-javascript/plan.md new file mode 100644 index 0000000..55660ec --- /dev/null +++ b/plans/260918-0647-migrate-newsletter-engine-to-javascript/plan.md @@ -0,0 +1,238 @@ +--- +title: "Migrate the newsletter engine from Go to JavaScript" +description: "Port the 1168-LOC stdlib-only Go newsletter engine to Node ESM with a parser dependency, prove parity with a committed golden-output suite, restructure the mt-* skills around a single command reference, and retire Go from the repo." +status: in-progress +priority: P1 +effort: "2-3d" +tags: [newsletter, scripts, skills, migration, javascript] +created: 2026-09-18 +--- + +# Migrate the newsletter engine from Go to JavaScript + +## Overview + +`scripts/newsletter/` is a 10-file, 1168-LOC `package main` in Go, stdlib-only, +invoked as `go run ./scripts/newsletter ` from ten files +across `.claude/skills/`, `.agents/skills/`, `AGENTS.md`, `README.md`, and +`docs/multi-tool-usage.md`. This plan replaces it with a Node ESM engine invoked +as `node scripts/newsletter `, takes on a small set of justified npm +dependencies, proves behavioral parity with committed golden fixtures, folds the +skill layer onto one canonical command reference, and deletes Go from the repo +and CI. + +The engine has seven subcommands: `add-url`, `find-newsletter-number`, +`list-existing-tags`, `detect-image-source`, `find-substack-post`, +`fetch-via-defuddle`, `post-stats`. + +## Context: this reverses a same-day recommendation + +`plans/reports/research-260918-1139-script-language-choice.md` (2026-09-18) +concludes *"`scripts/newsletter/` stays Go. No GitHub API, no deps, no latency +problem, three agent runtimes served identically."* `CLAUDE.md` also ranks Go +above JavaScript for new work. + +The user reviewed that and chose to migrate anyway (2026-09-18). Phase 4 records +the override in the report itself so the repo record is not self-contradictory. +The plan does not re-litigate the decision; it does carry the report's strongest +objection forward as the tracked risk (see **Risks**, R1). + +## Goals + +| # | Goal | Priority | +|---|------|----------| +| 1 | Seven subcommands running on Node with output parity against the Go engine | P1 | +| 2 | Hand-rolled HTML/XML regex extraction replaced by a real parser | P1 | +| 3 | A committed regression suite the engine did not have before | P1 | +| 4 | One canonical command reference; no enumeration of the engine's commands in more than one file | P2 | +| 5 | Go fully removed from the repo, CI, and Netlify | P2 | +| 6 | One module extension across the repo's JavaScript | P3 | + +## Non-goals + +- `.github/scripts/` keeps its current logic. Its Octokit migration is a separate + recommendation in the same research report and is not in scope. Phase 5 renames + its files' extensions only — no behavior change. +- No change to what the skills *do* — only to what they invoke and where their + shared prose lives. +- No new subcommands. The port is behavior-preserving except where this plan + names a deliberate improvement. + +## Design decisions + +**Invocation stays positional and short.** `node scripts/newsletter [args]` +from the repo root, matching the Go contract one-for-one. Node directory +resolution requires `scripts/newsletter/index.js` (not `.mjs`), so the new engine +uses `.js`; the root `package.json` already declares `"type": "module"`, so `.js` +is ESM and the existing `eslint.config.mjs` already lints `**/*.js`. +`.github/scripts/` currently uses `.mjs`; phase 5 unifies it onto `.js` for the +same reasons (see that phase for the empirical check). + +**Module layout mirrors the Go files 1:1** so the parity diff is reviewable: +`index.js` (dispatch) and one module per current `.go` file. A 1:1 map makes any +behavioral drift attributable to a single file. + +**Dependencies (3, each load-bearing).** The user authorized npm dependencies +while asking to stay minimal: + +| Package | Replaces | Why it earns its place | +|---|---|---| +| `cheerio` ^1.2.0 | ~12 hand-rolled regexes in `html_text.go` + `find_substack_post.go` | One parser covers HTML *and* RSS/sitemap XML (`xmlMode`), so no second XML dep. Eliminates `captionForUUID`'s manual `LastIndex("`. The point of that proxy is +that it fetches from a *different IP* than the blocked caller. Running defuddle +locally fetches from this box's IP instead — a genuinely different IP from the +one WebFetch uses, but not the same third IP the proxy provides. Both are kept: +local defuddle first, `defuddle.md` on failure. Dropping the proxy would silently +narrow the skill's coverage. + +**Exit codes get more precise.** `fetch-via-defuddle` defines exit 2 (bad args) +vs 1 (fetch failed), but `go run` collapses every nonzero exit to 1 — documented +in a comment in `fetch_via_defuddle.go`. Node preserves them, so the distinction +becomes real for the first time. + +## Phases + +| # | Phase | Status | Depends on | +|---|-------|--------|------------| +| 1 | [Port the engine to JavaScript](./phase-01-port-engine-to-javascript.md) | Done | — | +| 2 | [Golden-output parity suite](./phase-02-parity-test-suite.md) | Done | 1 | +| 3 | [Skill restructure and call-site cutover](./phase-03-skills-restructure-and-cutover.md) | Done | 2 | +| 4 | [Retire the Go toolchain](./phase-04-retire-go-toolchain.md) | Done | 2, 3 | +| 5 | [Unify module extensions on `.js`](./phase-05-unify-module-extensions.md) | Done | 3 | + +Go and JS coexist through phases 1-3 — parity cannot be measured against a +deleted reference. Phase 4 is the only irreversible step. Phase 5 is P3 and +independent; drop it without affecting anything else. + +## Success criteria + +- [x] All seven subcommands run under `node scripts/newsletter ` and match + the Go engine on the phase-2 case matrix (byte-identical stdout, or drift + recorded as an accepted improvement with a reason). +- [x] `npm test` passes from a clean `npm ci`. +- [x] `npm run lint` passes over `scripts/newsletter/`. +- [x] One real `mt-add-url` flow (article + YouTube + Substack image) completes + end-to-end on the JS engine and writes the same markdown the Go engine + would have. +- [x] `grep -rn "go run" .claude .agents AGENTS.md README.md docs scripts` returns + nothing. Evaluated after phase 4 — phase 3's narrower version of this check + excludes `scripts/`, because the Go sources document their own invocation + and survive until phase 4 deletes them. +- [x] The engine's seven commands are enumerated in exactly one file + (`docs/newsletter/engine-commands.md`); every other mention is a link. +- [x] No `.go` file, no `go.mod`, no Go setup step, no `GO_VERSION` anywhere in + the repo. +- [x] One module extension: no `.mjs` outside `eslint.config.mjs`. +- [x] A fresh `git clone` + `npm ci` runs every subcommand successfully. + +## Risks + +**R1 — `npm ci` becomes a prerequisite the engine never had.** This is the +research report's strongest objection and the migration's real cost. Go's +stdlib-only engine ran on a bare clone; the JS engine will not, because +`node_modules` is gitignored. Three agent runtimes (Claude Code, OpenCode, Codex) +on a headless ARM64 box all hit this. +*Mitigation:* `index.js` catches `ERR_MODULE_NOT_FOUND` on the dependency imports +and exits with `newsletter engine: dependencies missing — run 'npm ci' from the +repo root` instead of a raw stack trace. Setup docs state it once. +*Signal it broke:* any skill run failing with a module-resolution error. +*Response:* if this proves to be recurring friction rather than a one-time setup +step, vendor the three deps or fall back to the stdlib implementations in +`html_text.js` — do not re-litigate the language choice. + +**R2 — silent output drift in the port.** Seven of the Go functions encode +non-obvious decisions (query-string order preservation, `omitempty`, rune-vs-byte +length, `null` vs omitted fields). A JS-idiomatic rewrite breaks them quietly and +the damage lands in published posts. +*Mitigation:* phase 2's golden fixtures are captured from the Go engine *before* +the JS is trusted, and phase 1 lists all seven traps explicitly. +*Signal:* any fixture mismatch. +*Response:* match Go, do not "improve" — parity first, improvements as a separate +recorded decision. + +**R3 — cheerio changes extraction results.** Replacing regexes with a real parser +is the point, but a parser will legitimately extract *different* (usually better) +text on malformed markup. +*Mitigation:* treat extraction differences as expected-to-review rather than +expected-to-match; phase 2 records each one with a judgment. +*Response:* accept improvements, document them; reject anything that loses +content the regex found. + +**R4 — Codex may not follow symlinked skill files.** Phase 3's `.agents` +de-duplication assumes it does; this is untested. +*Mitigation:* verify before committing the symlinks. +*Response:* fall back to the current thin-pointer adapters, which already work. + +## Resolved during validation (2026-09-18) + +**Skill naming — uniform `mt--`.** Two of the six violate it today; +the other four already conform, so renaming them would be churn. + +| Current | New | +|---|---| +| `mt-add-url` | unchanged (dispatcher) | +| `mt-add-post` | **`mt-add-article`** — it adds an article *to* a post | +| `mt-add-video` | unchanged | +| `mt-add-image` | unchanged | +| `mt-add-tags` | unchanged | +| `mt-webfetch` | **`mt-fetch-url`** — verb-object, like its siblings | + +**Fixture redirection uses the subprocess CWD, not a config knob.** Both engines +already resolve `content/post` from the process working directory, so phase 2's +tests set `cwd` and neither engine gains a test-only environment variable. + +**R1 mitigation stays at a clear error message.** No auto-`npm ci` in the skills +and no self-installing engine; `npm ci` is a documented one-time setup step. + +**Module extension unified on `.js`** — see phase 5. + +## Validation Log + +### Session 1 — 2026-09-18 + +**Verification pass:** Standard tier (5 phases at time of writing: 4). +Claims checked 24 · Verified 19 · Failed 5. + +| # | Claim | Result | Evidence | +|---|---|---|---| +| F1 | `netlify.toml` `GO_VERSION` at line 4 | FAILED | it is line 3; `HUGO_VERSION` is line 4 — corrected in phase 4 | +| F2 | `hugo.yml` Setup Go step at lines 32-37 | FAILED | it is 33-37 — corrected in phase 4 | +| F3 | `hugo.yml` Go verify line at 61 | FAILED | it is line 64 — corrected in phase 4 | +| F4 | Phase 3 criterion `grep -rn "go run" … scripts` → empty | FAILED | the 10 Go sources carry 16 `go run` occurrences in their own doc comments, so it cannot pass before phase 4 — criterion rescoped to exclude `scripts/` | +| F5 | Phase 2 requires `NEWSLETTER_CONTENT_DIR`; phase 1 never adds it | FAILED | contract gap between phases — resolved by dropping the env var for CWD redirection | +| V1 | Node directory resolution needs `index.js`, not `index.mjs` | VERIFIED | empirical: `node eng` fails with `Cannot find module` when only `index.mjs` exists; resolves once `index.js` is added | +| V2 | Hugo has no `[module]` config, so Go has no other consumer | VERIFIED | grep across `config/` returns nothing | +| V3 | All phase-3 call-site line numbers | VERIFIED | `grep -n` across `.claude`, `.agents`, `AGENTS.md`, `README.md`, `docs/` | +| V4 | `defuddle` exports `./node`; `linkedom` is an optional dep | VERIFIED | `npm view defuddle exports optionalDependencies` | +| V5 | `opencode.json` allows `node *` and never allowed `go *` | VERIFIED | read directly | + +**Questions asked:** 5 across two rounds. All answered; no unresolved decisions. + +### Whole-Plan Consistency Sweep + +Run after propagating the decisions above to phases 1-5. + +- `NEWSLETTER_CONTENT_DIR` removed from every plan file — 0 remaining hits. +- `mt-add-post` / `mt-webfetch` appear only as *old* names in phase 3's rename + table and call-site inventory, which is correct. +- The design-decisions `.mjs` note in `plan.md` reconciled with phase 5's + existence (phase 1 carried no such note). +- Non-goals reconciled: `.github/scripts/` logic stays out of scope; its + extensions do not. +- Success criteria in `plan.md` and phases 3, 4, 5 agree on what `grep` must + return and when. +- Two contradictions found and fixed during the sweep: + 1. The dependency table listed `defuddle` as *replacing* the `defuddle.md` + proxy, contradicting the two-tier design decision that keeps both. + 2. `plan.md`'s `go run` criterion and phase 3's differed on whether `scripts/` + is included; both now state which phase they are evaluated after. +- **Unresolved contradictions: 0.** diff --git a/plans/reports/code-review-260918-newsletter-js-migration.md b/plans/reports/code-review-260918-newsletter-js-migration.md new file mode 100644 index 0000000..85e6380 --- /dev/null +++ b/plans/reports/code-review-260918-newsletter-js-migration.md @@ -0,0 +1,308 @@ +# Code Review — Newsletter engine Go → JavaScript migration (2026-09-18) + +Reviewer pass over the uncommitted change set on `main`. Read-only: no file in +the change set was modified. Verification was done by reading the JS against +`git show HEAD:scripts/newsletter/*.go`, running `npm run lint` / `npm test`, and +probing the engine directly. + +## Scope + +- `scripts/newsletter/` — 10 new `.js` modules + 2 test files + `__fixtures__/` + (~1,050 LOC), replacing 10 deleted `.go` files (1,168 LOC) and `go.mod` +- 6 `mt-*` skills restructured, `.agents/skills/` converted to symlinks +- `docs/newsletter/engine-commands.md` (new), `post-mechanics.md` (moved), + `AGENTS.md`, `README.md`, `docs/multi-tool-usage.md` +- `.github/scripts/*.mjs` → `.js`, `hugo.yml`, `update-projects.yml`, + `netlify.toml`, `.gitignore`, `package.json`, `eslint.config.mjs` + +## Overall assessment + +The port is careful and, on the evidence, faithful. All seven named traps are +genuinely handled — verified by reading, not by trusting the report. The golden +suite is real regression coverage (byte-for-byte stdout plus exit codes through +the actual CLI surface), not phantom tests. No scope drift: no invented +abstractions, no `any` widening, no catch-and-swallow beyond the deliberate +"return empty on network failure" contract the Go engine already had. + +`npm run lint` passes clean. `npm test` passes 33/33 (8 network cases skipped). + +One structural defect and three contract gaps are worth fixing before this is +committed. None is a security issue; the threat model here (a personal blog's +content CLI, run by the repo owner) does not warrant hardening beyond what is +present. + +## Critical + +None. + +## High + +### H1 — `index.js` runs the CLI as an import side effect + +`scripts/newsletter/index.js:95` calls `main()` unconditionally at module scope, +with no entry-point guard. The four command modules that emit JSON +(`add-url.js`, `detect-image-source.js`, `find-substack-post.js`, +`post-stats.js`) statically `import { printJson } from "./index.js"`. Importing +any of them therefore *runs the CLI* against the host process's `argv`. + +Reproduced: + +``` +$ node -e 'import("./scripts/newsletter/post-stats.js")' +Usage: node scripts/newsletter [args] +... +exit=1 +``` + +A one-line unit test for an exported pure function fails the same way: + +```js +import { countPostEntries } from ".../post-stats.js"; // → suite dies, exit 1 +``` + +Impact — this is the practical answer to "can you break `npm test`": yes, by +adding the most obvious next test. The port exports `detectYouTube`, +`extractInnerUrl`, `countPostEntries`, `parseLastmod` and `loadPublications` +specifically so they can be tested, and today none of them can be. The parity +suite only survives because it imports `html-text.js`, the one helper module with +no back-edge to `index.js`. Worse than the failure itself: if the host argv ever +begins with a valid command name, the import silently *executes that command* +instead of erroring. + +The circular import is known — the comment at `index.js:93` works around its +deadlock symptom ("command modules import printJson from this file, so a +top-level await here would deadlock") rather than removing the cycle. + +Fix (either one, both small): + +- move `printJson` into its own module (`output.js`) so no command module imports + the entry point; or +- guard the call: + `if (process.argv[1] && pathToFileURL(process.argv[1]).href === import.meta.url) main().catch(...)` + +## Medium + +### M1 — the local defuddle tier can consume the proxy tier's turn on junk output + +`fetch-via-defuddle.js:27,32` gates local success on exactly two conditions: +`res.ok`, and `body.trim() !== ""`. Anything else that comes back 200 with text — +a Cloudflare interstitial, a "Verifying you are human" page, a "Subscribe to keep +reading" paywall stub — extracts to a non-empty body, so `runFetchViaDefuddle` +returns at line 69 and **the `defuddle.md` proxy never runs**. + +That is the scenario the proxy was deliberately kept for. `mt-fetch-url` is +invoked only *after* WebFetch was already blocked, which strongly predicts that a +fetch from this same box is also blocked. The agent-level mitigation in +`mt-fetch-url/SKILL.md` ("If tier 1 ... returns a body with no usable content, +try a reader proxy") routes past the failure to `r.jina.ai` — skipping the +proxy stage entirely, because the engine already reported exit 0. + +Not re-litigating the two-tier decision (accepted, and the ordering is right). +The gap is the gate. A minimal quality check before returning local output — +e.g. fall through when `result.wordCount` is below a small threshold, or when the +body matches a short challenge/paywall shape — restores the intended chain. + +### M2 — the local tier's output shape breaks `mt-fetch-url`'s documented parsing contract + +The proxy returns YAML frontmatter plus body: + +``` +--- +title: "Example Domain" +site: "example.com" +source: "https://example.com/" +... +--- + +``` + +The local tier returns `String(result.content)` only — bare markdown, no +frontmatter, no title. Verified against `https://example.com/`. + +`mt-fetch-url/SKILL.md` still instructs the agent, for the tier the engine now +tries **first**: + +- step 4: "Parse the returned markdown (tier 1 has YAML frontmatter with + title/description/etc.)" +- Output handling: "The frontmatter gives you the page title for free — + preferred over parsing from HTML." +- Failure modes: "Only frontmatter with no body" — a proxy-shaped signal that + cannot occur on the local tier. + +So on the common path the agent is told to read a title that is not there. For a +skill whose output feeds an article heading in a published post, a missing title +is exactly the "wrong output lands in a post" failure mode worth catching. + +`Defuddle(...)` already returns `title`, `author`, `description`, `domain`, +`site`, `published` and `wordCount` (confirmed by inspecting the result object) — +all discarded at `fetch-via-defuddle.js:31`. Emitting an equivalent frontmatter +block from those fields makes both tiers shape-compatible and satisfies the +skill as written. Alternatively, correct the skill text. Aligning the engine is +the smaller change and keeps the skill's title guidance true. + +### M3 — output-contract changes not recorded in the parity report + +Acceptance criterion 3 asks for anything that changed without being called out. +Three items, all benign or improvements, none currently documented: + +1. **Sitemap `` entities are now decoded.** Go used + `([^<]+)` on raw text; the port uses `$(el).children("loc").text()`, + which decodes. Verified: `https://x.com/p/a?b=1&c=2` now yields + `https://x.com/p/a?b=1&c=2` where Go yielded the literal `&`. This changes + both the URL the deep crawl fetches and the `postUrl` emitted on a sitemap + hit. JS is correct per the sitemap spec; Go was fetching a broken URL. Worth + recording as an accepted improvement alongside the RSS `` note. + +2. **`itemLink` is not byte-identical to Go for numeric entities.** The report + states link values are "byte-identical to what Go produced". The cheerio + parse/serialize round trip normalizes `&` → `&`: + + ``` + https://x.com/p?a=1&b=2&c=3 + Go: …?a=1&b=2&c=3 JS: …?a=1&b=2&c=3 + ``` + + `&` and CDATA both round-trip exactly, so real Substack feeds are + unaffected. The claim just needs narrowing to "`&` and CDATA". + +3. **`cleanUrl` inherits WHATWG normalizations Go's `url.Parse` did not apply.** + Verified: + + | input | Go | JS | + |---|---|---| + | `https://x.com/a/../b?keep=1` | unchanged | `https://x.com/b?keep=1` | + | `https://x.com:443/p` | port kept | port stripped | + | `https://x.com/p?a=b c&d=1` | raw space kept | `a=b%20c` | + + The query-verbatim trap itself is correctly handled — the port rebuilds the + query by string surgery and never touches `URLSearchParams`, and + `%20`/`%7e`/valueless-key all survive (confirmed by the golden case and by + direct probe). These three are path/authority-level, not query-level, and are + implausible for real newsletter inputs. Record rather than change. + +## Low + +### L1 — `engine-commands.md` overstates the missing-dependency contract + +"Every subcommand prints `newsletter engine: dependencies missing …` and exits 1 +if that step was skipped." Verified against a copy of the engine outside +`node_modules` reach: + +- `find-substack-post` → correct message, exit 1 ✔ +- `fetch-via-defuddle` → `ERR_MODULE_NOT_FOUND` from the dynamic + `import("defuddle/node")` is caught by `runFetchViaDefuddle`'s own try block, + reported as "local defuddle failed", then the proxy runs and the command + **succeeds with exit 0** + +The runtime behaviour is better than the doc (graceful degradation). Fix the +sentence, not the code. + +### L2 — `\s` is wider in JS than in Go + +`NEWSLETTER_NUM_RE`, `TAGS_LINE_RE` and the three `post-stats` regexes use `\s`, +which in JavaScript matches NBSP and other Unicode spaces; Go's RE2 `\s` is +`[\t\n\f\r ]`. So `Newsletter #42` now parses where it previously did not. +An improvement, but it is a silent widening of a parsing contract and belongs in +the accepted-differences list. + +### L3 — one export without JSDoc + +`find-newsletter-number.js:9` — `export const NEWSLETTER_NUM_RE` is the only +exported symbol in the engine with no doc comment. Every other export +(30 of them) is annotated. Repo policy is JavaScript + JSDoc; this is the one +gap. + +### L4 — nothing in CI runs `npm test` or `npm run lint` + +`hugo.yml` runs `npm ci` (guarded, for the site build) and `update-projects.yml` +runs `npm ci` for the projects script. Neither runs the new suite, so the +regression coverage this migration just bought only exists on the developer's +machine. A job on `pull_request` / `push` running `npm ci && npm run lint && +npm test` is a few lines. Flagging as a recommendation, not a blocker — CI was +explicitly out of scope for this pass. + +## Verified correct (risk calibration) + +Checked directly rather than taken from the report, because the report is the +artifact under review: + +- **Seven traps.** Query rebuilt verbatim by string surgery, never + `URLSearchParams` (`url-utils.js:49-66`). `hasBoundaryMatch` is an index loop + with the same delimiter set and optional-trailing-slash rule as Go + (`url-utils.js:259-298`); `/p/foo` vs stored `/p/foo-bar` probed and correctly + *not* a duplicate. Optional fields are added conditionally, so `title`, + `author`, `uuid`, `innerUrl` are absent rather than `""`. `cutoff` initialises + to `null` and is always emitted. `[...text].length` counts code points. + `padStart(6)` reproduces `%6d`. `decodeURIComponent` is wrapped and falls back + to the raw substring. +- **JSON key order and types** match the Go struct field order one-for-one in all + four JSON-emitting commands, and the suite asserts stdout byte-for-byte. +- **Walk order.** `collectMarkdown` sorts each directory's entries lexically and + recurses in place, reproducing `filepath.WalkDir` ordering — which is what makes + the `list-existing-tags` tie-break deterministic. `Array.prototype.sort` is + stable (ES2019+), matching `sort.SliceStable`. +- **`captionForUuid`** is faithful despite the rewrite. Probed the divergence + candidates — uuid first appearing in a ``, in an earlier ``, or + on an ancestor of the figure — all return `""` in both engines. +- **`process.exit()` after a write** is safe here: on Linux, pipe writes from + `process.stdout`/`stderr` are synchronous, so no diagnostic is truncated. +- **Call sites.** Every command named in `.claude/skills/`, `docs/`, `AGENTS.md` + and `README.md` exists with the arguments it is given. `grep "go run"` across + the repo is empty. All six `.agents/skills/*/SKILL.md` symlinks resolve. + `mt-add-post` / `mt-webfetch` survive only inside `plans/`, which is correct. +- **Go retirement is complete.** No `.go`, no `go.mod`/`go.sum`, no `GO_VERSION`, + no `setup-go`. `eslint.config.mjs` is the only remaining `.mjs`. + `.github/scripts` internal imports were updated to `.js`. + +## Carried over from Go, not a regression + +`urlBoundaryOk` treats `_` as a delimiter, so `https://x.com/p/foo` is reported +as a duplicate of a stored `https://x.com/p/foo_bar` (probed: `true`). The `_` +is there to let `_WxH` match, and the behaviour is byte-identical to the Go +original, so it is out of scope for this migration. Recording it because the +consequence — a URL silently dropped from a newsletter as a false duplicate — is +the kind of failure that is hard to notice after the fact. + +## Recommended actions + +1. **H1** — remove the import cycle (move `printJson` out of `index.js`) or guard + `main()`. Then add unit tests for the pure exports that cannot be tested today. +2. **M2** — emit frontmatter from the local tier's `title`/`author`/`site`/ + `description`, or correct `mt-fetch-url/SKILL.md`'s output-handling section. +3. **M1** — add a minimal quality gate before the local tier returns, so a + 200-with-challenge-page does not short-circuit the proxy. +4. **M3** — add the three items to the parity report's accepted differences and + narrow the `itemLink` "byte-identical" claim. +5. **L1** — correct the missing-dependency sentence in `engine-commands.md`. +6. **L2, L3, L4** — record the `\s` widening; add JSDoc to `NEWSLETTER_NUM_RE`; + consider a CI job running lint + test. + +## Plan status + +All five phases in +`plans/260918-0647-migrate-newsletter-engine-to-javascript/plan.md` are marked +Done and every success-criteria checkbox is ticked. Spot-checked against the +repo, each ticked criterion holds. Two carry caveats worth noting before the plan +is closed: the "`npm test` passes from a clean `npm ci`" criterion is true but +fragile per H1, and "one real `mt-add-url` flow completes end-to-end" was not +re-verified in this pass. Plan file not modified — plan mutation belongs to the +lead. + +## Metrics + +- Type coverage: JSDoc on 30 of 31 exported symbols (L3); no TypeScript, per repo policy +- Test: 33 deterministic assertions pass offline in 2.3s; 8 live cases opt-in +- Lint: 0 issues (`eslint .`) +- Build: n/a (no build step); Go toolchain fully removed from CI and Netlify + +## Unresolved questions + +1. M2 — align the engine's local tier to the proxy's frontmatter shape, or + rewrite the skill's output-handling section? The engine change is smaller and + keeps the skill's title guidance true, but it is a (small) output-contract + change on a command the user has already accepted as changed. +2. M1 — is a word-count/shape heuristic wanted at all, or is the agent-level + "no usable content" judgement in `mt-fetch-url` considered sufficient? That + judgement currently routes past the proxy rather than into it. +3. L4 — is CI for lint/test in scope, or deliberately deferred with the rest of + the unexercised CI work? diff --git a/plans/reports/parity-260918-newsletter-js-migration-report.md b/plans/reports/parity-260918-newsletter-js-migration-report.md new file mode 100644 index 0000000..1fdb7c9 --- /dev/null +++ b/plans/reports/parity-260918-newsletter-js-migration-report.md @@ -0,0 +1,174 @@ +# Parity Report — Newsletter Engine Go → JavaScript (2026-09-18) + +The Go engine's stdout was captured for 26 deterministic cases before the +JavaScript port was trusted, committed under +`scripts/newsletter/__fixtures__/golden/`, and asserted byte-for-byte by +`scripts/newsletter/parity.test.js`. Live-upstream behaviour is covered by +`network.test.js`, which is opt-in. + +## Results + +`npm test` — 40 assertions, 40 pass, 0 fail, offline. +`NEWSLETTER_NET=1 npm test` — 48 assertions, 48 pass, 0 fail. + +| Subcommand | Deterministic cases | Live cases | Result | +|---|---|---|---| +| `add-url` | 15 | 4 | pass | +| `detect-image-source` | 4 | — | pass | +| `find-newsletter-number` | 3 | — | pass | +| `list-existing-tags` | 1 | — | pass | +| `post-stats` | 3 | — | pass | +| `find-substack-post` | 3 (extraction, saved markup) | 3 | pass | +| `fetch-via-defuddle` | 1 (exit 2) | 2 | pass | + +Content-scanning commands are pointed at `__fixtures__/repo` by setting the +subprocess working directory. Both engines resolve `content/post` from the +process CWD, so neither gained a test-only configuration knob. Verified by +renaming the real `content/post` away and re-running: 32/32 still pass. + +`add-url` performs a HEAD request on every URL, so its deterministic cases use +`.invalid` hosts (RFC 2606, guaranteed not to resolve). The status is a stable +`"000"` whether or not the machine is online. + +## Trap coverage + +Each of the seven traps identified in phase 1 has a case that goes red when the +naive implementation is reintroduced. Verified by mutating the engine and +re-running: + +| Trap | Discriminating case | Reintroducing the trap | +|---|---|---| +| Query kept verbatim | `add-url-query-verbatim` (`%20`, valueless key, lowercase `%7e`) | 1 failure | +| Boundary-aware dedup | `add-url-prefix-not-duplicate` | 3 failures | +| Empty optional fields omitted | `add-url-article-no-metadata`, `detect-image-non-substack` | 3 failures | +| `cutoff` is `null`, not omitted | live deep-miss case | see below | +| Code points, not UTF-16 units | the 70-code-point bullet | 5 failures | +| Six-column right-aligned counts | `list-existing-tags-main` | 3 failures | +| Malformed percent escape | `detect-image-malformed-percent` | 3 failures | + +The first discriminating case chosen for the query trap did not discriminate: +`%7E` and `+` survive a `URLSearchParams` round trip unchanged. The case was +replaced with one that does — `%20` becomes `+`, a valueless key gains `=`, and +lowercase `%7e` is upper-cased by the re-encoder. + +`cutoff` is exercised live only. The publication list is deliberately not +test-configurable (the "no test-only knob" decision), so the branch that emits +`"cutoff": null` cannot be reached offline. A live `--deep` miss was diffed +against the Go engine directly and came back byte-identical. + +## Accepted differences + +**The parser recovers a TOC bullet the regex dropped.** On the live ByteByteGo +feed, `extractCandidates` returns one more candidate than the Go regex did — the +first bullet of the "in this issue" list, +`"An LLM needs evidence to answer questions"`. The markup is +`

  • …

    • `, identical in shape to the bullets the regex did +match, so this is the regex losing an item, not the parser inventing one. Every +candidate the regex found is still present and in the same order; the suite +asserts that explicitly rather than pinning the full list. + +Accepted: more complete candidate lists, nothing lost. This is the R3 case the +plan anticipated. + +**`fetch-via-defuddle` exit 2 is now observable.** The Go source defined exit 2 +for bad arguments and exit 1 for a failed fetch, but `go run` collapsed every +nonzero exit to 1. Node preserves them, so the distinction is real for the first +time. No caller depended on the collapsed behaviour. + +**`fetch-via-defuddle` gained a local extraction tier.** Local defuddle runs +first, the `defuddle.md` proxy remains as the second tier. The proxy is the tier +that matters when this machine's IP is the one being blocked, so it was not +dropped. Verified on ARM64: `defuddle/node` + `linkedom` import and extract +without native dependencies. + +**`--uuid` no longer accepts the single-dash form.** Go's `flag` package accepted +`-uuid`; `node:util`'s `parseArgs` accepts only `--uuid`. Every documented call +site uses the double-dash form. Accepted narrowing. + +**RSS `` entity handling is unchanged, deliberately.** cheerio's `.text()` +decodes entities, which would have turned a stored `&` into `&`. The port +reads the element's raw serialized inner content instead. `&` and CDATA +round-trip exactly; a *numeric* entity is normalized by the parser's +re-serialization (`&` → `&`), which no observed Substack feed emits. + +**Sitemap `` entities are now decoded.** Go's `([^<]+)` kept +`&` literal, so a sitemap entry with an escaped ampersand produced a URL the +crawler then failed to fetch and a broken `postUrl` on a hit. `children("loc").text()` +decodes it. XML says the decoded form is the URL; accepted as a fix. + +**`cleanUrl` inherits WHATWG normalizations Go's `url.Parse` did not apply** — +dot-segment removal (`/a/../b` → `/b`), default-port stripping (`:443`), and a +raw space becoming `%20`. These are path- and authority-level; the query is still +rebuilt by string surgery and never re-encoded, which is what the stored +`clean_url` values depend on. No observed input in `content/post` is affected. + +**JavaScript `\s` matches more than RE2's `\s`.** `NEWSLETTER_NUM_RE`, +`TAGS_LINE_RE` and the `post-stats` patterns now also match a non-breaking space +where Go did not. A silent widening, and in this content the forgiving direction: +`Newsletter #132` typed with an NBSP would be found rather than missed. + +## Carried over from Go, not introduced here + +`urlBoundaryOk` treats `_` as a delimiter, so a stored +`https://x.invalid/p/foo_bar` makes `https://x.invalid/p/foo` report as a +duplicate. Byte-identical to the Go behaviour and therefore out of this +migration's scope, recorded because the consequence — a URL silently dropped as a +false duplicate — is hard to notice after the fact. + +## Found during review, fixed + +**Piping any command into a reader that closes early crashed.** +`node scripts/newsletter list-existing-tags | head -3` printed an EPIPE stack +trace; Go took SIGPIPE and died silently. `index.js` now exits quietly on EPIPE, +with a regression test. + +**Importing a command module ran the CLI.** The four JSON-emitting commands +imported `printJson` from `index.js`, which calls `main()` at module scope — so +importing any of them executed the dispatcher against the host process's argv, +and the pure functions the port exports for testing could not be imported at all. +`printJson` moved to `json-out.js`, breaking the cycle. Six unit tests now cover +`detectYouTube`, `extractInnerUrl`, `countPostEntries`, `parseLastmod`, +`loadPublications` and `findMostRecentNewsletter`, which is the standing proof +the cycle stays gone. + +**The local defuddle tier could consume the proxy tier's turn.** A bot wall +answers 200 with a real body, which extracted to non-empty and returned exit 0 — +so the `defuddle.md` proxy, the tier that exists precisely because this machine's +IP is blocked, never ran. A challenge-shape check on the extraction title and the +opening of the body now counts that as a local failure. Marker-based rather than +length-based on purpose: a legitimate short page (example.com is 17 words) must +still succeed. + +**The two tiers returned different shapes.** The proxy returns YAML frontmatter +plus body; the local tier returned a bare body, while the skill told the agent +tier 1 carries a title in frontmatter — for a value that becomes an article +heading in a published post. The local tier now emits the same frontmatter +(title, author, description, site, published, source) from defuddle's metadata. + +## Fixtures + +Reviewed by eye before commit. No fixture was found to encode a Go bug. + +- `__fixtures__/repo/` — six posts covering every shape a URL is stored in + (bare, trailing slash, markdown autolink, markdown link, image), a newsletter + in an older year holding a *higher* number than the newest year, and the three + `post-stats` shapes. +- `__fixtures__/repo-empty/`, `__fixtures__/repo-year-fallback/` — the empty tree + and the newest-year-has-none cases for `find-newsletter-number`. +- `__fixtures__/markup/bytebytego-feed.xml` — two real feed items, 64 KB. +- `__fixtures__/markup/bytebytego-sitemap.xml` — 40 real sitemap entries, 6 KB. +- `__fixtures__/markup/edge-cases.html` — synthetic: a nested `
    `, a + caption-less figure, and bullets at exactly 5, 6, 70 and 71 code points with a + non-BMP character. + +The live feed carries no `
    ` at all, which is why the engine's own +comment says ByteByteGo does not caption its images. Caption extraction is +therefore covered by the synthetic nested-figure fixture, where the innermost +enclosing figure must supply the caption and a caption-less figure must yield +nothing rather than borrowing the outer one. + +## Verdict + +Parity proven. The JavaScript engine reproduces the Go engine on every captured +case, and every difference is deliberate, recorded above, and an improvement or +an accepted narrowing. Safe to cut the call sites over. diff --git a/plans/reports/research-260918-1139-script-language-choice.md b/plans/reports/research-260918-1139-script-language-choice.md index 0ee1c7d..875728c 100644 --- a/plans/reports/research-260918-1139-script-language-choice.md +++ b/plans/reports/research-260918-1139-script-language-choice.md @@ -120,6 +120,17 @@ engine does not call the GitHub API. 1. **`scripts/newsletter/` stays Go.** No GitHub API, no deps, no latency problem, three agent runtimes served identically. Preserve the no-`go.sum` invariant. + + > **Superseded 2026-09-18.** The user reviewed this recommendation the same + > day and chose to migrate the engine to JavaScript anyway, to collapse the + > repo onto one language and replace the hand-rolled HTML/XML regexes with a + > real parser. Carried out by + > `plans/260918-0647-migrate-newsletter-engine-to-javascript/`; parity proven + > in `plans/reports/parity-260918-newsletter-js-migration-report.md`. The + > recommendation above is left as written — it records what the evidence + > supported at the time, and its strongest objection (a bare clone no longer + > runs the engine without `npm ci`) is carried forward as risk R1 in that + > plan. 2. **`.github/scripts/` stays JavaScript; migrate to `octokit`.** Add a root `package.json` (+ lockfile) with `octokit` as the single dependency, plus ESLint and JSDoc per repo policy. Expected outcome: `lib/github-api.mjs` @@ -133,6 +144,10 @@ engine does not call the GitHub API. ### Fix first (latent, found while scouting) +> **Resolved 2026-09-18 by removal.** Go was retired from the repo along with the +> engine, so there are no pins left to align. + + `go.mod` declares `go 1.26`, but `hugo.yml` pins `GO_VERSION: 1.25.5` and `netlify.toml` sets `GO_VERSION = "1.25.5"`. Running any `go` command in those environments only succeeds because `GOTOOLCHAIN=auto` silently downloads 1.26.