feat(workflow-state): expose exact-object operability evidence - #605
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughWorkflow 상태 Durable Object에 ChangesWorkflow 상태 operability
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Scheduler
participant routeWorkflowStateCommand
participant NoemaWorkflowState
participant WorkflowStateRepository
participant DurableObjectStorage
Scheduler->>routeWorkflowStateCommand: read_operability(plan)
routeWorkflowStateCommand->>NoemaWorkflowState: execution-scoped command
NoemaWorkflowState->>WorkflowStateRepository: readState(plan)
WorkflowStateRepository-->>NoemaWorkflowState: retained authority or conflict
NoemaWorkflowState->>DurableObjectStorage: read sql.databaseSize
DurableObjectStorage-->>NoemaWorkflowState: database size or storage error
NoemaWorkflowState-->>Scheduler: database_size_bytes or failure
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
seonghobae
left a comment
There was a problem hiding this comment.
Current-head owner review: source/test boundary is narrow and preserves the existing execution-scoped NOEMA_WORKFLOW_STATE authority. The new operation returns only object-local SQLite byte count, projected transport drops caller-only fields, object substitution is checked before observation, and storage-metadata failure maps to the existing storage-unavailable class. One valid completion finding remains before Ready/merge: ADR-0013/OPERABILITY/CHANGELOG (and any canonical gap text discovered by a fresh search) still describe the protected Workflow / Task authority without this candidate read_operability evidence producer. Converge those docs as candidate/source-only truth while preserving ADR-0013 Proposed and the separate deployed transaction/restart/recovery/p95/release acceptance. Do not present unit/fake storage bytes as production storage-growth evidence.
seonghobae
left a comment
There was a problem hiding this comment.
Current-head review on 5193fcd2b18996f1c7bebcb2baa44600f96415c8: reviewed all five changed files against protected main@585b2f09c418e7d90e9e2729ee51f384e0602d9f, including the private command projection, exact-object identity check, SQLite metadata failure mapping, operability regression, ADR-0013, OPERABILITY and CHANGELOG. The earlier documentation/transport-semantics finding is repaired: the canonical admitted plan intentionally crosses the private Durable Object boundary for re-admission, caller-only properties outside the command schema are projected out, and the operability success result is only database_size_bytes. ADR-0013 remains Proposed and deployed transaction/restart/recovery, workload denominator, p95, PITR/rollback and immutable release remain separate #541 evidence. Fresh baseline search found no additional source-producer contradiction. No additional source/test/DDD/documentation finding found at this head; no inline review threads are open. This is COMMENT-only, not self-approval. Merge authority still requires this exact head's newly generated hosted gates to become terminal GREEN.
seonghobae
left a comment
There was a problem hiding this comment.
New valid current-head finding after deeper authority review: read_operability verifies the hashed Durable Object name but does not require the retained execution-plan authority/state before reading storage.sql.databaseSize. A caller can therefore route an arbitrary valid uninitialized execution/plan to its deterministic object and receive a byte count for an object that has never become Workflow / Task execution authority. That is too weak for the exact-object operational-evidence claim in #541. Test-only exact 31a1e599fb76a4598b86e58c54ce51ccaca5ab86 now requires an uninitialized exact object to fail with conflict, initializes the exact authority before positive/metadata-failure cases, and leaves foreign-object rejection intact. No source fix is included in this commit. This is COMMENT-only, not self-approval; wait for a real RED on this exact before the minimal causal source repair.
seonghobae
left a comment
There was a problem hiding this comment.
Current-head review finding: test-only 31a1e599fb76a4598b86e58c54ce51ccaca5ab86 correctly exposed that read_operability could previously read storage.sql.databaseSize after object-name validation without proving that the execution had a retained Noema Workflow / Task state + plan authority. That would let a caller obtain a plausible object-local metric from an uninitialized object, which is not an admissible operational receipt. Exact 7639856ee6d20a59a144944ed21f1abc273fd386 applies the minimal causal source repair: the operation first calls the existing repository readState(plan) authority path, so missing/malformed/mismatched retained authority fails closed before platform storage metadata is read; the returned success payload remains only { database_size_bytes } and no second state/observability store is introduced. Remaining repair finding before Ready: ADR-0013 still says this operation “does not read retained workflow state”; that sentence and the sequence/test prose must be made code-current as “validate/read current retained authority without returning its payload.” Current-head hosted gates are newly queued and predecessor results are not merge authority.
seonghobae
left a comment
There was a problem hiding this comment.
Current-head review on b4fd8abb77a69847655e04a03fa63a72ea6d56b4: reviewed the five-file delta against protected main@585b2f09c418e7d90e9e2729ee51f384e0602d9f, including the read_operability transport projection, retained Workflow / Task authority check before SQLite metadata access, failure classification, regression coverage, ADR-0013, OPERABILITY and CHANGELOG. The prior authority and documentation findings are repaired. The operation remains observation-only, returns only { database_size_bytes }, does not create a second state/metrics authority, and leaves deployed transaction/restart/recovery, representative storage-growth denominator, p95, PITR/rollback and immutable release as separate #541 evidence. ADR-0013 remains Proposed. Fresh inline-thread sweep is empty. Application CI 34542741791, reviewer-ci 34542741784, required Security Scan 34542741748, and patch-validator-image 34542741760 are all terminal SUCCESS on this exact head. No additional source/test/DDD/documentation finding found. COMMENT-only; not self-approval.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Problem
Protected Workflow / Task Execution already has one execution-scoped SQLite Durable Object authority, but its private command surface could not emit the exact-object storage observation required by #541 operational acceptance. Operators therefore lacked a bounded way to bind later storage-growth evidence to the same execution authority without inspecting a foreign/global metric source.
RED → causal repair lineage
Initial test-only
24a4d6a00131bf4075db75a14f97b394a8d8d53areached hosted application CI34535567948/ verify job103066254765: checkout, live-base validation, install and typecheck passed before release tests failed. It exposed a non-canonical fixture (read_onlyinstead ofpure | idempotent | side_effecting) and direct Durable Object cases proving protected source did not yet recognizeread_operability.ac373c5b45bfa9be6c34dc431f2dc06ffc7814a4repaired only that fixture.351666088865c94fb5a2c576e118e8563a80a707added the missing private operation to the existingNOEMA_WORKFLOW_STATEadapter;614174529e573a3e73d620806d979491d848b83dadded the throwing-storage-metadata regression. Historical exact758231307dd34716e2e2e05838f625330bed23f3later reached terminal SUCCESS on application CI34537148500, reviewer-ci34537148434, required Security Scan34537148591, and patch-validator-image34537148637. Those results belong only to that historical exact.Canonical documentation then converged on the same boundary:
635c2c65aa4c3627d64c861de753c130957a66d7updates OPERABILITY,55643e6a3d942fc460938d009d6e3b2d6e533760records the Unreleased CHANGELOG,3d762ecc9fdf013f3124bf0770a7eb8f616531c3removes moving-current Draft wording,469842ab387a99cb4a4cc35d1fb4d16e7f11282fmakes caller-only transport projection explicit,af87e8e23377fbe753c224b88d47b6723c88bd48aligns CHANGELOG wording, and5193fcd2b18996f1c7bebcb2baa44600f96415c8made ADR-0013 code-current for that revision.Fresh review then found a real authority defect: an operability read could validate the plan and hashed Durable Object name yet still read
storage.sql.databaseSizebefore proving that the object retained current Noema execution-plan/state authority. Test-only31a1e599fb76a4598b86e58c54ce51ccaca5ab86adds the missing regression: an uninitialized object must conflict before platform storage metadata is read. Its hosted generation was cancelled by latercancel-in-progressand is not claimed as terminal RED/GREEN evidence.Causal source
7639856ee6d20a59a144944ed21f1abc273fd386reuses the existingDurableWorkflowStateRepository.readState(plan)authority path before readingdatabaseSize. Missing/malformed/mismatched retained execution-plan or workflow-state authority therefore fails closed without creating a second state/observability store.b4fd8abb77a69847655e04a03fa63a72ea6d56b4updates ADR-0013 to the exact implementation: retained state is validated internally but never returned; success remains only{ database_size_bytes }.Boundary
This PR stays inside Noema Workflow / Task Execution + State / Checkpoint. It reuses the admitted execution plan and deterministic hashed object identity, projects caller-only fields out of the private command transport, rejects foreign or uninitialized authority before observation, and maps unavailable/throwing/negative/non-integer platform storage metadata to the existing storage-unavailable contract. It does not create a second state store, expose retained workflow/task state in the response, add a public HTTP route, alter graph publication, own Keyverse/CGC/provider/quarantine/outbound truth, or turn an observation into mutation/retry/recovery authority.
The object-local byte count is only a source-level evidence producer. Production Durable Object transaction/restart/recovery, representative exact-object storage-growth denominator, synchronous-path p95, PITR/equivalent rollback, deployment and immutable release remain #541 evidence. ADR-0013 remains
Proposed. Fresh Noema GitHub Release inventory is empty.Current exact-head acceptance
Current head is
b4fd8abb77a69847655e04a03fa63a72ea6d56b4against protected base585b2f09c418e7d90e9e2729ee51f384e0602d9f. Fresh current-head review after the authority and ADR repair found no additional source/test/DDD/documentation finding, and the latest thread sweep is empty. Application CI34542741791, reviewer-ci34542741784, required Security Scan34542741748, and patch-validator-image34542741760are fresh runs for this exact and are not replaced by predecessor GREEN. Keep this PR Draft until all four are terminal GREEN and a final exact-head/base/thread sweep remains clean.Related: #541, #584, #5, #36.
Summary by CodeRabbit
새 기능
database_size_bytes로 확인할 수 있는 관찰 전용 기능을 추가했습니다.문서