refactor(cli): finish withClient migration — comment + tests#182
Merged
Conversation
The comment command was the only client-using command left with manual analytics + getConfig + assertWritable + ConfluenceClient + try/catch scaffolding after #181, because its catch block needed to print API response details and inline-comment-specific hints. Add an `onError(error, ...actionArgs)` option to withClient that runs between the standard `Error:` log line and process.exit, and extend handleCommandError with a matching `onExtra` callback. The hook is wrapped in try/catch so a throwing hint path cannot break the error flow. Move the comment command's API-response dump and inline-meta hint into the hook. Existing behavior (output ordering, exit code, failure tracking) is preserved. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The withClient helper sits on the shared error/analytics/client path for
every CLI command that talks to the server but had no direct test
coverage. Add six cases covering: context injection ({ client, config,
analytics } + forwarded action args), the writable-on-readOnly exit
path, failure tracking on handler throw, the new onError hook
(invocation order before process.exit, action-arg forwarding, throw
isolation), and getConfig failure being treated as a tracked failure.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
🎉 This PR is included in version 2.6.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
commentcommand — the last client-using command still on the old manual scaffolding after refactor(cli): extract withClient helper to remove command boilerplate #181 — ontowithClient, via a newonError(error, ...actionArgs)hook that preserves the command's API-response dump and inline-meta hint.withClientwrapper (6 cases). Until now this shared error/analytics/client path on every server-touching command had zero direct coverage.Why
Follow-up to the review of #181:
commentwas left on the old pattern because its catch block does extra work. The newonErrorhook gives it a place to live.withClient's try/exit/track path with no direct test. Any regression here would silently break every command's error handling.The remaining DRY issue around handlers re-typing the command name in
analytics.track('name', true)(#2) is intentionally left out to keep this PR small. It would touch all 26 handlers and is better as its own follow-up.Test plan
npm test— 676 tests pass (16 suites), including the 6 newwith-client.test.jscasesnpm run lint— cleanconfluence comment --helpon a writable profile to confirm command still registers🤖 Generated with Claude Code