Validate Empty Body Error Handling · Entire

Double check this claim.

I thought 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).

4w ago·3.8s·9 calls

Confirmed: the claim is correct for update.

In cmd/entire/cli/trail_review_cmd.go:

    req.Body = stringPtr(strings.TrimSpace(opts.Body))
    ```

So it becomes `*""`.

- `--severity ""` sets `opts.SeverityChanged = true`, then:

```go
    severity := strings.ToLower(strings.TrimSpace(opts.Severity))
    req.Severity = stringPtr(severity)
    ```

So it also becomes `*""`.

- `trailReviewCommentPatchHasFields` only checks pointer presence:

```go
    return req.Body != nil || req.Severity != nil || ...
    ```

So empty strings pass.

- The JSON tags are `omitempty`, but because these are non-nil pointers, they will still marshal as:

```json
    {"body": ""}
    ```

or

```json
    {"severity": ""}
    ```

By contrast, `add` does reject empty body:

```go
    body := strings.TrimSpace(opts.Body)
    if body == "" {
        return ..., errors.New("finding body is required (pass --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.

4w ago·3.0s·4 calls

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/cli` passes.

---

**Entire Trail review comment**  
**ID:** 019ee023-54d4-7f48-bcad-d359e9624b93  
**URL:** [Link](/content/gh/entireio/cli/trails/619/clean-up-trail-finding-commands/index.html)  
**Severity:** high  
**Context:** `cmd/entire/cli/trail_review_cmd.go:1271`  
**Selected context:**

```go
fmt.Fprintln(w, "  entire trail watch")

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...

4w ago·4.7s·4 calls

It wasn’t removed from entire trail.

Confirmed:

I added a regression assertion that entire trail watch remains registered while entire trail finding watch stays removed.

Entire Trail review comment
ID: 019ee04b-e048-7154-b198-75b06fa9f3d4
URL: Link
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.

4w ago·5.1s·1 call

This is already fixed on the branch.

Current behavior in buildTrailReviewCommentPatchRequest:

Added tests cover both cases. Commit: 99ba8655f Reject empty finding update fields

Entire Trail review comment
ID: 019ee04b-eed2-75f3-a041-f87ba717e27e
URL: Link
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.

4w ago·2.0s·3 calls

Done. Moved Long into the cobra.Command literal next to Use/Short, and kept flags together.

Validation:

Entire Trail review comment
ID: 019ee04b-fe42-7d75-a92e-781902040bbb
URL: Link
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.

4w ago·2.0s·4 calls

Done. Updated printTrailReviewCommentsTable to render the tabwriter output into a buffer without embedded indentation, then apply indentation via a helper.

Validation:

Resolve all "entire trail finding" once done some might have been addressed already and some are nits.