diff --git a/backend/.sqlx/query-1564e75bd7888dc54baec981bf9d855df12d32b0102c38b15e5f81162a00a66b.json b/backend/.sqlx/query-1564e75bd7888dc54baec981bf9d855df12d32b0102c38b15e5f81162a00a66b.json deleted file mode 100644 index 315816c8dc..0000000000 --- a/backend/.sqlx/query-1564e75bd7888dc54baec981bf9d855df12d32b0102c38b15e5f81162a00a66b.json +++ /dev/null @@ -1,23 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "SELECT resource_type = 'ai_agent' FROM resource WHERE workspace_id = $1 AND path = $2", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "?column?", - "type_info": "Bool" - } - ], - "parameters": { - "Left": [ - "Text", - "Text" - ] - }, - "nullable": [ - null - ] - }, - "hash": "1564e75bd7888dc54baec981bf9d855df12d32b0102c38b15e5f81162a00a66b" -} diff --git a/backend/.sqlx/query-15f53f40a49b538ca0d4456f69b8d801e8c121f1e0cd904a77ce55bcf9bc5053.json b/backend/.sqlx/query-15f53f40a49b538ca0d4456f69b8d801e8c121f1e0cd904a77ce55bcf9bc5053.json new file mode 100644 index 0000000000..6510374dfc --- /dev/null +++ b/backend/.sqlx/query-15f53f40a49b538ca0d4456f69b8d801e8c121f1e0cd904a77ce55bcf9bc5053.json @@ -0,0 +1,29 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT resource_type, value AS \"value: sqlx::types::Json>\"\n FROM resource WHERE workspace_id = $1 AND path = $2", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "resource_type", + "type_info": "Varchar" + }, + { + "ordinal": 1, + "name": "value: sqlx::types::Json>", + "type_info": "Jsonb" + } + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [ + false, + true + ] + }, + "hash": "15f53f40a49b538ca0d4456f69b8d801e8c121f1e0cd904a77ce55bcf9bc5053" +} diff --git a/backend/.sqlx/query-1d5c68d62b35386ada241fe84a511e33926d151d1a916dfa6c0959217b0232b3.json b/backend/.sqlx/query-9b88afcbecd66e5e29658463a2b8c2aa44d714bcc95e242ef329f6fd71c345c1.json similarity index 81% rename from backend/.sqlx/query-1d5c68d62b35386ada241fe84a511e33926d151d1a916dfa6c0959217b0232b3.json rename to backend/.sqlx/query-9b88afcbecd66e5e29658463a2b8c2aa44d714bcc95e242ef329f6fd71c345c1.json index 436f77a7b3..08dfcfd6bd 100644 --- a/backend/.sqlx/query-1d5c68d62b35386ada241fe84a511e33926d151d1a916dfa6c0959217b0232b3.json +++ b/backend/.sqlx/query-9b88afcbecd66e5e29658463a2b8c2aa44d714bcc95e242ef329f6fd71c345c1.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "SELECT resource.workspace_id, resource.path, resource.value, resource.description,\n resource.resource_type, resource.extra_perms, resource.created_by, resource.edited_at,\n resource.labels,\n folder_labels(resource.workspace_id, resource.path) as \"inherited_labels?\",\n (now() > account.expires_at) as is_expired, account.refresh_token != '' as is_refreshed,\n account.refresh_error,\n variable.path IS NOT NULL as is_linked,\n variable.is_oauth as \"is_oauth?\",\n variable.account,\n ws_specific.path IS NOT NULL as ws_specific,\n null::bool as draft_only,\n null::bool as is_draft,\n resource.on_behalf_of\n FROM resource\n LEFT JOIN variable ON variable.path = resource.path AND variable.workspace_id = $2\n LEFT JOIN account ON variable.account = account.id AND account.workspace_id = $2\n LEFT JOIN ws_specific ON ws_specific.path = resource.path AND ws_specific.workspace_id = $2 AND ws_specific.item_kind = 'resource'\n WHERE resource.path = $1 AND resource.workspace_id = $2", + "query": "SELECT resource.workspace_id, resource.path, resource.value, resource.description,\n resource.resource_type, resource.extra_perms, resource.created_by, resource.edited_at,\n resource.labels,\n folder_labels(resource.workspace_id, resource.path) as \"inherited_labels?\",\n (now() > account.expires_at) as is_expired, account.refresh_token != '' as is_refreshed,\n account.refresh_error,\n variable.path IS NOT NULL as is_linked,\n variable.is_oauth as \"is_oauth?\",\n variable.account,\n ws_specific.path IS NOT NULL as ws_specific,\n null::bool as draft_only,\n null::bool as is_draft\n FROM resource\n LEFT JOIN variable ON variable.path = resource.path AND variable.workspace_id = $2\n LEFT JOIN account ON variable.account = account.id AND account.workspace_id = $2\n LEFT JOIN ws_specific ON ws_specific.path = resource.path AND ws_specific.workspace_id = $2 AND ws_specific.item_kind = 'resource'\n WHERE resource.path = $1 AND resource.workspace_id = $2", "describe": { "columns": [ { @@ -97,11 +97,6 @@ "ordinal": 18, "name": "is_draft", "type_info": "Bool" - }, - { - "ordinal": 19, - "name": "on_behalf_of", - "type_info": "Varchar" } ], "parameters": { @@ -129,9 +124,8 @@ true, null, null, - null, - true + null ] }, - "hash": "1d5c68d62b35386ada241fe84a511e33926d151d1a916dfa6c0959217b0232b3" + "hash": "9b88afcbecd66e5e29658463a2b8c2aa44d714bcc95e242ef329f6fd71c345c1" } diff --git a/backend/.sqlx/query-c6c5e40893aa766cd55af5d65d5a19ac213f3a99656e6efea94d13c9af4d904a.json b/backend/.sqlx/query-c6c5e40893aa766cd55af5d65d5a19ac213f3a99656e6efea94d13c9af4d904a.json deleted file mode 100644 index 2c7a534092..0000000000 --- a/backend/.sqlx/query-c6c5e40893aa766cd55af5d65d5a19ac213f3a99656e6efea94d13c9af4d904a.json +++ /dev/null @@ -1,16 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "UPDATE resource SET on_behalf_of = $1 WHERE workspace_id = $2 AND path = $3", - "describe": { - "columns": [], - "parameters": { - "Left": [ - "Varchar", - "Text", - "Text" - ] - }, - "nullable": [] - }, - "hash": "c6c5e40893aa766cd55af5d65d5a19ac213f3a99656e6efea94d13c9af4d904a" -} diff --git a/backend/.sqlx/query-38a233bd5816deb70773796a682a5bceb415da87b69296f9ac50679392274f2d.json b/backend/.sqlx/query-f3a740394038ddfced97254d8dcd4de49f63fb6dc4c8aab126dbe7cd611a862c.json similarity index 55% rename from backend/.sqlx/query-38a233bd5816deb70773796a682a5bceb415da87b69296f9ac50679392274f2d.json rename to backend/.sqlx/query-f3a740394038ddfced97254d8dcd4de49f63fb6dc4c8aab126dbe7cd611a862c.json index 182457e96e..6ee5709618 100644 --- a/backend/.sqlx/query-38a233bd5816deb70773796a682a5bceb415da87b69296f9ac50679392274f2d.json +++ b/backend/.sqlx/query-f3a740394038ddfced97254d8dcd4de49f63fb6dc4c8aab126dbe7cd611a862c.json @@ -1,17 +1,12 @@ { "db_name": "PostgreSQL", - "query": "SELECT value AS \"value: sqlx::types::Json\", on_behalf_of\n FROM resource WHERE workspace_id = $1 AND path = $2 AND resource_type = 'ai_agent'", + "query": "SELECT value AS \"value: sqlx::types::Json\"\n FROM resource WHERE workspace_id = $1 AND path = $2 AND resource_type = 'ai_agent'", "describe": { "columns": [ { "ordinal": 0, "name": "value: sqlx::types::Json", "type_info": "Jsonb" - }, - { - "ordinal": 1, - "name": "on_behalf_of", - "type_info": "Varchar" } ], "parameters": { @@ -21,9 +16,8 @@ ] }, "nullable": [ - true, true ] }, - "hash": "38a233bd5816deb70773796a682a5bceb415da87b69296f9ac50679392274f2d" + "hash": "f3a740394038ddfced97254d8dcd4de49f63fb6dc4c8aab126dbe7cd611a862c" } diff --git a/backend/migrations/20260925112726_agent_on_behalf_of.down.sql b/backend/migrations/20260925112726_agent_on_behalf_of.down.sql index b876f35e17..db77ee094c 100644 --- a/backend/migrations/20260925112726_agent_on_behalf_of.down.sql +++ b/backend/migrations/20260925112726_agent_on_behalf_of.down.sql @@ -1,3 +1,4 @@ -DROP TRIGGER IF EXISTS reset_agent_on_behalf_of ON resource; -DROP FUNCTION IF EXISTS reset_agent_on_behalf_of(); -ALTER TABLE resource DROP COLUMN IF EXISTS on_behalf_of; +ALTER TABLE resource DISABLE TRIGGER record_resource_version_update_trigger; +UPDATE resource SET value = value - 'on_behalf_of' + WHERE resource_type = 'ai_agent' AND jsonb_typeof(value) = 'object' AND value ? 'on_behalf_of'; +ALTER TABLE resource ENABLE TRIGGER record_resource_version_update_trigger; diff --git a/backend/migrations/20260925112726_agent_on_behalf_of.up.sql b/backend/migrations/20260925112726_agent_on_behalf_of.up.sql index f0077becaf..5c047a9d11 100644 --- a/backend/migrations/20260925112726_agent_on_behalf_of.up.sql +++ b/backend/migrations/20260925112726_agent_on_behalf_of.up.sql @@ -1,27 +1,7 @@ --- The identity an AI agent runs as when someone runs it from its own page, as a flow's or a --- script's `on_behalf_of` is: a principal (`u/`, `g/`), resolved at deploy time. --- Only `ai_agent` resources read it; every other resource type leaves it NULL. -ALTER TABLE resource ADD COLUMN IF NOT EXISTS on_behalf_of VARCHAR(255); - --- An agent's value is code it runs (inline tools, instructions), so a write of it that does not --- say who the agent now runs as must not keep running it as whoever wrote the previous one: --- anyone who can write the value would otherwise act with that account's permissions. The API's --- writers set the column again in the same transaction; any other path (a direct SQL update, a --- writer added later) leaves it cleared, which runs the agent as whoever runs it. -CREATE OR REPLACE FUNCTION reset_agent_on_behalf_of() RETURNS trigger AS $$ -BEGIN - IF NEW.resource_type = 'ai_agent' - AND NEW.value IS DISTINCT FROM OLD.value - AND NEW.on_behalf_of IS NOT DISTINCT FROM OLD.on_behalf_of THEN - NEW.on_behalf_of := NULL; - END IF; - RETURN NEW; -END; -$$ LANGUAGE plpgsql; - -CREATE TRIGGER reset_agent_on_behalf_of - BEFORE UPDATE ON resource - FOR EACH ROW EXECUTE FUNCTION reset_agent_on_behalf_of(); +-- An AI agent's value carries the identity it runs as when run from its own page, as a flow's or +-- a script's `on_behalf_of` is: a principal (`u/`, `g/`), resolved by the server on +-- every write of the agent. An agent saved before this ran as whoever ran it; give it the +-- account that wrote its current version, the one a write now records. -- Mirrors `users::username_to_permissioned_as`, as the sibling backfills do. `pg_temp` lives for -- the whole session and migrations share one connection, so replace any earlier copy and drop @@ -36,12 +16,14 @@ RETURNS VARCHAR AS $$ END; $$ LANGUAGE SQL IMMUTABLE; --- An agent saved before this column ran as whoever ran it; give it the account that wrote its --- current version, the one a deploy now records. A principal wider than --- `v2_job.permissioned_as` could not be enqueued, so it is left unset, as the sibling backfills --- leave it (20260801043001, 20260911085221). +-- Filling in the identity is not an edit of the agent, so it records no version. +ALTER TABLE resource DISABLE TRIGGER record_resource_version_update_trigger; + +-- A principal wider than `v2_job.permissioned_as` could not be enqueued, so it is left unset, as +-- the sibling backfills leave it (20260801043001, 20260911085221). UPDATE resource r - SET on_behalf_of = pg_temp.username_to_permissioned_as(v.created_by) + SET value = jsonb_set(r.value, '{on_behalf_of}', + to_jsonb(pg_temp.username_to_permissioned_as(v.created_by))) FROM ( SELECT DISTINCT ON (workspace_id, path) workspace_id, path, created_by FROM resource_version @@ -50,9 +32,12 @@ UPDATE resource r WHERE r.workspace_id = v.workspace_id AND r.path = v.path AND r.resource_type = 'ai_agent' - AND r.on_behalf_of IS NULL + AND jsonb_typeof(r.value) = 'object' + AND NOT (r.value ? 'on_behalf_of') AND v.created_by IS NOT NULL AND v.created_by <> '' AND length(pg_temp.username_to_permissioned_as(v.created_by)) <= 55; +ALTER TABLE resource ENABLE TRIGGER record_resource_version_update_trigger; + DROP FUNCTION pg_temp.username_to_permissioned_as(VARCHAR); diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index 70d552614d..ff084a44e4 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -31606,12 +31606,9 @@ components: type: string ws_specific: type: boolean - on_behalf_of: - type: string - description: ai_agent only. The identity the agent runs as (`u/`, `g/`), kept only with preserve_on_behalf_of by a caller allowed to; otherwise the agent runs as the caller. preserve_on_behalf_of: type: boolean - description: ai_agent only. Keep on_behalf_of rather than running as the caller. Honoured for admins and members of wm_deployers. + description: ai_agent only. Keep the identity `value.on_behalf_of` names (`u/`, `g/`) rather than the caller's own. Honoured for admins and members of wm_deployers; otherwise every write of an agent sets it to the caller. required: - path - value @@ -31636,12 +31633,9 @@ components: type: string ws_specific: type: boolean - on_behalf_of: - type: string - description: ai_agent only. The identity the agent runs as (`u/`, `g/`), kept only with preserve_on_behalf_of by a caller allowed to; a new value otherwise makes the agent run as the caller. preserve_on_behalf_of: type: boolean - description: ai_agent only. Keep on_behalf_of rather than running as the caller. Honoured for admins and members of wm_deployers. + description: ai_agent only. Keep the identity `value.on_behalf_of` names (`u/`, `g/`) rather than the caller's own. Honoured for admins and members of wm_deployers; otherwise every write of an agent sets it to the caller. Resource: type: object @@ -31762,9 +31756,6 @@ components: (over a deployed row or a synthesized draft-only row). Frontend appends a `*` to the displayed name. type: boolean - on_behalf_of: - type: string - description: ai_agent only. The identity the agent runs as when run from its own page. agent_memory: description: | For an `ai_agent` resource only, its `memory` setting: the one part diff --git a/backend/windmill-api/src/agent_runs.rs b/backend/windmill-api/src/agent_runs.rs index ab1560d56d..b9c835fad1 100644 --- a/backend/windmill-api/src/agent_runs.rs +++ b/backend/windmill-api/src/agent_runs.rs @@ -3,7 +3,7 @@ //! The run is built here from the stored agent rather than sent by the client, which is what lets //! anyone who can read the agent run it, operators included, the way a deployed flow runs by path: //! a preview takes arbitrary code, this takes only the run's inputs. It runs as the agent's -//! `on_behalf_of`, resolved when the agent was deployed. +//! `on_behalf_of`, which the server resolves on every write of the agent. use std::collections::HashMap; @@ -46,8 +46,8 @@ pub(crate) async fn run_agent( // Read through the caller's own permissions: reading the agent is what allows running it. let mut tx = user_db.clone().begin(&authed).await?; - let row = sqlx::query!( - "SELECT value AS \"value: sqlx::types::Json\", on_behalf_of + let value = sqlx::query_scalar!( + "SELECT value AS \"value: sqlx::types::Json\" FROM resource WHERE workspace_id = $1 AND path = $2 AND resource_type = 'ai_agent'", w_id, path @@ -55,13 +55,17 @@ pub(crate) async fn run_agent( .fetch_optional(&mut *tx) .await?; tx.commit().await?; - let row = not_found_if_none(row, "Agent", path)?; - let Some(value) = row.value else { + let Some(sqlx::types::Json(mut value)) = not_found_if_none(value, "Agent", path)? else { return Err(Error::BadRequest(format!( "Agent {path} has no configuration" ))); }; - let config = config_to_draft(value.0)?; + // Who the agent runs as, not one of its inputs. + let agent_obo = value + .as_object_mut() + .and_then(|o| o.remove("on_behalf_of")) + .and_then(|v| v.as_str().map(str::to_string)); + let config = config_to_draft(value)?; // A chat turn is filed where the agent's chat lists its conversations, which a flow cannot // name (flow paths carry no `.`), so the two never share a conversation list. @@ -74,7 +78,7 @@ pub(crate) async fn run_agent( let flow_value = agent_flow(&config, chat)?; let on_behalf_of = - windmill_common::on_behalf_of_from_permissioned_as(row.on_behalf_of.as_deref(), &w_id, &db) + windmill_common::on_behalf_of_from_permissioned_as(agent_obo.as_deref(), &w_id, &db) .await?; // As `run_flow` picks the identity of a deployed flow: the agent's own when it has one, the // caller's otherwise, which lends nobody's permissions. diff --git a/backend/windmill-api/src/workspaces_export.rs b/backend/windmill-api/src/workspaces_export.rs index ea79aaa572..544984f164 100644 --- a/backend/windmill-api/src/workspaces_export.rs +++ b/backend/windmill-api/src/workspaces_export.rs @@ -933,7 +933,14 @@ pub(crate) async fn tarball_workspace( .fetch_all(&mut *tx) .await?; - for resource in resources { + for mut resource in resources { + // An agent's identity belongs to this workspace, and whoever writes it elsewhere gets + // their own, as `to_string_without_metadata` leaves out a flow's. + if resource.resource_type == "ai_agent" { + if let Some(obj) = resource.value.as_mut().and_then(|v| v.as_object_mut()) { + obj.remove("on_behalf_of"); + } + } let resource_str = &to_string_without_metadata(&resource, ExtraPermsBehavior::Drop, None).unwrap(); archive diff --git a/backend/windmill-store/src/resources.rs b/backend/windmill-store/src/resources.rs index b54b05f640..89ebb0996e 100644 --- a/backend/windmill-store/src/resources.rs +++ b/backend/windmill-store/src/resources.rs @@ -204,10 +204,6 @@ pub struct ListableResource { #[serde(skip_serializing_if = "Option::is_none")] #[sqlx(default)] pub is_draft: Option, - /// `ai_agent` only: the identity it runs as when run from its own page. - #[serde(skip_serializing_if = "Option::is_none")] - #[sqlx(default)] - pub on_behalf_of: Option, } /// A row of the resource listing: the resource as a single read returns it, plus what only the @@ -238,10 +234,8 @@ pub struct CreateResource { pub labels: Option>, #[serde(default)] pub ws_specific: Option, - /// `ai_agent` only: the identity to keep, honoured with `preserve_on_behalf_of` for a caller - /// allowed to keep one (see `set_agent_on_behalf_of`). - #[serde(default)] - pub on_behalf_of: Option, + /// `ai_agent` only: keep the identity `value.on_behalf_of` names, for a caller allowed to + /// (see `resolve_agent_on_behalf_of`). #[serde(default)] pub preserve_on_behalf_of: Option, } @@ -253,7 +247,6 @@ struct EditResource { resource_type: Option, labels: Option>, ws_specific: Option, - on_behalf_of: Option, preserve_on_behalf_of: Option, } @@ -569,8 +562,6 @@ async fn list_resources( draft_only: Some(true), // Synthesized rows are the authed user's draft. is_draft: Some(true), - // Resolved on the first deploy; a draft runs nowhere but the editor's preview. - on_behalf_of: None, }; rows.push(ListedResource { resource, agent_memory, draft_path }); } @@ -604,8 +595,7 @@ async fn get_resource( variable.account, ws_specific.path IS NOT NULL as ws_specific, null::bool as draft_only, - null::bool as is_draft, - resource.on_behalf_of + null::bool as is_draft FROM resource LEFT JOIN variable ON variable.path = resource.path AND variable.workspace_id = $2 LEFT JOIN account ON variable.account = account.id AND account.workspace_id = $2 @@ -1228,7 +1218,20 @@ async fn create_resource( check_path_conflict(&mut tx, &w_id, &resource.path).await?; } - let res_value = resource.value.unwrap_or_default(); + let mut res_value = resource.value.unwrap_or_default(); + if resource.resource_type == "ai_agent" { + res_value = resolve_agent_raw_value( + &mut tx, + &authed, + &db, + &w_id, + &resource.path, + &res_value, + resource.preserve_on_behalf_of.unwrap_or(false), + true, + ) + .await?; + } let raw_json = sqlx::types::Json(res_value.as_ref()); if resource.path.starts_with("f/app_themes/") { @@ -1301,20 +1304,6 @@ async fn create_resource( } } - if resource.resource_type == "ai_agent" { - set_agent_on_behalf_of( - &mut tx, - &authed, - &db, - &w_id, - &resource.path, - resource.on_behalf_of.as_deref(), - resource.preserve_on_behalf_of.unwrap_or(false), - true, - ) - .await?; - } - // Mirror update_resource: Some(true) inserts, Some(false) clears (only // meaningful on the upsert path, since a pure create has no existing row), // None leaves the existing flag alone. @@ -2083,9 +2072,6 @@ async fn update_resource( if let Some(npath) = &ns.path { sqlb.set_str("path", npath); } - if let Some(nvalue) = &ns.value { - sqlb.set_str("value", nvalue.to_string()); - } if let Some(nrt) = &ns.resource_type { sqlb.set_str("resource_type", nrt); } @@ -2193,6 +2179,48 @@ async fn update_resource( } } + let current = sqlx::query!( + "SELECT resource_type, value AS \"value: sqlx::types::Json>\" + FROM resource WHERE workspace_id = $1 AND path = $2", + &w_id, + path + ) + .fetch_optional(&mut *tx) + .await?; + let resource_type = ns + .resource_type + .clone() + .or_else(|| current.as_ref().map(|c| c.resource_type.clone())); + // Retyping a resource into an agent makes its value an agent's, identity included, so it is + // resolved as a written one is. + let retyped = ns.resource_type.as_deref() == Some("ai_agent") + && current + .as_ref() + .is_some_and(|c| c.resource_type != "ai_agent"); + let nvalue = match &ns.value { + Some(v) => Some(v.clone()), + None if retyped => current.and_then(|c| c.value).map(|v| v.0), + None => None, + }; + if let Some(nvalue) = nvalue { + let nvalue = if resource_type.as_deref() == Some("ai_agent") { + resolve_agent_raw_value( + &mut tx, + &authed, + &db, + &w_id, + ns.path.as_deref().unwrap_or(path), + &nvalue, + ns.preserve_on_behalf_of.unwrap_or(false), + false, + ) + .await? + } else { + nvalue + }; + sqlb.set_str("value", nvalue.to_string()); + } + let sql = sqlb.sql().map_err(|e| Error::internal_err(e.to_string()))?; let npath_o: Option = sqlx::query_scalar(&sql) .fetch_optional(&mut *tx) @@ -2201,33 +2229,6 @@ async fn update_resource( let npath = not_found_if_none(npath_o, "Resource", path)?; - // A description or label edit changes nothing the agent runs, so only a new value or an - // identity asked for re-resolves who it runs as. - if ns.value.is_some() || ns.on_behalf_of.is_some() { - let is_agent = sqlx::query_scalar!( - "SELECT resource_type = 'ai_agent' FROM resource WHERE workspace_id = $1 AND path = $2", - &w_id, - &npath - ) - .fetch_optional(&mut *tx) - .await? - .flatten() - .unwrap_or(false); - if is_agent { - set_agent_on_behalf_of( - &mut tx, - &authed, - &db, - &w_id, - &npath, - ns.on_behalf_of.as_deref(), - ns.preserve_on_behalf_of.unwrap_or(false), - false, - ) - .await?; - } - } - if let Some(nlabels) = &ns.labels { sqlx::query!( "UPDATE resource SET labels = $1 WHERE path = $2 AND workspace_id = $3", @@ -2389,21 +2390,27 @@ async fn update_resource_value( Ok(format!("value of resource {} updated", path)) } -/// Records the identity an `ai_agent` runs as, resolved on every write of it as a flow's -/// `on_behalf_of` is on every deploy: the writer's own, unless one allowed to keep another asks -/// to, and a folder's default for a new agent. Called in the writer's transaction after the -/// value lands, since a value change clears the column (`reset_agent_on_behalf_of`). -async fn set_agent_on_behalf_of( +/// Rewrites the identity an `ai_agent` runs as (`value.on_behalf_of`), resolved on every write of +/// it as a flow's `on_behalf_of` is on every deploy: the writer's own, unless one allowed to keep +/// the one the value names asks to, and a folder's default for a new agent. The value is written +/// by anyone who can write the agent, so the key it arrives with is only a request. +async fn resolve_agent_on_behalf_of( tx: &mut Transaction<'_, Postgres>, authed: &ApiAuthed, db: &DB, w_id: &str, path: &str, - requested: Option<&str>, + value: &mut serde_json::Value, preserve: bool, created: bool, ) -> Result<()> { - let (mut requested, mut preserve) = (requested.map(str::to_string), preserve); + let Some(obj) = value.as_object_mut() else { + return Ok(()); + }; + let mut requested = obj + .remove("on_behalf_of") + .and_then(|v| v.as_str().map(str::to_string)); + let mut preserve = preserve; let explicit = requested.is_some() && preserve && windmill_common::can_preserve_on_behalf_of(authed); if created && !explicit && windmill_common::can_preserve_on_behalf_of(authed) { @@ -2425,14 +2432,9 @@ async fn set_agent_on_behalf_of( db, ) .await?; - sqlx::query!( - "UPDATE resource SET on_behalf_of = $1 WHERE workspace_id = $2 AND path = $3", - resolved, - w_id, - path - ) - .execute(&mut **tx) - .await?; + if let Some(resolved) = resolved.as_ref() { + obj.insert("on_behalf_of".to_string(), resolved.clone().into()); + } if let Some(kept) = windmill_common::check_on_behalf_of_preservation( resolved.as_deref(), preserve, @@ -2457,6 +2459,22 @@ async fn set_agent_on_behalf_of( Ok(()) } +/// `resolve_agent_on_behalf_of` on a value as the request carried it. +async fn resolve_agent_raw_value( + tx: &mut Transaction<'_, Postgres>, + authed: &ApiAuthed, + db: &DB, + w_id: &str, + path: &str, + value: &RawValue, + preserve: bool, + created: bool, +) -> Result> { + let mut parsed: serde_json::Value = serde_json::from_str(value.get())?; + resolve_agent_on_behalf_of(tx, authed, db, w_id, path, &mut parsed, preserve, created).await?; + Ok(serde_json::value::to_raw_value(&parsed)?) +} + /// Write a resource's value and run everything that has to follow it: a version row, the audit /// entry, deployment metadata, the webhook, and dependent CI tests. Shared with version restore /// so a restored value is indistinguishable downstream from any other edit. @@ -2485,6 +2503,20 @@ async fn set_resource_value( let mut tx = user_db.clone().begin(authed).await?; + let mut value = value; + if let Some(v) = value.as_mut() { + let resource_type = sqlx::query_scalar!( + "SELECT resource_type FROM resource WHERE workspace_id = $1 AND path = $2", + w_id, + path + ) + .fetch_optional(&mut *tx) + .await?; + if resource_type.as_deref() == Some("ai_agent") { + resolve_agent_on_behalf_of(&mut tx, authed, db, w_id, path, v, false, false).await?; + } + } + // `RETURNING resource_type` rather than a second lookup: the advisory below has to know the // type to leave `state` and `cache` alone, and this statement already runs. let updated = sqlx::query_scalar!( @@ -2499,9 +2531,6 @@ async fn set_resource_value( let Some(resource_type) = updated else { return Err(Error::NotFound(format!("Resource {} not found", path))); }; - if resource_type == "ai_agent" { - set_agent_on_behalf_of(&mut tx, authed, db, w_id, path, None, false, false).await?; - } audit_log( &mut *tx, authed, diff --git a/cli/src/commands/resource/resource.ts b/cli/src/commands/resource/resource.ts index 7cef4caf1f..f33ae00c98 100644 --- a/cli/src/commands/resource/resource.ts +++ b/cli/src/commands/resource/resource.ts @@ -20,6 +20,11 @@ import { Resource } from "../../../gen/types.gen.ts"; import { readInlinePathSync, readTextFile } from "../../utils/utils.ts"; import { isWorkspaceSpecificFile } from "../../core/specific_items.ts"; import { getCurrentGitBranch } from "../../utils/git.ts"; +import { + buildPermissionedAsContext, + type PermissionedAsContext, +} from "../../core/permissioned_as.ts"; +import { readEffectiveSyncBehavior } from "../../core/conf.ts"; export interface ResourceFile { value: any; @@ -83,6 +88,7 @@ export async function pushResource( // only round-trip the canonical layout); the standalone `resource push` // command pushes a single explicit file, where any pointer is fine. enforceCanonicalFileset?: boolean, + permissionedAsContext?: PermissionedAsContext, ): Promise { remotePath = removeType(remotePath, "resource"); try { @@ -93,6 +99,11 @@ export async function pushResource( } catch { // flow doesn't exist } + const claimed = agentOnBehalfOf( + resource, + localResource, + permissionedAsContext + ); // Helper function to resolve inline content const resolveInlineContent = async () => { @@ -133,11 +144,23 @@ export async function pushResource( // Only resolve inline content if we're actually updating await resolveInlineContent(); + if (claimed) { + log.info(`Preserving ${claimed} as permissioned_as for agent ${remotePath}`); + } await wmill.updateResource({ workspace: workspace, path: remotePath.replaceAll(SEP, "/"), - requestBody: { ...localResource, ...(wsSpecific !== undefined ? { ws_specific: wsSpecific } : {}) }, + requestBody: { + ...localResource, + ...(wsSpecific !== undefined ? { ws_specific: wsSpecific } : {}), + ...(claimed + ? { + value: { ...localResource.value, on_behalf_of: claimed }, + preserve_on_behalf_of: true, + } + : {}), + }, }); } else { // New resource - resolve inline content @@ -163,6 +186,39 @@ export async function pushResource( } } +/** + * An agent runs as the identity in its value's `on_behalf_of`, which the backend + * sets to whoever writes the agent. Like an app's, it is never taken from the + * tracked file, which the pull leaves it out of: the deployed one is claimed + * back only by a pusher the backend lets keep it, an admin or a `wm_deployers` + * member. Strips the key from `localResource` (and from the deployed value, so + * an unchanged agent still compares equal) and returns the identity to claim. + */ +export function agentOnBehalfOf( + deployed: ResourceFile | Resource | undefined, + localResource: ResourceFile, + permissionedAsContext: PermissionedAsContext | undefined +): string | undefined { + if ((deployed?.resource_type ?? localResource.resource_type) !== "ai_agent") { + return undefined; + } + const local = localResource.value; + if (local && typeof local === "object") { + delete local.on_behalf_of; + } + const value = deployed?.value; + const onBehalfOf = + value && typeof value === "object" ? value.on_behalf_of : undefined; + if (deployed && onBehalfOf !== undefined) { + const { on_behalf_of: _, ...rest } = value; + deployed.value = rest; + } + if (!onBehalfOf || !permissionedAsContext?.userIsAdminOrDeployer) { + return undefined; + } + return onBehalfOf; +} + type PushOptions = GlobalOptions; async function push(opts: PushOptions, filePath: string, remotePath: string) { const workspace = await resolveWorkspace(opts); @@ -184,7 +240,13 @@ async function push(opts: PushOptions, filePath: string, remotePath: string) { remotePath, undefined, parseFromFile(filePath), - filePath // Pass the local file path for branch-specific inline content resolution + filePath, // Pass the local file path for branch-specific inline content resolution + undefined, + undefined, + await buildPermissionedAsContext( + workspace.workspaceId, + await readEffectiveSyncBehavior(opts, workspace) + ) ); log.info(colors.bold.underline.green(`Resource ${remotePath} pushed`)); } diff --git a/cli/src/commands/sync/sync.ts b/cli/src/commands/sync/sync.ts index 6185decc5a..15a4eda4a7 100644 --- a/cli/src/commands/sync/sync.ts +++ b/cli/src/commands/sync/sync.ts @@ -5918,6 +5918,7 @@ export async function push( resourceFilePath, isFileResWsSpecific ? true : undefined, true, + permissionedAsContext, ); } // Already-synced parents got the full content this run. diff --git a/cli/src/core/permissioned_as.ts b/cli/src/core/permissioned_as.ts index d0301373da..231da0c014 100644 --- a/cli/src/core/permissioned_as.ts +++ b/cli/src/core/permissioned_as.ts @@ -246,6 +246,15 @@ export async function preCheckPermissionedAs( if (match) { currentOwner = match[1]; } + } else if (typeStr === "resource") { + // An agent always runs as someone, and the file never says who. + if (/["']?resource_type["']?\s*:\s*["']?ai_agent\b/.test(beforeContent)) { + wouldChangeItems.push({ + path: change.path, + currentOwner: "(agent owner)", + }); + } + continue; } else if (typeStr.endsWith("_trigger")) { wouldChangeItems.push({ path: change.path, diff --git a/cli/src/types.ts b/cli/src/types.ts index 866b2be21e..1a7b895ffe 100644 --- a/cli/src/types.ts +++ b/cli/src/types.ts @@ -248,7 +248,7 @@ export async function pushObj( } else if (typeEnding === "resource") { if (!alreadySynced.includes(p)) { alreadySynced.push(p); - await pushResource(workspace, p, befObj, newObj, originalLocalPath || p, wsSpecific, true); + await pushResource(workspace, p, befObj, newObj, originalLocalPath || p, wsSpecific, true, permissionedAsContext); } } else if (typeEnding === "resource-type") { await pushResourceType(workspace, p, befObj, newObj); diff --git a/cli/test/resource_push_agent_obo_unit.test.ts b/cli/test/resource_push_agent_obo_unit.test.ts new file mode 100644 index 0000000000..ca129637ce --- /dev/null +++ b/cli/test/resource_push_agent_obo_unit.test.ts @@ -0,0 +1,55 @@ +/** + * An agent's run-as identity lives in its value, which the pull leaves out. A + * push must neither see that gap as a change nor let the file pick who the + * agent runs as: only the deployed identity is claimed back, and only by a + * pusher the backend lets keep it. + */ + +import { expect, test } from "bun:test"; +import { isSuperset } from "../src/types.ts"; +import { agentOnBehalfOf } from "../src/commands/resource/resource.ts"; + +const ctx = (userIsAdminOrDeployer: boolean) => ({ + userCache: new Map(), + userIsAdminOrDeployer, + userEmail: "me@x.dev", +}); + +function agents(localObo?: string) { + const deployed = { + resource_type: "ai_agent", + value: { system_prompt: "hi", on_behalf_of: "u/bob" }, + } as any; + const local = { + resource_type: "ai_agent", + value: { system_prompt: "hi", ...(localObo ? { on_behalf_of: localObo } : {}) }, + } as any; + return { deployed, local }; +} + +test("an unchanged agent still compares equal to the deployed one", () => { + const { deployed, local } = agents(); + agentOnBehalfOf(deployed, local, ctx(true)); + expect(isSuperset(local, deployed)).toBe(true); +}); + +test("an admin or deployer claims the deployed identity, never the file's", () => { + const { deployed, local } = agents("u/admin"); + expect(agentOnBehalfOf(deployed, local, ctx(true))).toBe("u/bob"); + expect(local.value.on_behalf_of).toBeUndefined(); +}); + +test("anyone else claims nothing", () => { + const { deployed, local } = agents("u/bob"); + expect(agentOnBehalfOf(deployed, local, ctx(false))).toBeUndefined(); + expect(agentOnBehalfOf(agents().deployed, agents().local, undefined)).toBeUndefined(); + expect(local.value.on_behalf_of).toBeUndefined(); +}); + +test("other resources are left alone", () => { + const deployed = { resource_type: "json", value: { on_behalf_of: "u/bob" } } as any; + const local = { resource_type: "json", value: { on_behalf_of: "u/x" } } as any; + expect(agentOnBehalfOf(deployed, local, ctx(true))).toBeUndefined(); + expect(local.value.on_behalf_of).toBe("u/x"); + expect(deployed.value.on_behalf_of).toBe("u/bob"); +}); diff --git a/frontend/src/lib/components/flows/agentDraft.svelte.ts b/frontend/src/lib/components/flows/agentDraft.svelte.ts index b76f9bb471..a7e44941f3 100644 --- a/frontend/src/lib/components/flows/agentDraft.svelte.ts +++ b/frontend/src/lib/components/flows/agentDraft.svelte.ts @@ -144,18 +144,29 @@ export function agentDraftDeployRefusal( */ type AgentWriteResult = { ok: true } | { ok: false; error: string; notAnAgent?: true } +/** Who a deploy makes the agent run as. The server resolves it as it does a flow's: without + * `preserve`, or from someone not allowed to keep another identity, it is the deployer. */ +export interface AgentRunAs { + permissionedAs: string | undefined + preserve: boolean +} + async function writeAgentResource( workspace: string, fromPath: string, state: AgentResourceState, - noDeployed: boolean + noDeployed: boolean, + runAs?: AgentRunAs ): Promise { const body = { path: state.path, - value: state.args, + value: runAs?.permissionedAs + ? { ...state.args, on_behalf_of: runAs.permissionedAs } + : state.args, description: state.description, labels: state.labels, - ws_specific: state.wsSpecific + ws_specific: state.wsSpecific, + preserve_on_behalf_of: runAs?.preserve || undefined } try { if (noDeployed) { @@ -221,9 +232,12 @@ export interface AgentDraftHandle { /** Why this path cannot be edited here, if it cannot. Render it instead of the form. */ readonly refusal: string | undefined readonly sync: TriggerDraftSync + /** Who the deployed agent runs as when run from its page; undefined when nothing is deployed + * or it runs as whoever runs it. */ + readonly onBehalfOf: string | undefined /** Write the current state to the resource and drop the draft. Resolves to the path written, * which differs from the one loaded when the draft renames the agent, or undefined on failure. */ - deploy: () => Promise + deploy: (runAs?: AgentRunAs) => Promise } /** @@ -237,6 +251,7 @@ export function useAgentDraft(opts: AgentDraftOptions): AgentDraftHandle { let loading = $state(true) let noDeployed = $state(false) let canWriteResource = $state(true) + let onBehalfOf = $state(undefined) /** Guards the load against a path that changed under a slow response. */ let loadedFor = $state(undefined) /** Why the loaded path cannot be edited here, if it cannot. Gates the sync for as long as that @@ -324,10 +339,15 @@ export function useAgentDraft(opts: AgentDraftOptions): AgentDraftHandle { return } noDeployed = Boolean((r as any).no_deployed) + // Who it runs as is the deploy's to decide, not a setting the form or a draft holds. + const { on_behalf_of, ...args } = (r.value ?? {}) as AIAgentConfig & { + on_behalf_of?: string + } + onBehalfOf = noDeployed ? undefined : on_behalf_of const deployedState: AgentResourceState = { path: r.path, description: r.description ?? '', - args: (r.value ?? {}) as AIAgentConfig, + args, // Only where nothing else answers for the type: with a deployed row both the // create below and the review page's `deployDraft` read it from there, and // carrying it would make every draft the generic resource editor writes — which @@ -408,7 +428,7 @@ export function useAgentDraft(opts: AgentDraftOptions): AgentDraftHandle { }) }) - async function deploy(): Promise { + async function deploy(runAs?: AgentRunAs): Promise { const ws = opts.workspace() const fromPath = opts.path() const s = state @@ -424,7 +444,7 @@ export function useAgentDraft(opts: AgentDraftOptions): AgentDraftHandle { // made during the request as saved, and the banner would clear on a value the server never // received; against the snapshot it stays a draft, which is what it is. const submitted = structuredClone($state.snapshot(s)) as AgentResourceState - const written = await writeAgentResource(ws, fromPath, submitted, noDeployed) + const written = await writeAgentResource(ws, fromPath, submitted, noDeployed, runAs) if (!written.ok) { // A path that is no longer an agent tears this editor down; anything else is a plain error // the user can retry from the form as it stands. @@ -440,6 +460,10 @@ export function useAgentDraft(opts: AgentDraftOptions): AgentDraftHandle { logReusableAgentUsage(noDeployed ? 'saved' : 'updated') deployed = submitted noDeployed = false + // What the server resolved, which a request to keep an identity does not decide alone. + void ResourceService.getResource({ workspace: ws, path: submitted.path }) + .then((r) => (onBehalfOf = (r.value as { on_behalf_of?: string } | undefined)?.on_behalf_of)) + .catch(() => {}) const renamed = submitted.path !== fromPath // Only when the form still holds exactly what was sent. `discard` resets the handle's cell to // what it is given, and the apply-effect copies that back over the form: against an edit made @@ -479,6 +503,9 @@ export function useAgentDraft(opts: AgentDraftOptions): AgentDraftHandle { get refusal() { return refusal }, + get onBehalfOf() { + return onBehalfOf + }, sync, deploy } diff --git a/frontend/src/lib/components/flows/content/AgentEditorHost.svelte b/frontend/src/lib/components/flows/content/AgentEditorHost.svelte index 58d9e3be76..2d903a0811 100644 --- a/frontend/src/lib/components/flows/content/AgentEditorHost.svelte +++ b/frontend/src/lib/components/flows/content/AgentEditorHost.svelte @@ -17,6 +17,7 @@ import { goto } from '$lib/navigation' import { deepEqual } from 'fast-equals' import type { Flow, FlowModule, InputTransform, Job, OpenFlow } from '$lib/gen' + import { JobService } from '$lib/gen' import { emptySchema, type StateStore } from '$lib/utils' import type { FlowInput } from '$lib/components/flows/types' import type { FlowEditorContext, FlowInputEditorState, FlowPanelDetachContext } from '../types' @@ -50,7 +51,8 @@ keepsManagedMemory } from '../agentFormFields' import { toolDisplayName, type AgentTool } from '../agentToolUtils' - import { useAgentDraft } from '../agentDraft.svelte' + import { useAgentDraft, type AgentRunAs } from '../agentDraft.svelte' + import PermissionedAsLine from '$lib/components/triggers/PermissionedAsLine.svelte' import Path from '$lib/components/Path.svelte' import Label from '$lib/components/Label.svelte' import { sendUserToast } from '$lib/toast' @@ -70,8 +72,6 @@ /** The deployed agent to run rather than a draft to edit: the run pane leads, and the form * beside it only shows the configuration. Fixed for the mount's lifetime. */ view?: boolean - /** Why this reader cannot run the agent, if they cannot. Shown in place of the run pane. */ - runBlockedReason?: string /** Opens the agent's configuration, offered beside the model it runs with. */ onOpenConfig?: () => void } @@ -85,7 +85,6 @@ onSaved = undefined, isNew = false, view = false, - runBlockedReason = undefined, onOpenConfig = undefined }: Props = $props() @@ -351,6 +350,16 @@ inputs?: Record ): Promise { if (!agentModule) return undefined + // A view runs the deployed agent as its on-behalf-of identity, which anyone who can read it + // may do; the editor previews the draft as the one editing it. + if (view && workspace) { + return await JobService.runAgent({ + workspace, + path, + memoryId: conversationId, + requestBody: { ...(inputs ?? {}), user_message: userMessage } + }) + } return await runFlowPreview( { ...(inputs ?? {}), user_message: userMessage }, agentChatFlow($state.snapshot(agentModule) as FlowModule), @@ -367,15 +376,13 @@ let invisible_to_owner = $state(undefined) let overrideTag = $state(undefined) - /** One run from the form, as the same one-step flow a chat turn runs but outside any - * conversation, then onto its run page. */ + /** One run of the deployed agent from the view's form, outside any conversation, then onto its + * run page. */ async function runOnce(_scheduledFor: string | undefined, args: Record) { - if (!agentModule) return + if (!workspace) return runLoading = true try { - const flow = agentChatFlow($state.snapshot(agentModule) as FlowModule) - flow.value.chat_input_enabled = false - const id = await runFlowPreview(args, flow, path, undefined, undefined, undefined, workspace) + const id = await JobService.runAgent({ workspace, path, requestBody: args }) await goto(`/run/${id}?workspace=${workspace}`) } finally { runLoading = false @@ -437,12 +444,15 @@ * only report after the request. */ let pathError = $state('') + /** Who the next deploy makes the agent run as, as the "Permissioned as" line picked it. */ + let runAs = $state(undefined) + export function deploy(): Promise { if (pathError) { sendUserToast(`Cannot deploy the agent: ${pathError}`, true) return Promise.resolve(false) } - return draft.deploy().then(async (written) => { + return draft.deploy(runAs).then(async (written) => { // The path the write landed on, which a rename moves off the one this editor opened. if (written) await onSaved?.(written) return written !== undefined @@ -558,6 +568,18 @@ disabled={readOnly} /> + + {#if !readOnly} +
+ + (runAs = { permissionedAs, preserve })} + /> +
+ {/if} {/if} @@ -588,132 +610,121 @@ {/snippet} {#snippet runPane()} - {#if runBlockedReason} -
- {runBlockedReason} - {#if onOpenConfig} - {@render configurationButton()} - {/if} -
- {:else} -
- - {#if view} - - -
-
- -
+ +
+
+
- {:else} -
- - {#if !chatGap?.memory && !readOnly} - - Managed memory keeps the conversation between runs. It's useful if you chat with - this agent, use it in a flow in chat mode, or pass it a memory id. Otherwise, you - can turn it off. - - {/if} - - -
- -
- - - + Managed memory keeps the conversation between runs. It's useful if you chat with this + agent, use it in a flow in chat mode, or pass it a memory id. Otherwise, you can turn + it off. + + {/if} + + +
+ - - -
- {/if} - {#if chatMounted} -
- {#if chatGap?.memory} -
- -

Chat needs managed memory

-

- {chatGap.memoryCanTurnOn - ? 'Without it, every message would be answered without the ones before it.' - : 'This agent replays a fixed list of messages. Switch its memory to managed to chat with it.'} -

- {#if chatGap.memoryCanTurnOn && !readOnly} - - {/if}
- {:else if chatGap?.noStream} - - {chatGap.noStream === 'image' - ? 'Image answers do not stream: each one shows once its run ends.' - : 'Streaming is off: each answer shows once its run ends.'} - - {/if} - -
- -
+
+
- {/if} -
- {/if} +
+ {/if} +
{/snippet} - {#if chatAvailable && !runBlockedReason && testPane?.mode} + {#if chatAvailable && testPane?.mode} testPane?.mode, @@ -176,7 +165,6 @@ workspace={ws} enableAi={$copilotInfo.enabled} view - {runBlockedReason} onOpenConfig={() => configModal?.open()} /> {/key}