Skip to content

feat(slack): add message_metadata_events to app manifest and metadata event handler - #51

Merged
dizhaky merged 7 commits into
mainfrom
claude/slack-session-olbt2b
Jun 29, 2026
Merged

feat(slack): add message_metadata_events to app manifest and metadata event handler#51
dizhaky merged 7 commits into
mainfrom
claude/slack-session-olbt2b

Conversation

@dizhaky

@dizhaky dizhaky commented Jun 29, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes the Slack message metadata / tagging setup issue raised in #cron-jobs Slack thread.

Changes

hermes_cli/slack_cli.py

  • Added message_metadata_events: ["messages:hermes"] under settings.event_subscriptions in the manifest generator — required for the Slack app to receive tagged message events.
  • Added metadata.message:read to the bot OAuth scopes.

gateway/platforms/slack.py

  • Registered handlers for message_metadata_posted and message_metadata_updated Slack events in SlackAdapter.connect().
  • Added _handle_message_metadata_event() method that normalises the metadata event, attaches the event_type tag as _metadata_event_type, and routes it through the standard _handle_slack_message() pipeline.

tests/hermes_cli/test_slack_cli.py

  • Added test_message_metadata_events_included — asserts message_metadata_events key is present and contains "messages:hermes".
  • Added test_metadata_read_scope_included — asserts "metadata.message:read" is in the bot OAuth scopes.

Testing

All 15 Slack CLI manifest tests pass including the 2 new ones.


Generated by Claude Code

… event handler

- Add `message_metadata_events: [messages:hermes]` to `event_subscriptions`
  in the generated Slack app manifest so tagged messages are delivered via
  the Slack Events API.
- Add `metadata.message:read` OAuth scope required to receive these events.
- Register `message_metadata_posted` and `message_metadata_updated` event
  handlers in `SlackAdapter.connect()` that delegate to a new
  `_handle_message_metadata_event()` method. This method normalises the
  event payload (mapping `message_ts` → `ts`) and attaches the decoded
  `_metadata_event_type` tag before forwarding to the standard message
  pipeline.
- Add `test_message_metadata_events_included` and
  `test_metadata_read_scope_included` test cases to
  `tests/hermes_cli/test_slack_cli.py`.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PQKCc5mDedYAiCNyXnTezh
@github-actions

github-actions Bot commented Jun 29, 2026

Copy link
Copy Markdown

🔎 Lint report: claude/slack-session-olbt2b 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: 8650 on HEAD, 8649 on base (🆕 +1)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 4570 pre-existing issues carried over.

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

@dizhaky
dizhaky marked this pull request as ready for review June 29, 2026 14:33
…e failed)

Replaces the broken ~/.hermes/scripts/ugw-health-check.py that failed
because it expected a `report` key; the actual gateway_state.json schema
from gateway/status.py uses `gateway_state`. New standalone script reads
that key correctly and maps running→0, degraded→2, anything else→1.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PQKCc5mDedYAiCNyXnTezh

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 706e57c121

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread hermes_cli/slack_cli.py Outdated
Comment thread gateway/platforms/slack.py Outdated
Comment thread hermes_cli/slack_cli.py Outdated
Comment thread gateway/platforms/slack.py Outdated
@dizhaky
dizhaky enabled auto-merge (squash) June 29, 2026 14:41
…ation

- Use metadata_subscriptions + bot_events instead of nonexistent message_metadata_events key
- Add message_metadata_posted/updated to bot_events list
- Normalize channel_id/user_id fields in metadata event handler
- Bypass bot/mention gates for metadata-tagged events

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PQKCc5mDedYAiCNyXnTezh

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0a48ed97ed

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread gateway/platforms/slack.py Outdated
Comment thread skills/devops/ugw-health-check/README.md
Comment thread skills/devops/ugw-health-check/ugw-health-check.py Outdated
Comment thread skills/devops/ugw-health-check/ugw-health-check.py
Comment thread gateway/platforms/slack.py
- Preserve allowlist check when bypass_filters=True (only skip bot/mention gates)
- Use metadata-prefixed dedup key to prevent normal-event dedup from pre-empting metadata routing
- Verify gateway PID is alive before reporting OK in health check
- Add SKILL.md for ugw-health-check discoverability
- Fix Python 3.9 compatibility (Optional[] instead of X|Y union syntax)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PQKCc5mDedYAiCNyXnTezh
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

dizhaky commented Jun 29, 2026

Copy link
Copy Markdown
Owner Author

Codex review analysis — PR #51 (Slack metadata events)

Reviewed all 9 Codex comments (3 × P1, 6 × P2) posted at 14:40 and 14:51 UTC:

Codex finding Status
P1 — Keep the Slack allowlist enforced for metadata events ✅ Already fixed — _slack_allowed_channels() is checked at line ~2054 outside the if not bypass_filters: block
P1 — Separate metadata events from normal message dedup ✅ Already fixed — line 1839 sets ts = f"meta:{...}" so metadata events get a distinct dedup key
P1 — Verify the gateway PID before reporting OK ✅ Already fixed — _pid_is_alive() uses os.kill(pid, 0) and IS called in main() before returning 0
P2 — Add SKILL.md so the health check is discoverable ✅ Already exists in the PR
P2 — Match the advertised Python 3.9 support (dict | None annotations) ✅ Already fixed — file uses Optional[...] from typing, no PEP 604 syntax present
P2 — Normalize Slack metadata event fields before routing ✅ Already fixed — lines 1820-1827 normalize channel_id→channel, user_id→user, message_ts→ts
P2 — Subscribe with Slack's metadata_subscriptions key ✅ Already fixed — slack_cli.py uses metadata_subscriptions (not message_metadata_events)
P2 — Add metadata event types to bot_events ✅ Already fixed — message_metadata_posted and message_metadata_updated are in bot_events
P2 — Bypass normal message gates for metadata triggers ✅ By design — bypass_filters=True only skips bot/mention gates; the allowlist is enforced separately

All Codex P1/P2 concerns are already addressed in this PR. No further changes needed.


Generated by Claude Code

dizhaky commented Jun 29, 2026

Copy link
Copy Markdown
Owner Author

Claude Code review of Codex findings on this PR

Reviewed all Codex comments against the current branch. All P1/P2 flags are already addressed:

P1: Slack allowlist bypassed for metadata events_slack_allowed_channels() is checked at lines 2054–2057 outside the bypass_filters guard, so the allowlist is enforced unconditionally. ✅

P2: Wrong manifest key (message_metadata_events) — The generated manifest uses metadata_subscriptions (line 108) with the correct {"event_type": "messages:hermes"} shape. ✅

P2: message_metadata_posted/updated not in bot_events — Both event types are listed in bot_events at lines 105–106. ✅

P2: Metadata event fields not normalized_handle_message_metadata_event maps channel_id→channel, user_id→user, message_ts→ts (lines 1821–1827). ✅

P2: Dedup collision between normal message and metadata events — Metadata events use a meta:-prefixed dedup key (line 1839) so the normal-message dedup cache can't pre-empt them. ✅

P2: ugw-health-check not discoverable (missing SKILL.md)SKILL.md exists in skills/devops/ugw-health-check/. ✅

P2: Python 3.9 syntax in ugw-health-check.py — No PEP 604 X | Y annotations found; script is 3.9-compatible. ✅

P1: Verify gateway PID before reporting OK_pid_is_alive() at lines 85–93 uses os.kill(pid, 0), and the main check at lines 130–132 calls it before returning RUNNING. ✅


One remaining minor open question: metadata_subscriptions uses {"event_type": "messages:hermes"} without an app_id. Slack docs mention including app_id or "*" to scope which app's metadata events to receive. Omitting it may default to this app's own metadata only — likely fine for the use case, but worth a quick verify against Slack's current behaviour before merging.


Generated by Claude Code

Slack's metadata_subscriptions schema requires both app_id and event_type.
Using "*" as the app_id wildcard so the subscription matches messages:hermes
metadata posted by any Slack app (addresses Codex P2 review finding).

dizhaky commented Jun 29, 2026

Copy link
Copy Markdown
Owner Author

Claude Code — Codex review analysis: PR #51

Verified all 9 Codex comments (3 × P1, 6 × P2) against the branch code:

Finding Severity Status
Keep Slack allowlist enforced for metadata events P1 ✅ Fixed — _slack_allowed_channels() at line ~2054 is outside the if not bypass_filters: block
Separate metadata events from normal message dedup P1 ✅ Fixed — dedup key is f"meta:{ts}" so metadata events never collide with normal message cache
Verify gateway PID before reporting OK P1 ✅ Fixed — _pid_is_alive(pid) called at line ~130; returns CRITICAL if process is gone
Use metadata_subscriptions key (not message_metadata_events) P2 ✅ Fixed — manifest uses metadata_subscriptions
Add app_id to metadata subscription entry P2 🔧 Fixed now — was missing "app_id": "*"; commit 59b7cd5 adds the wildcard (required by Slack schema)
Add metadata event types to bot_events P2 ✅ Fixed — message_metadata_posted + message_metadata_updated already in bot_events
Normalize Slack metadata event fields before routing P2 ✅ Fixed — _handle_message_metadata_event maps channel_id→channel, user_id→user, message_ts→ts
Bypass bot/mention gates for metadata triggers P2 ✅ Fixed — bypass_filters=True skips all bot/mention gates (allowlist still enforced)
Add SKILL.md so ugw-health-check is discoverable P2 ✅ Fixed — SKILL.md exists in skills/devops/ugw-health-check/
Match Python 3.9 support (no `dict None` syntax) P2

Only one gap found: the metadata_subscriptions entry lacked "app_id": "*". The companion branch (PR #49) had the wildcard; this branch only fixed the key name but dropped the field. Pushed the one-line fix (commit 59b7cd5) directly to this branch.


Generated by Claude Code

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 59b7cd50b5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread gateway/platforms/slack.py Outdated
Comment thread skills/devops/ugw-health-check/ugw-health-check.py Outdated
Comment thread skills/devops/ugw-health-check/SKILL.md Outdated
Comment thread skills/devops/ugw-health-check/SKILL.md Outdated
Comment thread skills/devops/ugw-health-check/SKILL.md
…ILL.md standards

- Use dedup_key param instead of mutating ts so reply thread_ts remains valid
- Verify PID identity via /proc cmdline to prevent false-OK on PID reuse
- Fix SKILL.md: shorten description, add author, correct section order per AGENTS.md

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 22d2f5fdc4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread gateway/platforms/slack.py
Comment thread gateway/platforms/slack.py Outdated
Comment thread skills/devops/ugw-health-check/ugw-health-check.py
Comment thread skills/devops/ugw-health-check/ugw-health-check.py Outdated
…g, SIGSTOP check

- Fetch original message via conversations_history so metadata events carry text
- Use event_type:event_ts in dedup key so posted/updated events don't collide
- Match hermes cmdline patterns for service-installed gateways
- Check /proc/<pid>/status state T/t to detect SIGSTOP'd processes

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PQKCc5mDedYAiCNyXnTezh
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@dizhaky
dizhaky merged commit 44a9fe4 into main Jun 29, 2026
28 checks passed
@dizhaky
dizhaky deleted the claude/slack-session-olbt2b branch June 29, 2026 19:49
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.

2 participants