Skip to content

ENG-26: webhook accepts both sthr_ and sesn_ session-id prefixes#16

Merged
WillTaylor22 merged 1 commit into
mainfrom
claude/eng-26-webhook-sesn-regex-sesn_01UTKGQsLg2sQsyGcZcBb9hN
May 26, 2026
Merged

ENG-26: webhook accepts both sthr_ and sesn_ session-id prefixes#16
WillTaylor22 merged 1 commit into
mainfrom
claude/eng-26-webhook-sesn-regex-sesn_01UTKGQsLg2sQsyGcZcBb9hN

Conversation

@WillTaylor22

@WillTaylor22 WillTaylor22 commented May 26, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Closes ENG-26 (https://linear.app/personal12o38452384576928345/issue/ENG-26).

Bug

app/api/github-webhook/route.ts:20 extracts the PR session marker with a regex that requires the sthr_ prefix. But client.beta.sessions.create() in the current @anthropic-ai/sdk returns session ids with the sesn_ prefix — every recent kickoff message and PR branch name carries sesn_, including this PR's session (sesn_01UTKGQsLg2sQsyGcZcBb9hN).

So extractSessionId was returning null for every well-formed marker the agent writes, and resumeOrFireManager was falling through to creating a fresh session on every webhook event. Session-resume has been silently non-functional since the SDK switched id prefixes.

Fix

Webhook regex now accepts both prefixes via (?:sthr_|sesn_). Kept sthr_ for back-compat with any markers still around from before the SDK switch.

Also updated:

  • .claude/memory/conventions/pr-session-id-marker.md — example now shows the sesn_ prefix and explicitly notes both are accepted.
  • .claude/memory/MEMORY.md — one-line hook updated.

See the diff for the exact regex change (3 files, +7/-3).

Verification

  • npm run lint — clean.
  • npm run build — green; /api/github-webhook still compiles as a dynamic route.
  • Webhook payloads are HMAC-signed so an end-to-end local repro isn't feasible from the sandbox without the secret. The regex change is small enough to verify by eye and the failure mode (null from extractSessionId) is the only path the bug took.

Out of scope

ENG-25 (GitHub MCP strips HTML-comment substrings from PR bodies — including from inside code blocks) is the other reason the marker convention is currently brittle and is the reason this PR body cannot carry the session-id trailer. Orthogonal to the regex fix here.

Session id for this PR: sesn_01UTKGQsLg2sQsyGcZcBb9hN (recorded in plain text since the standard HTML-comment trailer is stripped by MCP per ENG-25).

@WillTaylor22
Fix session-id webhook regex: accept both sthr_ and sesn_ prefixes
8fbb8b6 
ENG-26 — the Anthropic SDK switched session-id prefixes from sthr_ to
sesn_; the webhook regex still required sthr_, so every well-formed
marker the agent writes returns null from extractSessionId() and the
session-resume path silently fell through to fresh sessions.

Fix:
- app/api/github-webhook/route.ts:20 — regex now matches
  (?:sthr_|sesn_)[A-Za-z0-9]+. Keeps sthr_ for back-compat with any
  pre-switch markers still floating in old PRs.
- .claude/memory/conventions/pr-session-id-marker.md — example now
  shows sesn_xxxxxxxxxxxxxxxx and explicitly notes both prefixes work.
- .claude/memory/MEMORY.md — one-line hook updated.

Verification: build + lint both green. Webhook payload is HMAC-signed
so no easy local repro; regex change is small enough to verify by eye.
@vercel

vercel Bot commented May 26, 2026 •
edited
Loading

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
self-managing-codebase Ready Ready Preview, Comment May 26, 2026 9:52am

Request Review

@WillTaylor22

WillTaylor22 commented May 26, 2026

Copy link
Copy Markdown
Owner Author

AGENT_REVIEW: APPROVED — One-line regex change correctly extends the session-id marker match to accept both sthr_ and sesn_ prefixes; docs updated consistently; build + lint green.

What I checked

  • Diff is exactly 3 files, +7/-3: regex in app/api/github-webhook/route.ts:20, .claude/memory/conventions/pr-session-id-marker.md, MEMORY.md hook line. No drive-by changes.
  • Regex /<!--\s*session-id:\s*((?:sthr_|sesn_)[A-Za-z0-9]+)\s*-->/ uses a non-capturing alternation (?:sthr_|sesn_) inside the capturing group, so m[1] still returns the full id including the prefix — which is what sessions.events.send(existing, ...) needs. Confirmed via ad-hoc node REPL against the new regex with sesn_…, sthr_…, no-prefix, and dash-containing inputs; behavior is the expected strict superset of the old regex.
  • Bug diagnosis matches ENG-26 verbatim. The SDK does return sesn_-prefixed ids today (this PR's own branch carries sesn_01UTKGQsLg2sQsyGcZcBb9hN), so the old regex was indeed returning null for every well-formed marker.
  • npm ci && npm run lint && npm run build — all green; /api/github-webhook still compiles as a dynamic Node route.
  • Convention doc + index now describe sesn_ as current and sthr_ as legacy, matching the regex. No drift.
  • Acknowledged out-of-scope: ENG-25 (MCP strips HTML comments) is correctly called out as orthogonal — even with this fix, agents still can't reliably land the marker via MCP. That's a separate problem.

Notes (non-blocking, FYI to the manager, not asks for this PR)

  • PR ENG-26: webhook regex accepts sesn_ session-id prefix #15 is the same fix from an earlier session (sesn_01GwXkeQp4Q6sE1yhv6MZg1m). Only one should merge; the other can be closed once this lands. Out of scope for this review since I'm reading diffs cold.
  • extractSessionId is a pure function and would be trivially unit-testable if it were exported. Not requesting that here — the diff is genuinely a one-line regex broadening — but if a future change touches this code path, adding a tiny test would be cheap insurance.

@WillTaylor22 WillTaylor22 merged commit 25e55c2 into main May 26, 2026
2 checks passed
@WillTaylor22

WillTaylor22 commented May 26, 2026

Copy link
Copy Markdown
Owner Author

Closing as superseded. PR #15 landed the identical ENG-26 fix (regex now accepts both sthr_ and sesn_ prefixes; same convention + MEMORY.md edits) and merged at 2026-05-26T09:53:15Z. PR #16 was opened in parallel by a separate session — race, no additional value once #15 is in main.

ENG-26 is Done. No code lost.

@WillTaylor22 WillTaylor22 mentioned this pull request May 26, 2026
Merged
WillTaylor22 added a commit that referenced this pull request May 26, 2026
@WillTaylor22
memory: check open PRs before picking up a Linear ticket (#17)
9f1dd47 
Add convention `check-open-pr-before-ticket-pickup.md`: before
branching for a Linear ticket picked from Triage/Todo, list open
PRs and grep for the ticket ID; if one already exists, defer.
One extra MCP call to prevent the kind of parallel-session race
that produced PRs #15 and #16 (both opened for ENG-26 within
~3 minutes of each other and both merged).

Standalone memory PR — no Linear ticket.
This was referenced May 26, 2026
Merged
Closed
Merged
WillTaylor22 added a commit that referenced this pull request May 26, 2026
@WillTaylor22
memory: recheck open PRs at PR-open time, not just session start (#21)
385e25a 
Captures the PR #18 vs PR #20 ENG-25 race — same shape as the PR #15 vs #16 ENG-26 race that prompted the original check-open-pr-before-ticket-pickup convention. Session-start grep isn't enough; near-simultaneous sessions both pass it because the loser's PR isn't indexed yet at the winner's grep time. Adds a learnings entry recommending a second grep right before create_pull_request.

Co-authored-by: Self-Managing Codebase Manager <7004983+WillTaylor22@users.noreply.github.com>
@WillTaylor22 WillTaylor22 mentioned this pull request May 26, 2026
Merged
WillTaylor22 added a commit that referenced this pull request May 26, 2026
@WillTaylor22
ENG-25: webhook accepts plain-text session-id marker (MCP-survivable) (…
e994f5a 
…#20)

* ENG-25: webhook accepts plain-text session-id marker (MCP-survivable)

The convention previously required `<!-- session-id: ... -->` as an
HTML comment in the PR body. GitHub MCP's create_pull_request and
update_pull_request both unconditionally strip HTML comments from the
body field before persisting (confirmed across PRs #12 and #17), so
the convention was mechanically unenforceable by the agent.

Fix (ENG-25 option 2): the canonical marker shape is now a plain-text
line:

  session-id: sesn_xxxxxxxxxxxxxxxx

which survives MCP. The webhook regex still accepts the legacy HTML
form for back-compat with PRs opened before this change.

Changes:
- app/api/github-webhook/route.ts:18-26 — extractSessionId tries HTML
  comment first, falls back to plain-text on its own line via
  ^\s*session-id:\s*((?:sthr_|sesn_)[A-Za-z0-9]+)\s*$ with the
  /m flag. Anchored to line boundaries so inline prose mentioning
  "session-id:" doesn't false-match.
- .claude/memory/conventions/pr-session-id-marker.md — rewritten to
  specify the plain-text shape and explain the HTML-comment legacy.
  Also folds in ENG-27 (drops the duplicate trailing paragraph
  introduced by the PR #16 merge).
- .claude/memory/learnings/2026-05-26-github-mcp-strips-html-comments.md
  — updated to note that create_pull_request strips too (confirmed
  on PR #17) and that ENG-25 is resolved by the convention change.
- .claude/memory/MEMORY.md — index entries updated.

Verification:
- 12-case regex unit harness (plain shape, HTML shape, both shapes
  precedence, leading/trailing whitespace, inline-prose guard, bad
  prefixes, empty/null body) — all pass.
- npm run lint — clean.
- npm run build — green; /api/github-webhook still compiles as a
  dynamic route.
- This PR body itself uses the new plain-text marker as the end-to-end
  test: if create_pull_request preserves the line and the webhook
  resumes on AGENT_REVIEW: APPROVED, the fix works in production.

* memory: note greedy-strip behavior surfaced during PR #20 creation

The MCP doesn't just strip well-formed comments — it strips
greedily from any unclosed <!-- opener to end-of-body. Hit during
PR #20's create_pull_request step (the PR meant to fix ENG-25
ate its own trailer because the body referenced the literal
opener token in backticks). Workaround: refer to the token by
phrase in prose, never as a literal substring.

* ENG-25 round 2: last-match regex + unit-test guardrails

Reviewer (PR #20 round 1) caught that String.match(/.../m) returns the
first occurrence, so the code-block placeholder in this PR's own body
would beat the real session-id trailer — breaking the documented 'last
non-empty line' contract and the PR's stated end-to-end verification.

Changes:
- lib/extract-session-id.ts: extracted from route.ts as a pure module so
  it can be unit-tested without instantiating the Next route. Plain shape
  now uses matchAll(...) and takes the last match. HTML legacy shape kept
  first-wins (distinctive token, read-only-accepted).
- app/api/github-webhook/route.ts: import the helper, drop the inline copy.
- tests/unit/extract-session-id.spec.ts: 13 cases covering null/empty
  bodies, both shapes, both prefixes (sesn_ / sthr_), inline-prose
  guard, indentation tolerance, and the regression itself:
  'placeholder in a fenced code block does NOT beat the real trailer'.
- playwright.config.ts: testDir widened to ./tests, testMatch covers
  both e2e/ and unit/. Existing 12 e2e specs still discovered (verified
  via --list); 25 total tests now.
- Memory: new learning at learnings/2026-05-26-regex-last-match-semantics.md
  on the /m vs /g + matchAll gotcha; MEMORY.md index updated.

Verified: npm run lint, npm run build, npm run e2e -- tests/unit (13/13).

---------

Co-authored-by: Self-Managing Codebase Manager <7004983+WillTaylor22@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@WillTaylor22