[mache-73b885] refactor(writeback): validate + lint via leyline v0.7.8 validate op — drop in-process tree-sitter from the write path#527
Merged
Conversation
…) + regen baseline v0.7.8 ships the two mache-blocking adds: validate emit_ast (one parse serves syntax validation + _ast/defs/refs SQL - the writeback+linter migration carrier) and identifier-as-VALUE refs in the Go extractor. Baseline regenerated against the new pin: dead_code 16 -> 3 - the 13 grandfathered false-positives (cobra RunE handlers, test-suite factories) collapse exactly as predicted when ley-line-140307 shipped; the 3 survivors are the true positives the tree-sitter projection also flagged. Verified: fresh SHA-verified download, deterministic build.
…ne validate op (drop in-process tree-sitter) Work items 3+4: the last two in-process CGO tree-sitter consumers on the write-back path now ride the pinned leyline daemon (ley-line-open v0.7.8): - internal/leyline: Validate client over SendOp for the daemon validate op, incl. the emit_ast extension (one parse -> syntax verdict + SQL-shaped _ast/defs/refs/imports rows in the response). Daemon acquired lazily per call via DiscoverOrStart; responses without the errors key (or without ast when emit was requested) are hard daemon-too-old errors, never silent. - internal/writeback: Validate/ASTErrors keep their exact signatures and 0-based ValidationError shape but delegate to the daemon; new ValidateWithAST returns the same-parse AST payload for the linter. Language set is now the daemon validate set (go/py/js/ts/tsx/rs/ex/exs); .tf/.hcl/.sql/.yaml/.md/.toml become pass-through (documented). - internal/linter: nil-slice rule reimplemented as SQL over the emit_ast rows (in-memory modernc sqlite, direct-child via hierarchical node_id); LintAST consumes the write path's existing parse so mount_nfs runs ONE parse per write instead of two. The old rule's false positive on initialized slices (var x []T = v) is fixed by the expression_list check. - cmd/mount_nfs.go: validate+lint folded into one ValidateWithAST call. - tests/integration_test.go: sitter fixture parsing replaced by a hand-written functions/<name>/source topology; write-exercising tests gated on a private pinned daemon (never downloaded, skip when absent). - internal/lltest: FakeDaemon (canned wire responses over UDS) and StartPinnedDaemon (SHA-pinned binary, isolated arena/socket) test doubles. grep -rln smacker/go-tree-sitter internal/writeback/ internal/linter/ tests/ now returns nothing.
… + cover new exports Smell-gate burndown for the previous commit: shared lltest.UsePinnedDaemon replaces three per-package copies, StartPinnedDaemon fan-out split into pinnedBinaryOrSkip/awaitSocket, table-driven LintAST + e2e verdict tests replace near-duplicate bodies, and the new exports (ValidateContent, PinnedBinaryVersion, FakeDaemon) get direct tests.
… StartPinnedDaemon
…nned binary Surfaced by the writeback migration: the daemon SPAWN path still did a raw LookPath + unchecked ~/.mache/bin stat (the mache-0acdf6 drift), so a stale PATH leyline (0.7.3 on this very machine) would be spawned as the daemon - and with the validate op requiring >= v0.7.8, every write-back would draft with a daemon-too-old diagnostic. The spawn now routes through ResolveBinary (PATH -> cache -> SHA-verified download, exact pin at every tier; respects MACHE_NO_LEYLINE). An already-RUNNING daemon (fast paths) is still honored as-is - it is the user's own; the wire handshake and per-op probes cover it. The three spawn-branch tests (socket-timeout / crash / clean-exit during startup) now seed PIN-VERSIONED stubs and set MACHE_NO_LEYLINE so tests can never hit the network.
…+ honest pass-through docs + infra/syntax distinction Review findings on PR #527: 1. RESTORED HCL validation (was should-fix 1, taken as behavior not docs): broken HCL passed through unvalidated AND hclwrite.Format - a token formatter, not a validator - MANGLED it before splicing to the source file where pre-73b885 it drafted. HCL/Terraform now validates in-process via hclsyntax (pure Go, same hashicorp/hcl module), first-error ValidationError with 0-based positions. SupportedPath semantics = validated-at-all (daemon set + HCL). 2. Honest pass-through docs (should-fix 2): the public validate package and SupportedPath now state plainly that ~25 extensions the old grammar set covered (.c/.cpp/.java/.rb/.php/.kt/.lua/.sh/...) splice unvalidated until leyline grows grammars (ley-line-open-e5addb). 3. Infra vs syntax distinction (nit 2 + open questions): mount write-back prefixes _diagnostics with 'validator unavailable' for non-ValidationError failures; serve write_file reports status validator_unavailable instead of claiming validation_error.
1. The package header still carried the exact stale sentence should-fix 1 targeted (HCL 'passes through, hclwrite remains the structural check') 80 lines above code doing the opposite - rewritten to the hclsyntax reality, with the LLO grammar bead (ley-line-open-e5addb) named as the recovery path for the unvalidated set. 2. The diagnostics flavor (ASTErrors / public ContentErrors) skipped the in-process HCL branch: broken HCL errored via Content() but produced EMPTY diagnostics. Routed through a shared hclErrors helper returning ALL error diagnostics (validateHCL keeps the first-error contract). 3. Tests the review noted missing: ContentErrors on broken HCL (public surface, no daemon), and write_file status=validator_unavailable on infra failure (dead socket + MACHE_NO_LEYLINE).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Completes bead mache-73b885 (items 3+4): the write-back path no longer parses with in-process CGO tree-sitter.
grep smacker internal/writeback/ internal/linter/ tests/→ empty. PR-B (mache-37ae8b) is now sized at exactly 23 remaining sitter importers, all parse-side.Pin: ley-line-open v0.7.5 → v0.7.8 (SHA-verified)
v0.7.8 ships both mache-blocking adds. Baseline regenerated in the same commit per the standing rule — dead_code 16 → 3: the 13 grandfathered identifier-as-VALUE false-positives (cobra
RunE:handlers, suite factories) collapse now that the extractor emits those refs; the 3 survivors are the true positives the tree-sitter projection also flags.Writeback validation → leyline
validateopinternal/leyline/validate.go: typed client over the existing UDS machinery; codes against theerrors[]wire (0-based row/col, byte ranges, MISSING zero-width); a response without theerrorskey is a hard "daemon too old, need ≥ v0.7.8" error — never a silent fallback.internal/writeback/validate.go: sameValidatesignature +ValidationErrorshape (draft-mode diagnostics unchanged);ValidateWithASTreturns theemit_astpayload..tf/.sql/.toml/...) now pass through unvalidated (documented + test-pinned; hclwrite remains HCL's structural check).Linter → SQL over the same parse
internal/linterreimplements the nil-slice rule as SQL over theemit_astrows (in-memory modernc sqlite; direct-child structure via the materialized-path node_id). mount_nfs runs validate+lint from ONE parse per write. Bonus: fixes the old rule's false positive onvar x []T = v.DiscoverOrStart spawns only the exact-pinned binary
Surfaced during this work: the daemon SPAWN path still did raw LookPath + unchecked cache stat (mache-0acdf6 drift) — a stale PATH leyline would be spawned and every write would draft with daemon-too-old. The spawn now routes through
ResolveBinary(exact pin at every tier, SHA-verified download, MACHE_NO_LEYLINE respected). An already-running daemon is still honored (user-owned; wire handshake + per-op probes cover it). Spawn-branch tests seed pin-versioned stubs and can no longer hit the network.Test infrastructure
internal/lltest: FakeDaemon (canned UDS wire) + StartPinnedDaemon (private arena/socket, skip-never-download). Unit tests cover the wire contract, position mapping, daemon-too-old, pass-through; gated e2e covers real broken-Go positions, emit_ast round trip, nil-slice lint, and the full write-back pipeline through GraphFS (all executed locally, none skipped).tests/integration_test.gono longer imports sitter.Verification
Full
task checkgreen end-to-end;task smellspasses with 10 ratchet findings burned down structurally rather than baselined; fresh SHA-verified 0.7.8 download exercised.After this merges
mache-73b885 closes. PR-B (mache-37ae8b) deletes the remaining 23 sitter importers + CGO_ENABLED=0 releases, pending the language-coverage decision (leyline parses 11/28 registry languages).
🤖 Generated with Claude Code