store/gcs: GCS object-store adapter + integration harness (+ Go 1.26.4)#10
Merged
Conversation
store/gcs is the GCS-backed store.ObjectStore — the last Phase-1 library slice before Morris can fully cut over (extraction, chunking, content, embedding, and now storage all land on the library). A clean lift from Morris, conformed to maestro-cms's interface. - store/gcs: Get/Put/Delete/Exists over cloud.google.com/go/storage. Keys are GCS object names verbatim (no prefix/normalization), storage.ErrObjectNotExist maps to store.ErrObjectNotFound. Deliberate divergence from Morris: Delete reports ErrObjectNotFound on a missing key per OUR interface contract, not Morris's idempotent delete. New(...option)/NewWithClient/Close/Bucket. Opt-in subpackage (ADR 0006): core `store` stays stdlib-only (verified: 0 cloud deps in `go list -deps ./store`); depguard already denies core from importing it. - Integration harness (closes deferred-tooling #1): build-tagged (//go:build integration) round-trip tests against a Dockerized fsouza/fake-gcs-server (no official Google GCS emulator exists; fsouza is the de-facto standard, also used by Google's own Go client tests). New `make test-integration` starts the emulator with -public-host (so downloads resolve), waits for readiness, runs the tagged tests via STORAGE_EMULATOR_HOST, and tears it down. Manual-dispatch integration.yml workflow runs the same on CI. Default `make test` and CI stay network-/Docker-free (tests are tag-gated and t.Skip without the emulator). - Bump Go 1.26.3 -> 1.26.4: govulncheck flagged two newly-published stdlib CVEs (net/textproto GO-2026-5039, crypto/x509 GO-2026-5037), repo-wide and reachable from ordinary code; 1.26.4 patches both. Verified govulncheck is clean under 1.26.4. Verified: make lint, go test ./..., go vet ./..., govulncheck (clean), and make test-integration (real fake-gcs-server round-trips) all pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds an opt-in Google Cloud Storage adapter (store/gcs) implementing the core store.ObjectStore interface, plus a Docker-backed integration-test harness to validate real round-trips against a GCS-compatible emulator. This completes the Phase-1 “storage” slice while keeping the core store package dependency-free (per ADR 0006).
Changes:
- Add
store/gcsadapter implementingGet/Put/Delete/Existsovercloud.google.com/go/storage, including not-found mapping tostore.ErrObjectNotFound. - Add build-tagged integration tests and a
make test-integrationharness (Dockerizedfsouza/fake-gcs-server), plus a manual-dispatch GitHub Actions workflow. - Update docs/spec + deferred-tooling status to reflect the adapter and integration harness landing.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
store/gcs/gcs.go |
New GCS-backed store.ObjectStore implementation and constructors. |
store/gcs/gcs_integration_test.go |
Build-tagged integration tests exercising the adapter against an emulator endpoint. |
Makefile |
Adds test-integration target to orchestrate fake-gcs-server + tagged tests. |
go.mod |
Adds required GCS SDK dependencies and updates the Go toolchain directive. |
go.sum |
Records new dependency checksums pulled in by the GCS SDK. |
docs/spec-v1.md |
Updates spec to mention the landed store/gcs adapter + integration test harness. |
docs/deferred-tooling.md |
Marks the integration harness/workflow item as DONE and documents approach. |
.github/workflows/integration.yml |
Adds manual-dispatch workflow to run make test-integration in CI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…; review nits Addresses external review + Copilot on #10. - Put no longer finalizes a partial object on a mid-stream reader error. A GCS Writer buffers and uploads on Close, so the prior `_ = w.Close()` after a failed io.Copy could commit a truncated object. Now the Writer gets a child context that is canceled before Close on copy failure, aborting the upload. New integration test TestGCSPutAbortsOnReaderError proves no object is left. - Client ownership is explicit: New owns the client it creates and closes it; NewWithClient does NOT (the caller keeps ownership), so sharing one client across Stores is safe. NewWithClient now panics on empty bucket / nil client (wiring bugs) instead of yielding a Store that panics on first use. - New's doc no longer overstates ctx lifetime: ctx is used only to construct the client and is not retained. - test-integration is idempotent: it pre-removes any leftover container and tolerates stop failures, so an interrupted run doesn't wedge the next. - Integration test checks the io.ReadAll error it previously dropped. Verified: make lint, go test ./..., go vet ./..., and make test-integration (all 6 store/gcs round-trips incl. the abort path) pass under go1.26.4. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ror (review nit) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
store/gcsis the last Phase-1 library slice — with it, extraction, chunking, content, embedding, and storage all live on the library, so Morris can fully cut over and retire its bespoke storage + embedding code.Adapter
store.ObjectStore(Get/Put/Delete/Exists) overcloud.google.com/go/storage. Keys are GCS object names verbatim (no prefix/normalization), per the opaque-key contract.storage.ErrObjectNotExist→store.ErrObjectNotFound.DeletereportsErrObjectNotFoundon a missing key per our interface, rather than Morris's idempotent delete — matching thetestcmsmemory fake.New(ctx, bucket, ...option.ClientOption),NewWithClient, plusClose/Bucket.storestays stdlib-only — verifiedgo list -deps ./storehas 0 cloud deps; the existing depguard rule already denies core from importing it. The SDK's large transitive tree is fully confined here.Integration harness (closes deferred-tooling #1)
fsouza/fake-gcs-server— the de-facto standard, also used by Google's own Go client tests.//go:build integration) round-trip tests: Put/Get/Exists/Delete, overwrite, and not-found mapping (incl. the divergent Delete).make test-integrationstarts the emulator (-public-hostso downloads resolve), waits for readiness, runs the tagged tests viaSTORAGE_EMULATOR_HOST, and tears it down. Manual-dispatchintegration.ymlruns the same on CI.make testand CI stay network-/Docker-free: the tests are tag-gated andt.Skipwithout the emulator.Piggybacked: Go 1.26.3 → 1.26.4 (security)
govulncheckflagged two newly-published stdlib CVEs —net/textproto(GO-2026-5039) andcrypto/x509(GO-2026-5037) — present in 1.26.3, repo-wide and reachable from ordinary code, fixed in 1.26.4. These pass yesterday's CI but would now failgovulncheckon any PR (and the weekly security scan onmain). Bumped thegodirective; verifiedgovulncheckis clean under 1.26.4. (Per the discussion, folded into this PR rather than a separate bump.)Verification
make lint,go test ./...,go vet ./...,govulncheck(clean), andmake test-integration(real fake-gcs-server round-trips) all pass under go1.26.4.🤖 Generated with Claude Code