Conversation
8b3c6d7 to
f543e81
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b3c6d76af
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if result == nil { | ||
| return output.ErrNotFound("item", itemIDStr) |
There was a problem hiding this comment.
When --type is omitted, any SDK error during the vault/upload/document probes is ignored and the command falls through to ErrNotFound. That turns authentication failures, permission errors, or transient network issues into a misleading “item not found” response (previously the API error was returned). Consider returning convertSDKError for non-404s (or keeping the first error) instead of unconditionally emitting not-found here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in PR #73 - now tracks first error and checks if it was a 404 or something else. Non-404 errors (auth, permission, network) are returned instead of masking as 'not found'.
| return output.ErrUsageHint( | ||
| fmt.Sprintf("Item %s not found", itemID), | ||
| fmt.Sprintf("Item %s not found", itemIDStr), | ||
| "Specify --type if needed", |
There was a problem hiding this comment.
In the auto-detect branch for updates, any error from the SDK Get probes is treated as a miss and eventually converted into Item not found. This masks real failures like expired tokens, permission denials, or 5xx responses and makes the CLI report the wrong root cause. It would be safer to return convertSDKError for non-404 errors (or propagate the first error) instead of always returning a not-found usage hint.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in PR #73 - same fix applied to update auto-detect path. Non-404 errors are now properly returned.
Fixes for Codex review comments: 1. webhooks.go: Let server use default webhook types instead of hard-coding (PR #58 feedback) 2. templates.go: Use raw API for archived/trashed status since SDK List() only returns active templates (PR #60 feedback) 3. campfire.go: Validate ParseInt errors before DeleteLine to avoid silent 0 IDs (PR #64 feedback) 4. recordings.go: Add comment clarifying that Unarchive() works for both archived AND trashed recordings via status/active endpoint (PR #68) 5. files.go: Preserve SDK errors in auto-detect mode instead of masking auth/permission errors as "not found" (PR #70 feedback)
Fixes for Codex review comments: 1. webhooks.go: Let server use default webhook types instead of hard-coding (PR #58 feedback) 2. templates.go: Use raw API for archived/trashed status since SDK List() only returns active templates (PR #60 feedback) 3. campfire.go: Validate ParseInt errors before DeleteLine to avoid silent 0 IDs (PR #64 feedback) 4. recordings.go: Add comment clarifying that Unarchive() works for both archived AND trashed recordings via status/active endpoint (PR #68) 5. files.go: Preserve SDK errors in auto-detect mode instead of masking auth/permission errors as "not found" (PR #70 feedback)
Fixes for Codex review comments: 1. webhooks.go: Let server use default webhook types instead of hard-coding (PR #58 feedback) 2. templates.go: Use raw API for archived/trashed status since SDK List() only returns active templates (PR #60 feedback) 3. campfire.go: Validate ParseInt errors before DeleteLine to avoid silent 0 IDs (PR #64 feedback) 4. recordings.go: Add comment clarifying that Unarchive() works for both archived AND trashed recordings via status/active endpoint (PR #68) 5. files.go: Preserve SDK errors in auto-detect mode instead of masking auth/permission errors as "not found" (PR #70 feedback)
1 participant
No description provided.