[bassilkhilo-ag2]

What

Structural tests for two merged PRs:

• PR fix(discord-bridge): delivery-idempotency sentinel for poll_results (restart-safety) #1048: delivery-idempotency sentinels in poll_results
• PR feat(dm-result): REST multipart upload — actually deliver [file:] attachments #1029: send_allowlist.py as the shared file-attachment policy module

Background

PR #1048 — delivery-idempotency

Before this fix, if the bridge crashed between channel.send() returning success and archive_file() completing, on restart the result file would still be in results/ and would be re-sent — duplicate delivery. Fix: three sentinel helpers around the send/archive sequence:

1. _is_delivered(task_id) — check before send
2. _mark_delivered(task_id) — touch after send
3. _clear_delivered(task_id) — remove after archive

PR #1029 — send_allowlist shared module

The file-attachment delivery policy ([file:|send:|attach:] markers) was duplicated between discord-bridge.py and dm-result.py. The copies had silently diverged — dm-result.py was missing Desktop, Documents, and personal-notes roots. src/send_allowlist.py is the single source of truth for the policy now.

Tests (22 assertions)

discord-bridge.py (PR #1048) — 13 assertions

#	Assertion
1	DELIVERED_DIR defined → state/discord-delivered
2	_delivered_sentinel_path helper exists
3	Sentinel files use .sentinel extension
4	_mark_delivered defined
5	_is_delivered defined
6	_clear_delivered defined
7	_mark_delivered uses mkdir(parents=True, exist_ok=True)
8	_mark_delivered has try/except (sentinel failure never kills send)
9	_is_delivered returns False on exception (fail-open)
10	_is_delivered(task_id) called in poll_results
11	_mark_delivered(task_id) called in poll_results
12	_clear_delivered(task_id) called in poll_results
13	Call order: _is_delivered < _mark_delivered < _clear_delivered
14	"already delivered" log message exists for skip path
15	_mark_delivered appears after channel.send in poll_results

send_allowlist.py (PR #1029) — 9 assertions

#	Assertion
16	SEND_ALLOWED_ROOTS defined
17	results/ and notes/ in roots
18	SEND_ALLOWED_PREFIXES defined
19	/tmp/sutando- prefix included
20	is_path_sendable() defined
21	realpath() used to defeat symlink/traversal attacks
22	os.path.isfile() check rejects directories
23	os.sep suffix prevents prefix-pollution attacks
24	discord-bridge.py imports from send_allowlist

Run

python3 tests/discord-bridge-delivery-idempotency.test.py

🤖 Generated with Claude Code

[VasiliyRad]

Not approving yet — scope mismatch + undocumented production changes.

1. Title says "test(...)" but ships production code. src/discord-bridge.py (+124/-46), src/send_allowlist.py (+106 new), and src/health-check.py (+14/-2) are feature implementations, not tests. The PR body documents #1048 and #1029 well, but the commit-message prefix test(...) is misleading — this is feat+test.

2. Undocumented join-trigger fast path (lines 2247-2268). A new _dv_message_is_join_phrase / _dv_handle_join_trigger block was added to _handle_discord_message that bypasses requireMention. This isn't mentioned in the PR description at all and appears unrelated to either #1048 or #1029. Should be a separate PR or at minimum documented.

3. Undocumented poll_proactive owner-resolution change (lines 3347-3354). Adds explicit access_data.get("owner") lookup (#1117 per inline comment). Also unrelated to either referenced PR.

4. Test assertions are structural greps, not behavioral. The 22 tests verify strings exist in source files (e.g., "def _mark_delivered" in bridge). They won't catch semantic regressions like wrong sentinel ordering or a broken skip path. This is fine as a smoke-check layer, but the PR body implies stronger coverage than delivered.

5. build_log.md is a new file covering passes 207-214. If the repo already has a build_log.md convention elsewhere (workspace), shipping one in the repo root may conflict.

Suggestion: Split the unrelated changes (join-trigger, owner-resolution) into their own PRs, retitle this one to feat+test(...), and it's good to go. The sentinel implementation and allowlist extraction look correct.

[bassilkhilo-ag2]

Status update (2026-05-26): VasiliyRad's review is correct — the production code in this diff (src/discord-bridge.py +124/-46, src/send_allowlist.py +106) is the root cause of the scope issue. However, this is the same root cause affecting 7 other test/* PRs: the branch was cut from a local main that was 3 commits ahead of upstream origin/main. Those upstream commits already contain the delivery-idempotency sentinels (PR #1048 → commit 33789fc), the allowlist (PR #1029 → commit 6a1ba31), and the join-trigger fast-path (PR #1080/#1113 → commit 45c75b2).

Verification: cherry-picking just the test commit (050de1f) onto origin/main is conflict-free and produces ONLY tests/discord-bridge-delivery-idempotency.test.py — all 22 tests pass. No production code changes would appear in the diff after a rebase.

A rebase onto origin/main is pending owner go-ahead (same batch as PRs #1156, #1158, #1159, #1161, #1162, #1163). After the force-push this PR will be test-only, scope-clean, and ready for approval.

[bassilkhilo-ag2]

Branch rebased onto origin/main — bundled junk (build_log.md, health-check.py, health-check-bridge-multi-pid.test.py) stripped. Diff is now clean: only the intended test file(s).