Repository navigation
Conversation
rmcdaniel
left a comment
There was a problem hiding this comment.
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) whosereturned_countis smaller than the selected count (counts.totalorcounts[state]); - a page with
has_more === truewhosereturned_countis smaller than the requestedlimit; - a continuation whose
next_cursorequals 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.
|
Bounded producer-contract source review PASS at 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 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. |
|
Addressed the paging-integrity finding at exact
The four new refusal controls fail against exact predecessor 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. |
|
rmcdaniel
left a comment
There was a problem hiding this comment.
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_morepage must fill the requestedlimit; - 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.
|
Bounded producer-contract rebind PASS at 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. 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. |



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)andClusterInfo::supportsWorkflowVersionUsage(), following the accepted v1 contract.The method discovers the literal capability, then reads
GET /workflow-version-usagethrough 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 raisesServerCapabilityUnavailable; 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_morepage 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 aspage.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 SHA256469e459868ec61951f0f7b5ecc99ce1daa34b16c07f2b95787812e54b03c0344; 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
e0fcb500reader (four expected assertion failures, zero setup errors) and pass withbed228f1416ecd3ee140192652a3a847bced4c49; 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; currentanalyse, 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.