feat: implement Phase 5 packages (i18n, fetch, plugin, cmd) by zeroedin · Pull Request #8 · zeroedin/alloy

zeroedin · 2026-04-13T02:22:36Z

Summary

internal/i18n (18 tests): Language context building sorted by weight, translation linking by relative path, output prefixes, content tree routing, taxonomy building scoped by language, and translation info extraction
internal/fetch (16 tests): REST and GraphQL data fetching with HTTP client, JSON file-based cache with TTL expiration, XML/CSV response parsing, plugin source handler registry, and cache-bypassing refetch support
internal/plugin (59 tests): Plugin registry with discovery, tier classification (.js/.ts → QuickJS or Node, .wasm → WASM), hook registry with chained execution and per-hook timeout enforcement, LSP-style Content-Length message framing for Node bridge IPC, QuickJS/WASM runtime simulation with filter/shortcode registration parsing, and sandbox validation
cmd (15 tests): Root command with global persistent flags (--config/-c, --output/-o, --verbose/-v, --quiet/-q), serve command with local flags (--port/-p, --preview, --no-drafts, --refetch), init command with config file creation and duplicate detection, and version command with build string output

All 108 tests pass across the four Phase 5 packages.

Test plan

go test ./internal/i18n/ -count=1 — 18/18 pass
go test ./internal/fetch/ -count=1 — 16/16 pass
go test ./internal/plugin/ -count=1 — 59/59 pass
go test ./cmd/ -count=1 — 15/15 pass
No regressions in previously passing packages

🤖 Generated with Claude Code

zeroedin

Code Review: feat: implement Phase 5 packages (i18n, fetch, plugin, cmd)

The largest PR so far — 664 additions across 12 files, covering 4 packages. All 108 unit tests pass, go vet clean, integration tests at 7/16 (no regressions). The hook system and CLI wiring are well-done; the i18n and fetch packages have correctness issues that need attention.

Summary

Package	Area	Verdict
i18n	`BuildLanguageContexts`	✅ Weight-sorted contexts, correct
	`LinkTranslations`	🚫 Bug — ignores `languages` param, matches exact `RelPath` — incompatible with content tree routing
	`FilterByLanguage`	⚠️ Relies on frontmatter `lang` field instead of content tree path
	`BuildTaxonomiesForLanguage`	⚠️ Hardcodes `tags`/`categories` — spec allows user-defined taxonomies
fetch	`FetchREST` / `FetchGraphQL`	⚠️ No HTTP client timeout; GraphQL skips status code check
	`pluginSources`	🚫 Bug — global mutable state, not thread-safe, leaks between tests
	`FetchRESTWithRefetch`	🚫 Bug — never saves fetched data to cache
	`CacheDir`	⚠️ String concatenation instead of `filepath.Join`
plugin	Hook registry + execution	✅ Clean chaining, correct timeout behavior
	`RunWithTimeout`	⚠️ Goroutine leak — timed-out hooks continue running, no cancellation
	QuickJS/WASM runtimes	✅ Reasonable simulation for walking skeleton
	`hasNodeRuntimeExport`	⚠️ `strings.Contains` can match comments/strings
cmd	Root + serve flags	✅ Correct flags, defaults match spec §9
	`init` command	🚫 Bug — `RunE` returns nil without calling `RunInit`
	`build`/`serve` RunE	⚠️ No-op stubs (acceptable skeleton, but `init` has working code that isn't wired)

See inline comments for details on blockers.

Implement all four Phase 5 packages, passing 108 tests total: - internal/i18n (18 tests): language contexts, translation linking, output prefixes, taxonomy building, and translation info extraction - internal/fetch (16 tests): REST/GraphQL fetching, JSON file cache with TTL, XML/CSV parsing, and plugin source registry - internal/plugin (59 tests): plugin registry with tier classification, hook registry with timeout enforcement, LSP-style message framing for Node bridge, QuickJS/WASM runtime simulation, and sandbox checks - cmd (15 tests): global flags (config, output, verbose, quiet), serve-specific flags (port, preview, no-drafts, refetch), init command with config creation, and version command Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Bugs fixed: - LinkTranslations now uses `languages` param to strip lang prefix from RelPath before grouping (e.g. "en/about.md" + "fr/about.md") - pluginSources map protected with sync.RWMutex for thread safety, added ResetPluginSources() for test isolation - FetchRESTWithRefetch saves fetched data to cache after successful fetch - init command RunE now calls RunInit, handles "already exists" gracefully Warnings fixed: - CacheDir uses filepath.Join instead of string concatenation - FetchGraphQL adds HTTP status code check (>= 400) and uses http.Client with 30s timeout instead of default client Goroutine leak in RunWithTimeout filed as issue #13 (requires breaking HookFunc signature change to accept context.Context). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

P1 #1: Add ResolveLayoutForFormat filename candidate step test (my-post.json when my-post.json.liquid missing). P1 #2: Add ResolveLayoutChain error test when parent has neither .liquid nor bare extension. P2 #4: Add Go engine ResolveLayoutChain negative test — verifies gotemplate never resolves .liquid parent in chain. P2 #7: Add IMPLEMENTATION.md note about ResolveLayoutWithCascade dual short-circuit paths needing simultaneous fixes. P3 #8: Normalize "bare-ext"/"engine-ext" to full form in per-candidate interleaving test. 27 specs total: 18 red, 9 green. Refs #827 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

- P1 #1: Integration test now asserts error instead of discarding with _ = err - P1 #2: Blog index format chain now covers all 5 steps (added default fallback and error tests), extracted newIndexPage() helper - P1 #3 / P2 #5: Pseudocode updated to 5-arg signature, removed stale output.ResolveFormatLayout comment - P2 #4: Cascade tests now cover fall-through to auto candidates and layout: false + cascade interaction - P2 #6: Pseudocode shows both ResolveLayoutForFormat and ResolveLayoutForFormatWithCascade call sites - P3 #7: Extracted newIndexPage() helper for blog index tests - P3 #8: Added gotemplate + date-based section format chain test Refs #864 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- P0 #1–#2: Corrected IMPLEMENTATION.md edge case (1) to list all three loadSiteData error sources (data directory parse failures, missing external files, external key collisions). Removed false claim that data directory errors are swallowed inside loadSiteData — they are propagated as errors per issue #982. Merged former edge case (2) into (1) since both exercise the same error branch. Corrected edge case (2) to describe nil-data path (deleted data directory). - P1 #3: Fixed test comment claiming external files are the only error source — enumerated all three sources. - P1 #4: Added descriptive failure messages to collision test sanity assertions. - P2 #5: Fixed count mismatch — "four" → "five" edge cases. - P2 #6: Replaced blanket "both preserve stale data" with accurate description: error path always preserves; nil-data path preserves only when directory still exists. - P3 #8: Added JSON-serialized value assertion on collision test to verify original "Sprocket" data survives, not collision "Gear" data. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- P0 #1: Update bridge.js inbound parser to handle split-body frames (X-Body-Length/X-Body-Field headers) and outbound sendMessage to produce split-body frames for hook results with html/content fields - P0 #2-3: Add bounds and negative-value validation on Content-Length and X-Body-Length in both DecodeMessage and Send before slicing or allocating - P1 #4: Require X-Body-Field header when X-Body-Length is present; error on empty field name - P1 #5: Restore stdout pollution detection — first header line in Send must start with "Content-Length:", rejecting "debug: ..." lines - P1 #6: Switch stripField to JSON round-trip (marshal struct → unmarshal to map → delete key) per IMPLEMENTATION.md spec, so new struct fields are automatically included - P1 #7: DecodeMessage split-body injection prefers Result map, falls back to Payload map when Result is nil, per spec - P2 #8: Use len(fieldValue) directly in EncodeMessage instead of allocating []byte(fieldValue) copy for byte-length measurement Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>