feat(walk): add walk command for querying OpenAPI specs#304
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new oastools "walk" CLI with subcommands (operations, schemas, parameters, responses, security, paths), extension-filter parsing/matching, summary/detail rendering, walker collectors for parameters/responses/security schemes, CLI routing/flags, comprehensive tests, and design docs. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (user)
participant Walk as Walk.Handler
participant Parser as parser.ParseResult
participant Walker as walker.Collectors
participant Filter as ExtensionFilter/Matchers
participant Render as RenderSummaryTable/RenderDetail
CLI->>Walk: invoke subcommand + flags + spec path
Walk->>Parser: parseSpec(specPath, resolveRefs)
Parser-->>Walk: ParseResult
Walk->>Walker: Collect<entity>(ParseResult)
Walker-->>Walk: Collected items
Walk->>Filter: apply filters (method/path/status/name/type/extension)
Filter-->>Walk: filtered items
Walk->>Render: render summary or detail (format, quiet)
Render-->>CLI: output
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #304 +/- ##
==========================================
+ Coverage 84.57% 84.61% +0.03%
==========================================
Files 166 166
Lines 25119 25184 +65
==========================================
+ Hits 21245 21310 +65
Misses 2623 2623
Partials 1251 1251
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@cmd/oastools/commands/walk_operations_test.go`:
- Around line 58-429: Tests in walk_operations_test.go use manual
if/t.Fatalf/t.Errorf assertions; replace them with stretchr/testify's require
and assert (e.g., require.NoError, require.Len, assert.Equal) across all tests
such as TestWalkOperations_ListAll, TestWalkOperations_FilterByMethod,
TestWalkOperations_FilterByPath, TestWalkOperations_FilterByTag,
TestWalkOperations_FilterByDeprecated, TestWalkOperations_FilterByOperationID,
TestWalkOperations_FilterByExtension, TestWalkOperations_FilterByExtensionValue,
TestWalkOperations_FilterByExtensionInvalid, TestWalkOperations_CombinedFilters,
TestWalkOperations_SummaryTableOutput, TestWalkOperations_SummaryTableQuiet,
TestWalkOperations_DetailOutput, TestWalkOperations_DetailOutputYAML,
TestWalkOperations_NoArgsError, TestWalkOperations_InvalidFormat,
TestWalkOperations_MatchOperationMethod, and
TestWalkOperations_MatchOperationTag; add the testify import (require and
assert) to the test file, replace error checks with require.NoError(t, err),
replace expected length checks with require.Len/require.Equal, and use
assert.Equal/True/False where appropriate for value comparisons and string
contains checks.
In `@cmd/oastools/commands/walk_parameters_test.go`:
- Around line 52-72: The helper function captureStdout in
walk_parameters_test.go should be extracted into a shared test helper file
(e.g., test_helpers_test.go) within the same package so other tests can reuse
it; move the captureStdout implementation as-is into the new file, delete
duplicate copies in other test files, and update any tests referring to local
copies to call captureStdout from the shared helper (no API changes needed since
it can remain unexported).
In `@cmd/oastools/commands/walk_paths_test.go`:
- Around line 46-383: Replace raw t.Fatalf/t.Errorf/t.Error checks in the test
functions (e.g., TestWalkPaths_ListAll, TestWalkPaths_FilterByPathExact,
TestWalkPaths_FilterByPathGlob, TestWalkPaths_FilterByExtension*,
TestWalkPaths_FilterCombined*, TestWalkPaths_FilterNonexistentPath,
TestPathMethods, TestWalkPaths_SummaryTableOutput,
TestWalkPaths_SummaryTableQuiet, TestWalkPaths_DetailOutput*,
TestWalkPaths_NoArgsError, TestWalkPaths_InvalidFormat,
TestWalkPaths_CollectPathsNilResult) with stretchr/testify helpers: use
require.NoError/require.Error/require.Equal/require.Len for fatal-style checks
(replace t.Fatalf and checks that should stop the test) and use
assert.Equal/assert.Contains/assert.Len for non-fatal checks (replace
t.Errorf/t.Error); add the necessary imports
(github.com/stretchr/testify/require and github.com/stretchr/testify/assert) and
update assertions accordingly (e.g., require.NoError(t, err), require.Len(t,
matched, 3), assert.Contains(t, output, "/pets"), require.Equal(t, "/pets",
matched[0].pathTemplate), require.True/False where appropriate) so all tests
conform to the repo guideline to use testify assertions.
In `@cmd/oastools/commands/walk_paths.go`:
- Around line 118-145: The pathMethods function currently uses hardcoded HTTP
method strings; update it to use the package-level constants
(httputil.MethodGet, MethodPut, MethodPost, MethodDelete, MethodOptions,
MethodHead, MethodPatch, MethodTrace) instead of literals, appending
strings.ToUpper(httputil.MethodX) to the methods slice where you currently
append e.g. "GET", and add the httputil import; keep the same nil checks on
pi.Get, pi.Put, etc., and return strings.Join(methods, ", ") as before so
display formatting matches walk_operations.go / walk_responses.go patterns.
In `@cmd/oastools/commands/walk_render_test.go`:
- Around line 61-92: Add two tests for RenderDetail: one that passes FormatText
and asserts YAML-style output (e.g., check for "key:" in the buffer) to verify
text defaults to YAML, and one that passes an unsupported format string (e.g.,
"xml") and asserts that RenderDetail returns a non-nil error; target the
RenderDetail function and use the same test pattern as
TestRenderDetail_YAML/TestRenderDetail_JSON to construct buffers and nodes and
to check returned error and output content.
In `@cmd/oastools/commands/walk_responses_test.go`:
- Around line 61-247: Replace manual t.Fatalf/t.Errorf assertions in the
TestHandleWalkResponses_* tests with stretchr/testify assertions: add an import
for "github.com/stretchr/testify/require" and
"github.com/stretchr/testify/assert"; use require.* (e.g., require.Equal,
require.NoError, require.Len, require.True) for fatal conditions such as nil
errors or expected lengths (tests that used t.Fatalf) and use assert.* for
non-fatal checks (previous t.Errorf checks). Update tests referencing helper
functions like collectTestResponses, filterResponsesByStatus,
filterResponsesByPath, filterResponsesByMethod, ParseExtensionFilter,
RenderSummaryTable, and handleWalkResponses to call require/ assert with
equivalent messages and values so behavior and expectations remain identical.
In `@cmd/oastools/commands/walk_responses.go`:
- Around line 34-41: The order of validation in the command handler is
inconsistent: call ValidateOutputFormat(flags.Format) before checking fs.NArg()
so the output format is validated first (matching walk_schemas.go). Update the
sequence in the function that contains the fs.NArg() check and the
ValidateOutputFormat call (the walk_responses command handler) so
ValidateOutputFormat(flags.Format) executes prior to the missing-spec-file check
and return its error immediately if non-nil.
In `@cmd/oastools/commands/walk_schemas_test.go`:
- Around line 58-655: The tests use plain t.Fatal/t.Errorf/string checks; switch
them to stretchr/testify assertions by importing
"github.com/stretchr/testify/require" and "github.com/stretchr/testify/assert"
and replacing fatal preconditions with require.* (e.g.,
require.Error/require.NoError/require.False/require.True/require.Contains/require.Equal)
and non-fatal verifications with assert.*; update tests referencing
handleWalkSchemas, TestHandleWalkSchemas_*, TestSchemaTypeMatches (use
require.NoError for setup like walker.CollectSchemas and ParseExtensionFilter),
RenderSummaryTable/RenderDetail/renderNoResults (use require.NoError for
RenderDetail), schemaTypeMatches, testSchemaParseResult, and capture helpers to
use require/assert appropriately so tests fail fast on setup errors and use
assert for content checks. Ensure imports are added and unused t.* calls
removed.
In `@cmd/oastools/commands/walk_security_test.go`:
- Around line 35-223: Replace raw testing.T checks in tests like
TestFilterSecuritySchemes_* , TestHandleWalkSecurity_*, TestRenderSecurity* with
stretchr/testify assertions: add import "github.com/stretchr/testify/require"
(and "assert" where non-fatal checks preferred), then convert
t.Fatalf/t.Errorf/t.Error to
require.NoError/require.Error/require.Equal/require.Len/require.EqualError or
assert.Len/assert.Equal as appropriate; for example use require.NoError(t, err),
require.Len(t, matched, N), require.Equal(t, "bearerAuth", matched[0].Name), and
require.EqualError(t, err, "walk security requires a spec file argument") when
checking error messages; ensure tests referencing functions
filterSecuritySchemes, handleWalkSecurity, renderSecuritySummary,
renderSecurityDetail and collectTestSecuritySchemes are updated accordingly and
imports adjusted.
|
Claude finished @erraggy's task in 12m 13s —— View job Addressing CodeRabbit Review Comments ✅All 9 review items from @coderabbitai have been implemented and pushed in commit e47fc21.
Summary10 files changed (+407/-945 lines) — net reduction of 538 lines by replacing verbose
All tests pass ( |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@cmd/oastools/commands/walk_paths.go`:
- Around line 79-92: The collectPaths function should explicitly guard against a
nil *parser.ParseResult to avoid panics: add a nil check at the start of
collectPaths(result *parser.ParseResult) and return a descriptive error (e.g.,
"nil ParseResult") if result is nil, before calling walker.Walk; this ensures
callers of collectPaths and the underlying walker.Walk are not invoked with a
nil pointer and tests expecting a graceful error will pass.
- Around line 119-147: The pathMethods function currently ignores OAS 3.2+
PathItem.Query and PathItem.AdditionalOperations; update pathMethods (in
cmd/oastools/commands/walk_paths.go) to check pi.Query and append
strings.ToUpper(httputil.MethodQuery) when non-nil, and to handle
pi.AdditionalOperations by collecting its keys, sorting them for deterministic
ordering, then appending each key (upper-cased) to the methods slice; finally
return the joined string as before.
In `@cmd/oastools/commands/walk_responses.go`:
- Around line 129-138: The filterResponsesByMethod function should validate the
incoming method against the known HTTP method constants before filtering: inside
filterResponsesByMethod, normalize the input (strings.ToUpper or ToLower) and
check it against a set of allowed methods built from the httputil/http Method
constants (e.g., httputil.MethodGet, httputil.MethodPost, etc.); if the method
is not one of those constants, return an empty slice (or nil) immediately to
surface invalid input rather than silently producing empty results, otherwise
proceed with the existing case-insensitive comparison logic on r.Method to build
the result slice.
e47fc21 to
7c45287
Compare
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
walker/collectors_test.go (1)
386-631:⚠️ Potential issue | 🟡 MinorReplace HTTP method string literals with
httputilconstants.Tests currently use
"get","post", etc. as string literals. Importgithub.com/erraggy/oastools/internal/httputiland usehttputil.MethodGet,httputil.MethodPost,httputil.MethodPatch,httputil.MethodOptions,httputil.MethodHead, andhttputil.MethodTraceinstead. This applies to map accesses likecollector.ByMethod["get"]and theexpectedMethodsslice inTestCollectOperations_AllMethods.🧭 Example update
- expectedMethods := []string{"get", "post", "put", "delete", "patch", "options", "head", "trace"} + expectedMethods := []string{ + httputil.MethodGet, + httputil.MethodPost, + httputil.MethodPut, + httputil.MethodDelete, + httputil.MethodPatch, + httputil.MethodOptions, + httputil.MethodHead, + httputil.MethodTrace, + }
🤖 Fix all issues with AI agents
In `@cmd/oastools/commands/walk_extension_filter.go`:
- Around line 25-53: ParseExtensionFilter currently passes raw segments to
parseExtensionExpr which causes failures when users include spaces; trim
whitespace around OR groups and AND parts before validation by applying
strings.TrimSpace to each orPart and part (i.e., inside ParseExtensionFilter
before calling parseExtensionExpr). Also trim expr.Key and expr.Val inside
parseExtensionExpr (or ensure parseExtensionExpr trims its inputs) so key prefix
checks (x-…) and value comparisons are whitespace-robust; update both
ParseExtensionFilter and parseExtensionExpr/ExtensionExpr handling to normalize
whitespace.
In `@cmd/oastools/commands/walk_operations.go`:
- Around line 155-163: The current renderOperationsDetail loops and calls
RenderDetail per OperationInfo which emits multiple JSON objects; change it to
call RenderDetail once with the entire ops slice when flags.Format == "json" (or
always, if RenderDetail supports a slice) so a single valid JSON array is
produced; update RenderDetail usage/signature as needed to accept
[]*walker.OperationInfo (or []OperationInfo) and preserve method/path context,
and remove the per-op loop in renderOperationsDetail to pass ops directly.
- Around line 15-36: handleWalkOperations currently returns any error from
fs.Parse(args), which causes flag.ErrHelp for -h/--help to propagate and yield a
non-zero exit; modify the Parse error handling so that when fs.Parse(args)
returns flag.ErrHelp you treat it as success (return nil) instead of propagating
the error. Locate handleWalkOperations and its call to fs.Parse(args) and change
the logic to explicitly check for flag.ErrHelp (from the flag package) and
return nil in that case, while still returning other parse errors unchanged.
In `@cmd/oastools/commands/walk_parameters_test.go`:
- Around line 52-71: The helper captureStdout can leak stdout if fn() calls
t.Fatal (which exits early); wrap restoration of os.Stdout and closing the pipe
in a defer so they always run: after creating r,w and setting os.Stdout = w, add
a deferred closure that closes w and restores os.Stdout (e.g. defer func(){ _ =
w.Close(); os.Stdout = old }()), then call fn() and finally read from r into buf
as before; update captureStdout to ensure the defer runs even when fn invokes
t.Fatal or panics.
- Around line 74-298: Replace the manual t.* assertions in
TestHandleWalkParameters_* tests with stretchr/testify helpers: import
"github.com/stretchr/testify/require" and "github.com/stretchr/testify/assert"
and use require.NoError/require.Error for immediate-fatal checks (e.g., after
calling handleWalkParameters in tests like TestHandleWalkParameters_MissingFile,
_InvalidFormat, _ListAll, etc.), and use
assert.Contains/assert.NotContains/assert.Empty/assert.True where the tests
currently use strings.Contains and t.Errorf/t.Error for non-fatal checks; keep
helpers like handleWalkParameters, writeParameterTestSpec and captureStdout
unchanged but update their test assertions to require/assert patterns (e.g.,
require.NoError(t, err), assert.Contains(t, output, "limit"),
assert.NotContains(t, output, "id"), require.Equal(t, "", output) for the
"NoResults" stdout check).
In `@cmd/oastools/commands/walk_render_test.go`:
- Around line 1-92: Replace manual if-checks in the tests
(TestRenderSummaryTable, TestRenderSummaryTable_Quiet,
TestRenderSummaryTable_Empty, TestRenderDetail_YAML, TestRenderDetail_JSON) with
stretchr/testify assertions: add an import for
"github.com/stretchr/testify/require" (or assert where non-fatal is OK), use
require.Contains/require.NotContains for string containment, require.NoError for
error checks from RenderDetail, and require.Empty/require.Equal for empty buffer
length; update the assertions inside each test to call these testify helpers
instead of the current if/... t.Error/t.Fatalf logic.
In `@cmd/oastools/commands/walk_render.go`:
- Around line 63-64: Update the godoc for RenderDetail to accurately list
supported output formats (JSON, YAML, and Text) by editing the comment above the
RenderDetail function to mention FormatText/Text in addition to JSON and YAML so
the public API docs reflect that RenderDetail(w io.Writer, node any, format
string, quiet bool) supports the Text format (FormatText).
- Around line 15-60: RenderSummaryTable can panic when rows have more columns
than headers because widths is sized to len(headers); compute maxCols =
max(len(headers), max len of any row), allocate widths = make([]int, maxCols),
and populate widths using header values for indexes < len(headers) and empty
string for missing headers; when printing headers and rows, iterate to maxCols
and use empty string for any missing header or row cell (e.g., treat cell = ""
if i >= len(row) and header = "" if i >= len(headers)) so all accesses to
widths[i] are safe.
In `@cmd/oastools/commands/walk_security.go`:
- Around line 72-101: The filterSecuritySchemes function and the
summary-rendering loop must guard against nil SecurityScheme entries to avoid
panics for malformed specs; update filterSecuritySchemes to immediately skip any
info where info.SecurityScheme == nil (before checking info.SecurityScheme.Type
or Extra) and likewise update the summary rendering code that iterates over
schemes (the loop that accesses info.SecurityScheme fields) to skip entries with
nil info.SecurityScheme so all dereferences are safe.
- Around line 122-130: renderSecurityDetail currently passes only
info.SecurityScheme to RenderDetail, losing the scheme's name and JSONPath;
change it to render the full walker.SecuritySchemeInfo (or a small wrapper
struct containing SecurityScheme plus Name and JSONPath) so the detail output is
self-describing when multiple matches occur. Update the loop in
renderSecurityDetail to pass the info object (e.g., RenderDetail(os.Stdout,
info, flags.Format, flags.Quiet)) and, if necessary, adjust RenderDetail's input
handling to accept the wrapper/type so it formats the extra fields (referencing
renderSecurityDetail, walker.SecuritySchemeInfo, info.Name and info.JSONPath).
Ensure error wrapping remains the same.
In `@cmd/oastools/commands/walk.go`:
- Around line 116-145: Update the walk usage text in printWalkUsage to clarify
that the --format flag applies to detail output only (i.e., when --detail is
set) and adjust the examples accordingly (e.g., change the responses example to
include --detail before --format json so the output is JSON suitable for jq);
reference the behavior in RenderSummaryTable vs RenderDetail and flags.Format
when making the wording change (or alternatively implement consistent JSON/YAML
summary rendering across subcommands if you prefer behavioral change).
- Around line 11-45: Refactor HandleWalk and printWalkUsage to use a single
source of truth (e.g., a map[string]func([]string) error or a slice of
subcommand structs) that pairs subcommand names with handler functions
(handlers: handleWalkOperations, handleWalkSchemas, handleWalkParameters,
handleWalkResponses, handleWalkSecurity, handleWalkPaths); replace the hardcoded
switch in HandleWalk with a lookup in that map to dispatch the subcommand and
use the same map/slice to generate the usage text and the list of valid
subcommands in the "unknown walk subcommand" error, while preserving the
existing --help/-h/help handling logic.
In `@docs/plans/2026-02-12-walk-command-implementation.md`:
- Around line 21-28: The fenced diagram block in the document lacks a language
tag which can trigger markdownlint MD040; update the triple-backtick fence that
surrounds the Phase 1/2/3 diagram so it includes a language identifier (e.g.,
change the opening ``` to ```text), ensuring the fenced block renders as a code
block with a language tag; if your
docs/plans/2026-02-12-walk-command-implementation.md is intentionally exempt
from linting, you may alternatively add a per-file or block-level lint
suppression comment, but the straightforward fix is to add the `text` language
tag to the diagram fence.
Add `oastools walk` with 6 subcommands for exploring OpenAPI documents: - operations: list/inspect operations with method, path, tag filters - schemas: list/inspect schemas with name, type, component/inline filters - parameters: list/inspect parameters with name, location, path filters - responses: list/inspect responses with status code, path, method filters - security: list/inspect security schemes with name, type filters - paths: list/inspect path items with path pattern filters Architecture follows a Collect-Filter-Render pattern: 1. Collector functions walk the spec and build indexed collections 2. Subcommand-specific filters narrow results 3. Output renders as summary tables or full detail (JSON/YAML) Key features: - Extension filter DSL: AND (+), OR (,), negation (!, !=), value matching - Path glob matching (* matches one segment) - Status code wildcards (2xx, 4xx) - Quiet mode for piping, --detail for full node output - $ref resolution via --resolve-refs Includes collector APIs in walker package (CollectOperations, CollectSchemas, CollectParameters, CollectResponses, CollectSecuritySchemes) with indexed lookups (ByPath, ByMethod, ByTag, ByName, ByLocation, ByStatusCode). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
7c45287 to
64ecfc6
Compare
erraggy
left a comment
There was a problem hiding this comment.
Re: collectors_test.go httputil constants (outside diff range)
Acknowledged — the string literals ("get", "post", etc.) happen to equal the constant values (httputil.MethodGet, httputil.MethodPost), so this is a readability improvement rather than a correctness fix. Since it's outside the PR diff range, I'll address it in a follow-up if/when we touch those test lines.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@cmd/oastools/commands/walk_parameters.go`:
- Around line 83-85: The name filter in walk_parameters.go currently uses a
case-sensitive equality check (filterName != info.Name); change this to a
case-insensitive comparison (use strings.EqualFold) so filterName and info.Name
are matched ignoring case—update the conditional that references filterName and
info.Name to use strings.EqualFold (negated as needed) to maintain consistency
with other commands like "walk schemas".
In `@docs/plans/2026-02-12-walk-command-design.md`:
- Line 240: Add a single trailing newline to the end of the document so the file
ends with a newline character; locate the closing code fence (the final triple
backticks) in docs/plans/2026-02-12-walk-command-design.md and insert one
newline character after it to satisfy markdownlint MD047.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@cmd/oastools/commands/walk_parameters.go`:
- Around line 53-54: The method-filter handling should follow the codebase's
case-insensitive pattern used by neighboring filters: stop normalizing
filterMethod and instead perform a case-insensitive comparison using
strings.EqualFold between info.Method and filterMethod (i.e., replace the
current normalization + exact-equality check with strings.EqualFold(info.Method,
filterMethod)); this aligns with the pattern in other filters and keeps behavior
consistent with the defensive approach used elsewhere such as in
walk_responses.go.
Summary
oastools walkwith 6 subcommands for exploring OpenAPI documents:operations,schemas,parameters,responses,security, andpathswalkerpackageSubcommands
operations--method,--path,--tag,--deprecated,--operationIdschemas--name,--type,--component,--inlineparameters--name,--in,--path,--methodresponses--status(supports2xxwildcards),--path,--methodsecurity--name,--typepaths--path(supports*glob)All subcommands share
--format(text/json/yaml),-q/--quiet,--detail,--extension, and--resolve-refs.Collector APIs
New
walkerpackage collector functions with indexed lookups:CollectOperations→ByPath,ByMethod,ByTagCollectSchemas→ByPath,ByName,Components,InlineCollectParameters→ByName,ByLocation,ByPathCollectResponses→ByStatusCode,ByPathCollectSecuritySchemes→ByNameExtension filter DSL
Examples
Test plan
!)FormatExtensionsdeterministic output testsmatchPathglob andmatchStatusCodewildcard testsByName,ByLocation,ByStatusCode,ByPathassertionsmake checkpasses (7,847 tests, lint clean)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests