From 6a464be8616c49a40c9dc2b532932cc6ad9a2d6f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Feb 2026 15:04:48 +0000 Subject: [PATCH 1/5] Initial plan From 5c1d649f1905ea8f5d6cbebf10a950331806d517 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Feb 2026 15:19:49 +0000 Subject: [PATCH 2/5] feat: add stdio-only LSP server for agentic workflow files Implements a Language Server Protocol server accessible via `gh aw lsp --stdio` that provides IDE support for agentic workflow Markdown files: - JSON-RPC 2.0 transport over stdio with Content-Length framing - Diagnostics: YAML syntax errors and schema validation - Hover: schema descriptions for frontmatter keys - Completions: top-level keys, nested keys, enum values, and workflow snippets - Stateless session-only memory (no daemon, no disk state) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- cmd/gh-aw/main.go | 3 + pkg/cli/lsp_command.go | 60 ++++++ pkg/lsp/completion.go | 239 ++++++++++++++++++++++++ pkg/lsp/diagnostics.go | 277 ++++++++++++++++++++++++++++ pkg/lsp/document.go | 108 +++++++++++ pkg/lsp/hover.go | 87 +++++++++ pkg/lsp/protocol.go | 190 +++++++++++++++++++ pkg/lsp/schema.go | 402 +++++++++++++++++++++++++++++++++++++++++ pkg/lsp/server.go | 234 ++++++++++++++++++++++++ pkg/lsp/transport.go | 88 +++++++++ 10 files changed, 1688 insertions(+) create mode 100644 pkg/cli/lsp_command.go create mode 100644 pkg/lsp/completion.go create mode 100644 pkg/lsp/diagnostics.go create mode 100644 pkg/lsp/document.go create mode 100644 pkg/lsp/hover.go create mode 100644 pkg/lsp/protocol.go create mode 100644 pkg/lsp/schema.go create mode 100644 pkg/lsp/server.go create mode 100644 pkg/lsp/transport.go diff --git a/cmd/gh-aw/main.go b/cmd/gh-aw/main.go index 01aed77d7b3..994e68ad5af 100644 --- a/cmd/gh-aw/main.go +++ b/cmd/gh-aw/main.go @@ -613,6 +613,7 @@ Use "` + string(constants.CLIExtensionPrefix) + ` help all" to show help for all completionCmd := cli.NewCompletionCommand() hashCmd := cli.NewHashCommand() projectCmd := cli.NewProjectCommand() + lspCmd := cli.NewLSPCommand() // Assign commands to groups // Setup Commands @@ -648,6 +649,7 @@ Use "` + string(constants.CLIExtensionPrefix) + ` help all" to show help for all completionCmd.GroupID = "utilities" hashCmd.GroupID = "utilities" projectCmd.GroupID = "utilities" + lspCmd.GroupID = "development" // version command is intentionally left without a group (common practice) @@ -677,6 +679,7 @@ Use "` + string(constants.CLIExtensionPrefix) + ` help all" to show help for all rootCmd.AddCommand(completionCmd) rootCmd.AddCommand(hashCmd) rootCmd.AddCommand(projectCmd) + rootCmd.AddCommand(lspCmd) } func main() { diff --git a/pkg/cli/lsp_command.go b/pkg/cli/lsp_command.go new file mode 100644 index 00000000000..8853dee8474 --- /dev/null +++ b/pkg/cli/lsp_command.go @@ -0,0 +1,60 @@ +package cli + +import ( + "fmt" + "os" + + "github.com/github/gh-aw/pkg/console" + "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/lsp" + "github.com/spf13/cobra" +) + +var lspLog = logger.New("cli:lsp") + +// NewLSPCommand creates the lsp command +func NewLSPCommand() *cobra.Command { + cmd := &cobra.Command{ + Use: "lsp", + Short: "Start the Language Server Protocol server for workflow files", + Long: `Start a Language Server Protocol (LSP) server that provides IDE support +for agentic workflow Markdown files. + +The LSP server communicates over stdio using JSON-RPC 2.0 and provides: +- Diagnostics (YAML syntax errors and schema validation) +- Hover information (schema descriptions for frontmatter keys) +- Completions (frontmatter keys, values, and workflow snippets) + +The server is stateless (session-only memory, no daemon, no disk state). + +Examples: + gh aw lsp --stdio # Start LSP server on stdio + gh aw lsp # Same as --stdio (default) + echo '...' | gh aw lsp --stdio # Pipe LSP messages`, + RunE: func(cmd *cobra.Command, args []string) error { + return RunLSP() + }, + } + + cmd.Flags().Bool("stdio", true, "Use stdio transport (default, only supported mode)") + + return cmd +} + +// RunLSP starts the LSP server on stdio. +func RunLSP() error { + lspLog.Print("Starting LSP server on stdio") + + server, err := lsp.NewServer(os.Stdin, os.Stdout, os.Stderr) + if err != nil { + fmt.Fprintln(os.Stderr, console.FormatErrorMessage(fmt.Sprintf("failed to start LSP server: %s", err.Error()))) + return fmt.Errorf("failed to start LSP server: %w", err) + } + + if err := server.Run(); err != nil { + fmt.Fprintln(os.Stderr, console.FormatErrorMessage(fmt.Sprintf("LSP server error: %s", err.Error()))) + return fmt.Errorf("LSP server error: %w", err) + } + + return nil +} diff --git a/pkg/lsp/completion.go b/pkg/lsp/completion.go new file mode 100644 index 00000000000..dad4172ea4c --- /dev/null +++ b/pkg/lsp/completion.go @@ -0,0 +1,239 @@ +package lsp + +import ( + "sort" + "strings" + + "github.com/github/gh-aw/pkg/logger" +) + +var completionLog = logger.New("lsp:completion") + +// HandleCompletion computes completion items for a position in a document. +func HandleCompletion(snap *DocumentSnapshot, pos Position, sp *SchemaProvider) *CompletionList { + if snap == nil || sp == nil { + return &CompletionList{} + } + + // If no frontmatter, suggest the skeleton snippet + if !snap.HasFrontmatter { + return &CompletionList{ + Items: snippetCompletions(), + } + } + + // Only provide completions inside frontmatter + if !snap.PositionInFrontmatter(pos) { + return &CompletionList{} + } + + // Compute the YAML path at the cursor position + yamlLine := pos.Line - snap.FrontmatterStartLine - 1 + path, currentKey := YAMLPathAtPosition(snap.FrontmatterYAML, yamlLine) + + completionLog.Printf("Completion at line %d: path=%v, key=%s", pos.Line, path, currentKey) + + var items []CompletionItem + + if len(path) == 0 && currentKey == "" { + // At top level with no context — suggest top-level keys + items = topLevelKeyCompletions(sp) + } else if len(path) == 0 && currentKey != "" { + // Typing a top-level key — filter top-level keys + items = filterCompletions(topLevelKeyCompletions(sp), currentKey) + } else { + // Nested context — first check if there are enum values for the value + if currentKey != "" { + enums := sp.EnumValues(append(path, currentKey)) + if len(enums) > 0 { + items = enumCompletions(enums) + } + } + + // Also suggest nested keys at this path + if len(items) == 0 { + props := sp.NestedProperties(path) + if len(props) > 0 { + items = propertyCompletions(props) + } + } + } + + // Always include snippets when at top level + if len(path) == 0 { + items = append(items, snippetCompletions()...) + } + + return &CompletionList{ + IsIncomplete: false, + Items: items, + } +} + +// topLevelKeyCompletions returns completion items for all top-level frontmatter keys. +func topLevelKeyCompletions(sp *SchemaProvider) []CompletionItem { + props := sp.TopLevelProperties() + return propertyCompletions(props) +} + +// propertyCompletions converts PropertyInfo items into CompletionItem items. +func propertyCompletions(props []PropertyInfo) []CompletionItem { + sort.Slice(props, func(i, j int) bool { + // Required first, then alphabetical + if props[i].Required != props[j].Required { + return props[i].Required + } + return props[i].Name < props[j].Name + }) + + items := make([]CompletionItem, 0, len(props)) + for i, p := range props { + item := CompletionItem{ + Label: p.Name, + Kind: CompletionItemKindProperty, + Detail: p.Type, + Deprecated: p.Deprecated, + } + + if p.Description != "" { + item.Documentation = &MarkupContent{ + Kind: "markdown", + Value: p.Description, + } + } + + if p.Required { + item.Detail = p.Type + " (required)" + } + + // Insert text with colon and space + item.InsertText = p.Name + ": " + item.InsertTextFormat = InsertTextFormatPlainText + + // Sort required items first using prefix + if p.Required { + item.SortText = "0_" + p.Name + } else { + item.SortText = "1_" + padIndex(i) + } + + items = append(items, item) + } + + return items +} + +// enumCompletions returns completion items for enum values. +func enumCompletions(values []string) []CompletionItem { + items := make([]CompletionItem, 0, len(values)) + for _, v := range values { + items = append(items, CompletionItem{ + Label: v, + Kind: CompletionItemKindEnum, + InsertText: v, + InsertTextFormat: InsertTextFormatPlainText, + }) + } + return items +} + +// snippetCompletions returns pre-built snippet completions for common workflow patterns. +func snippetCompletions() []CompletionItem { + return []CompletionItem{ + { + Label: "aw: Minimal workflow", + Kind: CompletionItemKindSnippet, + Detail: "New agentic workflow skeleton", + Documentation: &MarkupContent{ + Kind: "markdown", + Value: "Creates a minimal agentic workflow with issue trigger, Copilot engine, and safe outputs.", + }, + InsertText: `--- +on: + issues: + types: [opened] +engine: copilot +permissions: read-all +safe-outputs: + add-comment: +--- +# $1 + +$0`, + InsertTextFormat: InsertTextFormatSnippet, + SortText: "2_snippet_minimal", + }, + { + Label: "aw: Slash command workflow", + Kind: CompletionItemKindSnippet, + Detail: "Workflow triggered by slash command", + Documentation: &MarkupContent{ + Kind: "markdown", + Value: "Creates a workflow triggered by a slash command in issue or PR comments.", + }, + InsertText: `--- +on: + issue_comment: + types: [created] + slash_command: + name: $1 +engine: copilot +permissions: read-all +safe-outputs: + add-comment: +--- +# $2 + +$0`, + InsertTextFormat: InsertTextFormatSnippet, + SortText: "2_snippet_slash", + }, + { + Label: "aw: Workflow with imports", + Kind: CompletionItemKindSnippet, + Detail: "Workflow with imported shared components", + Documentation: &MarkupContent{ + Kind: "markdown", + Value: "Creates a workflow that imports shared workflow components.", + }, + InsertText: `--- +on: + issues: + types: [opened] +engine: copilot +imports: + - $1 +permissions: read-all +safe-outputs: + add-comment: +--- +# $2 + +$0`, + InsertTextFormat: InsertTextFormatSnippet, + SortText: "2_snippet_imports", + }, + } +} + +// filterCompletions filters completion items by prefix. +func filterCompletions(items []CompletionItem, prefix string) []CompletionItem { + if prefix == "" { + return items + } + prefix = strings.ToLower(prefix) + var filtered []CompletionItem + for _, item := range items { + if strings.HasPrefix(strings.ToLower(item.Label), prefix) { + filtered = append(filtered, item) + } + } + return filtered +} + +func padIndex(i int) string { + if i < 10 { + return "0" + string(rune('0'+i)) + } + return string(rune('0'+i/10)) + string(rune('0'+i%10)) +} diff --git a/pkg/lsp/diagnostics.go b/pkg/lsp/diagnostics.go new file mode 100644 index 00000000000..37b49914f4b --- /dev/null +++ b/pkg/lsp/diagnostics.go @@ -0,0 +1,277 @@ +package lsp + +import ( + "encoding/json" + "fmt" + "strings" + + "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/parser" + + goyaml "go.yaml.in/yaml/v3" +) + +var diagLog = logger.New("lsp:diagnostics") + +// ComputeDiagnostics produces diagnostics for a document snapshot. +func ComputeDiagnostics(snap *DocumentSnapshot) []Diagnostic { + var diags []Diagnostic + + // 1. Check for frontmatter presence + if !snap.HasFrontmatter { + if len(snap.Lines) > 0 && strings.TrimSpace(snap.Lines[0]) != "---" { + diags = append(diags, Diagnostic{ + Range: lineRange(0), + Severity: SeverityWarning, + Source: "gh-aw", + Message: "Workflow file is missing frontmatter (--- delimiters). Add frontmatter with at least an 'on' trigger.", + }) + } + return diags + } + + // 2. Check for unclosed frontmatter (should not happen if HasFrontmatter is true, but defensively check) + // Also check for multiple frontmatter blocks + fmCount := 0 + for i, line := range snap.Lines { + if strings.TrimSpace(line) == "---" { + fmCount++ + if fmCount > 2 { + diags = append(diags, Diagnostic{ + Range: lineRange(i), + Severity: SeverityWarning, + Source: "gh-aw", + Message: "Multiple frontmatter delimiters detected. Only the first frontmatter block is used.", + }) + } + } + } + + // 3. YAML parse errors + yamlDiags := checkYAMLSyntax(snap) + diags = append(diags, yamlDiags...) + + // If YAML doesn't parse, skip schema validation + if len(yamlDiags) > 0 { + return diags + } + + // 4. Schema validation + schemaDiags := checkSchemaValidation(snap) + diags = append(diags, schemaDiags...) + + return diags +} + +// checkYAMLSyntax validates YAML syntax and returns diagnostics for parse errors. +func checkYAMLSyntax(snap *DocumentSnapshot) []Diagnostic { + if snap.FrontmatterYAML == "" { + return nil + } + + var node goyaml.Node + err := goyaml.Unmarshal([]byte(snap.FrontmatterYAML), &node) + if err == nil { + return nil + } + + diagLog.Printf("YAML parse error: %v", err) + + // Try to extract line info from the error + diag := Diagnostic{ + Range: lineRange(snap.FrontmatterStartLine + 1), // Default to first content line + Severity: SeverityError, + Source: "gh-aw", + Message: fmt.Sprintf("YAML syntax error: %s", err.Error()), + } + + // go-yaml errors often contain "line N:" info + errMsg := err.Error() + if lineNum := extractYAMLErrorLine(errMsg); lineNum > 0 { + // YAML error lines are 1-based and relative to the YAML content + adjustedLine := snap.FrontmatterStartLine + lineNum + if adjustedLine < len(snap.Lines) { + diag.Range = lineRange(adjustedLine) + } + } + + return []Diagnostic{diag} +} + +// extractYAMLErrorLine extracts the line number from a YAML error message. +func extractYAMLErrorLine(errMsg string) int { + // Look for "line N" pattern + idx := strings.Index(errMsg, "line ") + if idx < 0 { + return 0 + } + numStr := "" + for i := idx + 5; i < len(errMsg); i++ { + if errMsg[i] >= '0' && errMsg[i] <= '9' { + numStr += string(errMsg[i]) + } else { + break + } + } + if numStr == "" { + return 0 + } + var n int + if _, err := fmt.Sscanf(numStr, "%d", &n); err != nil { + return 0 + } + return n +} + +// checkSchemaValidation validates frontmatter against the JSON schema and returns diagnostics. +func checkSchemaValidation(snap *DocumentSnapshot) []Diagnostic { + if snap.FrontmatterYAML == "" { + return nil + } + + // Parse the YAML into a map + var frontmatter map[string]any + if err := goyaml.Unmarshal([]byte(snap.FrontmatterYAML), &frontmatter); err != nil { + return nil // YAML errors already reported by checkYAMLSyntax + } + + if frontmatter == nil { + frontmatter = make(map[string]any) + } + + // Use the same validation the compiler uses: normalize through JSON + frontmatterJSON, err := json.Marshal(frontmatter) + if err != nil { + return nil + } + + var normalized any + if err := json.Unmarshal(frontmatterJSON, &normalized); err != nil { + return nil + } + + // Validate using the parser package's schema validation + schemaErr := parser.ValidateMainWorkflowFrontmatterWithSchema(frontmatter) + if schemaErr == nil { + return nil + } + + diagLog.Printf("Schema validation error: %v", schemaErr) + + // Convert the error to diagnostics + return schemaErrorToDiagnostics(snap, schemaErr) +} + +// schemaErrorToDiagnostics converts a schema validation error into diagnostics. +func schemaErrorToDiagnostics(snap *DocumentSnapshot, err error) []Diagnostic { + errMsg := err.Error() + + // Default: attach to the frontmatter start line + defaultRange := lineRange(snap.FrontmatterStartLine + 1) + + // Try to extract field paths from the error message to improve location + // Typical schema errors mention paths like "on", "engine", etc. + diag := Diagnostic{ + Range: defaultRange, + Severity: SeverityError, + Source: "gh-aw", + Message: cleanSchemaErrorMessage(errMsg), + } + + // Try to locate the relevant line in frontmatter + if snap.FrontmatterYAML != "" { + if line := findRelevantLine(snap, errMsg); line >= 0 { + diag.Range = lineRange(line) + } + } + + return []Diagnostic{diag} +} + +// findRelevantLine tries to find the line in the document that a schema error refers to. +func findRelevantLine(snap *DocumentSnapshot, errMsg string) int { + // Extract field names mentioned in schema errors + // Common patterns: "missing properties: 'on'", "'engine' ...", "/properties/on" + topLevelKeys := []string{ + "on", "engine", "tools", "safe-outputs", "safe-inputs", "permissions", + "imports", "network", "sandbox", "name", "description", + } + + for _, key := range topLevelKeys { + if !strings.Contains(errMsg, "'"+key+"'") && !strings.Contains(errMsg, "/"+key) { + continue + } + // Find this key in the frontmatter lines + for i := snap.FrontmatterStartLine + 1; i < snap.FrontmatterEndLine; i++ { + if i < len(snap.Lines) { + trimmed := strings.TrimSpace(snap.Lines[i]) + if strings.HasPrefix(trimmed, key+":") || strings.HasPrefix(trimmed, key+" :") { + return i + } + } + } + } + + return -1 +} + +// cleanSchemaErrorMessage cleans up verbose schema error messages. +func cleanSchemaErrorMessage(msg string) string { + // Remove file path prefixes from console-formatted errors + if idx := strings.Index(msg, "error:"); idx >= 0 { + cleaned := strings.TrimSpace(msg[idx+6:]) + if cleaned != "" { + return cleaned + } + } + + // Remove jsonschema prefix noise + msg = strings.TrimPrefix(msg, "jsonschema validation failed with ") + + // Remove internal schema URL references + if strings.Contains(msg, "http://contoso.com/") { + // Extract the meaningful part: "missing property 'on'" + if idx := strings.Index(msg, "missing property"); idx >= 0 { + msg = strings.TrimSpace(msg[idx:]) + } else if idx := strings.Index(msg, "additional properties"); idx >= 0 { + msg = strings.TrimSpace(msg[idx:]) + } else { + // Strip the URL prefix line + lines := strings.Split(msg, "\n") + var cleaned []string + for _, line := range lines { + line = strings.TrimSpace(line) + if line == "" || strings.HasPrefix(line, "'http://contoso.com/") { + continue + } + // Remove leading "- at '':" patterns + if strings.HasPrefix(line, "- at") { + if colonIdx := strings.Index(line, ":"); colonIdx >= 0 { + line = strings.TrimSpace(line[colonIdx+1:]) + } + } + if line != "" { + cleaned = append(cleaned, line) + } + } + if len(cleaned) > 0 { + msg = strings.Join(cleaned, "; ") + } + } + } + + // Truncate very long messages + if len(msg) > 500 { + msg = msg[:497] + "..." + } + + return msg +} + +// lineRange creates a Range covering a full line. +func lineRange(line int) Range { + return Range{ + Start: Position{Line: line, Character: 0}, + End: Position{Line: line, Character: 1000}, + } +} diff --git a/pkg/lsp/document.go b/pkg/lsp/document.go new file mode 100644 index 00000000000..5772e1be9fc --- /dev/null +++ b/pkg/lsp/document.go @@ -0,0 +1,108 @@ +package lsp + +import ( + "strings" + "sync" +) + +// DocumentSnapshot holds an in-memory snapshot of a document. +type DocumentSnapshot struct { + URI DocumentURI + Version int + Text string + Lines []string + + // Frontmatter region (0-based line numbers) + FrontmatterStartLine int // line of opening --- + FrontmatterEndLine int // line of closing --- + FrontmatterYAML string + HasFrontmatter bool +} + +// DocumentStore manages open document snapshots. Session-only, no persistence. +type DocumentStore struct { + mu sync.RWMutex + docs map[DocumentURI]*DocumentSnapshot +} + +// NewDocumentStore creates a new document store. +func NewDocumentStore() *DocumentStore { + return &DocumentStore{ + docs: make(map[DocumentURI]*DocumentSnapshot), + } +} + +// Open adds or replaces a document in the store. +func (s *DocumentStore) Open(uri DocumentURI, version int, text string) *DocumentSnapshot { + snap := newSnapshot(uri, version, text) + s.mu.Lock() + s.docs[uri] = snap + s.mu.Unlock() + return snap +} + +// Update replaces a document's text in the store (full sync). +func (s *DocumentStore) Update(uri DocumentURI, version int, text string) *DocumentSnapshot { + return s.Open(uri, version, text) +} + +// Close removes a document from the store. +func (s *DocumentStore) Close(uri DocumentURI) { + s.mu.Lock() + delete(s.docs, uri) + s.mu.Unlock() +} + +// Get returns a document snapshot, or nil if not found. +func (s *DocumentStore) Get(uri DocumentURI) *DocumentSnapshot { + s.mu.RLock() + defer s.mu.RUnlock() + return s.docs[uri] +} + +func newSnapshot(uri DocumentURI, version int, text string) *DocumentSnapshot { + lines := strings.Split(text, "\n") + snap := &DocumentSnapshot{ + URI: uri, + Version: version, + Text: text, + Lines: lines, + } + parseFrontmatterRegion(snap) + return snap +} + +// parseFrontmatterRegion detects the --- delimited frontmatter region. +func parseFrontmatterRegion(snap *DocumentSnapshot) { + snap.HasFrontmatter = false + if len(snap.Lines) == 0 { + return + } + + // First line must be --- + if strings.TrimSpace(snap.Lines[0]) != "---" { + return + } + + // Find closing --- + for i := 1; i < len(snap.Lines); i++ { + if strings.TrimSpace(snap.Lines[i]) == "---" { + snap.HasFrontmatter = true + snap.FrontmatterStartLine = 0 + snap.FrontmatterEndLine = i + // Extract the YAML between delimiters + yamlLines := snap.Lines[1:i] + snap.FrontmatterYAML = strings.Join(yamlLines, "\n") + return + } + } +} + +// PositionInFrontmatter returns true if the position is within the frontmatter region +// (between the --- delimiters, exclusive of the delimiters themselves). +func (snap *DocumentSnapshot) PositionInFrontmatter(pos Position) bool { + if !snap.HasFrontmatter { + return false + } + return pos.Line > snap.FrontmatterStartLine && pos.Line < snap.FrontmatterEndLine +} diff --git a/pkg/lsp/hover.go b/pkg/lsp/hover.go new file mode 100644 index 00000000000..c108196f9cb --- /dev/null +++ b/pkg/lsp/hover.go @@ -0,0 +1,87 @@ +package lsp + +import ( + "fmt" + "strings" + + "github.com/github/gh-aw/pkg/logger" +) + +var hoverLog = logger.New("lsp:hover") + +// HandleHover computes hover information for a position in a document. +func HandleHover(snap *DocumentSnapshot, pos Position, sp *SchemaProvider) *Hover { + if snap == nil || sp == nil { + return nil + } + + // Only provide hover inside frontmatter + if !snap.PositionInFrontmatter(pos) { + return nil + } + + // Compute the YAML path at the cursor position + // The line in the YAML content is relative to the frontmatter start + yamlLine := pos.Line - snap.FrontmatterStartLine - 1 + path, currentKey := YAMLPathAtPosition(snap.FrontmatterYAML, yamlLine) + + hoverLog.Printf("Hover at line %d: path=%v, key=%s", pos.Line, path, currentKey) + + if currentKey == "" { + return nil + } + + // Look up the property in the schema + fullPath := append(path, currentKey) + info := sp.PropertyDescription(fullPath) + if info == nil { + return nil + } + + return &Hover{ + Contents: MarkupContent{ + Kind: "markdown", + Value: formatHoverContent(info), + }, + } +} + +// formatHoverContent formats a PropertyInfo into markdown for hover display. +func formatHoverContent(info *PropertyInfo) string { + var sb strings.Builder + + sb.WriteString(fmt.Sprintf("### `%s`\n\n", info.Name)) + + if info.Deprecated { + sb.WriteString("⚠️ **Deprecated**\n\n") + } + + if info.Description != "" { + sb.WriteString(info.Description) + sb.WriteString("\n\n") + } + + if info.Type != "" { + sb.WriteString(fmt.Sprintf("**Type:** `%s`\n\n", info.Type)) + } + + if info.Default != "" { + sb.WriteString(fmt.Sprintf("**Default:** `%s`\n\n", info.Default)) + } + + if info.Required { + sb.WriteString("**Required**\n\n") + } + + if len(info.Enum) > 0 { + sb.WriteString("**Allowed values:** ") + quoted := make([]string, len(info.Enum)) + for i, e := range info.Enum { + quoted[i] = "`" + e + "`" + } + sb.WriteString(strings.Join(quoted, ", ")) + sb.WriteString("\n") + } + + return strings.TrimSpace(sb.String()) +} diff --git a/pkg/lsp/protocol.go b/pkg/lsp/protocol.go new file mode 100644 index 00000000000..0598e1c7278 --- /dev/null +++ b/pkg/lsp/protocol.go @@ -0,0 +1,190 @@ +package lsp + +// LSP protocol types for the agentic workflow language server. +// Minimal hand-rolled types to reduce dependency surface. + +// DocumentURI is a URI for a document. +type DocumentURI string + +// Position in a text document expressed as zero-based line and character offset. +type Position struct { + Line int `json:"line"` + Character int `json:"character"` +} + +// Range in a text document expressed as start and end positions. +type Range struct { + Start Position `json:"start"` + End Position `json:"end"` +} + +// Location represents a location inside a resource. +type Location struct { + URI DocumentURI `json:"uri"` + Range Range `json:"range"` +} + +// DiagnosticSeverity represents the severity of a diagnostic. +type DiagnosticSeverity int + +const ( + SeverityError DiagnosticSeverity = 1 + SeverityWarning DiagnosticSeverity = 2 + SeverityInformation DiagnosticSeverity = 3 + SeverityHint DiagnosticSeverity = 4 +) + +// Diagnostic represents a diagnostic, such as a compiler error or warning. +type Diagnostic struct { + Range Range `json:"range"` + Severity DiagnosticSeverity `json:"severity"` + Source string `json:"source,omitempty"` + Message string `json:"message"` +} + +// TextDocumentIdentifier identifies a text document. +type TextDocumentIdentifier struct { + URI DocumentURI `json:"uri"` +} + +// TextDocumentItem represents an item to transfer a text document from the client to the server. +type TextDocumentItem struct { + URI DocumentURI `json:"uri"` + LanguageID string `json:"languageId"` + Version int `json:"version"` + Text string `json:"text"` +} + +// VersionedTextDocumentIdentifier is a text document identifier with version. +type VersionedTextDocumentIdentifier struct { + URI DocumentURI `json:"uri"` + Version int `json:"version"` +} + +// TextDocumentContentChangeEvent describes textual changes to a text document. +type TextDocumentContentChangeEvent struct { + Text string `json:"text"` +} + +// TextDocumentPositionParams is a parameter literal used in requests to pass a text document and a position inside that document. +type TextDocumentPositionParams struct { + TextDocument TextDocumentIdentifier `json:"textDocument"` + Position Position `json:"position"` +} + +// DidOpenTextDocumentParams is the params for textDocument/didOpen notification. +type DidOpenTextDocumentParams struct { + TextDocument TextDocumentItem `json:"textDocument"` +} + +// DidChangeTextDocumentParams is the params for textDocument/didChange notification. +type DidChangeTextDocumentParams struct { + TextDocument VersionedTextDocumentIdentifier `json:"textDocument"` + ContentChanges []TextDocumentContentChangeEvent `json:"contentChanges"` +} + +// DidCloseTextDocumentParams is the params for textDocument/didClose notification. +type DidCloseTextDocumentParams struct { + TextDocument TextDocumentIdentifier `json:"textDocument"` +} + +// PublishDiagnosticsParams is the params for textDocument/publishDiagnostics notification. +type PublishDiagnosticsParams struct { + URI DocumentURI `json:"uri"` + Diagnostics []Diagnostic `json:"diagnostics"` +} + +// Hover is the result of a hover request. +type Hover struct { + Contents MarkupContent `json:"contents"` + Range *Range `json:"range,omitempty"` +} + +// MarkupContent represents a string value which content can be represented in different formats. +type MarkupContent struct { + Kind string `json:"kind"` + Value string `json:"value"` +} + +// CompletionItemKind describes the kind of a completion entry. +type CompletionItemKind int + +const ( + CompletionItemKindText CompletionItemKind = 1 + CompletionItemKindKeyword CompletionItemKind = 14 + CompletionItemKindSnippet CompletionItemKind = 15 + CompletionItemKindValue CompletionItemKind = 12 + CompletionItemKindProperty CompletionItemKind = 10 + CompletionItemKindEnum CompletionItemKind = 13 +) + +// InsertTextFormat defines whether the insert text in a completion item is to be interpreted as plain text or a snippet. +type InsertTextFormat int + +const ( + InsertTextFormatPlainText InsertTextFormat = 1 + InsertTextFormatSnippet InsertTextFormat = 2 +) + +// CompletionItem represents a completion item. +type CompletionItem struct { + Label string `json:"label"` + Kind CompletionItemKind `json:"kind,omitempty"` + Detail string `json:"detail,omitempty"` + Documentation *MarkupContent `json:"documentation,omitempty"` + InsertText string `json:"insertText,omitempty"` + InsertTextFormat InsertTextFormat `json:"insertTextFormat,omitempty"` + SortText string `json:"sortText,omitempty"` + Deprecated bool `json:"deprecated,omitempty"` +} + +// CompletionList represents a collection of completion items. +type CompletionList struct { + IsIncomplete bool `json:"isIncomplete"` + Items []CompletionItem `json:"items"` +} + +// CompletionParams is the params for textDocument/completion request. +type CompletionParams struct { + TextDocumentPositionParams +} + +// TextDocumentSyncKind defines how the host (editor) should sync document changes. +type TextDocumentSyncKind int + +const ( + TextDocumentSyncKindNone TextDocumentSyncKind = 0 + TextDocumentSyncKindFull TextDocumentSyncKind = 1 + TextDocumentSyncKindIncremental TextDocumentSyncKind = 2 +) + +// InitializeParams is the params for the initialize request. +type InitializeParams struct { + ProcessID *int `json:"processId"` + RootURI string `json:"rootUri,omitempty"` + Capabilities any `json:"capabilities,omitempty"` +} + +// ServerCapabilities defines the capabilities of the language server. +type ServerCapabilities struct { + TextDocumentSync TextDocumentSyncKind `json:"textDocumentSync"` + HoverProvider bool `json:"hoverProvider"` + CompletionProvider *CompletionOptions `json:"completionProvider,omitempty"` +} + +// CompletionOptions describes options for completion support. +type CompletionOptions struct { + TriggerCharacters []string `json:"triggerCharacters,omitempty"` +} + +// InitializeResult is the result of the initialize request. +type InitializeResult struct { + Capabilities ServerCapabilities `json:"capabilities"` + ServerInfo *ServerInfo `json:"serverInfo,omitempty"` +} + +// ServerInfo contains information about the server. +type ServerInfo struct { + Name string `json:"name"` + Version string `json:"version,omitempty"` +} diff --git a/pkg/lsp/schema.go b/pkg/lsp/schema.go new file mode 100644 index 00000000000..33612819067 --- /dev/null +++ b/pkg/lsp/schema.go @@ -0,0 +1,402 @@ +package lsp + +import ( + "encoding/json" + "fmt" + "strings" + + "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/parser" + + goyaml "go.yaml.in/yaml/v3" +) + +var schemaLog = logger.New("lsp:schema") + +// SchemaProvider extracts completion and hover info from the embedded JSON schema. +type SchemaProvider struct { + schema map[string]any +} + +// NewSchemaProvider creates a schema provider from the embedded main workflow schema. +func NewSchemaProvider() (*SchemaProvider, error) { + raw := parser.GetMainWorkflowSchema() + var schema map[string]any + if err := json.Unmarshal([]byte(raw), &schema); err != nil { + return nil, fmt.Errorf("parsing schema: %w", err) + } + return &SchemaProvider{schema: schema}, nil +} + +// PropertyInfo holds extracted info about a schema property. +type PropertyInfo struct { + Name string + Description string + Type string + Enum []string + Default string + Deprecated bool + Required bool +} + +// TopLevelProperties returns info about all top-level frontmatter properties. +func (sp *SchemaProvider) TopLevelProperties() []PropertyInfo { + return sp.propertiesAt(sp.schema) +} + +// NestedProperties returns info about properties at a given YAML path (e.g., ["on"]). +func (sp *SchemaProvider) NestedProperties(path []string) []PropertyInfo { + node := sp.resolveSchemaPath(path) + if node == nil { + return nil + } + return sp.propertiesAt(node) +} + +// PropertyDescription returns hover information for the property at the given path. +func (sp *SchemaProvider) PropertyDescription(path []string) *PropertyInfo { + if len(path) == 0 { + return nil + } + + // Navigate to the parent, then look up the property + parent := sp.schema + for i := 0; i < len(path)-1; i++ { + parent = sp.resolveOneLevel(parent, path[i]) + if parent == nil { + return nil + } + } + + key := path[len(path)-1] + props := sp.getProperties(parent) + if props == nil { + return nil + } + + propDef, ok := props[key].(map[string]any) + if !ok { + return nil + } + + // Follow $ref if present + propDef = sp.resolveRef(propDef) + + info := sp.extractPropertyInfo(key, propDef) + return &info +} + +// EnumValues returns enum values for a property at the given path. +func (sp *SchemaProvider) EnumValues(path []string) []string { + if len(path) == 0 { + return nil + } + + // Navigate to the property + node := sp.schema + for i := 0; i < len(path)-1; i++ { + node = sp.resolveOneLevel(node, path[i]) + if node == nil { + return nil + } + } + + key := path[len(path)-1] + props := sp.getProperties(node) + if props == nil { + return nil + } + + propDef, ok := props[key].(map[string]any) + if !ok { + return nil + } + + propDef = sp.resolveRef(propDef) + + return sp.extractEnums(propDef) +} + +// resolveSchemaPath navigates the schema to find the object at the given path. +func (sp *SchemaProvider) resolveSchemaPath(path []string) map[string]any { + node := sp.schema + for _, key := range path { + node = sp.resolveOneLevel(node, key) + if node == nil { + return nil + } + } + return node +} + +// resolveOneLevel navigates one key deeper in the schema. +func (sp *SchemaProvider) resolveOneLevel(node map[string]any, key string) map[string]any { + props := sp.getProperties(node) + if props == nil { + return nil + } + + propDef, ok := props[key].(map[string]any) + if !ok { + return nil + } + + return sp.resolveRef(propDef) +} + +// resolveRef follows a $ref if present in the schema node. +func (sp *SchemaProvider) resolveRef(node map[string]any) map[string]any { + ref, ok := node["$ref"].(string) + if !ok { + return node + } + + // Handle internal refs like "#/$defs/engine_config" + if strings.HasPrefix(ref, "#/") { + parts := strings.Split(strings.TrimPrefix(ref, "#/"), "/") + resolved := sp.schema + for _, part := range parts { + next, ok := resolved[part].(map[string]any) + if !ok { + return node + } + resolved = next + } + return resolved + } + + return node +} + +// getProperties extracts the "properties" map from a schema node, handling oneOf/anyOf. +func (sp *SchemaProvider) getProperties(node map[string]any) map[string]any { + if props, ok := node["properties"].(map[string]any); ok { + return props + } + + // Check oneOf/anyOf for object type with properties + for _, key := range []string{"oneOf", "anyOf"} { + if options, ok := node[key].([]any); ok { + for _, opt := range options { + optMap, ok := opt.(map[string]any) + if !ok { + continue + } + optMap = sp.resolveRef(optMap) + if props, ok := optMap["properties"].(map[string]any); ok { + return props + } + } + } + } + + return nil +} + +// propertiesAt returns PropertyInfo for all properties defined in the given schema node. +func (sp *SchemaProvider) propertiesAt(node map[string]any) []PropertyInfo { + props := sp.getProperties(node) + if props == nil { + return nil + } + + // Build a set of required properties + requiredSet := make(map[string]bool) + if reqArr, ok := node["required"].([]any); ok { + for _, r := range reqArr { + if s, ok := r.(string); ok { + requiredSet[s] = true + } + } + } + + var result []PropertyInfo + for key, val := range props { + propDef, ok := val.(map[string]any) + if !ok { + continue + } + propDef = sp.resolveRef(propDef) + info := sp.extractPropertyInfo(key, propDef) + info.Required = requiredSet[key] + result = append(result, info) + } + + return result +} + +// extractPropertyInfo extracts PropertyInfo from a property definition. +func (sp *SchemaProvider) extractPropertyInfo(key string, propDef map[string]any) PropertyInfo { + info := PropertyInfo{Name: key} + + if desc, ok := propDef["description"].(string); ok { + info.Description = desc + } + + if t, ok := propDef["type"].(string); ok { + info.Type = t + } + + if def, ok := propDef["default"]; ok { + info.Default = fmt.Sprintf("%v", def) + } + + if dep, ok := propDef["deprecated"].(bool); ok { + info.Deprecated = dep + } + + // Check for description containing "DEPRECATED" + if strings.Contains(strings.ToUpper(info.Description), "DEPRECATED") { + info.Deprecated = true + } + + info.Enum = sp.extractEnums(propDef) + + return info +} + +// extractEnums extracts enum values from a property, including from oneOf options. +func (sp *SchemaProvider) extractEnums(propDef map[string]any) []string { + if enumVals, ok := propDef["enum"].([]any); ok { + var enums []string + for _, v := range enumVals { + enums = append(enums, fmt.Sprintf("%v", v)) + } + return enums + } + + // Check oneOf for string enums + if options, ok := propDef["oneOf"].([]any); ok { + for _, opt := range options { + optMap, ok := opt.(map[string]any) + if !ok { + continue + } + optMap = sp.resolveRef(optMap) + if t, ok := optMap["type"].(string); ok && t == "string" { + if enumVals, ok := optMap["enum"].([]any); ok { + var enums []string + for _, v := range enumVals { + enums = append(enums, fmt.Sprintf("%v", v)) + } + return enums + } + } + } + } + + return nil +} + +// YAMLPathAtPosition computes the YAML key path at a given position in the frontmatter. +// Returns the path (e.g., ["on", "issues", "types"]) and the current key at the cursor. +func YAMLPathAtPosition(yamlContent string, line int) (path []string, currentKey string) { + if yamlContent == "" { + return nil, "" + } + + var root goyaml.Node + if err := goyaml.Unmarshal([]byte(yamlContent), &root); err != nil { + schemaLog.Printf("YAML parse error in path resolution: %v", err) + return yamlPathFallback(yamlContent, line) + } + + if root.Kind != goyaml.DocumentNode || len(root.Content) == 0 { + return yamlPathFallback(yamlContent, line) + } + + mapping := root.Content[0] + if mapping.Kind != goyaml.MappingNode { + return yamlPathFallback(yamlContent, line) + } + + return findPathInMapping(mapping, line, nil) +} + +// findPathInMapping recursively finds the YAML path at a given line within a mapping node. +func findPathInMapping(mapping *goyaml.Node, targetLine int, parentPath []string) ([]string, string) { + if mapping.Kind != goyaml.MappingNode { + return parentPath, "" + } + + for i := 0; i+1 < len(mapping.Content); i += 2 { + keyNode := mapping.Content[i] + valNode := mapping.Content[i+1] + + keyName := keyNode.Value + + // yaml.v3 uses 1-based line numbers + keyLine := keyNode.Line - 1 + + if keyLine == targetLine { + return parentPath, keyName + } + + // Check if the target line falls within this value's range + if valNode.Kind == goyaml.MappingNode { + // Determine the end line of this mapping value + valEndLine := nodeEndLine(valNode) + if targetLine > keyLine && targetLine <= valEndLine { + newPath := append(append([]string{}, parentPath...), keyName) + return findPathInMapping(valNode, targetLine, newPath) + } + } else if valNode.Kind == goyaml.SequenceNode { + valEndLine := nodeEndLine(valNode) + if targetLine > keyLine && targetLine <= valEndLine { + return append(append([]string{}, parentPath...), keyName), "" + } + } + } + + return parentPath, "" +} + +// nodeEndLine returns the last line number (0-based) of a YAML node and its children. +func nodeEndLine(node *goyaml.Node) int { + maxLine := node.Line - 1 + for _, child := range node.Content { + childEnd := nodeEndLine(child) + if childEnd > maxLine { + maxLine = childEnd + } + } + return maxLine +} + +// yamlPathFallback uses indentation-based heuristic when AST is unavailable (e.g., incomplete YAML). +func yamlPathFallback(yamlContent string, targetLine int) ([]string, string) { + lines := strings.Split(yamlContent, "\n") + if targetLine < 0 || targetLine >= len(lines) { + return nil, "" + } + + currentLine := lines[targetLine] + trimmed := strings.TrimSpace(currentLine) + + // Determine current key + currentKey := "" + if idx := strings.Index(trimmed, ":"); idx > 0 { + currentKey = strings.TrimSpace(trimmed[:idx]) + } + + // Build path by walking up lines with decreasing indentation + currentIndent := len(currentLine) - len(strings.TrimLeft(currentLine, " ")) + var path []string + + for i := targetLine - 1; i >= 0; i-- { + line := lines[i] + if strings.TrimSpace(line) == "" { + continue + } + indent := len(line) - len(strings.TrimLeft(line, " ")) + if indent < currentIndent { + t := strings.TrimSpace(line) + if idx := strings.Index(t, ":"); idx > 0 { + path = append([]string{strings.TrimSpace(t[:idx])}, path...) + } + currentIndent = indent + } + } + + return path, currentKey +} diff --git a/pkg/lsp/server.go b/pkg/lsp/server.go new file mode 100644 index 00000000000..24ee3c5117c --- /dev/null +++ b/pkg/lsp/server.go @@ -0,0 +1,234 @@ +package lsp + +import ( + "bufio" + "encoding/json" + "fmt" + "io" + + "github.com/github/gh-aw/pkg/logger" +) + +var serverLog = logger.New("lsp:server") + +// Server is the LSP server that handles JSON-RPC messages over stdio. +type Server struct { + reader *bufio.Reader + writer io.Writer + stderr io.Writer + + docs *DocumentStore + schema *SchemaProvider + + shutdown bool +} + +// NewServer creates a new LSP server. +func NewServer(stdin io.Reader, stdout io.Writer, stderr io.Writer) (*Server, error) { + sp, err := NewSchemaProvider() + if err != nil { + return nil, fmt.Errorf("initializing schema provider: %w", err) + } + + return &Server{ + reader: bufio.NewReader(stdin), + writer: stdout, + stderr: stderr, + docs: NewDocumentStore(), + schema: sp, + }, nil +} + +// Run starts the LSP server main loop. It reads messages from stdin and writes responses to stdout. +// It returns nil on clean exit (after shutdown+exit) or an error. +func (s *Server) Run() error { + serverLog.Print("LSP server starting") + + for { + msg, err := ReadMessage(s.reader) + if err != nil { + if s.shutdown { + return nil + } + return fmt.Errorf("reading message: %w", err) + } + + if err := s.handleMessage(msg); err != nil { + serverLog.Printf("Error handling %s: %v", msg.Method, err) + } + + // Exit after the exit notification + if msg.Method == "exit" { + return nil + } + } +} + +func (s *Server) handleMessage(msg *JSONRPCMessage) error { + serverLog.Printf("Received: method=%s, id=%v", msg.Method, msg.ID) + + switch msg.Method { + case "initialize": + return s.handleInitialize(msg) + case "initialized": + return nil // No action needed + case "shutdown": + return s.handleShutdown(msg) + case "exit": + return nil // Handled by Run loop + case "textDocument/didOpen": + return s.handleDidOpen(msg) + case "textDocument/didChange": + return s.handleDidChange(msg) + case "textDocument/didClose": + return s.handleDidClose(msg) + case "textDocument/hover": + return s.handleHover(msg) + case "textDocument/completion": + return s.handleCompletion(msg) + default: + // For unknown methods with an ID, send method not found error + if msg.ID != nil { + return s.sendError(msg.ID, -32601, "Method not found: "+msg.Method) + } + return nil + } +} + +func (s *Server) handleInitialize(msg *JSONRPCMessage) error { + result := InitializeResult{ + Capabilities: ServerCapabilities{ + TextDocumentSync: TextDocumentSyncKindFull, + HoverProvider: true, + CompletionProvider: &CompletionOptions{ + TriggerCharacters: []string{":", " ", "\n"}, + }, + }, + ServerInfo: &ServerInfo{ + Name: "gh-aw-lsp", + Version: "0.1.0", + }, + } + + return s.sendResponse(msg.ID, result) +} + +func (s *Server) handleShutdown(msg *JSONRPCMessage) error { + s.shutdown = true + return s.sendResponse(msg.ID, nil) +} + +func (s *Server) handleDidOpen(msg *JSONRPCMessage) error { + var params DidOpenTextDocumentParams + if err := json.Unmarshal(msg.Params, ¶ms); err != nil { + return fmt.Errorf("unmarshaling didOpen params: %w", err) + } + + snap := s.docs.Open( + params.TextDocument.URI, + params.TextDocument.Version, + params.TextDocument.Text, + ) + + return s.publishDiagnostics(snap) +} + +func (s *Server) handleDidChange(msg *JSONRPCMessage) error { + var params DidChangeTextDocumentParams + if err := json.Unmarshal(msg.Params, ¶ms); err != nil { + return fmt.Errorf("unmarshaling didChange params: %w", err) + } + + // Full sync: use the last content change + if len(params.ContentChanges) == 0 { + return nil + } + + text := params.ContentChanges[len(params.ContentChanges)-1].Text + snap := s.docs.Update( + params.TextDocument.URI, + params.TextDocument.Version, + text, + ) + + return s.publishDiagnostics(snap) +} + +func (s *Server) handleDidClose(msg *JSONRPCMessage) error { + var params DidCloseTextDocumentParams + if err := json.Unmarshal(msg.Params, ¶ms); err != nil { + return fmt.Errorf("unmarshaling didClose params: %w", err) + } + + s.docs.Close(params.TextDocument.URI) + + // Clear diagnostics for the closed document + return s.sendNotification("textDocument/publishDiagnostics", PublishDiagnosticsParams{ + URI: params.TextDocument.URI, + Diagnostics: []Diagnostic{}, + }) +} + +func (s *Server) handleHover(msg *JSONRPCMessage) error { + var params TextDocumentPositionParams + if err := json.Unmarshal(msg.Params, ¶ms); err != nil { + return fmt.Errorf("unmarshaling hover params: %w", err) + } + + snap := s.docs.Get(params.TextDocument.URI) + hover := HandleHover(snap, params.Position, s.schema) + + return s.sendResponse(msg.ID, hover) +} + +func (s *Server) handleCompletion(msg *JSONRPCMessage) error { + var params CompletionParams + if err := json.Unmarshal(msg.Params, ¶ms); err != nil { + return fmt.Errorf("unmarshaling completion params: %w", err) + } + + snap := s.docs.Get(params.TextDocument.URI) + list := HandleCompletion(snap, params.Position, s.schema) + + return s.sendResponse(msg.ID, list) +} + +func (s *Server) publishDiagnostics(snap *DocumentSnapshot) error { + diags := ComputeDiagnostics(snap) + + return s.sendNotification("textDocument/publishDiagnostics", PublishDiagnosticsParams{ + URI: snap.URI, + Diagnostics: diags, + }) +} + +func (s *Server) sendResponse(id any, result any) error { + msg := &JSONRPCMessage{ + ID: id, + Result: result, + } + return WriteMessage(s.writer, msg) +} + +func (s *Server) sendError(id any, code int, message string) error { + msg := &JSONRPCMessage{ + ID: id, + Error: &JSONRPCError{ + Code: code, + Message: message, + }, + } + return WriteMessage(s.writer, msg) +} + +func (s *Server) sendNotification(method string, params any) error { + raw, err := json.Marshal(params) + if err != nil { + return fmt.Errorf("marshaling notification params: %w", err) + } + msg := &JSONRPCMessage{ + Method: method, + Params: raw, + } + return WriteMessage(s.writer, msg) +} diff --git a/pkg/lsp/transport.go b/pkg/lsp/transport.go new file mode 100644 index 00000000000..a0d4d17ffff --- /dev/null +++ b/pkg/lsp/transport.go @@ -0,0 +1,88 @@ +package lsp + +import ( + "bufio" + "encoding/json" + "fmt" + "io" + "strconv" + "strings" +) + +// JSONRPCMessage is a JSON-RPC 2.0 message. +type JSONRPCMessage struct { + JSONRPC string `json:"jsonrpc"` + ID any `json:"id,omitempty"` + Method string `json:"method,omitempty"` + Params json.RawMessage `json:"params,omitempty"` + Result any `json:"result,omitempty"` + Error *JSONRPCError `json:"error,omitempty"` +} + +// JSONRPCError represents a JSON-RPC error. +type JSONRPCError struct { + Code int `json:"code"` + Message string `json:"message"` +} + +// ReadMessage reads a single LSP message from the reader using Content-Length framing. +func ReadMessage(reader *bufio.Reader) (*JSONRPCMessage, error) { + var contentLength int + + // Read headers + for { + line, err := reader.ReadString('\n') + if err != nil { + return nil, fmt.Errorf("reading header: %w", err) + } + + line = strings.TrimSpace(line) + if line == "" { + break // End of headers + } + + if strings.HasPrefix(line, "Content-Length: ") { + val := strings.TrimPrefix(line, "Content-Length: ") + contentLength, err = strconv.Atoi(val) + if err != nil { + return nil, fmt.Errorf("invalid Content-Length: %w", err) + } + } + } + + if contentLength == 0 { + return nil, fmt.Errorf("missing Content-Length header") + } + + // Read body + body := make([]byte, contentLength) + if _, err := io.ReadFull(reader, body); err != nil { + return nil, fmt.Errorf("reading body: %w", err) + } + + var msg JSONRPCMessage + if err := json.Unmarshal(body, &msg); err != nil { + return nil, fmt.Errorf("unmarshaling message: %w", err) + } + + return &msg, nil +} + +// WriteMessage writes an LSP message to the writer with Content-Length framing. +func WriteMessage(writer io.Writer, msg *JSONRPCMessage) error { + msg.JSONRPC = "2.0" + body, err := json.Marshal(msg) + if err != nil { + return fmt.Errorf("marshaling message: %w", err) + } + + header := fmt.Sprintf("Content-Length: %d\r\n\r\n", len(body)) + if _, err := io.WriteString(writer, header); err != nil { + return fmt.Errorf("writing header: %w", err) + } + if _, err := writer.Write(body); err != nil { + return fmt.Errorf("writing body: %w", err) + } + + return nil +} From a802b2aea0e2ad8a2ee0274eb615f4c5a51bff7e Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Feb 2026 15:32:28 +0000 Subject: [PATCH 3/5] fix: address lint issues in LSP tests and schema provider - Fix staticcheck: use tagged switch instead of if-else chain - Fix testifylint: use require for error assertions, avoid float comparisons - Fix schema provider to preserve local descriptions when following $ref - Fix diagnostics to validate empty frontmatter against schema - Fix multiple frontmatter detection test Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- go.mod | 2 +- pkg/lsp/completion_test.go | 136 +++++++++++++++++++ pkg/lsp/diagnostics.go | 13 +- pkg/lsp/diagnostics_test.go | 126 ++++++++++++++++++ pkg/lsp/document_test.go | 130 ++++++++++++++++++ pkg/lsp/hover_test.go | 93 +++++++++++++ pkg/lsp/protocol.go | 2 +- pkg/lsp/schema.go | 16 ++- pkg/lsp/schema_test.go | 151 +++++++++++++++++++++ pkg/lsp/server_test.go | 258 ++++++++++++++++++++++++++++++++++++ pkg/lsp/transport_test.go | 78 +++++++++++ 11 files changed, 994 insertions(+), 11 deletions(-) create mode 100644 pkg/lsp/completion_test.go create mode 100644 pkg/lsp/diagnostics_test.go create mode 100644 pkg/lsp/document_test.go create mode 100644 pkg/lsp/hover_test.go create mode 100644 pkg/lsp/schema_test.go create mode 100644 pkg/lsp/server_test.go create mode 100644 pkg/lsp/transport_test.go diff --git a/go.mod b/go.mod index 55b9d417e7b..9382ea3aa19 100644 --- a/go.mod +++ b/go.mod @@ -20,6 +20,7 @@ require ( github.com/sourcegraph/conc v0.3.0 github.com/spf13/cobra v1.10.2 github.com/stretchr/testify v1.11.1 + go.yaml.in/yaml/v3 v3.0.4 golang.org/x/crypto v0.48.0 golang.org/x/mod v0.33.0 golang.org/x/term v0.40.0 @@ -101,7 +102,6 @@ require ( go.opentelemetry.io/otel/metric v1.37.0 // indirect go.opentelemetry.io/otel/trace v1.37.0 // indirect go.uber.org/multierr v1.11.0 // indirect - go.yaml.in/yaml/v3 v3.0.4 // indirect go.yaml.in/yaml/v4 v4.0.0-rc.4 // indirect golang.org/x/exp v0.0.0-20240909161429-701f63a606c0 // indirect golang.org/x/exp/typeparams v0.0.0-20251023183803-a4bb9ffd2546 // indirect diff --git a/pkg/lsp/completion_test.go b/pkg/lsp/completion_test.go new file mode 100644 index 00000000000..b8ba5cd9925 --- /dev/null +++ b/pkg/lsp/completion_test.go @@ -0,0 +1,136 @@ +//go:build !integration + +package lsp + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestHandleCompletion_TopLevel(t *testing.T) { + sp, err := NewSchemaProvider() + require.NoError(t, err, "NewSchemaProvider should succeed") + + snap := newSnapshot(DocumentURI("file:///test.md"), 1, + "---\non:\n issues:\n types: [opened]\nengine: copilot\n---\n# Title") + + // Completion at top-level (line 4 = "engine: copilot") + list := HandleCompletion(snap, Position{Line: 4, Character: 0}, sp) + require.NotNil(t, list, "should return completion list") + assert.NotEmpty(t, list.Items, "should have completion items") + + // Should include top-level property or enum suggestions + snippets + hasEngine := false + for _, item := range list.Items { + if item.Label == "engine" { + hasEngine = true + break + } + } + assert.True(t, hasEngine, "should include 'engine' in completions") +} + +func TestHandleCompletion_NestedUnderOn(t *testing.T) { + sp, err := NewSchemaProvider() + require.NoError(t, err, "NewSchemaProvider should succeed") + + snap := newSnapshot(DocumentURI("file:///test.md"), 1, + "---\non:\n issues:\n types: [opened]\nengine: copilot\n---\n# Title") + + // Completion inside "on:" block (line 2 = " issues:") + list := HandleCompletion(snap, Position{Line: 2, Character: 2}, sp) + require.NotNil(t, list, "should return completion list") + assert.NotEmpty(t, list.Items, "should have nested completion items") + + // Should include event trigger types + labels := make(map[string]bool) + for _, item := range list.Items { + labels[item.Label] = true + } + assert.True(t, labels["issues"], "should suggest 'issues'") + assert.True(t, labels["pull_request"], "should suggest 'pull_request'") +} + +func TestHandleCompletion_NoFrontmatter(t *testing.T) { + sp, err := NewSchemaProvider() + require.NoError(t, err, "NewSchemaProvider should succeed") + + snap := newSnapshot(DocumentURI("file:///test.md"), 1, "# Just Markdown") + + list := HandleCompletion(snap, Position{Line: 0, Character: 0}, sp) + require.NotNil(t, list, "should return completion list") + + // Should suggest snippets + hasSnippet := false + for _, item := range list.Items { + if item.Kind == CompletionItemKindSnippet { + hasSnippet = true + break + } + } + assert.True(t, hasSnippet, "should include snippet completions when no frontmatter") +} + +func TestHandleCompletion_OutsideFrontmatter(t *testing.T) { + sp, err := NewSchemaProvider() + require.NoError(t, err, "NewSchemaProvider should succeed") + + snap := newSnapshot(DocumentURI("file:///test.md"), 1, + "---\non: issues\n---\n# Title") + + // Completion outside frontmatter (line 3 = "# Title") + list := HandleCompletion(snap, Position{Line: 3, Character: 0}, sp) + require.NotNil(t, list, "should return completion list") + assert.Empty(t, list.Items, "should have no items outside frontmatter") +} + +func TestHandleCompletion_NilSnapshot(t *testing.T) { + sp, err := NewSchemaProvider() + require.NoError(t, err, "NewSchemaProvider should succeed") + + list := HandleCompletion(nil, Position{Line: 0, Character: 0}, sp) + require.NotNil(t, list, "should return empty completion list for nil snapshot") + assert.Empty(t, list.Items, "should have no items for nil snapshot") +} + +func TestSnippetCompletions(t *testing.T) { + snippets := snippetCompletions() + assert.NotEmpty(t, snippets, "should have snippet completions") + + for _, s := range snippets { + assert.Equal(t, CompletionItemKindSnippet, s.Kind, "all snippets should have snippet kind") + assert.Equal(t, InsertTextFormatSnippet, s.InsertTextFormat, "all snippets should use snippet format") + assert.NotEmpty(t, s.InsertText, "all snippets should have insert text") + } +} + +func TestFilterCompletions(t *testing.T) { + items := []CompletionItem{ + {Label: "engine"}, + {Label: "env"}, + {Label: "on"}, + {Label: "imports"}, + } + + filtered := filterCompletions(items, "en") + assert.Len(t, filtered, 2, "should filter to items starting with 'en'") + + labels := make([]string, len(filtered)) + for i, item := range filtered { + labels[i] = item.Label + } + assert.Contains(t, labels, "engine", "should include 'engine'") + assert.Contains(t, labels, "env", "should include 'env'") +} + +func TestFilterCompletions_EmptyPrefix(t *testing.T) { + items := []CompletionItem{ + {Label: "engine"}, + {Label: "on"}, + } + + filtered := filterCompletions(items, "") + assert.Len(t, filtered, len(items), "empty prefix should return all items") +} diff --git a/pkg/lsp/diagnostics.go b/pkg/lsp/diagnostics.go index 37b49914f4b..4ee56ea4ac9 100644 --- a/pkg/lsp/diagnostics.go +++ b/pkg/lsp/diagnostics.go @@ -125,14 +125,13 @@ func extractYAMLErrorLine(errMsg string) int { // checkSchemaValidation validates frontmatter against the JSON schema and returns diagnostics. func checkSchemaValidation(snap *DocumentSnapshot) []Diagnostic { - if snap.FrontmatterYAML == "" { - return nil - } - - // Parse the YAML into a map var frontmatter map[string]any - if err := goyaml.Unmarshal([]byte(snap.FrontmatterYAML), &frontmatter); err != nil { - return nil // YAML errors already reported by checkYAMLSyntax + + if snap.FrontmatterYAML != "" { + // Parse the YAML into a map + if err := goyaml.Unmarshal([]byte(snap.FrontmatterYAML), &frontmatter); err != nil { + return nil // YAML errors already reported by checkYAMLSyntax + } } if frontmatter == nil { diff --git a/pkg/lsp/diagnostics_test.go b/pkg/lsp/diagnostics_test.go new file mode 100644 index 00000000000..a3943677f88 --- /dev/null +++ b/pkg/lsp/diagnostics_test.go @@ -0,0 +1,126 @@ +//go:build !integration + +package lsp + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestComputeDiagnostics_ValidDocument(t *testing.T) { + snap := newSnapshot(DocumentURI("file:///test.md"), 1, + "---\non:\n issues:\n types: [opened]\nengine: copilot\n---\n# Title") + + diags := ComputeDiagnostics(snap) + assert.Empty(t, diags, "valid document should produce no diagnostics") +} + +func TestComputeDiagnostics_MissingFrontmatter(t *testing.T) { + snap := newSnapshot(DocumentURI("file:///test.md"), 1, "# Just Markdown") + + diags := ComputeDiagnostics(snap) + require.Len(t, diags, 1, "should produce one diagnostic") + assert.Equal(t, SeverityWarning, diags[0].Severity, "should be a warning") + assert.Contains(t, diags[0].Message, "missing frontmatter", "message should mention missing frontmatter") +} + +func TestComputeDiagnostics_YAMLSyntaxError(t *testing.T) { + // Invalid YAML indentation + snap := newSnapshot(DocumentURI("file:///test.md"), 1, + "---\non:\n issues:\n types: [opened\n---\n# Title") + + diags := ComputeDiagnostics(snap) + require.NotEmpty(t, diags, "should produce diagnostics for YAML error") + assert.Equal(t, SeverityError, diags[0].Severity, "should be an error") + assert.Contains(t, diags[0].Message, "YAML syntax error", "message should mention YAML syntax error") +} + +func TestComputeDiagnostics_MissingRequiredField(t *testing.T) { + // Missing required "on" field + snap := newSnapshot(DocumentURI("file:///test.md"), 1, + "---\nengine: copilot\n---\n# Title") + + diags := ComputeDiagnostics(snap) + require.NotEmpty(t, diags, "should produce diagnostics for missing 'on'") + assert.Equal(t, SeverityError, diags[0].Severity, "should be an error") + assert.Contains(t, diags[0].Message, "on", "message should mention 'on'") +} + +func TestComputeDiagnostics_MultipleFrontmatterBlocks(t *testing.T) { + snap := newSnapshot(DocumentURI("file:///test.md"), 1, + "---\non:\n issues:\n types: [opened]\n---\n# Title\n---\nmore stuff\n---") + + diags := ComputeDiagnostics(snap) + // Should have a warning about multiple frontmatter blocks + hasMultipleWarning := false + for _, d := range diags { + if d.Severity == SeverityWarning && strings.Contains(d.Message, "Multiple frontmatter") { + hasMultipleWarning = true + break + } + } + assert.True(t, hasMultipleWarning, "should warn about multiple frontmatter blocks") +} + +func TestComputeDiagnostics_EmptyFrontmatter(t *testing.T) { + snap := newSnapshot(DocumentURI("file:///test.md"), 1, "---\n---\n# Title") + + diags := ComputeDiagnostics(snap) + // Should produce a diagnostic for missing "on" field + require.NotEmpty(t, diags, "empty frontmatter should produce diagnostics for missing 'on'") +} + +func TestCleanSchemaErrorMessage(t *testing.T) { + tests := []struct { + name string + input string + expected string + }{ + { + name: "missing property", + input: "'http://contoso.com/main-workflow-schema.json#'\n- at '': missing property 'on'", + expected: "missing property 'on'", + }, + { + name: "simple error passthrough", + input: "invalid value for engine", + expected: "invalid value for engine", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := cleanSchemaErrorMessage(tt.input) + assert.Equal(t, tt.expected, result, "cleaned message should match") + }) + } +} + +func TestExtractYAMLErrorLine(t *testing.T) { + tests := []struct { + name string + errMsg string + expected int + }{ + { + name: "line number present", + errMsg: "yaml: line 3: could not find expected ':'", + expected: 3, + }, + { + name: "no line number", + errMsg: "yaml: unexpected end of stream", + expected: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := extractYAMLErrorLine(tt.errMsg) + assert.Equal(t, tt.expected, result, "extracted line should match") + }) + } +} diff --git a/pkg/lsp/document_test.go b/pkg/lsp/document_test.go new file mode 100644 index 00000000000..911e241cad5 --- /dev/null +++ b/pkg/lsp/document_test.go @@ -0,0 +1,130 @@ +//go:build !integration + +package lsp + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewDocumentStore(t *testing.T) { + store := NewDocumentStore() + require.NotNil(t, store, "document store should be created") +} + +func TestDocumentStore_OpenAndGet(t *testing.T) { + store := NewDocumentStore() + uri := DocumentURI("file:///test.md") + + snap := store.Open(uri, 1, "---\non: issues\n---\n# Test") + require.NotNil(t, snap, "snapshot should be created on open") + assert.Equal(t, uri, snap.URI, "URI should match") + assert.Equal(t, 1, snap.Version, "version should match") + + got := store.Get(uri) + require.NotNil(t, got, "should retrieve opened document") + assert.Equal(t, snap, got, "retrieved snapshot should match opened snapshot") +} + +func TestDocumentStore_GetNonExistent(t *testing.T) { + store := NewDocumentStore() + got := store.Get(DocumentURI("file:///nonexistent.md")) + assert.Nil(t, got, "should return nil for non-existent document") +} + +func TestDocumentStore_Close(t *testing.T) { + store := NewDocumentStore() + uri := DocumentURI("file:///test.md") + + store.Open(uri, 1, "---\non: issues\n---") + store.Close(uri) + + got := store.Get(uri) + assert.Nil(t, got, "should return nil after close") +} + +func TestDocumentStore_Update(t *testing.T) { + store := NewDocumentStore() + uri := DocumentURI("file:///test.md") + + store.Open(uri, 1, "---\non: issues\n---") + snap := store.Update(uri, 2, "---\non: pull_request\n---") + + assert.Equal(t, 2, snap.Version, "version should be updated") + assert.Contains(t, snap.Text, "pull_request", "text should be updated") +} + +func TestParseFrontmatterRegion(t *testing.T) { + tests := []struct { + name string + text string + hasFrontmatter bool + frontmatterStart int + frontmatterEnd int + frontmatterYAML string + }{ + { + name: "valid frontmatter", + text: "---\non: issues\nengine: copilot\n---\n# Title", + hasFrontmatter: true, + frontmatterStart: 0, + frontmatterEnd: 3, + frontmatterYAML: "on: issues\nengine: copilot", + }, + { + name: "no frontmatter", + text: "# Just Markdown\nSome content", + hasFrontmatter: false, + }, + { + name: "unclosed frontmatter", + text: "---\non: issues\nengine: copilot", + hasFrontmatter: false, + }, + { + name: "empty frontmatter", + text: "---\n---\n# Title", + hasFrontmatter: true, + frontmatterStart: 0, + frontmatterEnd: 1, + frontmatterYAML: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + snap := newSnapshot(DocumentURI("file:///test.md"), 1, tt.text) + assert.Equal(t, tt.hasFrontmatter, snap.HasFrontmatter, "HasFrontmatter should match") + if tt.hasFrontmatter { + assert.Equal(t, tt.frontmatterStart, snap.FrontmatterStartLine, "FrontmatterStartLine should match") + assert.Equal(t, tt.frontmatterEnd, snap.FrontmatterEndLine, "FrontmatterEndLine should match") + assert.Equal(t, tt.frontmatterYAML, snap.FrontmatterYAML, "FrontmatterYAML should match") + } + }) + } +} + +func TestPositionInFrontmatter(t *testing.T) { + snap := newSnapshot(DocumentURI("file:///test.md"), 1, "---\non: issues\nengine: copilot\n---\n# Title") + + tests := []struct { + name string + position Position + inside bool + }{ + {"on opening delimiter", Position{Line: 0, Character: 0}, false}, + {"first content line", Position{Line: 1, Character: 0}, true}, + {"second content line", Position{Line: 2, Character: 5}, true}, + {"on closing delimiter", Position{Line: 3, Character: 0}, false}, + {"after frontmatter", Position{Line: 4, Character: 0}, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := snap.PositionInFrontmatter(tt.position) + assert.Equal(t, tt.inside, result, "PositionInFrontmatter should match for %s", tt.name) + }) + } +} diff --git a/pkg/lsp/hover_test.go b/pkg/lsp/hover_test.go new file mode 100644 index 00000000000..ca6153d020a --- /dev/null +++ b/pkg/lsp/hover_test.go @@ -0,0 +1,93 @@ +//go:build !integration + +package lsp + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestHandleHover_InsideFrontmatter(t *testing.T) { + sp, err := NewSchemaProvider() + require.NoError(t, err, "NewSchemaProvider should succeed") + + snap := newSnapshot(DocumentURI("file:///test.md"), 1, + "---\non:\n issues:\n types: [opened]\nengine: copilot\n---\n# Title") + + // Hover on "engine" (line 4 in document) + hover := HandleHover(snap, Position{Line: 4, Character: 2}, sp) + require.NotNil(t, hover, "should return hover for 'engine' key") + assert.Equal(t, "markdown", hover.Contents.Kind, "hover should be markdown") + assert.Contains(t, hover.Contents.Value, "engine", "hover should mention engine") +} + +func TestHandleHover_OutsideFrontmatter(t *testing.T) { + sp, err := NewSchemaProvider() + require.NoError(t, err, "NewSchemaProvider should succeed") + + snap := newSnapshot(DocumentURI("file:///test.md"), 1, + "---\non: issues\n---\n# Title") + + // Hover on markdown content (line 3) + hover := HandleHover(snap, Position{Line: 3, Character: 0}, sp) + assert.Nil(t, hover, "should return nil for position outside frontmatter") +} + +func TestHandleHover_NoFrontmatter(t *testing.T) { + sp, err := NewSchemaProvider() + require.NoError(t, err, "NewSchemaProvider should succeed") + + snap := newSnapshot(DocumentURI("file:///test.md"), 1, "# Just Markdown") + + hover := HandleHover(snap, Position{Line: 0, Character: 0}, sp) + assert.Nil(t, hover, "should return nil when no frontmatter") +} + +func TestHandleHover_NilSnapshot(t *testing.T) { + sp, err := NewSchemaProvider() + require.NoError(t, err, "NewSchemaProvider should succeed") + + hover := HandleHover(nil, Position{Line: 0, Character: 0}, sp) + assert.Nil(t, hover, "should return nil for nil snapshot") +} + +func TestFormatHoverContent(t *testing.T) { + info := &PropertyInfo{ + Name: "engine", + Description: "AI engine configuration", + Type: "string", + Default: "copilot", + Required: false, + Enum: []string{"copilot", "claude", "codex"}, + } + + result := formatHoverContent(info) + assert.Contains(t, result, "### `engine`", "should contain header") + assert.Contains(t, result, "AI engine configuration", "should contain description") + assert.Contains(t, result, "`string`", "should contain type") + assert.Contains(t, result, "`copilot`", "should contain default") + assert.Contains(t, result, "`claude`", "should contain enum value") +} + +func TestFormatHoverContent_Deprecated(t *testing.T) { + info := &PropertyInfo{ + Name: "infer", + Description: "Deprecated field", + Deprecated: true, + } + + result := formatHoverContent(info) + assert.Contains(t, result, "Deprecated", "should show deprecated warning") +} + +func TestFormatHoverContent_Required(t *testing.T) { + info := &PropertyInfo{ + Name: "on", + Required: true, + } + + result := formatHoverContent(info) + assert.Contains(t, result, "Required", "should show required status") +} diff --git a/pkg/lsp/protocol.go b/pkg/lsp/protocol.go index 0598e1c7278..821878361ed 100644 --- a/pkg/lsp/protocol.go +++ b/pkg/lsp/protocol.go @@ -133,7 +133,7 @@ type CompletionItem struct { Detail string `json:"detail,omitempty"` Documentation *MarkupContent `json:"documentation,omitempty"` InsertText string `json:"insertText,omitempty"` - InsertTextFormat InsertTextFormat `json:"insertTextFormat,omitempty"` + InsertTextFormat InsertTextFormat `json:"insertTextFormat,omitempty"` SortText string `json:"sortText,omitempty"` Deprecated bool `json:"deprecated,omitempty"` } diff --git a/pkg/lsp/schema.go b/pkg/lsp/schema.go index 33612819067..8a65e490141 100644 --- a/pkg/lsp/schema.go +++ b/pkg/lsp/schema.go @@ -79,10 +79,21 @@ func (sp *SchemaProvider) PropertyDescription(path []string) *PropertyInfo { return nil } + // Preserve local description and default before resolving $ref + localDesc, _ := propDef["description"].(string) + localDefault, hasLocalDefault := propDef["default"] + // Follow $ref if present propDef = sp.resolveRef(propDef) info := sp.extractPropertyInfo(key, propDef) + // Use local description if available (more specific than ref target) + if localDesc != "" { + info.Description = localDesc + } + if hasLocalDefault { + info.Default = fmt.Sprintf("%v", localDefault) + } return &info } @@ -333,14 +344,15 @@ func findPathInMapping(mapping *goyaml.Node, targetLine int, parentPath []string } // Check if the target line falls within this value's range - if valNode.Kind == goyaml.MappingNode { + switch valNode.Kind { + case goyaml.MappingNode: // Determine the end line of this mapping value valEndLine := nodeEndLine(valNode) if targetLine > keyLine && targetLine <= valEndLine { newPath := append(append([]string{}, parentPath...), keyName) return findPathInMapping(valNode, targetLine, newPath) } - } else if valNode.Kind == goyaml.SequenceNode { + case goyaml.SequenceNode: valEndLine := nodeEndLine(valNode) if targetLine > keyLine && targetLine <= valEndLine { return append(append([]string{}, parentPath...), keyName), "" diff --git a/pkg/lsp/schema_test.go b/pkg/lsp/schema_test.go new file mode 100644 index 00000000000..78136a8164e --- /dev/null +++ b/pkg/lsp/schema_test.go @@ -0,0 +1,151 @@ +//go:build !integration + +package lsp + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestNewSchemaProvider(t *testing.T) { + sp, err := NewSchemaProvider() + require.NoError(t, err, "NewSchemaProvider should succeed") + require.NotNil(t, sp, "schema provider should not be nil") +} + +func TestSchemaProvider_TopLevelProperties(t *testing.T) { + sp, err := NewSchemaProvider() + require.NoError(t, err, "NewSchemaProvider should succeed") + + props := sp.TopLevelProperties() + assert.NotEmpty(t, props, "should return top-level properties") + + // Check for required well-known properties + names := make(map[string]bool) + for _, p := range props { + names[p.Name] = true + } + + assert.True(t, names["on"], "should contain 'on' property") + assert.True(t, names["engine"], "should contain 'engine' property") + assert.True(t, names["imports"], "should contain 'imports' property") + assert.True(t, names["tools"], "should contain 'tools' property") + assert.True(t, names["safe-outputs"], "should contain 'safe-outputs' property") +} + +func TestSchemaProvider_NestedProperties_On(t *testing.T) { + sp, err := NewSchemaProvider() + require.NoError(t, err, "NewSchemaProvider should succeed") + + props := sp.NestedProperties([]string{"on"}) + assert.NotEmpty(t, props, "should return nested properties for 'on'") + + names := make(map[string]bool) + for _, p := range props { + names[p.Name] = true + } + + assert.True(t, names["issues"], "should contain 'issues' trigger") + assert.True(t, names["pull_request"], "should contain 'pull_request' trigger") + assert.True(t, names["slash_command"], "should contain 'slash_command' trigger") +} + +func TestSchemaProvider_PropertyDescription(t *testing.T) { + sp, err := NewSchemaProvider() + require.NoError(t, err, "NewSchemaProvider should succeed") + + info := sp.PropertyDescription([]string{"on"}) + require.NotNil(t, info, "should return property info for 'on'") + assert.Equal(t, "on", info.Name, "name should be 'on'") + assert.NotEmpty(t, info.Description, "description should not be empty") + + info = sp.PropertyDescription([]string{"engine"}) + require.NotNil(t, info, "should return property info for 'engine'") + assert.NotEmpty(t, info.Description, "engine description should not be empty") +} + +func TestSchemaProvider_PropertyDescription_NotFound(t *testing.T) { + sp, err := NewSchemaProvider() + require.NoError(t, err, "NewSchemaProvider should succeed") + + info := sp.PropertyDescription([]string{"nonexistent-property"}) + assert.Nil(t, info, "should return nil for unknown property") +} + +func TestSchemaProvider_EnumValues(t *testing.T) { + sp, err := NewSchemaProvider() + require.NoError(t, err, "NewSchemaProvider should succeed") + + enums := sp.EnumValues([]string{"engine"}) + assert.NotEmpty(t, enums, "should return enum values for 'engine'") + assert.Contains(t, enums, "copilot", "should contain 'copilot'") + assert.Contains(t, enums, "claude", "should contain 'claude'") +} + +func TestYAMLPathAtPosition(t *testing.T) { + tests := []struct { + name string + yaml string + line int + wantPath []string + wantCurrent string + }{ + { + name: "top-level key", + yaml: "on:\n issues:\n types: [opened]\nengine: copilot", + line: 0, + wantPath: nil, + wantCurrent: "on", + }, + { + name: "nested key under on", + yaml: "on:\n issues:\n types: [opened]\nengine: copilot", + line: 1, + wantPath: []string{"on"}, + wantCurrent: "issues", + }, + { + name: "deeply nested key", + yaml: "on:\n issues:\n types: [opened]\nengine: copilot", + line: 2, + wantPath: []string{"on", "issues"}, + wantCurrent: "types", + }, + { + name: "second top-level key", + yaml: "on:\n issues:\n types: [opened]\nengine: copilot", + line: 3, + wantPath: nil, + wantCurrent: "engine", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + path, current := YAMLPathAtPosition(tt.yaml, tt.line) + if tt.wantPath == nil { + assert.Empty(t, path, "path should be empty for %s", tt.name) + } else { + assert.Equal(t, tt.wantPath, path, "path should match for %s", tt.name) + } + assert.Equal(t, tt.wantCurrent, current, "current key should match for %s", tt.name) + }) + } +} + +func TestYAMLPathAtPosition_EmptyContent(t *testing.T) { + path, key := YAMLPathAtPosition("", 0) + assert.Nil(t, path, "path should be nil for empty content") + assert.Empty(t, key, "key should be empty for empty content") +} + +func TestYAMLPathAtPosition_InvalidYAML(t *testing.T) { + // Invalid YAML should fallback to indentation heuristic + yaml := "on:\n issues\n types: [opened" + path, key := YAMLPathAtPosition(yaml, 0) + // Should still work via fallback + assert.Equal(t, "on", key, "should detect key via fallback") + assert.Empty(t, path, "path should be empty at top level") +} diff --git a/pkg/lsp/server_test.go b/pkg/lsp/server_test.go new file mode 100644 index 00000000000..675adce6d74 --- /dev/null +++ b/pkg/lsp/server_test.go @@ -0,0 +1,258 @@ +//go:build !integration + +package lsp + +import ( + "bytes" + "encoding/json" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestServer_InitializeShutdownExit(t *testing.T) { + input := "" + input += buildLSPMsg(t, map[string]any{ + "jsonrpc": "2.0", "id": 1, "method": "initialize", + "params": map[string]any{"processId": nil, "rootUri": "file:///test"}, + }) + input += buildLSPMsg(t, map[string]any{ + "jsonrpc": "2.0", "method": "initialized", "params": map[string]any{}, + }) + input += buildLSPMsg(t, map[string]any{ + "jsonrpc": "2.0", "id": 2, "method": "shutdown", + }) + input += buildLSPMsg(t, map[string]any{ + "jsonrpc": "2.0", "method": "exit", + }) + + var stdout bytes.Buffer + server, err := NewServer(strings.NewReader(input), &stdout, &bytes.Buffer{}) + require.NoError(t, err, "NewServer should succeed") + + err = server.Run() + require.NoError(t, err, "server should exit cleanly") + + // Parse responses + responses := parseLSPMessages(t, stdout.Bytes()) + require.GreaterOrEqual(t, len(responses), 2, "should have at least 2 responses (initialize + shutdown)") + + // Check initialize response + initResp := responses[0] + result := initResp["result"].(map[string]any) + caps := result["capabilities"].(map[string]any) + assert.NotNil(t, caps["textDocumentSync"], "should have text document sync") + assert.Equal(t, true, caps["hoverProvider"], "should have hover provider") + assert.NotNil(t, caps["completionProvider"], "should have completion provider") +} + +func TestServer_DidOpenPublishesDiagnostics(t *testing.T) { + input := "" + input += buildLSPMsg(t, map[string]any{ + "jsonrpc": "2.0", "id": 1, "method": "initialize", + "params": map[string]any{"processId": nil}, + }) + input += buildLSPMsg(t, map[string]any{ + "jsonrpc": "2.0", "method": "initialized", "params": map[string]any{}, + }) + input += buildLSPMsg(t, map[string]any{ + "jsonrpc": "2.0", "method": "textDocument/didOpen", + "params": map[string]any{ + "textDocument": map[string]any{ + "uri": "file:///test.md", "languageId": "markdown", + "version": 1, "text": "---\nengine: copilot\n---\n# Title", + }, + }, + }) + input += buildLSPMsg(t, map[string]any{ + "jsonrpc": "2.0", "id": 99, "method": "shutdown", + }) + input += buildLSPMsg(t, map[string]any{ + "jsonrpc": "2.0", "method": "exit", + }) + + var stdout bytes.Buffer + server, err := NewServer(strings.NewReader(input), &stdout, &bytes.Buffer{}) + require.NoError(t, err, "NewServer should succeed") + + err = server.Run() + require.NoError(t, err, "server should exit cleanly") + + responses := parseLSPMessages(t, stdout.Bytes()) + + // Find the diagnostics notification + var diagnosticsFound bool + for _, resp := range responses { + if resp["method"] == "textDocument/publishDiagnostics" { + diagnosticsFound = true + params := resp["params"].(map[string]any) + assert.Equal(t, "file:///test.md", params["uri"], "diagnostics URI should match") + diags, ok := params["diagnostics"].([]any) + if ok && len(diags) > 0 { + // Should have error about missing 'on' + firstDiag := diags[0].(map[string]any) + assert.Contains(t, firstDiag["message"].(string), "on", "diagnostic should mention 'on'") + } + break + } + } + assert.True(t, diagnosticsFound, "should publish diagnostics on didOpen") +} + +func TestServer_HoverReturnsResult(t *testing.T) { + docText := "---\non:\n issues:\n types: [opened]\nengine: copilot\n---\n# Title" + + input := "" + input += buildLSPMsg(t, map[string]any{ + "jsonrpc": "2.0", "id": 1, "method": "initialize", + "params": map[string]any{"processId": nil}, + }) + input += buildLSPMsg(t, map[string]any{ + "jsonrpc": "2.0", "method": "initialized", "params": map[string]any{}, + }) + input += buildLSPMsg(t, map[string]any{ + "jsonrpc": "2.0", "method": "textDocument/didOpen", + "params": map[string]any{ + "textDocument": map[string]any{ + "uri": "file:///test.md", "languageId": "markdown", + "version": 1, "text": docText, + }, + }, + }) + input += buildLSPMsg(t, map[string]any{ + "jsonrpc": "2.0", "id": 2, "method": "textDocument/hover", + "params": map[string]any{ + "textDocument": map[string]any{"uri": "file:///test.md"}, + "position": map[string]any{"line": 4, "character": 2}, + }, + }) + input += buildLSPMsg(t, map[string]any{ + "jsonrpc": "2.0", "id": 99, "method": "shutdown", + }) + input += buildLSPMsg(t, map[string]any{ + "jsonrpc": "2.0", "method": "exit", + }) + + var stdout bytes.Buffer + server, err := NewServer(strings.NewReader(input), &stdout, &bytes.Buffer{}) + require.NoError(t, err, "NewServer should succeed") + + err = server.Run() + require.NoError(t, err, "server should exit cleanly") + + responses := parseLSPMessages(t, stdout.Bytes()) + + // Find hover response (id=2) + var hoverResp map[string]any + for _, resp := range responses { + id, ok := resp["id"] + if ok && id != nil { + idNum, isNum := id.(float64) + if isNum && int(idNum) == 2 { + hoverResp = resp + break + } + } + } + require.NotNil(t, hoverResp, "should have hover response") + result := hoverResp["result"].(map[string]any) + contents := result["contents"].(map[string]any) + assert.Equal(t, "markdown", contents["kind"], "hover should be markdown") + assert.Contains(t, contents["value"].(string), "engine", "hover should mention engine") +} + +func TestServer_UnknownMethodReturnsError(t *testing.T) { + input := "" + input += buildLSPMsg(t, map[string]any{ + "jsonrpc": "2.0", "id": 1, "method": "initialize", + "params": map[string]any{"processId": nil}, + }) + input += buildLSPMsg(t, map[string]any{ + "jsonrpc": "2.0", "id": 2, "method": "textDocument/unknownMethod", + }) + input += buildLSPMsg(t, map[string]any{ + "jsonrpc": "2.0", "id": 3, "method": "shutdown", + }) + input += buildLSPMsg(t, map[string]any{ + "jsonrpc": "2.0", "method": "exit", + }) + + var stdout bytes.Buffer + server, err := NewServer(strings.NewReader(input), &stdout, &bytes.Buffer{}) + require.NoError(t, err, "NewServer should succeed") + + err = server.Run() + require.NoError(t, err, "server should exit cleanly") + + responses := parseLSPMessages(t, stdout.Bytes()) + + // Find error response (id=2) + var errorResp map[string]any + for _, resp := range responses { + id, ok := resp["id"] + if ok && id != nil { + idNum, isNum := id.(float64) + if isNum && int(idNum) == 2 { + errorResp = resp + break + } + } + } + require.NotNil(t, errorResp, "should have error response for unknown method") + errObj := errorResp["error"].(map[string]any) + code, ok := errObj["code"].(float64) + require.True(t, ok, "error code should be a number") + assert.Equal(t, -32601, int(code), "should be method not found error") +} + +// Helper functions + +func buildLSPMsg(t *testing.T, msg map[string]any) string { + t.Helper() + body, err := json.Marshal(msg) + require.NoError(t, err, "marshaling test message") + return "Content-Length: " + json.Number(func() string { + b, _ := json.Marshal(len(body)) + return string(b) + }()).String() + "\r\n\r\n" + string(body) +} + +func parseLSPMessages(t *testing.T, data []byte) []map[string]any { + t.Helper() + var messages []map[string]any + pos := 0 + prefix := []byte("Content-Length: ") + + for pos < len(data) { + cl := bytes.Index(data[pos:], prefix) + if cl == -1 { + break + } + cl += pos + + hdrEnd := bytes.Index(data[cl:], []byte("\r\n\r\n")) + if hdrEnd == -1 { + break + } + hdrEnd += cl + + lengthStr := string(data[cl+len(prefix) : hdrEnd]) + var length int + err := json.Unmarshal([]byte(lengthStr), &length) + require.NoError(t, err, "parsing content length") + + bodyStart := hdrEnd + 4 + body := data[bodyStart : bodyStart+length] + + var msg map[string]any + err = json.Unmarshal(body, &msg) + require.NoError(t, err, "parsing response body") + + messages = append(messages, msg) + pos = bodyStart + length + } + + return messages +} diff --git a/pkg/lsp/transport_test.go b/pkg/lsp/transport_test.go new file mode 100644 index 00000000000..956d768df08 --- /dev/null +++ b/pkg/lsp/transport_test.go @@ -0,0 +1,78 @@ +//go:build !integration + +package lsp + +import ( + "bufio" + "bytes" + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestReadWriteMessage(t *testing.T) { + msg := &JSONRPCMessage{ + JSONRPC: "2.0", + ID: float64(1), + Method: "initialize", + } + + // Write + var buf bytes.Buffer + err := WriteMessage(&buf, msg) + require.NoError(t, err, "WriteMessage should succeed") + + // Read back + reader := bufio.NewReader(&buf) + got, err := ReadMessage(reader) + require.NoError(t, err, "ReadMessage should succeed") + + assert.Equal(t, "2.0", got.JSONRPC, "JSONRPC version should match") + assert.NotNil(t, got.ID, "ID should not be nil") + assert.Equal(t, "initialize", got.Method, "method should match") +} + +func TestReadMessage_MissingContentLength(t *testing.T) { + input := "\r\n\r\n{}" + reader := bufio.NewReader(bytes.NewBufferString(input)) + _, err := ReadMessage(reader) + assert.Error(t, err, "should error on missing Content-Length") +} + +func TestWriteMessage_ContentLengthFraming(t *testing.T) { + msg := &JSONRPCMessage{ + ID: float64(1), + Method: "test", + } + + var buf bytes.Buffer + err := WriteMessage(&buf, msg) + require.NoError(t, err, "WriteMessage should succeed") + + output := buf.String() + assert.Contains(t, output, "Content-Length: ", "should contain Content-Length header") + assert.Contains(t, output, "\r\n\r\n", "should contain header/body separator") +} + +func TestReadWriteMessage_WithParams(t *testing.T) { + params, _ := json.Marshal(map[string]string{"key": "value"}) + msg := &JSONRPCMessage{ + ID: float64(42), + Method: "textDocument/hover", + Params: params, + } + + var buf bytes.Buffer + err := WriteMessage(&buf, msg) + require.NoError(t, err, "WriteMessage should succeed") + + reader := bufio.NewReader(&buf) + got, err := ReadMessage(reader) + require.NoError(t, err, "ReadMessage should succeed") + + assert.NotNil(t, got.ID, "ID should not be nil") + assert.Equal(t, "textDocument/hover", got.Method, "method should match") + assert.NotNil(t, got.Params, "params should not be nil") +} From bb31125cfc991b4e7a73cd75de0eb8dcb9b40041 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Feb 2026 15:34:14 +0000 Subject: [PATCH 4/5] fix: address code review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Simplify Content-Length header construction in server tests (use fmt.Sprintf) - Fix padIndex to handle values ≥100 correctly (use fmt.Sprintf with zero-padding) Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/lsp/completion.go | 6 ++---- pkg/lsp/server_test.go | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/pkg/lsp/completion.go b/pkg/lsp/completion.go index dad4172ea4c..4300c419afd 100644 --- a/pkg/lsp/completion.go +++ b/pkg/lsp/completion.go @@ -1,6 +1,7 @@ package lsp import ( + "fmt" "sort" "strings" @@ -232,8 +233,5 @@ func filterCompletions(items []CompletionItem, prefix string) []CompletionItem { } func padIndex(i int) string { - if i < 10 { - return "0" + string(rune('0'+i)) - } - return string(rune('0'+i/10)) + string(rune('0'+i%10)) + return fmt.Sprintf("%04d", i) } diff --git a/pkg/lsp/server_test.go b/pkg/lsp/server_test.go index 675adce6d74..9a480c851c6 100644 --- a/pkg/lsp/server_test.go +++ b/pkg/lsp/server_test.go @@ -5,6 +5,7 @@ package lsp import ( "bytes" "encoding/json" + "fmt" "strings" "testing" @@ -213,10 +214,7 @@ func buildLSPMsg(t *testing.T, msg map[string]any) string { t.Helper() body, err := json.Marshal(msg) require.NoError(t, err, "marshaling test message") - return "Content-Length: " + json.Number(func() string { - b, _ := json.Marshal(len(body)) - return string(b) - }()).String() + "\r\n\r\n" + string(body) + return fmt.Sprintf("Content-Length: %d\r\n\r\n%s", len(body), string(body)) } func parseLSPMessages(t *testing.T, data []byte) []map[string]any { From 714aa30ee3944c393d12d356597600c9d7205d73 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 20 Feb 2026 15:45:29 +0000 Subject: [PATCH 5/5] Changes before error encountered Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/lsp_command.go | 7 ++----- pkg/lsp/server.go | 22 ++++++++++++---------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/pkg/cli/lsp_command.go b/pkg/cli/lsp_command.go index 8853dee8474..c2047935cbf 100644 --- a/pkg/cli/lsp_command.go +++ b/pkg/cli/lsp_command.go @@ -28,16 +28,13 @@ The LSP server communicates over stdio using JSON-RPC 2.0 and provides: The server is stateless (session-only memory, no daemon, no disk state). Examples: - gh aw lsp --stdio # Start LSP server on stdio - gh aw lsp # Same as --stdio (default) - echo '...' | gh aw lsp --stdio # Pipe LSP messages`, + gh aw lsp # Start LSP server on stdio + echo '...' | gh aw lsp # Pipe LSP messages`, RunE: func(cmd *cobra.Command, args []string) error { return RunLSP() }, } - cmd.Flags().Bool("stdio", true, "Use stdio transport (default, only supported mode)") - return cmd } diff --git a/pkg/lsp/server.go b/pkg/lsp/server.go index 24ee3c5117c..4a6d44dedbf 100644 --- a/pkg/lsp/server.go +++ b/pkg/lsp/server.go @@ -17,25 +17,27 @@ type Server struct { writer io.Writer stderr io.Writer - docs *DocumentStore - schema *SchemaProvider + docs *DocumentStore + schema *SchemaProvider + version string shutdown bool } // NewServer creates a new LSP server. -func NewServer(stdin io.Reader, stdout io.Writer, stderr io.Writer) (*Server, error) { +func NewServer(stdin io.Reader, stdout io.Writer, stderr io.Writer, version string) (*Server, error) { sp, err := NewSchemaProvider() if err != nil { return nil, fmt.Errorf("initializing schema provider: %w", err) } return &Server{ - reader: bufio.NewReader(stdin), - writer: stdout, - stderr: stderr, - docs: NewDocumentStore(), - schema: sp, + reader: bufio.NewReader(stdin), + writer: stdout, + stderr: stderr, + docs: NewDocumentStore(), + schema: sp, + version: version, }, nil } @@ -105,8 +107,8 @@ func (s *Server) handleInitialize(msg *JSONRPCMessage) error { }, }, ServerInfo: &ServerInfo{ - Name: "gh-aw-lsp", - Version: "0.1.0", + Name: "gh-aw", + Version: s.version, }, }