Fix multi-user agent publishing after language controller refactor#711
Merged
Fix multi-user agent publishing after language controller refactor#711
Conversation
Needed for multi-agent feature. Got lost during LanguageController refactor because mulit-user test were false positive in CI
…ors - all classes loaded by all - adjust test fixture
Contributor
📝 WalkthroughWalkthroughConsolidates agent signing and publishing into a context-aware API, adds per-thread runtime AgentContext and propagation through language execution, updates JS bindings and GraphQL flows to use runtime context, and adjusts tests/scripts to use test:// URIs and propagate real exit codes. Changes
Sequence DiagramsequenceDiagram
participant GQL as GraphQL
participant AgentSvc as AgentService
participant LangCtrl as LanguageController
participant LangRT as LanguageRuntime
participant Script as Script/AGENT
GQL->>AgentSvc: publish_agent_to_language(context)
AgentSvc->>AgentSvc: get_agent_for_context(context)
AgentSvc->>AgentSvc: agent_to_publish_json(agent)
AgentSvc->>LangCtrl: execute_on_language_with_context(script, context)
LangCtrl->>LangRT: execute_with_context(script, context)
LangRT->>LangRT: set_runtime_agent_context(context)
LangRT->>Script: run script (reads get_runtime_agent_context())
Script-->>LangRT: execution result
LangRT->>LangRT: restore main_agent context
LangRT-->>LangCtrl: result
LangCtrl-->>AgentSvc: result
AgentSvc-->>GQL: success/error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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)
rust-executor/src/languages/language_runtime_handle.rs (1)
74-82:⚠️ Potential issue | 🟠 MajorAvoid logging full
LanguageOperationdebug payloads.Line 75 now serializes
ExecutewithAgentContext, which can expose managed-user emails (and script content) in logs.🔐 Suggested safe logging change
- let op_name = format!("{:?}", operation); + let op_name = match &operation { + LanguageOperation::Execute(_, _) => "Execute", + LanguageOperation::LoadModule(_) => "LoadModule", + LanguageOperation::LoadLanguage(_) => "LoadLanguage", + LanguageOperation::RegisterCallbacks => "RegisterCallbacks", + LanguageOperation::Teardown => "Teardown", + }; @@ - &op_name[..op_name.len().min(100)] + op_name );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust-executor/src/languages/language_runtime_handle.rs` around lines 74 - 82, The current debug log in send serializes the full LanguageOperation (op_name) which may expose sensitive AgentContext fields; update the logging to avoid printing the full debug payload by matching on LanguageOperation inside send (or implement a safe display helper) and log only non-sensitive details (e.g., the operation variant name and a truncated id or safe flags) instead of the entire op_name string, then replace the debug call that references op_name with this sanitized summary while preserving language_address context.rust-executor/src/js_core/agent_extension.rs (1)
124-132:⚠️ Potential issue | 🟠 MajorMake
agent_sign_string_hexcontext-aware too.Line 131 still routes through
sign_string_hex(payload), which uses main-agent signing; managed-user calls can produce signatures from the wrong key.🛠️ Suggested fix
use crate::{ agent::{ create_signed_expression, did, did_document, did_for_context, sign_for_context, - sign_string_hex, signing_key_id_for_context, AgentContext, AgentService, + signing_key_id_for_context, AgentContext, AgentService, }, graphql::graphql_types::{Agent, AgentStatus}, }; @@ #[op2] #[string] fn agent_sign_string_hex(#[string] payload: String) -> Result<String, AnyhowWrapperError> { - sign_string_hex(payload).map_err(AnyhowWrapperError::from) + let ctx = get_runtime_agent_context(); + let payload_bytes = crate::agent::signatures::hash_message(&payload); + let signature = + sign_for_context(&payload_bytes, &ctx).map_err(AnyhowWrapperError::from)?; + Ok(hex::encode(signature)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust-executor/src/js_core/agent_extension.rs` around lines 124 - 132, The agent_sign_string_hex function currently calls the global sign_string_hex (main-agent key); change it to use a context-aware signing routine so managed-user requests use the runtime agent key. Replace the call to sign_string_hex(payload) with a context-aware variant (e.g., sign_string_hex_for_context(payload, &get_runtime_agent_context())) and propagate errors with map_err(AnyhowWrapperError::from); if such a helper doesn't exist, implement sign_string_hex_for_context to delegate to the context-aware signer (similar to sign_for_context) and use get_runtime_agent_context() inside agent_sign_string_hex.
🧹 Nitpick comments (3)
tests/js/tests/multi-user-simple.test.ts (1)
1241-1247: Add retry polling before asserting shared SDNA class visibility.Line 1243 asserts eventual cross-user propagation immediately; this can still flake under gossip timing variance.
As per coding guidelines: `tests/**/*.{js,ts}`: Add retry logic with appropriate timeouts to fix flaky tests related to cross-agent data visibility, rather than changing from Local to Network GetStrategy.💡 Suggested stabilization patch
+ const pollSubjectClasses = async (p: any, minCount: number, timeoutMs = 20000) => { + const start = Date.now(); + let classes: any[] = []; + while (Date.now() - start < timeoutMs) { + classes = await p.subjectClasses(); + if (classes.length >= minCount) return classes; + await sleep(500); + } + return classes; + }; + - let classesSeenByUser1 = await perspective1.subjectClasses() + let classesSeenByUser1 = await pollSubjectClasses(perspective1, 2); console.log("User 1 sees classes:", classesSeenByUser1); // In a shared neighbourhood, SDNA links propagate, so both users // eventually see both classes once sync completes. expect(classesSeenByUser1.length).to.equal(2); - let classesSeenByUser2 = await user2SharedPerspective!.subjectClasses() + let classesSeenByUser2 = await pollSubjectClasses(user2SharedPerspective!, 2); console.log("User 2 sees classes:", classesSeenByUser2); expect(classesSeenByUser2.length).to.equal(2);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/js/tests/multi-user-simple.test.ts` around lines 1241 - 1247, The test asserts shared SDNA visibility immediately and can flake; wrap the read for user2SharedPerspective.subjectClasses() (and the earlier classesSeenByUser1 check if needed) in a retry/polling loop that re-fetches subjectClasses() until classesSeenByUser2.length === 2 or a timeout is reached (use a sensible total timeout and poll interval), then assert the length; locate and modify the calls to user2SharedPerspective.subjectClasses() and the expect(classesSeenByUser2.length).to.equal(2) to perform the retry before making the final assertion.rust-executor/src/graphql/mutation_resolvers.rs (1)
552-564: Align publish-failure semantics across profile update branches.This main-agent branch now fails the mutation when publish fails, while the managed-user branch logs and continues. Consider unifying this behavior for predictable API semantics.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust-executor/src/graphql/mutation_resolvers.rs` around lines 552 - 564, The publish-to-language failure handling is inconsistent: the AgentService::publish_agent_to_language call for AgentContext::main_agent() converts failures into a FieldError and fails the mutation, while the managed-user branch only logs and continues; update the managed-user branch to mirror the main-agent behavior (or vice-versa if you prefer failing both) so API semantics are consistent—specifically, locate the call to AgentService::publish_agent_to_language for the managed-user context and change its error handling to map the error into a FieldError with a clear message (similar to the current main-agent map_err) rather than just logging and swallowing the error.rust-executor/src/languages/language_runtime.rs (1)
280-285: Restore previous context after execution instead of forcing main-agent.At Line 283, resetting unconditionally to
main_agent()works for today’s flow, but restoring the prior context is safer for future nested/context-stacked execution patterns.♻️ Proposed refactor
- LanguageOperation::Execute(script, ref agent_ctx) => { - set_runtime_agent_context(agent_ctx); - let r = self.execute(&script).await; - set_runtime_agent_context(&AgentContext::main_agent()); - r - } + LanguageOperation::Execute(script, ref agent_ctx) => { + let previous_ctx = get_runtime_agent_context(); + set_runtime_agent_context(agent_ctx); + let r = self.execute(&script).await; + set_runtime_agent_context(&previous_ctx); + r + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust-executor/src/languages/language_runtime.rs` around lines 280 - 285, Currently the code unconditionally resets context to AgentContext::main_agent() after LanguageOperation::Execute; instead capture the prior context before calling set_runtime_agent_context(agent_ctx), then await self.execute(&script) and restore the captured prior context via set_runtime_agent_context(&previous_ctx) so the original context is returned (and ensure the restore happens regardless of the execution result by restoring after the await and returning the result); reference LanguageOperation::Execute, set_runtime_agent_context, AgentContext::main_agent, and self.execute to locate and update the logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@rust-executor/src/js_core/agent_extension.rs`:
- Around line 124-132: The agent_sign_string_hex function currently calls the
global sign_string_hex (main-agent key); change it to use a context-aware
signing routine so managed-user requests use the runtime agent key. Replace the
call to sign_string_hex(payload) with a context-aware variant (e.g.,
sign_string_hex_for_context(payload, &get_runtime_agent_context())) and
propagate errors with map_err(AnyhowWrapperError::from); if such a helper
doesn't exist, implement sign_string_hex_for_context to delegate to the
context-aware signer (similar to sign_for_context) and use
get_runtime_agent_context() inside agent_sign_string_hex.
In `@rust-executor/src/languages/language_runtime_handle.rs`:
- Around line 74-82: The current debug log in send serializes the full
LanguageOperation (op_name) which may expose sensitive AgentContext fields;
update the logging to avoid printing the full debug payload by matching on
LanguageOperation inside send (or implement a safe display helper) and log only
non-sensitive details (e.g., the operation variant name and a truncated id or
safe flags) instead of the entire op_name string, then replace the debug call
that references op_name with this sanitized summary while preserving
language_address context.
---
Nitpick comments:
In `@rust-executor/src/graphql/mutation_resolvers.rs`:
- Around line 552-564: The publish-to-language failure handling is inconsistent:
the AgentService::publish_agent_to_language call for AgentContext::main_agent()
converts failures into a FieldError and fails the mutation, while the
managed-user branch only logs and continues; update the managed-user branch to
mirror the main-agent behavior (or vice-versa if you prefer failing both) so API
semantics are consistent—specifically, locate the call to
AgentService::publish_agent_to_language for the managed-user context and change
its error handling to map the error into a FieldError with a clear message
(similar to the current main-agent map_err) rather than just logging and
swallowing the error.
In `@rust-executor/src/languages/language_runtime.rs`:
- Around line 280-285: Currently the code unconditionally resets context to
AgentContext::main_agent() after LanguageOperation::Execute; instead capture the
prior context before calling set_runtime_agent_context(agent_ctx), then await
self.execute(&script) and restore the captured prior context via
set_runtime_agent_context(&previous_ctx) so the original context is returned
(and ensure the restore happens regardless of the execution result by restoring
after the await and returning the result); reference LanguageOperation::Execute,
set_runtime_agent_context, AgentContext::main_agent, and self.execute to locate
and update the logic.
In `@tests/js/tests/multi-user-simple.test.ts`:
- Around line 1241-1247: The test asserts shared SDNA visibility immediately and
can flake; wrap the read for user2SharedPerspective.subjectClasses() (and the
earlier classesSeenByUser1 check if needed) in a retry/polling loop that
re-fetches subjectClasses() until classesSeenByUser2.length === 2 or a timeout
is reached (use a sensible total timeout and poll interval), then assert the
length; locate and modify the calls to user2SharedPerspective.subjectClasses()
and the expect(classesSeenByUser2.length).to.equal(2) to perform the retry
before making the final assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9d576700-1a4f-4799-a9d7-8ee526337180
📒 Files selected for processing (10)
rust-executor/src/agent/mod.rsrust-executor/src/graphql/mutation_resolvers.rsrust-executor/src/js_core/agent_extension.rsrust-executor/src/js_core/language_bootstrap.jsrust-executor/src/languages/language_runtime.rsrust-executor/src/languages/language_runtime_handle.rsrust-executor/src/languages/mod.rstests/js/email-verification-test-with-setup.shtests/js/test-multi-user-with-setup.shtests/js/tests/multi-user-simple.test.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rust-executor/src/agent/mod.rs`:
- Around line 513-527: get_agent_for_context currently passes raw
AgentContext.user_email into AgentService::with_global_instance which calls
load_user_agent_profile that builds a filesystem path; validate/sanitize the
email before using it as a path segment to prevent path traversal. In
get_agent_for_context, before calling AgentService::with_global_instance(|svc|
svc.load_user_agent_profile(email)), call a validation helper (e.g.,
validate_user_path_segment or sanitize_user_email) that rejects or canonicalizes
inputs containing path separators, "..", null bytes, or other unsafe characters
(or enforces a strict regex for allowed username characters), and return an
error (Anyhow) when validation fails. Apply the same check for any other call
sites that feed user_email into filesystem paths (referenced symbols:
get_agent_for_context, AgentContext.user_email,
AgentService::with_global_instance, load_user_agent_profile).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7ab2a078-3f53-4350-a9a3-f628a5baf568
📒 Files selected for processing (2)
rust-executor/src/agent/mod.rsrust-executor/src/js_core/agent_extension.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- rust-executor/src/js_core/agent_extension.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rust-executor/src/agent/mod.rs`:
- Around line 517-523: publish_agent_to_language() currently loads the stored
profile via Self::get_agent_for_context(...) and publishes agent data with
controller.expression_create(...) without ensuring the profile DID matches the
signing context; add a consistency check after obtaining `agent` (from
get_agent_for_context) to compare the profile's DID (e.g., agent.did) with the
signing context's DID (e.g., context.did or appropriate context identity field)
and return an error (or bail) if they differ before calling
agent_to_publish_json(...) and controller.expression_create(...), so you never
sign under one DID and publish a different agent DID.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ed742839-a1ac-43c3-9fe1-e953340bffd8
📒 Files selected for processing (1)
rust-executor/src/agent/mod.rs
jhweir
previously approved these changes
Mar 5, 2026
…after-lang-refactor
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
rust-executor/src/graphql/mutation_resolvers.rs (1)
214-217: Consider making publish failures observable to callers (or retrying).These paths currently warn and continue on publish failure, which can leave agent state unpublished while the mutation reports success. Aligning these with a retry/backoff or surfaced error would improve consistency.
💡 Minimal pattern for init flows
if let Err(e) = AgentService::publish_agent_to_language(&AgentContext::main_agent()).await { log::warn!("Error publishing agent expression: {}", e); + init_errors.push(format!("Agent publish failed: {}", e)); }Also applies to: 395-400, 453-458, 696-702
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rust-executor/src/graphql/mutation_resolvers.rs` around lines 214 - 217, The current calls to AgentService::publish_agent_to_language (e.g., the call using AgentContext::main_agent() in this mutation resolver, and the similar calls at the other noted locations) silently log failures and continue; change these so publish failures are observable to callers (or retried) by either propagating the error back through the resolver result or implementing a simple retry/backoff before logging and failing. Concretely, update the callsites that currently use if let Err(e) = AgentService::publish_agent_to_language(...).await { log::warn!(...) } to instead attempt a small retry loop (with exponential backoff) and return or map the final Err into the resolver's error return type (so the mutation surfaces failure), or if you prefer retries only, document and add a bounded retry with logging of the final error; ensure you update all occurrences of publish_agent_to_language and adjust the resolver return signature to propagate the error where necessary.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@rust-executor/src/agent/mod.rs`:
- Around line 517-524: The DID consistency check currently compares
AgentService.did (from get_agent_for_context) to did_for_context(context), but
signing uses the wallet key via sign_for_context, so change the comparison to
use the DID derived from the signing-key source instead of did_for_context;
locate the block in mod.rs around get_agent_for_context and replace the
right-hand side with the DID resolution used by sign_for_context (e.g., call the
helper that derives the wallet/signing-key DID or derive it the same way
sign_for_context does), and update the error message to indicate a signing-key
DID mismatch if they differ.
---
Nitpick comments:
In `@rust-executor/src/graphql/mutation_resolvers.rs`:
- Around line 214-217: The current calls to
AgentService::publish_agent_to_language (e.g., the call using
AgentContext::main_agent() in this mutation resolver, and the similar calls at
the other noted locations) silently log failures and continue; change these so
publish failures are observable to callers (or retried) by either propagating
the error back through the resolver result or implementing a simple
retry/backoff before logging and failing. Concretely, update the callsites that
currently use if let Err(e) = AgentService::publish_agent_to_language(...).await
{ log::warn!(...) } to instead attempt a small retry loop (with exponential
backoff) and return or map the final Err into the resolver's error return type
(so the mutation surfaces failure), or if you prefer retries only, document and
add a bounded retry with logging of the final error; ensure you update all
occurrences of publish_agent_to_language and adjust the resolver return
signature to propagate the error where necessary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4b2d01a7-a334-4759-b7b0-2b8bd79fdbb4
📒 Files selected for processing (3)
rust-executor/src/agent/mod.rsrust-executor/src/graphql/mutation_resolvers.rsrust-executor/src/languages/mod.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- rust-executor/src/languages/mod.rs
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
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
2 participants
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.
Nico's tl;dr:
multi-user and email tests did not propagate the exit code to the CI so they were green dispite failures.
Because of that, the LanguageController refactoring introduced failures: agent profiles were not written to the agent language for managed users. The language was not run with the right user context and the agent language refused to store a profile for someone else (the main agent).
This PR:
AgentContextthat gets set before each script execution so that language JS code dynamically resolves to the correct user's DID and signing keys. (we had that in the old JS LanguageController)Claude's verbose description:
Core fix — per-runtime agent context
language_runtime.rs: Addedthread_local! CURRENT_AGENT_CONTEXTwithset_runtime_agent_context()/get_runtime_agent_context()helpers. Inprocess_requests(), the context is set from the channel message before script execution and reset tomain_agent()afterward. ExtendedLanguageOperation::Executeto carry anAgentContext.language_runtime_handle.rs: Addedexecute_with_context(script, agent_context)method that passes the context through the channel alongside the script.agent_extension.rs: Changedagent_did(),agent_sign(),agent_signing_key_id(), andagent_create_signed_expression()Deno ops to read fromget_runtime_agent_context()instead of hardcodedAgentContext::main_agent().language_bootstrap.js: ChangedagentProxy.didandagentProxy.signingKeyIdfrom static values (cached at init time) to getters that callAGENT.did()/AGENT.signingKeyId()on every access, so they dynamically reflect the current thread-local context.mod.rs(languages): Addedexecute_on_language_with_context(). Updatedexpression_create()to route through this method, passing theagent_contextparameter that was previously ignored for non-literal languages.Agent service consolidation
agent/mod.rs: Unifiedensure_agent_expression(),publish_main_agent_to_language(), andpublish_user_agent_to_language()into a singlepublish_agent_to_language(context: &AgentContext). Addedagent_to_publish_json()which strips link decoration fields (proof.valid/proof.invalid/status) in Rust before publishing, rather than relying on the JS side.mutation_resolvers.rs: All 6 publish call sites now use the unifiedpublish_agent_to_language()with the appropriateAgentContext.Test script fix
test-multi-user-with-setup.shandemail-verification-test-with-setup.sh: Propagate the test process exit code instead of always exiting 0 (previously CI would report success even when tests failed).Test fixes
"user1"→"test://user1") to satisfy link source validation.Note
Medium Risk
Touches core signing/publishing paths and introduces thread-local context switching in language runtimes, which could cause incorrect authorship/signatures if context reset or concurrency assumptions break.
Overview
Fixes multi-user agent publishing after the language controller refactor by propagating
AgentContextthrough language runtime execution and making JS agent identifiers/signing resolve dynamically per request.Introduces a per-runtime thread-local agent context (
LanguageOperation::Execute(script, AgentContext)plusexecute_on_language_with_context) and updates Deno agent ops andlanguage_bootstrap.jsto use the runtime context so expressions are created/signed as the intended managed user.Consolidates agent profile publishing into
AgentService::publish_agent_to_language(context)with DID mismatch checks, strips decorated link fields before publishing, adds basic filesystem-path validation for user emails, and updates GraphQL mutation call sites accordingly. Test harness scripts now propagate exit codes, and multi-user tests are adjusted for stricter link URIs, shared SDNA propagation expectations, and more reliable cross-node sync via polling.Written by Cursor Bugbot for commit 703dc79. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes
Tests