Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion docs/ENGINE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
10 changes: 10 additions & 0 deletions engine/contract/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,16 @@ pub struct SessionSpec {
pub name: String,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub command: Option<Vec<String>>,
/// 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<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub cwd: Option<String>,
#[serde(default, skip_serializing_if = "Option::is_none")]
Expand Down
1 change: 1 addition & 0 deletions engine/server/src/agent_tasks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions engine/server/src/assistant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2172,6 +2172,7 @@ mod tests {
.create(SessionSpec {
name: "cat".into(),
command: Some(vec!["cat".into()]),
template: None,
cwd: None,
env: None,
prompt: None,
Expand Down
150 changes: 150 additions & 0 deletions engine/server/src/sessions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Vec<_>>()
.join(", ");
Err(ApiError::BadRequest(format!(
"unknown session template {name:?}; configured: {known}"
)))
}

pub struct SessionRegistry {
cfg: Arc<Config>,
bus: EventBus,
Expand Down Expand Up @@ -47,6 +82,31 @@ impl SessionRegistry {

pub fn create(&self, mut spec: SessionSpec) -> Result<Arc<Session>> {
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.
Expand Down Expand Up @@ -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)?;
Expand Down Expand Up @@ -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"
);
}
}
9 changes: 8 additions & 1 deletion src/vogt/adapters/engine/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down
18 changes: 16 additions & 2 deletions src/vogt/application/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
31 changes: 26 additions & 5 deletions src/vogt/application/services/sessions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
*,
Expand All @@ -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
Expand Down
47 changes: 47 additions & 0 deletions tests/test_sessions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
Loading