refactor(channels): delete the two dead outbound-upload surfaces

[chenmingwei23]

Problem / Motivation

Two outbound-upload surfaces shipped in #4832 as the seams the O3 Telegram / O4b Slack transports were expected to consume. Neither gained a consumer, and both are dead on main today:

• messaging/split.py::open_fence_at_end -- zero callers anywhere, and zero tests. Its only other appearance is its own __all__ entry.
• DiscordTransport.send_message_with_files -- zero production callers. The renderer uploads through the client verb (renderer.py::_land_sealed -> DiscordClient.send_message_with_files), and the file_send path resolves send_document by name (dashboard/upload_destination.py:284), never the extraction verb. The transport verb's only callers were its own two tests.

Why it matters

Dead code that looks like a designed extension point is worse than dead code that looks dead. A reader arriving at DiscordTransport.send_message_with_files finds a documented "transport-level upload verb" with a ceiling policy and a warning branch, and reasonably concludes the upload path goes through it -- it does not. The same goes for a public open_fence_at_end in a module whose spec enumerates its exports: it advertises a fence-state accessor no caller has ever needed, and its two tests (for the transport verb) buy confidence in a path no user reaches.

It also costs maintenance directly. The transport verb carried an over-cap drop branch whose reasoning had to be kept true against the client's real ceilings, and it kept a module logger alive for one warning that never fires in production.

What changed (motivation -> approach -> change)

Task 1 and task 2 of #4919 both asked "give it a consumer or delete it". No consumer landed, and both surfaces stay reachable in git history if a future one wants them back, so deletion is the answer. Pure subtraction -- no replacement code, no behaviour change on any live path:

• messaging/split.py: the function and its __all__ entry. Its _advance / _lines helpers stay, because iter_fence_spans and iter_fence_lines drive them.
• discord/transport.py: the verb and its over-cap drop branch, plus the three imports that branch was the last user of (Sequence, DISCORD_MAX_FILES_PER_MESSAGE, logging) and the now-unused module logger. send_document -- the verb file_send actually calls -- is untouched.
• Two docstring cross-references would have dangled, so they now name live symbols: Discord's send_document points at the client verb it is the name-preserving counterpart of, and Telegram's send_document points at Discord's send_document rather than at the deleted one.

MessagingTransport declares no upload verb, so nothing is left abstract by the removal.

Task 3 of #4919 (have split_markdown_safe declare its context-degrading tier so the Discord renderer can drop its synthetic ![x](/tmp/x.png) probe) is NOT a deletion and does not belong in this PR. It is carried forward as #7540, which also records that the third half of that task -- hoisting the restricted-session upload gate into a channel-neutral helper -- has already landed as messaging/upload_gate.py::uploads_restricted.

Tests

No new tests: a deletion's proof is that nothing needed the thing.

• Removed test_discord_outbound_files.py::TestTransportVerb (2 tests) -- the deleted verb's only callers.
• Two mock transports in test_file_send_channel.py no longer declare a verb the real class cannot have. In the delivering case spec_set=["send_document"] now IS the guard the removed assert_not_called used to be, and a stricter one: an upload path reaching for any other verb raises on the mock instead of being audited after the fact. The verb-less-Discord-transport case is modelled with send_message, which keeps its actual pin (a discord link whose transport lacks send_document is a skip, not a crash -- distinct from the neighbouring teams case, which never reaches the verb lookup at all).

Targeted runs, all green: test_messaging_split.py, test_discord_outbound_files.py, test_file_send_channel.py (252 passed), and the adjacent transport consumers test_discord.py, test_capability_ledger.py, test_channel_registry.py, test_channel_transport_outbound_authz.py (320 passed). flake8 / isort / black clean on all five touched files; scripts/check_black_formatting.py passes in scope.

Manual verification

N/A -- unit coverage sufficient. Nothing on a live path changes: the two deleted symbols have no production callers, verified by grep across src/, test/ and docs/ for both names, with every surviving hit belonging to the client verb of the same name.

Related Issues

Closes #4919
Refs #7540

Pattern harvest

Rule candidate: lint
Pattern: a module-level def exported in __all__ with no reference anywhere in src/ or test/. open_fence_at_end survived a year because __all__ membership reads as intent and no gate distinguishes "public API" from "public and unused" -- a vulture-style unused-symbol pass over src/kiro_crew/**, behind a shrinking baseline like the black and per-file-coverage gates, would have flagged it the day its consumer failed to land. The Discord half is weaker as a rule candidate (the method is reachable in principle through duck-typed transport dispatch, so a naive callers-count check would false-positive across the channel layer) and its real tell was different: a verb whose only callers are its own tests. That one is a review-prompt line rather than a lint.

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 7386cd9b1324dae80986e2b4a92e2ed7bf8d63dc — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Pure dead-code removal, verified dead: no remaining callers of either deleted symbol, no transport interface break, and spec references point at the surviving client-level verbs.

[DESIGN-REVIEWED] 7386cd9

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 7386cd9b1324dae80986e2b4a92e2ed7bf8d63dc and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 7386cd9

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 7386cd9b1324dae80986e2b4a92e2ed7bf8d63dc: <one-sentence reason>

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 7386cd9b1324dae80986e2b4a92e2ed7bf8d63dc — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 7386cd9

Verdict parsed from the review's SHA-scoped output markers for commit 7386cd9b1324dae80986e2b4a92e2ed7bf8d63dc.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 7386cd9b1324dae80986e2b4a92e2ed7bf8d63dc: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 7386cd9b1324dae80986e2b4a92e2ed7bf8d63dc — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All claims verified. Both deletions are clean: open_fence_at_end has zero references anywhere in the repo, every surviving send_message_with_files hit belongs to the untouched DiscordClient verb (production caller renderer.py:967), no string/getattr dispatch names the deleted transport verb, and no spec documented either symbol. The docstring retargets are forced by the deletion. Final review:

First-Principles-Verdict: PASS

Pure subtraction of two zero-consumer surfaces, with every riding item forced by the deletion itself — this is the change this lane exists to ask for.

What this change ships

Intent: delete two upload seams that shipped for transports that never landed, so readers stop trusting a documented path no message takes. This is a SUBTRACTION (a fix by deletion), not an addition.

1. messaging/split.py::open_fence_at_end deleted with its __all__ entry — justified; grep open_fence_at_end: 0 matches repo-wide at HEAD, and no spec section named it.
2. DiscordTransport.send_message_with_files deleted, with its over-cap drop branch, logger, and three imports — justified; every surviving send_message_with_files hit (17) is the DiscordClient verb, whose one production caller is discord/renderer.py:967; no getattr/string dispatch names the transport verb (0 matches), and upload_destination.py:284 resolves send_document only.
3. Two docstring cross-references retargeted to live symbols (discord/transport.py, telegram/transport.py) — rides along, but forced: leaving them would dangle onto a deleted symbol.
4. Two TestTransportVerb tests deleted — justified; they were the verb's only callers.
5. Mock transport in test_file_send_channel.py narrowed to spec_set=["send_document"], replacing assert_not_called — declared; strictly tighter guard.
6. Verb-less-Discord-transport test remodelled with send_message — declared; keeps the skip-not-crash pin without declaring an attribute the real class no longer has.

The description's zero-caller claims, the spec's silence on both symbols, and the client-verb survivals were each verified by grep, not taken from the PR text. Nothing to subtract from a subtraction.

[FIRST-PRINCIPLES-REVIEWED] 7386cd9

[chenmingwei23]

Disposition for the two red shards on 65882c382 -- Backend Tests (3.10, 3) and Backend Tests (Windows) (3). Both are the same assertion, and neither belongs to this PR:

FAILED test/test_security_posture.py::TestGateSideLogRedactorSpelling::
       test_no_new_gate_side_log_line_reads_the_baseline_redactor
AssertionError: New gate-side log/audit line(s) reading the BASELINE redactor
  dashboard/handlers/memory.py: 2 sites, census says 0

Evidence that it is main-owned:

• The ratchet's two inputs -- src/kiro_crew/dashboard/handlers/memory.py and test/test_security_posture.py -- are byte-identical between origin/main and this branch (git diff origin/main HEAD -- <both paths> is empty). _package_baseline_log_census() walks the package source, so its verdict here IS main's verdict.
• This PR's diff is five files: messaging/split.py, discord/transport.py, telegram/transport.py, test/test_discord_outbound_files.py, test/test_file_send_channel.py. It contains zero lines of memory.py and zero lines of test_security_posture.py.
• Reproduced locally on this branch: 1 failed, 4 passed in that class -- the same single failure, with nothing from this PR in the selection.
• The two counted sites are the redact_and_truncate(...) log writes at memory.py:911 and memory.py:1065, which reach a log call through a baseline redactor rather than redact_log_via_context.

Tracked on main as #7549. Not folding a fix for it into this PR: it is someone else's file and the correct clearance is either raising _BASELINE_LOG_SITE_CENSUS with a stated reason or routing those two sites through redact_log_via_context -- a decision that belongs with the owner of #7283, not with a deletion PR. Once main lands it, this branch gets rebased to cut a fresh merge ref (a re-run cannot absorb a base-branch fix).

Everything else on this head is green: 51 checks pass, all five review lanes clean with zero findings (Design PASS, GPT 5.6 no findings, Opus 4.8 no findings, First Principles PASS, UX success).

[bolichen97]

Tier 1 auto-approve: refactor (5 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: code-move/dead-code removal only — deletes the two unused outbound-upload surfaces (DiscordTransport.send_message_with_files and messaging/split.open_fence_at_end), no behaviour change on live paths.

[iamwhatever]

Tier 1 auto-approve: refactor (5 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: deletes two surfaces verified dead by repo-wide search (DiscordTransport.send_message_with_files and messaging.split.open_fence_at_end, the latter exported in all with no caller anywhere); the live Discord upload path calls DiscordClient.send_message_with_files directly and the per-message file cap stays enforced as the renderer's extraction budget, so no behaviour change, and the messaging spec's reference is to the client method that remains.

[bolichen97]

Tier 1 auto-approve: refactor (5 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: deletes two surfaces with no remaining callers, verified against the PR head -- DiscordTransport.send_message_with_files (the renderer calls DiscordClient.send_message_with_files directly and never went through the wrapper) and messaging/split.open_fence_at_end (zero references repo-wide). No limit is lost with the wrapper: the 10-attachment ceiling it re-checked is enforced upstream at extraction (renderer passes max_files=DISCORD_MAX_FILES_PER_MESSAGE), and DISCORD_MAX_FILES_PER_MESSAGE is still imported and used there.