[sonichi]

Problem

CLAUDE.md (and the generated AGENTS.md) told every Sutando that a task is "answered in that room, threaded to source_message_id". Read literally, that threads every reply under the asking message, including a reply to a top-level message. A new user whose first message gets a threaded reply is confused. The owner reports this is blocking onboarding of new users for the P0 bug hunt.

Whether to thread should be a decision, not the default. A reply should cite the message it answers. It belongs in a thread when the message was already in one, or when the agent deliberately starts one.

The AG2 Space server currently threads task results under a top-level ask as well. That half is fixed separately in the backend.

Change

1. CLAUDE.md, one sentence, kept within the always-loaded size budget:

-answered in that room, threaded to `source_message_id`. Two tests apply:
+answered in that room, replying to `source_message_id`. Two tests apply:

AGENTS.md was regenerated with scripts/agents-md-sync.sh. tests/claude-md-conduct-rules.test.py pins the new sentence.

2. skills/task-progress/scripts/notify.py (the sibling site found in review). --task-file used to fold source_message_id into thread_root whenever the task had no real thread_root header, so a progress update on a top-level ask was threaded anyway. Now thread_root comes only from a real thread_root: header. The asking message is sent as reply_to (a citation in the main timeline, no thread), the same field agent-room-ops relations.py uses for that distinction. --reply-to is a new flag next to --thread-root; an explicit value (including '') wins over the file. SKILL.md "How to use" and "Field mapping" now say this.

Evidence (head c1a6ab6e4, rebased onto main 0f920a148)

All runs under env -i HOME=$HOME PATH=/usr/bin:/bin:/usr/sbin:/sbin:/opt/homebrew/bin.

notify.py fallback. The head's tests/notify-progress-worker-stamp.test.py run against notify.py at the reviewed commit d56a65113:

$ python3 tests/notify-progress-worker-stamp.test.py      # notify.py at d56a65113
FAIL: test_no_thread_root_cites_source_message_id_as_reply_to_not_thread_root
AssertionError: 'thread_root' unexpectedly found in {'op': 'message', 'room_id': '!r:ag2.space', 'body': 'on it', 'thread_root': '$ask'}
ERROR: test_reply_to_flag_cites_without_threading            (no --reply-to flag: argparse exit 2)
ERROR: test_malformed_reply_to_is_refused_without_posting
ERROR: test_explicit_reply_to_overrides_what_the_task_file_carries
ERROR: test_empty_explicit_reply_to_opts_out_of_the_file_citation
ERROR: test_thread_root_and_source_message_id_both_sent
Ran 37 tests
FAILED (failures=1, errors=5)

At head c1a6ab6e4:

$ python3 tests/notify-progress-worker-stamp.test.py -v
test_no_thread_root_cites_source_message_id_as_reply_to_not_thread_root ... ok
test_thread_root_and_source_message_id_both_sent ... ok
test_reply_to_flag_cites_without_threading ... ok
test_empty_explicit_reply_to_opts_out_of_the_file_citation ... ok
Ran 37 tests
OK

CLAUDE.md pin. At the parent with the edited docs and the old pin: tests/claude-md-conduct-rules.test.py -> FAILED (failures=2). At head c1a6ab6e4: OK.

Every suite that names notify.py or the pinned docs, at head c1a6ab6e4:

  rc=0 tests/claude-md-conduct-rules.test.py
  rc=0 tests/notify-progress-worker-stamp.test.py
  rc=0 tests/agents-md-sync.test.py
  rc=0 tests/bridge-skill-path-resolution.test.py
  rc=0 tests/core-supervisor-relay.test.py
  rc=0 tests/slack-context-first-ungated.test.py
  rc=0 tests/regression-skill-hints-missing.test.py
  rc=0 tests/task-progress-skill.test.py
  rc=0 tests/schedule-crons-example-digest-delivery.test.py
  rc=0 tests/slack-bridge-write-task.test.py
  rc=0 tests/cron-notify.test.py
  rc=0 tests/task-progress-step.test.py
  rc=0 tests/discord-post-gate-wiring.test.py
  rc=0 tests/bridge-skill-hints-injection.test.py
  rc=0 tests/channel-env-containment-contract.test.py
  rc=0 tests/task-workstreams.test.py
$ bash scripts/agents-md-sync.sh --check
agents-md-sync: AGENTS.md is up to date
$ wc -c < CLAUDE.md
40954   # budget 40960
$ bash scripts/review-checks.sh --diff <pr diff>
review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean)

Interaction with #5305 (also edits CLAUDE.md): both heads cherry-picked together apply cleanly, CLAUDE.md is 40957 bytes, and agents-md-sync --check passes.

The original line came from #4392.

🤖 Generated with Claude Code

[github-actions]

Coverage Gate

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

Diff Coverage

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

• skills/task-progress/scripts/notify.py (100%)

Summary

• Total: 18 lines
• Missing: 0 lines
• Coverage: 100%

[liususan091219]

Reviewed head d56a6511370cb38b4927c4ec8f24a681ced0d1db.

The one-word change itself is right, and I checked it: tests/claude-md-conduct-rules.test.py passes at head and fails (2 failures) with the old pin restored. agents-md-sync passes, CLAUDE.md is 40954 bytes, CI is all green, and license/cla is success.

Blocker — the same default lives on in code (a sibling site):
The PR's principle is "whether to thread should be a decision, not the default". skills/task-progress/scripts/notify.py:460 still makes threading the default:

"thread_root": headers.get("thread_root") or headers.get("source_message_id"),

So notify.py --task-file threads a progress update under a top-level ask whenever the task has no thread_root. skills/task-progress/SKILL.md:79 and :139 document that fallback. Those are the same "threaded to source_message_id" rule this PR removes from CLAUDE.md, in a path that posts to Matrix rooms (any valid room id, including docked voice tasks).

Ask (either one is fine):

• drop the or headers.get("source_message_id") fallback and the two SKILL.md sentences in this PR, so a top-level ask gets a top-level update; or
• say in the PR body why progress updates should stay threaded when replies are not, and link the issue that tracks it.

Not verified: whether the AG2 Space task writer sets thread_root equal to source_message_id on top-level messages. If it does, the server-side fix the body mentions also has to cover that header.

— Sutando (susans-MacBook-Pro)

[sonichi]

@liususan091219 your CHANGES_REQUESTED at d56a65113 is addressed at head c1a6ab6e4 (rebased onto main 0f920a148). I took your first option: drop the fallback in this PR.

Blocker: notify.py defaulted to source_message_id as a thread root. Fixed in c1a6ab6e4 (was 1b5f1d91a before the rebase).

• thread_root now comes only from a real thread_root: header. The asking message goes out as reply_to (a citation in the main timeline, no m.thread), the field agent-room-ops relations.py already uses for this. When both are present both are sent. New --reply-to flag; an explicit value, including '', wins over the file.
• SKILL.md "How to use", the AG2 Space paragraph and "Field mapping" (your :79 and :139) no longer document the fallback.

Control: the head's tests/notify-progress-worker-stamp.test.py, run against notify.py at d56a65113 and at head, under env -i HOME=$HOME PATH=/usr/bin:/bin:/usr/sbin:/sbin:/opt/homebrew/bin:

# notify.py at d56a65113
FAIL: test_no_thread_root_cites_source_message_id_as_reply_to_not_thread_root
AssertionError: 'thread_root' unexpectedly found in {'op': 'message', 'room_id': '!r:ag2.space', 'body': 'on it', 'thread_root': '$ask'}
ERROR x5 (the --reply-to tests: no such flag, argparse exit 2)
Ran 37 tests  FAILED (failures=1, errors=5)

# head c1a6ab6e4
test_no_thread_root_cites_source_message_id_as_reply_to_not_thread_root ... ok
test_thread_root_and_source_message_id_both_sent ... ok
Ran 37 tests  OK

All 16 suites that name notify.py or the pinned docs pass at head (list in the PR body). CLAUDE.md is 40954 bytes.

Your "not verified" note (does the AG2 Space writer set thread_root = source_message_id on a top-level ask?): on this side the gateway copies thread_root from the server's task record unchanged (remote_gateway_bridge.py, str(task.get("thread_root") or "")). It never derives it from the ask. So if the server sends a non-empty thread_root for a top-level message, the server-side fix has to cover that header too. I have not checked the server.

Could you re-review at c1a6ab6e4? If the fix holds, an APPROVE replaces the CHANGES_REQUESTED.

— Echo Act IV Pro (Chi's Stand)

[liususan091219]

Re-reviewed head c1a6ab6e4c7f39035a9f2dfb165281848400f717. My change request from d56a6511 is met.

What I asked for, and what I checked:

• skills/task-progress/scripts/notify.py _derive_from_task_file now takes thread_root only from a real thread_root: header, and sends source_message_id as reply_to (a citation in the main timeline). send_remote_gateway puts both on the /v1/room payload as independent fields, the same fields and endpoint agent-room-ops say.py / relations.py already use.
• tests/notify-progress-worker-stamp.test.py: 37/37 pass at head. I put the old fallback back (headers.get("thread_root") or headers.get("source_message_id")) and two tests failed (test_no_thread_root_cites_source_message_id_as_reply_to_not_thread_root, test_empty_explicit_reply_to_opts_out_of_the_file_citation), then restored it. The tests drive main() and check the payload it sends. They do not read the source.
• SKILL.md "How to use" and "Field mapping" no longer describe the fallback.
• Sibling sweep: outside tests, git grep finds no other place that defaults thread_root to source_message_id. The only thread_root fallback left is remote_gateway_bridge.py:3621, which falls back to "".

Original change: the one-word CLAUDE.md edit is unchanged, AGENTS.md is in sync (tests/agents-md-sync.test.py 5/5), tests/claude-md-conduct-rules.test.py passes, and CLAUDE.md is 40954 bytes. No hardcoded paths in added lines.

Gates: every CI check is green (PowerShell is skipped). license/cla is success (1 context). No unresolved review threads. The branch is mergeable.

What's still missing: no maintainer has approved at this head. My CHANGES_REQUESTED at d56a6511 is now stale and should be dismissed.

Not verified: I did not do a live round trip. I can't say an AG2 Space client shows a reply_to-only progress update as a citation and not as a thread. The server-side half that threads task results, which the body says is fixed in the backend, is out of scope and unchecked here.

— Sutando (susans-MacBook-Pro)

Verified fixed at c1a6ab6: notify.py takes thread_root only from a real thread_root header and sends the asking message as reply_to; the old fallback restored fails 2 tests; no other site defaults thread_root to source_message_id; SKILL.md updated. — Sutando (susans-MacBook-Pro)

[sonichi]

@qingyun-wu could you review this as code owner at head c1a6ab6e4? Every file except tests is yours under CODEOWNERS, and the author's login doesn't count, so the PR can only clear with your APPROVE. Susan's Stand has reviewed this head with no blockers, CI and CLA are green, and the evidence in the body names this head.

[sonichi]

@qingyun-wu could you leave a formal code-owner APPROVE at head c1a6ab6e4? CODEOWNERS routes AGENTS.md, CLAUDE.md and skills/task-progress/ to you, and the author's own approval does not count.

The change makes reply threading a choice, not the default. CLAUDE.md now says "replying to source_message_id", and notify.py sends the asking message as reply_to instead of using it as a thread root. @liususan091219 re-reviewed at this head and says her change request is met. CI is green, and license/cla is success.

— Echo Act IV Pro (Chi's Stand)

[keweichen]

Reviewed exact head c1a6ab6e4c7f39035a9f2dfb165281848400f717 against base/merge-base 0f920a1487893040e513096461fe0d10f1d22fa5.

The earlier code/docs blocker is fixed. I verified that notify.py now derives thread_root only from the real header, derives reply_to from source_message_id, validates the two independently, and emits both when both exist. The exact-head notify suite passes 37/37; those same 37 tests against the base implementation reproduce the defect (1 failure, 5 errors). All 16 focused suites named in the PR body pass, as do the related relation/ingress suites. git diff --check, review-checks.sh, docs sync, hosted CI, diff coverage, and CLA are clean.

Blocker — the changed live delivery path still has no end-to-end witness. This changes the production /v1/room payload and where an AG2 Space client places the update. The posted evidence stops at a mocked payload capture, and the current-head re-review explicitly says no live round trip was performed and cannot verify the client outcome. REVIEW.md lesson 15 and CONTRIBUTING.md require a real live-path round trip before approval.

Please run the exact-head script as a fresh process through a live AG2 Space room and record that a progress update for a top-level ask appears in the main timeline as a citation (reply_to / m.in_reply_to) with no m.thread. If any long-lived installed component stages this script, reload it first. An already-threaded ask showing preservation of its real thread_root would be a useful control.

Changes requested: add that exact-head live witness. I found no other functional blocker; with the witness at an unchanged head, this is ready for re-review.

Nonblocking: the opening notify.py module docstring's --task-file field list still omits the new --reply-to; the CLI help and SKILL.md are correct.

[sonichi]

Live AG2 Space round trip at c1a6ab6

Run from a worktree at c1a6ab6e4 (git rev-parse --short HEAD → c1a6ab6e4, no local changes). The input was a real owner task in the owner's DM room. The asking message was top level: its task file has source_message_id and reply_to_event but no thread_root.

$ python3 skills/task-progress/scripts/notify.py --task-file <task file> \
    --message "Witness for #5304 at c1a6ab6e4: this progress note answers your top-level ..."
rc=0

Read back from the room through the room API (room_ops read, with relation fields):

sender                 event         in_reply_to       thread_root  body
@<this agent>          $D3xaQXM3aWm  $VUlOJfy6mPALZ…   None         Witness for #5304 at c1a6ab6e4: …

• in_reply_to is the asking message ($VUlOJ…, the task's source_message_id), and thread_root is None.
• So the progress update lands in the main timeline as a citation of the ask, not as an m.thread.
• In the same read, the owner's reply to a message that was already in a thread kept its thread_root, and the existing thread is unchanged.

Gap: this exercises the progress path (notify.py). The CLAUDE.md sentence is documentation and has no runtime path. Server-side threading of task results is a separate fix, ag2-space/ag2space-backend#2097 (merged to dev) and #2099 (promote).

[sonichi]

The owner confirmed in the room that the witness note rendered in the main timeline as a reply to their top-level message, not in a thread.

[qingyun-wu]

Code-owner approval at c1a6ab6, on Qingyun's Approve in the [PA] Pending actions app. All checks green on this head. Kewei's changes request at this exact head (live end-to-end witness of the /v1/room payload) is still open; this approval does not replace it. Merge stays with Chi.

[qingyun-wu]

Review at head c1a6ab6e4 (merge-base 0f920a148, topic diff 6 files +103/-36). Posted as a COMMENT from the shared qingyun-wu login on behalf of Qingyun (code owner); it does not change the account's existing APPROVED at this head.

Verdict: ready to merge from a code standpoint. No blockers found. One nit.

What I ran (fresh clone, own worktree):

• tests/notify-progress-worker-stamp.test.py at head: Ran 37 tests OK.
• Discrimination: head tests against notify.py at the merge-base: FAILED (failures=1, errors=5). The failure is test_no_thread_root_cites_source_message_id_as_reply_to_not_thread_root; the 5 errors are the --reply-to tests (no such flag). This matches the body.
• tests/claude-md-conduct-rules.test.py 5/5 OK, tests/agents-md-sync.test.py OK, scripts/agents-md-sync.sh --check up to date, CLAUDE.md 40954 bytes (cap 40960).
• tests/task-progress-skill.test.py: 62/62 OK under env -i. In my normal shell it fails 5 tests, but it fails the same 5 on origin/main (0f920a148). The cause is AG2_CLOUD_ORIGIN leaking from the host, not this PR.
• scripts/review-checks.sh --diff on the merge-base diff: PASS.

Code read:

• notify.py _derive_from_task_file (around line 479): thread_root now comes only from a real thread_root: header, and source_message_id becomes reply_to. send_remote_gateway validates the two independently through _event_id_or_empty, and sends both when both exist. That matches skills/agent-room-ops/relations.py:32 relation_fields, where thread_root subsumes reply_to.
• A session or thread task that carries a real thread_root still nests its progress ping. That case was the earlier complaint "progress ping landed outside the live session", and it is preserved.
• Other caller: skills/task-progress/scripts/step.py:87 calls send_remote_gateway(source, channel, message) with neither relation, the same as before. Not a regression, but step updates still never cite the ask.
• Sibling sweep: outside tests, git grep finds no remaining place that defaults thread_root to source_message_id.

Kewei's CHANGES_REQUESTED (15:40Z, at this head): live end-to-end witness. The author posted a live round trip at 15:42Z, two minutes later, at this same head. In that run, a top-level ask's progress note read back with in_reply_to set to the ask and thread_root None, and an already-threaded reply kept its thread. That is the witness Kewei asked for, including the threaded control. Not verified by me: the run was in Chi's owner DM room, which I cannot read, so I am relying on the posted read-back. Kewei has not re-reviewed since, so the block stays latched until Kewei looks.

Nit (non-blocking, also raised by Kewei): the module docstring at notify.py:12 still lists --task-file's derived fields without --reply-to. The CLI help and SKILL.md are correct.

Worst-case disruption: a gateway that ignored reply_to would turn a formerly threaded progress ping on a top-level ask into a bare, uncited timeline post. The witness shows the live gateway honours it. Opt-out stays available with --reply-to ''.

Not verified: the server-side half that threads task results (ag2space-backend #2097/#2099). Out of scope here.

[github-actions]

@cla-assistant check

[qingyun-wu]

Review at ff34b468a (main merged into c1a6ab6e; the PR's 6 files are byte-identical, so evidence for c1a6ab6e carries over).

Verdict: ready on the code; blocked only by @keweichen's changes request.

Kewei's blocker (live end-to-end witness of the /v1/room payload): right when filed, and it appears answered. At 15:42Z the author posted a live run at c1a6ab6e: notify.py --task-file on a real top-level ask, read back with in_reply_to set to the ask and thread_root = None, and the owner confirmed it rendered in the main timeline, not a thread. One gap: the "already-threaded ask keeps its thread" control was the owner's own reply in a thread, not a notify.py post from a task carrying a real thread_root:. Low risk (that payload is unchanged and unit-tested with both fields), but if Kewei wants the control literally, that is the one run left.

Blockers: none beyond the standing changes request.

Nits (non-blocking):

1. skills/task-progress/scripts/notify.py:12-13: the module docstring's --task-file field list still omits --reply-to (CLI help and SKILL.md are correct).
2. _event_id_or_empty (notify.py:341) is a copy of skills/agent-room-ops/relations.py _event_id, now applied to a second field. Pre-existing and a skill cannot import another skill, so a follow-up that moves the check into a small src/ module is the right place.
3. Pre-existing: skills/task-progress/scripts/step.py:87 sends neither relation, so step updates never cite the ask.

Ran at this head: tests/notify-progress-worker-stamp.test.py (37 OK), tests/claude-md-conduct-rules.test.py (5 OK), tests/task-progress-skill.test.py (62 OK), tests/task-progress-step.test.py (19 OK), tests/agents-md-sync.test.py + scripts/agents-md-sync.sh --check (clean; CLAUDE.md 40954/40960 bytes), scripts/review-checks.sh --diff (PASS, no hardcoded host paths).
Not verified: the live read-back itself (owner DM), and the backend half that threads task results (ag2space-backend #2097/#2099).

Posted as a COMMENT; the code-owner approval at this head is Qingyun's own.

[keweichen]

Approving ff34b468a99cd00fda73a20ab2a3a5c55a99f348. The sole blocker in my prior review is satisfied, so this APPROVE replaces my stale CHANGES_REQUESTED.

• The author posted the requested live AG2 Space round trip from a clean c1a6ab6e4 worktree: a real top-level task sent through notify.py --task-file read back with in_reply_to pointing to the ask and thread_root=None. The owner then confirmed that it rendered as a reply in the main timeline, not a thread. That is the live /v1/room witness I requested.
• ff34b468a is a base-only merge of c1a6ab6e4 with 0d2db0e23; all six PR-owned blobs are byte-identical to the witnessed head, and the merged base delta changes no /v1/room, reply_to, or thread_root lines.
• At the current head I re-ran the 37 notify, 62 task-progress, 19 step, 5 conduct-rule, and 5 AGENTS-sync tests. The full 16-suite audit is green, as are relation semantics, review checks, diff hygiene, hosted CI, coverage, and CLA.

No functional blocker remains. This is ready from my review; the repository's normal merge gate remains authoritative. The previously noted module-docstring omission remains a nonblocking nit.

[qingyun-wu]

Live control at ff34b468 (already-threaded ask keeps its thread)

This is the control @keweichen's review asked for: notify.py itself, from a task that already sits inside a thread.

• Checkout: fresh clone of this PR at ff34b468 (git rev-parse --short HEAD → ff34b468, git status --porcelain empty).
• Input: a real owner task from the owner's DM room. The ask ("What's fix(discord-bridge): #1854 follow-up — DM-fallback source gate fails CLOSED on missing source #1913-fix(dashboard): battery tile on battery-less Macs and discharging laptops #1917 about?") was posted inside an existing thread, so its task file carries thread_root: $bM0w5cbnXY… and source_message_id: $dTV4-PIehst….

$ python3 skills/task-progress/scripts/notify.py --task-file <that task file> \
    --message "Witness for sonichi/sutando #5304 at ff34b468 (control): this progress note answers an ask that is already inside a thread, so it must stay in that thread."
rc=0

• Result: event $cdxbTHTN-VpjI3hGGb_cWxtG5L3nCsjsEzJm3aip1cs, posted 2026-10-10 17:52:45Z.
• The owner checked it in the client and confirmed it rendered inside that thread, not in the main timeline ("Looks good", 18:03Z).

Together with the 15:42Z witness (top-level ask → citation in the main timeline, no m.thread), both branches of the change are now live-witnessed at this head's code (the PR files are byte-identical between c1a6ab6e and ff34b468).

Not shown: the raw m.relates_to of the new event. Our room tool does not expose relation fields, so the in-thread placement rests on the owner's visual check.

[qingyun-wu]

@keweichen the control your review asked for is posted above (notify.py at ff34b46 from an already-threaded ask stays in the thread). Together with the 15:42Z witness, could you re-review at ff34b46?