feat(nodes): add LaserData memory tool node (tool_laserdata_memory) - #1760
Conversation
Durable, shared agent memory (remember/recall/improve/forget) backed by LaserData's Laser SDK memory primitive over Apache Iggy (issue #1733). First memory tool node wrapping an async (PyO3) SDK: persistent bridge loop in IGlobal, lazy single-flight connect, run_coroutine_threadsafe with configured timeout. Dynamic test gated on LASER_CONNECTION_STRING. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ation Refine tool_laserdata_memory after live testing against a real LaserData Cloud deployment (free tier, GCP us-west1): - Deployment dropdown (qdrant profile pattern): local Apache Iggy vs LaserData Cloud, each with its own connection help text and defaults. - Remove the token field entirely — Cloud authenticates with credentials inside the connection string; there is no separate token (verified live). - Pin laser-sdk DOWN to 0.0.1rc19 deliberately: rc20+ wheels speak only Apache Iggy's VSR cluster protocol, which neither LaserData Cloud nor stock Apache Iggy serve today (VSR requests time out; rc19 connects and completes the full memory round-trip). API is signature-identical to rc21; re-pin upward when their deployments answer VSR. - New advanced 'stream' field (default rocketride-memory): rc19 requires a default stream pinned at connect for laser.memory() to resolve. - Cloud connection strings need explicit ?tls=true (rc21's documented auto-attach does not fire); documented in field help + README. Tests: 343 passing (316 contract + 27 unit); ruff clean. Live e2e: full remember/recall/improve/forget round-trip with tombstone verification passed against LaserData Cloud, both direct SDK and end-to-end through chat UI -> agent_deepagent -> node. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
🤖 Internal: Discord sync markerAuto-managed by the Discord notification workflow. Stores the linked Discord message ID and forum thread ID. Do not edit or delete. |
📝 WalkthroughWalkthroughThe PR adds a LaserData-backed memory tool node with persistent asynchronous connection handling, deployment profiles, four synchronous memory operations, structured error handling, documentation, and isolated tests. ChangesLaserData memory integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Agent
participant IInstance
participant IGlobal
participant LaserSDK
Agent->>IInstance: invoke memory tool
IInstance->>IGlobal: run SDK coroutine
IGlobal->>LaserSDK: execute memory operation
LaserSDK-->>IGlobal: return memory result
IGlobal-->>IInstance: return structured result
IInstance-->>Agent: return tool response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nodes/src/nodes/tool_laserdata_memory/IGlobal.py`:
- Around line 155-176: Update endGlobal and the run/get_laser lifecycle to
coordinate teardown with concurrent operations: mark the shared IGlobal as
closing before clearing resources, have run() reject new submissions and ensure
in-flight futures are completed or failed promptly, then stop and close the loop
only after those operations are handled. Preserve the existing close timeout and
cleanup behavior while preventing callers from waiting for an operation timeout
after loop shutdown.
- Around line 92-101: Update the numeric configuration handling in beginGlobal
so malformed recall_limit or op_timeout values cannot let int() raise an
uncaught ValueError. Match the deliberate user-facing validation/error pattern
already used for connection_string, while preserving the existing defaults and
min/max clamping for valid numeric values.
In `@nodes/src/nodes/tool_laserdata_memory/IInstance.py`:
- Around line 155-158: Update the limit clamping in the recall flow of IInstance
to use IGlobal._MAX_RECALL_LIMIT instead of the hardcoded 200 value, while
preserving the existing lower bound and input validation behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e458b49e-7a4e-4341-8958-6c7f6f215c8f
⛔ Files ignored due to path filters (1)
nodes/src/nodes/tool_laserdata_memory/laserdata.svgis excluded by!**/*.svg
📒 Files selected for processing (8)
nodes/src/nodes/tool_laserdata_memory/IGlobal.pynodes/src/nodes/tool_laserdata_memory/IInstance.pynodes/src/nodes/tool_laserdata_memory/README.mdnodes/src/nodes/tool_laserdata_memory/__init__.pynodes/src/nodes/tool_laserdata_memory/requirements.txtnodes/src/nodes/tool_laserdata_memory/services.jsonnodes/test/tool_laserdata_memory/__init__.pynodes/test/tool_laserdata_memory/test_tools.py
…, robust numeric config Address CodeRabbit review on #1760: - endGlobal now flips a closing flag and cancels pending bridge futures, so concurrent in-flight tool calls fail fast ('node is closing') instead of waiting out their full op_timeout against a stopped loop. - Malformed numeric config (recall_limit/op_timeout) falls back to defaults instead of crashing beginGlobal with a raw ValueError. - recall's limit clamp uses _MAX_RECALL_LIMIT instead of a duplicated 200. Tests: 346 passing (3 new: malformed-number fallback, in-flight cancel fail-fast, run-rejected-while-closing); ruff clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nodes/src/nodes/tool_laserdata_memory/IGlobal.py (1)
124-149: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winResidual TOCTOU race lets an in-flight call miss cancellation and hang for the full
op_timeoutduring shutdown.
run()'s closing/loop check and itspending.add(future)aren't atomic withendGlobal()'s_closing = True+ pending-swap-and-cancel. IfendGlobal()runs between those tworun()steps, the future is submitted to the loop but never registered in any pending set (sinceself._pendingis alreadyNoneby the timerun()reads it), so it's never cancelled. The loop has already been stopped, sofuture.result(budget)blocks for the fullop_timeoutinstead of failing fast with'laserdata: node is closing'— undermining the exact shutdown guarantee this PR adds.test_end_global_cancels_in_flight_callsonly covers the case where the future is already in_pendingbeforeendGlobal()runs, so this window is untested.Guard the check-and-register step in
run()and the flip-and-swap step inendGlobal()with one dedicated lock (not_connect_lock—get_laser()callsself.run(...)while holding it, so reusing it here would deadlock).🔒 Proposed fix using a dedicated lock
_connect_lock: threading.Lock | None = None _laser: Any = None _closing: bool = False _pending: Any = None + _pending_lock: threading.Lock | None = None+ self._pending_lock = threading.Lock() self._connect_lock = threading.Lock()loop = self._loop - if self._closing or loop is None or not loop.is_running(): + if loop is None or not loop.is_running(): coro.close() raise RuntimeError('laserdata: node is not open') budget = self.op_timeout if timeout is None else timeout future = asyncio.run_coroutine_threadsafe(coro, loop) - pending = self._pending - if pending is not None: - pending.add(future) + with self._pending_lock: + if self._closing: + future.cancel() + raise RuntimeError('laserdata: node is closing') + pending = self._pending + if pending is not None: + pending.add(future) try:def endGlobal(self) -> None: """Close the connection, stop the bridge loop, and clear secrets.""" - self._closing = True laser, self._laser = self._laser, None loop, self._loop = self._loop, None thread, self._loop_thread = self._loop_thread, None - pending, self._pending = self._pending, None + with self._pending_lock: + self._closing = True + pending, self._pending = self._pending, NoneAlso applies to: 174-187
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nodes/src/nodes/tool_laserdata_memory/IGlobal.py` around lines 124 - 149, Protect the shutdown race between run()’s closing/loop validation and pending registration by adding a dedicated lock, and use that same lock in endGlobal() when setting _closing, swapping _pending, and initiating cancellation. In run(), hold the lock through validation, coroutine submission, and adding the future to the pending set; do not reuse _connect_lock because get_laser() can call run() while holding it. Preserve the existing timeout and closing-error behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nodes/test/tool_laserdata_memory/test_tools.py`:
- Around line 434-437: Bound the wait loop around worker startup and
glb._pending with a finite deadline or timeout, and fail the test explicitly if
pending registration does not occur in time. Preserve the existing polling
behavior while ensuring regressions cannot leave the test hanging indefinitely.
---
Outside diff comments:
In `@nodes/src/nodes/tool_laserdata_memory/IGlobal.py`:
- Around line 124-149: Protect the shutdown race between run()’s closing/loop
validation and pending registration by adding a dedicated lock, and use that
same lock in endGlobal() when setting _closing, swapping _pending, and
initiating cancellation. In run(), hold the lock through validation, coroutine
submission, and adding the future to the pending set; do not reuse _connect_lock
because get_laser() can call run() while holding it. Preserve the existing
timeout and closing-error behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b3ae2b5d-eb9c-4cfc-88de-8783620149ba
📒 Files selected for processing (3)
nodes/src/nodes/tool_laserdata_memory/IGlobal.pynodes/src/nodes/tool_laserdata_memory/IInstance.pynodes/test/tool_laserdata_memory/test_tools.py
asclearuc
left a comment
asclearuc
left a comment
There was a problem hiding this comment.
@dylan-savage thanks for this — it is a carefully built node: the async->sync bridge, the lazy single-flight connect, the teardown cancellation, and the deliberate rc19 pin are all well reasoned, and the test coverage is thorough.
I am approving. The three inline notes are non-blocking: two are DRY cleanups (reuse the ai.common.utils validators require_str / int_arg / config_int rather than the private _req_str / _opt_str / _int_or), and one asks you to confirm the connect-error path cannot leak the connection-string password. CodeRabbit's earlier endGlobal race and the two lower-severity items look addressed; its one still-open note is a Minor — the unbounded while not glb._pending busy-wait in test_end_global_cancels_in_flight_calls (test_tools.py:437), worth a deadline.
LaserData confirmed the rc20+ protocol flip and shipped VSR-enabled Cloud deployments (free tier included, deployments created on/after 2026-07-31; older ones must be recreated). Verified live on a fresh deployment with rc21: full remember/recall/improve/forget round-trip over VSR, including TLS auto-attach via the SDK's embedded root CA — the earlier ?tls=true requirement is dropped from docs (the apparent auto-TLS failure was the VSR mismatch masking the handshake, not a TLS bug). Docs updated: Cloud + local field help and README now state the VSR server requirement (laser-stack for local; stock pre-VSR Iggy images cannot talk to this SDK). Tests: 346 passing; ruff clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
nodes/src/nodes/tool_laserdata_memory/README.md (2)
11-16: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winInclude
streamin the shared-memory identity.The README says that agents share memory when the deployment and namespace match. Lines 85-86 also require matching
streamvalues. If streams differ, agents do not address the same memory topics.Update this sentence to require the same deployment, stream, and namespace.
Proposed documentation change
-Memory survives pipeline restarts and is shared by every agent pointing at the same deployment and namespace. +Memory survives pipeline restarts and is shared by every agent pointing at the same deployment, stream, and namespace.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nodes/src/nodes/tool_laserdata_memory/README.md` around lines 11 - 16, Update the shared-memory description in the README to state that agents share memory only when deployment, stream, and namespace all match; leave the surrounding Laser SDK behavior description unchanged.
84-87: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winState the namespace requirement when overrides are disabled.
When
allow_namespace_overrideisfalse, callers cannot providenamespaceper call. The configurednamespacemust be non-empty. Clarify this condition instead of saying only “required here or per call.”Proposed documentation change
-- `namespace`: default memory scope for all connected agents; required here or per call. +- `namespace`: default memory scope for all connected agents. Required when + `allow_namespace_override` is `false`; otherwise callers may provide it per call.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nodes/src/nodes/tool_laserdata_memory/README.md` around lines 84 - 87, Update the `namespace` option description in the tool laserdata memory README to state that it must be configured with a non-empty value when `allow_namespace_override` is false; otherwise, it may be supplied per call when overrides are enabled.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@nodes/src/nodes/tool_laserdata_memory/README.md`:
- Around line 11-16: Update the shared-memory description in the README to state
that agents share memory only when deployment, stream, and namespace all match;
leave the surrounding Laser SDK behavior description unchanged.
- Around line 84-87: Update the `namespace` option description in the tool
laserdata memory README to state that it must be configured with a non-empty
value when `allow_namespace_override` is false; otherwise, it may be supplied
per call when overrides are enabled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 663b2f03-b7ec-4b33-8c78-70270cbe8df2
📒 Files selected for processing (3)
nodes/src/nodes/tool_laserdata_memory/README.mdnodes/src/nodes/tool_laserdata_memory/requirements.txtnodes/src/nodes/tool_laserdata_memory/services.json
|
Final verification on laser-sdk rc21 (post c348416): LaserData shipped VSR-enabled Cloud deployments overnight (confirmed directly with their team — deployments created on/after 2026-07-31). Re-ran the full live e2e on a fresh free-tier deployment: chat UI → agent_deepagent → node → LaserData Cloud over VSR with bare-string auto-TLS — remember/recall/improve/forget all pass, 5/5 including tombstone verification. The node is current with the vendor's latest SDK and their production Cloud. |
…rrors Address review on #1760 (asclearuc + CodeRabbit): - Replace private _req_str/_opt_str and the manual limit clamp with the shared ai.common.utils validators (require_str, optional_str via a thin stripping wrapper, int_arg). Behavior change per int_arg contract: a present non-int 'limit' now raises instead of silently defaulting. - Replace _int_or + manual clamps with shared config_int; per its semantics a configured 0 now means 'use default' instead of clamping to the floor. - Scrub connection-string credentials from SDK exception text before it reaches tool results or logs (_safe_error): the full DSN and the bare password are both redacted. Observed rc21 errors only echo host:port, but the secure field warrants defense in depth. - Bound the in-flight-cancel test's readiness wait with a 5s deadline. Tests: 348 passing (stub harness mirrors the shared validator contracts); ruff clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
nodes/src/nodes/tool_laserdata_memory/IGlobal.py (2)
120-146: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winNarrow TOCTOU race between
run()andendGlobal()can reintroduce the fixed teardown hang.
run()checksself._closing, creates the future, then registers it inself._pendingas three separate steps. IfendGlobal()(lines 172-176) runs its_closing = True/_pendingswap in between the check and the registration, the new future is created against aloopthat is about to stop, but it is never added to the (already-swapped-to-None) pending set and so is never cancelled. The caller then waits out the fullop_timeoutinstead of failing fast — a narrower recurrence of the hang this PR's_closing/_pendingmechanism was built to eliminate.The window is small, but the module's own comment (lines 54-55) states agents issue parallel tool calls against this shared
IGlobal, so teardown racing a fresh call is a real scenario, not a hypothetical one.🔒️ Proposed fix: make the check-create-register sequence atomic with the teardown swap
_connect_lock: threading.Lock | None = None + # Guards the closing-check + future create/register sequence in run() + # against a concurrent endGlobal() swap. Distinct from _connect_lock, + # which get_laser() holds while calling run() (reusing it here would + # deadlock). + _state_lock: threading.Lock | None = None _laser: Any = None @@ self._connect_lock = threading.Lock() + self._state_lock = threading.Lock() @@ def run(self, coro: Coroutine, *, timeout: float | None = None) -> Any: - loop = self._loop - if self._closing or loop is None or not loop.is_running(): - coro.close() - raise RuntimeError('laserdata: node is not open') - budget = self.op_timeout if timeout is None else timeout - future = asyncio.run_coroutine_threadsafe(coro, loop) - pending = self._pending - if pending is not None: - pending.add(future) + with self._state_lock: + loop = self._loop + if self._closing or loop is None or not loop.is_running(): + coro.close() + raise RuntimeError('laserdata: node is not open') + future = asyncio.run_coroutine_threadsafe(coro, loop) + pending = self._pending + if pending is not None: + pending.add(future) + budget = self.op_timeout if timeout is None else timeout try: return future.result(budget) @@ def endGlobal(self) -> None: - self._closing = True - laser, self._laser = self._laser, None - loop, self._loop = self._loop, None - thread, self._loop_thread = self._loop_thread, None - pending, self._pending = self._pending, None + with self._state_lock: + self._closing = True + laser, self._laser = self._laser, None + loop, self._loop = self._loop, None + thread, self._loop_thread = self._loop_thread, None + pending, self._pending = self._pending, None🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nodes/src/nodes/tool_laserdata_memory/IGlobal.py` around lines 120 - 146, Make the check, future creation, and registration in IGlobal.run atomic with endGlobal’s _closing/_pending swap by protecting both sequences with the same synchronization mechanism. Ensure a run either registers its future before teardown swaps the pending set, or observes closing and fails immediately; preserve existing cancellation, timeout, cleanup, and coroutine-closing behavior.
184-192: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick winScrub the close-failure warning before logging.
Interpolating
{e}can propagate the LaserData connection string and bare password into pipe logs.IInstance.pyalready uses_safe_error(cfg, exc)for SDK errors after connect/run; move it toIGlobal.py(where it can accesscfg.connection_string) and use it inendGlobalbeforewarning(...). There is noIInstanceimport fromIGlobal, so this does not create a circular import.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@nodes/src/nodes/tool_laserdata_memory/IGlobal.py` around lines 184 - 192, Sanitize the close exception in endGlobal before logging instead of interpolating the raw exception. Move or reuse the existing _safe_error(cfg, exc) helper from IInstance.py in IGlobal.py, passing the available cfg.connection_string context, and use the sanitized result in the laserdata close-failure warning.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@nodes/test/tool_laserdata_memory/test_tools.py`:
- Around line 601-604: The existing tests do not cover credential redaction when
connection cleanup fails. After fixing the close-failure handling in
IGlobal.endGlobal, add a test alongside test_sdk_error_scrubbed_of_credentials
that makes the fake laser’s __aexit__ raise an exception containing the
password, then assert the resulting warning omits the credential and preserves
the expected scrubbed error behavior.
---
Outside diff comments:
In `@nodes/src/nodes/tool_laserdata_memory/IGlobal.py`:
- Around line 120-146: Make the check, future creation, and registration in
IGlobal.run atomic with endGlobal’s _closing/_pending swap by protecting both
sequences with the same synchronization mechanism. Ensure a run either registers
its future before teardown swaps the pending set, or observes closing and fails
immediately; preserve existing cancellation, timeout, cleanup, and
coroutine-closing behavior.
- Around line 184-192: Sanitize the close exception in endGlobal before logging
instead of interpolating the raw exception. Move or reuse the existing
_safe_error(cfg, exc) helper from IInstance.py in IGlobal.py, passing the
available cfg.connection_string context, and use the sanitized result in the
laserdata close-failure warning.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 32b08885-42c6-4122-861d-2c3f3ebd79b5
📒 Files selected for processing (3)
nodes/src/nodes/tool_laserdata_memory/IGlobal.pynodes/src/nodes/tool_laserdata_memory/IInstance.pynodes/test/tool_laserdata_memory/test_tools.py
There was a problem hiding this comment.
Approving. I scoped my review to the two things most likely to bite in production rather than re-reading what Alexandru already covered: the dependency, and the credential.
First, the practical blocker — Alexandru's approval is gone. He approved at 06:36 PT; the pushes at 10:26 and 10:48 dismissed it, because dismiss_stale_reviews is on. Nothing was wrong with either. This approval replaces it, and it will vanish the same way if anything else is pushed.
The dependency is safer than the PR describes
The commit says "un-pin to laser-sdk rc21", and I want to correct that in the record because it matters for anyone deciding whether to ship this: it is pinned, exactly, at laser-sdk==0.0.1rc21. That is the strongest form available here — the package has no stable release at all, only 0.0.1rc3 through rc21, so there is nothing more conservative to pin to.
I checked the two ways this could still hurt us, since depends() resolves every node's requirements in a single solve and one bad pin takes down all of them, not just this node:
1. Does an exact prerelease pin need --prerelease=allow? No. Resolved it directly:
$ uv pip compile req.txt # laser-sdk==0.0.1rc21, no flags
laser-sdk==0.0.1rc21
An explicit == to a prerelease enables prereleases for that package alone, so the shared solve is unaffected.
2. Can it conflict with another node? No — and this is the strongest thing about it. Every entry in requires_dist sits behind extra == "testing":
pytest, pytest-asyncio, pytest-bdd, pytest-timeout, ruff -- all extra == "testing"
So a plain install pulls in zero runtime dependencies. There is no version of any shared library it can disagree with. That makes this about the safest node addition possible on the dependency axis.
It also ships abi3 wheels — cp310-abi3 for macOS (x86_64, arm64, universal2), manylinux x86_64 and aarch64, and win_amd64. Worth stating explicitly for a PyO3 package: nothing builds from source on any platform we run, so no Rust toolchain is needed at install time and the multi-OS runners are covered.
The one residual risk is inherent and not yours to fix: a prerelease can be yanked, and the header already documents that rc20 changed the wire protocol. Since dependencies are still resolved at pod start rather than baked (saas #443), a yank would surface as a start failure. That is an argument for #443, not against this PR.
The credential choice is right, and someone will "fix" it
beginGlobal falls back to os.environ.get('LASER_CONNECTION_STRING'), documented as user:password@host:port. Every other tool node uses a ROCKETRIDE_-prefixed variable — tool_tavily has ROCKETRIDE_TAVILY_KEY — so this reads at first glance like an oversight, and the obvious review note is "rename it for consistency."
Do not. tool_tavily's own comment says why: "Pipeline env vars must be ROCKETRIDE_-prefixed (only those are substituted)." Substitution is the point — ROCKETRIDE_* is the namespace a pipeline author can reference. A connection string with an embedded password does not belong in a namespace designed to be referenced from pipeline text; an un-prefixed variable read straight from the process environment does.
So the un-prefixed name is the correct call. Two consequences worth accepting deliberately rather than discovering later:
${LASER_CONNECTION_STRING}will not expand inside a pipeline. The node readsos.environdirectly, so it works when set on the process — that is the intended path and the only one that should work.- The node-test framework maps
ROCKETRIDE_<PROVIDER>_<ATTR>onto config, so this variable is outside that mechanism. Worth confirming the tests configure it explicitly rather than relying on the mapping.
My one request: put a sentence next to that os.environ.get saying it is un-prefixed on purpose, because prefixed variables are pipeline-referenceable. Without it, the next person reads an inconsistency, "fixes" it, and moves a database credential into the wrong namespace. A one-line comment prevents that.
On deploying it
Happy to help land it, but I am not going to push it to production off a relayed instruction — I will confirm the target with Rod directly, since he asked me on Wednesday to hold production deploys. Note also that merging to develop by itself deploys nothing, so "merged" and "deployed" are two separate steps here. More in my reply to you.
3 participants
Summary
tool_laserdata_memory— an agent-invocable memory tool node (classType: ["tool"],lanes: {}) backed by LaserData's Laser SDK memory primitive over Apache Iggy; mirrorstool_mem0/tool_cognee/tool_xtrace_memory.remember/recall/improve/forget— durable, shared,namespace-scoped agent memory with per-call conversation scoping.
IGlobal(daemon thread), lazy single-flight connect,run_coroutine_threadsafewith aconfigurable timeout; errors raise (
ValueError/RuntimeError), never error dicts.with mode-specific connection help. All auth travels in the connection string — no token
field (verified against a live Cloud deployment).
laser-sdkpinned to0.0.1rc19deliberately: rc20+ wheels speak only Iggy's VSRcluster protocol, which neither LaserData Cloud nor stock Apache Iggy serve today. rc19
completes the full round-trip live; the memory API is signature-identical to rc21
(rationale in requirements.txt + README; re-pin upward when their deployments answer VSR).
Type
feature (new agent-tool node) + docs + tests
Testing
nodes/test/tool_laserdata_memory/, stubharness, no engine/SDK needed) + contract coverage (316 pass with the node discovered)
remember → recall → improve → forget round-trip with tombstone verification against a
real LaserData Cloud deployment, both direct SDK and end-to-end through
chat UI →
agent_deepagent→ nodepin rationale, upstream links) with
ROCKETRIDE:GENERATED:PARAMSmarker blockChecklist
nodes/test/only)Notes for reviewers
"requires": ["LASER_CONNECTION_STRING"], so the framework only generates the case whenthat env var is present (a dev machine with a live server) and omits it otherwise —
the declarative equivalent of the conftest
skip_nodesentries thattool_mem0/tool_xtrace_memoryuse, with no shared-code edit.*.laserdata.cloudTLSauto-attach doesn't fire (
?tls=truerequired); LaserData Cloud does not yet answer VSR.Linked Issue
Closes #1733
Summary by CodeRabbit
New Features
Documentation
Tests