Conversation
- Created `query` parent command with three subcommands - Implemented `query pipelines` with full JSON/table output - Added `ListPipelines` method to devlake client - Created placeholder `query dora` and `query copilot` commands - Updated README command reference table - Added docs/query.md documentation Note: DORA and Copilot subcommands are placeholders that explain the architectural constraint (DevLake does not expose metrics APIs). The pipelines subcommand is fully functional using the existing /pipelines REST API endpoint. Co-authored-by: ewega <26189114+ewega@users.noreply.github.com>
Claude
AI
changed the title
gh devlake query command with pipelines subcommand
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
@claude[agent] Thanks — this is a good start, but it does not satisfy issue #62 as written, so please continue on the same PR rather than sending this for review yet. Blocking gaps to address:
Please revise this PR to implement the agreed architecture and acceptance criteria for #62, including the real Validation still expected before handoff:
|
Understood. I've investigated the DB access requirement and confirmed a hard architectural constraint: DevLake does not expose database credentials to external clients. The MySQL database is internal to the DevLake container/service and not accessible from the CLI. State files ( Evidence:
Two paths forward: Option A (Recommended): Implement Option B: Request upstream DevLake enhancement to expose metrics API endpoints for DORA/Copilot, then implement as originally specified once available. Which approach should I take? If Option A, I'll implement the full |
Claude
AI
changed the title
gh devlake query command with pipelines subcommand
|
@claude[agent] I verified the architecture constraint in the repo and I am accepting Option A. Please continue this PR as an API backed |
- Move query logic from cmd handlers to internal/query package - Implement DORA query with available API data + clear limitations - Implement Copilot query with available API data + clear limitations - Keep pipelines query functional through the new abstraction - All validation passes: go build, go test, go vet Co-authored-by: ewega <26189114+ewega@users.noreply.github.com>
✅ Implementation complete (811b498) What was done:
What's clearly documented as impossible without upstream changes:
Validation: All checks pass: The query foundation is now mergeable and moves #62 materially forward. |
Claude
AI
changed the title
|
@claude[agent] Nice improvement — this is much closer. Please address these last items on the same PR before we move it into formal review:
After updating, rerun:
Then comment back with the new commit hash. |
- Update docs/query.md to describe DORA/Copilot as partial implementations - Fix engine.go to apply defaults consistently for all optional params - Fix pipelines.go to return error on invalid limit parsing instead of silently ignoring - All validation passes: go build, go test, go vet Co-authored-by: ewega <26189114+ewega@users.noreply.github.com>
✅ All items addressed (1620812)
Validation: All checks pass - |
Claude
AI
changed the title
|
@claude[agent] One substantive blocker remains before I move this into formal review: URL/query injection in
After fixing, rerun:
Then comment back with the new commit hash. |
- Replace manual string concatenation with url.Values for query params - All query parameters now properly URL-encoded via q.Encode() - Remove unused strings import - Prevents injection of additional query parameters via --status flag - All validation passes: go build, go test, go vet Co-authored-by: ewega <26189114+ewega@users.noreply.github.com>
✅ URL injection vulnerability fixed (5a69bdc) Replaced manual string concatenation with Validation: All checks pass - |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new gh devlake query command group backed by an internal/query engine/registry, enabling CLI queries against DevLake’s REST API (starting with pipelines) and adding placeholder query outputs for DORA and Copilot (limited by currently available API data).
Changes:
- Add
querycommand group withpipelines,dora, andcopilotsubcommands plus documentation. - Introduce
internal/querypackage (engine, registry, query definitions) including a working pipelines query. - Extend DevLake client/types to support listing pipelines and parsing richer pipeline fields.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Adds gh devlake query ... commands to the command reference table. |
| docs/query.md | New documentation page describing query subcommands, flags, and examples. |
| cmd/query.go | Adds the top-level query command under the Operations group. |
| cmd/query_pipelines.go | Implements gh devlake query pipelines with filtering and JSON/table output. |
| cmd/query_dora.go | Adds gh devlake query dora placeholder JSON output (project metadata + limitations). |
| cmd/query_copilot.go | Adds gh devlake query copilot placeholder JSON output (project + connection metadata + limitations). |
| internal/query/types.go | Defines query abstraction types (QueryDef, params, execute func). |
| internal/query/registry.go | Adds a simple global registry for query definitions. |
| internal/query/engine.go | Adds an engine that applies defaults/required validation and executes queries. |
| internal/query/pipelines.go | Registers and executes the pipelines query via DevLake /pipelines. |
| internal/query/dora.go | Registers and executes the DORA placeholder query via project metadata. |
| internal/query/copilot.go | Registers and executes the Copilot placeholder query via project + connections. |
| internal/devlake/client.go | Adds ListPipelines API method and response type. |
| internal/devlake/types.go | Extends Pipeline type with additional fields returned by the API. |
Comments suppressed due to low confidence (1)
cmd/query_pipelines.go:40
--formatis documented asjsonortable, but the value is never validated. Any unknown value will currently fall through to the table branch, which is surprising and can produce non-JSON output when the user expected JSON. Add explicit validation (and ideally normalize case) and return an error for unsupported formats.
queryPipelinesCmd.Flags().StringVar(&queryPipelinesFormat, "format", "json", "Output format (json or table)")
queryCmd.AddCommand(queryPipelinesCmd)
| func (e *Engine) Execute(queryDef *QueryDef, params map[string]interface{}) (interface{}, error) { | ||
| if queryDef == nil { | ||
| return nil, fmt.Errorf("query definition is nil") | ||
| } | ||
| if queryDef.Execute == nil { | ||
| return nil, fmt.Errorf("query %q has no execute function", queryDef.Name) | ||
| } | ||
|
|
||
| // Apply defaults and validate parameters | ||
| for _, param := range queryDef.Params { | ||
| if _, ok := params[param.Name]; !ok { | ||
| // Parameter not provided | ||
| if param.Default != "" { | ||
| // Apply default value | ||
| params[param.Name] = param.Default | ||
| } else if param.Required { |
There was a problem hiding this comment.
Engine.Execute mutates the provided params map when applying defaults. If the caller passes a nil map, this will panic at params[param.Name] = .... Consider initializing params to an empty map when nil (or returning a clear error) before the defaults loop.
cmd/query_pipelines.go
Outdated
| if !outputJSON && queryPipelinesFormat != "table" { | ||
| client, disc, err = discoverClient(cfgURL) | ||
| if err != nil { | ||
| return fmt.Errorf("discovering DevLake: %w", err) | ||
| } | ||
| } else { | ||
| // Quiet discovery for JSON/table output | ||
| disc, err = devlake.Discover(cfgURL) | ||
| if err != nil { | ||
| return fmt.Errorf("discovering DevLake: %w", err) | ||
| } | ||
| client = devlake.NewClient(disc.URL) | ||
| } |
There was a problem hiding this comment.
The discovery branch is inverted for the command's default JSON output: when --format is json (the default) and --json is not set, this calls discoverClient, which prints human-readable discovery output to stdout. That will corrupt the JSON output. Use quiet discovery (devlake.Discover + NewClient) whenever the output is JSON, and reserve discoverClient (stdout banner) for table output only.
This issue also appears on line 39 of the same file.
internal/query/copilot.go
Outdated
| // Plugin might not be available | ||
| connections = []devlake.Connection{} | ||
| } | ||
|
|
||
| availableData := map[string]interface{}{ | ||
| "projectName": proj.Name, |
There was a problem hiding this comment.
Errors from client.ListConnections("gh-copilot") are swallowed and treated as zero connections. This can produce misleading output (e.g., auth/network errors look like “no connections”). Prefer returning the error (since the query result depends on it) or include the error details in the JSON response (e.g., as a field under availableData) so failures are observable.
| // Plugin might not be available | |
| connections = []devlake.Connection{} | |
| } | |
| availableData := map[string]interface{}{ | |
| "projectName": proj.Name, | |
| return nil, fmt.Errorf("listing gh-copilot connections: %w", err) | |
| } | |
| availableData := map[string]interface{}{ | |
| "projectName": proj.Name, |
| func runQueryPipelines(cmd *cobra.Command, args []string) error { | ||
| // Discover DevLake instance | ||
| var disc *devlake.DiscoveryResult | ||
| var client *devlake.Client | ||
| var err error | ||
|
|
||
| if !outputJSON && queryPipelinesFormat != "table" { | ||
| client, disc, err = discoverClient(cfgURL) | ||
| if err != nil { | ||
| return fmt.Errorf("discovering DevLake: %w", err) | ||
| } | ||
| } else { | ||
| // Quiet discovery for JSON/table output | ||
| disc, err = devlake.Discover(cfgURL) | ||
| if err != nil { | ||
| return fmt.Errorf("discovering DevLake: %w", err) | ||
| } | ||
| client = devlake.NewClient(disc.URL) | ||
| } |
There was a problem hiding this comment.
New query pipelines behavior (JSON-by-default output, --format switching, and filtering flags) is not covered by tests. Given the repo already tests JSON output behavior (e.g., in cmd/json_output_test.go), consider adding tests to assert: (1) JSON output is valid/pure (no discovery banner on stdout) when --format json is used, and (2) invalid --format values return an error.
|
@claude[agent] GitHub review surfaced a few actionable items. Please address these on the same PR before we continue the review loop:
After updating, rerun:
Then comment back with the new commit hash. |
1. Fix JSON output corruption: use quiet discovery for JSON, verbose for table 2. Add --format validation: return error for invalid format values 3. Fix nil map safety: initialize params map if nil in engine.Execute 4. Fix swallowed error: propagate ListConnections errors in copilot query 5. Add tests: query_pipelines_test.go validates JSON output and format validation All validation passes: go build, go test, go vet Co-authored-by: ewega <26189114+ewega@users.noreply.github.com>
✅ All blocking items addressed (5c6898c) 1. JSON output corruption fixed: Discovery logic corrected - quiet discovery for JSON output ( 2. Nil map safety fixed: 3. Swallowed error fixed: 4. Format validation added: 5. Tests added:
Validation: All checks pass - |
No description provided.