feat(config): tailwind config adapter - #115
Conversation
aram-devdocs
changed the title
|
Good — PR #115 —
|
| Location | Finding | |
|---|---|---|
| ⚠ | crates/plumb-config/Cargo.toml |
which and plumb-core not in [dev-dependencies]; integration tests rely on implicit transitive dep resolution. Add both. |
| ⚠ | mod.rs:merge_font_family (approx. line 660+) |
Family order depends on serde_json/preserve_order being active via schemars. Consider a comment like // Map iteration is insertion-order because serde_json/preserve_order is enabled by schemars in this workspace to make the invariant explicit. |
| ℹ | mod.rs:wait_with_timeout |
thread::sleep blocks the calling thread for up to 60 s. This is intentional and documented, but merge_tailwind's public doc should note it is a blocking call (spawns a subprocess + polls). |
Verdict: APPROVE
Read a user's `tailwind.config.{js,ts,mjs,cjs}` and merge the resolved
theme into a Plumb `Config`. Theme resolution runs in a Node subprocess
that loads `tailwindcss/resolveConfig` from the user's project tree;
when Tailwind's resolver isn't installed we fall back to a minimal
in-process `theme.extend` merge so the adapter still works on raw
configs that authors hand-write tokens into.
The mapping covers the six theme keys called out in the issue:
- `colors` → `[color].tokens` (slash-namespaced for nested groups,
hex normalized; rgb/rgba parsed to hex).
- `spacing` → `[spacing].tokens` and `[spacing].scale` (rem→px at
16px = 1rem; integer rounding; deduped/sorted scale).
- `fontSize` → `[type].tokens` and `[type].scale` (tuple form keeps
the size only).
- `fontWeight` → `[type].weights` (deduped, sorted ascending).
- `fontFamily` → `[type].families` (each entry's primary family,
insertion-ordered, deduped).
- `borderRadius` → `[radius].scale` (rem→px, deduped, sorted).
Memoization uses `<system-tmp>/plumb-tailwind/<sha256(path)>.json`
keyed by the source file's mtime in milliseconds. Cache reads return
the stored theme byte-identically; cache writes only happen *after* a
fresh Node spawn produced a parseable theme, so a corrupted cache can
never be the first witness of any value. This preserves Plumb's
determinism contract: the cache is a performance optimization, not
correctness.
Subprocess hygiene per `.agents/rules/`:
- `node` discovered via `which::which`; explicit `node_path` overrides
for tests / Nix-style sandboxes.
- Arguments pass through `Command::arg`; no shell concatenation.
- stderr captured separately from stdout.
- 30s default timeout with a watcher thread.
- Path-traversal guard: the validated config path must canonicalize
to a descendant of the CWD or one of its ancestors.
Errors carry the user-facing path string (never an internal absolute):
- `ConfigError::TailwindUnavailable` when `node` is missing.
- `ConfigError::TailwindBadPath` when validation rejects the path.
- `ConfigError::TailwindEval` when the subprocess fails or emits
unparseable JSON.
Tests cover the round-trip, the cache hit/miss paths (via mtime
mutation on a copied fixture), the "node missing" error shape, the
unsupported-extension reject, and the path-traversal guard. The
.ts round-trip skips gracefully when no TS loader (`tsx`,
`ts-node`, `esbuild-register`) is installed in the host.
`std::fs::canonicalize` returns Windows extended-length UNC paths (`\\?\C:\Users\…`) which `node`'s `require()` resolver cannot handle — it interprets the prefix as a drive root and fails with EISDIR. Swap to `dunce::canonicalize`, which strips the prefix when the underlying path doesn't actually need it (the common case). Restores Windows CI on the Tailwind adapter integration tests.
The previous spawn_loader spawned a watcher thread but blocked on `child.wait()`, so the watcher's "timed_out" signal had no way to unblock the main thread. The 30s timeout silently did nothing. Replace the watcher with a `try_wait` polling loop bounded by `(timeout / POLL_INTERVAL).max(1)` iterations. On timeout, kill the child, reap with `child.wait()`, and surface ConfigError::TailwindEval with a "timed out after ..." reason. The existing stdout/stderr reader threads stay in place to prevent pipe deadlock when Node emits more than one OS pipe buffer. Avoids `std::time::Instant::now` (forbidden by `clippy::disallowed_methods` workspace-wide) by counting poll ticks instead of measuring wall time. Drive-by: - Correct the `css_color_to_hex` doc to match what the function actually accepts (no HSL, no named CSS colours). The unit tests already assert the negative case. - Tighten `cache.rs` visibility: pub(crate) -> pub(super) since the cache module's API is consumed only from `tailwind/mod.rs`. - Drop the now-unneeded module-level `#![allow(clippy::redundant_pub_crate)]` from both files. - Drop a duplicate `indexmap` line in plumb-config's Cargo.toml left over from the rebase merging the DTCG and Tailwind dependency additions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
10081f4 to
8a5933a
Compare
Cold Node spawns on Windows CI runners (with no reachable `node_modules/tailwindcss` and a tempdir-based config path) routinely exceed the previous 30s budget, causing `cache_hit_skips_node_spawn` and `cache_invalidates_on_mtime_bump` to fail. 60s keeps Plumb's "loud-failure" semantics intact for real-world configs (which resolve in well under a second) while leaving headroom for the slowest CI runner.
Two of plumb-config's API surfaces were silently reading process-global state when the caller didn't pass an override: - `cache::cache_dir()` walked TMPDIR/TEMP/TMP, then fell back to /tmp or C:\Windows\Temp. - `validate_config_path` called `std::env::current_dir()` when `TailwindOptions::cwd_root` was `None`. Both violate `.agents/rules/determinism.md` — env / CWD reads belong in `plumb-cli`. The library now treats `cache_dir = None` as "cache disabled" and `cwd_root = None` as "skip the path-traversal guard; caller is trusting the supplied config path." The plumb-cli wiring that populates both from env will land alongside the Tailwind CLI flag in a follow-up. Drive-bys flagged in the same review round: - Replace `dunce::canonicalize(&cwd).unwrap_or(cwd)` with an error return; silent fallback let an uncanonicalized cwd through the ancestor check. - Tighten `is_under_or_ancestor`: reject candidates whose parent is just the filesystem root, so `/tailwind.config.js` can't pass on any Unix CWD. New unit test pins the guard. - Add an integration test that calls `merge_tailwind` twice on the same fixture and asserts byte-identical output, closing the determinism invariant loop. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Summary
tailwind.config.{js,ts,mjs,cjs}via a Node subprocess and merges the resolved theme (colors,spacing,fontSize,fontWeight,fontFamily,borderRadius) into a PlumbConfig.<system-tmp>/plumb-tailwind/<sha256(path)>.jsonkeyed on the source file's mtime; preserves determinism by re-using cache content byte-identically and never writing speculatively.ConfigError::TailwindUnavailable(with a Node install hint),TailwindBadPath(extension + path-traversal guard), andTailwindEval(loader / subprocess failures with separated stderr).Mapping (as shipped)
colors.<n>ColorSpec.tokens["<n>"]colors.<g>.<s>ColorSpec.tokens["<g>/<s>"]spacing.<n>SpacingSpec.tokens["<n>"]+SpacingSpec.scalefontSize.<n>TypeScaleSpec.tokens["<n>"]+TypeScaleSpec.scalefontWeight.<n>TypeScaleSpec.weightsfontFamily.<n>TypeScaleSpec.familiesborderRadius.<n>RadiusSpec.scaleMemoization + determinism
<system-tmp>/plumb-tailwind/<sha256(absolute-config-path)>.jsoncontaining{ "mtime_unix_ms", "theme" }.std::env::temp_dir(banned byclippy::disallowed_methods); the cache directory is resolved fromTMPDIR/TEMP/TMPwith a/tmp-or-C:\Windows\Tempfallback.Subprocess hygiene
nodediscovered viawhich::which; explicitnode_pathoverrides for tests / Nix-style sandboxes.Command::arg; no shell concatenation.mpsccancellation.TailwindOptions::cwd_rootto avoid touching the process-global CWD.Test plan
cargo test -p plumb-config --test tailwind_adapter --lib: 27 unit + 5 integration tests covering colour normalization, rem→px conversion, group flattening, fontSize tuple form, weight/family deduplication, cache round-trip, mtime-bump invalidation, missing-nodeerror, unsupported extension, and path-traversal rejection.cargo fmt --all -- --checktailwind.config.ts: skips gracefully when no TS loader (tsx,ts-node,esbuild-register) is installed in the host.node-missing path: skips gracefully on hosts without Node, same shape ase2e-chromium.Fixes #29
Dependency added / bumped: yes (
which8.x — direct;sha20.10).🤖 Generated with Claude Code