Skip to content

feat(tools/discourse): add a read-only Discourse topic bridge - #1755

Merged
potiuk merged 4 commits into
apache:mainfrom
MakerFlyFly:feat/discourse-topic-view-upstream
Oct 24, 2026
Merged

potiuk merged 4 commits into
apache:mainfrom
MakerFlyFly:feat/discourse-topic-view-upstream

Conversation

@MakerFlyFly

@MakerFlyFly MakerFlyFly commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Add the first implementation for #307: a stdlib-only, read-only magpie-discourse topic view <id|url> [--max-posts N] command for structured forum context.
  • Read the requested post stream in batches of at most 20, preserve HTML bodies and reply relationships, and fail explicitly on missing posts or malformed responses.
  • Include workspace, inventory and specification integration. Declare contract:mail-archive with Coverage: partial; this is not yet a complete/selectable archive backend.

Proposed delivery plan

We intend to deliver the broader issue as separate, independently useful PRs.
Each includes its implementation, tests and required documentation/integration.
The sequence and details remain open to maintainer feedback.

Step Capability
PR1 - this PR Read a topic and its posts
PR2 List recent topics in a category
PR3 Search forum topics
PR4 Read user profiles and recent activity
PR5 Reply after explicit user confirmation
PR6 Create a topic after explicit user confirmation
PR7 Complete the remaining mail-archive contract operations

Only PR1 is implemented here.
Skill integration, mega-topic pagination and response-byte/total-duration budgets remain follow-up design work.

Type of change

  • Tool / bridge
  • Python package
  • Documentation

Test plan

  • 279 mocked package tests, including native short-link precedence, explicit bare non-ASCII ID errors, ASCII-only IDs/post numbers, encoded and Unicode slugs, and canonical URL round trips; ruff check, ruff format check and mypy pass.
  • Repository integration: vendor-neutrality-score 42, skill-and-tool-validator 493, dev 277, spec-validator 87 and spec-inventory 31 pass; 930 total, no skips.
  • Full prek run --all-files --verbose, including offline lychee and all four workspace hooks: 40 hooks passed; 51 workspace members, 9,440 passed, 361 conditional skips, 4 deselected and 26 passing subtests.
  • Magpie checklist for both review follow-up commits: A-E pass, 0 blocking and 0 advisory items.
  • Historical local zizmor 1.30.1 offline scan (2026-10-23): no unsuppressed findings. Workflow files are unchanged in this correction; the scan was not repeated. API-dependent online audits were unavailable without a GitHub token and are not claimed as run.
  • Historical anonymous CLI smoke test, 2026-10-20: topic 183261, 3 GETs, 45 unique HTML posts, boolean body markers and truncated: true. No forum writes. This input-validation follow-up uses mocked HTTP; the earlier live smoke test is retained as historical evidence.
  • Conditional workspace skips and the absent optional Claude CLI are recorded separately; skipped checks are not counted as passes.
  • These local results apply to the audited commit eee77046 on the existing e6e5c2e feature baseline; they are not a full test run on the latest upstream main.
  • Static merge check against upstream main 27268f28 is clean; the merged tree was not tested.
  • Checks for older PR heads do not establish CI success for eee77046; updated-head GitHub CI will be assessed separately.

RFC-AI-0004 compliance

  • Sandbox: network reach declared; adopters allow their configured forum host.
  • Vendor neutrality: vendor-specific behavior stays in the partial adapter.
  • Write-access discipline: GET only, including redirects.
  • Privacy LLM: no model calls; callers establish visibility and apply existing privacy approval before sending content to a model.

Linked issues

Refs #307.
This first slice does not close the broader request.

Notes for reviewers

  • Numeric topic/post URLs follow Discourse's native route precedence: /t/2026/42 reads topic 2026 and /t/42/7 reads topic 42. Normal slug generation rejects numeric-only slugs. Topic IDs and post numbers use ASCII digits; Unicode slugs remain supported. See the Discourse routes and slug generation.
  • Bare non-ASCII numeric IDs fail before URL parsing with an explicit ASCII-digit requirement. URL routing and Unicode slug support are unchanged. Encoded numeric-slug cases have mocked coverage, without a live-forum verification.
  • Every response post requires a string cooked body, including posts beyond the output cap. Explicit empty strings, whitespace and placeholders are preserved; missing/null/invalid bodies fail the whole read.
  • body_hidden relays the optional boolean cooked_hidden marker, defaulting to false. It is independent of hidden; false proves neither original text nor public visibility.
  • Missing requested posts fail immediately after their batch, without later requests, retries or partial success. Duplicate stream IDs are rejected. Self-consistent empty results remain valid.
  • Messages follow forum stream order, which can differ from chronological archive order. truncated means only the post-count limit; it is not a byte or total-time budget. Mega topics without an ordinary ID stream are explicitly unsupported.
  • Credentials stay within the configured HTTPS origin and forum path. API usernames must be ASCII; keys must encode as latin-1. Paths use one strict UTF-8 decode; encoded separators, remaining percent escapes and ambiguous paths are refused. Configured base paths must percent-encode Unicode and spaces.
  • The output does not classify visibility or prove content is public. No skills, sandbox configuration, existing version numbers or CHANGELOG are changed.

AI disclosure

AI-generated PR description.
Claude Code assisted the initial prototype; Codex (GPT-6) assisted revisions, tests, validation and PR preparation.
The author has manually reviewed the implementation and all follow-up fixes through eee77046 and takes responsibility for this contribution.

Projects discussing design and governance on Discourse need structured
topic context alongside their existing mail and forge sources.

Read a topic and its bounded post stream with a stdlib-only CLI. Preserve
rendered bodies and server body-replacement markers, and fail explicitly
on missing requested posts or malformed responses.

Keep optional API credentials within the configured HTTPS forum origin
and path. Wire the package into the workspace, adapter inventory and
specifications while declaring partial mail-archive coverage.

This is the first implementation slice. Additional read commands,
confirmed writes and the remaining archive verbs are follow-up work.

Generated-by: Codex (GPT-6)
@github-actions github-actions Bot added family:tools tools/* family:docs Docs, MISSION.md, READMEs family:ci .github workflows, prek, validators contract:mail-archive Tool capability: mailing-list / forum archive reads labels Oct 21, 2026

@Kaap10 Kaap10 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.

Thank you for this contribution! This is an exceptionally clean, well-tested, and secure initial slice for #307.

Review Summary

Issue #307 is actively tracked and directly relevant to the project roadmap (Discourse context is needed by projects like Python core, Rust, NixOS, Julia, Docker, and ASF podlings). Delivering this in scoped, incremental PRs is an effective strategy.

Key Highlights & Strengths

  1. Strict Standard-Library Discipline:
    • Zero external runtime dependencies; uses standard library urllib, http.client, dataclasses, and json.
  2. Robust Security Boundaries (RFC-AI-0004 & AGENTS.md):
    • Enforces GET-only at the transport and redirect levels.
    • SameOriginRedirectHandler ensures credentials (Api-Key, Api-Username) never cross origin or escape the configured forum subpath.
    • Rejects directory traversal (..), backslashes, control characters, and ambiguous percent-encoding (%2f, %5c, %25) prior to dispatch.
    • Forum post content (cooked HTML) is explicitly treated as untrusted external data.
  3. Comprehensive Error Handling & Edge Cases:
    • Fails fast on missing post IDs rather than returning incomplete thread data.
    • Rejects unsupported mega-topics cleanly (isMegaTopic: true).
    • Normalizes both legacy string tags and modern Discourse tag objects.
  4. Complete Repository Integration:
    • Correctly registered across pyproject.toml, uv.lock, .github/labeler.yml, docs/adapters/registry.md, docs/labels-and-capabilities.md, docs/vendor-neutrality.md, and tools/spec-loop/specs/adapters.md.
    • All 20 CI checks passed green on GitHub Actions.

Follow-Up Design Notes for Future PRs (PR2–PR7)

  • Rate-Limiting (HTTP 429) Backoff: The client currently surfaces Retry-After on 429 without unbounded retries. In PR2/PR3 (category lists and full-text searches), consider an optional bounded backoff policy for large sweeps.
  • Downstream HTML Handling: Downstream skills reading body_format: "html" should ensure proper sanitization/stripping before rendering into terminal or plain-text markdown summaries.

Comment thread tools/mail-archive/README.md Resolved
@MakerFlyFly

Copy link
Copy Markdown
Contributor Author

Thx for your review! I'll keep your suggestions in mind for the follow-up PRs

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A good first slice of #307: the bridge is GET-only, refuses a redirect to another origin before it can forward the API key, insists on https://, rejects header injection through the credentials, and fails loudly on a missing post instead of returning a partial thread; declaring Coverage: partial until PR7 is the right call. One parsing ambiguity in topic URLs below.

Smaller observations

  • See inline comments on tools/discourse/src/magpie_discourse/topics.py:55.

This review was drafted by an AI-assisted tool and
confirmed by an Apache Magpie maintainer. The findings
below are observations, not blockers; an Apache Magpie
maintainer — a real person — will take the next look at the
PR. If you think a finding is mis-applied, please reply on
the PR and a maintainer will weigh in.

More on how Apache Magpie handles maintainer review:
Contributing guide.

Two numeric path segments can identify a numeric slug and topic id or a
topic id and post number. Prioritizing the short form can silently fetch
a different topic, and comparing the response id cannot detect that.

Reject the ambiguous form before HTTP, including escaped numeric slugs.
Keep explicit topic ids and unambiguous topic URLs, and document the
deliberate compatibility limit without adding network disambiguation.

Exercise the CLI, subpath deployments, share links and numeric-slug post
URLs with mocked HTTP. Preserve the existing JSON and read-only scope.

Generated-by: Codex (GPT-6)
@MakerFlyFly
MakerFlyFly marked this pull request as draft October 23, 2026 16:13
Restore Discourse's numeric topic/post route precedence so notification
links such as /t/42/7 resolve to topic 42. Normal Discourse slug generation
rejects numeric-only slugs.

Use ASCII digits for topic ids and post numbers, and cover native short
links, encoded slugs, Unicode input and canonical URL round trips. Align
the README and adapter specification with the supported routes.

Generated-by: Codex (GPT-6)
Report an ASCII-digit requirement for bare non-ASCII numeric references
before attempting URL parsing. Preserve native topic URL routing, Unicode
slugs, positive-integer validation and request-free input failures.

Cover mixed digits, superscripts and whitespace while checking the CLI
error text and retaining the existing credential and request assertions.

Generated-by: Codex (GPT-6)
@MakerFlyFly
MakerFlyFly marked this pull request as ready for review October 23, 2026 17:56
@MakerFlyFly
MakerFlyFly requested a review from potiuk October 24, 2026 02:38

@potiuk potiuk left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for checking the topic-URL point against Discourse itself: its slug generator refuses all-digit slugs ("they would be indistinguishable from id's", lib/slug.rb), so /t/2026/42 can only mean topic 2026, post 42, and following Discourse's route precedence is the right reading. The README and spec now say so, and the new tests pin the precedence, the round trip of the canonical URL, and the refusal of non-ASCII digits before any request. With that, this first slice of #307 is ready.


This review was drafted by an AI-assisted tool and
confirmed by an Apache Magpie maintainer. The maintainer
approving this PR has read the findings and signed off. If
something feels off, please reply on the PR and a maintainer
will follow up.

More on how Apache Magpie handles maintainer review:
Contributing guide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contract:mail-archive Tool capability: mailing-list / forum archive reads family:ci .github workflows, prek, validators family:docs Docs, MISSION.md, READMEs family:tools tools/*

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants