Skip to content

feat: add spreadsheet-file connector#303

Open
andrei-hasna wants to merge 1 commit into
mainfrom
open-connectors-cursor/open-connectors/b02e4a11-6ef6-458f-a386-017745ae97c2-1e4b35a2
Open

feat: add spreadsheet-file connector#303
andrei-hasna wants to merge 1 commit into
mainfrom
open-connectors-cursor/open-connectors/b02e4a11-6ef6-458f-a386-017745ae97c2-1e4b35a2

Conversation

@andrei-hasna

Copy link
Copy Markdown
Contributor

Summary

  • Add connectors/spreadsheet-file package (@hasna/connect-spreadsheet-file) with Bearer-authenticated REST client, CLI, types/utils, and multi-profile config
  • Register spreadsheet-file in Data & Analytics category
  • Add focused client tests covering Bearer auth, query params, path encoding, and error handling

Test plan

  • cd connectors/spreadsheet-file && bun install && bun run typecheck && bun run build
  • bun test connectors/spreadsheet-file/src/api/client.test.ts (10 pass)
  • bun run typecheck (repo root)
  • bun test (1196 pass, 1 pre-existing unrelated fail in CLI events/webhooks test)
  • bun run dev -- --help smoke test

Implement SpreadsheetFile as a first-class open-connectors package with Bearer API client, CLI, registry entry, and focused client tests.
@andrei-hasna

Copy link
Copy Markdown
Contributor Author

Worker review blocked merge for now.

Evidence checked on 2026-07-07 from isolated worktree:

  • PR is open/non-draft; GitHub CI build is green; GitHub currently reports mergeable/clean.
  • Local safe-merge check found the PR head commit 4315d406 has parent 79f3a48c; current main is cc84ac86. A clean cherry-pick onto current main was possible locally as 26da08ba with only the expected 19 files changed, but I did not push it because functional review still blocks merge.
  • No browser-use/Playwright/Puppeteer/Selenium, internal beepmedia/hasnaxyz, blacklist-name, or obvious hardcoded credential pattern was found in the PR diff.
  • Focused validation passed on the repaired local branch: bun test connectors/spreadsheet-file/src/api/client.test.ts (10 pass), bun run --cwd connectors/spreadsheet-file typecheck, bun run --cwd connectors/spreadsheet-file build, and git diff --check origin/main..HEAD.

Merge blockers:

  1. connectors/spreadsheet-file/src/api/client.ts defaults to https://api.spreadsheet-file.com/v1, and the README documents the same endpoint. From this machine, getent hosts api.spreadsheet-file.com returned no host and curl -I --max-time 10 https://api.spreadsheet-file.com/v1 failed with Could not resolve host. I could not find public provenance for this API host. Please provide the real API base URL/provenance, or change the connector to require an explicit base URL instead of shipping an unreachable default.
  2. Env-only CLI usage can still touch local profile storage: getClient() passes getBaseUrl(), and getBaseUrl() falls through to loadProfile(), which calls ensureConfigDir()/mkdirSync(). In read-only-home or env-only use, this can fail before making an API request. The CLI should avoid profile reads/writes when SPREADSHEET_FILE_API_KEY and/or SPREADSHEET_FILE_BASE_URL supply all needed config, or handle read-only config paths gracefully.
  3. Tests are too narrow for the new surface: they cover the generic HTTP client only. Please add coverage for files/events/search wrappers, CLI/env config behavior, and any base URL behavior chosen above.

I am leaving the PR open and unmerged until these are addressed.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant