Repository navigation
feat(runtime): add session runtime foundation - #800
Merged
Merged
Conversation
This was referenced Jul 14, 2026
Fodesu
force-pushed
the
feat/session-runtime-foundation
branch
8 times, most recently
from
July 16, 2026 10:19
5364532 to
28f0ccc
Compare
Fodesu
marked this pull request as ready for review
July 16, 2026 10:33
qqqqqf-q
approved these changes
Jul 16, 2026
Member
There was a problem hiding this comment.
Reviewed the architecture and ran the full test matrix locally against real Redis 7 + PostgreSQL 16 (memory contract suite, Redis contract suite — verified 0 skips, runtimefence integration, migration 0111 up/down/re-run path, and the fence integration tests in message/acpagent/userinput/toolapproval). All green.
Highlights from the review:
- Epoch + monotonic Seq delta protocol with gap detection and snapshot-fallback is sound; no silent event loss path (buffer overflow emits a dropped-marker that forces reconciliation).
- Fence validation and business writes share one transaction (
FOR NO KEY UPDATElock-as-validation) — no check-then-act window. Lock ordering (bot → session) is explicit and covered by dedicated lock-order tests. - Legacy/unfenced rows (
NULLtoken) are verifiably untouched by supersede logic, and the migration test covers upgrade-from-existing-data, idempotent re-run, and sequence non-regression. - The schema/query doc shared with the team matches the actual diff — including the honest callout that
LockSessionDecisionSequenceserializes per-session decision creation on all paths (fenced or not) to fix theMAX(short_id)+1race. Verified the lock strengths in SQL (FOR KEY SHAREon bots,FOR UPDATEonly on the session row during decision creation), so the blast radius is per-session and negligible.
Non-blocking notes, fine as follow-ups:
message/service.gocarries two fixes unrelated to fencing (asset-creation failures now propagate instead ofWarn, andDeleteMessagesByX→ClearHistoryByX) — worth a line in the PR description confirming they're intentional.Managerhas ~28m.distributed != nilbranches; memory mode re-inlines the distributed invariants rather than living behind the backend interface. A third backend would touch all those branch points. The shared contract-test suite compensates, but worth keeping in mind.- The 7
StartRun*variants all funnel into one path — an options struct would shrink the API surface for integrators. - Orphaned-run reaping is lazy (only on
Snapshot()calls); a session nobody subscribes to staysrunninguntil state TTL. Acceptable, but curious if an active reaper is planned. - Every
HandleAgentEventround-trips the full snapshot JSON through a Redis WATCH/MULTI transaction with an unbounded retry loop — likely the main perf question once #801 wires live token streaming through this. Will raise it there.
Foundation-only as advertised: verified Activate/NextSessionRuntimeFenceToken have zero production call sites, so deploying this alone changes no behavior.
Fodesu
force-pushed
the
feat/session-runtime-foundation
branch
from
July 17, 2026 08:38
28f0ccc to
f581887
Compare
Fodesu
force-pushed
the
feat/session-runtime-foundation
branch
from
July 17, 2026 09:12
f581887 to
845e672
Compare
3 tasks
qqqqqf-q
added a commit
to qqqqqf-q/Memoh
that referenced
this pull request
Jul 17, 2026
The felinics#800 CI step passed only because the script was a no-op. Now that typecheck really runs, it reports ~476 pre-existing errors; gating on that immediately would block every PR. Run it with continue-on-error so the backlog stays visible in CI logs without turning main red.
B405
qqqqqf-q
added a commit
that referenced
this pull request
Jul 17, 2026
* fix(web): make typecheck script actually check app source @memohai/web had no working typecheck: the root tsconfig.json is a solution-style file with "files": [], and no script pointed vue-tsc at tsconfig.app.json, which is the config that actually includes src/. Add a typecheck script that runs vue-tsc against tsconfig.app.json. It currently reports 476 pre-existing errors (218 in apps/web src, 258 in the @felinic/ui submodule sources); this change only makes the backlog visible and intentionally does not wire the script into lint/CI gates. * ci(web): keep typecheck step non-blocking until error backlog clears The #800 CI step passed only because the script was a no-op. Now that typecheck really runs, it reports ~476 pre-existing errors; gating on that immediately would block every PR. Run it with continue-on-error so the backlog stays visible in CI logs without turning main red.
This was referenced Sep 5, 2026
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
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
Validation
go test ./internal/config ./internal/sessionruntime ./internal/runtimefence ./internal/message ./internal/toolapproval ./internal/userinput ./internal/acpagentpnpm test --run --testTimeout=15000 apps/web/src/composables/api/useChat.ws.test.ts apps/web/src/store/chat-list.test.tspnpm --filter @memohai/web exec vue-tsc --noEmit