feat: reads (read-only HTTP projections) + persist write-path hardening (9.13.0)#157
Merged
Conversation
…ions Part of 9.12.0 reads concept. ReadDefinition normalizes read config (null | string uri-shorthand | options array), validates keys (rejects action/method/unknown) and URI (trim, reject empty/slash-only/ placeholder), defaults status=200, available_events=null. Claude-Session: https://claude.ai/code/session_01Jsh4xaG4TL8GAfCcqZk9PD
- MachineRouter::register() parses reads (true|assoc), relaxes both content
early-returns for reads-only registrations, validates duplicate URIs and
GET machineId-bound collisions, registers GET /{machineId}/{uri} read routes.
- MachineController::handleSnapshot() — zero-write read path (restore + buildResponse,
no send/persist/lock); output split mirrors handleEndpoint; RestoringStateException to 404.
- buildResponse() now honors includeAvailableEvents (omits availableEvents key when false);
fixes endpoints' available_events too.
Claude-Session: https://claude.ai/code/session_01Jsh4xaG4TL8GAfCcqZk9PD
…-safe seq) - persist(): upsert only the dirty slice (history[max(0,watermark-1)..]) instead of the whole history, keeping the full in-memory baseline walk. Adds State::persistedEventCount watermark (captured at restore, advanced after persist). O(new+1) upsert — clears the 65,535-placeholder ceiling on long histories. Index 0 stays full; last row full snapshot. - send(): for transactional events, persist() now runs inside the transition's DB transaction so a persist failure rolls back the action's committed DB writes (no state/log desync). - State::setCurrentEventBehavior(): sequence_number from max(history)+1, not count — prune-safe. - restoreCurrentStateDefinition(): idMap miss throws RestoringStateException (foreign/wrong-class machineId) instead of a raw undefined-key error, so reads map it to 404 uniformly. Claude-Session: https://claude.ai/code/session_01Jsh4xaG4TL8GAfCcqZk9PD
…dening - ReadDefinitionTest: value forms, key/URI rejection, normalization. - ReadSnapshotTest: registration (true/assoc/reads-only/collision/duplicate), envelope, zero-write, output shaping (array + OutputBehavior), available_events omit, 404 (unknown + wrong-class). - IncrementalPersistTest: dirty-slice-only upsert + cross-restore context round-trip. - PersistAtomicityTest: persist failure rolls back the action's DB write. - SequenceNumberTest: prune-safe max()+1 sequencing. Claude-Session: https://claude.ai/code/session_01Jsh4xaG4TL8GAfCcqZk9PD
…links - New docs/laravel-integration/reads.md (concept, API, options, envelope, output, errors, optional migration). - persistence.md: Write-Path Hardening note (incremental dirty-slice upsert, prune-safe sequencing) + transactional persist-in-transaction clarification. - endpoints.md: commands-vs-queries cross-link to reads. - VitePress sidebar: add Reads under Laravel Integration. Claude-Session: https://claude.ai/code/session_01Jsh4xaG4TL8GAfCcqZk9PD
Ships with the release via plugin-dist per the skill ride-along rule. Claude-Session: https://claude.ai/code/session_01Jsh4xaG4TL8GAfCcqZk9PD
…est gaps) - restoreCurrentStateDefinition: multi-value (parallel) idMap miss now also throws RestoringStateException (uniform 404 for foreign machineIds). - MachineRouter collision check: also rejects machineId-bound GET forwarded endpoints. - Tests: large-history (>=5462 rows) persists without placeholder exhaustion; atomicity test now also asserts not-wedged recovery via trigger injection; archived-machine read returns 200; endpoint-side available_events=false omits the key. Claude-Session: https://claude.ai/code/session_01Jsh4xaG4TL8GAfCcqZk9PD
…scale Restoring 5500 rows pushed the parallel suite near its 300s budget. Replace with a 600-row history that asserts the persist upsert stays bounded (<50 rows) regardless of history size — a stronger, portable proof the placeholder ceiling cannot be reached. The literal >=5462-row MySQL-ceiling regression belongs in LocalQA (MySQL semantics). Claude-Session: https://claude.ai/code/session_01Jsh4xaG4TL8GAfCcqZk9PD
CI's Rector check (PHP 8.5) flags these as RemoveNullNamedArgOnNullDefaultParamRector — the ChildMachineCompletionJob constructor param defaults to null, so the explicit null named args are no-ops. Pre-existing lines surfaced by the cache-fresh CI run; removing them is behavior-preserving (344 delegation/job tests still pass). Claude-Session: https://claude.ai/code/session_01Jsh4xaG4TL8GAfCcqZk9PD
…older) The 'empty/`/`/placeholder' shorthand read like a typo with stray slashes. Claude-Session: https://claude.ai/code/session_01Jsh4xaG4TL8GAfCcqZk9PD
…output discriminator Addresses review feedback (tkaratug): the output-split discriminator was duplicated in three places (handleSnapshot, handleEndpoint, forwarded-response path). Extract to one private helper so all three call sites share it and a future output-form change can't drift across copies. Behavior-identical (phpstan 0, output/endpoint/forwarded tests pass). Claude-Session: https://claude.ai/code/session_01Jsh4xaG4TL8GAfCcqZk9PD
…) + archived-read caveat Follow-ups from final PR review: - Test the multi-value/parallel idMap-miss path: reading a parallel machine's id under a different class hits findCommonParallelAncestor's RestoringStateException → 404 (was untested). - Add tests/LocalQA/IncrementalPersistCeilingQATest.php: the literal >=5462-row MySQL placeholder-ceiling regression (MySQL-only; unit suite proves the bound portably). - Make the zero-write guarantee's archived-machine exception explicit in reads.md and SKILL.md. Claude-Session: https://claude.ai/code/session_01Jsh4xaG4TL8GAfCcqZk9PD
…t docblock Note that only restoreStateFromRootEventId() sets the watermark; a State built another way (fresh machine, or scenario/job re-hydration bypassing restore) keeps it at 0, so that instance's next persist() writes the full history — correct, just not incremental. Addresses review feedback; comment-only, no behavior change. Claude-Session: https://claude.ai/code/session_01Jsh4xaG4TL8GAfCcqZk9PD
…vel 13 support) Local tags were stale when the version was first chosen; the remote already had 9.12.0 (Laravel 13, PR #154). Rename the spec file, title, task plan, and tp marker to 9.13.0. Claude-Session: https://claude.ai/code/session_01Jsh4xaG4TL8GAfCcqZk9PD
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
1 participant
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.
Summary
Adds
reads— declarative, zero-write HTTP projections of a machine's current state — and hardens the persistence write path.readsare the query side of the command/query split:endpoints:send events and write history;reads:only look.Motivation
A frontend polling a machine's status was modelled as a targetless event. Every poll ran the full
send()→persist()pipeline and appended ~4 rows tomachine_events. A long-open screen bloated one machine's history to ~5,458 rows;persist()re-upserted the entire history each time, hit MySQL's 65,535-placeholder ceiling, threwQueryException, and wedged the machine. Reading state is a query, not an event.What's in this PR
reads(zero-write by construction) —MachineRouter::register(['reads' => true | [...]])→ GET, machineId-bound routes;MachineController::handleSnapshot()restores read-only and returns the standard envelope (nosend/persist/lock);ReadDefinitionvalue object; output shaping reuses the endpointoutputparsing via a sharedsplitOutputDefinition()helper;buildResponse()now honoursincludeAvailableEvents(fixes endpoints too).Persist write-path hardening — incremental persist (dirty-slice upsert,
O(new+1), clears the placeholder ceiling); atomicity (persist inside the transition's transaction for transactional events — no action-committed-but-events-unwritten desync); prune-safemax()+1sequencing; restore-pathidMap-miss →RestoringStateException(clean 404).Docs & skill — new
docs/laravel-integration/reads.md, persistence write-path-hardening notes, endpoints↔reads cross-links, and the agent-skill updates that ship viaplugin-dist.Testing
Full gate green: phpstan 0, ~2,670 tests pass, type coverage 100%, across the PHP 8.3–8.5 × Laravel 11–13 × prefer-lowest/stable matrix. New coverage:
ReadDefinition, read registration (forms/collision/duplicate/reads-only), snapshot envelope + zero-write + archived restore, output routing,available_eventsomission (reads + endpoints), 404 (unknown + wrong-class, single- and multi-value), incremental dirty-slice + bounded-upsert-at-scale + cross-restore round-trip, atomicity rollback and not-wedged recovery, prune-safe sequencing. The literal ≥5,462-row MySQL ceiling regression has its own LocalQA test (tests/LocalQA/IncrementalPersistCeilingQATest.php) — verified passing on real MySQL.Review & audit
Spec converged over 5 adversarial review rounds (2 consecutive clean); implementation verified by a post-implementation audit (2 clean rounds). Both human reviewers' feedback addressed.
Notes
Purely additive — absent
reads, no new routes; persist hardening is backward-compatible.9.12.0is the Laravel 13 release; this ships as9.13.0.https://claude.ai/code/session_01Jsh4xaG4TL8GAfCcqZk9PD