You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "bug-superman (via GitHub)" <gi...@apache.org> on 2026/09/03 06:47:51 UTC
[PR] fix(runtime-host): lazily adopt legacy Session Connection identity [maka]
bug-superman opened a new pull request, #4627:
URL: https://github.com/apache/maka/pull/4627
Closes #3860.
## Summary
- Add a Host-owned, revision/CAS-gated lazy adoption path for unlocked legacy Sessions.
- Revalidate the adopted identity through the bound ID + slug resolver before execution.
- Keep locked legacy Sessions fail-closed and require explicit account selection.
- Run adoption before fresh root execution, recovery, and safe-boundary continuation admission.
- Preserve the existing immutable Connection ID propagation into AgentRuns and derived Session paths.
## Safety
Bound execution continues to resolve by immutable `connectionId` and verifies the stored slug. Missing IDs and ID/slug mismatches never fall back to a same-slug catalog entry, so deleted/reused Connections cannot cross credentials. OAuth reconciliation remains ID-bound through the existing resolver.
A pre-upgrade legacy Session has no deletion tombstone or historical identity evidence. If its original Connection was deleted and the slug reused before this upgrade, the original identity cannot be reconstructed; locked legacy Sessions therefore remain blocked instead of being silently rebound.
## Verification
- `npm --workspace @maka/runtime-host run typecheck` (Node 24)
- `npm --workspace @maka/runtime-host run build` (Node 24)
- Focused runtime-host session catalog and host availability tests: all pass
- Focused storage runtime-policy tests: all pass
- Full monorepo build reaches unrelated pre-existing `@maka/ui` type errors after core/storage/runtime/runtime-host compile successfully.
--
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] fix(runtime-host): lazily adopt legacy Session Connection identity [maka]
Posted by "bug-superman (via GitHub)" <gi...@apache.org>.
bug-superman commented on code in PR #4627:
URL: https://github.com/apache/maka/pull/4627#discussion_r3930323640
##########
packages/runtime-host/src/server/session-catalog-coordinator.ts:
##########
@@ -226,6 +226,62 @@ export class HostSessionCatalogCoordinator {
if (!outcome.ok) throw new Error(outcome.error.message);
}
+ /**
+ * Compatibility migration for pre-identity Sessions that have not yet
+ * committed a user turn. The write is revision/CAS guarded and the resulting
Review Comment:
Fixed in d13385701. The comment now explicitly describes the connectionLocked flag being false and documents why locked legacy Sessions remain untouched.
##########
packages/runtime-host/src/server/session-catalog-coordinator.ts:
##########
@@ -226,6 +226,62 @@ export class HostSessionCatalogCoordinator {
if (!outcome.ok) throw new Error(outcome.error.message);
}
+ /**
+ * Compatibility migration for pre-identity Sessions that have not yet
+ * committed a user turn. The write is revision/CAS guarded and the resulting
+ * bound identity is revalidated through the ID-based execution resolver.
+ * The optional lease lets an already-admitted execution reuse its Session
+ * lane; callers without one are serialized here. Locked legacy Sessions stay
+ * untouched because the catalog cannot prove historical ownership after
+ * delete-and-reuse of a slug.
+ */
+ async adoptLegacySessionConnectionIdentity(
+ sessionId: string,
+ admissionLease?: SessionAdmissionLease,
+ ): Promise<SessionHeaderSnapshot | undefined> {
+ return admissionLease
+ ? this.#admission.runAdmitted(sessionId, admissionLease, () =>
+ this.#adoptLegacySessionConnectionIdentity(sessionId),
+ )
+ : this.#admission.run(sessionId, () => this.#adoptLegacySessionConnectionIdentity(sessionId));
+ }
+
+ async #adoptLegacySessionConnectionIdentity(
+ sessionId: string,
+ ): Promise<SessionHeaderSnapshot | undefined> {
+ const current = await this.#stores.readHeaderRecordSnapshot(sessionId);
+ if (
+ current.header.llmConnectionId !== undefined ||
+ current.header.backend === 'fake' ||
+ current.header.connectionLocked
+ ) {
+ return undefined;
+ }
+ const connection = (await this.#runtimePolicy.connectionCatalog.getSnapshot()).connections.find(
Review Comment:
Fixed in d13385701. The raw connectionCatalog snapshot lookup was removed. Adoption now reuses resolveExecutionConnection with the catalog_slug reference and takes the canonical connectionId from its ready result; the redundant post-commit resolver is gone.
##########
packages/runtime-host/src/server/session-catalog-coordinator.ts:
##########
@@ -226,6 +226,62 @@ export class HostSessionCatalogCoordinator {
if (!outcome.ok) throw new Error(outcome.error.message);
}
+ /**
+ * Compatibility migration for pre-identity Sessions that have not yet
+ * committed a user turn. The write is revision/CAS guarded and the resulting
+ * bound identity is revalidated through the ID-based execution resolver.
+ * The optional lease lets an already-admitted execution reuse its Session
+ * lane; callers without one are serialized here. Locked legacy Sessions stay
+ * untouched because the catalog cannot prove historical ownership after
+ * delete-and-reuse of a slug.
+ */
+ async adoptLegacySessionConnectionIdentity(
+ sessionId: string,
+ admissionLease?: SessionAdmissionLease,
+ ): Promise<SessionHeaderSnapshot | undefined> {
+ return admissionLease
+ ? this.#admission.runAdmitted(sessionId, admissionLease, () =>
+ this.#adoptLegacySessionConnectionIdentity(sessionId),
+ )
+ : this.#admission.run(sessionId, () => this.#adoptLegacySessionConnectionIdentity(sessionId));
+ }
+
+ async #adoptLegacySessionConnectionIdentity(
+ sessionId: string,
+ ): Promise<SessionHeaderSnapshot | undefined> {
+ const current = await this.#stores.readHeaderRecordSnapshot(sessionId);
+ if (
+ current.header.llmConnectionId !== undefined ||
+ current.header.backend === 'fake' ||
+ current.header.connectionLocked
+ ) {
+ return undefined;
+ }
+ const connection = (await this.#runtimePolicy.connectionCatalog.getSnapshot()).connections.find(
+ (candidate) => candidate.slug === current.header.llmConnectionSlug,
+ );
+ if (!connection) return undefined;
+ let committed: SessionHeaderSnapshot;
+ try {
+ committed = await this.#stores.updateHeaderVersioned(
Review Comment:
Fixed in d13385701. Adoption now calls the existing catalog_slug resolver before the revision/CAS update, and no llmConnectionId is persisted unless resolution is ready. The not_found and identity_mismatch tests now assert that the Session remains unbound.
--
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] fix(runtime-host): lazily adopt legacy Session Connection identity [maka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #4627: fix(runtime-host): lazily adopt legacy Session Connection identity
URL: https://github.com/apache/maka/pull/4627
--
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] fix(runtime-host): lazily adopt legacy Session Connection identity [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #4627:
URL: https://github.com/apache/maka/pull/4627#discussion_r3927060164
##########
packages/runtime-host/src/server/session-catalog-coordinator.ts:
##########
@@ -226,6 +226,62 @@ export class HostSessionCatalogCoordinator {
if (!outcome.ok) throw new Error(outcome.error.message);
}
+ /**
+ * Compatibility migration for pre-identity Sessions that have not yet
+ * committed a user turn. The write is revision/CAS guarded and the resulting
+ * bound identity is revalidated through the ID-based execution resolver.
+ * The optional lease lets an already-admitted execution reuse its Session
+ * lane; callers without one are serialized here. Locked legacy Sessions stay
+ * untouched because the catalog cannot prove historical ownership after
+ * delete-and-reuse of a slug.
+ */
+ async adoptLegacySessionConnectionIdentity(
+ sessionId: string,
+ admissionLease?: SessionAdmissionLease,
+ ): Promise<SessionHeaderSnapshot | undefined> {
+ return admissionLease
+ ? this.#admission.runAdmitted(sessionId, admissionLease, () =>
+ this.#adoptLegacySessionConnectionIdentity(sessionId),
+ )
+ : this.#admission.run(sessionId, () => this.#adoptLegacySessionConnectionIdentity(sessionId));
+ }
+
+ async #adoptLegacySessionConnectionIdentity(
+ sessionId: string,
+ ): Promise<SessionHeaderSnapshot | undefined> {
+ const current = await this.#stores.readHeaderRecordSnapshot(sessionId);
+ if (
+ current.header.llmConnectionId !== undefined ||
+ current.header.backend === 'fake' ||
+ current.header.connectionLocked
+ ) {
+ return undefined;
+ }
+ const connection = (await this.#runtimePolicy.connectionCatalog.getSnapshot()).connections.find(
+ (candidate) => candidate.slug === current.header.llmConnectionSlug,
+ );
+ if (!connection) return undefined;
+ let committed: SessionHeaderSnapshot;
+ try {
+ committed = await this.#stores.updateHeaderVersioned(
Review Comment:
P2: the CAS write happens before the revalidation below, and a failed revalidation does not undo it. If the catalog changes between the snapshot on line 260 and `resolveExecutionConnection` on line 275, or the two simply disagree, the method returns `undefined` (caller sees the pre-write header and fails closed for this execution) while the store now holds a `llmConnectionId` that never passed revalidation. On the next admission the first guard on line 254 sees a defined id and skips adoption entirely, so the Session executes with that binding and nothing ever clears it. Credentials cannot cross, since `bound` never falls back to slug, but it is still a committed wrong state, and the new test at `session-catalog-coordinator.test.ts:1144` asserts the bad id stays persisted. Smallest fix: revalidate first and commit only on a resolvable identity. #3860 asks for revalidation before execution, which validate-then-write also satisfies, with one fewer residual state.
##########
packages/runtime-host/src/server/session-catalog-coordinator.ts:
##########
@@ -226,6 +226,62 @@ export class HostSessionCatalogCoordinator {
if (!outcome.ok) throw new Error(outcome.error.message);
}
+ /**
+ * Compatibility migration for pre-identity Sessions that have not yet
+ * committed a user turn. The write is revision/CAS guarded and the resulting
+ * bound identity is revalidated through the ID-based execution resolver.
+ * The optional lease lets an already-admitted execution reuse its Session
+ * lane; callers without one are serialized here. Locked legacy Sessions stay
+ * untouched because the catalog cannot prove historical ownership after
+ * delete-and-reuse of a slug.
+ */
+ async adoptLegacySessionConnectionIdentity(
+ sessionId: string,
+ admissionLease?: SessionAdmissionLease,
+ ): Promise<SessionHeaderSnapshot | undefined> {
+ return admissionLease
+ ? this.#admission.runAdmitted(sessionId, admissionLease, () =>
+ this.#adoptLegacySessionConnectionIdentity(sessionId),
+ )
+ : this.#admission.run(sessionId, () => this.#adoptLegacySessionConnectionIdentity(sessionId));
+ }
+
+ async #adoptLegacySessionConnectionIdentity(
+ sessionId: string,
+ ): Promise<SessionHeaderSnapshot | undefined> {
+ const current = await this.#stores.readHeaderRecordSnapshot(sessionId);
+ if (
+ current.header.llmConnectionId !== undefined ||
+ current.header.backend === 'fake' ||
+ current.header.connectionLocked
+ ) {
+ return undefined;
+ }
+ const connection = (await this.#runtimePolicy.connectionCatalog.getSnapshot()).connections.find(
Review Comment:
P2: this is a second slug to identity resolver. `resolveExecutionConnection({ kind: 'catalog_slug', connectionSlug })` already does exactly this lookup, inside the runtime policy lane and with the retired and enabled checks, at `packages/storage/src/runtime-policy/coordinator.ts:826`. Resolving here off a raw catalog snapshot is what lets the two disagree, which is precisely the case the post-commit revalidation then has to fail closed on. Going through the existing resolver and taking `connection.connectionId` off the ready result removes this duplication and lets the revalidation block on lines 275 to 282 disappear.
##########
packages/runtime-host/src/server/session-catalog-coordinator.ts:
##########
@@ -226,6 +226,62 @@ export class HostSessionCatalogCoordinator {
if (!outcome.ok) throw new Error(outcome.error.message);
}
+ /**
+ * Compatibility migration for pre-identity Sessions that have not yet
+ * committed a user turn. The write is revision/CAS guarded and the resulting
Review Comment:
P3: the comment says "have not yet committed a user turn" but the code branches on `connectionLocked` (line 256). They are equivalent today because the store sets the lock on the first user message, but the comment should name the field it actually reads, otherwise it becomes wrong the moment `connectionLocked` picks up another meaning.
--
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] fix(runtime-host): lazily adopt legacy Session Connection identity [maka]
Posted by "testikun (via GitHub)" <gi...@apache.org>.
testikun commented on PR #4627:
URL: https://github.com/apache/maka/pull/4627#issuecomment-5522343148
Generative tooling disclosure: OpenAI Codex contributed materially to this change; final review and ownership remain with the human contributor.
--
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] fix(runtime-host): lazily adopt legacy Session Connection identity [maka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4627:
URL: https://github.com/apache/maka/pull/4627#issuecomment-5976835857
<!-- maka-lifecycle:stale-warning -->
This pull request has had no new commits for 30 days and has been marked stale. It will be closed in 7 days unless a new commit is pushed.
Comments do not reset this timer: only a new commit does. If the pull request is intentionally long-lived, a maintainer can apply the `pinned` label.
--
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] fix(runtime-host): lazily adopt legacy Session Connection identity [maka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #4627:
URL: https://github.com/apache/maka/pull/4627#issuecomment-6117537264
<!-- maka-lifecycle:stale-close -->
This pull request has been closed because it received no new commits during the 7-day grace period. This is not a judgement on the merit of the change; it only keeps the review queue aligned with work that is actively in flight.
The branch and review history are preserved. Push a new commit and reopen the pull request whenever the work is ready to continue.
--
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