[robzolkos]

Fixes #944.

What

urlarg.Parse no longer resolves a URL to an item its path runs past. Every command that takes <id|url> goes through it, so the fix applies to all of them at once.

The rule is about what the extra segments name:

• Segments after an item's ID that hold no other ID name that item. A segment holds no ID only when it starts with a letter, so 222.json, 222.html or %32%32%32 is never trimmed away. Examples are todos/789/edit, todos/789/dates/edit, comments/789/compositions/edit and vaults/111/documents/new. Parse routes the URL again without those segments, so a page of an API item still names it. A chat line's edit page names the line, and an occurrence's edit page keeps its date. main already resolved most of these, because the prefix match landed on the same item.
• Segments that hold another ID name another item. Parse resolves that item through the paths it knows, which it rewrites to the item's own path before routing:
  • a child that bc3 serves both nested and at top level: vaults/{id}/{vaults,documents,google_documents,cloud_files,uploads}/{id}, todolists/{id}/todos/{id} and questionnaires/{id}/questions/{id}. Runs of slashes in the path become one and a trailing slash is dropped, as Rails does when routing. Then a .json suffix is dropped, so vaults/111/documents/222.json/#__recording_789 names comment 789 on document 222.
  • an upload's download/{filename} link and a version's download/{filename} and representations/{key} links. bc3 links downloads and previews to a version even when it is the latest. The filename goes with the link, so an all-digit filename such as download/12345 is not read as an ID.
• Any other path with another ID returns nil. Examples are questions/1/answers/2, messages/789/boosts/5, card_tables/cards/789/steps/5 and uploads/456/versions/7. Callers already refuse a nil result. IsURL still returns true for these URLs, so callers report a URL error and do not treat the URL as a name.
• API routes that end in an ID that is not a recording return nil. These are questions/{id}/answers/by/{person}, people/{person}/out_of_office, reports/todos/assigned/{person}, reports/users/progress/{person} and embeds/{sgid}. On main, recordings trash with an answers-by-person URL acts on the recording that shares the person's number. people/{person} still parses as a person, because its type says so and show handles it.

URL (after /99999/buckets/123)	main	This PR
/schedule_entries/456/occurrences/20261022/edit	entry 456, no date	entry 456, occurrence 20261022
/vaults/111/documents/456	folder 111	document 456
/chats/789/lines/111/edit	chat 789	line 111
/todos/789/dates/edit	to-do 789	to-do 789
/vaults/111/children/222	folder 111	nil
/questions/111/answers/by/789	recording 789 (a person)	nil

Why

The SDK router tries its API routes first, and those match the whole path. When none matches, it falls back to structural patterns that match only the start of the path and drop the rest. The fault is in the SDK, filed as basecamp/basecamp-sdk#1036. This PR adds the check at the CLI's single entry point, so the CLI does not wait for an SDK release.

An earlier version of this PR refused every structural match not in a list of known bc3 forms. Review kept finding real bc3 links that main resolved and that list refused, such as dates/edit and compositions/edit. The list could not be finished. Whether the extra segments hold an ID can be checked on any URL.

On main, against an unreachable server:

• basecamp recordings trash <chat line edit URL> sends PUT /recordings/789/status/trashed.json, which trashes the whole chat.
• basecamp spotlight <occurrence edit URL> spotlights the whole recurring series.

On this branch, the first command trashes line 111 and the second is refused with the existing occurrence message.

Not included

• basecamp url parse does not print the occurrence date. That was already true for plain occurrence URLs.
• A refused URL reaches commands that use ExtractID as the raw string. The command then fails with its existing "Invalid ID" error, not a message that names the URL.
• API routes whose ID is not a recording but whose type names it, such as webhooks/{id}, templates/{id} and categories/{id}, parse as before. A command that ignores the type can still act on the recording that shares the number, as on main.
• An account-level page with no ID after the account, such as my/profile/edit, still parses as the account, as on main.

Testing

• internal/urlarg/urlarg_trailing_test.go has 56 table cases that compare the whole Parsed result. Against main's parser, 35 fail. The 21 that pass are forms where the prefix match gives the right item, and they check that the fix does not refuse them. Against eddc55d, 17 fail. Against c5da2a5, 10 fail: a trailing slash after .json, a doubled slash, and a nested ID with a suffix, a percent escape or a sign.
• Removing any one part of the change fails at least 3 cases: the download rewrite, the .json drop, the person and sgid check, the second routing without the trailing words, the slash collapse (6) or the letter-start rule for a trimmed segment (4).
• e2e/url.bats: url parse names a document in a folder and refuses messages/789/boosts/5.
• I ran 141 URLs through Parse on main and on this branch. 54 results changed. In 30 the result went from the parent, an empty result or nil to the item itself, the child, line, person or Ping, or the occurrence date came back. In 24 a URL that named a different item now returns nil. None went from the right item to nil, and none resolves to an item other than the one the URL names. The other 87 did not change.
• bin/ci passes

---

Summary by cubic

Fixes #944 so urlarg.Parse no longer resolves a URL to the parent item when its path runs past the item. Every command that takes <id|url> goes through Parse, so the chat line edit URL now trashes the line, not the whole chat, and an occurrence edit URL is refused rather than spotlighting the whole series.

Behavior

• Words after an item's ID name that item, so the URL is routed again without them: todos/789/edit, todos/789/dates/edit, comments/789/compositions/edit, and unknown pages all still name the item. An occurrence's edit page keeps its date. A segment counts as a word only if it starts with a letter: one that starts with digits, a percent escape, or a sign (222.json, 222.html, %32%32%32) is treated as an ID and refused rather than resolved to the parent.
• Siblings bc3 serves also at top level are rewritten to the item's own path: vaults/{id}/{vaults,documents,google_documents,cloud_files,uploads}/{id}, todolists/{id}/todos/{id}, and questionnaires/{id}/questions/{id}. A .json suffix is dropped before rewriting.
• Upload download and preview links, which bc3 points at a version even when it is the latest, resolve to the upload: uploads/{id}/download/{filename}, uploads/{id}/versions/{n}/download/{filename}, uploads/{id}/versions/{n}/representations/{key}, and uploads/{id}/versions/new. A bare uploads/{id}/versions/{n} is not a page bc3 serves and stays refused.
• Any remaining extra ID, plus paths ending in a person's ID or an embed's sgid, returns nil. Since IsURL still returns true, callers report a URL error instead of treating the URL as a name.
• Runs of slashes in the path are collapsed and a trailing slash dropped, as Rails routes them, so a suffix or ID before them still ends the path.

The root cause is the SDK router's structural fallback matching only a path prefix (basecamp/basecamp-sdk#1036); this PR adds the check at the CLI's single entry point so it does not wait for an SDK release.

^{Written for commit 960bb08. Summary will update on new commits.}

[Copilot]

🟢 Approved

Static review supports the focused parser fix and regression coverage, with no unresolved blocking findings.

No open issues 🎉

What changed in this PR

Fixes #944 at the CLI’s shared URL parser so URLs with extra path segments no longer silently target a parent item.

Changes:

• Normalize supported edit pages, nested child URLs, and upload download links to the intended item.
• Reject unsupported trailing paths while preserving URL classification for caller error handling.
• Add unit and end-to-end regression coverage for correct item selection and rejection.

[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

File	Description
internal/​urlarg/​urlarg.go	Normalize supported item paths and reject incomplete structural matches.
internal/​urlarg/​urlarg_trailing_test.go	Cover trailing-path handling and refused-URL classification.
e2e/​url.bats	Verify nested-document resolution and unsupported-path rejection.

_{Review effort: Balanced}

[robzolkos]

Codex review of 9bc74dd (gpt-6.1-sol, run locally; it has no GitHub threads). One finding, fixed in eddc55d.

• Versioned upload links stopped resolving. …/uploads/456/versions/7/download/a.pdf resolved to upload 456 on main, but returned nil on 9bc74dd. bc3 builds these links even for the latest version. They resolve to the upload again, and so do a version's preview image (…/versions/7/representations/<key>) and the new-version page (…/versions/new). bc3 serves no page at …/versions/7 itself, so that path is still refused. The test that asserted the regression now expects upload 456.

bin/ci passes on eddc55d.

[Copilot]

🟢 Approved

Static review found no blocking issues, and the regression coverage addresses the intended normalization and rejection behavior.

No open issues 🎉

_{Review effort: Balanced}

[robzolkos]

Codex review of eddc55d (gpt-6.1-sol, run locally). Its three findings are addressed in c5da2a5, which also changes the approach.

Each Codex round had found real bc3 URLs that worked on main and that this PR now refused, because refusal depended on an open-ended table of bc3 URL forms. Parse now uses one rule instead. Extra path segments with no other ID in them (edit, dates/edit, compositions/edit) name the same item. A URL is refused only when an ID is left over after the existing child and download rewrites run.

• todos/789/dates/edit and comments/789/compositions/edit were refused: they resolve again.
• Nested .json URLs (vaults/111/documents/222.json#__recording_789) were refused: fixed. A trailing .json is dropped first.
• questions/{q}/answers/by/{person} returned a person ID as a recording (older bug): refused. So are the other API routes that capture a person or embed ID instead of a recording: people/{id}/out_of_office, reports/todos/assigned/{id}, reports/users/progress/{id} and embeds/{sgid}.

A probe of 97 URLs against main: 18 now resolve to the correct item, 15 that named a different item are refused, and none that were right on main are refused. bin/ci passes on c5da2a5.

[Copilot]

🟡 Changes recommended

JSON URLs with trailing slashes can still resolve to a parent and direct destructive commands at the wrong item.

1 open issue

• Prevent trailing slash after .json from selecting parent resources · New

_{Review effort: Balanced}

[robzolkos]

Codex review of c5da2a5 (gpt-6.1-sol, run locally).

• .json/ escapes normalization: the same finding as Copilot's thread, fixed in 960bb08. Codex also caught that card_tables/lists/222.json/ was newly refused. It resolves as on main again.
• schedule update with an occurrence URL edits the whole series: declined for this PR and filed as Occurrence URLs make commands act on the whole recurring series #947. The bug is in the command, not the parser. After this PR, Parse keeps the occurrence date, but schedule update drops it.

[Copilot]

🟢 Approved

The routing changes address the reported parent-targeting errors, with regression coverage and no blocking issues identified.

No open issues 🎉

1 resolved since last review

• Prevent trailing slash after .json from selecting parent resources

_{Review effort: Balanced}