Built-in and native tools by vigoo · Pull Request #3841 · golemcloud/golem

vigoo · 2026-09-08T08:17:04Z

Resolves GOL-24

netlify · 2026-09-08T08:17:09Z

✅ Deploy Preview for golemcloud canceled.

Name	Link
🔨 Latest commit	`dbd9ad0`
🔍 Latest deploy log	https://app.netlify.com/projects/golemcloud/deploys/6aa81a35b6c78d0008e9b2b5

github-actions · 2026-09-08T08:22:31Z

📖 Docs preview: https://docs-5ygaf8kbc-golem-cloud.vercel.app

Built from commit dbd9ad0de81c75b019d8e6bdce532243052d96cf by docs.yaml.

Amp-Thread-ID: https://ampcode.com/threads/T-01a07abb-341e-76cc-9a80-cbd07ef73745 Co-authored-by: Amp <amp@ampcode.com>

Amp-Thread-ID: https://ampcode.com/threads/T-01a07abb-341e-76cc-9a80-cbd07ef73745 Co-authored-by: Amp <amp@ampcode.com> # Conflicts: # golem-worker-executor/src/worker/mod.rs

Amp-Thread-ID: https://ampcode.com/threads/T-01a07abb-341e-76cc-9a80-cbd07ef73745 Co-authored-by: Amp <amp@ampcode.com>

noise64 · 2026-09-14T11:55:00Z

+    let result = await_native_entity_body(
+        &runner_abort,
+        handler.invoke(retained.context_mut(), native_invocation),
+    )
+    .await;
+    retained
+        .context_mut()
+        .durable_ctx_mut()
+        .set_invocation_principal(None);
+    if let Err(error) = retained.prepare_parent_end().await {
+        return (Err(error), Some(Box::new(retained)));
+    }
+    let result = match result {
+        Ok(result) => {
+            if let Some(error) = stdout_limit_error(
+                stdout_observer
+                    .as_ref()
+                    .and_then(AttachmentObserver::terminal_snapshot)
+                    .is_some_and(|terminal| terminal.host_resource_exhausted),
+            ) {
+                encode_tool_operation_terminal(Err(error)).await
+            } else {
+                encode_tool_operation_terminal(result).await
+            }


Could we mirror the component invocation path's post-body has_pending_live_admission_rejection guard for native tools? A native handler can catch an attachment admission error and return success; this path then encodes an ordinary terminal, while the later accounting check does not replace that response with the pending resource-exhaustion failure. Please preserve the rejection before terminal encoding and cover a handler that catches the host error.

noise64 · 2026-09-14T11:55:00Z

+    async fn invoke(
+        &self,
+        ctx: &mut Ctx,
+        invocation: NativeToolInvocation,
+    ) -> Result<NativeToolResult, WorkerExecutorError> {
+        let result = self
+            .0
+            .invoke(
+                ctx,
+                golem_native_tool::NativeToolInvocation {
+                    command_path: invocation.command_path,
+                    input: invocation.input,
+                    principal: invocation.principal,
+                    stdin: invocation.stdin.map(|input| Box::new(input) as _),
+                    stdout: invocation.stdout.map(|output| Box::new(output) as _),
+                },
+            )


Could we expose and forward a safe cancellation handle through golem_native_tool::NativeToolInvocation? The executor invocation carries a CancellationToken, but NativeToolAdapter constructs the public invocation without it, so macro/SDK-authored native tools cannot observe cancellation even though direct executor handlers can. A cancellation test through the adapter would pin the supported authoring path.

noise64 · 2026-09-14T11:55:00Z

+    retained
+        .context_mut()
+        .durable_ctx_mut()
+        .set_invocation_principal(None);
+    if let Err(error) = retained.prepare_parent_end().await {
+        return (Err(error), Some(Box::new(retained)));
+    }
+    let result = match result {


Could we preserve the primary handler or infrastructure error when prepare_parent_end also fails, matching the component path? The native path currently returns the cleanup failure before examining the handler result, which can mask the failure that caused the invocation to abort. Substituting the cleanup error only when the body succeeded would retain the more useful diagnosis.

noise64 · 2026-09-14T11:55:00Z

+            if implementations.contains_key(&ambient.name)
+                || app.tool_declarations().contains_key(&ambient.name)


What is the intended rollout policy when a future Golem release introduces an ambient tool whose name is already used by an application or selected account-owned tool? AFAIU existing deployments remain intact, but the next deployment will fail. I guess this is fine (and similar to account shared ones), just wanted to double check.

noise64 · 2026-09-14T11:55:00Z

@@ -1 +1 @@
-9b8236ff15b23a2da0c9bbaaa28149cd9875f93d6a519dab7b5ed98c8dbdfd75
+8432554acdeedaabbb278c764a8a66243102ea9197309ffa6248071564c9a3c3


This should bump the diff model version, and create a v6 fingerprint

noise64 · 2026-09-14T11:55:00Z

+        // Ambient tools are registry-owned input. Resolve them before local validation and
+        // hashing; planning remains read-only.
+        let ambient_plan = self
+            .ctx
+            .golem_clients()
+            .await?
+            .environment
+            .get_environment_deployment_plan(&environment.environment_id.0)
+            .await
+            .map_service_error()?;


prepare_deployment now fetches the same deployment plan twice; I think one read should be enough (also the first read calls it ambient_plan, which is not entirely correct)

ambientTools appears to be the first field in this DTO that is not information identifying or describing the staged diff side itself, but is auxiliary registry metadata used to construct the proposed side, so i think it would be clearer to separate it explicitly in the response DTO (or have some comment on it / naming convention for possible future similar ones?)

noise64 · 2026-09-14T11:55:00Z

+#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
+#[serde(untagged)]
+pub enum ManifestConfigKeyScope {
+    All(String),
+    Keys(Vec<String>),
+}


Some of our checks are only visible in the JSON schema, but that is extra meta, not the main validation we use in the CLI:

The JSON Schema constraints are useful diagnostics, but they are not currently enforced by Rust when the manifest deserializes successfully: the untagged scope enums accept any string and any string list, while path validation happens only during deployment. Please enforce the wildcard/list shape through serde and validate every path while loading/building the application in app_raw.rs or app.rs. The same issue exists for the pre-existing secret scopes, and the generic audit also found that tool-binding keys rely on the schema regex without equivalent Rust load-time validation.

noise64 · 2026-09-14T11:55:00Z

+          "items": {
+            "type": "string",
+            "not": {
+              "const": "*"
+            }
+          }
+        }
+      ]
+    },


It seems we keep modifying these dev one, unlike how they were originally intended to be used. I guess this can be the new norm, but we might want to adjust the skills so we do not modify non dev versions. And we might want to think about automating the publishing of these, to guarantee no update on the non dev one (not in this PR, just want to mention).

Amp-Thread-ID: https://ampcode.com/threads/T-01a07abb-341e-76cc-9a80-cbd07ef73745 Co-authored-by: Amp <amp@ampcode.com>

vigoo · 2026-09-08T10:10:37Z

+    pub struct DeploymentPlanAmbientToolEntry {
+        pub release_id: crate::model::tool_release::ToolReleaseId,
+        pub name: ToolName,
+        pub version: String,


Define either a new transparent newtype for this like DeploymentVersion or use that, if appropriate. Same applies for metadata_version below.

vigoo · 2026-09-09T08:07:29Z

+#[test]
+async fn can_extract_tool_metadata_from_component_bytes() -> anyhow::Result<()> {
+    let wasm = std::fs::read(
+        "../test-components/tool-streaming/golem-temp/agents/golem_it_tool_streaming_rust_provider_release.wasm",


Should not use a temp build dir - use the wasm from ../test-components where the test component build script places it

vigoo · 2026-09-09T08:08:13Z

    pub environment_state_service: Arc<EnvironmentStateService>,
    pub http_api_deployment_service: Arc<HttpApiDeploymentService>,
    pub mcp_deployment_service: Arc<McpDeploymentService>,
+    pub native_tool_catalog: Arc<crate::services::native_tool_catalog::NativeToolCatalog>,


Why not import the trait - don't use fully qualified paths where not necessary (not just this line)

vigoo · 2026-09-09T08:38:19Z

+        let mut registrations = vec![NativeToolRegistration {
+            definition: native_test_helper_definition(),
+            handler: Arc::new(NativeToolAdapter(
+                __GolemNativeToolInvokerNativeDurableHelperImplNativeDurableHelper::new(


What is this horrible type name and (if it is macro generated) why do we need to explicitly use here? Let's avoid that

vigoo · 2026-09-09T08:48:29Z

+/// conformance of these literals against their referenced type node is checked
+/// separately by [`validate_tool`], which has the per-command field context
+/// needed to resolve `value-is` references.
+#[cfg(any())]


There are 12 #[cfg(any())] blocks in this file (none existed before the move). That is dead code that is merely compiled out — delete it instead. We do not keep old code paths around (see the backward-compatibility section in AGENTS.md).

vigoo · 2026-09-09T08:49:35Z

+            &deployment_write_service,
+            &tool_release_service,
+        )
+        .await


Built-in tool provisioning failure aborts registry startup here, while built-in plugin provisioning just above only warns. Pick one behaviour deliberately for both. Hard-failing is arguably right for protected system releases, but then the plugin path should match (or the difference should be documented).

vigoo · 2026-09-09T08:49:37Z

@@ -120,11 +120,6 @@ impl ToolReleaseRecord {
                "system tool release coordinate does not match its definition"


The "protected system tool releases must use a host source" check was removed here so that component-implemented built-ins can also be protected system releases. Nothing now enforces the source/availability pairing though: Ambient ⇒ Host source and Grantable ⇒ Component source. Please reinstate that invariant in its new form (here or in ToolReleaseService::provision_system_release).

vigoo · 2026-09-09T08:49:39Z


+static NATIVE_TEST_EFFECTS: AtomicUsize = AtomicUsize::new(0);
+static NATIVE_TEST_HELPER_EFFECTS: AtomicUsize = AtomicUsize::new(0);
+static NATIVE_TEST_TOOL_METADATA: OnceLock<golem_common::schema::tool::Tool> = OnceLock::new();


Process-global mutable state in test utils (OnceLock + install_native_test_tool_metadata) makes the native test tool registration depend on which test ran first in the process and leaks between tests. Register test tools explicitly on the test context/catalog instead — this goes away naturally once the catalog is a bootstrap-time service passed into the context rather than a static fn.

vigoo · 2026-09-09T08:49:40Z

+        let ordinary_remote_tools = data
            .remote_tools
+            .iter()
+            .filter(|deployment| !ambient_catalog.contains_key(&deployment.name))


The CLI rejects a top-level tools entry whose name collides with an ambient tool, but the API silently accepts it: the user's release/provision/environment_binding is dropped here and replaced with the ambient one, and only agent_bindings survive. The server should reject the collision too (or at least error when the requested release differs from the ambient release) instead of rewriting the request.

vigoo · 2026-09-09T08:49:42Z

+                    crate::services::environment_tool_grant::ResolvedGrantedToolRelease {
+                        owner: AccountSummary {
+                            id: release.owner_account_id,
+                            name: "Golem system".to_string(),


Hardcoded display name; take it from the actual built-in tool owner account instead of a literal.

Amp-Thread-ID: https://ampcode.com/threads/T-01a07abb-341e-76cc-9a80-cbd07ef73745 Co-authored-by: Amp <amp@ampcode.com>

Built-in and native tools

4425099

vigoo and others added 3 commits September 9, 2026 15:29

Address built-in and native tool review findings

705f373

Amp-Thread-ID: https://ampcode.com/threads/T-01a07abb-341e-76cc-9a80-cbd07ef73745 Co-authored-by: Amp <amp@ampcode.com>

Merge remote-tracking branch 'origin/main' into gol-24

63a0c37

Amp-Thread-ID: https://ampcode.com/threads/T-01a07abb-341e-76cc-9a80-cbd07ef73745 Co-authored-by: Amp <amp@ampcode.com> # Conflicts: # golem-worker-executor/src/worker/mod.rs

Fix CI builds and all-features tool metadata tests

c4b16c8

Amp-Thread-ID: https://ampcode.com/threads/T-01a07abb-341e-76cc-9a80-cbd07ef73745 Co-authored-by: Amp <amp@ampcode.com>

vigoo marked this pull request as ready for review September 9, 2026 15:15

vigoo requested a review from a team September 9, 2026 15:15

vigoo and others added 2 commits September 9, 2026 18:10

Synchronize lifecycle regression tests and refresh diff fingerprint

ad21a6c

Amp-Thread-ID: https://ampcode.com/threads/T-01a07abb-341e-76cc-9a80-cbd07ef73745 Co-authored-by: Amp <amp@ampcode.com>

Merge latest main into gol-24

7503b34

Amp-Thread-ID: https://ampcode.com/threads/T-01a07abb-341e-76cc-9a80-cbd07ef73745 Co-authored-by: Amp <amp@ampcode.com>

noise64 reviewed Sep 14, 2026

View reviewed changes

vigoo and others added 2 commits September 14, 2026 16:12

Address native tool cancellation and deployment review feedback

2d6709c

Amp-Thread-ID: https://ampcode.com/threads/T-01a07abb-341e-76cc-9a80-cbd07ef73745 Co-authored-by: Amp <amp@ampcode.com>

Merge main and retain Effect manifest support in extracted tests

d0b9d3b

Amp-Thread-ID: https://ampcode.com/threads/T-01a07abb-341e-76cc-9a80-cbd07ef73745 Co-authored-by: Amp <amp@ampcode.com>

vigoo commented Sep 14, 2026

View reviewed changes

Wait for initialization before interruption test oplog baseline

dbd9ad0

Amp-Thread-ID: https://ampcode.com/threads/T-01a07abb-341e-76cc-9a80-cbd07ef73745 Co-authored-by: Amp <amp@ampcode.com>

noise64 approved these changes Sep 14, 2026

View reviewed changes

vigoo merged commit 38299ee into main Sep 14, 2026
71 checks passed

vigoo deleted the gol-24 branch September 14, 2026 17:55

github-actions Bot locked and limited conversation to collaborators Sep 14, 2026

		if implementations.contains_key(&ambient.name)
		\|\| app.tool_declarations().contains_key(&ambient.name)

		@@ -1 +1 @@
		9b8236ff15b23a2da0c9bbaaa28149cd9875f93d6a519dab7b5ed98c8dbdfd75
		8432554acdeedaabbb278c764a8a66243102ea9197309ffa6248071564c9a3c3

		@@ -120,11 +120,6 @@ impl ToolReleaseRecord {
		"system tool release coordinate does not match its definition"

Uh oh!

Conversation

vigoo commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

netlify Bot commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for golemcloud canceled.

Uh oh!

github-actions Bot commented Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vigoo commented Sep 8, 2026 •

edited

Loading

netlify Bot commented Sep 8, 2026 •

edited

Loading

github-actions Bot commented Sep 8, 2026 •

edited

Loading