Skip to content

feat(discord): add discord-dump script + dumping-channel-content skill (was agents#226) - #588

Open
alex-nsheaps[bot] wants to merge 3 commits into
mainfrom
alex/discord-dump-plugin
Open

alex-nsheaps[bot] wants to merge 3 commits into
mainfrom
alex/discord-dump-plugin

Conversation

@alex-nsheaps

@alex-nsheaps alex-nsheaps Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Moves discord-dump.py from nsheaps/agents#226 into plugins/discord/scripts/discord-dump.py per Nate's directive 2026-06-09 17:45Z
  • Adds plugins/discord/skills/dumping-channel-content/SKILL.md describing when to use channel dumping, all CLI args, output format, --verify flag, and Discord auth setup
  • Bumps plugins/discord/package.json version 0.1.0 → 0.2.0

Script capabilities

The script (discord-dump.py) exports Discord channels/threads to JSONL via the REST API v10. Stdlib-only (no external deps). Key features: guild-wide dumps (--all), thread traversal (--with-threads), date/user filters, forum thread support, and --verify for completeness validation.

Test plan

  • Run python3 plugins/discord/scripts/discord-dump.py --help — confirm all flags documented in SKILL.md appear
  • Dump a small channel with --verify — confirm output JSONL and no assertion errors
  • Confirm skill appears in skill list after plugin reinstall

🤖 Generated with Claude Code

alex-nsheaps Bot and others added 2 commits June 9, 2026 13:55
Moves scripts/discord-dump.py from nsheaps/agents PR #226 into the
discord plugin. Stdlib-only Python 3 tool that exports channel/thread
messages to JSONL via the Discord REST API v10. Supports --verify flag,
date/user filters, guild-wide dumps, and forum thread traversal.

Co-Authored-By: Agent Alex Picard <alex-nsheaps[bot]@users.noreply.github.com>
Adds a new skill describing when and how to use discord-dump.py for
channel/thread exports: invocation examples, all CLI args, output format,
--verify flag, and Discord auth setup.

Co-Authored-By: Agent Alex Picard <alex-nsheaps[bot]@users.noreply.github.com>
@alex-nsheaps alex-nsheaps Bot added the request-review Force an AI code review on a draft PR (open non-draft PRs review automatically) label Jun 9, 2026
@henry-nsheaps henry-nsheaps Bot removed the request-review Force an AI code review on a draft PR (open non-draft PRs review automatically) label Jun 9, 2026
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Plugin Version Preview

Preview only — plugin versions and marketplace.json are bumped automatically on merge to main, not in this PR. Manual bumps to higher versions are preserved. See the file annotations for the pending change on each plugin.json.

Plugin Base Current Action
1pass 0.6.3 0.6.3 Auto-bumped
github 0.2.0 0.1.18 Auto-bumped
github-app 0.6.0 0.5.2 Auto-bumped

henry-nsheaps[bot]

This comment was marked as outdated.

@henry-nsheaps henry-nsheaps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

❌ Some changes need to be made — re-review finds the same issues unaddressed (no new commits since last review)

SKILL.md frontmatter never opens with --- (line 1 is a commented setext heading) → name/description/allowed-tools won't parse — thread
❌ "Starter message handling" doc says the starter is skipped; the code includes it — thread
⚠️ Dead list_channel_threads with a literal {{guild_id}} placeholder request — thread
⚠️ --verify false-fails when --start/--user/--exclude-user are used — thread
⚠️ --all --with-threads silently drops archived threads — thread
⚠️ Version bumped in package.json, not the plugin.json the marketplace reads — thread
✅ Stdlib-only, sensible rate-limit/retry handling, JSONL output, chronological ordering

🖱️ Click to expand for full details

This is a re-review. The PR head (fdd429d) is unchanged since my prior review and none of the six open threads have replies or fixes, so every finding below still stands. Details remain in the existing inline threads (linked above) rather than being re-posted.

Code Quality

The script is well-organized for a stdlib-only tool — clean DiscordClient with 429/5xx backoff and proactive bucket cooldown, chronological pagination, and a thoughtful --verify mode. Two correctness/accuracy problems and some cleanup keep the score at 70%:

  • Broken skill frontmatter (blocking). dumping-channel-content/SKILL.md doesn't start with ---; the <!-- UPSTREAM: discord --> comment precedes the fence, and mise run lint then folded it into a ## … setext heading and mangled allowed-tools into Bash(python3 \*). Verified on disk with cat -A — the metadata will not be parsed.
  • Doc contradicts code (blocking). The "Starter message handling" section claims the starter message is skipped to avoid double-counting; dump_to_file deliberately fetches and includes it.
  • Dead code with a broken placeholder. list_channel_threads is unused and issues a request to the literal path /guilds/{guild_id}/threads/active. Remove per YAGNI.
  • Minor: iter_channel_messages takes an unused is_thread param and ends with an unreachable return + stale comment; the verify PASS line prints 'present' on both ternary branches.

Correctness / Completeness

  • --verify + filters → false FAIL. Verification flags any message older than min_id, but --start/--user/--exclude-user legitimately exclude older messages, so a correct filtered dump exits non-zero.
  • --all --with-threads omits archived threads while the single-channel path includes them — an inconsistent completeness gap.

Versioning / Repo conventions

  • The bump landed in the MCP server's package.json; .claude-plugin/plugin.json (the marketplace source of truth) is still 0.1.0 on disk, so the intended 0.2.0 minor bump won't take effect (auto-bump on merge would only apply a patch).
  • SPEC.md still lists only access, configure, forum-thread-creation — the new dumping-channel-content skill isn't recorded (spec drift; see the plugin-spec-sync skill).

Security

No material concerns — the bot token is read from an env var and never echoed, requests are GET-only against the official API, and the User-Agent is static (it does still point at the old nsheaps/agents repo, cosmetic). 90%.

Simplicity

78% — dropping the dead list_channel_threads and the unused is_thread param/unreachable return would tighten the module without losing functionality.

Confidence

88% — I re-verified the frontmatter breakage byte-for-byte (cat -A), confirmed plugin.json is still 0.1.0, and confirmed SPEC.md omits the new skill. I did not run the script against a live Discord guild, so the network/pagination paths are reasoned about, not executed.

Recommended follow-ups (non-blocking):

  • Update plugins/discord/SPEC.md to include the new skill (or run plugin-spec-sync).
  • Refresh the static User-Agent URL from nsheaps/agents to nsheaps/ai-mktpl.
  • Remove the unused is_thread parameter and the unreachable trailing return in iter_channel_messages.

To improve future reviews I'd benefit from access to Bash(python3 ...) in a sandbox with a test Discord token so I can exercise the pagination/--verify paths rather than reasoning about them.

Notes:12

Footnotes

  1. Workflow Run: nsheaps/ai-mktpl actions run 27225449500

  2. Discord channel-types / message pagination reference: Discord Developer Docs — Channel

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.

0 participants