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: 13 additions & 0 deletions docs/ENGINE.md
Original file line number Diff line number Diff line change
Expand Up @@ -854,6 +854,19 @@ than these routes.
Absent means the session opens in `cwd`, byte-identical to a request from
before the field existed. A wake returns to the worktree already cut and
does not cut another.
`session.start`'s `cwd` (WI-1671) is the other way in: the caller names an
existing worktree and vogt-core sends it as this `cwd`, with no
`SessionSpec.worktree`. It must be an absolute path that exists, sit under
the workspace `Active/` directory (the parent of the project's registered
checkout, not any path component named `Active`), and be a git worktree of
the same repository as that checkout (the same git `commondir`). The
checkout, and any subdirectory of it, is refused: its git toplevel is
the checkout, so a worker cannot start inside the shared tree. A linked
worktree nested under the checkout has its own toplevel and is accepted. A symlink is accepted as the directory it resolves to.
Combining it with an explicit `worktree: true` is refused before the
engine is asked. A purpose whose profile would have set `worktree` (the
`implement` profile does) does not: `cwd` means the checkout already
exists, so the profile's worktree is left out.
- `POST /api/sessions/:id/conversation` `{"agent", "id", "ended"?}` ->
`SessionSummary` (WI-962). Links the agent conversation running in the
session, or with `ended` unlinks it while it is still the session's. See
Expand Down
21 changes: 20 additions & 1 deletion src/vogt/application/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -4226,7 +4226,26 @@ class StartSessionParams(Params):
"disturb it. Requires a project, or a work item that belongs to "
"one. Unset, the purpose's dispatch profile decides (WI-1323), and "
"with no purpose the session opens in the project's checkout, as "
"`false` does. Recorded on the audit row."
"`false` does. Cannot be combined with `cwd` (WI-1671). Recorded "
"on the audit row."
),
)
cwd: str | None = Field(
default=None,
description=(
"Open the session in this existing directory instead of the "
"project's registered checkout (WI-1671). Use it when a worktree "
"already exists and `worktree=true` cannot cut a new one — a "
"private origin the engine cannot fetch, for example. The path "
"must exist, must sit under the workspace's `Active/` directory "
"(the parent of the project's checkout), must not be inside "
"that checkout (a subdirectory shares its git toplevel; a "
"linked worktree nested under it does not), and must be a "
"git worktree of the same "
"repository (the same `commondir`). Anything else is refused, "
"and so is combining it with an explicit `worktree=true`. A "
"purpose profile that would have set `worktree` does not, when "
"`cwd` is given. Recorded as the session's cwd."
),
)
reason: Reason = Field(description="Why this write is being made (audited).")
Expand Down
143 changes: 142 additions & 1 deletion src/vogt/application/services/sessions.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@

from __future__ import annotations

import os
import re
import subprocess
import time
from datetime import datetime, timedelta
from pathlib import Path
Expand Down Expand Up @@ -124,6 +126,7 @@
from vogt.application.services.preferences import dispatch_settings
from vogt.application.services.views import why
from vogt.application.writes import WriteOutcome, audited_action, audited_write
from vogt.collectors.base import CollectorError
from vogt.core import delivery, oversight, runtime
from vogt.core.auth import Scope, hash_token, issue, parse_scopes
from vogt.core.branches import default_branch_name
Expand Down Expand Up @@ -251,14 +254,27 @@ def start_session(ctx: AppContext, params: StartSessionParams) -> SessionResult:
# Resolved only after both guards above, and a profile carries neither a
# role nor a permission posture, so naming a purpose can never raise a
# start past what the caller could ask for directly (WI-1323).
# Before the profile fills unset fields (WI-1671). Only a worktree the
# caller asked for conflicts with `cwd`: the implement profile sets
# `worktree` itself, and that is the case `cwd` exists for.
if params.cwd and "worktree" in params.model_fields_set and params.worktree:
msg = (
"cwd and worktree cannot be combined: worktree cuts a new "
"checkout, cwd opens one that already exists"
)
raise InvalidRequest(msg)
params, overrides, read_only = _resolve_purpose(ctx, params)
if params.cwd and params.worktree:
params = params.model_copy(update={"worktree": None})
# After the profile fills the model, before anything reaches the engine:
# a project's review policy is a rule, and a start that would run a model
# it forbids is refused rather than spawned (WI-1590).
_refuse_forbidden_model(ctx, params)
started_by_session, started_by_kind = _started_by(ctx, engine)
session_id = ctx.id_factory("ses")
subject = _subject(ctx, params, session_id)
if params.cwd:
subject.cwd = _existing_worktree_cwd(params.cwd, subject.cwd)
actor_ref = f"agent:session:{session_id}"
session_scopes = _session_scopes(ctx)
credential = issue(session_scopes)
Expand Down Expand Up @@ -333,7 +349,7 @@ def start_session(ctx: AppContext, params: StartSessionParams) -> SessionResult:
# at the registry's root, which is what the engine reports back anyway.
cwd = (
started.cwd
if started.cwd and (params.resume or params.worktree)
if started.cwd and (params.resume or params.worktree or params.cwd)
else subject.cwd
)

Expand Down Expand Up @@ -3094,6 +3110,131 @@ def default_name(self) -> str:
return self.work_item_ref or self.project_slug


def _git_probe(root: Path, flag: str) -> str:
"""One `git rev-parse` question, with nothing the repo can run.

`rev-parse` runs no hooks. `core.fsmonitor` and `core.hooksPath` are
overridden so a repo's monitor and hooks are not consulted, and
`GIT_CONFIG_NOSYSTEM` drops the system gitconfig. Inherited `GIT_DIR`,
`GIT_WORK_TREE` and `GIT_COMMON_DIR` are removed so the engine's own
environment cannot point the probe at a different repository.
"""

env = {
key: value
for key, value in os.environ.items()
if key not in {"GIT_DIR", "GIT_WORK_TREE", "GIT_COMMON_DIR", "GIT_CONFIG_COUNT"}
}
env["GIT_CONFIG_NOSYSTEM"] = "1"
try:
completed = subprocess.run( # fixed argv, never a shell
[
"git",
"-C",
str(root),
"-c",
"core.fsmonitor=",
"-c",
"core.hooksPath=/dev/null",
"rev-parse",
flag,
],
capture_output=True,
text=True,
timeout=20,
check=False,
env=env,
)
except (OSError, subprocess.SubprocessError) as exc:
msg = f"git could not be run against {root}: {exc}"
raise CollectorError(msg) from exc
if completed.returncode != 0:
detail = completed.stderr.strip() or f"exit {completed.returncode}"
msg = f"git rev-parse could not be read in {root}: {detail}"
raise CollectorError(msg)
return completed.stdout.strip()


def _existing_worktree_cwd(raw: str, project_root: str) -> str:
"""The directory a `cwd` start opens in, or a refusal (WI-1671).

Four checks, in the order a wrong path fails them: it exists, it sits
under the workspace's real `Active/` directory (the parent of the
project's registered checkout, not any path component named `Active`),
it is not inside that checkout (its git toplevel differs, so a linked
worktree nested under the checkout still passes), and it is a git
worktree of the same repository. The comparison is git's `commondir`,
which a linked worktree and the main checkout of one repository share
and two repositories do not.
"""
path = Path(raw).expanduser()
if not path.is_absolute():
msg = f"cwd {raw!r} must be an absolute path"
raise InvalidRequest(msg)
if not path.is_dir():
msg = f"cwd {raw!r} does not exist"
raise InvalidRequest(msg)
try:
resolved = path.resolve(strict=True)
except OSError as exc:
msg = f"cwd {raw!r} cannot be resolved: {exc}"
raise InvalidRequest(msg) from exc
checkout = Path(project_root)
if not checkout.is_dir():
msg = (
f"the project's checkout {project_root!r} does not exist, so "
"cwd cannot be checked against it"
)
raise InvalidRequest(msg)
checkout_resolved = checkout.resolve()
active = checkout_resolved.parent
if active.name != "Active" or not resolved.is_relative_to(active):
msg = f"cwd {resolved} is not under the workspace Active/ directory ({active})"
raise InvalidRequest(msg)
try:
asked = _git_probe(resolved, "--git-common-dir")
owned = _git_probe(checkout_resolved, "--git-common-dir")
# A subdirectory of the shared checkout shares its commondir and is
# under Active/, so the path comparison above misses it. Its
# toplevel is the checkout, which a linked worktree's is not.
toplevel = _git_probe(resolved, "--show-toplevel")
except CollectorError as exc:
msg = (
f"cwd {resolved} is not a git worktree of the project's "
f"repository ({project_root}): {exc}"
)
raise InvalidRequest(msg) from exc
asked_dir = Path(asked)
owned_dir = Path(owned)
if not asked_dir.is_absolute():
asked_dir = resolved / asked_dir
if not owned_dir.is_absolute():
owned_dir = checkout / owned_dir
try:
same = asked_dir.resolve() == owned_dir.resolve()
except OSError as exc:
msg = f"cwd {raw!r} is not a git worktree of the project's repository: {exc}"
raise InvalidRequest(msg) from exc
if not same:
msg = (
f"cwd {resolved} is not a git worktree of the project's "
f"repository ({project_root})"
)
raise InvalidRequest(msg)
try:
top = Path(toplevel).resolve()
except OSError as exc:
msg = f"cwd {resolved} is not a git worktree: {exc}"
raise InvalidRequest(msg) from exc
if top == checkout_resolved:
msg = (
f"cwd {resolved} is inside the project's shared checkout; name a "
"worktree of it instead"
)
raise InvalidRequest(msg)
return str(resolved)


def _worktree_branch(subject: _Subject, session_id: str) -> str:
"""The branch a session's own worktree is cut onto (WI-1298).

Expand Down
5 changes: 4 additions & 1 deletion src/vogt/registry/operations.py
Original file line number Diff line number Diff line change
Expand Up @@ -1019,7 +1019,10 @@ def build_operations() -> list[Operation[Any, Any]]:
"Open a coding session (a terminal) for a work item or a "
"project. With `template` (e.g. claude) it runs that agent, "
"and a `task` is delivered as the agent's first prompt, so it "
"starts working instead of opening idle. Returns both ids "
"starts working instead of opening idle. `cwd` opens it in an "
"existing worktree under the workspace Active/ directory "
"instead of the project's checkout; it cannot be combined "
"with `worktree=true`. Returns both ids "
"(ses_… and engine_session_id); then poll session_screen "
"until `ready`."
),
Expand Down
128 changes: 128 additions & 0 deletions tests/test_sessions.py
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,134 @@ def test_a_worktree_session_is_cut_from_the_integration_branch(
assert "worktree" not in engine.last_spec


def test_a_cwd_opens_in_an_existing_worktree(
wired: AppContext,
engine: StandInEngine,
tmp_path: Path,
monkeypatch: pytest.MonkeyPatch,
) -> None:
"""WI-1671: name a worktree that already exists and start in it.

The engine is handed that directory as cwd and is not asked to cut one.
The Active/ check is the parent of the project's checkout, so a directory
that merely has an `Active` component elsewhere is refused, and so is the
shared checkout itself. A relative path, a stranger repository, a missing
directory, and an explicit worktree=true are refused before any session
is started. A symlink is accepted as the directory it points at.
"""
active_root = tmp_path / "Active"
checkout = active_root / "vogt"
checkout.mkdir(parents=True)
worktree = active_root / "vogt-wi-7"
worktree.mkdir()
linked = active_root / "vogt-link"
linked.symlink_to(worktree, target_is_directory=True)
stranger = active_root / "other"
stranger.mkdir()
decoy = tmp_path / "decoy" / "Active" / "evil"
decoy.mkdir(parents=True)
common = tmp_path / "common.git"
other = tmp_path / "other.git"
seen: list[Path] = []

nested = checkout / "src"
nested.mkdir()

def fake_git(root: Path, flag: str) -> str:
seen.append(root)
if flag == "--show-toplevel":
# A subdirectory of the checkout reports the checkout. A linked
# worktree reports itself.
if root.resolve() in {worktree.resolve(), linked.resolve()}:
return str(worktree.resolve())
return str(checkout.resolve())
if root.resolve() in {worktree.resolve(), checkout.resolve(), nested.resolve()}:
return str(common)
return str(other)

monkeypatch.setattr("vogt.application.services.sessions._git_probe", fake_git)
register_project(
wired,
RegisterProjectParams(name="Checkout", root_path=str(checkout), reason=WHY),
)

result = start_session(
wired,
StartSessionParams(project="checkout", cwd=str(worktree), reason=WHY),
)
assert engine.last_spec["cwd"] == str(worktree.resolve())
assert "worktree" not in engine.last_spec
assert result.session.cwd == str(worktree.resolve())

linked_result = start_session(
wired,
StartSessionParams(project="checkout", cwd=str(linked), reason=WHY),
)
assert linked_result.session.cwd == str(worktree.resolve())

with pytest.raises(InvalidRequest, match="absolute"):
start_session(
wired,
StartSessionParams(project="checkout", cwd="Active/vogt-wi-7", reason=WHY),
)
with pytest.raises(InvalidRequest, match="shared checkout"):
start_session(
wired,
StartSessionParams(project="checkout", cwd=str(checkout), reason=WHY),
)
with pytest.raises(InvalidRequest, match="shared checkout"):
start_session(
wired,
StartSessionParams(project="checkout", cwd=str(nested), reason=WHY),
)
with pytest.raises(InvalidRequest, match="not under the workspace Active"):
start_session(
wired,
StartSessionParams(project="checkout", cwd=str(decoy), reason=WHY),
)
with pytest.raises(InvalidRequest, match="not a git worktree"):
start_session(
wired,
StartSessionParams(project="checkout", cwd=str(stranger), reason=WHY),
)
with pytest.raises(InvalidRequest, match="does not exist"):
start_session(
wired,
StartSessionParams(
project="checkout",
cwd=str(active_root / "missing"),
reason=WHY,
),
)
with pytest.raises(InvalidRequest, match="cannot be combined"):
start_session(
wired,
StartSessionParams(
project="checkout",
cwd=str(worktree),
worktree=True,
reason=WHY,
),
)
# The implement profile sets worktree itself. A cwd still wins, and the
# engine is not asked to cut one.
purpose = start_session(
wired,
StartSessionParams(
project="checkout",
cwd=str(worktree),
purpose="implement",
reason=WHY,
),
)
assert purpose.session.cwd == str(worktree.resolve())
assert "worktree" not in engine.last_spec
# The refusals never reached the engine: two accepted starts, then the
# purpose start.
assert engine.counter == 3
assert decoy.resolve() not in seen


def test_the_whole_session_clock_is_unset_unless_the_project_sets_it(
wired: AppContext, engine: StandInEngine
) -> None:
Expand Down
Loading