Validate Empty Body Error Handling
Double check this claim. I thougth empty body errors?
(question) Empty-string clears. --body "" and --severity "" produce req.Body/req.Severity = &"", which pass trailReviewCommentPatchHasFields and get sent as explicit clears. If the server treats empty body/severity as valid, a finding can be blanked out. Confirm that's intended server-side, or reject empty --body the way add does (buildTrailReviewCommentInput requires a non-empty body).
Confirmed: the claim is correct for update.
In cmd/entire/cli/trail_review_cmd.go:
--body ""setsopts.BodyChanged = true, then:
So it becomes *"".
--severity ""setsopts.SeverityChanged = true, then:
So it also becomes *"".
trailReviewCommentPatchHasFieldsonly checks pointer presence:
So empty strings pass.
- The JSON tags are
omitempty, but because these are non-nil pointers, they will still marshal as:
or
By contrast, add does reject empty body:
So: empty body errors on add, but not on update.
If empty-body clearing is not intended, buildTrailReviewCommentPatchRequest should reject BodyChanged && strings.TrimSpace(opts.Body) == "". For severity, current behavior intentionally permits empty as a “clear” unless we decide to reject it too.
Update should also reject.
Implemented update rejection for empty values.
Changes:
cmd/entire/cli/trail_review_cmd.go--body ""/ whitespace now errors:finding body is required (pass --body)--severity ""/ whitespace now errors as invalid severity
cmd/entire/cli/trail_review_cmd_test.go- Added coverage for empty body and empty severity on update
Validation:
go test ./cmd/entire/clipasses.
Commit and push
Committed and pushed.
Commit: 99ba8655f Reject empty finding update fields
Branch: trail-finding-command-cleanup
Entire Trail review comment ID: 019ee023-54d4-7f48-bcad-d359e9624b93 URL: https://entire.io/gh/entireio/cli/trails/619/clean-up-trail-finding-commands Severity: high Context: cmd/entire/cli/trail_review_cmd.go:1271 Selected context:
The help text on line 1271 suggests running entire trail watch, but the watch command was removed from the finding subcommands in this PR and no replacement at the trail level is visible in this diff. This creates a broken user-facing reference.
Why was it removed from "entire trail" help? Should still be there, just not on trail finding...
It wasn’t removed from entire trail.
Confirmed:
newTrailCmd()still hascmd.AddCommand(newTrailWatchCmd())entire trail watch --helpworks- The PR only removed
watchfromentire trail finding
I added a regression assertion that entire trail watch remains registered while entire trail finding watch stays removed.
Committed and pushed:
aa7950671 Assert trail watch remains registered
merge latest main
Merged latest origin/main into trail-finding-command-cleanup.
Validation:
go test ./cmd/entire/clipasses- Working tree clean
Entire Trail review comment
ID: 019ee04b-e048-7154-b198-75b06fa9f3d4
URL: https://entire.io/gh/entireio/cli/trails/619/clean-up-trail-finding-commands
Severity: medium
Context: cmd/entire/cli/trail_review_cmd.go
buildTrailReviewCommentPatchRequest (~line 703) treats an explicitly-empty flag as a clear: --body "" or --severity "" (flag Changed) sends an empty string to the server. Severity validation only runs on non-empty values, so "" slips through. Since add requires a non-empty body, blanking a body via update is inconsistent. Confirm the server treats "" as clear vs. persisting an empty finding; if clearing isn't intended, skip empty strings like unchanged fields are skipped.
This is already fixed on the branch.
Current behavior in buildTrailReviewCommentPatchRequest:
--body ""/ whitespace returns:finding body is required (pass --body)--severity ""/ whitespace returns invalid severity- Empty strings are no longer sent to the server as clears
Added tests cover both cases. Commit: 99ba8655f Reject empty finding update fields
Entire Trail review comment ID: 019ee04b-eed2-75f3-a041-f87ba717e27e URL: https://entire.io/gh/entireio/cli/trails/619/clean-up-trail-finding-commands Severity: low Context: cmd/entire/cli/trail_review_cmd.go Stylistic: in newTrailReviewApplyCmd, cmd.Long (~line 258) is assigned between two cmd.Flags() calls. Harmless, but tidier to set Long next to Use/Short.
Done. Moved Long into the cobra.Command literal next to Use/Short, and kept flags together.
Validation:
gofmtgo test ./cmd/entire/cli -run TestTrailCommandSurfaceUsesFindings -count=1passes.
Entire Trail review comment ID: 019ee04b-fe42-7d75-a92e-781902040bbb URL: https://entire.io/gh/entireio/cli/trails/619/clean-up-trail-finding-commands Severity: low Context: cmd/entire/cli/trail_review_cmd.go printTrailReviewCommentsTable (~line 1285) hard-codes a two-space indent into the tabwriter header line (" ID\t...") and each row. Works, but indenting via tabwriter padding/prefix would be cleaner if this pattern spreads.
Done. Updated printTrailReviewCommentsTable to render the tabwriter output into a buffer without embedded indentation, then apply indentation via a helper.
Validation:
gofmt- Targeted test passes.
Resolve all "entire trail finding" once done some might have been addressed already and some are nits.