feat(sheet-sync): Google Sheets ↔ task board two-way sync - #14
Merged
Conversation
Parse and validate .forge/sheets/<alias>.toml into SheetMapping via a RawMapping -> TryFrom pipeline: id_column present and mapped to `id`, every writable field mapped, columns non-empty, and every status_map target resolved through forge_tasks::TaskStatus::parse. Each check fails loud, naming the offending value. Adds alias_path() ancestor walk mirroring crates/tasks/src/config.rs. TDD: RED captured against a no-op TryFrom stub before implementing the real validation logic.
…cted start Extract private testable core locate_alias_path(start, alias) that walks ancestors from an injected path. Keep public alias_path(alias) unchanged, calling the core with current_dir() and preserving fallback semantics. Add two tests exercising ancestor walk and cache miss via tempfile fixtures.
…dup handling Adds model::Grid (raw sheet headers/rows) and mapping::map_grid, the header-resolution boundary between the Sheets API and CanonicalRow. Fails loud on any mapping.columns header missing from the grid; excludes blank-id rows from duplicate detection; excludes duplicate-id and terminal-status rows from the mapped result (recording the latter in skipped_terminal); tolerates ragged rows without panicking.
State/StateEntry store the .forge/sheets/<alias>.state.toml join between sheet rows and forge tasks (task_id, content_hash, last_push_status). load() treats a missing file as an empty State rather than an error; save() creates parent dirs and best-effort tightens permissions on Unix without failing the save if chmod is unsupported. upsert() inserts or overwrites by row_id.
Add board_plan.rs: pure logic (no CQL/network) that turns CanonicalRows
into BoardOp::{Create,Update,Skip}. build_create_request composes
title/body/priority/skills/metadata deterministically; target_status
resolves a row's Status through SheetMapping.status_map (defaulting to
Triage); content_hash is an inline FNV-1a over sorted field kv pairs
(no new deps, no clock/random). plan_pull applies the never-move-backward
rule: once a task's existing board status is dev-owned (InProgress,
Blocked, Complete), an Update patch leaves status untouched rather than
letting a stale sheet row push it backward.
…ld ids
- Rename DEV_OWNED_STATUSES to PROTECTED_STATUSES and add TaskStatus::Archived
so a pull can never un-archive a closed task.
- Add CanonicalField::as_canonical_str() and use it in content_hash instead of
the {field:?} Debug format, keeping the persisted content_hash stable across
Debug-format changes.
…t radius Adds plan_push (crates/sheet-sync/src/push_plan.rs): turns a PushRequest into the exact CellEdits to write back to the sheet. Fails loud on a missing id column, an unknown row_id, or an ambiguous/duplicate row_id. Silently drops a requested field outside mapping.writable (blast radius), but errors if a writable field's header is missing from the grid. Status edits additionally require the never-move-backward handoff gate: the new value must be in dev_writable_status and the row's current sheet Status must not already be terminal; fix_ver/resolution_notes have no such gate. A defense-in-depth pass re-checks every emitted edit against mapping.writable before returning. Adds CellEdit and col_index_to_a1 (0-based index -> spreadsheet column letters) to model.rs, reusable by the later A1-addressing writer task. Pure logic, no network. 9 new tests cover the brief's (a)-(g) cases plus ragged-row and no-op-request edge cases; 44/44 crate tests green, clippy clean, fmt clean, workspace builds.
…with dry-run Adds the I/O boundary traits (SheetsApi, BoardSink) plus FakeSheets/FakeBoard test doubles, and wires the existing pure engine (mapping, board_plan, push_plan, state) into end-to-end pull/push orchestration with dry-run support — fully testable without network or CQL.
…actually written push() previously set last_push_status = req.status.clone() whenever any edit was written, regardless of whether a Status-column edit was among them. This clobbered a real prior last_push_status on a fix_ver/notes-only push (req.status: None), and phantom-recorded a status that was never written when plan_push's terminal-status handoff gate dropped the status edit. Now last_push_status only updates when edits actually contains an edit for the mapping's Status header.
…task request structs) - BoardExec wraps forge_tasks::TaskStore and implements BoardSink: Skip is a no-op, Create calls create_task then a follow-up update_task when the target status isn't the store default (Triage), Update calls update_task directly. - needs_status_followup/status_patch are pure, unit-tested without a store; a #[ignore]d live test exercises the real create+follow-up path against a CQL node on 127.0.0.1:9042. - Add Clone to CreateTaskRequest/UpdateTaskPatch so apply(&BoardOp) can clone the request/patch out of the borrowed op.
Adds the live SheetsApi impl (crates/sheet-sync/src/sheets/google.rs) over the Google Sheets v4 REST API via ureq, and the installed-app OAuth 2.0 loopback flow + refresh-token cache (crates/sheet-sync/src/oauth.rs). Branching logic lives in pure, unit-tested helpers (parse_client_secret_json, parse_token_response, build_auth_url, parse_redirect_query, parse_values_response, build_batch_update_body, quote_sheet_name); the socket/HTTP glue itself is thin. Required trait change: SheetsApi::write_cells now takes a `tab: &str` (Google's values:batchUpdate ranges must be sheet-qualified, e.g. 'QA Log'!M2, for multi-tab spreadsheets) — updated FakeSheets and the sync::push call site accordingly; existing sync tests stay green. Adds ureq/base64/dirs deps to crates/sheet-sync/Cargo.toml (versions match crates/ingest/crates/fmem-client, already in the dependency tree, so no new cargo-deny surface).
…hmod failures, bound loopback read ureq v3 defaults http_status_as_error=true, making the oauth.rs/google.rs status.is_success()+redact_token_fields error branches unreachable dead code; disable it on both agents so Google's error_description survives. Also log (not swallow) refresh-token chmod failures, and bound the loopback redirect's read_line to 8KiB.
Wires forge_sheet_sync into the frg CLI (`frg sheet auth|pull|push`) and the MCP server (sheet_auth/sheet_pull/sheet_push, tier 1). Adds a shared `forge_sheet_sync::resolve_alias` helper so both entry points resolve an alias's .forge/sheets/<alias>.toml mapping and derive its sidecar state path the same way, and fail loud on a bad alias before any OAuth/CQL/ network call.
- crates/sheet-sync/examples/spoton-qa.toml: committed example mapping (git-ignored .forge/sheets/ is the runtime location) with correct key ordering (writable/dev_writable_status/terminal_status before [columns]/[status_map], per config.rs's parses_the_spec_example fixture) - tests/example_mapping.rs: guards the shipped example against rot via include_str! + SheetMapping::from_toml_str - skills/sheet-sync/SKILL.md: operating procedure for pull/work/push - CHANGELOG.md: Unreleased/Added entry for the sheet-sync feature
…oud duplicate headers - pull now persists sidecar state after each successful Create/Update, not once at the end, so a mid-loop board.apply failure can't cause a re-pull to duplicate already-created tasks. - SheetsApi::write_cells re-asserts the write blast radius (assert_edits_within_writable) against an explicit allowed_headers set before any network/record step, independent of push_plan's own filtering. - map_grid and plan_push both refuse (ensure_unique_mapped_headers) a sheet with a duplicated *mapped* header, since map_grid's HashMap index (last-wins) and plan_push's .position() (first-match) would otherwise silently resolve it to different columns.
… disclose state chmod failures Three whole-branch-review minors hardened in crates/sheet-sync: - oauth::generate_state now draws 16 bytes from getrandom (OS CSPRNG), hex-encoded, instead of a predictable time+pid+port mix, closing a CSRF weakness in the state nonce. getrandom is pinned to 0.2 to reuse the major already pulled in by the ureq/rustls-platform-verifier/ring chain. - oauth::token_cache_path returns Result instead of panicking via dirs::config_dir().expect(...), since it's a pub fn in a library crate; save_refresh_token/load_refresh_token propagate the error. - state::tighten_permissions_best_effort now eprintln!s (path + OS error only) on a chmod failure instead of discarding it via `let _ = ...`, matching the disclosure pattern already used in oauth.rs's identical helper. Still best-effort — a chmod failure does not fail the save.
…ecs (no client IDs in repo)
bkearns
force-pushed
the
feat/gsheet-task-sync
branch
from
July 14, 2026 21:51
1f79d8c to
6fdd4c7
Compare
…T) for headless pull/push Adds a Google service-account JWT-bearer flow (RFC 7523) as an alternative to the interactive OAuth loopback flow, so `frg sheet pull/push` works in headless/CI/agent environments with no browser. Signs with `ring::signature::RsaKeyPair` (RS256/PKCS#1 v1.5 over SHA-256) rather than the RustCrypto `rsa` crate, which fails `cargo deny` (RUSTSEC-2023-0071, Marvin-attack timing side channel). `ring` is constant-time, carries no such advisory, and was already in the tree transitively via ureq -> rustls -> ring, so `cargo deny check` stays green. - `service_account`: parses a Google SA key JSON, builds+signs the JWT assertion, exchanges it at the token endpoint for an access token. Offline round-trip test verifies the RS256 signature cryptographically against a throwaway fixture key, with no network. - `credentials`: resolves service-account (preferred, headless) vs. OAuth (interactive) based on `FORGE_GOOGLE_SERVICE_ACCOUNT` / `[google] service_account_path`, mirroring the existing OAuth client's env/config precedence. Fails loud naming both routes if neither is configured. - Rewires the CLI + MCP `sheet_auth`/`sheet_pull`/`sheet_push` call sites onto `credentials::access_token`/`authorize`; `sheet_auth` is now a no-op headless check when a service account is configured.
…p docs + redact key in Debug
Adds crates/sheet-sync/examples/provision-service-account.sh (gcloud-based,
project/SA-name/key-path parameterized, tolerant of pre-existing project/SA,
prints the SA email and next steps loudly), documents the headless
service-account auth path as the recommended one for agents in
skills/sheet-sync/SKILL.md and README.md (alongside the existing interactive
OAuth path), and hardens ServiceAccount's Debug impl to redact
private_key_pem so it can never leak via an incidental {:?}.
The QA sheet auto-fills its id column ~1000 rows past the last real bug, so pull was importing ~990 empty template rows as junk tasks. A row with a non-blank id but a blank/absent Title is not a bug (same heuristic as the working Python sync) — map_grid now excludes it into a new skipped_empty list, and PullReport reports the count.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds a generic Google Sheets ↔ task board two-way sync to forge as a new crate
crates/sheet-sync(forge-sheet-sync), surfaced asfrg sheet auth|pull|pushand MCP toolssheet_auth/sheet_pull/sheet_push. Bumps the workspace to0.15.0.The motivating use case: drive development agentically from a client-owned Google Sheet of bugs — pull open rows into the forge task board, work them, and push fix status/version/notes back to the sheet the client already reads, without ever clobbering their cells.
Design
normalize,mapping,board_plan,push_plan,sync) is pure and unit-tested againstFakeSheets/FakeBoard; onlyGoogleSheets(ureq) andBoardExec(CQLforge-tasks) touch the outside world. Keeps the coverage floor reachable and the safety rules testable..forge/sheets/<alias>.toml(git-ignored; example committed atcrates/sheet-sync/examples/spoton-qa.toml): column map,writableset,status_map,dev_writable_status,terminal_status..forge/sheets/<alias>.state.toml(the store has no upsert-by-external-key), persisted incrementally so a mid-batch failure never duplicates tasks on re-pull.spreadsheetsscope only, CSPRNG CSRF nonce, refresh token 0600 in~/.config/forge/, tokens redacted from all diagnostics.Write-back safety (writes to someone else's sheet)
writablecolumns are ever written; enforced inpush_planand re-asserted at thewrite_cellsboundary (defense in depth).In Progress,In Review,Fixed - Needs Verification) and never overwrites client-owned/terminal states.fix_ver/resolution_notesare always ours.Archived).--dry-runon both pull and push prints the exact effect and writes nothing.Tests & gates
cargo test --workspace= 1262 passed / 0 failed; fmt /clippy --workspace -D warnings/cargo doc -D warnings/cargo denyclean; coverage lines 70.5% / fn 68.1% / regions 72.4% (above the 70/66/68 floors). Live Google/CQL paths are#[ignore]-gated.Docs
Design spec
specs/implemented/feat-sheet-sync.md; operating skillskills/sheet-sync/SKILL.md; CHANGELOG under Unreleased.Deferred (follow-ups)
Evidence/attachments canonical field yet (the QA sheet has such a column).sheet_authover MCP blocks on interactive browser consent — auth is CLI-first.