Skip to content

github-devloop implementation for #42 - #43

Merged
chronoai-fkst[bot] merged 5 commits into
qa-tools-fkst-stagingfrom
devloop/issue/ChronoAIProject/talos/42/ready-github-devloop-issue-ChronoAIProject-talos-42-intake-0095365767-3457801545
Sep 9, 2026
Merged

github-devloop implementation for #42#43
chronoai-fkst[bot] merged 5 commits into
qa-tools-fkst-stagingfrom
devloop/issue/ChronoAIProject/talos/42/ready-github-devloop-issue-ChronoAIProject-talos-42-intake-0095365767-3457801545

Conversation

@chronoai-fkst

@chronoai-fkst chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

github-devloop implementation PR for issue #42

@chronoai-fkst

chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop PR child open

@chronoai-fkst

chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop PR is ready for review

@chronoai-fkst

chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop PR is ready for review

@chronoai-fkst chronoai-fkst Bot mentioned this pull request Sep 9, 2026
@chronoai-fkst

chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop PR review decision: reject
Verdicts: teleology=reject parsimony=reject fidelity=reject natural-ownership=reject proportional-containment=reject
Blocking gap: missing legacy action migration

teleology:
Established security and schema-evolution practice requires unsafe legacy authority records to be atomically migrated, quarantined, or rejected before new fenced execution begins; a read-compatible but mutation-incompatible branch is not a valid transition. The fidelity peer moves me from my narrower carrier-validation objection to this root failure. Mongo still returns legacy pendingActions as executable work (control-plane/src/storage/mongo-repository.ts:557, control-plane/src/storage/mongo-repository.ts:566), but dispatch CAS operates only on embedded, versioned sessionActions (control-plane/src/storage/mongo-repository.ts:583, control-plane/src/storage/mongo-repository.ts:594, control-plane/src/storage/mongo-repository.ts:596). Worse, recovery deliberately requeues those unsafe legacy actions (control-plane/src/services/task-service.ts:589), and the retained contract requires them to remain pending after restart (control-plane/src/storage/repository-contract.test.ts:795, control-plane/src/storage/repository-contract.test.ts:799). This directly contradicts the frozen policy forbidding silent legacy re-execution and requiring an atomic, bounded, restart-safe quarantine or verified drain.

parsimony:
Established security practice requires untrusted persisted credentials to pass total runtime validation and fail closed with the uniform authorization response. The root claim remains false: Mongo reconstructs Task through an unchecked assertion (control-plane/src/storage/mongo-repository.ts:901), while isValidDispatchBinding() verifies only binding?.schemaVersion before dereferencing binding.dispatchId.length, binding.workerId.length, and binding.machineId.length (control-plane/src/services/session-service.ts:261). Thus a persisted versioned binding with dispatchId: null throws before unauthorized('unauthorized'); the HTTP boundary maps that non-TalosError to 500 internal_error (control-plane/src/http/server.ts:113, control-plane/src/http/server.ts:128), directly violating the frozen requirement that malformed bindings return the common 401. Teleology’s dual-carrier bypass and the fidelity/proportional-containment legacy-transition findings identify additional defects, while natural-ownership’s proxy-test finding concerns verification; none supplies evidence that malformed persisted data is impossible or defeats this independently goal-blocking path.

fidelity:
Established schema-evolution practice requires unsafe legacy work to be atomically migrated, quarantined, or rejected before the new mutation path becomes authoritative; no peer proves that practice inapplicable. The immutable dispatch-binding design is sound for new records, but the root merge-readiness claim still fails the frozen legacy policy: getPendingSessionAction() continues returning legacy pendingActions records (control-plane/src/storage/mongo-repository.ts:557, control-plane/src/storage/mongo-repository.ts:566), while dispatch CAS requires versioned embedded sessionActions (control-plane/src/storage/mongo-repository.ts:583, control-plane/src/storage/mongo-repository.ts:596) and requeue/finalization mutate only that embedded array (control-plane/src/storage/mongo-repository.ts:626, control-plane/src/storage/mongo-repository.ts:662). The legacy decoder merely asserts the old document as PendingSessionAction (control-plane/src/storage/mongo-repository.ts:921), and initialization creates indexes but performs no bounded migration or verified drain (control-plane/src/storage/mongo-repository.ts:81). Teleology’s carrier-validation claim, parsimony’s malformed-binding claim, and natural-ownership’s obsolete-contract claim identify additional defects; none defeats the evidence that existing legacy in-flight records remain visible yet unresolvable.

natural-ownership:
Established schema-evolution practice requires a total, restart-safe transition: unsafe legacy authority records must be atomically migrated, quarantined, or rejected before the new ownership model becomes authoritative. The peer attack defeats my narrower test-proxy focus because the production design itself violates that rule. getPendingSessionAction() still exposes legacy pendingActions as actionable work (control-plane/src/storage/mongo-repository.ts:557, control-plane/src/storage/mongo-repository.ts:566), but takePendingSessionAction() transitions only versioned actions embedded in the owning Task’s sessionActions array (control-plane/src/storage/mongo-repository.ts:583, control-plane/src/storage/mongo-repository.ts:596). The legacy decoder merely casts persisted data and supplies no safe generation or binding (control-plane/src/storage/mongo-repository.ts:921), while the existing restart contract preserves the legacy record as pending (control-plane/src/storage/repository-contract.test.ts:795, control-plane/src/storage/repository-contract.test.ts:799). Thus an unsafe pre-upgrade action remains visible yet cannot be dispatched, quarantined, or terminalized through the new natural-owner CAS. That directly fails the frozen requirement for atomic bounded quarantine or verified drain-before-upgrade, so the root merge-readiness claim fails regardless of the otherwise sound Task-document ownership placement.

proportional-containment:
Established schema-evolution practice requires unsafe legacy in-flight records to be atomically migrated, quarantined, or rejected under a verified drain policy; no evidence proves that practice is inapplicable. The root merge-readiness claim therefore fails independently of leaf-level test concerns. The parsimony peer’s claim that the legacy fallback is merely advisory is defeated by the frozen “Legacy Mongo policy,” which explicitly requires a bounded, restart-safe transition. Production code still returns legacy pendingActions as live work (control-plane/src/storage/mongo-repository.ts:566), but takePendingSessionAction() mutates only versioned embedded sessionActions (control-plane/src/storage/mongo-repository.ts:583), and finalizeSessionAction() likewise updates only the Task document’s embedded array (control-plane/src/storage/mongo-repository.ts:662). Legacy decoding merely casts the old record into PendingSessionAction without establishing schema version or dispatch generation (control-plane/src/storage/mongo-repository.ts:921). Even existing claim recovery calls finalizeSessionAction() or requeueSessionAction() for legacy records (control-plane/src/services/task-service.ts:572, control-plane/src/services/task-service.ts:589), but those repository methods no longer mutate pending_actions. Thus an upgrade can leave an unsafe action visible indefinitely while neither dispatching nor quarantining it; fidelity corroborates this blocker, while the other peers identify additional failures that do not displace it.

⟦AI:FKST⟧

@chronoai-fkst

chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop merge gate failed: missing legacy action migration
Reproduce locally with scripts/run.sh test from the repository root.

@chronoai-fkst

chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop fix pushed for re-review

Previous reviewed head: 2a5419f
New head: f01cc1e

@chronoai-fkst

chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop PR is ready for review

@chronoai-fkst

chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop PR review decision: reject
Verdicts: teleology=reject parsimony=reject fidelity=comment natural-ownership=reject proportional-containment=approve
Blocking gap: missing interrupted migration proof

teleology:
The production half of the named gap is closed: initialization creates a migration index and runs quarantine before returning (control-plane/src/storage/mongo-repository.ts:115, control-plane/src/storage/mongo-repository.ts:127); legacy in-flight actions are no longer returned as executable (control-plane/src/storage/mongo-repository.ts:573); and each bounded batch conditionally terminalizes only still-unsafe records while preserving completed records (control-plane/src/storage/mongo-repository.ts:724). However, the frozen acceptance requirement explicitly demands interrupted/restarted migration behavior. The new test completes both batches before closing (control-plane/src/storage/repository-contract.test.ts:2532, control-plane/src/storage/repository-contract.test.ts:2534) and restarts only after confirming zero in-flight records (control-plane/src/storage/repository-contract.test.ts:2535, control-plane/src/storage/repository-contract.test.ts:2550). No failure occurs between batches or during bulkWrite. Calling this test “restart-safe” therefore evidences narrative-over-verification and proxy-over-truth on the safety property for which the migration exists. Add fault injection after a partially applied batch, restart, and prove all remaining records converge without changing prior terminal winners.

parsimony:
ESSENCE: Legacy in-flight actions lack immutable dispatch authority, so upgrade must deterministically terminalize them without losing owner-visible results or permitting worker retries. Established schema-evolution practice is an idempotent, conditional per-record transition in bounded batches, proven resumable by fault injection and restart. IDEAL: quarantine each legacy pending_actions record with a stable terminal result, interrupt between batches, restart, and verify remaining records migrate while completed records are unchanged. SIX SMELLS: Magic numbers—LEGACY_ACTION_MIGRATION_BATCH_SIZE = 100 is named and consistent with local maintenance sizing, so acceptable. Proxy-over-truth—the retired pending_actions collection is a valid legacy discriminator. Symptom branches—the single migration loop adds no authorization branches. Narrative-over-verification—the test named “bounded restart-safe batches” lets initialize() finish every batch before closing, then merely restarts an already-complete migration (control-plane/src/storage/repository-contract.test.ts:2489, control-plane/src/storage/repository-contract.test.ts:2532, control-plane/src/storage/repository-contract.test.ts:2549). Missing-inevitability—the conditional bulk updates appear resumable (control-plane/src/storage/mongo-repository.ts:724), but no test interrupts them, despite the frozen acceptance requirement for “interrupted/restarted bounded migration behavior.” Skipped-purpose—quarantine results remain readable and worker authority is not synthesized. Thus the prior migration gap is implemented but not fully closed: the blocking narrative-over-verification smell is the absent required interruption proof.

fidelity:
The previously named gap, “missing legacy action migration,” is closed. The fix follows established restartable schema-migration practice: startup waits for quarantineLegacySessionActions(), selects at most 100 active legacy records per batch, and atomically changes each only while its state remains pending or dispatched (control-plane/src/storage/mongo-repository.ts:115, control-plane/src/storage/mongo-repository.ts:127, control-plane/src/storage/mongo-repository.ts:724). Completed records are excluded on subsequent runs, while quarantined results remain readable through the legacy result decoder (control-plane/src/storage/mongo-repository.ts:706). The test verifies two bounded batches, elimination of active legacy records, stable recovery results, and idempotence after restart (control-plane/src/storage/repository-contract.test.ts:2489, control-plane/src/storage/repository-contract.test.ts:2534, control-plane/src/storage/repository-contract.test.ts:2549). Advisory narrative-over-verification smell: despite its “restart-safe” title, the test restarts only after all 101 records migrate; it does not interrupt between batches. A fault-injected restart test would verify that premise directly, but the production state predicate makes the current fix good-enough and non-blocking.

natural-ownership:
Established schema-evolution practice requires fault-tested restartability for destructive authority migrations. The production fix is naturally owned: MongoRepository.initialize() invokes quarantineLegacySessionActions() (control-plane/src/storage/mongo-repository.ts:127), which processes indexed batches and uses state-guarded per-record updates (control-plane/src/storage/mongo-repository.ts:724). However, the named gap is not fully closed because of the evidenced narrative-over-verification smell: the test titled "quarantines legacy in-flight actions in bounded restart-safe batches" runs initialize() through both batches to completion (control-plane/src/storage/repository-contract.test.ts:2489, control-plane/src/storage/repository-contract.test.ts:2532), then reconnects only after migration has finished (control-plane/src/storage/repository-contract.test.ts:2549). It never interrupts between batches, during bulkWrite(), or after partial completion, despite the frozen acceptance requirement for interrupted/restarted migration behavior. For this P0 authority transition, successful rerun after completion is not proof of recovery from interruption.

proportional-containment:
The named gap, “missing legacy action migration,” is closed. MongoRepository.initialize() creates a targeted partial index and invokes repository-owned quarantine before service startup (control-plane/src/storage/mongo-repository.ts:89, control-plane/src/storage/mongo-repository.ts:115, control-plane/src/storage/mongo-repository.ts:127). The migration selects only legacy pending/dispatched records in named 100-record batches and atomically writes a stable terminal recovery result plus timestamp; repeated or interrupted execution is idempotent because completed records leave the selection set (control-plane/src/storage/mongo-repository.ts:724). The former live-work fallback is removed, so getPendingSessionAction() now returns only Task-owned versioned actions (control-plane/src/storage/mongo-repository.ts:573), while quarantined and previously completed legacy results remain readable (control-plane/src/storage/mongo-repository.ts:706). The regression test verifies two bounded batches, complete removal of live legacy work, stable result readability, preservation of an already-terminal record, and unchanged state after repository restart (control-plane/src/storage/repository-contract.test.ts:2487). Against the six smells: no problematic magic number, proxy-over-truth, symptom branch, narrative-only claim, missing inevitability, or skipped purpose is evidenced. The intervention is proportionate in scope, authority, and duration: it is confined to the Mongo storage owner and unsafe legacy states rather than generalized into higher layers.

⟦AI:FKST⟧

@chronoai-fkst

chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop fix-loop reflection: continue

Reason:
Established restart-safe, idempotent schema-migration practice governs this P0 authority transition; the rounds have converged from a missing legacy quarantine to a bounded verification gap, and fault-injected interruption/restart proof is explicitly required by the frozen acceptance bounds rather than a reviewer-driven specification expansion.

@chronoai-fkst

chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop timeout redrive attempt: fixing 2

⟦AI:FKST⟧

@chronoai-fkst

chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop fix pushed for re-review

Previous reviewed head: f01cc1e
New head: c44cb79

@chronoai-fkst

chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop PR is ready for review

@chronoai-fkst

chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop PR review decision: approve
Verdicts: teleology=approve parsimony=approve fidelity=approve natural-ownership=approve proportional-containment=approve

teleology:
Established practice for authority-schema upgrades is bounded, idempotent migration with conditional writes and fault-injected restart proof. ESSENCE: unsafe legacy actions must become permanently non-executable while preserving stable owner-readable terminal results across interruption. IDEAL: migrate indexed batches with state-guarded updates, crash after a committed subset, reconstruct the repository, and prove remaining records converge without rewriting winners. Six-smell comparison: magic numbers—LEGACY_ACTION_MIGRATION_BATCH_SIZE names the bound; proxy-over-truth—the test verifies persisted live-record counts and results, not merely calls; symptom branches—the latest delta adds no production workaround; narrative-over-verification—the injected failure occurs after the first bulkWrite() commits, leaving exactly one live record (control-plane/src/storage/repository-contract.test.ts:2535, control-plane/src/storage/repository-contract.test.ts:2542, control-plane/src/storage/repository-contract.test.ts:2544); missing-inevitability—the reconstructed repository processes that remainder and reaches zero while preserving the earlier winner timestamp (control-plane/src/storage/repository-contract.test.ts:2557, control-plane/src/storage/repository-contract.test.ts:2561, control-plane/src/storage/repository-contract.test.ts:2569), matching the migration’s conditional selection and updates (control-plane/src/storage/mongo-repository.ts:724); skipped-purpose—the quarantine runs before initialization returns and completed legacy results remain readable (control-plane/src/storage/mongo-repository.ts:127, control-plane/src/storage/mongo-repository.ts:706). The sole unsettled prior gap, interrupted migration proof, is now directly closed.

parsimony:
ESSENCE: Legacy in-flight actions lack immutable dispatch authority, so migration must quarantine them deterministically while preserving readable terminal results and denying renewed execution. Established best practice is bounded, idempotent, state-guarded migration with fault-injected restart verification. IDEAL: execute one real batch, interrupt startup, confirm partially migrated durable state, reconnect, and prove convergence without altering existing winners. Six-smell comparison: magic numbers—100 and 101 deliberately exercise the named production batch boundary, not unexplained policy; proxy-over-truth—the command monitor measures batch size, while actual Mongo state and repository reads verify outcomes (control-plane/src/storage/repository-contract.test.ts:2542, control-plane/src/storage/repository-contract.test.ts:2544, control-plane/src/storage/repository-contract.test.ts:2549); symptom branches—the change adds only test fault injection, no production branch; narrative-over-verification—the prior gap is closed because the real bulkWrite succeeds and then startup fails, leaving one unsafe record before a fresh repository completes migration (control-plane/src/storage/repository-contract.test.ts:2535, control-plane/src/storage/repository-contract.test.ts:2545, control-plane/src/storage/repository-contract.test.ts:2558); missing-inevitability—restart removes the remainder and preserves the first batch’s terminal timestamp (control-plane/src/storage/repository-contract.test.ts:2561, control-plane/src/storage/repository-contract.test.ts:2563, control-plane/src/storage/repository-contract.test.ts:2569); skipped-purpose—the already-terminal record is preserved and migrated results remain owner-readable. The previously named blocker is therefore resolved cleanly.

fidelity:
ESSENCE: Unsafe legacy actions must become non-executable while preserving stable owner-visible terminal results across interrupted upgrades. Established practice is bounded, idempotent schema migration using conditional per-record transitions and restart verification. IDEAL: fault the migration after durable partial progress, restart it, and prove remaining records converge without rewriting completed winners. SIX SMELLS: Magic numbers—the batch size is named and operationally tested, not unexplained (control-plane/src/storage/mongo-repository.ts:31, control-plane/src/storage/repository-contract.test.ts:2544). Proxy-over-truth—the new test observes actual Mongo writes and persisted action states, directly measuring migration progress rather than a surrogate (control-plane/src/storage/repository-contract.test.ts:2495, control-plane/src/storage/repository-contract.test.ts:2545). Symptom branches—none added; the production loop remains one state-driven migration path. Narrative-over-verification—the prior gap is closed: the test executes the real bulkWrite(), injects failure after durable partial migration, and requires initialize() to fail (control-plane/src/storage/repository-contract.test.ts:2535, control-plane/src/storage/repository-contract.test.ts:2542). Missing-inevitability—restart processes the remaining record and proves zero unsafe records remain, while the source transition condition permits only pending/dispatched records to change (control-plane/src/storage/repository-contract.test.ts:2557, control-plane/src/storage/repository-contract.test.ts:2569, control-plane/src/storage/mongo-repository.ts:724). Skipped-purpose—the test verifies quarantined results remain readable and an existing winner’s timestamp remains unchanged (control-plane/src/storage/repository-contract.test.ts:2548, control-plane/src/storage/repository-contract.test.ts:2569). The only unsettled prior blocker is therefore directly resolved without

natural-ownership:
Established practice for restart-safe schema evolution is repository-owned, bounded, idempotent migration with conditional writes and fault-injected recovery proof. ESSENCE: unsafe legacy actions must become terminal without regaining worker authority, and interruption must not lose progress or alter prior winners. IDEAL: fail after a committed batch, restart through a fresh repository/client, migrate the remainder, and verify completed records remain immutable. The current head does exactly that at the natural owner: production startup invokes quarantineLegacySessionActions(), whose state-guarded updates make completed records leave the migration set (control-plane/src/storage/mongo-repository.ts:127, control-plane/src/storage/mongo-repository.ts:724). The test now wraps the real Mongo bulkWrite, throws after the first committed 100-record batch, verifies one live record remains and migrated results are readable, reconnects with a fresh MongoRepository, then proves the remaining record converges while the original completedAt is unchanged (control-plane/src/storage/repository-contract.test.ts:2535, control-plane/src/storage/repository-contract.test.ts:2542, control-plane/src/storage/repository-contract.test.ts:2557, control-plane/src/storage/repository-contract.test.ts:2561). Six smells: the 100 bound is named and directly exercised; no proxy substitutes for persisted Mongo state; no symptom authorization branch is added; narrative-over-verification is closed by actual interruption; conditional state removal establishes inevitability on rerun; and owner-visible recovery plus terminal immutability preserve the purpose.

proportional-containment:
ESSENCE: Established restart-safe schema-migration practice governs: legacy authority-bearing records require bounded, idempotent, conditional transitions whose recovery is proven under interruption; novelty belongs only where that practice demonstrably fails. IDEAL: the Mongo repository atomically quarantines unsafe records in batches, survives an unknown write outcome, resumes on startup, preserves prior terminal winners, and exposes no worker authority. SIX SMELLS: magic numbers—LEGACY_ACTION_MIGRATION_BATCH_SIZE is named and bounded; proxy-over-truth—the test observes actual pending_actions writes and persisted states, not an indirect counter; symptom branches—the fix adds no production branch; narrative-over-verification—the new head now executes bulkWrite, injects failure before acknowledgement, confirms one record remains, restarts a fresh client/repository, and verifies completion (control-plane/src/storage/repository-contract.test.ts:2535, control-plane/src/storage/repository-contract.test.ts:2542, control-plane/src/storage/repository-contract.test.ts:2557); missing-inevitability—the production loop selects remaining unsafe records and conditionally updates each by current state, so restart converges without rewriting completed winners (control-plane/src/storage/mongo-repository.ts:724, control-plane/src/storage/mongo-repository.ts:734); skipped-purpose—the test verifies the first quarantine winner remains unchanged and the remaining action becomes readable as quarantined (control-plane/src/storage/repository-contract.test.ts:2566, control-plane/src/storage/repository-contract.test.ts:2569). The intervention is proportionately contained in the Mongo repository-specific contract and closes the previously recorded gap without hoisting migration policy into broader service layers.

⟦AI:FKST⟧

@chronoai-fkst

chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop merge gate failed: own-ci-red
Reproduce locally with scripts/run.sh test from the repository root.

@chronoai-fkst

chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop timeout redrive attempt: fixing 6

⟦AI:FKST⟧

@chronoai-fkst

chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop merge gate failed: own-ci-red
Reproduce locally with scripts/run.sh test from the repository root.

@chronoai-fkst

chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop fix pushed for re-review

Previous reviewed head: c44cb79
New head: 6287b65

@chronoai-fkst

chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop PR is ready for review

@chronoai-fkst

chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop PR review decision: reject
Verdicts: teleology=approve parsimony=comment fidelity=reject natural-ownership=reject proportional-containment=comment
Blocking gap: browser execution proxy assertion

teleology:
ESSENCE: Established distributed-systems practice is operation-scoped immutable credentials, monotonic fencing, linearizable first-writer-wins CAS, and bounded idempotent restart-safe migration. The purpose is to make terminal action authority survive mutable Task lease changes without accepting stale results or losing the terminal winner. IDEAL: keep each action and its server-authored ActionDispatchBinding in one authoritative Task document; atomically dispatch and terminalize against the exact generation and current lease; authenticate before payload validation; preserve the winning binding through teardown; quarantine unsafe legacy work before startup. SIX SMELLS: magic numbers—the new two-second test lease and 50ms margin are test-local and derived from the expiry being exercised, not hidden authorization policy (control-plane/src/storage/repository-contract.test.ts:1712, control-plane/src/storage/repository-contract.test.ts:1724); proxy-over-truth—the contracts inspect persisted bindings/results and actual interrupted Mongo migration state (control-plane/src/storage/repository-contract.test.ts:2591); symptom branches—the guarded worker path and unguarded teardown path represent genuinely different authorities, while both atomically preserve the action’s binding (control-plane/src/storage/mongo-repository.ts:654, control-plane/src/storage/mongo-repository.ts:703); narrative-over-verification—the HTTP test now correctly proves that a bound existing winner returns 409 action_already_completed before malformed-result validation (control-plane/src/http/session-routes.integration.test.ts:330); missing-inevitability—the CAS matches binding, claim generation, current lease, state, and expiry (control-plane/src/storage/mongo-repository.ts:705); skipped-purpose—the final delta fixes teardown terminalization to copy the immutable stored binding rather than trust a caller-supplied result (control-plane/src/storage/mongo-repository.ts:681)

parsimony:
ESSENCE: Established distributed-systems practice binds retries to immutable dispatch credentials and linearizable CAS; established test practice avoids wall-clock sleeps in favor of deterministic synchronization. The previously approved design remains settled. IDEAL: retain the final delta’s repository-owned teardown CAS, which preserves the stored binding atomically, while making the reclaim test advance authoritative database time deterministically. SIX SMELLS: magic numbers/proxy-over-truth—the new test waits for a two-second real lease plus an unexplained 50 ms cushion (control-plane/src/storage/repository-contract.test.ts:1712, control-plane/src/storage/repository-contract.test.ts:1726), adding latency and scheduler-dependent flakiness rather than directly establishing expiry; replace this with an explicit expired persisted lease or controllable Mongo-time boundary. Symptom branches—the guarded/unguarded split is justified by worker versus teardown authority, and the pipeline correctly derives bound versus unbound completion from the authoritative action (control-plane/src/storage/mongo-repository.ts:654, control-plane/src/storage/mongo-repository.ts:681). Narrative-over-verification and missing-inevitability—the updated HTTP assertion now directly verifies the required winner-before-payload-validation 409 (control-plane/src/http/session-routes.integration.test.ts:330). Skipped-purpose—none; immutable winner retention remains intact. This is advisory test ugliness, not a stated-requirement failure.

fidelity:
ESSENCE: This is a linearizable authorization and idempotency problem: terminal retries must be decided from immutable dispatch authority and durable winner state, not mutable Task state or execution surrogates. Established practice is atomic compare-and-set fencing plus tests that observe authoritative persisted outcomes. IDEAL: Bind dispatch and terminalization atomically, preserve the winning binding, and verify stale-denial, winner immutability, and redispatch behavior directly—without claiming Browser exactly-once. SIX SMELLS: The final change correctly preserves stored bindings during teardown and directly asserts terminal outcomes, but it also edits and retains browserExecutions: the counter increments merely when takePendingSessionAction() returns and is then asserted as 1 (control-plane/src/storage/repository-contract.test.ts:2241, control-plane/src/storage/repository-contract.test.ts:2243, control-plane/src/storage/repository-contract.test.ts:2273; repeated at control-plane/src/storage/repository-contract.test.ts:2289, control-plane/src/storage/repository-contract.test.ts:2291, control-plane/src/storage/repository-contract.test.ts:2321). That measures repository dispatch, not Browser execution, so it is evidenced proxy-over-truth and narrative-over-verification, and it fails the explicit requirement to remove or replace PR #25’s fixed-by-construction browserExecutions assertions. No blocking magic-number, symptom-branch, missing-inevitability, or skipped-purpose smell is otherwise evidenced.

natural-ownership:
ESSENCE: Established fenced-command and idempotent-recovery practice places dispatch authority and first-writer terminalization in the durable repository aggregate with linearizable CAS; consumers must not infer exactly-once execution from downstream observations. IDEAL: retain the immutable dispatch binding with the action, arbitrate result-versus-teardown at the Task-document CAS, and verify durable winner, stale-generation denial, and absence of executable actions—without claiming Browser execution exactly-once. The production placement is now natural: Mongo preserves the action-owned binding during unguarded teardown terminalization (control-plane/src/storage/mongo-repository.ts:649), and the service authenticates retries from that binding (control-plane/src/services/session-service.ts:190). Six smells: magic numbers—no blocker; bounds are named. Symptom branches—no blocker; authority remains centralized. Missing-inevitability—the durable CAS assertions are adequate. But proxy-over-truth and narrative-over-verification remain in the final delta: both race tests increment the local browserExecutions variable solely when the test itself receives a dispatched record (control-plane/src/storage/repository-contract.test.ts:2241, control-plane/src/storage/repository-contract.test.ts:2243, control-plane/src/storage/repository-contract.test.ts:2289, control-plane/src/storage/repository-contract.test.ts:2291) and then assert 1 (control-plane/src/storage/repository-contract.test.ts:2273, control-plane/src/storage/repository-contract.test.ts:2321). This cannot observe Browser execution or re-execution and directly retains the frozen specification’s explicitly prohibited fixed-by-construction assertion. Skipped-purpose follows: the required removal or replacement was not performed despite these lines being rewritten in the current head. Replace these counters with assertions over the repository-owned invariant only.

proportional-containment:
ESSENCE: Established distributed-systems practice governs: retry authority belongs to an immutable dispatch attempt, protected by monotonic fencing and repository-linearized CAS, while schema migration remains bounded, conditional, and restart-safe. IDEAL: keep dispatch and terminal-winner authority in the repository, with service orchestration and route-level non-disclosure but no broader abstraction. Six smells: magic numbers—no new unexplained policy; the clock injection makes lease truth explicit (control-plane/src/http/session-routes.integration.test.ts:167). Proxy-over-truth—advisory concern: the latest repair changes repository-wide Document to Mongo’s [key: string]: any abstraction (control-plane/src/storage/mongo-repository.ts:6; node_modules/bson/bson.d.ts:726) even though the immediate typing escape remains local to $push (control-plane/src/storage/mongo-repository.ts:564); preserve the former unknown boundary or narrow the cast rather than weakening every collection. Symptom branches—the guarded worker CAS and unguarded teardown path reflect distinct authorities, not lifecycle patches (control-plane/src/storage/mongo-repository.ts:654, control-plane/src/storage/mongo-repository.ts:704). Narrative-over-verification—the corrected HTTP test directly proves an authenticated malformed retry crosses binding authentication and returns 409 (control-plane/src/http/session-routes.integration.test.ts:332). Missing-inevitability—the settled bounded migration remains unchanged. Skipped-purpose—the Mongo teardown update preserves the dispatched binding with the terminal winner (control-plane/src/storage/mongo-repository.ts:680). No goal-blocking regression is evidenced; the typing over-hoist is advisory.

⟦AI:FKST⟧

@chronoai-fkst

chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop merge gate failed: browser execution proxy assertion
Reproduce locally with scripts/run.sh test from the repository root.

@chronoai-fkst

chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop fix pushed for re-review

Previous reviewed head: 6287b65
New head: 694b203

@chronoai-fkst

chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop PR is ready for review

@chronoai-fkst

chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop PR review decision: approve
Verdicts: teleology=approve parsimony=approve fidelity=approve natural-ownership=approve proportional-containment=approve

teleology:
ESSENCE: This is an idempotent distributed-recovery problem governed by immutable operation credentials, monotonic fencing, linearizable first-writer-wins CAS, and tests of authoritative durable state—not execution proxies. Its purpose is to preserve one terminal winner across lease mutation and teardown while rejecting stale authority. IDEAL: Bind each dispatch generation immutably to its action, arbitrate terminalization in the repository, and prove the winner, binding, and absence of executable residue directly. SIX-SMELL COMPARISON: magic numbers—none added; the delta is six deletions. Proxy-over-truth—the named gap is closed: both fixed-by-construction browserExecutions counters and assertions are removed (diff.patch:1482, diff.patch:1501, diff.patch:1522, diff.patch:1541). Symptom branches—none added. Narrative-over-verification—the tests still verify the actual winner and retained dispatchBinding (control-plane/src/storage/repository-contract.test.ts:2260, control-plane/src/storage/repository-contract.test.ts:2263, control-plane/src/storage/repository-contract.test.ts:2305, control-plane/src/storage/repository-contract.test.ts:2308). Missing-inevitability—both losing terminalizations fail, the action slot is absent, and redispatch is denied (control-plane/src/storage/repository-contract.test.ts:2267, control-plane/src/storage/repository-contract.test.ts:2270, control-plane/src/storage/repository-contract.test.ts:2312, control-plane/src/storage/repository-contract.test.ts:2315). Skipped-purpose—none: the form now matches what this repository contract can truthfully establish without pretending to observe Browser execution. The sole named blocker is cleanly resolved.

parsimony:
ESSENCE: Established fenced-command and idempotent-recovery practice requires tests to observe authoritative durable state, not infer Browser execution from repository dispatch. IDEAL: arbitrate worker-versus-teardown through repository CAS, then assert the immutable winner, absence of executable work, and denial of further dispatch. Six-smell comparison: magic numbers—none added; the fix is purely subtractive. Proxy-over-truth—the named gap is closed: both fixed-by-construction browserExecutions counters and their toBe(1) assertions are deleted (diff.patch:1482, diff.patch:1501, diff.patch:1522, diff.patch:1541). Symptom branches—none added. Narrative-over-verification—the remaining tests directly verify the winning persisted result and retained dispatchBinding (control-plane/src/storage/repository-contract.test.ts:2263, control-plane/src/storage/repository-contract.test.ts:2308). Missing-inevitability—they also prove no pending action remains and redispatch is denied (control-plane/src/storage/repository-contract.test.ts:2267, control-plane/src/storage/repository-contract.test.ts:2270, control-plane/src/storage/repository-contract.test.ts:2312, control-plane/src/storage/repository-contract.test.ts:2315). Skipped-purpose—none; the explicit requirement to remove or replace the Browser-execution proxy is now satisfied without replacement ceremony or production complexity. No in-scope fix regression is evidenced.

fidelity:
ESSENCE: This is a linearizable authorization/idempotency problem; established practice verifies durable authoritative state and fencing outcomes, never infers Browser execution from a repository-return counter. IDEAL: race worker terminalization against teardown, then directly prove the unique durable winner, retained immutable dispatch binding, loser rejection, absence of executable action state, and prevention of duplicate in-flight work. SIX SMELLS: magic numbers—none added; proxy-over-truth—the named gap is closed because the latest delta removes both browserExecutions counters and assertions, while dispatch is followed by direct winner verification (control-plane/src/storage/repository-contract.test.ts:2241, control-plane/src/storage/repository-contract.test.ts:2263); symptom branches—none added; narrative-over-verification—both race directions now verify actual repository outcomes, loser CAS failure, and no remaining pending action (control-plane/src/storage/repository-contract.test.ts:2260, control-plane/src/storage/repository-contract.test.ts:2267, control-plane/src/storage/repository-contract.test.ts:2305, control-plane/src/storage/repository-contract.test.ts:2312); missing-inevitability—repeat finalization and redispatch attempts are directly rejected (control-plane/src/storage/repository-contract.test.ts:2268, control-plane/src/storage/repository-contract.test.ts:2270, control-plane/src/storage/repository-contract.test.ts:2313, control-plane/src/storage/repository-contract.test.ts:2315); skipped-purpose—the tests preserve the real contract without claiming Browser exactly-once. The named gap is closed with no fix regression.

natural-ownership:
ESSENCE: Established fenced-command and idempotent-recovery practice assigns immutable dispatch authority, first-writer terminalization, and durable winner state to the repository aggregate with linearizable CAS; downstream tests must observe those authoritative facts, not infer Browser execution. IDEAL: keep each action’s ActionDispatchBinding and terminal winner in the owning Task document, then verify winner immutability, stale-attempt denial, and removal from executable slots. Six smells: magic numbers—none introduced by this fix; proxy-over-truth—the named gap is closed because the latest delta removes only the fixed-by-construction browserExecutions counters, while the tests directly assert the stored worker or teardown winner and its binding (control-plane/src/storage/repository-contract.test.ts:2260, control-plane/src/storage/repository-contract.test.ts:2263, control-plane/src/storage/repository-contract.test.ts:2305, control-plane/src/storage/repository-contract.test.ts:2308); symptom branches—none added; narrative-over-verification—the tests also prove the loser and subsequent retries cannot overwrite the winner (control-plane/src/storage/repository-contract.test.ts:2268, control-plane/src/storage/repository-contract.test.ts:2313); missing-inevitability—absence of remaining executable work and redispatch denial are checked at the repository boundary (control-plane/src/storage/repository-contract.test.ts:2267, control-plane/src/storage/repository-contract.test.ts:2270, control-plane/src/storage/repository-contract.test.ts:2312, control-plane/src/storage/repository-contract.test.ts:2315); skipped-purpose—the explicitly prohibited Browser exactly-once proxy is gone without weakening the naturally owned repository invariant. No fix regression exists within the stated scope.

proportional-containment:
ESSENCE: Established concurrency-testing practice requires assertions over authoritative durable state and linearizable outcomes, not counters that merely mirror a test’s control flow; containment belongs in the repository contract because Browser execution exactly-once is explicitly outside scope. IDEAL: Race the worker and teardown terminalizers, then verify the durable winner, immutable dispatch binding, losing CAS, absence of executable work, and action-slot availability—without inferring downstream execution. SIX SMELLS: magic numbers—none introduced; proxy-over-truth—the final delta removes both browserExecutions counters and their fixed-by-construction assertions; symptom branches—no production or test branch was added; narrative-over-verification—the tests directly verify the winning stored result and binding (control-plane/src/storage/repository-contract.test.ts:2263, control-plane/src/storage/repository-contract.test.ts:2308); missing-inevitability—both losers remain rejected, no pending action remains, and redispatch cannot reopen the terminal action (control-plane/src/storage/repository-contract.test.ts:2267, control-plane/src/storage/repository-contract.test.ts:2268, control-plane/src/storage/repository-contract.test.ts:2270, control-plane/src/storage/repository-contract.test.ts:2312); skipped-purpose—the retained assertions prove repository-owned first-writer-wins and slot recovery without claiming Browser exactly-once (control-plane/src/storage/repository-contract.test.ts:2271, control-plane/src/storage/repository-contract.test.ts:2272). The named gap is closed, and the repair is proportionately limited to the two offending contract tests.

⟦AI:FKST⟧

@chronoai-fkst

chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop is merging PR #43

@chronoai-fkst
chronoai-fkst Bot merged commit a7fc21d into qa-tools-fkst-staging Sep 9, 2026
4 checks passed
@chronoai-fkst

chronoai-fkst Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

github-devloop merged PR #43

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.

0 participants