Skip to content

Add typed workflow version-usage reads - #427

Draft
rmcdaniel wants to merge 3 commits into
mainfrom
agent/sdk-php/220-version-usage
Draft

rmcdaniel wants to merge 3 commits into
mainfrom
agent/sdk-php/220-version-usage

Conversation

@rmcdaniel

@rmcdaniel rmcdaniel commented Nov 14, 2026 •

Copy link
Copy Markdown
Member

Waterline's accepted version-usage consumer has no official PHP SDK read method. Add Client::workflowVersionUsage(workflowType, changeId, minVersion, state: null, limit: 50, cursor: null) and ClusterInfo::supportsWorkflowVersionUsage(), following the accepted v1 contract.

The method discovers the literal capability, then reads GET /workflow-version-usage through the existing control-auth, namespace, metrics and configured control-request timeout path. Both requests remain metadata-only, with no argument/result/external-payload hydration. Missing/false capability raises ServerCapabilityUnavailable; authentication, storage, validation, invalid-cursor and timeout errors stay errors. No raw HTTP or first-page fallback is added. Each request retains the existing timeout policy; this does not introduce a new aggregate deadline across discovery and the page read.

Immutable typed page/count/run models retain signed32-bit versions, all-state counts before filtering, explicit unknown reasons and the untouched opaque cursor. They validate the fixed v1 shape, exact query/namespace echoes, classification/minimum consistency, counts and page limits, byte-exact ascending run IDs and the first-page upper bound. The request cursor binds page validation: a terminal first page must include the complete selected cohort, every has_more page must fill its limit, and a continuation must advance its opaque cursor. A later terminal cursor page may be empty as live membership changes. The raw validated v1 response remains available as page.raw. The guide explains live traversal, preserving unknowns, and why this is not a fleet snapshot or branch-removal authorization.

Contract binding: Workflow c93cadad8a0104b6e7e5f38e022c3b06c26c86e6, schema SHA256 469e459868ec61951f0f7b5ecc99ce1daa34b16c07f2b95787812e54b03c0344; native source is merged Server1221 / c40707776ba0ae424985024d2ea341a6bdbe71c9. No published endpoint or SDK package containing this new API is claimed.

Local source validation on retained PHP8.4.27/vendor: 53 reader/model/metrics tests /490 assertions, six actual control-timeout cases /78 assertions (including the cURL-disabled native path), full PHPStan with zero errors, Composer strict manifest validation, and public-boundary/diff checks. The timeout controls use the existing loopback fixture; no Server/SDK Worker, package install or external runtime ran. The four new paging-integrity controls each fail with the previous e0fcb500 reader (four expected assertion failures, zero setup errors) and pass with bed228f1416ecd3ee140192652a3a847bced4c49; the valid later-empty control also passes. No dependency or installer/profile change is included.

Producer-contract review passed the initial source; the paging-integrity finding is corrected at current head bed228f1416ecd3ee140192652a3a847bced4c49. Independent Security source rebind passed, and current Sonar reports no open issues. Normal exact-head CI remains required; current analyse, PHP8.3 and stock framework lanes stop at the retired setup archive with HTTP404 before execution (run43607777704). The existing stock PHP8.3 setup dependency is currently held on central400/404 activation; no unactivated helper, override or stopped419/420 path is selected. Publication, installed consumer acceptance, the other shared220 implementations and final guide delivery remain unfinished; no release/runtime selection or shared220 closure is proposed here.

Implements the PHP client source slice of durable-workflow/.github#220.

@rmcdaniel rmcdaniel left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

HOLD on exact 311abe7f6a611d079f83c011362939e7781eb41e.

The typed response parser does not enforce the v1 keyset page-completeness/progress invariants. In WorkflowVersionUsagePage::fromArray(), lines 61-72 only require a positive partial page when has_more is true and never distinguish a first request from a cursor continuation. Client::workflowVersionUsage() also does not compare the returned cursor with the supplied cursor.

As a result, all three malformed responses are currently accepted:

  • a first terminal page (cursor === null, has_more === false) whose returned_count is smaller than the selected count (counts.total or counts[state]);
  • a page with has_more === true whose returned_count is smaller than the requested limit;
  • a continuation whose next_cursor equals the supplied cursor.

The first case can silently omit open runs from an apparently terminal rollout audit; the latter two violate strict keyset progress and can hide omissions or cause a consumer loop. The shared CLI consumer already enforces the corresponding first-page and full-continuation rules in PR #158.

Please validate with the request context after parsing: require a first terminal page to contain the complete selected cohort; require every has_more page to fill limit; and refuse an unchanged next cursor. Retain later terminal cursor pages as valid even when empty because live membership can change. Add causal controls for unfiltered and state-filtered first pages, short has_more pages, repeated cursors, and the valid later-empty case.

The capability gate, control credential/namespace binding, exact query echo, closed response fields/states/reasons, signed bounds, byte-exact ordering, payload non-hydration, and error propagation otherwise look sound at this head.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Bounded producer-contract source review PASS at 311abe7f6a611d079f83c011362939e7781eb41e, against accepted Workflow c93cadad8a0104b6e7e5f38e022c3b06c26c86e6 and schema SHA-256 469e459868ec61951f0f7b5ecc99ce1daa34b16c07f2b95787812e54b03c0344.

I read the client method, existing request/response pipeline, capability helper, all three immutable models, route metric registration, guide and regression cases. Discovery requires literal true; both requests use the existing control credential/namespace/timeout path and explicitly select no payload hydration. The collision-free route, exact query echoes, signed-32-bit versions, all-state count reconciliation, canonical open statuses, preserved unknown reasons, byte ordering and upper bound agree with v1. Cursors stay opaque and untouched; server/validation/storage/authentication/timeout failures remain errors. Default service namespace is explicitly default, matching the native echo contract. The guide accurately describes live paging and each request's existing timeout policy without inventing an aggregate deadline or removal authorization.

No source-contract parity finding. I did not execute SDK tests: the posted source/deadline results remain owner evidence. Current hosted qualification, independent Security review, any central CI activation dependency, normal source delivery, publication and installed consumer acceptance remain the SDK owner's gates. This review selects no endpoint/runtime or release and changes no existing stop.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Addressed the paging-integrity finding at exact bed228f1416ecd3ee140192652a3a847bced4c49 / tree a0f0851c49c20aebc894c2b637ba0523b4e3a5d5.

Client::workflowVersionUsage() now passes the actual request cursor into the typed page reader. It refuses incomplete terminal first pages against the selected all-state count, short has_more pages, and an unchanged next cursor. It still accepts a later terminal cursor page with zero members and nonzero all-state counts, preserving live-membership behavior. No cursor decoding or fallback was added.

The four new refusal controls fail against exact predecessor e0fcb500 Client/page source with four expected assertion failures and zero setup errors. On the candidate, all 53 reader/model/metrics tests /490 assertions pass, including the later-empty positive; full PHPStan, strict Composer validation, dependency-boundary and diff checks pass. The six existing real control-timeout cases /78 assertions remain scoped to the unchanged request path. Local source tests use fake metadata or the existing timeout fixture; no installed runtime or Worker acceptance is claimed.

Please rebind the bounded source review to this head. Current-head hosted checks are running separately. The central setup activation dependency, normal integration/publication and actual consumer acceptance remain open; no stopped path is resumed.

@sonarqubecloud

Copy link
Copy Markdown

@rmcdaniel rmcdaniel left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Security source rebind PASS on exact bed228f1416ecd3ee140192652a3a847bced4c49 / tree a0f0851c49c20aebc894c2b637ba0523b4e3a5d5, narrowly closing the paging-integrity HOLD from review 5905086312.

The three-file successor passes the actual request cursor into the typed page parser and enforces all three missing invariants:

  • a first terminal page must contain the complete selected all-state or filtered cohort;
  • every has_more page must fill the requested limit;
  • a continuation cannot return the same opaque cursor.

The checks preserve the required live behavior: a later terminal cursor page may be empty while all-state counts remain nonzero. No cursor decoding, restart fallback, filter relaxation or payload hydration was introduced. The four new causal controls cover unfiltered and filtered incomplete first pages, a short continuation, and a repeated cursor; the separate later-empty positive keeps the legitimate continuation boundary open. The signed bounds, exact query/namespace echo, byte ordering, upper bound, closed status/reason fields, capability gate, control credential and error propagation remain unchanged.

No further source finding in this bounded delta. Current-head hosted CI, central setup activation, normal integration/publication and installed endpoint acceptance remain separate required gates; this PASS selects no runtime, endpoint publication, release or stopped path.

@rmcdaniel

Copy link
Copy Markdown
Member Author

Bounded producer-contract rebind PASS at bed228f1416ecd3ee140192652a3a847bced4c49.

My prior review missed the request-bound completeness checks identified by Security. I read this three-file successor and the four causal negatives plus later-page positive. Client now supplies the actual opaque request cursor. A first terminal page must contain the entire selected cohort, every page advertising a continuation must fill its limit, and the next opaque cursor must differ from the incoming cursor. A later terminal page can still be empty with nonzero cohort counts, as required by the live keyset contract. No cursor decoding or rewritten filters are introduced; the signed-minimum fixture now represents a complete valid cohort.

The earlier auth, literal capability, metadata-only transport, counts/unknowns, exact identity and error-preservation review remains applicable. No further source-contract finding on this delta. I did not execute the SDK controls; fresh hosted qualification, Security review, activation dependencies, normal delivery and publication/installed acceptance remain the SDK owner's gates.

This branch has not been deployed

No deployments
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.

1 participant