Repository navigation
fix: isolate terminal plugin error snapshots - #866
Open
zhongkechen wants to merge 6 commits into
Open
zhongkechen wants to merge 6 commits into
zhongkechen wants to merge 6 commits into
Conversation
3 tasks done
Contributor
Codex AI reviewNo actionable findings or violations of CONTRIBUTING rules 1–9 identified. Unit and e2e regressions cover both changed error-copy paths. Residual test risk: tests and type checks were inspected but not executed during this read-only review. Reviewed commit |
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue #, if available
Fixes #852.
Description of changes
Terminal operation hooks now copy the error's mutable stack-trace list at the two
existing assignments:
_end_infofor checkpointed operations andon_child_context_endfor child contexts that finish without a checkpoint.A plugin clearing its snapshot cannot change stored error diagnostics, a later
snapshot, or the
ChildContextErrorsubsequently raised to the caller.The runtime diff is exactly two
error=lines.OperationInfo.from_operationremains a staticmethod, and both terminal-info constructors keep their existing
field conversion and delivery behavior.
Design
changing the public API. The requested review revision narrows the change to
the two existing error assignments.
_copy_error. This reuses the narrow list copy without introducing a factoryrefactor, subclass behavior change, deep copy, or scheduling changes.
stack-trace list. The virtual-child path also reuses the ErrorObject to raise
ChildContextErrorafter the terminal hook returns, so a plugin could changethe user's exception even without a child checkpoint.
changes.
Validation
STEP, CALLBACK, CHAINED_INVOKE, and CONTEXT, through natural and queued delivery.
PR head (235d8de): both observed a cleared original/raised stack trace. They
pass with the second error-copy assignment.
hatch run dev-core:test --tb=short: 2,121 passed and 24 subtests passed.hatch run dev-core:typecheck: no issues in 95 source files.hatch fmt --check, commit-message validation, andgit diff --check:passed.
Public-handler coverage verifies stored step failures replay without executing
step bodies and virtual-child failures reach the caller with their original
message/type/data/stack despite a mutating terminal plugin. Metadata, hook counts,
replay flags, and absent/empty error semantics remain covered.
Existing test update:
test_builds_terminal_operation_infonow supplies a mutablestack and verifies snapshot mutation leaves the input error intact; its previous
field/delivery expectations remain. The earlier PR-added subclass-factory test
is replaced by terminal-info field preservation, matching the unchanged static
factory contract.
Assumptions
and the public durable handler. No new deployed-service assumption is needed
for these ownership changes.
change OTel rendering or testing-service behavior.
Decisions not covered by the issue or the design note
behavior is not changed by this fix.
Plugins receiving the same hook object still share that snapshot.
Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and
redistribute this contribution, under the terms of your choice.