Skip to content

Add plugin hook-info field-shape requirements 10-19..10-22 - #72

Open
wangyb-A wants to merge 12 commits into
mainfrom
plugin-hook-field-shape
Open

Add plugin hook-info field-shape requirements 10-19..10-22#72
wangyb-A wants to merge 12 commits into
mainfrom
plugin-hook-field-shape

Conversation

@wangyb-A

@wangyb-A wangyb-A commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds 5 plugin-suite requirements (10-19..10-23) probing the field shape of every plugin hook's info parameter across SDKs, using the union of SDK field sets as the reference schema.

Design: each handler logs one single-line JSON record per hook event — a canonical camelCase dump of that hook's own info. Unexposed fields are omitted, so a missing key = a failed assertion = an SDK parity gap. Maps → <name>Count; timestamps → ISO-8601 regex; presence-only probes via /.*/; key-absence guards via /.*/ + count: 0. Payload values (execution input/result, operation result) are not part of the asserted schemas. 10-14 on main is untouched.

ID Hooks probed
10-19 invocation-start / invocation-end
10-20 operation-start / operation-end
10-21 attempt-start / attempt-end (retry, ordered 1→2)
10-22 operation-change (delta item + map membership)
10-23 context-typed ops (parallel): subType, isReplayingChildren

Live validation (us-west-2)

Req Python JS Java
10-19 ❌ no operations maps ❌ no isFirstInvocation on end info ❌ no maps / end-side start time
10-20
10-21 ❌ failed-attempt-end timestamps ❌ no isReplay
10-22 ❌ reduced change item
10-23 ❌ no isReplayingChildren
Totals 22/23 20/23 18/21

Every red is a filed SDK gap: aws-durable-execution-sdk-python#617, aws-durable-execution-sdk-js#803, aws-durable-execution-sdk-java#604. Full 10-1..10-18 regression green everywhere.

Land this first — the companion handler PRs (python#615, js#800, java#602) reference these IDs.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@wangyb-A
wangyb-A temporarily deployed to ai-pr-review-runtime August 7, 2026 19:09 — with GitHub Actions Inactive
@wangyb-A
wangyb-A had a problem deploying to ai-pr-review-runtime August 7, 2026 19:09 — with GitHub Actions Error
@github-actions

This comment has been minimized.

@wangyb-A
wangyb-A force-pushed the plugin-hook-field-shape branch from cf46661 to 5a0dd6f Compare August 7, 2026 20:49
@wangyb-A
wangyb-A temporarily deployed to ai-pr-review-runtime August 7, 2026 20:49 — with GitHub Actions Inactive
@wangyb-A
wangyb-A temporarily deployed to ai-pr-review-runtime August 7, 2026 20:49 — with GitHub Actions Inactive
@github-actions

This comment has been minimized.

@wangyb-A
wangyb-A temporarily deployed to ai-pr-review-runtime August 7, 2026 21:56 — with GitHub Actions Inactive
@wangyb-A
wangyb-A temporarily deployed to ai-pr-review-runtime August 7, 2026 21:56 — with GitHub Actions Inactive
@github-actions

This comment has been minimized.

@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Claude AI review

This PR adds five additive, declarative plugin-suite requirement YAMLs (10-1910-23) probing the field shape of cross-SDK plugin hook info parameters. No runner code changes.

I reviewed each file against the engine that evaluates it (cloudwatch.py matcher, variables.py placeholder context, history.py, validate.py) and against sibling requirements for convention parity.

No actionable findings. Verifications that passed:

  • Match semantics: /…/ values map to re.search(pattern, str(actual).strip()), bare scalars to exact/numeric/bool equality, and a field must be present to match — so presence probes (/.*/), negative probes (count: 0), and value probes behave as intended. All regexes are valid Python ((?i) correctly at pattern start; ^\d{4}-, ^[1-9]\d*$, ^arn: coerce ints/bools via str()).
  • Placeholders: ${INPUT_1} comes from Variables; ${ID1}/${CTX_P} are bound by the history matcher, which runs before log validation, so all are resolved when ExpectedLogs substitution runs. No unbound references.
  • History/EventIds: wait (10-19) matches 10-16; retry (10-21) matches 1-11/10-13/10-15; single-step (10-20/10-22) matches 1-1; parent-only ContextStarted (10-23) matches 10-18. ExpectedResult shapes decode correctly (raw invocation result vs. serialized '"task-a"' operation result vs. [a-done, b-done] list).
  • Cardinality: exact counts and min_count thresholds are consistent with the described invocation/replay flows; after anchors all match ≥1 record (no typo-guard errors); presence-only probes correctly go red when a field is absent (true parity signal, no false-pass).
  • IDs/YAML: 10-1910-23 are unique stems (no 10-2 collision); values are valid plain scalars under the repo's yamllint config.

Residual test risk (inherent, not a PR defect):

  • Correctness of the assertions ultimately depends on live SDK behavior; CI only lints the YAML and cannot exercise these against a durable-execution backend, so an authoring error in a probe would surface only at live-run time. The author documents having validated all five live in us-west-2.
  • 10-23's min_count: 3 presence probe and the after ordering probes lean on service-side replay/timestamp behavior; the file appropriately asserts key presence (not value) where timing is nondeterministic, and CloudWatch's epoch-ms resolution treats sub-ms events as concurrent (pre-existing engine behavior).
  • 10-22's primary probe ANDs the full field set into one count: 1 assertion, so a red does not isolate which field is missing (unlike the decoupled probes in 10-19/10-21). This is a diagnostic-granularity trade-off, not a correctness issue.

Reviewed commit 93d741772685a016167e18a1b06668ea474c2fb1. Workflow run

@wangyb-A
wangyb-A temporarily deployed to ai-pr-review-runtime August 7, 2026 23:04 — with GitHub Actions Inactive
@wangyb-A
wangyb-A had a problem deploying to ai-pr-review-runtime August 7, 2026 23:04 — with GitHub Actions Error
@github-actions

This comment has been minimized.

Review follow-up: the replay-start matcher only probed
updatedOperationsCount, so a replay record missing requestId,
operationsCount, or executionStartTimestamp still passed. Add
decoupled count:2 presence probes for the three inherited fields
(mirroring the existing end-side block). updatedOperationsCount
deliberately keeps only the replay-specific nonzero probe: whether
an SDK surfaces an empty updated-operations collection before the
first invocation is an unasserted implementation detail.
@wangyb-A
wangyb-A temporarily deployed to ai-pr-review-runtime August 7, 2026 23:49 — with GitHub Actions Inactive
@wangyb-A
wangyb-A temporarily deployed to ai-pr-review-runtime August 7, 2026 23:49 — with GitHub Actions Inactive
@github-actions

This comment has been minimized.

Review follow-ups:
- error/executionError absence guards used /.+/ with count: 0, which
  an empty-string error value would slip past; use /.*/ so any present
  key (even empty) violates the guard (10-19, 10-20, 10-21).
- Prose still claimed input/result values are asserted BY VALUE after
  the GA descope; rewritten as 'dumped when exposed but unasserted --
  experimental' so the YAML contract matches the executable matchers
  (10-19, 10-20, 10-22).
- 10-22 description narrowed to delta-item shape + full-map MEMBERSHIP,
  matching what inFullMap actually checks.
@wangyb-A
wangyb-A temporarily deployed to ai-pr-review-runtime August 8, 2026 00:15 — with GitHub Actions Inactive
@wangyb-A
wangyb-A had a problem deploying to ai-pr-review-runtime August 8, 2026 00:15 — with GitHub Actions Error
Requirements are plain test descriptions: executionInput,
executionResult, and operation result no longer appear in the
canonical dump schemas, and all GA-scope / experimental design
commentary is removed. Assertions are unchanged.
@wangyb-A
wangyb-A had a problem deploying to ai-pr-review-runtime August 8, 2026 01:00 — with GitHub Actions Error
@wangyb-A
wangyb-A deployed to ai-pr-review-runtime August 8, 2026 01:00 — with GitHub Actions Active
Comment on lines +66 to +96
- match:
plugin: CONFPLUGIN
hook: operation-start
type: /(?i)context/
subType: /(?i)branch/
isReplay: false
count: 2
# Both branch functions run live at least once each (branch A in invocation
# 1, branch B in the replay invocation) with children-replay=false. At-least:
# some SDKs also re-run a COMPLETED childless branch function during replay,
# legitimately reporting false again.
- match:
plugin: CONFPLUGIN
hook: fn-start
subType: /(?i)branch/
isReplayingChildren: false
min_count: 2
# THE field-surface probe: every branch fn-start record carries the
# children-replay indicator key — including branch A's REPLAY re-run. The
# minimum of 3 (branch A live + branch A replay + branch B live) cannot be
# satisfied by the two live records alone, so the replayed re-run must also
# bear the key. The indicator's TRUE value depends on service-side
# checkpoint timing (read from the checkpointed context details) and proved
# nondeterministic across live runs, so values are dumped for observability
# but only key presence is asserted.
- match:
plugin: CONFPLUGIN
hook: fn-start
subType: /(?i)branch/
isReplayingChildren: /.*/
min_count: 3

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Codex AI review

High: These aggregate counts do not correlate records to either branch. Branch A's live and replay records can alone satisfy min_count: 2 when its replay value is legitimately false, allowing an incorrect value for branch B. Likewise, permitted extra reruns can satisfy min_count: 3 even when another fn-start omits the key, contradicting the “every record” requirement. Give branches stable names or bind their IDs and assert each branch separately. To verify key presence across an unbounded number of reruns, emit an explicit hasIsReplayingChildren flag and assert that no record reports false.

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Codex AI review

Found one high-severity false-positive in requirement 10-23. Review was static as required; runtime behavior was not exercised.

Reviewed commit cfcf58e24a6eb317fb9e3906cdfbd97fc835a667. Workflow run

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