You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "Sun-GLiang (via GitHub)" <gi...@apache.org> on 2026/08/23 07:02:27 UTC

[PR] refactor(core): separate persisted decode boundaries [maka]

Sun-GLiang opened a new pull request, #3562:
URL: https://github.com/apache/maka/pull/3562

   ## Summary
   
   Separate strict decoding for new input and wire data from compatibility decoding at durable-storage boundaries.
   
   - introduce invariant `PersistedValue<T>` branding for raw durable values
   - add persisted decoders for AgentRun, SessionHeader, StoredMessage/ToolResult, and ScheduledTask records
   - fold retired AgentRun states/fields and `execute` permission modes only on persisted read paths
   - cover RuntimeEvent replay, conversation copy/archive, continuation claims, pending scheduled fires, metadata, catalogs, transcripts, and backup restore
   - keep create, update, import, append, live tool output, and protocol paths strict
   - perform no database rewrite, schema migration, or compatibility epoch bump
   
   Fixes #3413
   
   ## Verification
   
   - `npm --workspace @maka/core test` — 629 passed
   - `npm --workspace @maka/storage test` — 902 passed, 14 skipped
   - focused RuntimeEvent read-model and conversation-copy tests — 68 passed
   - `npm --workspace @maka/runtime test` — 3072 passed, 13 skipped; one pre-existing environment-dependent macOS executable-root assertion fails when Node is installed at `/usr/local/bin/node`
   - `npm run lint`
   - `npm run format:check`
   - `npm run build`
   - `npm run typecheck`
   - `git diff --check`
   
   ## AI use
   
   Select exactly one:
   
   - [ ] No generative tool made a substantive contribution
   - [x] Generative tooling made a substantive contribution
   
   Tool(s) and scope: Codex implemented the persisted decoding boundary refactor and regression tests under human review. Each substantive commit includes a `Generated-by: Codex` trailer.
   
   ## Checklist
   
   - [x] Tests cover the change and fail without it
   - [x] Lint, format, typecheck and the affected suites pass locally
   
   Does this PR entail a change in behavior?
   
   - [x] Yes — described under Summary above
   - [ ] No


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org


Re: [PR] refactor(core): separate persisted decode boundaries [maka]

Posted by "Sun-GLiang (via GitHub)" <gi...@apache.org>.
Sun-GLiang commented on PR #3562:
URL: https://github.com/apache/maka/pull/3562#issuecomment-5386346600

   Conflicts have been resolved. Please rerun the CI checks. Thanks!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org


Re: [PR] refactor(core): separate persisted decode boundaries [maka]

Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han merged PR #3562:
URL: https://github.com/apache/maka/pull/3562


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org


Re: [PR] refactor(core): separate persisted decode boundaries [maka]

Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #3562:
URL: https://github.com/apache/maka/pull/3562#discussion_r3839282903


##########
packages/core/src/persisted-value.ts:
##########
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+declare const persistedValueBrand: unique symbol;
+
+/**
+ * An untrusted value read from durable state for the domain type `T`.
+ *
+ * The function-valued brand keeps `T` invariant, so related domain types
+ * cannot be substituted across persistence seams. The value has no runtime
+ * wrapper: a persistence adapter marks the raw value, and a decoder is the
+ * only intended place that casts it back to `unknown` for inspection.
+ */
+export type PersistedValue<T> = {
+  readonly [persistedValueBrand]: (value: T) => T;
+};
+
+/** Mark an untrusted value at the point where durable state enters the process. */
+export function markPersisted<T>(value: unknown): PersistedValue<T> {
+  return value as PersistedValue<T>;

Review Comment:
   [P3] The brand can enforce "this must be decoded", but it cannot enforce "this call site really is a persistence boundary" — `markPersisted<T>(value: unknown)` is an unchecked cast.
   
   So anyone can hand it a non-persisted value (a live wire frame, say) and that value silently takes the tolerant decoding path, quietly reinstating exactly the laxity this PR separates out — with nothing visible in the type system. Thirteen files call it today.
   
   `persisted-value-contract.test.ts` can't catch this by construction: it proves type discipline, not provenance. Cheapest fix is an approved-callers list with an assertion test, since the set is small and enumerable. Cleaner fix is a restricted-import rule or a module-private factory so `markPersisted` is only reachable from designated persistence adapters.



##########
packages/runtime-host/src/protocol/session-turns.ts:
##########
@@ -406,7 +406,7 @@ function decodeSessionTurnContribution(value: unknown): SessionTurnContribution
       'sequence',
       'message',
     ]);
-    const message = decodeStoredMessage(state.message);
+    const message = decodeCanonicalMessage(state.message);

Review Comment:
   [P3] The same wire carries two different trust treatments, and neither end documents why.
   
   Transcripts arriving over the wire get wrapped in `markPersisted(...)` and decoded **tolerantly** (`apps/desktop/src/main/runtime-host-client.ts:136`, `packages/cli/src/runtime-host-session-channel.ts:50`, `packages/cli/src/runtime-host-session-driver.ts:94`), while turn contributions arriving over the wire are decoded **strictly** here.
   
   Each choice is defensible on its own — transcripts are history and may come from an older Host, turn contributions are a fresh projection. But the two rationales can't both hold: if the Host always normalizes on read, transcripts don't need tolerance; if it doesn't always, the strict side is the exposed one.
   
   Not asking for a behavior change, just a line at each seam naming the trust source and the reason. Without it the next person to touch this has no way to tell which side is load-bearing.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org