Skip to content

fix: add padId to padUpdate/padCreate hook context #7452

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JohnMcLear merged 1 commit into from
Apr 4, 2026
+1 −0
Merged

fix: add padId to padUpdate/padCreate hook context #7452

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/node/db/Pad.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ class Pad {
authorId && authorManager.addPad(authorId, this.id),
hooks.aCallAll(hook, {
pad: this,
padId: this.id,
authorId,
Comment on lines 126 to 129
Copy link
Copy Markdown

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot Apr 4, 2026

Choose a reason for hiding this comment

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

Action required

1. padid undocumented in hooks 📘 Rule violation ⚙ Maintainability

The PR adds a new top-level padId property to the padCreate/padUpdate hook context, but the
server-side hooks documentation does not mention it. This violates the requirement to update doc/
alongside public hook payload changes, leaving plugin authors unaware of the new field.
Agent Prompt 
## Issue description
The `padCreate` and `padUpdate` hook context now includes a new top-level `padId` field, but the hook documentation does not list it.

## Issue Context
Plugins often serialize the hook context, and `pad.toJSON()` strips `id`, so `padId` is the reliable serialized identifier. Docs should reflect the new payload shape.

## Fix Focus Areas
- doc/api/hooks_server-side.md[267-328]
- doc/api/hooks_server-side.adoc[270-331]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines 126 to 129
Copy link
Copy Markdown

@qodo-free-for-open-source-projects qodo-free-for-open-source-projects Bot Apr 4, 2026

Choose a reason for hiding this comment

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

Action required

2. No regression test added 📘 Rule violation ☼ Reliability

The PR changes the public hook payload by adding padId to the padCreate/padUpdate context, but
the provided diff includes no accompanying automated regression test. Without a test, this bug can
easily reappear if the field is removed or refactored away.
Agent Prompt 
## Issue description
There is no automated regression test asserting that the `padCreate` and `padUpdate` hook context includes `padId` (and that it survives serialization).

## Issue Context
The change is meant to ensure plugins can reliably determine the pad identifier even if they serialize the hook context. A regression test should fail if `padId` is removed/reverted.

## Fix Focus Areas
- src/tests/backend/specs/Pad.ts[1-142]
- src/node/db/Pad.ts[110-142]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

get author() {
pad_utils.warnDeprecated(`${hook} hook author context is deprecated; use authorId instead`);
Expand Down
Loading