Skip to content

refactor(discord-bridge): single source of truth for [file:|send:|attach:] regex - #990

Merged
sonichi merged 2 commits into
sonichi:mainfrom
VasiliyRad:refactor/discord-bridge-file-marker-helper
May 22, 2026
Merged

sonichi merged 2 commits into
sonichi:mainfrom
VasiliyRad:refactor/discord-bridge-file-marker-helper

Conversation

@VasiliyRad

Copy link
Copy Markdown
Contributor

Why

The [file:|send:|attach:] marker regex is duplicated inline at three sites in discord-bridge.py:

Line (pre-PR) Path Used by
2832 poll_results channel replies
2974 poll_proactive owner DMs
3130 poll_dm_fallback channel-redirect branch

The pattern requires absolute paths (/... or ~/...) — relative paths previously resolved against CWD, which differed between launchd-managed and bare-shell runs. Any future hardening to the regex would need to land in all three places or one path would drift.

Changes

  • _FILE_MARKER_RE — module-level compiled regex.
  • _split_file_markers(text) -> (clean_text, files) — pure helper. Three call sites now call it instead of re-defining the regex.

No behavior change. Refactor is byte-for-byte equivalent for valid inputs; the only difference is that the regex compiles once at module-load instead of per polling-loop iteration (minor perf win).

Tests

tests/discord-bridge-file-markers.test.py — 10 cases pinning the contract:

Case Pins
no markers / empty / single Common cases.
file / send / attach aliases All three keywords.
~/... allowed Home-relative paths.
relative paths rejected Pre-fix regression guard.
multiple markers preserve order Textual order for the send loop.
[reply: 12345] does NOT match The reply directive uses a different keyword.
unknown keywords ([path:] etc.) don't match Strictness.
Architectural — regex body appears in source exactly once A future refactor that re-introduces an inline copy fails here.

All 10 pass locally.

Test plan

  • python3 tests/discord-bridge-file-markers.test.py — 10/10 pass
  • Syntax check
  • Manual: send a reply with a [file: /tmp/sutando-x.png] marker → verify file delivered as attachment, marker removed from text

🤖 Generated with Claude Code

…ach:] regex

The `[file:|send:|attach:]` marker regex was duplicated inline at THREE
sites:

  - poll_results (channel replies)
  - poll_proactive (owner DMs)
  - poll_dm_fallback channel-redirect

Any future hardening to the pattern had to land in all three places
or one path would drift. The pre-existing pattern required absolute
paths (`/...` or `~/...`) because relative paths resolved against
the bridge's CWD, which differed between launchd-managed and bare-
shell runs.

## Changes

- `_FILE_MARKER_RE` — module-level compiled regex.
- `_split_file_markers(text) -> (clean_text, files)` — pure helper.
  Three call sites now call it instead of re-defining the regex.

No behavior change. Refactor is byte-for-byte equivalent for valid
inputs; the only difference is that the regex compiles once at
module-load instead of per polling-loop iteration (minor perf win).

## Tests

`tests/discord-bridge-file-markers.test.py` (10 cases):

  - common case (no markers), empty input, single-marker extraction
  - `file` / `send` / `attach` keyword aliases
  - absolute paths only — relative paths rejected
  - multiple markers preserve textual order
  - `[reply: <id>]` directive does NOT match the file pattern
  - unknown keywords (`[path:]`, `[url:]`, etc.) don't silently extract
  - ARCHITECTURAL: the regex body appears exactly once in the source
    — guards against a future refactor accidentally re-introducing
    an inline copy at a fourth call site

All 10 pass locally.

Co-Authored-By: Vasiliy's Sutando <noreply@anthropic.com>
@CLAassistant

CLAassistant commented May 21, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@sonichi

sonichi commented May 21, 2026

Copy link
Copy Markdown
Owner

LGTM. Refactor with a structural test, which is the right shape for "consolidate inline duplicates" — without the architectural assertion that the regex body appears exactly once in the source, this PR would just be cosmetic and the drift recurs on the next change.

All three call sites converted (poll_results, poll_proactive, poll_dm_fallback). The relative-path rejection is explicitly tested — important because that's the actual constraint distinguishing this regex from a casual file-extraction pattern (launchd CWD ≠ bare-shell CWD makes relative paths silently break for proactive DMs).

[reply: 12345] correctly stays excluded (different keyword), and the test covers it — easy to break by mistake if someone "generalizes" the file-marker keyword set later.

@sonichi
sonichi merged commit c45bbc4 into sonichi:main May 22, 2026
2 checks passed
liususan091219 pushed a commit to liususan091219/sutando-fork that referenced this pull request May 28, 2026
…ach:] regex (sonichi#990)

The `[file:|send:|attach:]` marker regex was duplicated inline at THREE
sites:

  - poll_results (channel replies)
  - poll_proactive (owner DMs)
  - poll_dm_fallback channel-redirect

Any future hardening to the pattern had to land in all three places
or one path would drift. The pre-existing pattern required absolute
paths (`/...` or `~/...`) because relative paths resolved against
the bridge's CWD, which differed between launchd-managed and bare-
shell runs.

## Changes

- `_FILE_MARKER_RE` — module-level compiled regex.
- `_split_file_markers(text) -> (clean_text, files)` — pure helper.
  Three call sites now call it instead of re-defining the regex.

No behavior change. Refactor is byte-for-byte equivalent for valid
inputs; the only difference is that the regex compiles once at
module-load instead of per polling-loop iteration (minor perf win).

## Tests

`tests/discord-bridge-file-markers.test.py` (10 cases):

  - common case (no markers), empty input, single-marker extraction
  - `file` / `send` / `attach` keyword aliases
  - absolute paths only — relative paths rejected
  - multiple markers preserve textual order
  - `[reply: <id>]` directive does NOT match the file pattern
  - unknown keywords (`[path:]`, `[url:]`, etc.) don't silently extract
  - ARCHITECTURAL: the regex body appears exactly once in the source
    — guards against a future refactor accidentally re-introducing
    an inline copy at a fourth call site

All 10 pass locally.

Co-authored-by: Vasiliy's Sutando <noreply@anthropic.com>
Co-authored-by: Chi Wang <4250911+sonichi@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants