Skip to content

fix(hooks): pass appended review on ctx.extra.review for after:review#436

Merged
eris-ths merged 2 commits into
mainfrom
fix/after-review-extra-payload
Jun 17, 2026
Merged

fix(hooks): pass appended review on ctx.extra.review for after:review#436
eris-ths merged 2 commits into
mainfrom
fix/after-review-extra-payload

Conversation

@eris-ths

Copy link
Copy Markdown
Owner

What

The gate review handler called fireAfterHook without the extra
argument
, so ctx.extra was undefined for every after:review
hook — even though docs/plugin-schema.md documents extra.review
as the contract:

Only before:review / after:review set extra.review (the
appended review record).

A hook reading ctx.extra.review.verdict to route reject/concern
verdicts therefore saw undefined and silently did nothing.

How

review.ts now passes the terminal (just-appended) review as
{ review }:

const appendedReview = updated.reviews[updated.reviews.length - 1];
await fireAfterHook(c.hookSubscriptions, 'review', updated, by, {
  review: appendedReview,
});

HookBus.fireAfterHook already accepted the optional extra arg —
only the call site was missing it.

Test

Adds a regression test in tests/interface/hookPluginLoader.test.ts
asserting an after:review hook receives lense / verdict /
comment / by on ctx.extra.review. Before this fix the same test
records NO_EXTRA.

How it surfaced

Dogfood. A review → issue automation hook on a private deployment
read ctx.extra.review.verdict and got nothing — the documented
payload never arrived. Tracing it back led to the missing call-site
argument.

Scope note

before:review carrying its own payload is out of scope here. It
fires before the append, so the documented "appended review" shape
needs a distinct decision (the review-to-be vs. the review-as-landed).
Tracking that separately rather than guessing in this PR.

Pre-existing test failures (unrelated to this PR)

main currently has ~19 failing tests unrelated to this change:
tools/lore-scope.sh is absent (stale dist/ artifacts),
#239 expects the removed --executor singular flag, and a
doctor issues-dir detection case. This PR's diff touches only the
after:review hook call site and its test; the new regression test
passes.

eris-ths added 2 commits June 17, 2026 12:55
The review handler called fireAfterHook without the extra argument,
so ctx.extra was undefined for after:review subscribers — even though
docs/plugin-schema.md documents extra.review as the contract. A hook
reading ctx.extra.review.verdict to route reject/concern verdicts
silently saw undefined.

Pass the terminal (just-appended) review as { review }. Add a
regression test asserting the hook receives lense/verdict/comment/by.

Surfaced by dogfood: a review→issue automation hook on a private
deployment read ctx.extra.review and got nothing.

before:review carrying its own payload is tracked separately (it
fires before the append, so the documented shape needs a distinct
decision).
@eris-ths eris-ths merged commit 22b7988 into main Jun 17, 2026
5 checks passed
@eris-ths eris-ths deleted the fix/after-review-extra-payload branch June 17, 2026 04:23
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