🍰 Crème Brûlée Inspector - A Delicate Stack of Linting and Language Services#333
Closed
cds-amal wants to merge 9 commits intosolana-foundation:mainfrom
Closed
🍰 Crème Brûlée Inspector - A Delicate Stack of Linting and Language Services#333cds-amal wants to merge 9 commits intosolana-foundation:mainfrom
cds-amal wants to merge 9 commits intosolana-foundation:mainfrom
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR introduces two major user-facing features for txtx - a doctor command for validation and LSP support for IDE integration. Both features are implemented using compiler-inspired design patterns to demonstrate how compiler architecture principles could benefit txtx without modifying the existing execution pipeline. The implementation serves as both a working solution and a proof of concept for future architectural considerations.
Key Changes:
- Introduces
txtx doctorcommand with multi-phase validation and rich error messages - Adds full LSP support with real-time validation, completions, and hover documentation
- Implements new testing infrastructure with type-safe
RunbookBuilderAPI
Architectural Focus:
- Demonstrates compiler-inspired validation phases without changing execution model
- Shows how validation can be separated from execution for better testing and error detection
Reviewed Changes
Copilot reviewed 114 out of 182 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
docs/VALIDATION_ARCHITECTURE.md |
Documents the validation system architecture and validation context design |
docs/README.md |
Provides comprehensive documentation structure for the project |
crates/txtx-test-utils/ |
New testing utilities with RunbookBuilder API and validation helpers |
crates/txtx-core/src/validation/ |
Core validation infrastructure with HCL parsing, manifest validation, and validation types |
crates/txtx-lsp/ |
Legacy LSP crate removal (files deleted as part of architecture consolidation) |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Comment on lines
+205
to
+207
| // by the test crate that has access to txtx-cli | ||
| // For now, we provide a simple fallback that uses HCL validation | ||
| eprintln!("INFO: Using basic HCL validation for LSP mode. Implement RunbookBuilderExt::validate_with_lsp_impl for full LSP validation."); |
There was a problem hiding this comment.
The eprintln! statement will pollute stderr during test runs. Consider using a proper logging framework or conditional compilation for debug builds only. Alternatively, this could be a comment explaining the fallback behavior.
Suggested change
| // by the test crate that has access to txtx-cli | |
| // For now, we provide a simple fallback that uses HCL validation | |
| eprintln!("INFO: Using basic HCL validation for LSP mode. Implement RunbookBuilderExt::validate_with_lsp_impl for full LSP validation."); | |
| // by the test crate that has access to txtx-cli. | |
| // For now, we provide a simple fallback that uses HCL validation. | |
| // If full LSP validation is needed, implement RunbookBuilderExt::validate_with_lsp_impl. |
Comment on lines
+264
to
+265
| let deprecated_inputs = | ||
| [("api_key", "api_token"), ("endpoint_url", "api_url"), ("rpc_endpoint", "rpc_url")]; |
There was a problem hiding this comment.
[nitpick] The deprecated input mappings are hardcoded in the validation rule. Consider moving these to a configuration file or making them configurable to allow projects to define their own deprecated inputs without code changes.
Contributor
Author
There was a problem hiding this comment.
I agree that making the deprecated inputs configurable would be valuable. However, I felt that adding a full configuration system for custom linting rules would be out of scope for this PR, which is already quite large and focused on introducing the coupled doctor command and LSP functionality.
The current hardcoded deprecated mappings serve as a proof-of-concept for how validation rules work in the doctor command. Making these configurable would involve:
- Designing a configuration schema for custom lint rules
- Adding configuration file loading and parsing
- Creating a rule registration system
- Potentially supporting different rule severity levels and auto-fix capabilities
I think this would be better addressed as follow-on work in a dedicated PR that introduces the infrastructure for customizable linters. That way we can properly design the configuration system and ensure it's extensible for other types of custom rules beyond just deprecated inputs.
cds-amal
added a commit
to cds-amal/txtx
that referenced
this pull request
Sep 20, 2025
Address: - solana-foundation#333 (comment) - solana-foundation#333 (comment) - Rename constructors to be more explicit: - new() now takes 3 params for limited validation (no addon specs) - new_with_addons() takes 4 params for full validation with addon specs
cds-amal
added a commit
to cds-amal/txtx
that referenced
this pull request
Sep 20, 2025
Resolves: - solana-foundation#333 (comment) The comment mentioned 'Deep addon configuration validation' but the module doesn't implement any addon configuration validation.
cds-amal
force-pushed
the
feat/lsp-doctor-pr
branch
from
a6042e4 to
ed8b294
Compare
Contributor
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 113 out of 174 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (1)
crates/txtx-core/src/validation/hcl_validator.rs:1
- The condition
if error.line > 0may not be correct for zero-based line numbering systems. If line numbers are 1-based (as suggested by span_to_position starting at line 1), then line 0 would be invalid. However, if they're 0-based, line 0 would be valid. This inconsistency could cause the first line to be reported as having no position.
//! HCL-based validation for the doctor command using hcl-edit
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| //! | ||
| //! ## Known Limitations | ||
| //! | ||
| //! 1. Circular dependency detection between actions is not implemented |
There was a problem hiding this comment.
This limitation comment should include guidance on when circular dependency detection will be implemented or alternative approaches users should take. Consider adding a TODO with issue reference or timeline.
Suggested change
| //! 1. Circular dependency detection between actions is not implemented | |
| //! 1. Circular dependency detection between actions is not implemented. | |
| //! TODO: See https://github.com/txtx-dev/txtx/issues/123 for progress on this feature. | |
| //! In the meantime, users should manually review their runbooks for circular dependencies or use external static analysis tools. |
| // LSP validation requires the RunbookBuilderExt trait to be implemented | ||
| // by the test crate that has access to txtx-cli | ||
| // For now, we provide a simple fallback that uses HCL validation | ||
| eprintln!("INFO: Using basic HCL validation for LSP mode. Implement RunbookBuilderExt::validate_with_lsp_impl for full LSP validation."); |
There was a problem hiding this comment.
Using eprintln! for informational messages in library code is not ideal. Consider using a proper logging framework or returning this information as part of the result structure so callers can decide how to handle it.
cds-amal
added a commit
to cds-amal/txtx
that referenced
this pull request
Sep 20, 2025
Resolves: - solana-foundation#333 (comment) Make validation capabilities explicit at compile time by using separate types instead of constructor variants. This prevents confusion about what can be validated and follows Rust's principle of making invalid states unrepresentable.
cds-amal
added a commit
to cds-amal/txtx
that referenced
this pull request
Sep 20, 2025
Resolves: - solana-foundation#333 (comment) Document intent explaining that get_content() and get_files() methods are intentionally preserved for future From/Into trait implementations that will convert RunbookBuilder into test harness inputs.
cds-amal
force-pushed
the
feat/lsp-doctor-pr
branch
from
c374e6f to
ce97359
Compare
Contributor
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 104 out of 182 changed files in this pull request and generated 6 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| use std::borrow::Cow; | ||
| use std::collections::{HashMap, HashSet, VecDeque}; | ||
| use std::marker::PhantomData; |
There was a problem hiding this comment.
[nitpick] PhantomData is imported but never used in this file. Consider removing this unused import to keep the code clean.
Suggested change
| use std::marker::PhantomData; |
Comment on lines
+867
to
+879
| pub struct BasicHclValidator<'a> { | ||
| result: &'a mut ValidationResult, | ||
| file_path: &'a str, | ||
| source: &'a str, | ||
| } | ||
|
|
||
| /// HCL validator with addon command specifications for parameter validation. | ||
| pub struct FullHclValidator<'a> { | ||
| result: &'a mut ValidationResult, | ||
| file_path: &'a str, | ||
| source: &'a str, | ||
| addon_specs: HashMap<String, Vec<(String, CommandSpecification)>>, | ||
| } |
There was a problem hiding this comment.
[nitpick] These two validator structs have nearly identical fields except for addon_specs. Consider using a single generic validator struct with an optional addon_specs parameter to reduce code duplication.
Comment on lines
+554
to
+561
| fn report_cycle_error(&mut self, dependency_type: DependencyType, cycle: Vec<String>) { | ||
| // Get positions for all items in the cycle (excluding the duplicate last element) | ||
| let cycle_len = cycle.len(); | ||
| let unique_cycle_items = if cycle_len > 0 && cycle.first() == cycle.last() { | ||
| &cycle[..cycle_len - 1] // Exclude the duplicate last element | ||
| } else { | ||
| &cycle[..] | ||
| }; |
There was a problem hiding this comment.
[nitpick] Consider extracting the cycle deduplication logic into a separate helper function since this pattern might be reused elsewhere. This would make the intent clearer and improve testability.
Comment on lines
+140
to
+159
| // TODO: This is a stopgap solution until we implement a proper compiler pipeline. | ||
| // | ||
| // Current limitation: txtx-core cannot directly depend on addon implementations | ||
| // (evm, bitcoin, svm, etc.) due to: | ||
| // - Heavy dependencies that would bloat core | ||
| // - WASM compatibility requirements | ||
| // - Optional addon features | ||
| // - Circular dependency concerns | ||
| // | ||
| // Current workaround: Two validation paths exist: | ||
| // 1. Simple validation (here) - returns empty specs, limited validation | ||
| // 2. Full validation (CLI/LSP) - passes in actual addon specs | ||
| // | ||
| // Future solution: A proper compiler pipeline with phases: | ||
| // Parse → Resolve (load addons) → Type Check → Optimize → Codegen | ||
| // The resolver phase would load addon specs based on addon declarations | ||
| // in the runbook, making them available for all subsequent phases. | ||
| // This would eliminate the architectural split between validation paths. | ||
| // | ||
| // For now, return empty map - actual implementation would use addon_registry |
There was a problem hiding this comment.
[nitpick] This is excellent documentation of a known architectural limitation. Consider creating a GitHub issue to track the implementation of the proper compiler pipeline mentioned in the TODO, so this technical debt doesn't get forgotten.
| }) | ||
| } | ||
|
|
||
| fn is_inherited_property(name: &str) -> bool { |
There was a problem hiding this comment.
[nitpick] Consider making this function public or moving it to a shared module if other parts of the codebase need to identify inherited properties. This logic might be useful elsewhere in the validation pipeline.
Suggested change
| fn is_inherited_property(name: &str) -> bool { | |
| pub fn is_inherited_property(name: &str) -> bool { |
Comment on lines
+232
to
+245
| ) { | ||
| // Look for "Reference to undefined signer" errors | ||
| for error in &validation_result.errors { | ||
| if error.message.starts_with("Reference to undefined signer") { | ||
| // Extract signer name | ||
| if let Some(signer_name) = error.message.split('\'').nth(1) { | ||
| // Map signer.foo to input.foo_eoa or similar | ||
| // This is a simplified mapping - real implementation would need | ||
| // to understand the actual signer-to-input mapping rules | ||
| let input_name = if signer_name == "operator" { | ||
| "operator_eoa".to_string() | ||
| } else { | ||
| format!("{}_address", signer_name) | ||
| }; |
There was a problem hiding this comment.
[nitpick] This hardcoded signer-to-input mapping is marked as simplified. Consider defining a configurable mapping or using a more systematic approach to determine the relationship between signers and their corresponding input variables.
Suggested change
| ) { | |
| // Look for "Reference to undefined signer" errors | |
| for error in &validation_result.errors { | |
| if error.message.starts_with("Reference to undefined signer") { | |
| // Extract signer name | |
| if let Some(signer_name) = error.message.split('\'').nth(1) { | |
| // Map signer.foo to input.foo_eoa or similar | |
| // This is a simplified mapping - real implementation would need | |
| // to understand the actual signer-to-input mapping rules | |
| let input_name = if signer_name == "operator" { | |
| "operator_eoa".to_string() | |
| } else { | |
| format!("{}_address", signer_name) | |
| }; | |
| // Accept a signer-to-input mapping as an argument | |
| pub fn add_undefined_signer_variables( | |
| variables: &mut HashMap<String, RunbookVariable>, | |
| validation_result: &ValidationResult, | |
| manifest: &WorkspaceManifest, | |
| environment: &HashMap<String, String>, | |
| cli_inputs: &HashMap<String, String>, | |
| signer_to_input_map: &HashMap<String, String>, | |
| ) { | |
| // Look for "Reference to undefined signer" errors | |
| for error in &validation_result.errors { | |
| if error.message.starts_with("Reference to undefined signer") { | |
| // Extract signer name | |
| if let Some(signer_name) = error.message.split('\'').nth(1) { | |
| // Use the mapping to resolve input variable name | |
| let input_name = signer_to_input_map | |
| .get(signer_name) | |
| .cloned() | |
| .unwrap_or_else(|| format!("{}_address", signer_name)); |
cds-amal
force-pushed
the
feat/lsp-doctor-pr
branch
3 times, most recently
from
eb8cbf8 to
c36ecb1
Compare
## Architecture Introduce a shared validation layer in txtx-core that serves as the foundation for both CLI linting and LSP real-time diagnostics. The architecture consists of: - **ValidationContext**: Central state management for all validation operations, reducing parameter passing through the validation pipeline - **HCL Validator**: Two-tier system (BasicHclValidator and FullHclValidator) that performs AST traversal and structural validation - **Runbook Collector**: Extracts domain-specific items (inputs, variables, actions, signers) from runbook files for analysis - **Rule System**: Extensible linter rule infrastructure with RuleIdentifier supporting both core and addon-scoped rules ## Changes Add validation module (txtx-core/src/validation/): - context.rs: ValidationContext with builder pattern for flexible configuration - validator.rs: High-level validate_runbook API with ValidatorConfig - hcl_validator/: AST visitor pattern with block processors and dependency graph - hcl_diagnostics.rs: Convert HCL parse errors to user-friendly diagnostics - linter_rules.rs: Core lint rules (cli-override, input-naming, sensitive-data) - manifest_validator.rs: Validate inputs against manifest environments - rule_id.rs: Typed rule identifiers with addon scope support - types.rs: Shared validation types (errors, warnings, suggestions) - file_boundary.rs: Track line mappings for multi-file runbooks Add runbook analysis (txtx-core/src/runbook/): - collector.rs: Extract RunbookItems from HCL AST via visitor pattern - variables.rs: Track variable definitions, references, and resolution Add C4 architecture annotations to core validation components: - ValidationContext: @c4-component with state management responsibilities - HCL Validator: @c4-component with two-phase validation responsibilities - Manifest Validator: @c4-component with input validation responsibilities - FileBoundaryMapper: @c4-component documenting normalization strategy pattern ## Context This foundational layer enables static analysis of txtx runbooks before execution. The shared validation context eliminates duplicate logic between CLI and LSP, while the collector enables features like "find all references", unused variable detection, flow validation with related locations, file boundary mapping for accurate multi-file error locations, and runbook-scoped references for proper LSP scoping. The rule system is designed for extensibility, allowing addons to register custom validation rules. C4 annotations document the validation architecture for auto-generation of component diagrams, establishing the foundation for architecture-as-code documentation strategy.
## Architecture Introduce txtx-test-utils as a centralized testing toolkit that consolidates validation and execution testing. The module provides two main testing paths: - **Validation Testing**: Fluent RunbookBuilder API with SimpleValidator for static analysis testing without execution overhead - **Execution Testing**: TestHarness for full runbook execution with mocked blockchain interactions (moved from txtx-core) The validation layer supports two modes: HCL-only validation for syntax and basic semantics, and full manifest validation for environment and input checking. ## Changes Add txtx-test-utils crate with: - builders/runbook_builder.rs: Fluent API for programmatic runbook construction - builders/runbook_builder_enhanced.rs: Extended builder with additional helpers - builders/parser.rs: HCL parsing utilities for test content - simple_validator.rs: Lightweight validation wrapper using ValidationContext - assertions/mod.rs: Test assertion macros (assert_error, assert_validation_error) - addon_registry.rs: Addon specification extraction for tests - test_harness.rs: Mock-based execution testing (moved from core) - README.md: Comprehensive documentation with usage patterns and limitations - examples/enhanced_builder_example.rs: Reference implementation ## Context This consolidation reduces test boilerplate across the codebase and provides consistent patterns for testing both validation rules and execution flows. The builder API enables readable, maintainable tests while the dual-mode validation allows testing at different levels of strictness. The clear documentation of limitations helps developers choose between unit tests with RunbookBuilder and integration tests with the full lint pipeline. These utilities support testing of flow validation with related locations, multi-file runbook validation with file boundary mapping, LSP workspace diagnostics, and runbook-scoped reference resolution.
## Synopsis ``` txtx lint [RUNBOOK] [OPTIONS] txtx lint RUNBOOK --gen-cli [--manifest-path PATH] [--env ENV] txtx lint RUNBOOK --gen-cli-full [--manifest-path PATH] [--env ENV] txtx lint [RUNBOOK] [--manifest-path PATH] [--env ENV] [--input KEY=VALUE...] txtx lint [RUNBOOK] [--format stylish|compact|json|quickfix] txtx lint --init ``` ## Architecture Implement `txtx lint` command that performs static analysis of runbooks and manifests using the validation infrastructure from txtx-core. The architecture consists of: - **Linter Engine** (validator.rs): Orchestrates validation using ValidationContext from core and coordinates rule execution - **Workspace Analyzer** (workspace.rs): Discovers txtx.yml manifests by searching upward from current directory and resolves runbook files - **Rule System** (rules.rs): Function-based validation rules with typed CliRuleId for errors/warnings/suggestions - **Formatters** (formatter.rs): Multiple output formats (stylish, compact, json, quickfix) for IDE/CI integration - **Configuration** (config.rs): Centralized LinterConfig with manifest path, environment selection, and CLI input overrides ## Changes Add lint command (txtx-cli/src/cli/lint/): - mod.rs: Main entry point with run_lint orchestration and --gen-cli support Add linter module (txtx-cli/src/cli/linter/): - validator.rs: Linter struct with validate_content and IntoManifest trait - workspace.rs: WorkspaceAnalyzer with upward manifest discovery - rules.rs: ValidationContext, ValidationIssue types, and rule implementations (input-defined, naming-convention, cli-override, sensitive-data) - rule_id.rs: CliRuleId enum for typed rule identification - formatter.rs: Format enum with stylish/compact/json/quickfix outputs - config.rs: LinterConfig for configuring validation runs - README.md: Architecture documentation and usage examples Add shared utilities: - common/addon_registry.rs: Extract addon specifications for validation Add C4 architecture annotations to linter components: - Linter Engine: @c4-component orchestrating validation pipeline - WorkspaceAnalyzer: @c4-component for manifest discovery and runbook resolution - Documents normalization strategy (multi-file → single-file) in annotations - Relationships: WorkspaceAnalyzer → Linter Engine → ValidationContext ## Testing Include 24 unit tests covering: - Manifest discovery (upward search, git root boundary, explicit paths) - Runbook resolution (direct paths, standard locations, not found cases) - Validation engine (error detection, manifest context integration) - Circular dependency detection (2-way, 3-way, false positive prevention) - Rule system (CLI rule ID display and identification) ## Context The lint command provides pre-execution validation similar to TypeScript's tsc, catching configuration errors before runbook execution. The workspace analyzer enables project-level linting without explicit manifest paths, while the formatter abstraction supports both human (stylish) and machine (json/quickfix) outputs for IDE integration. The --gen-cli flag generates CLI commands from runbook inputs, bridging declarative runbooks with imperative shell workflows. The workspace analyzer supports multi-file runbook validation by combining runbook sources and tracking file boundaries for accurate error reporting across flow definitions, variable references, and related locations.
## Synopsis ``` txtx lsp ``` Starts the Language Server Protocol server for IDE integration. Communicates over stdin/stdout using the LSP protocol. ## Architecture Implement LSP server for txtx runbooks providing IDE features through a handler-based architecture with AST-powered reference tracking and intelligent state management: - **Handler System**: Request handlers for completion, definition, hover, references, rename, and diagnostics with shared workspace state - **AST-Based References** (hcl_ast.rs): Uses hcl-edit parser (same as runtime/linter) for accurate reference extraction with strict and lenient modes - **Workspace State** (workspace/state.rs): Thread-safe state with validation caching, dependency tracking, and environment resolution - **State Machine** (workspace/state_machine.rs): Explicit workspace states (Ready, Validating, etc.) with audit trail for debugging - **Smart Validation**: Content hashing prevents redundant validation; dependency graph enables cascade validation through transitive dependencies (A→B→C) - **Multi-Environment Support**: Cross-file operations work across environment-specific files (e.g., signers.sepolia.tx, signers.mainnet.tx) - **Multi-File Runbooks**: Workspace diagnostics validate entire runbooks with file boundary mapping for accurate error locations across flow definitions - **Runbook-Scoped References**: References and rename operations properly scoped to runbook boundaries (variables, flows, actions, outputs) vs workspace-wide (inputs, signers) ## Changes Add LSP implementation (txtx-cli/src/cli/lsp/): - mod.rs: Main server loop with request routing - async_handler.rs: Async request handling with caching - hcl_ast.rs: AST-based reference extraction with visitor pattern Add handlers (handlers/): - completion.rs, definition.rs, hover.rs, references.rs, rename.rs - diagnostics.rs: Real-time diagnostics with multi-file runbook support - document_sync.rs, workspace.rs - environment_resolver.rs, workspace_discovery.rs, debug_dump.rs Add workspace state (workspace/): - state.rs: WorkspaceState with validation cache and dependency graph - state_machine.rs: MachineState with 50-transition audit trail - validation_state.rs: Per-document validation status tracking - dependency_graph.rs: Transitive dependency resolution with cycle detection - dependency_extractor.rs, documents.rs, manifests.rs, manifest_converter.rs Add validation integration (validation/): - adapter.rs, converter.rs, hcl_converter.rs: Adapt linter for LSP diagnostics Add multi-file support: - diagnostics_multi_file.rs: Workspace diagnostics for entire runbooks - multi_file.rs: Runbook expansion and file boundary mapping - Related locations in diagnostics for flow validation errors Add utilities: - utils/: environment.rs, file_scanner.rs - diagnostics.rs, diagnostics_hcl_integrated.rs - linter_adapter.rs, functions.rs Remove deprecated txtx-lsp crate (5039 lines) Add documentation (2158 lines): - docs/lsp-state-management.md: State architecture and implementation status - docs/lsp-sequence-diagram.md, docs/lsp-use-case-diagram.md - README.md, ASYNC_GUIDE.md ## Features - Code completion for inputs, actions, and signers - Go-to-definition across multiple files and environments - Find all references with runbook-scoped filtering - Rename refactoring with runbook scope awareness (updates references, preserves prefixes) - Real-time diagnostics using linter rules with multi-file runbook support - Flow validation with related locations showing missing inputs across files - File boundary mapping for accurate error locations in multi-file runbooks - Hover documentation for functions and actions - Environment-aware manifest resolution - Automatic cascade validation when dependencies change ## Testing Include 200+ unit and integration tests covering: - State machine transitions and audit trail - Cascade validation through dependency chains - Multi-file and multi-environment references/rename - Runbook-scoped reference filtering (variables, flows, actions vs inputs, signers) - HCL diagnostics integration - Multi-file runbook workspace diagnostics - Flow validation with related locations - Environment switching and manifest resolution - Dependency extraction and graph operations - Mock editor for end-to-end testing ## Context Provides IDE features for txtx runbooks with real-time validation feedback. AST-based reference extraction ensures consistency with runtime/linter and eliminates false positives in strings/comments. State machine enables observability for debugging. Multi-environment support handles enterprise workflows with environment-specific configurations. Multi-file runbook support enables proper flow validation with accurate error locations and related location tracking. Runbook-scoped references prevent cross-runbook pollution while maintaining workspace-wide visibility for manifest inputs. Integrates with CLI linter to share validation logic.
Add test fixtures and integration tests using the RunbookBuilder API from txtx-test-utils. Provides both demonstrative fixtures for users and programmatic test coverage for linter and LSP functionality. Add linter demonstration fixtures (addons/evm/fixtures/linter_demo/): - README.md: Documentation for linter command with usage examples - runbooks/correct_transfer.tx: Example showing proper action output usage - runbooks/problematic_transfer.tx: Common mistakes for demonstration - txtx.yml: Test manifest with environment configuration - linter_demo.sh, linter_with_links_demo.sh: Interactive demo scripts - test_linter.sh: Automated test script for fixture validation Add linter integration tests (crates/txtx-cli/tests/linter_tests_builder.rs): - builder-based test cases covering: - Circular variable detection (simple, chain, self-reference) - Input validation against manifest environments - Action output field validation - Variable and action reference resolution - Naming convention checks - CLI input override warnings - Multi-file runbook validation with flow definitions - Flow input validation across runbook files Add LSP integration tests (crates/txtx-cli/tests/lsp_tests_builder.rs): - LSP feature tests using RunbookBuilder: - Function hover information - Action type hover - Diagnostic generation for validation errors - Multi-file runbook diagnostics with file boundary mapping - Test helper macros (assert_has_diagnostic, assert_has_error) Add addon examples: - addons/evm/examples/list_addon_functions.rs: Utility to list available EVM functions Include 49 test functions covering: - Circular dependency detection and error reporting - Multi-environment input validation - Action output field existence checking - Variable scoping and resolution - Self-referential variable detection - Flow validation with related locations across multiple files - File boundary mapping for accurate multi-file error locations - LSP hover information for functions and actions - Runbook-scoped reference resolution These fixtures serve dual purposes: user-facing demonstration of linter capabilities and programmatic test coverage. The linter_demo fixtures provide runnable examples for documentation and training, while the builder-based tests enable maintainable integration testing. The RunbookBuilder API allows concise test creation without manual HCL string construction. Multi-file test coverage validates flow definitions, file boundary mapping, and related location tracking.
## Architecture Provide IDE integration for txtx through VSCode extension and Neovim plugin, both communicating with the txtx LSP server for language features: **VSCode Extension**: - LSP client connecting to `txtx lsp` command - LspPathResolver with priority: config → env → project binaries → workspace → system - Environment management with workspace-scoped persistence - Status bar indicators for LSP state and selected environment - Commands for environment selection and LSP restart **Neovim Plugin** (nvim-txtx): - Tree-sitter grammar for .tx syntax highlighting (5971 lines generated parser) - LSP client with Lspsaga integration for enhanced UI - Workspace discovery with automatic txtx.yml manifest parsing - Cross-file navigation (manifest → runbooks, runbooks → manifest definitions) - Environment switching through Lua commands ## Changes Add VSCode extension (vscode-extension/): - src/extension.ts: Main extension with LSP client lifecycle management (498 lines) - src/extension-refactored.ts: Alternative implementation (471 lines) - package.json: Extension manifest with commands and configuration - syntaxes/txtx.tmLanguage.json: TextMate grammar for syntax highlighting - language-configuration.json: Bracket matching and auto-closing pairs - README.md: Setup instructions and development guide Add VSCode tests (test/): - suite/workspace-state.test.ts: Workspace state management (554 lines) - suite/opening-order.test.ts: File opening order handling (401 lines) - suite/file-order.test.ts: File order scenarios (306 lines) - suite/environment-persistence.test.ts: Environment persistence (238 lines) - suite/definition.test.ts: Go-to-definition tests (186 lines) - suite/environment-timing.test.ts: Environment timing (161 lines) - unit/lsp-client.test.ts: LSP client unit tests (258 lines) - suite/basic.test.ts, hover.test.ts, lsp.test.ts, extension_test.ts Add Neovim plugin (vscode-extension/nvim-txtx/): - grammar.js: Tree-sitter grammar definition for .tx syntax - src/parser.c: Generated Tree-sitter parser (5971 lines) - src/grammar.json, src/node-types.json: Parser metadata - queries/highlights.scm: Syntax highlighting queries Add Neovim Lua modules (lua/txtx/): - init.lua: Plugin setup and initialization - lsp.lua: LSP client configuration (344 lines) - commands.lua: User commands for workspace operations (371 lines) - navigation.lua: Cross-file navigation (273 lines) - manifest.lua: Manifest parsing and environment management (232 lines) - workspace.lua: Workspace discovery (156 lines) - treesitter.lua: Tree-sitter integration (101 lines) - utils.lua: Utility functions (129 lines) Add build infrastructure: - scripts/build.sh: Compile Tree-sitter parser - bindings/: Node.js and Rust bindings for parser - README.md: Installation and usage guide (249 lines) ## Features VSCode: - Syntax highlighting, hover, completion, diagnostics, go-to-definition - Environment selector with workspace persistence - LSP auto-discovery from multiple locations - Status bar with server state and environment display - Multi-file runbook diagnostics with file boundary mapping - Runbook-scoped references and rename Neovim: - Tree-sitter syntax highlighting for .tx files - LSP integration with optional Lspsaga enhanced UI - Navigate from manifest locations to runbook files - Navigate from input/env references to definitions - Cross-file rename for manifest and runbook variables - Environment switching through commands - Flow validation with related locations across files - Runbook-scoped reference filtering ## Testing Include 64 test cases covering: - Workspace state management across file operations - File opening order and initialization timing - Environment persistence and switching - LSP client lifecycle and error handling - Go-to-definition across file types - Multi-file diagnostics with accurate error locations - Headless testing support ## Context Provides editor integration for txtx development in both VSCode and Neovim. The VSCode extension handles LSP binary discovery through multiple fallback mechanisms for both development and production use. The Neovim plugin uses Tree-sitter for accurate syntax highlighting and provides workspace-aware navigation. Both implementations leverage the shared txtx LSP server for language features, ensuring consistent behavior across editors. Multi-file runbook support enables proper flow validation with accurate error locations and related location tracking. Runbook-scoped references prevent cross-runbook pollution while maintaining workspace-wide visibility for manifest inputs.
…tions Add c4-generator crate to extract C4 architecture annotations from Rust source code and generate Structurizr DSL. ## Implementation - Scan all .rs files in crates/ for @c4-* annotations in doc comments - Extract component metadata: name, container, description, technology - Extract relationships: @c4-uses, @c4-relationship - Extract responsibilities: @c4-responsibility - Generate workspace-generated.dsl with Structurizr DSL format - Proper ID sanitization for view keys (a-zA-Z0-9_-) - Group components by container with component views ## Rationale Bash script approach would be too complex due to: - Nested process substitution causing hangs with heredocs - Subshell scope issues preventing associative array updates - Fragile sed/grep patterns for extracting multi-line annotations - sed inconsistencies across platforms (BSD sed on macOS vs GNU sed on Linux) - Unreliable handling of optional annotation fields - Platform-specific shell differences (bash vs zsh) Rust provides: - Type-safe data structures (Component, HashMap) - Reliable regex parsing with capture groups - Proper error handling vs silent failures - Cross-platform compatibility - Native binary performance
## Architecture Add comprehensive build infrastructure and developer tooling to streamline development workflow and handle build constraints: **Build System**: - Cargo aliases for building without supervisor UI (requires privileged tooling) - Separate unit and integration test targets for CLI, linter, and LSP - Just recipes providing ergonomic task runner interface - Development RUSTFLAGS to suppress common warnings during iteration **VSCode Integration**: - Launch configurations for debugging CLI, LSP, and tests - Task definitions for common build operations - Settings for Rust development with recommended extensions **Performance Monitoring**: - Criterion benchmarks for LSP request handling **Architecture Documentation**: - C4 diagram generation from code annotations - Structurizr Lite integration for viewing diagrams - Module dependency graph generation ## Changes Add Cargo configuration (.cargo/config.toml): - build-cli, build-cli-release: Build without supervisor UI - test-cli-unit, test-cli-int: Separate unit and integration test targets - test-cli-unit-linter, test-cli-int-linter: Linter-specific tests - test-cli-unit-lsp, test-cli-int-lsp: LSP-specific tests - test-hcl-diagnostics, test-lsp-validation: Focused validation tests - tokio_unstable rustflag for async runtime features Add justfile with 30 recipes (195 lines): **Build recipes**: - build, build-release: Build CLI with/without optimizations - check: Fast syntax/type checking without codegen - fmt: Format code with rustfmt - clean: Remove build artifacts **Test recipes**: - cli-unit, cli-int: CLI unit and integration tests - lint-unit, lint-int: Linter-specific unit and integration tests - lsp-unit, lsp-int: LSP-specific unit and integration tests - test <name>: Run specific test by name - test-verbose: Run tests with full output - watch: Auto-run tests on file changes **Coverage recipes**: - coverage: Generate HTML coverage report - coverage-ci: Generate JSON coverage for CI/CD pipelines - coverage-test <name>: Coverage for specific test module **Analysis recipes**: - complexity-high: Find functions with high cyclomatic complexity - complexity-file <file>: Analyze complexity of specific file **Documentation recipes**: - doc: Generate and open API documentation - doc-all: Generate docs for all packages including dependencies **Architecture recipes**: - arch-c4: Generate C4 diagrams from @c4-* code annotations - arch-view: Generate and view diagrams with Structurizr Lite (podman/docker) - arch-modules: Generate module dependency graph with cargo-modules Add architecture tooling: - scripts/generate-c4-from-code.sh: Bash reference implementation (deprecated) - Just recipes use Rust c4-generator for reliable cross-platform generation - Auto-detect podman/docker with :Z flag for SELinux compatibility - Output to workspace-generated.dsl (gitignored) Add VSCode configuration: - .vscode/launch.json: Debug configurations for CLI, LSP, tests - .vscode/tasks.json: Build task definitions - .vscode/settings.json: Rust-analyzer and editor settings - .vscode/settings.json.example: Template for team settings Add benchmarks: - benches/lsp_performance.rs: Criterion benchmarks for LSP completion and hover Update build configuration: - crates/txtx-cli/Cargo.toml: Add criterion dev-dependency, configure benches - crates/txtx-core/Cargo.toml: Update dependencies - Cargo.toml: Add c4-generator to workspace members - Cargo.lock: Update dependency resolution (257 line changes) Update gitignore: - .structurizr/: Structurizr Lite generated files - workspace.json: Structurizr workspace metadata - workspace-generated.dsl: Auto-generated from code annotations - Coverage reports, VSCode files, build artifacts Add development files: - bacon.toml: Configure bacon file watcher Update CLI structure: - src/cli/mod.rs: Add ls-envs command, refactor module organization (123 line changes) - src/cli/common/mod.rs: Add common utilities module - src/cli/docs/mod.rs, src/cli/runbooks/mod.rs: Minor updates ## Context The supervisor UI build requires npm/node and specific frontend tooling not available to all developers. The cargo aliases and justfile recipes enable CLI-only builds with --no-default-features. The granular test aliases allow running specific test suites (unit vs integration, linter vs LSP) for faster iteration during development of multi-file validation, flow tracking, and runbook-scoped reference features. Architecture tooling supports hybrid documentation strategy: - arch-c4 generates workspace-generated.dsl from @c4-* annotations - arch-view launches Structurizr Lite for interactive diagram viewing - Rust c4-generator replaces bash script for reliable cross-platform support - Bash script kept as reference but deprecated in favor of Rust utility
Add user documentation (docs/user/): - linter-guide.md: Complete guide with motivation, 4 validation rules, examples - linter-configuration.md: Configuration options and rule customization - lsp-guide.md: IDE integration guide with workflow benefits Add developer documentation (docs/developer/): - DEVELOPER.md: Development workflows, testing, build configuration - TESTING_GUIDE.md: Comprehensive testing strategies (495 lines) - TESTING_CONVENTIONS.md: Test organization and best practices - VALIDATION_ARCHITECTURE.md: Deep dive into validation system (393 lines) Add architecture documentation (docs/architecture/): - linter/architecture.md: Complete validation pipeline with Mermaid diagrams - linter/workspace.dsl: Hand-written Structurizr C4 model with dynamic views - linter/ARCHITECTURE_DOCS.md: Hybrid documentation strategy explanation - linter/README.md: Architecture viewing and generation guide - lsp/async-implementation.md: Async handlers with ~50% latency improvements - performance-improvements.md: Metrics and benchmarks Add Architecture Decision Records (docs/adr/): - 001-pr-architectural-premise.md: Separation of concerns and modularity - 002-eliminate-lsp-server-crate.md: Consolidate LSP into txtx-cli - 003-capture-everything-filter-later-pattern.md: Validation approach - 004-visitor-strategy-pattern-with-readonly-iterators.md: AST traversal Add feature documentation (docs/): - lint-lsp-features.md: Reference and rename scoping rules - Documents runbook-scoped references (variables, flows, actions, outputs) - Documents workspace-scoped references (inputs, signers) - Explains multi-file validation behavior with accurate error locations Add future planning (docs/internal/): - linter-plugin-system.md: Extensible validation system design - flow-field-navigation-plan.md: Planned flow field navigation feature Add documentation hub (docs/README.md): - Organized by audience: users, developers, architecture, internal - Clear navigation with descriptions Update root README.md: - Add features section highlighting validation, IDE integration, testing - Link to documentation structure Add .gitattributes: - Configure syntax highlighting for .tx files in GitHub Implement hybrid approach to architecture documentation: **Hand-written (workspace.dsl)**: - C4 model with system context, containers, components - Dynamic views showing validation flows (single-file, multi-file, flow validation) - User interactions and presentation layer - ~182 lines with rich narrative **Auto-generated (workspace-generated.dsl)**: - Components extracted from @c4-* code annotations - Kept in sync via `just arch-c4` - Gitignored as build artifact **Rationale**: - Hand-written captures runtime behavior and architectural intent - Auto-generated keeps component inventory synced with code - Structurizr DSL enables interactive viewing with `just arch-view` The documentation covers implemented features including multi-file flow validation with related locations, file boundary mapping for accurate error locations in multi-file runbooks, and runbook-scoped reference resolution in the LSP.
cds-amal
force-pushed
the
feat/lsp-doctor-pr
branch
from
c36ecb1 to
644767b
Compare
cds-amal
changed the title
Contributor
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 92 out of 234 changed files in this pull request and generated 4 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| /// | ||
| /// Removes the value from the set, and removes the key entirely if the | ||
| /// set becomes empty. This prevents memory leaks from empty collections. | ||
| fn remove_from_map<K, V>(map: &mut HashMap<K, HashSet<V>>, key: &K, value: &V) |
There was a problem hiding this comment.
[nitpick] Consider making this helper function public or moving it to a utility module since it's a generally useful operation for HashMap<K, HashSet> cleanup that might be needed elsewhere.
Comment on lines
+74
to
+80
| let input_re = INPUT_REGEX.get_or_init(|| Regex::new(r"\binput\.\w+").unwrap()); | ||
| let output_re = OUTPUT_REGEX.get_or_init(|| Regex::new(r"\boutput\.(\w+)").unwrap()); | ||
| let variable_ref_re = | ||
| VARIABLE_REF_REGEX.get_or_init(|| Regex::new(r"\bvariable\.(\w+)").unwrap()); | ||
| let action_def_re = ACTION_DEF_REGEX.get_or_init(|| Regex::new(r#"action\s+"(\w+)""#).unwrap()); | ||
| let variable_def_re = | ||
| VARIABLE_DEF_REGEX.get_or_init(|| Regex::new(r#"variable\s+"(\w+)""#).unwrap()); |
There was a problem hiding this comment.
The regex patterns use .unwrap() which will panic if the regex is invalid. Consider using lazy_static or const regex compilation to catch these errors at compile time, or handle the error gracefully.
Suggested change
| let input_re = INPUT_REGEX.get_or_init(|| Regex::new(r"\binput\.\w+").unwrap()); | |
| let output_re = OUTPUT_REGEX.get_or_init(|| Regex::new(r"\boutput\.(\w+)").unwrap()); | |
| let variable_ref_re = | |
| VARIABLE_REF_REGEX.get_or_init(|| Regex::new(r"\bvariable\.(\w+)").unwrap()); | |
| let action_def_re = ACTION_DEF_REGEX.get_or_init(|| Regex::new(r#"action\s+"(\w+)""#).unwrap()); | |
| let variable_def_re = | |
| VARIABLE_DEF_REGEX.get_or_init(|| Regex::new(r#"variable\s+"(\w+)""#).unwrap()); | |
| let input_re = INPUT_REGEX.get_or_init(|| Regex::new(r"\binput\.\w+").expect("Failed to compile input regex")); | |
| let output_re = OUTPUT_REGEX.get_or_init(|| Regex::new(r"\boutput\.(\w+)").expect("Failed to compile output regex")); | |
| let variable_ref_re = | |
| VARIABLE_REF_REGEX.get_or_init(|| Regex::new(r"\bvariable\.(\w+)").expect("Failed to compile variable reference regex")); | |
| let action_def_re = ACTION_DEF_REGEX.get_or_init(|| Regex::new(r#"action\s+"(\w+)""#).expect("Failed to compile action definition regex")); | |
| let variable_def_re = | |
| VARIABLE_DEF_REGEX.get_or_init(|| Regex::new(r#"variable\s+"(\w+)""#).expect("Failed to compile variable definition regex")); |
Comment on lines
+112
to
+113
| eprintln!("[DEBUG] get_runbook_name_for_file: checking file_path: {:?}", file_path); | ||
| eprintln!("[DEBUG] Manifest has {} runbooks", manifest.runbooks.len()); |
There was a problem hiding this comment.
Debug print statements should be removed from production code or replaced with proper logging using the log crate to allow runtime control of debug output.
Comment on lines
+7
to
+8
| #[allow(dead_code)] | ||
| pub fn hcl_to_lsp_diagnostic(hcl_diag: &HclDiagnostic, source: &str) -> Diagnostic { |
There was a problem hiding this comment.
[nitpick] Multiple functions are marked with #[allow(dead_code)]. If these are intended for future use, consider adding TODO comments explaining their purpose, or remove them if they're not needed.
cds-amal
changed the title
cds-amal
changed the title
|
@copilot |
MicaiahReid
requested changes
Oct 10, 2025
| cmd.gen_cli, | ||
| cmd.gen_cli_full, | ||
| )?; | ||
| } |
Collaborator
There was a problem hiding this comment.
Suggested change
| } | |
| run_lint( | |
| cmd.runbook, | |
| cmd.manifest_path, | |
| cmd.environment, | |
| cmd.inputs, | |
| cmd.format, | |
| linter_options, | |
| cmd.gen_cli, | |
| cmd.gen_cli_full, | |
| )?; |
Unnecessary clones since we don't need the values after this fn :)
Comment on lines
+438
to
+444
| let linter_options = LinterOptions { | ||
| config_path: cmd.config.clone(), | ||
| disabled_rules: cmd.disable_rules.clone(), | ||
| only_rules: cmd.only_rules.clone(), | ||
| fix: cmd.fix, | ||
| init: cmd.init, | ||
| }; |
Collaborator
There was a problem hiding this comment.
Suggested change
| let linter_options = LinterOptions { | |
| config_path: cmd.config.clone(), | |
| disabled_rules: cmd.disable_rules.clone(), | |
| only_rules: cmd.only_rules.clone(), | |
| fix: cmd.fix, | |
| init: cmd.init, | |
| }; | |
| let linter_options = LinterOptions { | |
| config_path: cmd.config, | |
| disabled_rules: cmd.disable_rules, | |
| only_rules: cmd.only_rules, | |
| fix: cmd.fix, | |
| init: cmd.init, | |
| }; |
Unnecessary clones since we don't need the values after this fn :)
| let result = linter.validate_content( | ||
| &content, | ||
| &location.to_string(), | ||
| self.config.manifest_path.as_ref(), |
Collaborator
There was a problem hiding this comment.
Suggested change
| self.config.manifest_path.as_ref(), | |
| self.manifest.clone(), |
If I'm understanding correctly, the current self.config.manifest_path.as_ref() will read the manifest from the fs again, rather than using the one we have in memory.
Comment on lines
+7
to
+45
| use std::sync::Arc; | ||
| use txtx_addon_network_bitcoin::BitcoinNetworkAddon; | ||
| use txtx_addon_network_evm::EvmNetworkAddon; | ||
| use txtx_addon_network_svm::SvmNetworkAddon; | ||
| use txtx_addon_telegram::TelegramAddon; | ||
| use txtx_core::kit::Addon; | ||
| use txtx_core::std::StdAddon; | ||
|
|
||
| /// Get all available addons as a shared reference | ||
| pub fn get_all_addons() -> Arc<Vec<Box<dyn Addon>>> { | ||
| let addons: Vec<Box<dyn Addon>> = vec![ | ||
| Box::new(StdAddon::new()), | ||
| Box::new(BitcoinNetworkAddon::new()), | ||
| Box::new(EvmNetworkAddon::new()), | ||
| Box::new(SvmNetworkAddon::new()), | ||
| Box::new(TelegramAddon::new()), | ||
| ]; | ||
|
|
||
| // Add optional addons if available | ||
| #[cfg(feature = "ovm")] | ||
| { | ||
| use txtx_addon_network_ovm::OvmNetworkAddon; | ||
| addons.push(Box::new(OvmNetworkAddon::new())); | ||
| } | ||
|
|
||
| #[cfg(feature = "stacks")] | ||
| { | ||
| use txtx_addon_network_stacks::StacksNetworkAddon; | ||
| addons.push(Box::new(StacksNetworkAddon::new())); | ||
| } | ||
|
|
||
| #[cfg(feature = "sp1")] | ||
| { | ||
| use txtx_addon_sp1::Sp1NetworkAddon; | ||
| addons.push(Box::new(Sp1NetworkAddon::new())); | ||
| } | ||
|
|
||
| Arc::new(addons) | ||
| } |
Collaborator
There was a problem hiding this comment.
Suggested change
| use std::sync::Arc; | |
| use txtx_addon_network_bitcoin::BitcoinNetworkAddon; | |
| use txtx_addon_network_evm::EvmNetworkAddon; | |
| use txtx_addon_network_svm::SvmNetworkAddon; | |
| use txtx_addon_telegram::TelegramAddon; | |
| use txtx_core::kit::Addon; | |
| use txtx_core::std::StdAddon; | |
| /// Get all available addons as a shared reference | |
| pub fn get_all_addons() -> Arc<Vec<Box<dyn Addon>>> { | |
| let addons: Vec<Box<dyn Addon>> = vec![ | |
| Box::new(StdAddon::new()), | |
| Box::new(BitcoinNetworkAddon::new()), | |
| Box::new(EvmNetworkAddon::new()), | |
| Box::new(SvmNetworkAddon::new()), | |
| Box::new(TelegramAddon::new()), | |
| ]; | |
| // Add optional addons if available | |
| #[cfg(feature = "ovm")] | |
| { | |
| use txtx_addon_network_ovm::OvmNetworkAddon; | |
| addons.push(Box::new(OvmNetworkAddon::new())); | |
| } | |
| #[cfg(feature = "stacks")] | |
| { | |
| use txtx_addon_network_stacks::StacksNetworkAddon; | |
| addons.push(Box::new(StacksNetworkAddon::new())); | |
| } | |
| #[cfg(feature = "sp1")] | |
| { | |
| use txtx_addon_sp1::Sp1NetworkAddon; | |
| addons.push(Box::new(Sp1NetworkAddon::new())); | |
| } | |
| Arc::new(addons) | |
| } | |
| use std::sync::Arc; | |
| use txtx_addon_network_bitcoin::BitcoinNetworkAddon; | |
| use txtx_addon_network_evm::EvmNetworkAddon; | |
| use txtx_addon_network_svm::SvmNetworkAddon; | |
| use txtx_addon_telegram::TelegramAddon; | |
| use txtx_core::kit::Addon; | |
| use txtx_core::std::StdAddon; | |
| #[cfg(feature = "ovm")] | |
| use txtx_addon_network_ovm::OvmNetworkAddon; | |
| #[cfg(feature = "stacks")] | |
| use txtx_addon_network_stacks::StacksNetworkAddon; | |
| #[cfg(feature = "sp1")] | |
| use txtx_addon_sp1::Sp1NetworkAddon; | |
| /// Get all available addons as a shared reference | |
| pub fn get_all_addons() -> Arc<Vec<Box<dyn Addon>>> { | |
| let addons: Vec<Box<dyn Addon>> = vec![ | |
| Box::new(StdAddon::new()), | |
| Box::new(BitcoinNetworkAddon::new()), | |
| Box::new(EvmNetworkAddon::new()), | |
| Box::new(SvmNetworkAddon::new()), | |
| Box::new(TelegramAddon::new()), | |
| // Add optional addons if available | |
| #[cfg(feature = "ovm")] | |
| Box::new(OvmNetworkAddon::new()), | |
| #[cfg(feature = "stacks")] | |
| Box::new(StacksNetworkAddon::new()), | |
| #[cfg(feature = "sp1")] | |
| Box::new(Sp1NetworkAddon::new()), | |
| ]; | |
| Arc::new(addons) | |
| } |
The addons weren't mut, so when one of those features is enabled things break.
Comment on lines
+20
to
+23
| /// Create a new empty dependency graph | ||
| pub fn new() -> Self { | ||
| Self::default() | ||
| } |
Collaborator
There was a problem hiding this comment.
Suggested change
| /// Create a new empty dependency graph | |
| pub fn new() -> Self { | |
| Self::default() | |
| } |
Remove cause unused
| use std::fs; | ||
|
|
||
| #[derive(Clone, Copy, Debug)] | ||
| pub enum Format { |
Collaborator
There was a problem hiding this comment.
All of these Formats seem like overkill and additional maintenance, but I'm torn since it's already been written. No action for now, more putting this here to think about it.
| use std::path::PathBuf; | ||
| use txtx_core::validation::ValidationResult; | ||
|
|
||
| #[allow(dead_code)] // May be used in future CLI commands |
Collaborator
There was a problem hiding this comment.
Are these functions taking the future proofing too far?
Collaborator
There was a problem hiding this comment.
I'm not really sure of the usefullness of this README. It's a maintenance burden that is already outdated within this PR (the below documents --format github, which is not valid).
Thoughts on slimming down to what's crucial, or removing?
| let mut result = linter.validate_content( | ||
| &combined_content, | ||
| "multi-file runbook", | ||
| self.config.manifest_path.as_ref(), |
Collaborator
There was a problem hiding this comment.
Suggested change
| self.config.manifest_path.as_ref(), | |
| self.manifest.clone(), |
| /// 1. Using explicitly provided manifest path if available | ||
| /// 2. Searching upward from current directory for txtx.yml | ||
| /// 3. Returning None if no manifest found (will use simple validation) | ||
| fn resolve_manifest(explicit_path: &Option<PathBuf>) -> Result<Option<WorkspaceManifest>, String> { |
Collaborator
There was a problem hiding this comment.
Do you think it makes sense for this logic to exist on the WorkspaceManifest struct?
MicaiahReid
reviewed
Oct 10, 2025
Collaborator
There was a problem hiding this comment.
What's the purpose of this file? Can it be removed?
Contributor
Author
There was a problem hiding this comment.
Test, I wrote to snoop addons.
Contributor
Author
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
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.
🍰 Crème Brûlée Inspector - A Delicate Stack of Linting and Language Services
🎂 Release Name: Crème Brûlée Inspector
Like its namesake dessert, this release brings a satisfying crack of insight to your development experience. The linter torches away imperfections while the LSP provides that smooth, creamy developer experience underneath.
Serving Size: 240 files
Preparation Time: Feature-complete, fully tested, production-ready
Dietary Information: Zero breaking changes (100% additive)
Chef's Special: Real-time validation with a side of async performance
🍽️ Today's Menu
🔍 1. Command-Line Linter (
txtx lint)A discerning sommelier for your code, swirling each runbook to detect notes of undefined inputs and hints of misconfiguration.
New Command (served fresh):
txtx lint runbook.tx txtx lint --manifest txtx.yml --env production txtx lint --format json # friendly processing outputFeatures (hand-selected ingredients):
Files Added (mise en place):
crates/txtx-cli/src/cli/lint/mod.rs- CLI command entry pointcrates/txtx-cli/src/cli/linter/- Linter implementation (7 files)config.rs- Configuration managementformatter.rs- Output formatters (stylish, json, compact, doc)rules.rs- Validation rules (undefined-input, cli-override)validator.rs- Core validation logicworkspace.rs- Multi-file workspace handling💡 2. Language Server Protocol (LSP)
Your personal sous chef, whisking up completions, folding in definitions, and garnishing your IDE with real-time insights.
Features (artfully plated):
Files Added (kitchen equipment):
crates/txtx-cli/src/cli/lsp/- LSP implementation (~100 files)handlers/- LSP request handlers (completion, hover, definition, etc.)workspace/- Workspace state management and dependency trackingvalidation/- Validation adapterstests/- Comprehensive LSP integration testsArchitecture Highlights (secret sauce):
txtx-lspcrate, consolidates intotxtx-cli🧪 3. Test Utilities (
txtx-test-utils)A molecular gastronomy kit for txtx development - precise, repeatable, and delightfully scientific.
Features (laboratory-grade):
RunbookBuilder- Fluent API for constructing test runbooksExample (recipe card):
Files Added:
crates/txtx-test-utils/- New cratesrc/builders/runbook_builder.rs- Fluent test APIsrc/assertions/mod.rs- Test assertion helperssrc/simple_validator.rs- Validation utilities📊 4. Validation Infrastructure (
txtx-core)The foundational roux that binds our validation sauce together - shared between the linter and LSP for consistent flavor.
Features (base ingredients):
ValidationContext- Unified validation stateFiles Added:
crates/txtx-core/src/validation/- Core validationcontext.rs- Central validation statehcl_validator/- HCL parsing and validationmanifest_validator.rs- Environment validationlinter_rules.rs- Rule definitionscrates/txtx-core/src/runbook/- Variable extractioncollector.rs- Traverse runbooks to collect variablesvariables.rs- Variable analysis🎨 5. VSCode Extension + Neovim Plugin
Table service for your favorite editors - bringing the txtx experience directly to your development dining room.
VSCode Extension (fine china):
Neovim Plugin (artisanal flatware):
:TxtxSelectEnv,:TxtxShowState)Files Added:
vscode-extension/- Complete VSCode extensionsrc/extension.ts- Extension entry pointsyntaxes/txtx.tmLanguage.json- TextMate grammartest/suite/- Extension integration testsvscode-extension/nvim-txtx/- Neovim pluginlua/txtx/- Plugin implementationqueries/highlights.scm- Tree-sitter queriesgrammar.js- Tree-sitter grammar🏗️ 6. Development Tooling
The kitchen tools every chef needs - sharp, reliable, and always within reach.
Build Configuration:
.cargo/config.toml- Cargo aliases for testing without supervisor UIjustfile- Development commands and workflowsbacon.toml- Background compilation watcherArchitecture Documentation:
crates/c4-generator/- Extract C4 diagrams from code annotationsscripts/generate-c4-from-code.sh- Generate Structurizr DSL📚 7. Comprehensive Documentation
The cookbook, complete with family recipes, technique guides, and wine pairings.
User Guides (3 files):
docs/user/linter-guide.md- Linter usage and examplesdocs/user/linter-configuration.md- Configuration optionsdocs/user/lsp-guide.md- LSP setup and featuresDeveloper Docs (3 files):
docs/developer/DEVELOPER.md- Development setupdocs/developer/TESTING_GUIDE.md- Testing strategiesdocs/developer/VALIDATION_ARCHITECTURE.md- Validation deep diveArchitecture Docs (13 files):
docs/architecture/README.md- Architecture overviewdocs/architecture/linter/- Linter architecturedocs/architecture/lsp/- LSP architecture (async, state management, sequences)docs/adr/- Architecture Decision Records (4 ADRs)Internal Planning:
docs/internal/linter-plugin-system.md- Future extensibility proposal🎭 The Main Course
Appetizer: Linter Catches Undefined Inputs
Runbook (
deploy.tx):Manifest (
txtx.yml):Linter Output (served with a dollop of helpful context):
$ txtx lint deploy.tx --env production deploy.tx: 3:13 error Undefined input 'undefined_key' undefined-input ✖ 1 error foundMain Dish: LSP Provides Real-Time Feedback
In VSCode (à la carte service):
input.→ Auto-completion shows available inputs from manifestinput.contract→ Shows value from current environmentDessert: Test Utilities Simplify Testing
Before (store-bought):
After (homemade with love):
👨🍳 Kitchen Quality Control
Test Coverage (Health Inspection: ✅ Passed)
Linter Tests (taste-tested):
crates/txtx-cli/tests/linter_tests_builder.rs)LSP Tests (blind taste panel):
Core Validation Tests (USDA certified):
VSCode Extension Tests (Michelin approved):
Running Tests (Kitchen Timer)
Important: All tests use
--no-default-features --features clito avoid building the supervisor UI, which requires npm/node and other build tools only available to maintainers.Test Results (Final Tasting)
🍷 Tasting Notes
Quick Tasting (15 minutes)
For those who want to sample the highlights.
1. Check the linter works:
cd addons/evm/fixtures/linter_demo ./linter_demo.shExpected: See validation errors for undefined inputs, formatted output.
2. Try the test utilities:
3. Review main documentation:
4. Verify tests pass:
Full Degustation Menu (1-2 hours)
For the connoisseur who appreciates every nuance.
1. Linter Implementation (30 min):
crates/txtx-cli/src/cli/linter/directoryrules.rsformatter.rscargo test --test linter_tests_builder2. LSP Implementation (45 min):
crates/txtx-cli/src/cli/lsp/handlers/directoryworkspace/state_machine.rsasync_handler.rscargo test --package txtx-cli --test lsp_tests_builder3. Core Validation (20 min):
crates/txtx-core/src/validation/directoryValidationContextincontext.rshcl_validator/visitor.rscargo test --package txtx-core4. Documentation Quality (15 min):
Viewing Architecture Diagrams
The restaurant's blueprint - see how the kitchen really works.
Using Docker (Modern Kitchen)
Then open: http://localhost:8080
Using Podman (Artisanal Kitchen)
Using Just Recipe (Express Service)
# From project root just arch-viewWhat You'll See (The Floor Plan)
🔪 Chef's Secret Techniques
1. Eliminated
txtx-lspCrate (Simplified the Menu)Before: Separate LSP crate with duplicated validation logic
After: LSP integrated into
txtx-cli, shares validation with linterBenefits (efficiency gains):
See: ADR 002: Eliminate LSP Server Crate
2. Async LSP Handlers (Convection Oven Technology)
Performance: ~50% faster response times - like using a convection oven instead of conventional
Implementation (professional equipment):
See: LSP Async Implementation
3. State Machine for Workspace Management (Precision Temperature Control)
States: Uninitialized → Indexing → Ready → Validating → Ready
Benefits (consistent results):
See: LSP State Management
4. Visitor Pattern for HCL Validation (Mise en Place)
Pattern: Capture everything, filter later
Implementation (prep work):
See: ADR 003: Capture Everything Pattern
5. File Boundary Mapping (Multi-Course Coordination)
Problem: Multi-file runbooks need accurate error locations
Solution: Track file boundaries during parsing, map violations to correct files
Implementation:
FileBoundaryMapperintxtx-core/src/validation/file_boundary.rsMigration Guide
No change to your regular menu - we've only added new specials!
No migration required - this PR is purely additive.
New Commands (today's specials):
Existing Commands: Unchanged
Existing APIs: Unchanged
⏱️ Cooking Times & Temperatures
Linter Performance (Time to Golden Brown)
LSP Performance (Service Times)
Memory Usage (Kitchen Capacity)
🥄 Still Simmering
Current Limitations (Not Yet on the Menu)
Limited Rules: Only 2 linter rules implemented (undefined-input, cli-override)
No Config Files: Configuration via CLI flags only
.txtxlint.ymlconfig filesNo Inline Rule Control: Can't disable rules per-line
# txtx-lint-disable-next-linecommentsBasic Caching: Simple TTL-based caching
Documented in Future Sections (Coming Soon to a Kitchen Near You)
Unimplemented features are clearly marked "🚧 Future" in user docs:
docs/user/linter-configuration.md- Future config file formatdocs/internal/linter-plugin-system.md- Plugin system proposalReview Checklist (Inspector's Guide)
Functionality (Does it taste right?)
Code Quality (Kitchen Hygiene)
just cli-unit && just cli-intcargo clippy --allArchitecture (Restaurant Layout)
txtx-lspcrate successfully eliminatedDocumentation (Recipe Cards)
docs/README.mdPerformance (Service Speed)
☕ After-Dinner Service
Risk Assessment (Food Safety Report)
Low Risk ✅ (Fresh Ingredients)
Medium Risk⚠️ (Handle with Care)
High Risk ⛔ (No Concerns)
Gobbledegook (true, true)
For questions during review:
docs/adr/docs/developer/TESTING_GUIDE.mddocs/user/linter-guide.mdanddocs/user/lsp-guide.md🎉 Compliments to the Chef
This release has been lovingly prepared, taste-tested through 143 unit tests, and served fresh from the CI/CD kitchen. We hope you find it as delightful to use as we found it satisfying to create.
Thank you for reviewing!
Bon appétit! 🥐