fix(sheets): correct Connected Sheets sheet identity and SYNC_ALL extract reads

[ryo-touch]

Refs #938

Live validation of the read-only Connected Sheets surface from #989, against a real set of
BigQuery-backed spreadsheets, surfaced two defects. Both are invisible to the current tests
because the fixture does not match what the API actually returns.

Summary

• resolve a data source's linked sheet by data source id, and treat that sheet's own id as
  authoritative, so sheets datasource list stops reporting sheetId: 0
• fall back to an unranged, properties-only fetch when a ranged spreadsheets.get cannot supply
  an extract's column count, which unblocks every SYNC_ALL extract
• make the fixture server range-aware and drop dataSources[].sheetId from the fixture, so both
  paths are exercised the way the live API behaves

Details

datasource list always reported sheetId: 0. Live spreadsheets.get responses do not
populate Spreadsheet.dataSources[].sheetId; it comes back null. sheetsDataSourceToItem emitted
source.SheetId directly. The correct id is already available on the matched sheet, and
datasource table list uses it, so the two code paths now agree.

That same zero value also made findSheetsDataSourceSheet unsafe: it compared
properties.SheetId == source.SheetId before checking the data source id, so in a spreadsheet
containing a tab with sheet id 0, every data source would match that unrelated tab and report its
title and dataExecutionStatus. I could not reproduce this in practice — none of the 59
spreadsheets I scanned has a tab with sheet id 0 — so I am flagging it as latent rather than
observed. The ordering is wrong either way.

datasource table read failed for every SYNC_ALL extract. A SYNC_ALL table carries no
inline column list, so the column count has to come from dataSourceSheetProperties on the
associated DATA_SOURCE sheet. The anchor lookup passes ranges, and a ranged spreadsheets.get
returns only the sheets those ranges intersect, which excludes the DATA_SOURCE sheet:

ranges absent  -> grid sheets + DATA_SOURCE sheets
ranges present -> the anchor's grid sheet only

So dataSourceColumnCount found nothing and the command exited with
cannot determine columns for data-source table.

Proof

• make ci
• go test ./internal/cmd -run 'TestSheetsDataSource' -count=1 -v
• both new assertions were confirmed to fail before their corresponding fix: the sheet-identity
  test reports the decoy tab, and the SYNC_ALL read test reproduces the exact
  cannot determine columns error

Live validation was read-only throughout (--readonly on every invocation; no spreadsheet was
modified):

	before	after
data sources resolving a real linked sheet id	0 / 56	56 / 56
extracts readable via datasource table read	0 / 54	54 / 54

Corpus: 59 spreadsheets scanned, 21 with Connected Sheets, 54 anchored extracts. Every extract in
it uses SYNC_ALL; there was not a single SELECTED extract, which is why this path failing was
total rather than partial.

Two notes:

• the SYNC_ALL fallback costs a second spreadsheets.get on that path. Reading all 54 extracts
  back to back tripped Sheets rate limits a few times; pacing the loop cleared it. Worth knowing
  before scripting bulk extract reads.
• docs/sheets-connected.md presents --extra-scopes .../bigquery.readonly --force-consent as a
  required step. It is not required when the stored token already carries a superset: all of the
  above ran on a token holding cloud-platform and spreadsheets, with no re-consent. That may
  be a cheaper route to a live Connected Sheets environment than provisioning the exact scopes.

[clawsweeper]

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

[clawsweeper]

Codex review: needs maintainer review before merge. Reviewed August 17, 2026, 1:47 AM ET / 05:47 UTC.

ClawSweeper review

What this changes

This PR resolves a Connected Sheet by data-source ID and fetches unscoped column metadata when a SYNC_ALL extract lacks inline columns.

Merge readiness

✅ Ready for maintainer review

Keep open: current main still has both Connected Sheets read defects, while this focused PR supplies a source-aligned fix, regression coverage, and credible live after-fix validation. No blocking patch finding was identified.

Priority: P2
Reviewed head: 4d8b7d6467f659f6834a140c214db4c6bc9617ad

Review scores

Measure	Result	What it means
Overall readiness	🦞 diamond lobster (5/6)	Strong live validation and targeted regression coverage support a high-confidence, narrowly scoped repair.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (live_output): The PR body reports read-only, after-fix validation on 59 real spreadsheets: linked sheet IDs resolved for 56 of 56 sources and 54 of 54 SYNC_ALL extracts became readable.
Patch quality	🦞 diamond lobster (5/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (live_output): The PR body reports read-only, after-fix validation on 59 real spreadsheets: linked sheet IDs resolved for 56 of 56 sources and 54 of 54 SYNC_ALL extracts became readable.
Evidence reviewed	5 items	Current-main defect: Current main emits the top-level data source's SheetId and matches that ID before its data-source ID; an omitted API field therefore has Go's zero value and can select an unrelated sheet 0. SYNC_ALL failure path: Current main only obtains a column count from the range-scoped response and returns an error when that response omits the separate DATA_SOURCE sheet. Focused implementation: The branch resolves the linked sheet by data-source ID first, treats its sheet ID as authoritative, and makes a narrow unscoped metadata request only when no column count is available.
Findings	None	None.
Security	None	None.

How this fits together

The Sheets CLI queries Google Sheets metadata to list Connected Sheets data sources and locate extract anchors before issuing a bounded values read. The linked data-source sheet supplies status and, for SYNC_ALL extracts, the column definitions needed to form that read.

flowchart LR
A[Sheets CLI command] --> B[Google Sheets metadata]
B --> C[Linked data-source sheet]
C --> D[Extract column count]
D --> E[Bounded values request]
E --> F[CLI JSON or table output]

Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric	Value	Why it matters
Production versus regression coverage	production +41/-2, docs +2, tests/fixtures +215/-7	Most growth models the API’s real range behavior and protects both corrected paths.

Technical review

Best possible solution:

Land the scoped fix after required checks complete, retaining the additional metadata request only for extracts whose range-scoped response cannot provide columns.

Do we have a high-confidence way to reproduce the issue?

Yes—current main visibly lacks the SYNC_ALL metadata fallback and uses the absent top-level sheet ID before data-source identity; the branch adds focused fixtures for both paths and reports live before/after validation.

Is this the best way to solve the issue?

Yes—the conditional, properties-only fallback preserves SELECTED extract behavior while obtaining the one missing metadata value for SYNC_ALL extracts.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against eb85a99366eb.

Labels

Label justifications:

• P2: The PR repairs a bounded Connected Sheets read/list defect with no evidence of an urgent broader outage.
• rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body reports read-only, after-fix validation on 59 real spreadsheets: linked sheet IDs resolved for 56 of 56 sources and 54 of 54 SYNC_ALL extracts became readable.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body reports read-only, after-fix validation on 59 real spreadsheets: linked sheet IDs resolved for 56 of 56 sources and 54 of 54 SYNC_ALL extracts became readable.

Evidence

What I checked:

• Current-main defect: Current main emits the top-level data source's SheetId and matches that ID before its data-source ID; an omitted API field therefore has Go's zero value and can select an unrelated sheet 0. (internal/cmd/sheets_datasource.go:371, eb85a99366eb)
• SYNC_ALL failure path: Current main only obtains a column count from the range-scoped response and returns an error when that response omits the separate DATA_SOURCE sheet. (internal/cmd/sheets_datasource.go:289, eb85a99366eb)
• Focused implementation: The branch resolves the linked sheet by data-source ID first, treats its sheet ID as authoritative, and makes a narrow unscoped metadata request only when no column count is available. (internal/cmd/sheets_datasource.go:293, 4d8b7d6467f6)
• Regression coverage: The branch makes the fixture range-aware, covers a decoy sheet 0 and missing top-level sheet IDs, and asserts one extra lookup for SYNC_ALL versus none for SELECTED extracts. (internal/cmd/sheets_datasource_test.go:135, 4d8b7d6467f6)
• Feature provenance and ownership: Blame attributes the current read and sheet-resolution path to the released Connected Sheets implementation; repository context identifies its merged implementation PR as the feature origin. (internal/cmd/sheets_datasource.go:267, 45b5d766e137)

Likely related people:

• steipete: Repository context identifies steipete as the author of the merged Connected Sheets read implementation, and current-path blame resolves to the release that contains it. (role: original feature author and likely follow-up owner; confidence: high; commits: a4c96debef00, 45b5d766e137; files: internal/cmd/sheets_datasource.go, internal/cmd/sheets_datasource_test.go, docs/sheets-connected.md)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (1 earlier review cycle)

• reviewed 2026-08-17T05:20:59.299Z sha ccc98ef :: needs maintainer review before merge. :: none