Add OpenClaw memory adapter and MiniLM fix#66
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3bcbda9204
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| try StoreLockProbe.preflightExclusiveAccess( | ||
| at: URL(fileURLWithPath: storePath), | ||
| timeout: .milliseconds(50) | ||
| ) |
There was a problem hiding this comment.
brokerShutdownCompleted throws if StoreLockProbe.preflightExclusiveAccess cannot get the lock within 50ms, and shutdownStartedBroker does not catch that exception inside its retry loop. In practice, broker shutdown can leave the lock held briefly after the socket disappears, so this path can fail immediately instead of waiting մինչև the configured deadline, causing spurious shutdown errors even when the broker would have exited cleanly moments later.
Useful? React with 👍 / 👎.
| guard let match = filtered.first else { | ||
| throw BrokerValidationError.invalid("No resumable session manifest matched the requested selectors") | ||
| } | ||
| return match |
There was a problem hiding this comment.
resolveSessionManifest returns the first manifest matching selectors without filtering for .active, but sessionResume rejects ended manifests afterward. If the most recently updated matching manifest is ended while another matching manifest is still active, session_resume incorrectly fails with “already been ended” instead of resuming the active one.
Useful? React with 👍 / 👎.
| private func closeSession(_ sessionID: String) async { | ||
| guard let session = sessions.removeValue(forKey: sessionID) else { return } | ||
| await session.transport.disconnect() | ||
| logger.info("Closed HTTP session", metadata: ["sessionID": "\(sessionID)"]) |
There was a problem hiding this comment.
Each HTTP initialize creates and starts a dedicated Server, but closeSession only disconnects the transport and removes the session entry. Because server.stop() is never called on normal session end/expiry/app shutdown, per-session server state can remain running and accumulate over time, leading to avoidable resource leaks in long-lived HTTP deployments.
Useful? React with 👍 / 👎.
christopherkarani
force-pushed
the
codex/zig
branch
from
3bcbda9 to
64c0360
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64c0360093
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if let sessionID { | ||
| normalizedMetadata[MemoryMetadataKeys.promotedFromSession] = sessionID.uuidString |
There was a problem hiding this comment.
When memory_promote is called without session_id (allowed when exactly one session is active), the method resolves a session only for sourcing content, but later bookkeeping still checks the original optional argument. In that path sessionID stays nil, so wax.promoted_from_session is not written and no promotion event/lease refresh is recorded for the active session, which breaks provenance and session-derived promotion signals.
Useful? React with 👍 / 👎.
| while true { | ||
| try? await Task.sleep(for: .seconds(60)) |
There was a problem hiding this comment.
The cleanup worker runs in an infinite loop and ignores cancellation by using try? around Task.sleep. Because start() launches this as an untracked background task, stop() has no way to terminate it, so the task can outlive shutdown and keep waking forever, leaking resources across long-running HTTP app lifecycles.
Useful? React with 👍 / 👎.
christopherkarani
force-pushed
the
codex/zig
branch
from
64c0360 to
77dc3e0
Compare
Merge preparation statusLocal verification completed:
Review cleanup pushed in
Current merge blocker:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 77dc3e0ffe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let channel = try await bootstrap.bind(host: configuration.host, port: configuration.port).get() | ||
| self.channel = channel | ||
| Task { await sessionCleanupLoop() } | ||
| try await channel.closeFuture.get() | ||
| try await group.shutdownGracefully() |
There was a problem hiding this comment.
start() creates a MultiThreadedEventLoopGroup before binding, but shutdownGracefully() is only reached after channel.closeFuture completes. If bind throws (for example, port already in use), the function exits early and leaves the event-loop threads running, which leaks resources and can poison subsequent start attempts in long-lived processes/tests.
Useful? React with 👍 / 👎.
| let document = if let frameID { | ||
| documents.first { $0.frameId == frameID } | ||
| } else { | ||
| documents.sorted { $0.timestampMs > $1.timestampMs }.first | ||
| } |
There was a problem hiding this comment.
When memory_promote is called without frame_id, this branch selects the latest session document but never assigns that document’s frameId back to the variable later used for provenance and recall-signal lookup. As a result, promoted memories lose wax.promoted_from_frame and promotion scoring skips frame-specific recall signals in this common path.
Useful? React with 👍 / 👎.
1 participant
Summary
Verification