fix: include URL in api list --json output#411
Conversation
Fixes #388 - the --json flag was outputting only `apidoc` (missing the gateway URL). Now outputs the same processed array as the table view, which includes the URL built from `gwApiUrl`. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fixes #388 - keeps the apidoc JSON structure unchanged but updates the x-openwhisk.url field (previously always "not-used") to the actual gateway URL built from gwApiUrl + path. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🤖 PR Reviewer
The diff adds gateway URL enrichment to the JSON output of the API list command, patching x-openwhisk.url for each path/verb operation. The logic is straightforward and the tests cover both the happy path and the missing x-openwhisk case. However, there are a few defensive coding concerns: no null/undefined guards on result.apis, result.apis[0], or api.value before accessing nested properties, which could cause unhandled runtime errors if the API returns unexpected data.
📝 1 suggestion(s) - Please review inline comments below.
💡 How to re-trigger
Comment /review or /pr-reviewer on this PR
There was a problem hiding this comment.
🤖 PR Reviewer
The diff improves the JSON output logic by enriching the apidoc with gateway URLs, and adds useful test coverage. However, the previously raised null/undefined guard issue on result.apis, result.apis[0], and apidoc.paths is still not addressed — if the API returns an empty array, missing value, or missing paths, the code will throw a TypeError at runtime.
🔄 1 re-raised suggestion(s) from previous review
💡 How to re-trigger
Comment /review or /pr-reviewer on this PR
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
- Return {} when result.apis is empty instead of throwing TypeError
- Use structuredClone before modifying apidoc to prevent in-place mutation
- Add tests for empty result and non-mutation of the response object
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🤖 PR Reviewer
The diff addresses the previous re-raised concern by adding a null guard for result.apis[0] and using structuredClone to avoid mutating the SDK response. However, apidoc.paths and api.value still lack defensive checks — if api.value, api.value.apidoc, or api.value.apidoc.paths is null/undefined, the code will throw a TypeError. The previous suggestion's recommendation to guard apidoc.paths with || {} was not fully applied.
🔄 1 re-raised suggestion(s) from previous review
💡 How to re-trigger
Comment /review or /pr-reviewer on this PR
Guard api.value?.apidoc and apidoc.paths with nullish fallbacks to prevent TypeError when the SDK response has missing nested fields. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🤖 PR Reviewer
The diff addresses most of the previous suggestions by adding a guard for missing api and using optional chaining for api.value. The structuredClone usage is good for immutability. However, operation itself is still unguarded before accessing operation['x-openwhisk'], which can throw if a path verb maps to a non-object value. The result.apis array itself is also unguarded before indexing. Overall the code is much improved with good test coverage added.
📝 2 suggestion(s) - Please review inline comments below.
💡 How to re-trigger
Comment /review or /pr-reviewer on this PR
- Use optional chaining on result.apis?.[0] to handle null/undefined apis field - Guard operation access with typeof check before reading x-openwhisk, since OpenAPI verbs can hold non-object values (e.g. boolean true) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🤖 PR Reviewer
The diff addresses both previously raised issues: optional chaining guards result.apis?.[0] and the typeof operation === 'object' check prevents TypeError on non-object values. The use of structuredClone correctly prevents mutation of the original SDK response. Tests are thorough and cover all edge cases. No significant issues remain.
✅ LGTM! This PR looks good to merge.
💡 How to re-trigger
Comment /review or /pr-reviewer on this PR
There was a problem hiding this comment.
🤖 PR Reviewer
The diff adds defensive null-checks, a deep-clone to avoid mutating the SDK response, and URL enrichment for x-openwhisk operations. The implementation is clean and well-tested. One minor concern: paths not starting with '/' are silently skipped, which is correct for OpenAPI basePath-relative paths, but the behavior for paths without a leading slash (e.g. vendor extensions) is undocumented. Overall the change is solid and well-covered by tests.
✅ LGTM! This PR looks good to merge.
💡 How to re-trigger
Comment /review or /pr-reviewer on this PR
Summary
aio rt api list --jsonwas not showing the API URL--jsonoutput preserves the existingapidocJSON structure unchangedx-openwhisk.urlfield (previously always"not-used") is now populated with the actual gateway URL built fromgwApiUrl + pathTest plan
npm test -- --testPathPattern="api/list"— all tests passaio rt api list --jsonoutput retains theapidocstructure withx-openwhisk.urlnow set to the real URL🤖 Generated with Claude Code