Skip to content

docs(modules): precision follow-up — event-transaction scope, payload sensitivity (HT-71) - #84

Merged
zaridan merged 1 commit into
mainfrom
docs/ht-71-precision-followup
Jul 19, 2026
Merged

docs(modules): precision follow-up — event-transaction scope, payload sensitivity (HT-71)#84
zaridan merged 1 commit into
mainfrom
docs/ht-71-precision-followup

Conversation

@zaridan

@zaridan zaridan commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Closes the two CodeRabbit actionables posted in #81's final review round (which landed after merge): the blanket same-transaction claim now names the test.ping bypass, and the payload-privacy wording stops claiming "PII-free by construction" (tag strings are operator-authored; identifiers reveal existence). Docs-only.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation
    • Clarified that eight domain event types are recorded transactionally with state changes, while test.ping bypasses the outbox.
    • Updated webhook guidance to explain that message content is excluded from payloads and must be retrieved through the read API.
    • Added reminders to treat webhook payloads as sensitive, including operator-authored text and identifiers that may reveal conversation activity.

… sensitivity (HT-71)

The two CodeRabbit actionables that landed after #81's merge window:
the same-transaction claim now names test.ping's bypass, and the
payload-privacy claim no longer says "PII-free by construction" —
tag strings are operator-authored free text and identifiers still
reveal conversation existence.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 19, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The module documentation now specifies transactional typed event emission, the test.ping outbox exception, webhook content boundaries, read API retrieval, and remaining payload sensitivity.

Changes

Module documentation

Layer / File(s) Summary
Event and webhook semantics
docs/modules/README.md, docs/modules/webhooks.md
Typed events are documented as transactionally emitted except for test.ping; webhook guidance excludes message content, directs retrieval through the read API, and notes sensitive text and revealing identifiers.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Possibly related PRs

  • Helpthread/helpthread#81: Introduced related module documentation covering typed events and webhook payload semantics.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the documentation follow-up about event-transaction scope and payload sensitivity.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/ht-71-precision-followup

Comment @coderabbitai help to get the list of available commands.

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/modules/webhooks.md`:
- Around line 125-129: Update the webhook sensitivity guidance in the
surrounding documentation to state that identifiers reveal a conversation
exists, without implying it is active. Preserve the warning about sensitive
payloads and operator-authored free text, and account for conversations in
closed, pending, or spam states.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 49f27288-f102-4750-971a-9118ff9a123e

📥 Commits

Reviewing files that changed from the base of the PR and between 0943eb3 and 7e2c756.

📒 Files selected for processing (2)
  • docs/modules/README.md
  • docs/modules/webhooks.md

Comment thread docs/modules/webhooks.md
Comment on lines +125 to +129
own credentials once an event tells you something changed. Message content
therefore never transits a webhook — but treat payloads as sensitive
anyway: some fields are operator-authored free text (`conversation.tags_changed`
carries the tag strings themselves), and identifiers still reveal that a
given conversation exists and is active.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Avoid implying every identified conversation is active.

A webhook can reference conversations in closed, pending, or spam states, so the identifier alone establishes existence, not that the conversation is active.

Proposed wording
- and identifiers still reveal that a
- given conversation exists and is active.
+ and identifiers still reveal that a given conversation exists; some event
+ types may also expose its current status.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
own credentials once an event tells you something changed. Message content
therefore never transits a webhook — but treat payloads as sensitive
anyway: some fields are operator-authored free text (`conversation.tags_changed`
carries the tag strings themselves), and identifiers still reveal that a
given conversation exists and is active.
own credentials once an event tells you something changed. Message content
therefore never transits a webhook — but treat payloads as sensitive
anyway: some fields are operator-authored free text (`conversation.tags_changed`
carries the tag strings themselves), and identifiers still reveal that a given conversation exists; some event
types may also expose its current status.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/modules/webhooks.md` around lines 125 - 129, Update the webhook
sensitivity guidance in the surrounding documentation to state that identifiers
reveal a conversation exists, without implying it is active. Preserve the
warning about sensitive payloads and operator-authored free text, and account
for conversations in closed, pending, or spam states.

@zaridan
zaridan merged commit 071f715 into main Jul 19, 2026
5 checks passed
@zaridan
zaridan deleted the docs/ht-71-precision-followup branch July 19, 2026 17:59
zaridan added a commit that referenced this pull request Jul 19, 2026
…sics (#91)

PR #83 landed already-stale: main moved by nine PRs while it was in review,
and it shipped one claim that contradicts the charter.

- HT-71 operator guide was listed as an open PR under Next. #81 merged at
  17:48, ~2h before #83 merged at 19:39, and docs/modules/ has been on main
  since. Moved to Done with its precision follow-up (#84).
- Marketplace was listed under 'Not yet / deferred'. CHARTER §3/§4/§5 were
  amended the same day (HT-79, #86) to make it a launch-day component of
  Phase 3 — built now, proven as the dogfood install path. Removed from
  deferred; marketplace v1 spec (#87, draft) now leads Next.
- Added inbox basics (HT-76/77/78, #90, migrations 24-25): saved replies &
  macros, snooze, send & close — shipped engine features with no STATUS line.
- Added catalog reclassification (HT-75, #82): KB and end-user portal are
  paid, 71-module gap audit closed, open-core line restated. Passkeys stay
  core, reconciled in #85.
- Added passkey login spec (HT-75, specs/auth/passkeys.md, draft.3) to Next.

Co-authored-by: Claude Fable 5 <noreply@anthropic.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.

1 participant