refactor(compass): drop the unauthorized Store.UpdateMessageBlocks

[seal-agent]

Deletes Store.UpdateMessageBlocks — an exported message write with no membership check and no authorship check, and zero production callers.

Why it had to go rather than be documented

The method took a bare MessageID and rewrote that message's block set. No check that the caller is a member of the channel, no check that they authored the message. Its only references were its own definition and three tests.

AnswerAsk does not use it — it calls the unexported updateMessageBlocksExec directly against its transaction (messages.go:368), where the row has already been resolved through a membership JOIN and locked FOR UPDATE.

So it was a test-only exported method with no authorization, sitting beside authorizing writes. That is the trap worth removing: the unauthorized one is shorter to call and passes every test, so the next person reaching for "update a message's blocks" finds it first and it works. A doc comment saying "do not call this on a path fed by external input" is not a guard — and prose mistaken for a guard is the failure this codebase has hit repeatedly.

Deleting it is cheap precisely because nothing calls it. There is no migration and no caller to update; the exported surface simply loses a method that should never have been exported.

What changed

The wrapper is gone. Its three tests now call updateMessageBlocksExec directly (same package), preserving every assertion:

• TestUpdateMessageBlocksReplaces — block set is replaced, verified through ListMessages.
• TestUpdateMessageBlocksRejectsEmptyAskID — ErrInvalidArgument, the rejected update does not persist, and a non-empty ask id still round-trips.
• TestUpdateMessageBlocksUnknownNotFound — ErrNotFound for an unknown id.

The wrapper's substantive documentation (empty-set and unknown-message rejection, the ask-id immutability rationale) moved onto updateMessageBlocksExec rather than being deleted with it, plus a line stating it performs no membership or authorship check and is therefore deliberately unexported. Three stale comments that named the removed method as a live mutator were corrected.

The test function names are unchanged: they name the behavior under test — updating message blocks — which still exists. Renaming them would say something false about what disappeared.

updateMessageBlocksExec and AnswerAsk are byte-identical to main, verified by extracting each function body from both refs and comparing checksums, not by reading the diff. The only non-comment change in messages.go is the three deleted wrapper lines.

Verification

go build, go vet (including -tags "pgtest unix", since an untagged vet would not analyze the changed test file at all), gofmt -l empty, -race suite green, and golangci-lint with the same build tags reporting zero findings in hand-written code.

Real-Postgres pgtest run with -v against a live DSN: the three repointed tests PASS, and the full internal/store package shows 96 literal --- PASS lines with zero SKIP. That last part matters here — pgtest.RequireDSN skips rather than fails when no container runtime is available, so a bare ok can mean the entire suite silently skipped. These were confirmed executing.

Credit

Found by the comms lane, which spotted that the exported wrapper had no callers while I was adding an authorizing variant beside it. Matt ruled the deletion.

Spec-impact: none

Removes an unexported-in-effect internal method with no callers; no externally-visible behavior changes.

Targets main for review; lands in sealedsecurity/compass after the port.

---

Ported from sealedsecurity/sealed#1000, unchanged in content — Compass code now lives here, so this is where it ships. The sealed PR is closed in favour of this one.

Verification (this repo has no CI yet, so a local run is the only evidence there is): go build ./..., go vet -tags "pgtest unix" ./..., gofmt -l clean, go test -race ./internal/... ./server/..., and the pgtest suites for ./internal/store/, ./internal/comms/, ./server/ against a live postgres:17 — all green on this branch.

[seal-agent]

Cross-branch integration verified locally. This repo has no CI yet, so this is the only integration evidence there is — and it is the check that caught a real defect in the sealed copies of these same diffs (a deleted symbol still cited by a sibling branch, invisible to any single-diff review).

Merged all five of this lane's branches together onto main — #29, #30, #31, #32, #33 — in one tree:

merge boundary rc=0   merge drop rc=0   merge ask rc=0   merge t3 rc=0   merge schema rc=0

All five merge clean, no conflicts. On the combined tree, against a live postgres:17:

gofmt -l           clean
go build ./...     BUILD_OK
go vet -tags…      VET_OK
go test -race ./internal/... ./server/...        all pass
go test -tags pgtest ./internal/store/...        ok  39.358s
go test -tags pgtest ./internal/comms/...        ok  23.879s
go test -tags pgtest ./server/...                ok  20.286s

Worth stating plainly: ./server/ only goes green here because #29 is in the tree. On main today TestServeShutdownWithLiveCommsSubscriberReturnsClean fails against a live Postgres — pre-existing, confirmed on a clean main checkout — and #29 is the fix. The other four each show that same failure alone, and it is not theirs.

A per-package throwaway Postgres container also starves under this suite's parallelism (the store package hit its 600s timeout four times over); one shared database runs the same suite in ~30s. Relevant when CI is set up here.

[greptile-apps]

Greptile Summary

This PR removes the unused exported message-block update wrapper while retaining the existing internal transactional update helper.

• Repoints store tests directly at updateMessageBlocksExec.
• Moves the update invariants and authorization warning onto the unexported helper.
• Updates stale comments in the Go store and UI state code.

Confidence Score: 3/5

The message-update refactor appears sound, but the PR should not merge while the previously reported orphaned-container failure remains and the other prior blockers are unresolved.

ProvisionAgentWorkspace still returns after durable mapping failure without rolling back the workspace already created on the Runner; the current evidence also does not establish that the prior runner-scoping and migration findings have been fixed or invalidated.

Files Needing Attention: go/server/service.go, go/internal/store/agent_placements.go, go/internal/store/migrations/0004_agent_placement.sql

Important Files Changed

Filename	Overview
go/internal/store/messages.go	Removes an unreferenced exported wrapper while leaving the internal update implementation and AnswerAsk path unchanged.
go/internal/store/messages_test.go	Repoints the existing block-update behavior tests to the unexported helper without changing their assertions.
apps/ui/src/live/comms-state.ts	Rewords a comment to avoid naming the removed Store method; runtime behavior is unchanged.

_{Reviews (4): Last reviewed commit: "docs(store): decouple the boundary comme..." | Re-trigger Greptile}

[seal-agent]

Force-pushed a correction: the first push of this branch contained 37 files / +3699/-329 instead of this PR's 3 files / +30/-33.

Cause was mine and mechanical, not a content change — I ported the five branches in a loop that applied each patch and committed with git add -A, but a git checkout between iterations carried the working tree across, so this commit swept in the other four ports' files as well.

Caught by diffing each ported branch against its sealed-original patch and comparing normalized hunks: four matched byte-for-byte, this one didn't. Rebuilt from a verified-clean main (reset --hard + clean -fd, tree confirmed empty before applying), and all five branches now match their sealed originals exactly, with file counts agreeing:

boundary  files compass=  1 sealed=  1  identical=True
drop      files compass=  3 sealed=  3  identical=True
ask       files compass=  6 sealed=  6  identical=True
t3        files compass= 15 sealed= 15  identical=True
schema    files compass= 18 sealed= 18  identical=True

Re-verified on the corrected branch: gofmt clean, go build, go vet -tags "pgtest unix", and the store + comms pgtest suites green against a live postgres:17.

Note the integration transcript in my earlier comment was run on the merged tree of all five, so its result stands — but it could not have caught this, because a union-of-all-five commit merges into that same tree cleanly. Only the per-branch fidelity check found it.

[sealedsecurity-bot]

Reconciling the three open Greptile P1 threads — all obsolete

Greptile posted three P1 review comments against commit 644f1a7:

• runner-unscoped session attribution (go/internal/store/agent_placements.go)
• backfill discards container mappings (go/internal/store/migrations/0004_agent_placement.sql)
• failed placement leaves an orphaned container (go/server/service.go)

None applies to this PR. This PR's diff is exactly three files — go/internal/store/messages.go, go/internal/store/messages_test.go, apps/ui/src/live/comms-state.ts — and none of the three cited files is among them (git diff --name-only main <head> confirms). Those threads were posted against the first push of this branch, which mechanically swept in 37 files / four other ports' changes (a git checkout between loop iterations carried the working tree across); that push was force-corrected back to the intended 3-file diff, and Greptile marked all three threads outdated accordingly.

The cited concerns are real for the PRs that actually own those files (agent_placements / the 0004 migration / the placement rollback are #33's territory, where they were reviewed on their own merits), but they are not this refactor's. Resolving the three threads as obsolete. This PR's own review (mandatory skill://review, Opus) came back CLEAN — the only change is dropping the unauthorized exported Store.UpdateMessageBlocks wrapper, with the behavioral tests repointed to updateMessageBlocksExec and their assertions intact.

[mattwilkinsonn]

• refactor(compass): drop the unauthorized Store.UpdateMessageBlocks #30 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.