feat(trail): update body/metadata by number, id, or branch (incl. branchless)#1493
feat(trail): update body/metadata by number, id, or branch (incl. branchless)#1493paxos wants to merge 5 commits into
Conversation
`entire trail update` now accepts a positional <trail> and a --trail selector (number, id, or branch), so branchless trails are addressable; --branch is kept for back-compat. Body input gains --body-file and stdin (--body - / --body-file -). Body and metadata are sent as separate PATCHes since the API rejects combining them, and the interactive form seeds the body from the real editor document instead of the empty metadata column.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 2c7bcaa. Configure here.
| inputs.StatusChanged = true | ||
| inputs.Title = title | ||
| inputs.TitleChanged = true | ||
| inputs.Body = body |
There was a problem hiding this comment.
Failed load still wipes body
Medium Severity
If loading the current body for the interactive form fails, the form still starts with an empty body, and a successful submit always marks the body as changed. A separate body PATCH then sends an empty string, which can erase the existing collaborative document when the user only meant to update title or status.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 2c7bcaa. Configure here.
There was a problem hiding this comment.
Fixed in 3940a99. The interactive form now fails if the current body cannot be loaded instead of seeding an empty body, so an edit can no longer overwrite a body that merely failed to fetch. On top of that (eed63eb), a field is only marked changed when the user actually edits it, so an unedited body is never re-sent.
— response written by Claude (AI), on behalf of @paxos
| inputs.StatusChanged = true | ||
| inputs.Title = title | ||
| inputs.TitleChanged = true | ||
| inputs.Body = body |
There was a problem hiding this comment.
Trimmed seed rewrites body whitespace
Low Severity
Interactive updates seed the body from fetchTrailDescription, which trims leading and trailing whitespace from the snapshot, but the command still always PATCHes the body after the form and documents that CLI body input is written verbatim. Submitting without editing the body field persists the trimmed text and can drop meaningful surrounding whitespace from the live document.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 2c7bcaa. Configure here.
There was a problem hiding this comment.
Fixed in 3940a99. The interactive form now seeds from the raw text_snapshot via a new fetchTrailBody (no TrimSpace), so an edited save preserves the document’s whitespace. And an unedited body is not PATCHed at all (eed63eb), so a no-op save is non-destructive. fetchTrailDescription stays trimmed for show display, delegating to fetchTrailBody.
— response written by Claude (AI), on behalf of @paxos
There was a problem hiding this comment.
Pull request overview
This PR enhances entire trail update to support updating both metadata and the collaborative body for any trail (including branchless trails) by accepting a selector via positional arg / --trail (number, id, or branch) while keeping --branch for backward compatibility. It also adds body input via --body-file and stdin (-), and implements the required API behavior of sending metadata and body as separate PATCH requests.
Changes:
- Extend
trail updateto accept a trail selector (arg /--trail/ legacy--branch) and resolve the trail via the same selector logic used by other trail commands. - Add body input resolution for
--body,--body-file <path>, and stdin (--body -/--body-file -). - Split updates into two PATCH calls (metadata first, then body) and add unit tests covering body input resolution + PATCH splitting and numberless-trail rejection.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
cmd/entire/cli/trail_cmd.go |
Updates trail update CLI surface/selector handling, adds body input resolution + stdin/file support, and implements split PATCH behavior. |
cmd/entire/cli/trail_cmd_test.go |
Adjusts positional-arg test coverage and adds new unit tests for body-input resolution and split PATCH semantics. |
| body = "" | ||
| if found.Number > 0 { | ||
| if bt, derr := fetchTrailDescription(ctx, client, forge, owner, repoName, found.Number); derr == nil { | ||
| body = bt | ||
| } else { | ||
| fmt.Fprintf(errW, "Warning: could not load current body: %v\n", derr) | ||
| } | ||
| } |
There was a problem hiding this comment.
Adopted both suggestions in 3940a99: (a) the interactive update now fails when the current body can’t be loaded rather than seeding empty, and (b) it seeds from the raw text_snapshot (no TrimSpace) via fetchTrailBody. Combined with only-send-edited-fields (eed63eb), a no-op save is non-destructive and an edit preserves whitespace. New tests: TestFetchTrailBodyReturnsRawUntrimmedSnapshot, TestFetchTrailBodyReportsMissingDocument.
— response written by Claude (AI), on behalf of @paxos
The interactive update form always marked the body changed and PATCHed it. If the current body failed to load (empty seed) or was whitespace-trimmed by the detail endpoint, an unedited submit would erase or rewrite the live editor document. Mark each field changed only when the user edits it away from its seed, and skip the request entirely when nothing changed.
|
Addressed both Bugbot findings in eed63eb: Both stemmed from the interactive (no-flags) form always marking the body changed and PATCHing it. Now each field is marked changed only when the user edits it away from its seed (
Also added a "No changes" short-circuit when nothing was edited. New unit tests: This comment was generated by AI (Claude). |
Per review: the interactive update now seeds the body from the raw text_snapshot (via fetchTrailBody) so an edited save preserves the document whitespace, and fails when the current body cannot be loaded instead of seeding empty — which would let an edit silently overwrite a body that merely failed to fetch. fetchTrailDescription is kept (trimmed) for `show` display.
|
Addressed the Copilot review in 3940a99:
Combined with the earlier change (only re-send a field the user actually edited), an interactive no-op is non-destructive and an edit preserves whitespace. New tests: This comment was generated by AI (Claude). |
Avoid adding new "hello world" string literals (goconst occurrence count) by using local constants, and rename unused httptest handler params to _.
Per Codex review: feeding --branch into the generic number/id/branch selector made `--branch 123` resolve trail #123 instead of the branch named "123". Resolve --branch via findTrailByBranch (branch-only) and keep the positional arg / --trail on the generic selector.


https://entire.io/gh/entireio/cli/trails/637
TL;DR — Makes
entire trail updateaddress any trail by number, id, or branch — including branchless trails — so their body and metadata (status/title/labels) become editable from the CLI, plus file/stdin body input. CLI-only, no API changes. Stacked on #1476 (the body-read PR).Problem
entire trail updatewasNoArgs+--branch-only, so a branchless trail (no branch) could not be targeted at all — neither its body nor its metadata was editable from the CLI.show/checkout/deletealready resolve trails by number/id/branch;updatedid not.What changed (CLI only — no API changes)
updatetakes a positional[<trail>]and a--trailselector (number, id, or branch);--branchkept for back-compat. This unblocks branchless trails for every field — not just the body but also--status,--title,--add-label/--remove-label(e.g.entire trail update 1488 --status open).--body-file <path>and stdin via--body -/--body-file -, for multi-line / image-rich markdown.body_documentinstead of the empty metadata column, so it no longer clobbers the body.Scope (relative to the two tracking plans)
This PR is the
updatehalf of the work:update(+ file/stdin) —cli-trail-body-rwStep 2updateselector / branchless targeting (body and metadata) —cli-trail-targeting-consistencyStep 1show— provided by the base PR feat(trail): show the description and a browser URL in trail show #1476, not re-implemented heredeleteby id/branch (cli-trail-targeting-consistencyStep 2) —deletestays number-only in this PRTest plan
go test ./cmd/entire/cli/— full package green.entire trail update 1488 --body-file ./b.md,--body -,--trail, by id/branch, and--statuson a branchless trail.Notes
mainonce that merges.