Skip to content

fix(dsh): defer automatic work until step admission - #1856

Merged
Teingi merged 3 commits into
oceanbase:masterfrom
DongYao0:codex/fix-1785-hook-ordering
Oct 6, 2026
Merged

Teingi merged 3 commits into
oceanbase:masterfrom
DongYao0:codex/fix-1785-hook-ordering

Conversation

@DongYao0

@DongYao0 DongYao0 commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

Which issue or RFC does this PR close?

Addresses the hook-ordering part of #1785.

Rationale for this change

The DSH automatic pre-step hook previously performed PowerContext work before the downstream hook had admitted the step. Scope resolution, context preparation, prompt capture, and flushing could therefore occur even when a later hook rejected the step or failed.

This change defers all PowerContext side effects until the downstream decision returns kind: "enter".

What changes are included in this PR?

  • Run the downstream pre-step handler before starting PowerContext work.
  • Perform no PowerContext requests when the downstream step rejects or throws.
  • Use the final downstream message batch for context preparation and prompt capture.
  • Preserve cancellation and fail-open behavior.
  • Preserve single-snapshot injection.
  • Report downstream_failed and downstream_rejected in runtime observations.
  • Add regression coverage for rejected, failed, rewritten, empty, and cancellation paths.
  • Rebuild the checked-in DSH plugin output.

Are there any user-facing changes?

Yes. Rejected or failed DSH steps no longer trigger PowerContext requests or automatic capture side effects.

There are no breaking changes to public APIs or persisted formats.

How was this change tested?

  • pnpm --dir integrations/dsh/plugins/powercontext test:all
    • 20 test files passed.
    • 276 tests passed.
  • pnpm --dir integrations/dsh/plugins/powercontext build
    • Build completed successfully.
  • Runtime acceptance was also exercised locally. The updated hook reached the post-admission Scope, prepare, capture, and flush path. Final completion was unavailable because the configured test inference provider returned 503 inference_unavailable during flush.

AI usage statement

OpenAI Codex was used for source-code tracing, implementation, regression-test design, and validation. The changes were verified using the repository test suite and build process.

@DongYao0
DongYao0 requested a review from Teingi as a code owner October 6, 2026 05:46

@Teingi Teingi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

One capture regression remains when a slow pre-step hook runs downstream of PowerContext. Details inline.

if (content) observation?.record('injection', { state: 'running' })
let downstream: PreStepDecision
try {
downstream = await input.next()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

[P2] Start the PowerContext deadline after downstream admission

registerRecall() still starts the default 4-second deadline before this next() call. Moving recall and capture after it allows an accepted downstream hook to exhaust the budget before PowerContext starts. I reproduced this with DSH SDK 0.1.2-rc.1 and a local PowerContext Server: register the native hooks-codex handler during agent/session-start so it runs downstream, and have its UserPromptSubmit command run sleep 5 before allowing the prompt. The base captures the Source successfully (202); this head skips every automatic stage with deadline_exceeded, although the model turn completes normally. This depends on downstream registration order; the default static hook ordering was unaffected. Please start the PowerContext work budget after downstream acceptance while continuing to honor caller cancellation.

@Teingi Teingi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@Teingi
Teingi merged commit c7be357 into oceanbase:master Oct 6, 2026
24 of 25 checks passed
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