From 2ba8db66547e763a6ecc207da50ca3bcff5992dd Mon Sep 17 00:00:00 2001 From: thedancingdeveloper Date: Fri, 11 Sep 2026 02:14:15 +0000 Subject: [PATCH] =?UTF-8?q?feat(sessions):=20task-bearing=20agent=20sessio?= =?UTF-8?q?ns=20=E2=80=94=20a=20spoken=20"do=20X"=20opens=20an=20agent=20o?= =?UTF-8?q?n=20X?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gaps behind "spawn a session and check the containers": 1. The session tool had nowhere to put the *doing* part. A session opened on the project brief, which says in as many words that there is no task, and the agent waited — the person then typed what they had just said. session.start gains `task`, folded into the brief under a `## Task` heading so the agent opens already asked to do it. 2. `template` reached the engine as a literal command (`["claude"]`), an unwrapped binary, not the deployment's protected `["vogt-agent-auth", "run", "--", "claude"]`. Vogt now sends the template *name*; the engine expands it against its own `session_templates` by name or tag (an `agent`-tagged template wins an ambiguous tag, so `claude` reaches the protected Claude Code template). The wrapper stays in deployment config, out of the core and out of shipped code. An unknown name is 400 listing the configured templates, never silently a shell. Only taken when no explicit command is given, so the GUI (which sends a command) is untouched. The two together mean "start a session on komodo and check the containers" can open a protected Claude agent already briefed to check them. Engine: SessionSpec.template (contract) + resolve_template_in over the configured templates + expansion in SessionRegistry::create. Core: StartSessionParams.task, EngineClient.create_session template kwarg, _brief_with_task. Tests: template resolves by name then tag, agent tag wins ambiguity, unknown is refused; task folds into the brief and is absent without it; the template is sent by name for the engine to expand. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01YG9Nsxqc6CREb56Uisnmmy --- docs/ENGINE.md | 13 +- engine/contract/src/lib.rs | 10 ++ engine/server/src/agent_tasks.rs | 1 + engine/server/src/assistant.rs | 1 + engine/server/src/sessions.rs | 150 ++++++++++++++++++++++ src/vogt/adapters/engine/client.py | 9 +- src/vogt/application/models.py | 18 ++- src/vogt/application/services/sessions.py | 31 ++++- tests/test_sessions.py | 47 +++++++ 9 files changed, 271 insertions(+), 9 deletions(-) diff --git a/docs/ENGINE.md b/docs/ENGINE.md index b38198a1..95d9340e 100644 --- a/docs/ENGINE.md +++ b/docs/ENGINE.md @@ -552,7 +552,18 @@ is absent, empty, or all whitespace writes no file and sets no variable. that escapes via `..` is `400` rather than a shell in `/etc`. `name` is trimmed and must be non-empty and at most 256 bytes, on creation and on rename alike; outside that it is `400`, never truncated. Names need not be unique — -duplicates are confusing, not invalid. The remaining `SessionSpec` fields — +duplicates are confusing, not invalid. `SessionSpec` also carries an optional `template` — a session template *name* +or *tag* the engine expands into `command` (and env) against its configured +`session_templates`, when no explicit `command` is given (the GUI copies a +template's command into the spec itself, so it never uses this). A name matches +case-insensitively; failing that a tag matches, and an `agent`-tagged template +wins an ambiguous tag so `template: "claude"` reaches the deployment's +protected `["vogt-agent-auth", "run", "--", "claude"]` rather than a bare +binary. An unknown name is `400` listing the configured templates, never +silently a shell. This is where vogt-core sends the bare agent name it was +asked for and lets the deployment decide what protected command it runs. + +The remaining `SessionSpec` fields — `command`, `env`, `cols`, `rows`, `scrollback_bytes` — each fall back to the server's configured default when omitted. diff --git a/engine/contract/src/lib.rs b/engine/contract/src/lib.rs index 6ebf5bd2..d4c27820 100644 --- a/engine/contract/src/lib.rs +++ b/engine/contract/src/lib.rs @@ -26,6 +26,16 @@ pub struct SessionSpec { pub name: String, #[serde(default, skip_serializing_if = "Option::is_none")] pub command: Option>, + /// A session template to expand into `command` (and env), by name or by + /// tag, resolved against the engine's configured `session_templates`. + /// The engine owns the mapping because the command a template runs — a + /// `vogt-agent-auth run -- claude` wrapper, say — is that deployment's + /// configuration, not the caller's to spell out. Ignored when `command` + /// is given; an unknown name is refused. vogt-core sends the name it was + /// asked for (`claude`) and lets the engine turn it into the protected + /// command the deployment configured for it. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub template: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub cwd: Option, #[serde(default, skip_serializing_if = "Option::is_none")] diff --git a/engine/server/src/agent_tasks.rs b/engine/server/src/agent_tasks.rs index 9da512a7..9f955181 100644 --- a/engine/server/src/agent_tasks.rs +++ b/engine/server/src/agent_tasks.rs @@ -1461,6 +1461,7 @@ impl AgentTaskRegistry { let session = self.sessions.create(SessionSpec { name: session_name.clone(), command: Some(command), + template: None, cwd: task.cwd.clone(), env: Some(env), // A task run has already written its own prompt file above, with diff --git a/engine/server/src/assistant.rs b/engine/server/src/assistant.rs index fbd156d2..e0748516 100644 --- a/engine/server/src/assistant.rs +++ b/engine/server/src/assistant.rs @@ -2172,6 +2172,7 @@ mod tests { .create(SessionSpec { name: "cat".into(), command: Some(vec!["cat".into()]), + template: None, cwd: None, env: None, prompt: None, diff --git a/engine/server/src/sessions.rs b/engine/server/src/sessions.rs index a66d7d90..9a6c439e 100644 --- a/engine/server/src/sessions.rs +++ b/engine/server/src/sessions.rs @@ -16,6 +16,41 @@ use crate::{ workspace_path, }; +/// A configured session template matched by name (case-insensitive) or, +/// failing that, by tag — so a caller can say `claude` and reach the template +/// tagged `claude`. When a tag matches more than one template an `agent`-tagged +/// one wins, then the alphabetically-first name, so the choice is deterministic +/// rather than list-order-dependent. An unknown name is refused with the +/// configured names, never quietly started as a shell. +fn resolve_template_in<'a>( + templates: &'a [crate::config::SessionTemplate], + name: &str, +) -> Result<&'a crate::config::SessionTemplate> { + if let Some(t) = templates.iter().find(|t| t.name.eq_ignore_ascii_case(name)) { + return Ok(t); + } + let mut by_tag: Vec<&crate::config::SessionTemplate> = templates + .iter() + .filter(|t| t.tags.iter().any(|tag| tag.eq_ignore_ascii_case(name))) + .collect(); + by_tag.sort_by(|a, b| { + let a_agent = a.tags.iter().any(|g| g.eq_ignore_ascii_case("agent")); + let b_agent = b.tags.iter().any(|g| g.eq_ignore_ascii_case("agent")); + b_agent.cmp(&a_agent).then_with(|| a.name.cmp(&b.name)) + }); + if let Some(t) = by_tag.first() { + return Ok(t); + } + let known = templates + .iter() + .map(|t| t.name.as_str()) + .collect::>() + .join(", "); + Err(ApiError::BadRequest(format!( + "unknown session template {name:?}; configured: {known}" + ))) +} + pub struct SessionRegistry { cfg: Arc, bus: EventBus, @@ -47,6 +82,31 @@ impl SessionRegistry { pub fn create(&self, mut spec: SessionSpec) -> Result> { spec.name = normalize_session_name(&spec.name)?; + // Expand a template name into a command before anything downstream + // reads `command`. Only when the caller gave no explicit command — + // the GUI copies a template's command into the spec itself and sends + // that, so it never takes this path. vogt-core sends the bare name + // ("claude") and the deployment's config is where that becomes + // `vogt-agent-auth run -- claude`, keeping the wrapper out of the + // core and out of shipped code. + if spec.command.is_none() { + if let Some(name) = spec + .template + .as_deref() + .map(str::trim) + .filter(|t| !t.is_empty()) + { + let template = self.resolve_template(name)?; + spec.command = template.command.clone(); + if !template.env.is_empty() { + let mut env = template.env.clone(); + if let Some(existing) = spec.env.take() { + env.extend(existing); + } + spec.env = Some(env); + } + } + } // Resolve client-supplied cwd against workspace_root. Reject anything // that escapes the workspace via `..` so a stray API call can't spawn // a shell with cwd=/etc. @@ -222,6 +282,16 @@ impl SessionRegistry { out } + /// A configured session template matched by name (case-insensitive) or, + /// failing that, by tag — so a caller can say `claude` and reach the + /// template tagged `claude`. When a tag matches more than one template an + /// `agent`-tagged one wins, then the alphabetically-first name, so the + /// choice is deterministic rather than list-order-dependent. An unknown + /// name is refused with the configured names, never started as a shell. + fn resolve_template(&self, name: &str) -> Result<&crate::config::SessionTemplate> { + resolve_template_in(&self.cfg.session_templates, name) + } + pub fn rename(&self, id: Uuid, new_name: String) -> Result<()> { let s = self.get(id)?; let new_name = normalize_session_name(&new_name)?; @@ -298,4 +368,84 @@ mod tests { let err = normalize_session_name(&long).unwrap_err(); assert!(err.to_string().contains("at most 256 bytes")); } + #[test] + fn a_template_resolves_by_name_then_by_tag() { + use crate::config::SessionTemplate; + let templates = SessionTemplate::default_templates(); + // Exact name, case-insensitive. + assert_eq!( + super::resolve_template_in(&templates, "claude code (protected)") + .unwrap() + .name, + "Claude Code (protected)" + ); + // By tag: `claude` reaches the protected Claude template, whose + // command is the deployment's wrapper — the whole point. + let claude = super::resolve_template_in(&templates, "claude").unwrap(); + assert_eq!(claude.name, "Claude Code (protected)"); + assert!(claude + .command + .as_ref() + .unwrap() + .iter() + .any(|arg| arg == "claude")); + // A plain shell is still reachable by name. + assert_eq!( + super::resolve_template_in(&templates, "Shell") + .unwrap() + .name, + "Shell" + ); + } + + #[test] + fn an_unknown_template_is_refused_with_the_configured_names() { + use crate::config::SessionTemplate; + let templates = SessionTemplate::default_templates(); + let err = super::resolve_template_in(&templates, "kardashian").unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("unknown session template"), "{msg}"); + assert!(msg.contains("Claude Code (protected)"), "{msg}"); + } + + #[test] + fn the_agent_tag_wins_when_a_tag_is_ambiguous() { + use crate::config::SessionTemplate; + // Two templates share a tag; the agent one must win deterministically. + let templates = vec![ + SessionTemplate { + name: "Zeta Shell".into(), + description: String::new(), + command: Some(vec!["bash".into()]), + cwd: None, + env: vec![], + default_name: None, + match_repo_names: vec![], + match_path_prefixes: vec![], + tags: vec!["claude".into()], + }, + SessionTemplate { + name: "Alpha Agent".into(), + description: String::new(), + command: Some(vec![ + "vogt-agent-auth".into(), + "run".into(), + "--".into(), + "claude".into(), + ]), + cwd: None, + env: vec![], + default_name: None, + match_repo_names: vec![], + match_path_prefixes: vec![], + tags: vec!["agent".into(), "claude".into()], + }, + ]; + assert_eq!( + super::resolve_template_in(&templates, "claude") + .unwrap() + .name, + "Alpha Agent" + ); + } } diff --git a/src/vogt/adapters/engine/client.py b/src/vogt/adapters/engine/client.py index 3476c007..d538b8c6 100644 --- a/src/vogt/adapters/engine/client.py +++ b/src/vogt/adapters/engine/client.py @@ -356,7 +356,8 @@ def create_session( self, *, name: str, - command: list[str] | None, + command: list[str] | None = None, + template: str | None = None, cwd: str, env: dict[str, str] | None = None, prompt: str | None = None, @@ -374,6 +375,12 @@ def create_session( spec: dict[str, Any] = {"name": name, "cwd": cwd} if command: spec["command"] = command + if template: + # A template *name*, not a command: the engine expands it against + # its own `session_templates`, because the command a template runs + # (a `vogt-agent-auth run -- claude` wrapper, say) is that + # deployment's configuration and not Vogt's to spell out. + spec["template"] = template if prompt: # The engine writes this to a file on its own state directory and # tells the child where it is. Vogt sends the text rather diff --git a/src/vogt/application/models.py b/src/vogt/application/models.py index 39999b10..1679f8cb 100644 --- a/src/vogt/application/models.py +++ b/src/vogt/application/models.py @@ -2622,8 +2622,22 @@ class StartSessionParams(Params): template: str | None = Field( default=None, description=( - "Named session template to run, e.g. an agent CLI. Omitted means " - "the engine's default shell." + "Session template to run, by name or by tag — the engine expands " + "it against the deployment's templates. Use this to run an agent " + "rather than a plain shell: `claude` (or `codex`, `opencode`) " + "starts that agent under the deployment's protected wrapper, so " + "a request to *do* something in a session names one here. " + "Omitted means a plain shell, which does nothing until typed into." + ), + ) + task: str | None = Field( + default=None, + description=( + "What the session's agent should do, in the user's words. Folded " + "into the brief the agent opens with, so 'start a session on X " + "and check its containers' opens an agent already asked to check " + "them rather than an idle shell. Pair it with `template` (an " + "agent, not a plain shell) when the task is something to carry out." ), ) name: str | None = Field( diff --git a/src/vogt/application/services/sessions.py b/src/vogt/application/services/sessions.py index 56ed884e..45d052aa 100644 --- a/src/vogt/application/services/sessions.py +++ b/src/vogt/application/services/sessions.py @@ -94,7 +94,7 @@ def start_session(ctx: AppContext, params: StartSessionParams) -> SessionResult: template=params.template, cwd=subject.cwd, env=_session_env(ctx, session_id, credential.secret), - brief=subject.brief, + brief=_brief_with_task(subject.brief, params.task), model=params.model, effort=params.effort, ) @@ -558,6 +558,23 @@ def _existing(ctx: AppContext, session_id: str) -> CodingSession: return session +def _brief_with_task(brief: str, task: str | None) -> str: + """Fold an explicit task into the session's brief. + + The brief is context — the project, or the work item and why it ranks — + and by itself it asks the agent to do nothing (a project brief says so + in as many words). A spoken "start a session on komodo and check the + containers" carries the *doing* part separately; without it the agent + opens and waits, and the person has to type what they just said. + Appended rather than replacing so the agent keeps the context under + the task. + """ + task = (task or "").strip() + if not task: + return brief + return f"{brief.rstrip()}\n\n## Task\n\n{task}\n" + + def _start_on_engine( engine: EngineClient, *, @@ -572,10 +589,14 @@ def _start_on_engine( return engine.create_session( prompt=brief, name=name, - # A template names a command the *engine* knows; Vogt passes the name - # through rather than resolving it, because the command a template - # runs is that pod's configuration and not the estate's. - command=None if template is None else [template], + # A template names a command the *engine* knows; Vogt sends the + # name and the engine expands it against its own `session_templates`, + # because the command a template runs — a `vogt-agent-auth run -- + # claude` wrapper, say — is that pod's configuration and not the + # estate's. Sending the bare name is what lets "start an agent on + # this" reach the deployment's protected agent template rather than + # an unwrapped binary. + template=template, cwd=cwd, env=env, # Passed through for the same reason: *how* a model id diff --git a/tests/test_sessions.py b/tests/test_sessions.py index 498db58b..4c9b684b 100644 --- a/tests/test_sessions.py +++ b/tests/test_sessions.py @@ -243,6 +243,53 @@ def test_a_session_can_be_opened_on_a_project( assert engine.last_spec["cwd"] == ROOT +def test_a_task_is_folded_into_the_session_brief( + wired: AppContext, engine: StandInEngine +) -> None: + """A spoken request carries the doing part; the brief must too. + + Without this the agent opens on the project brief, which says in as many + words that there is no task, and waits — the person then types what they + already said out loud. + """ + start_session( + wired, + StartSessionParams( + project="vogt", + task="check how many containers need an update", + reason=WHY, + ), + ) + brief = engine.last_spec["prompt"] + assert "## Task" in brief + assert "check how many containers need an update" in brief + # The project context is still there, under the task. + assert brief.index("# vogt") < brief.index("## Task") + + +def test_no_task_leaves_the_brief_untouched( + wired: AppContext, engine: StandInEngine +) -> None: + start_session(wired, StartSessionParams(project="vogt", reason=WHY)) + assert "## Task" not in engine.last_spec["prompt"] + + +def test_a_template_is_sent_by_name_for_the_engine_to_expand( + wired: AppContext, engine: StandInEngine +) -> None: + """The engine owns the command a template runs; Vogt sends the name. + + Sending `claude` (not `["claude"]`) is what lets the engine expand it to + the deployment's protected wrapper rather than an unwrapped binary. + """ + start_session( + wired, + StartSessionParams(project="vogt", template="claude", reason=WHY), + ) + assert engine.last_spec.get("template") == "claude" + assert "command" not in engine.last_spec + + def test_a_session_needs_exactly_one_subject(wired: AppContext) -> None: with pytest.raises(InvalidRequest): start_session(wired, StartSessionParams(reason=WHY))