Skip to content

Add and change cloud files and Google documents in Docs & Files - #1012

Open
jeremy wants to merge 7 commits into
mainfrom
sdk-absorb/card-10476950575
Open

jeremy wants to merge 7 commits into
mainfrom
sdk-absorb/card-10476950575

Conversation

@jeremy

@jeremy jeremy commented Nov 7, 2026 •

Copy link
Copy Markdown
Member

Why

The gap card 10476950575 names four SDK operations the CLI doesn't reach: adding and changing cloud files and Google documents in Docs & Files. files show already reads both kinds; nothing could create or edit them.

What changed

Operation Command
CreateCloudFile basecamp files cloud-files create [title] --url … [--service] [--description] [--subscribe|--no-subscribe] [--visible-to-clients]
UpdateCloudFile basecamp files cloud-files update <id|url> [--url] [--title] [--service] [--description]
CreateGoogleDocument basecamp files google-docs create [title] --url … [--type] [--description] [--draft] [--subscribe|--no-subscribe] [--visible-to-clients]
UpdateGoogleDocument basecamp files google-docs update <id|url> [--url] [--title] [--type] [--description] [--publish] [--subscribe|--no-subscribe]

The groups hang off files, so they appear under its aliases too (docs, vaults, …), like files documents and files folders.

  • Update is a full replace in Basecamp. Both controllers build a new recordable from the permitted params alone, so an omitted title or description is erased. The update commands read the record first and send back every field no flag changes. The description goes back verbatim, as files update already does for documents. The known rich-text write-back problem with @mentions is card 10439714339.
  • --service is inferred from the URL's host with the same table doors uses, falling back to other. --type is inferred from a docs.google.com path (document, spreadsheets, presentation), falling back to other. An explicit flag wins. On update, a new URL changes the service or type only when its host or path names one.
  • Google documents publish on create unless --draft is given, matching files documents create; Basecamp's own default is a draft. On update, status is left out unless --publish is given, so Basecamp keeps the document's current status. --publish=false is refused, because a published document can't go back to a draft.
  • files show breadcrumbs for a cloud file or Google document now point at these update commands, not at files update, which can't change them.

Builds on

main at 29150b2, which already pins SDK v0.35.0, with main merged in through da36c00. No bump is needed.

Left out

  • --subscribe on cloud-files update. bc3 applies subscriptions on update only to a drafted recording (Recording::DraftSubscribers), and CloudFilesController always creates and updates a cloud file as active. The flag would be a no-op. On google-docs update it is offered and documented as affecting drafts only.
  • Subscriptions on files upload. The card also asks for upload notifications. CreateUpload is already reached, but files upload still sends no subscriber list. That part of the card is untouched, so the card stays open for it.

Tests

  • Unit tests in internal/commands/files_links_test.go cover:
    • inference of the service and type;
    • subscribers sent, [] for --no-subscribe, and none by default;
    • refusals before any request;
    • update sending back unchanged fields;
    • an empty --description clearing it;
    • draft and publish;
    • an explicit --type winning over the URL.
  • e2e validation in e2e/files.bats: create without --url, update with no fields, an unknown --type.
  • Smoke: e2e/smoke/smoke_file_links.bats creates and renames one of each against the QA project. The docs and vaults variants are marked out of scope in smoke_lifecycle.bats, as for the other files subgroups. check-smoke-coverage accounts for every leaf.
  • API-COVERAGE.md gains cloud_files and google_documents rows (232 of 232 tracked), and .surface is regenerated.
  • bin/ci passes locally on the merge with main through da36c00. TestHardenConfigDir in internal/cli (untouched here) fails under a umask of 077, because its fixtures are created without an explicit chmod and it expects 0755 and 0644; it passes under 022, as in CI.

Gap card

https://app.basecamp.com/2914079/buckets/48521764/card_tables/cards/10476950575


Summary by cubic

Adds files cloud-files create/update and files google-docs create/update, so the CLI can now add and change cloud files and Google documents in Docs & Files; previously files show could only read them.

  • Updates replace the whole record in Basecamp, so the update commands read the record first and send back every field no flag changes; an explicitly empty --description clears it.
  • Update accepts a URL only from a trusted Basecamp host naming one record of its own type in the configured account, as doors does; descriptions upload the local images they name.
  • --service is inferred from the URL host and --type from a docs.google.com path, with an explicit flag winning.
  • Google documents publish on create unless --draft is given; on update, status is untouched unless --publish.
  • files show breadcrumbs for these records now point at the new update commands, naming the ID create prints since files list doesn't list them; follow-up commands carry the profile and account.
  • --subscribe is omitted from cloud-files update because Basecamp only applies subscriptions to drafted records, and cloud files are always active; on google-docs update it affects drafts only.
  • Subscriptions on files upload are still unsupported, so the gap card stays open for that.

Written for commit a5aa00f. Summary will update on new commits.

View guided diff Turn on auto-fix

files cloud-files create/update and files google-docs create/update reach
the SDK's CreateCloudFile, UpdateCloudFile, CreateGoogleDocument and
UpdateGoogleDocument. Basecamp replaces the whole record on update, so the
update commands read it first and send back every field no flag changes.
Copilot AI balanced review requested due to automatic review settings November 7, 2026 17:29
@jeremy

jeremy commented Nov 7, 2026

Copy link
Copy Markdown
Member Author

@codex review

@github-actions github-actions Bot added commands CLI command implementations tests Tests (unit and e2e) docs labels Nov 7, 2026
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 7, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-14T16:43:23.952312Z a5aa00f Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ac407b1deb

ℹ️ 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".

Comment thread internal/commands/files_links.go Resolved

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Update commands fail to validate URL hosts, accounts, and resource types, allowing look-alike or cross-account URLs to trigger unintended reads or updates in the configured account.

3 open issues
What changed in this PR

Adds create and update commands for cloud files and Google documents under files and its aliases, extending Docs & Files support beyond reading these records.

Changes:

  • Adds service/type inference, subscription controls, and Google document draft/publish options.
  • Preserves fields not explicitly changed during full-replacement updates and adds type-specific update breadcrumbs.
  • Adds unit, validation, and smoke tests; updates API coverage and the CLI surface snapshot.

[!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/​commands/​files.go Registers new command groups and updates breadcrumbs.
internal/​commands/​files_links.go Implements cloud file and Google document creation and updates.
internal/​commands/​files_links_test.go Tests inference, validation, subscriptions, and field preservation.
e2e/​smoke/​smoke_lifecycle.bats Marks shared alias commands as covered by canonical commands.
e2e/​smoke/​smoke_file_links.bats Adds live create/update smoke scenarios.
e2e/​files.bats Adds command-line validation tests.
API-COVERAGE.md Documents cloud file and Google document endpoint coverage.
.surface Records new commands and flags across aliases.

Review effort: Balanced

Comment thread internal/commands/files_links.go Resolved
Comment thread e2e/smoke/smoke_file_links.bats Resolved
Comment thread internal/commands/files_links.go Resolved

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 8 files

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread internal/commands/files_links.go Resolved
Comment thread internal/commands/files_links.go Resolved
Comment thread e2e/smoke/smoke_file_links.bats Resolved
Comment thread e2e/smoke/smoke_file_links.bats Resolved
Comment thread internal/commands/files.go Resolved
…ocument writes

Update takes a URL only from a trusted Basecamp host, naming one record of
its own type in the configured account, as doors does. Descriptions upload
the local images they name, as the other Docs & Files writes do. The smoke
runner runs smoke_file_links.bats, and the files catalog lists the two
new groups.
Copilot AI balanced review requested due to automatic review settings November 7, 2026 17:48
@jeremy

jeremy commented Nov 7, 2026

Copy link
Copy Markdown
Member Author

@codex review

@cubic-dev-ai cubic-dev-ai Bot left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 5 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread internal/commands/files_links_test.go Resolved
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: 9cae368f5e

ℹ️ 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".

@jeremy

jeremy commented Nov 7, 2026

Copy link
Copy Markdown
Member Author

@codex review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Recovery hints recommend listings that cannot expose the needed IDs, and follow-up commands omit account/profile scope, so copied commands can fail to find the intended records.

2 open issues
1 previously missed issue

In code that hasn't changed since last review

Medium severity Include effective account and profile in breadcrumbs

internal/​commands/​files_links.go:270

The new show/update breadcrumbs omit the effective account and active profile, so copying them after using a one-off --account, --profile, or URL-supplied account can fail to find the record. Capture newFollowUpScope(app) before account resolution or URL adoption, then append scope.args(app.Config.AccountID) as doors.go does. Apply this to all four new handlers and the new update breadcrumbs in internal/commands/files.go:1555–1561.

2 resolved since last review

Review effort: Balanced

Comment thread internal/commands/files_links.go Resolved
Copilot AI balanced review requested due to automatic review settings November 7, 2026 17:53
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: e862ef6223

ℹ️ 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".

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Follow-up commands lose profile/account scope and may use the wrong credentials or account, while listing guidance incorrectly suggests that files list includes cloud files and Google documents.

2 open issues
1 previously missed issue

In code that hasn't changed since last review

Medium severity Preserve account scope in follow-up commands

internal/​commands/​files_links.go:270

With --profile, a one-off --account, or an account adopted from an update URL, these numeric-ID breadcrumbs lose the invocation's scope; copying them can use different credentials/account or fail noninteractively. Capture newFollowUpScope(app) before account resolution or linkIDArg, then append scope.args(app.Config.AccountID) to each new show/update breadcrumb, as doors does. Apply this to both create/update command pairs here and the update hints in internal/commands/files.go:1555,1561.

Review effort: Balanced

…p the account in follow-ups

files list doesn't list cloud files or Google documents, so the hints name
the ID create prints instead. Follow-up commands carry the profile and an
account that came from a URL or a flag, as doors does.
Copilot AI balanced review requested due to automatic review settings November 7, 2026 18:08
@jeremy

jeremy commented Nov 7, 2026

Copy link
Copy Markdown
Member Author

@codex review

Copilot AI previously approved these changes Nov 7, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approved

No active review issues block the new cloud-file and Google-document create/update commands.

1 open issue
1 resolved since last review

Review effort: Balanced

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cb8d8fa84a

ℹ️ 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".

Comment thread internal/commands/files.go Resolved
@jeremy

jeremy commented Nov 7, 2026

Copy link
Copy Markdown
Member Author

🤖 Converged on cb8d8fa: CI green, Copilot approved and Codex reported on this head, no open threads.

Changed in review:

  • Update takes a pasted URL only from a trusted Basecamp host, naming one record of its own type in the configured account (as doors does).
  • Descriptions upload the local images they name.
  • smoke_file_links.bats is registered in the smoke runner, and the smoke test asserts the description field directly.
  • The files catalog lists the two groups.
  • Hints no longer point at files list, which doesn't list these.
  • Follow-ups carry the profile and account.

Declined, with reasons in their threads:

  • Codex's wrong-type URL concern: bc3 scopes the lookup by type, and it is now refused up front anyway.
  • cubic's "send the current status back": bc3 keeps the recording's status when it is omitted.
  • Scoping files show's breadcrumbs: they were unscoped before this PR, like the rest of files.go.

cubic last reviewed 9cae368. Locally, TestHardenConfigDir in internal/cli fails the same way on the base commit on this machine and passes in CI. Marking ready for a person to merge.

@jeremy
jeremy marked this pull request as ready for review November 7, 2026 18:17
@jeremy
jeremy requested a review from a team as a code owner November 7, 2026 18:17
…950575

* origin/main:
  Fix show lookups for supported Basecamp item URLs (#1017)
  Stamp v0.25.0 (#1016)
  Put the gist of the writing instructions in every agent session (#1014)
  Stamp v0.24.3 (#1013)
  Read a slow keyring instead of calling its credential missing (#1011)
Copilot AI balanced review requested due to automatic review settings November 9, 2026 16:51
Copilot AI dismissed their stale review, a newer Copilot review was requested November 9, 2026 16:51
@jeremy

jeremy commented Nov 9, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: 2c6af3cc1b

ℹ️ 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".

Copilot AI previously approved these changes Nov 9, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approved

No active review issues block the new cloud-file and Google-document create/update commands.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

@jeremy

jeremy commented Nov 9, 2026

Copy link
Copy Markdown
Member Author

🤖 Converged again on 2c6af3c, which merges main through 7eba5aa (no conflicts; main's show URL lookups, keyring and instructions changes come in untouched). CI green, Copilot approved and Codex found no issues on this head, no open threads. Locally every bin/ci target passed except TestHardenConfigDir in internal/cli, the same machine-only failure noted before; neither this PR nor the merge touches that package. Ready for a person to merge.

…950575

* origin/main:
  Keep a details fold written in Markdown (#1024)
  Add basecamp feedback, shipped dark behind endpoint detection (#1021)
  Stamp v0.26.0 (#1023)
  Let connectors hear selected Campfires without mentions (#1022)
@jeremy
jeremy requested a balanced review from Copilot November 10, 2026 16:47
@jeremy

jeremy commented Nov 10, 2026

Copy link
Copy Markdown
Member Author

@codex review

Copilot AI dismissed their stale review, a newer Copilot review was requested November 10, 2026 16:47
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: 2c299d674a

ℹ️ 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".

Copilot AI previously approved these changes Nov 10, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approved

No active review issues block the new cloud-file and Google-document create/update commands.

No open issues 🎉

Review effort: Balanced

@jeremy

jeremy commented Nov 10, 2026

Copy link
Copy Markdown
Member Author

🤖 Converged again on 2c299d6, which merges main through da36c00 (#1021–#1024, including basecamp feedback from #1021). Both sides touched commands.go and commands_test.go, and they merged without conflict. bin/ci is green on the merge. CI green, Copilot approved, Codex found no issues, no open threads. Ready for a person to merge. The card stays in Triage, since upload subscribers are still open there.

* commit 'ef2b350fa4662d0060c2b5e90bcbe69e32e76dd0':
  Stop telling an agent to send a Markdown fold as HTML (#1030)
  Update nix flake and plugin version for v0.30.0 (#1042)
  Watch a connector in a terminal, and talk to its orchestrator (#1039)
  Lift the connector's cap on workers, and let a start set one (#1040)
  Never give the connector's monitor a timeout (#1035)
  Update nix flake and plugin version for v0.29.0 (#1037)
  Let assignments notify people with --notify (#1034)
  Update nix flake and plugin version for v0.28.1
  Bind local config trust to approved settings
  Update nix flake and plugin version for v0.28.0 (#1029)
  Let the connector do the work itself, on the coding agent you choose (#1026)
  Build with Go 1.26.11 (#1027)
  Open a release PR for the version bump instead of pushing it to main (#1025)
  Update nix flake and plugin version for v0.27.0
Copilot AI balanced review requested due to automatic review settings November 14, 2026 16:39
Copilot AI dismissed their stale review, a newer Copilot review was requested November 14, 2026 16:39
@jeremy

jeremy commented Nov 14, 2026

Copy link
Copy Markdown
Member Author

@codex review

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Full-record updates rely on SDK reads with unverified freshness under default caching and no established protection against intervening edits, requiring human validation of the data-loss risk.

No open issues 🎉

Review effort: Balanced

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: a5aa00ff57

ℹ️ 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".

@jeremy

jeremy commented Nov 14, 2026

Copy link
Copy Markdown
Member Author

🤖 Not doing this — answering Copilot's review of a5aa00f, which asks a person to look at the data-loss risk in cloud-files update and google-docs update. Both read the record and send every unchanged field back, because Basecamp replaces the whole record on update.

  • Freshness of the read: not a risk. The SDK's cache is a conditional request: every GET goes to Basecamp with If-None-Match, and the cached body is used only when Basecamp answers 304 for it (etag_doer.go in the SDK). No read is answered from the cache alone.
  • An edit between the read and the write: true, and unchanged since this PR opened. An edit someone else saves between the GET and the PUT is overwritten for the fields this run didn't set. The SDK has no conditional write to guard it with (nothing in it sends If-Match), and the alternative, sending only the flags given, would blank every other field on every update. Check-in schedule updates on main already fill in from a read the same way.

Whether body-replacing updates should get an optimistic check is the open question on this card, which covers every such command. No change in this PR; that decision is a person's.

@jeremy

jeremy commented Nov 14, 2026

Copy link
Copy Markdown
Member Author

🤖 Converged again on a5aa00f, which merges main through ef2b350 (#1025–#1042: SDK v0.36.0, the connector work, --notify on assignments). No conflicts. bin/ci is green on the merge and CI is green on the head. Codex found no issues on this head. Copilot reviewed it and commented rather than approved, with no inline findings; no open threads.

Review bodies: 1 finding answered (declined).
Declined: Copilot's review: the read these updates start from is revalidated with Basecamp on every GET, so it isn't stale.
Open for a decision: the same review: whether body-replacing updates need a check against an edit landing between the read and the write. That is a person's call, for every such command, on this card. My recommendation: not in this PR.

Ready for a person to merge.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commands CLI command implementations docs tests Tests (unit and e2e)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants