Skip to content

feat(discord): opt-in raw-reaction journal for durable triage state - #129

Merged
Kyzcreig merged 5 commits into
mainfrom
feat/discord-reaction-journal
Jun 30, 2026
Merged

Kyzcreig merged 5 commits into
mainfrom
feat/discord-reaction-journal

Conversation

@Kyzcreig

Copy link
Copy Markdown
Collaborator

What

When discord.reaction_journal (config.yaml) is set, the Discord adapter requests the non-privileged reactions intent and subscribes to on_raw_reaction_add/remove, appending one JSON line per raw transition to that path in the reaction_state core's journal schema: {channel_id, message_id, emoji, user_id, action, seq, ts}.

Why

Raw reaction events fire regardless of message cache, so a reaction on an old / un-cached message is still captured — the durable-triage use case (greenhouse seed_triage, which currently approximates this with a REST poller). This taps the gateway's existing Discord connection rather than opening a second gateway session on the same bot token (which Discord forbids).

Footprint / safety (per AGENTS.md rubric)

  • Edge feature, not core surface — lives entirely in the Discord adapter; no new model tool, no core change.
  • Config-gated in config.yaml, not a new HERMES_* env var (the path is non-secret config).
  • Default OFF — without the config key, no intent change, no extra gateway traffic, zero behavior change. Concrete consumer exists (seed_triage), so it's not speculative.
  • Best-effort writes — a journal error can never crash the gateway event loop (runs inside a discord.py handler).
  • Restart-safe seq — the monotonic counter seeds from the existing journal's max so a gateway restart never rewinds it.

Tests

tests/gateway/test_discord_reaction_journal.py (7, all green): exact-schema, custom-emoji name:id, add→remove ordering, restart-safe seq, unconfigured no-op, never-raises-on-bad-payload, and an end-to-end ingest by the reaction_state core proving byte-compatibility. Existing adapter suite stays green (27 in test_discord_connect).

Kyzcreig added 2 commits June 30, 2026 11:25
When discord.reaction_journal (config) is set, request the non-privileged
reactions intent and subscribe to on_raw_reaction_add/remove, appending one
JSON line per raw transition in the reaction_state core's journal schema
{channel_id, message_id, emoji, user_id, action, seq, ts}.

Raw events fire regardless of message cache, so a reaction on an old/un-cached
card is still captured — the durable-triage use case (greenhouse seed_triage)
the REST poller approximates. Default OFF: no intent change, no extra gateway
traffic, no behavior change for anyone who hasn't set the config key. Writes are
best-effort (a journal error can never crash the gateway event loop); the seq
counter seeds from the existing journal max so a restart never rewinds it.
Covers the journal contract (exact reaction_state schema), custom-emoji
name:id form, add→remove ordering, monotonic seq resuming above the existing
journal max (restart-safety), no-op when unconfigured, best-effort never-raises
on a bad payload, and an end-to-end ingest by the reaction_state core proving
byte-compatibility (skips cleanly if the core isn't in this checkout).
@github-actions

github-actions Bot commented Jun 30, 2026 •

Copy link
Copy Markdown

🔎 Lint report: feat/discord-reaction-journal vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 12214 on HEAD, 12212 on base (🆕 +2)

🆕 New issues (2):

Rule Count
unresolved-import 1
invalid-argument-type 1
First entries
tests/gateway/test_discord_reaction_journal.py:197: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`
tests/gateway/test_discord_reaction_journal.py:201: [invalid-argument-type] invalid-argument-type: Argument to function `module_from_spec` is incorrect: Expected `ModuleSpec`, found `ModuleSpec | None`

✅ Fixed issues: none

Unchanged: 6399 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@greptile-apps

greptile-apps Bot commented Jun 30, 2026 •

Copy link
Copy Markdown

Greptile Summary

This PR adds an opt-in raw-reaction journal to the Discord adapter: when discord.reaction_journal is set in config.yaml, the adapter requests the non-privileged reactions gateway intent and writes one JSON line per raw add/remove transition to a configurable file path, enabling the downstream reaction_state / seed_triage consumer to durably track reactions regardless of message-cache state.

  • Core path: all journal code is gated on _reaction_journal_path; without the config key nothing changes — no extra intent, no extra gateway traffic, no new behaviour.
  • Thread-safe seq generation: double-checked locking with a module-level _REACTION_SEQ_INIT_LOCK ensures the per-instance _reaction_seq_lock is initialised exactly once even under concurrent burst events; the lock then serialises read-increment-write so no two threads can mint the same seq.
  • All three previously identified issues are addressed: non-dict journal lines are skipped (no AttributeError-induced silent silencing), makedirs is guarded by a once-per-process flag, and file writes are offloaded to run_in_executor so an emoji storm cannot stall the gateway loop.

Confidence Score: 5/5

Safe to merge — the feature is entirely opt-in, defaults to off, and the three previously identified issues are all correctly resolved in this iteration.

The changed code is well-scoped to the Discord adapter with no core changes. Thread safety is handled correctly via double-checked locking. All defensive except blocks prevent journal errors from propagating to the gateway event loop. The test suite covers concurrency, edge-case journal lines, restart-safe seq, and downstream ingest compatibility.

No files require special attention.

Important Files Changed

Filename Overview
plugins/platforms/discord/adapter.py Adds _next_reaction_seq, _emit_reaction_journal, and _append_reaction_journal with correct double-checked locking, run_in_executor offload, and defensive exception handling; all three issues flagged in prior review rounds are fully addressed.
tests/gateway/test_discord_reaction_journal.py Nine tests covering exact schema, custom-emoji format, ordering, restart-safe seq, non-dict-line resilience, concurrent uniqueness (16-thread barrier test), no-op when unconfigured, bad-payload swallowing, and downstream ingestibility by the reaction_state core.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Discord Gateway
    participant DiscordAdapter (event loop)
    participant ThreadPool (executor)
    participant Journal File

    Discord Gateway->>DiscordAdapter (event loop): on_raw_reaction_add/remove(payload)
    DiscordAdapter (event loop)->>DiscordAdapter (event loop): _emit_reaction_journal(payload, action)
    DiscordAdapter (event loop)->>ThreadPool (executor): run_in_executor(_append_reaction_journal)
    Note over DiscordAdapter (event loop): returns immediately — loop not blocked

    ThreadPool (executor)->>ThreadPool (executor): _next_reaction_seq() [acquire _reaction_seq_lock]
    Note over ThreadPool (executor): First call: seed from existing journal max seq
    ThreadPool (executor)->>Journal File: read existing lines (seed only)
    Journal File-->>ThreadPool (executor): max seq found
    ThreadPool (executor)->>ThreadPool (executor): seq += 1, store _reaction_seq, release lock

    ThreadPool (executor)->>Journal File: append {channel_id, message_id, emoji, user_id, action, seq, ts}
    Journal File-->>ThreadPool (executor): write complete
    ThreadPool (executor)-->>DiscordAdapter (event loop): future resolved (best-effort)
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Discord Gateway
    participant DiscordAdapter (event loop)
    participant ThreadPool (executor)
    participant Journal File

    Discord Gateway->>DiscordAdapter (event loop): on_raw_reaction_add/remove(payload)
    DiscordAdapter (event loop)->>DiscordAdapter (event loop): _emit_reaction_journal(payload, action)
    DiscordAdapter (event loop)->>ThreadPool (executor): run_in_executor(_append_reaction_journal)
    Note over DiscordAdapter (event loop): returns immediately — loop not blocked

    ThreadPool (executor)->>ThreadPool (executor): _next_reaction_seq() [acquire _reaction_seq_lock]
    Note over ThreadPool (executor): First call: seed from existing journal max seq
    ThreadPool (executor)->>Journal File: read existing lines (seed only)
    Journal File-->>ThreadPool (executor): max seq found
    ThreadPool (executor)->>ThreadPool (executor): seq += 1, store _reaction_seq, release lock

    ThreadPool (executor)->>Journal File: append {channel_id, message_id, emoji, user_id, action, seq, ts}
    Journal File-->>ThreadPool (executor): write complete
    ThreadPool (executor)-->>DiscordAdapter (event loop): future resolved (best-effort)
Loading

Reviews (3): Last reviewed commit: "fix(discord): lock reaction-seq generato..." | Re-trigger Greptile

Comment thread plugins/platforms/discord/adapter.py Outdated
Comment thread plugins/platforms/discord/adapter.py Outdated
Comment thread plugins/platforms/discord/adapter.py Outdated
- P1: non-dict JSON line in the journal raised AttributeError out of seed
  scanning, leaving _reaction_seq unset → every subsequent append retried the
  same bad line and the journal went permanently dark. Now skip non-dict lines
  (isinstance guard) and catch AttributeError. RED-proven regression test added.
- P2: makedirs ran on every append; now guarded by a once-per-process flag.
- P2: sync file I/O ran inside the async on_raw_reaction handlers; now offloaded
  via run_in_executor (_emit_reaction_journal) so an emoji-storm can't stall the
  gateway event loop.
Comment thread plugins/platforms/discord/adapter.py Outdated
Kyzcreig added 2 commits June 30, 2026 11:44
…dapter

Ground-truthed the config path: the Discord adapter is env-driven by
convention (_apply_yaml_config translates discord.* YAML keys to DISCORD_*
env vars; it seeds nothing into PlatformConfig.extra). Reading
config.extra.get('reaction_journal') would therefore have been silently dead —
the config key never reaches the adapter. Fix:
- read DISCORD_REACTION_JOURNAL env (fallback to extra for nested
  platforms.discord.extra users),
- add the config.yaml discord.reaction_journal -> DISCORD_REACTION_JOURNAL
  bridge in _apply_yaml_config (env takes precedence, matching every other key),
- tests for the bridge + precedence; make the cross-repo core-ingest test
  path-agnostic (glob for tools/reaction_state.py) since the deployed layout
  moved to versioned symlink dirs.
Greptile P-level: _next_reaction_seq is now invoked from run_in_executor (the
async-offload fix), so its read-increment-write on self._reaction_seq raced —
two concurrent burst events could mint the SAME seq, and the reaction_state
core drops a duplicate/stale seq → silent event loss. Guard the
read-increment-write with a per-instance threading.Lock (lazily created under a
module-level init lock so the lazy-init is itself race-free). Added a
16-thread/50-iter concurrency test asserting all seqs are unique + a clean
monotonic run (RED-proven: the lock-free version mints ~750/800 duplicates).
@Kyzcreig
Kyzcreig merged commit 56be472 into main Jun 30, 2026
32 checks passed
@Kyzcreig
Kyzcreig deleted the feat/discord-reaction-journal branch June 30, 2026 18:53
@Kyzcreig
Kyzcreig restored the feat/discord-reaction-journal branch September 21, 2026 10:32
Kyzcreig added a commit that referenced this pull request Sep 26, 2026
Fork-PR audit verdict DROP (lead t_03e35f0e, card t_d72c6944): the
journal is written but nothing in the fleet reads it on a schedule.
The live journal (~/.hermes/greenhouse/reactions.jsonl) is at seq
40486 on 2026-09-25, so the feature does fire. Its only reader is the
manual `seed_triage.py report --journal` in ANG-Ventures/greenhouse-tools
(verdict-age column). That tool is not scheduled, and there is no local
checkout of it on the Studio.

Reverts 56be472. The conflict in adapter.py was resolved by keeping
main's seeded_extra/_skip_env_bridge shape and removing the
reaction_journal keys.

Verified: tests/gateway/test_discord_connect.py plus
test_discord_reactions.py give 19 passed and 1 failed. The failure is
test_post_connect_initialization_retries_fingerprint_after_timeout, and
it fails the same way on fork/main 382fc62.
ang-fleet-workers Bot pushed a commit that referenced this pull request Sep 27, 2026
Fork-PR audit verdict DROP (lead t_03e35f0e, card t_d72c6944): the
journal is written but nothing in the fleet reads it on a schedule.
The live journal (~/.hermes/greenhouse/reactions.jsonl) is at seq
40486 on 2026-09-25, so the feature does fire. Its only reader is the
manual `seed_triage.py report --journal` in ANG-Ventures/greenhouse-tools
(verdict-age column). That tool is not scheduled, and there is no local
checkout of it on the Studio.

Reverts 56be472. The conflict in adapter.py was resolved by keeping
main's seeded_extra/_skip_env_bridge shape and removing the
reaction_journal keys.

Verified: tests/gateway/test_discord_connect.py plus
test_discord_reactions.py give 19 passed and 1 failed. The failure is
test_post_connect_initialization_retries_fingerprint_after_timeout, and
it fails the same way on fork/main 382fc62.
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.

1 participant