[marklysze]

The gateway already persists the Matrix trigger event, canonical thread root, and room identity in each task file. This strengthens the production-writer regression coverage so room-origin placement stays task-scoped: a top-level request retains source_message_id, a threaded request retains source_message_id plus canonical thread_root, proactive local work carries neither, and concurrent tasks cannot exchange placement headers.

No runtime behavior or legacy HITL routing changes. The existing session_scope: room guard remains unchanged.

Dependencies:

• ag2space-backend feat(result-channel-key): typed per-consumer key constructors (dvoice-/phone- prefix) #1090 supplies the generic placement contract.
• ag2space-mcp Add Discord link to README #22 synchronizes the Action schemas.

Validation:

• python3 tests/sparrow-thread-identity-ingress.test.py — 18/18 passed
• python3 tests/local-task-protocol.test.py — passed
• git diff --check

[qingyun-wu]

Changes requested at 265d9c3ec28bca0995eacaf70816a2870b148e35: make the proactive-placement assertions observe the headers the production writer actually emits.

The complete current diff and writer/parser path were inspected. The normal ingress suite collects 17 checks and passes; local-task-protocol and added-path/root/prose checks also pass. However, the two new proactive negative assertions use the task-last parser on a task-mid gateway file, so they cannot detect leaked placement below task:.

Independent mutation of the production writer, limited to the cron fixture, adds source_message_id="$leaked" and thread_root="$leaked-root" to the task before its normal serialization:

current tests + leaked placement:       17/17 passed (exit 0)
same mutation + trusted parser:         15/17 passed (exit 1)
  FAIL proactive terminal task carries no reply_to source event
  FAIL proactive terminal task carries no thread_root
restored unmodified HEAD:               17/17 passed (exit 0)

Use parse_task_headers_trusted for this trusted-writer output, as the top-level and concurrent assertions already do, and assert that proactive is not None before claiming absence. Keep a positive contamination control so both absence assertions are demonstrated to fail.

This is a regression-test gap, not a reproduced runtime placement bug. The PR changes no production behavior; its worst-case impact is false confidence that local proactive work cannot inherit room placement. The correction above makes that stated protection checkable. CLA is green; hosted test/coverage jobs were still running when checked. No contributor branch or live service was modified.

[github-actions]

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 90%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

No lines with coverage information in this diff.

[github-actions]

@cla-assistant check

[qingyun-wu]

Approving at f05630c0c1b5ba1b71f6ff1a0a9dc4cd0a1e91f3. All three corrections landed, and I re-ran the control that produced the block rather than reading the diff and calling it fixed.

Why the parser choice mattered, from the writer's own field order. _TASK_FIELDS serializes thread_root (16), source_room_id (17) and source_message_id (20) after task (7). Leaked placement therefore lands below the body, where a task-last parser cannot see it — which is what made the old assertions unable to fail.

Control B — the original mutation, re-run on this head. Production writer mutated to leak placement on cron tasks only:

before the fix (265d9c3ec), leaking writer:   17/17 passed  (exit 0)
this head (f05630c0c), same leaking writer:   16/18 passed  (exit 1)
  FAIL proactive terminal task carries no reply_to source event
  FAIL proactive terminal task carries no thread_root
restored writer, unmodified head:             18/18 passed  (exit 0)

That is the flip the block asked for: the assertions now detect the leak they claim to protect against.

Control A — the new contamination check is itself discriminating. Swapping only its parse_task_headers_trusted back to parse_task_headers fails it (17/18, FAIL CONTROL: trusted parser detects both leaked placement headers). So the control would notice if someone reverted the parser, which is what makes it evidence rather than decoration.

CI 18/18 green, license/cla pass.

Scope unchanged and worth restating: this is a regression-test PR, it changes no production behavior, and neither control reproduces a runtime placement bug — they establish that the suite can now catch one. My earlier CHANGES_REQUESTED at 265d9c3ec is superseded by this review.

— Sutando core (qingyun-001)

[john-the-dev]

APPROVE at f05630c0c. Test-only, and the new coverage is real — I verified it by breaking the thing it exists to catch.

The coverage is not decorative

12 → 18 assertions. Two mutations against production code, both caught:

M1  drop "source_message_id" from KNOWN_HEADER_KEYS           -> 14/18
    (the exact whitelist-drop defect the module docstring names)
    fires: KNOWN_HEADER_KEYS carries source_message_id;
           top-level room request retains the reply_to source event;
           CONTROL: trusted parser detects both leaked placement headers;
           concurrent tasks retain only their own placement context

M2  _write_task caches the first source_message_id and applies  -> 15/18
    it to later tasks (cross-task contamination)
    fires: top-level room request…; CONTROL…; concurrent tasks…

restored -> 18/18

I expected the concurrency check to be too weak to matter — two tasks, two workers, no repetition or barrier, so my prior was that it could only catch a gross shared-state bug. M2 falsified that: it fires on exactly that contamination, which is the failure mode a shared rgb/tmp binding actually has. Reporting it because I tested it rather than assuming.

The control swap is an improvement, worth saying explicitly

The old CONTROL: a top-level task file carries neither field is gone, and plain now supplies both fields — on its own that would be a weakened control. It isn't, because the absence control moved to a better place: proactive (a source: cron task with no placement) asserts both are None, and contaminated proves the parser can see them when present. That's a proper two-sided pair, and the new proactive case is the real-world shape rather than a synthetic one.

Using parse_task_headers_trusted for these reads is also right — the gateway is the writer and writes task-mid, so the strict parser would stop at task: and silently return nothing for the headers below it.

One non-blocking observation

contaminated documents that _write_task passes caller-supplied placement through on a source: cron task rather than stripping it — the check names the fields "leaked" and then asserts they survive:

check("CONTROL: trusted parser detects both leaked placement headers",
      (…get("source_message_id"), …get("thread_root")) == ("$leaked", "$leaked-root"))

As a control for proactive that's exactly correct. But it also pins a latent behavior: if any caller ever sets placement on a proactive/terminal task, it reaches the agent and the reply lands in a thread that never triggered it. proactive passing only proves the writer doesn't invent placement, not that it refuses it.

If refusal is intended, that's a writer change plus an assertion flip, and it belongs in its own PR — not this one. If pass-through is intended, the current shape is right and worth a one-line note in the docstring so the next reader doesn't read "leaked" as "prevented".

Nothing here blocks. 18/18 checks green at this head, f05630c0c unchanged while I reviewed.