[lilyshen0722]

TASK-278, from Cofounder's security read (Medium, reproduced on pg-mem), plus the Info finding on #2269.

1. A reply must target a message in its own pod

When no threadRootId was sent, replyToMessageId was never checked to be in the post's pod. A reply could name a message in another pod and join that pod's thread, because the INSERT derives the root from the parent. This pod's reply preview then showed the other message's text and author. Production holds 2 such rows out of 7,274 replies, in 2 pods (2026-09-09 and 2026-10-07; read-only count).

• One check. threadRootResolver.assertReplyTargetInPod refuses a target that is not an id (reply_target_invalid), or that is missing or in another pod. The last two share one code, reply_target_unavailable, so a reply cannot probe which ids exist elsewhere; which it was goes to the log. resolveThreadRoot calls it first.
• Every write path checks before it writes, so a refusal cannot fall into a Mongo fallback:
  • human composer (messageController): 400;
  • agent path (agentMessageService): propagates, and routes/agentsRuntime maps it to 400;
  • socket send (server.ts): an error event.
    A store failure inside the check is left to the write, so a PG outage keeps its existing behaviour (503 for replies).
• SQL backstop. The INSERT writes no row unless the reply target, and any named root ($7), is in the pod: $1::varchar IN (SELECT … pod_id …). A zero-row result throws reply_target_unavailable before pods.updated_at is touched. The cast is load-bearing. The first head, 08a01e3, wrote $1 IN (…); pg-mem accepted it, and real Postgres refused every write with 42P08 inconsistent types deduced for parameter $1 (Cofounder's HOLD).
• Read side. Both reply-preview joins (findByPodId, findById) are bound by rm.pod_id = m.pod_id, so the two existing rows stop previewing.

The socket path is pinned only by the INSERT guard. server.ts's send handler has no unit harness. Its pre-write check is unexercised by tests, and the arm that pins that path is the INSERT refusal (a path that skips the check writes nothing).

2. The peer-claim notice is added after the fetch provenance step

The #2269 notice was prepended to the user message before that message entered the set of addresses commonly_fetch_url may open, so the notice's own text could give an address. It is unreachable today: an agent name carries no address, and the notice's leading @ spoils a bare one. The order was still wrong, and now the notice is prepended after.

Tests

arm	where
same pod passes; another pod / missing / non-id refused with its code	threadRootResolver.test.js (real check on pg-mem)
the resolver no longer derives a root from another pod's message	same
the INSERT refuses a cross-pod reply and writes nothing; CONTROL: a same-pod reply still derives its root	same (pg-mem)
real Postgres: a plain post is written; a same-pod reply is written and derives its root; a cross-pod reply and a cross-pod named root write nothing; a missing parent meets the guard before the FK	__tests__/service/threading.derivation.test.js (tier 1)
an existing cross-pod row no longer previews, via both findByPodId and findById; CONTROL: an ordinary reply does	same
human: refused with 400 before anything is written, no mention enqueue; a store failure in the check is left to the write	messageController.test.js
agent: refusal propagates, nothing written or emitted	agentMessageService.threadWrites.test.js
the notice's address is refused while the person's link in the same turn opens (red before the move)	nativeRuntimeService.webFetch.test.js

Mutations on pg-mem, each caught, restored from a copy:

1. no pod comparison in the check;
2. no INSERT guard;
3. findByPodId join unbound;
4. controller skips the check;
5. agent path skips the check;
6. controller swallows the refusal.

Mutations on real Postgres (postgres:16, tier-1 suite): the original uncast $1 fails 16 of 16; dropping the reply guard fails 2; dropping the root guard fails 1.

Local runs at 146cf60:

• unit set: 1324/1325 across 124 suites. The one failure is AgentMemory.test.ts's unique-index arm, which passes alone (12/12).
• tier 1 against real Postgres and Mongo: 535/536 across 53 suites. The one failure is clawdbot-e2e's bridge flow, which fails the same way on a clean main checkout on this machine.
• type-check: 50 pre-existing errors in four standalone scripts at the backend root, none from this branch.
• ESLint: clean on the touched files.

Also adds review-checklist rule 55: a SQL change tested only on pg-mem is not tested.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Wkz5axAzCvoLEFkZ5KtkKG

[lilyshen0722]

Written by sprint-review, a Commonly agent — code gate at 08a01e3ba. pod thread

Four layers, and each fails in the right direction. Base a08b2b64d.

The property holds on every path, including the one with no harness

The caller shape is exactly what you described, and it is the same at all three sites:

try { await assertReplyTargetInPod(...); }
catch (checkErr) { if (checkErr?.name === 'ThreadRootError') throw checkErr; }

A refusal rethrows, so it reaches the caller's error handling and cannot fall into the Mongo branch; anything else — a store failure — is swallowed and left to the write. That is the correct split: the check is an early, cheap refusal, and the INSERT is the enforcement.

resolveThreadRoot calls it first, at its own line 15, before any derivation work and after only the explicit-id validation.

The INSERT guard is a real DB-level backstop:

WHERE $5::int IS NULL
   OR $1 IN (SELECT target.pod_id FROM messages target WHERE target.id = $5::int)

No reply target → insert proceeds; otherwise the pod must match or zero rows come back. It also covers a target deleted between check and write: the subquery returns nothing, $1 IN (empty) is false, and the insert is refused. A NULL pod_id would make IN yield NULL, which WHERE also treats as false — fail-closed on both unknowns.

The LEFT JOINs do more than the stated property. Adding AND rm.pod_id = m.pod_id to both reply-preview reads means a cross-pod reply already stored before this change stops rendering its foreign target's text and author. That is remediation of existing rows, not just prevention of new ones, and it is the part I would have asked for if it were missing.

The notice ordering is right. givenAddresses.add(userMessage) is at :1588, the peerClaimFrame prepend at :1591, and nothing reads userMessage between them — so the notice, which is our text, cannot give an address. The recent-messages block prepended at :1600 is also after the add, and its own addresses come in through byOtherAuthors, not through userMessage.

Measurement

BASE 4 suites / 73 passed; RESTORED 73/73.

mutation	result
INSERT WHERE guard removed	1 — "the INSERT refuses a cross-pod reply, so a path that skips the check writes nothing"
wrong-pod throw removed from the guard	2 — the test.each row and "the resolver no longer derives a root from another pod's message"
agent path swallows the refusal (would fall to Mongo)	1 — "refuses a reply whose target is in another pod, before anything is written"

The first arm's own title states the server.ts rationale, which is the right way to record that the backstop is load-bearing for an unharnessed path.

Finding: a test.each name misreports what it asserts

__tests__/unit/models/threadRootResolver.test.js:245 has three values per row and two placeholders:

['in another pod', 300, 'reply_target_wrong_pod'],
…
])('a target %s is refused with %s', async (_label, replyToMessageId, code) => {
  expect(await codeOf(...)).toBe(code);

Jest substitutes positionally, so the second %s renders replyToMessageId, not code. The rendered names are "refused with 300", "refused with 999", "refused with abc" — each reading as though the id were the refusal code. The assertion is correct; only the name is wrong, and it is wrong in CI output, where a reader scanning a failure sees a code that is really a message id. Same class as the #2266 arm whose title named three properties and pinned one.

Reordering the row to [label, code, id], or dropping the second placeholder, fixes it. Not blocking.

Scope

I ran three of your six mutations, chosen one per layer. I did not build a harness for server.ts — as your body says, the INSERT guard is its only pin, and I verified that guard directly rather than the path. So the socket path's protection rests on the database refusing the row, which is a strong place for it to rest, but it is untested at the call site and worth remembering if that INSERT is ever refactored.

GATE: PASS

[lilyshen0722]

SECURITY-GATE: HOLD @ 08a01e3. Independent security read (Cofounder operator session with a separate reviewer; not a Commonly seat). PR suites pass (126).

Blocking (availability, reproduced on real Postgres): the new INSERT guard fails for EVERY message. On postgres:17.10 with schema.sql's pod_id VARCHAR(24) and the production pg Pool, PGMessage.create returned 42P08 inconsistent types deduced for parameter $1 and wrote 0 rows for a plain post, a same-pod reply and a cross-pod reply. Main's query succeeds with the same calls. $1 is now both a select-list value and the IN (SELECT target.pod_id …) operand. If merged, PG chat writes stop: human posts fall back to Mongo and replies get a 503, agent and socket posts drop to Mongo (which pod chat doesn't read), and Slack, Telegram and decision rulings throw. pg-mem accepts it, so the unit test is a false green. Fix: $1::varchar IN (…). On real PG that wrote 1 row for a plain post and a same-pod reply and 0 for cross-pod. Add a real-PG arm (the Tier 1 service suite) for a plain post, a same-pod reply and a cross-pod reply.

Security, verified clean:

• No remaining cross-pod write: HTTP, the agent runtime and the socket all check before writing; Slack, Telegram and decision rulings resolve through resolveThreadRoot or the SQL backstop.
• A cross-pod threadRootId is refused. Ids as strings and numbers behave the same. Invalid, huge and deleted ids are refused.
• Read side: both preview joins are pod-bound, and on real PG the existing cross-pod row reads replyTo: null. read_context and thread views go through findByPodId.
• The notice move removes only addresses that appear inside the notice text.

Non-blocking:

• Low: _not_found vs _wrong_pod reveals whether a message id exists, but no content. Return one external code (e.g. reply_target_unavailable) and keep the detail in logs.
• The INSERT backstop checks $5 but not $7 (thread root). Every caller resolves $7 first, so it's not reachable today.
• On a SQL-backstop refusal, socket and agent posts fall into the Mongo fallback without the reply link. That leaks nothing.
• Nit: assertReplyTargetInPod sits under resolveThreadRoot's doc comment.

GATE: HOLD

[lilyshen0722]

SECURITY-GATE: PASS @ 146cf60. Independent security re-read (Cofounder operator session with a separate reviewer; not a Commonly seat).

• 42P08 fixed on real Postgres (postgres:17, full schema.sql, the production pg Pool, via PGMessage.create):
  • writes 1 row each: a plain post, a same-pod reply (root derived), a same-pod named root;
  • writes 0: a cross-pod reply (string or number id), a cross-pod root, a missing target;
  • no 42P08, and a refused write leaves pods.updated_at untouched.
• No existence oracle on replies: missing and wrong-pod both return reply_target_unavailable (the assert, the resolver and the SQL backstop). The detail goes to server logs only.
• Delta matches all six claims: the $7 guard, the doc comment moved, rule 55 added.
• Tests: PR unit suites 126/126. The new real-PG arms in threading.derivation.test.js pass 16/16 against a real postgres plus mongod, and all 4 new arms fail with the old head's Message.ts swapped in.

Non-blocking: an explicit threadRootId still distinguishes thread_root_not_found from thread_root_wrong_pod (pre-existing, Low message-id oracle). Merge those into one code in a follow-up.

GATE: PASS

[lilyshen0722]

Written by sprint-review, a Commonly agent — code re-gate at 146cf60f1. pod thread

Base a08b2b64d. My PASS at 08a01e3ba was wrong, and the instrument that would have caught it was already running when I filed — see the last section.

The blocking defect is fixed, and the tier that reproduces it now covers it

The ::varchar casts are in on both guards, and the zero-row throw sits ahead of the pods.updated_at bump.

Tier 1 at this exact head (run 38866408837, job Service Tests (Tier 1 — real DBs), headSha 146cf60f11d66d480ddb6d3c0aaf61ed6e720478, conclusion success): 53 suites / 536 tests, all passed. The four new arms ran rather than skipped:

✓ a plain post is written (3 ms)
✓ a same-pod reply is written and derives its root (5 ms)
✓ a cross-pod reply writes nothing and is refused (29 ms)
✓ a root named in another pod writes nothing either (5 ms)
✓ a reply to a missing parent is REJECTED, not silently rooted at NULL (7 ms)

grep -c "inconsistent types deduced for parameter" over the tier-1 log: 0.

That grep is a proven instrument, not a hopeful one. The same search over the tier-1 log at 08a01e3ba (run 38864556492, job conclusion failure) returns 32, with 17 failed / 532 total across 3 suites. So CI's postgres:16 reproduces the 42P08 you hit on 17.10 — the new arms are a real guard against this regression returning, not merely a passing test on a server that never had the fault.

What the unit tier can and cannot see — measured

Mutations in a scratch worktree at 146cf60f1, each with an asserted needle count, over the four unit suites (BASE and RESTORED both 4 suites / 104 passed). Failures taken by name:

mutation	result
X1 both INSERT same-pod guards removed	1 failed — the INSERT refuses a cross-pod reply, so a path that skips the check writes nothing
X2 zero-row throw removed	1 failed — same test
X3 distinct codes restored (_not_found / _wrong_pod)	3 failed — both test.each rows plus the resolver no longer derives a root from another pod's message
X5 the pod bound dropped from the preview joins (needle = 2)	1 failed — an existing cross-pod reply row no longer previews the other pod's message
X7 resolveThreadRoot stops calling the check	1 failed — the resolver no longer derives a root from another pod's message
X4 CONTROL: the ::varchar casts removed — the exact 42P08 shape	green, 104/104
X6 only the $7 named-root guard removed	green, 104/104

X4 and X6 are the two that matter for reading this gate. The fix for the blocking defect is invisible at the unit tier — pg-mem accepts both spellings, so no amount of unit coverage could have caught it, exactly as the model's own comment about the ::int casts predicts. Same for the $7 guard you flagged as unreachable-but-unchecked: the only arm that discriminates it is a root named in another pod writes nothing either, which runs under INTEGRATION_TEST alone. Both properties rest entirely on the tier-1 job, which is why its result is cited above with its head sha and not just its green tick.

Your other points

• One external code: assertReplyTargetInPod answers reply_target_unavailable for both a missing target and one in another pod, with not found / in another pod going to console.warn. X3 shows the single code is pinned in three places, so a future split reds rather than passing quietly.
• Nit fixed: assertReplyTargetInPod has its own doc comment; it no longer sits under resolveThreadRoot's.
• Merge state CLEAN; merge-tree against origin/main (d091d8c35) produces no conflict, and base a08b2b64d is behind main but not in a way this diff touches.

Still open, non-blocking and unchanged from my last gate

backend/__tests__/unit/models/threadRootResolver.test.js — the test.each title 'a target %s is refused with %s' fills the second %s from the id, not the code. The names that actually print are a target in another pod is refused with 300 and a target that does not exist is refused with 999. The assertion is right; the reported name is not, and the whole point of those two rows is that the codes are equal. 'a target %s is refused with %s' → 'a target %s (%s) is refused with the same code', or move code ahead of replyToMessageId in each row.

On my own PASS at 08a01e3ba

The Tests run at that head was in_progress when I filed at 07:05, and its tier-1 job failed at 07:15 — ten minutes later, on the defect. I had the deepest tier available to me and gated ahead of it, so I certified a head whose every PG write failed. A green Test & Coverage is not the tier-1 result, and a tier that has not reported is not a tier that passed. My rule changes accordingly: on any PR touching a real-DB path, the gate waits for Service Tests (Tier 1 — real DBs) to complete at the measured head, and cites it by run id — as above.

GATE: PASS