Skip to content

test(canonical2): H10 reconcile-failure observability trap + behavior-gap todo#508

Closed
elliott-dandelion-cult wants to merge 1 commit intofrond-scribe/325-canonical2-pathB-rebasefrom
elliott/h10-throw-shape-trap
Closed

test(canonical2): H10 reconcile-failure observability trap + behavior-gap todo#508
elliott-dandelion-cult wants to merge 1 commit intofrond-scribe/325-canonical2-pathB-rebasefrom
elliott/h10-throw-shape-trap

Conversation

@elliott-dandelion-cult
Copy link
Copy Markdown

@elliott-dandelion-cult elliott-dandelion-cult commented May 1, 2026

Summary

  • Observability contract trap: asserts all three reconcile-failure surfaces (ctx.log.warn with [compaction-counter:reconcile-failed], emitAgentEvent with stream:"compaction" + warning data shape, onAgentEvent callback with same shape) fire when reconcileSessionStoreCompactionCountAfterSuccess rejects.
  • Behavior contract trap: pins that handleCompactionEnd emits completed=true in the phase:"end" event even when the reconcile rejects — makes a future fix visible as a test change.
  • it.todo: documents the fire-and-forget gap where handleCompactionEnd swallows reconcile failures in .catch and returns success regardless.

Audit-gap-#1 followup from the cohort cleanup waves. Wave-D observability landed at bbcf2f3 (fix: surface compaction count reconcile failures).

Test plan

  • pnpm vitest run src/agents/pi-embedded-subscribe.handlers.compaction.test.ts — 7 passed, 1 todo
  • pnpm tsgo:core — clean

…-gap todo

Pin the three observability surfaces (ctx.log.warn, emitAgentEvent,
onAgentEvent) that fire when reconcileSessionStoreCompactionCountAfterSuccess
rejects, and pin the behavior contract that handleCompactionEnd emits
completed=true regardless of reconcile outcome.

Add it.todo documenting the fire-and-forget gap (audit gap #1 from
wave-D bbcf2f3).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@elliott-dandelion-cult elliott-dandelion-cult force-pushed the elliott/h10-throw-shape-trap branch from 262d992 to a015af0 Compare May 1, 2026 23:56
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a015af0305

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

sessionKey: ctx.params.sessionKey,
data,
});
void Promise.resolve(ctx.params.onAgentEvent?.({ stream: "compaction", data })).catch(
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Guard reconcile-warning callback against sync throw

When reconcileSessionStoreCompactionCountAfterSuccess fails, this path now calls onAgentEvent via Promise.resolve(ctx.params.onAgentEvent?.(...)). Because the callback invocation is evaluated before Promise.resolve, a synchronous throw in onAgentEvent escapes this handler, turning the reconcile-failure path into an unhandled rejection instead of a logged warning. This can happen with custom onAgentEvent implementations that throw before returning a promise. Wrap the invocation in a deferred callback (for example, Promise.resolve().then(() => onAgentEvent?.(...))) so both sync throws and async rejections are contained.

Useful? React with 👍 / 👎.

@elliott-dandelion-cult elliott-dandelion-cult changed the base branch from cael/325-canonical2 to frond-scribe/325-canonical2-pathB-rebase May 2, 2026 00:11
@elliott-dandelion-cult
Copy link
Copy Markdown
Author

🌻 — base retargeted from cael/325-canonical2frond-scribe/325-canonical2-pathB-rebase. The trap test exercises behavior introduced by bbcf2f3ad8 (wave-D H10 cure), which lives on the rebase branch but hasn't yet merged to canonical2. PR base must have the cure under test. Diff is now correctly +113/-0 single file (was inflated to 1797/-8252 by the unmerged rebase delta).

Will merge with the rebase wave when it lands on canonical2.

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.

2 participants