Skip to content

Commit aa574fc

Browse files
Reject the ttyd transport structurally, not silently (ttyd retirement stage 3/7)
Closes R-05. Before this stage, an unrecognised value for `transport` — including the retired `ttyd` — silently resolved to `pty`, both in the request body (_resolve_transport fell through to a default) and via the STUDYLOOP_TRANSPORT env-var kill switch. A caller asking for a transport that no longer exists got a different one with no error, which is worse than a clean rejection: it hides the fact that the thing asked for is gone. `StartSessionRequest.transport` is now `Literal["pty", "acp"] | None`, so Pydantic itself 422s an unrecognised body value before the handler runs. `_resolve_transport()` raises the new `UnsupportedTransportError` for any non-empty `STUDYLOOP_TRANSPORT` other than "pty"; `start_session()` catches it and returns 422. Both paths are fixed in this one commit, per REMEDIATION-PLAN's Gate 1 amendment #2, with a mandatory test for each: `{"transport": "ttyd"}` -> 422, and `STUDYLOOP_TRANSPORT=ttyd` -> 422 (not silently pty). Deletes `_start_ttyd_session()` and `_ttyd_credentials()` (_start.py) — the dispatcher's fallback branch that reached the legacy tmux+ttyd path is gone along with them. `app.state.lan_username`/`lan_password` (web/app.py) are removed too: `_ttyd_credentials()` was their only reader, and the plan's own instruction is to leave no false authority markers rather than keep unread state whose documented purpose no longer applies. Real LAN Basic-Auth is unaffected — `create_app()` still wires `BasicAuthMiddleware` directly from its username/password parameters, which never went through app.state. Test reconciliation, beyond the manifest's literal line numbers where its granularity didn't reach: TestLanCredentialsOnAppState (test_lan_auth.py, 5 tests) is deleted as one unit — its whole premise (app.state as the ttyd/app auth divergence guard) is gone; test_start_rejects_no_tmux (test_web_session.py) is deleted since no surviving transport touches tmux, so its 503-on-no-multiplexer assertion has no home. Full ledger and rationale for each in evidence/M1/stage-3/04-manifest.md. Pass count 3875 -> 3870, exactly at the strict-manifest floor (3877 - 4 - 3 = 3870). Docs describing the ttyd server transport as still available to maintainers (system-overview.md, architecture/current.md:149, cli-reference.md, troubleshooting.md) are now stale and deliberately left for stage 7 per PLAN-retire-ttyd.md's cut-point rule ("not shippable in doc terms mid-flight ... never cut 0.1.0 mid-flight"). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 3e9d370 commit aa574fc

9 files changed

Lines changed: 114 additions & 385 deletions

File tree

‎packages/studyloop/src/studyloop/web/app.py‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -102,12 +102,6 @@ def create_app(
102102
# Resolved (and validated) up front so a bad --dev-engine fails at startup,
103103
# not on first page load.
104104
app.state.dev_engine = resolve_dev_engine(dev_engine) if dev_mode else None
105-
# Single source of truth for LAN Basic-Auth credentials. The ttyd start
106-
# path reads these instead of independently re-loading config.yaml, so the
107-
# app's auth and ttyd's auth can never silently diverge (a CLI --password
108-
# that was never written to config used to leave ttyd unauthenticated).
109-
app.state.lan_username = username
110-
app.state.lan_password = password
111105
app.state.explorer_tree_cache = None
112106
app.state.explorer_tree_fingerprint = None
113107
app.state.session_options_targets_cache = None

‎packages/studyloop/src/studyloop/web/routes/session/_models.py‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
from __future__ import annotations
44

5+
from typing import Literal
6+
57
from pydantic import BaseModel, Field
68

79

@@ -11,14 +13,15 @@ class StartSessionRequest(BaseModel):
1113
topic: str
1214
energy: int = Field(default=5, ge=1, le=10)
1315
agent: str | None = None
14-
transport: str | None = Field(
16+
transport: Literal["pty", "acp"] | None = Field(
1517
default=None,
1618
description=(
17-
"Session transport: 'pty' (default), 'ttyd' (legacy fallback), or "
18-
"'acp' (Agent Client Protocol, available for Kiro). "
19-
"STUDYLOOP_TRANSPORT env var forces 'pty' or 'ttyd' regardless of "
20-
"this field; 'acp' is body-only to keep the kill-switch semantics "
21-
"focused on the safe paths."
19+
"Session transport: 'pty' (default) or 'acp' (Agent Client "
20+
"Protocol, available for Kiro). Any other value — including the "
21+
"retired 'ttyd' — is rejected with 422, not silently downgraded. "
22+
"STUDYLOOP_TRANSPORT=pty is the only accepted env-var override; "
23+
"'acp' is body-only to keep the kill-switch semantics focused on "
24+
"the safe path."
2225
),
2326
)
2427

‎packages/studyloop/src/studyloop/web/routes/session/_start.py‎

Lines changed: 22 additions & 247 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
"""POST /session/start — PTY, ACP, and legacy ttyd paths."""
1+
"""POST /session/start — PTY and ACP session start."""
22

33
from __future__ import annotations
44

55
import hashlib
66
import logging
77
from datetime import UTC, datetime
88

9-
from fastapi import HTTPException, Request
9+
from fastapi import Request # noqa: TC002 - FastAPI needs Request at runtime for injection.
1010
from fastapi.responses import JSONResponse
1111

1212
from studyloop.session_state import (
@@ -19,6 +19,7 @@
1919
from studyloop.web.routes.session._models import _AGENT_INSTALL_HINTS, StartSessionRequest
2020
from studyloop.web.routes.session._router import router
2121
from studyloop.web.routes.session._transport import (
22+
UnsupportedTransportError,
2223
_resolve_transport,
2324
)
2425
from studyloop.web.services.session_start import (
@@ -124,29 +125,6 @@ async def _resolve_origin(request: Request) -> str:
124125
return origin
125126

126127

127-
def _ttyd_credentials(request: Request | None) -> tuple[str, str]:
128-
"""Resolve ttyd Basic-Auth creds from the app's single source of truth.
129-
130-
Reads ``(lan_username, lan_password)`` off ``request.app.state`` — the same
131-
values ``create_app`` used to install ``BasicAuthMiddleware``. Fails closed:
132-
if app.state is unreadable (no request wired through), refuse rather than
133-
guess, because a wrong guess spawns an unauthenticated PTY on the LAN.
134-
"""
135-
state = getattr(getattr(request, "app", None), "state", None)
136-
if state is None:
137-
raise HTTPException(
138-
status_code=500,
139-
detail=(
140-
"Cannot start ttyd: LAN credentials unavailable from app state "
141-
"(auth divergence guard)."
142-
),
143-
)
144-
return (
145-
getattr(state, "lan_username", "") or "",
146-
getattr(state, "lan_password", "") or "",
147-
)
148-
149-
150128
@router.post("/session/start")
151129
async def start_session(body: StartSessionRequest, request: Request) -> JSONResponse:
152130
"""Start a new study session from the web UI.
@@ -156,10 +134,16 @@ async def start_session(body: StartSessionRequest, request: Request) -> JSONResp
156134
- ``pty`` (default, plan §1.5b) — spawns the agent directly via
157135
``PTYTransport`` + ``active.acquire`` and returns a ``ws_url``
158136
that the browser feeds to ``/api/session/ws``. No tmux, no ttyd.
159-
- ``ttyd`` (legacy, plan §1.9 fallback) — runs the original
160-
tmux+ttyd flow for one deprecation window. Enable explicitly via
161-
``{"transport": "ttyd"}`` in the body or by exporting
162-
``STUDYLOOP_TRANSPORT=ttyd``.
137+
- ``acp`` (Agent Client Protocol, available for Kiro) — body-only,
138+
never selectable via ``STUDYLOOP_TRANSPORT``.
139+
140+
``transport`` is validated structurally by ``StartSessionRequest``
141+
(``Literal["pty", "acp"]``), so an unrecognised body value — including
142+
the retired ``ttyd`` — never reaches this handler; FastAPI rejects it
143+
with 422 first. ``STUDYLOOP_TRANSPORT`` is checked at runtime by
144+
``_resolve_transport`` and raises :class:`UnsupportedTransportError` for
145+
the same reason: a caller asking for a transport that no longer exists
146+
must see an error, not silently get ``pty`` instead (R-05).
163147
164148
Declared ``async`` so the PTY path's ``active.acquire`` runs on the
165149
FastAPI event loop — installing the SIGCHLD signal handler requires
@@ -174,12 +158,17 @@ async def start_session(body: StartSessionRequest, request: Request) -> JSONResp
174158
status_code=400,
175159
)
176160

177-
transport = _resolve_transport(body.transport)
178-
if transport == "pty":
179-
return await _start_pty_session(body, origin)
161+
try:
162+
transport = _resolve_transport(body.transport)
163+
except UnsupportedTransportError as exc:
164+
return JSONResponse(
165+
{"error": f"Unsupported transport: {exc.args[0]!r}. Allowed: ['pty', 'acp']"},
166+
status_code=422,
167+
)
168+
180169
if transport == "acp":
181170
return await _start_acp_session(body, origin)
182-
return _start_ttyd_session(body, origin, request)
171+
return await _start_pty_session(body, origin)
183172

184173

185174
async def _start_pty_session(
@@ -594,217 +583,3 @@ async def _start_acp_session(
594583
},
595584
status_code=201,
596585
)
597-
598-
599-
def _start_ttyd_session(
600-
body: StartSessionRequest, origin: str = _DEFAULT_ORIGIN, request: Request | None = None
601-
) -> JSONResponse:
602-
"""Legacy tmux start path (plan §1.9 emergency fallback), ttyd removed.
603-
604-
Kept as-is to guarantee a deprecation window. New development should
605-
target the PTY path above.
606-
607-
``request`` is unused since ttyd retirement stage 2 removed the spawn
608-
that read Basic-Auth credentials from ``app.state`` — kept only for the
609-
legacy CLI caller's signature until stage 3 deletes this function
610-
entirely.
611-
"""
612-
import os
613-
import shutil
614-
from pathlib import Path
615-
616-
from studyloop.multiplexer import get_backend
617-
618-
mux = get_backend()
619-
620-
# --- Pre-flight ---
621-
622-
if not mux.is_available():
623-
return JSONResponse(
624-
{"error": "Terminal multiplexer is required but not available"},
625-
status_code=503,
626-
)
627-
628-
from studyloop.web.routes import session as session_pkg
629-
630-
if session_pkg.is_session_active():
631-
return JSONResponse(
632-
{"error": "A session is already active"},
633-
status_code=409,
634-
)
635-
636-
# --- Resolve agent ---
637-
638-
from studyloop.agent_launcher import AGENTS, detect_agents
639-
640-
agent = body.agent
641-
if agent and agent not in AGENTS:
642-
return JSONResponse(
643-
{"error": f"Unknown agent: {agent}"},
644-
status_code=400,
645-
)
646-
if not agent:
647-
available = detect_agents()
648-
if not available:
649-
return JSONResponse(
650-
{"error": "No AI agent found on this machine"},
651-
status_code=503,
652-
)
653-
agent = available[0]
654-
655-
# Check agent binary is installed
656-
adapter = AGENTS[agent]
657-
if not shutil.which(adapter.binary):
658-
return JSONResponse(
659-
{"error": f"Agent '{agent}' binary not found: {adapter.binary}"},
660-
status_code=503,
661-
)
662-
663-
# --- Resolve topic config ---
664-
665-
topic_config = None
666-
try:
667-
from studyloop.logic.topic_resolver import resolve_topic
668-
from studyloop.settings import load_settings
669-
670-
settings = load_settings()
671-
if settings.topics:
672-
result = resolve_topic(body.topic, settings.topics)
673-
topic_config = result.resolved or (result.matches[0] if result.matches else None)
674-
except Exception:
675-
pass # Topic resolution is optional
676-
677-
# --- Clean zombies ---
678-
679-
try:
680-
from studyloop.session.cleanup import auto_clean_zombies
681-
682-
auto_clean_zombies()
683-
except Exception:
684-
pass
685-
686-
# --- Create DB session ---
687-
688-
from studyloop.history import start_study_session
689-
from studyloop.output import energy_to_label
690-
691-
energy_label = energy_to_label(body.energy)
692-
study_id = start_study_session(
693-
body.topic,
694-
energy_label,
695-
topic_slug=topic_config.slug if topic_config else None,
696-
)
697-
if not study_id:
698-
return JSONResponse(
699-
{"error": "Failed to create session record"},
700-
status_code=500,
701-
)
702-
703-
# --- Write session state ---
704-
705-
_ensure_session_dir()
706-
now = datetime.now(UTC).isoformat()
707-
write_session_state(
708-
{
709-
"study_session_id": study_id,
710-
"topic": body.topic,
711-
"energy": body.energy,
712-
"energy_label": energy_label,
713-
"mode": "focus",
714-
"timer_mode": "energy",
715-
"started_at": now,
716-
"start_time": now,
717-
"paused_at": None,
718-
"total_paused_seconds": 0,
719-
"origin": origin,
720-
}
721-
)
722-
TOPICS_FILE.touch(mode=0o600, exist_ok=True)
723-
PARKING_FILE.touch(mode=0o600, exist_ok=True)
724-
725-
# --- Session directory + tmux ---
726-
# slug_session_dir strips path-traversal from the user-controlled topic;
727-
# this session_name becomes a path segment (and is later rmtree'd on
728-
# cleanup), so an unsanitised "../.." here is a real escape vector.
729-
from studyloop.web.services.session_start import slug_session_dir
730-
731-
slug = slug_session_dir(body.topic)
732-
short_id = study_id[:8]
733-
session_name = f"study-{slug}-{short_id}"
734-
session_dir = SESSION_DIR / "sessions" / session_name
735-
736-
if mux.session_exists(session_name):
737-
mux.kill_session(session_name)
738-
739-
from studyloop.agent_launcher import build_canonical_persona
740-
from studyloop.session.orchestrator import (
741-
build_wrapped_agent_cmd,
742-
create_tmux_environment,
743-
setup_session_dir,
744-
)
745-
746-
setup_session_dir(session_dir, body.topic)
747-
748-
# Build persona
749-
canonical = build_canonical_persona("focus", body.topic, body.energy)
750-
persona_hash = hashlib.sha256(canonical.encode()).hexdigest()[:16]
751-
752-
from studyloop.history.sessions import update_persona_hash
753-
754-
update_persona_hash(study_id, persona_hash)
755-
756-
persona_file = adapter.setup(canonical, session_dir)
757-
if adapter.mcp_setup:
758-
adapter.mcp_setup(session_dir)
759-
760-
# Allow test injection
761-
test_agent_cmd = os.environ.get("STUDYLOOP_TEST_AGENT_CMD")
762-
if test_agent_cmd:
763-
agent_cmd = test_agent_cmd.format(persona_file=persona_file)
764-
else:
765-
# Check if session dir has prior agent history (resuming)
766-
claude_project_key = str(session_dir).replace("/", "-").lstrip("-")
767-
claude_project_dir = Path.home() / ".claude" / "projects" / claude_project_key
768-
is_resuming = claude_project_dir.exists()
769-
agent_cmd = adapter.launch_cmd(persona_file, is_resuming)
770-
771-
wrapped_cmd = build_wrapped_agent_cmd(session_dir, agent_cmd)
772-
773-
result = create_tmux_environment(
774-
session_name=session_name,
775-
session_dir=session_dir,
776-
wrapped_agent_cmd=wrapped_cmd,
777-
session_state_dir=SESSION_DIR,
778-
sidebar=False,
779-
)
780-
781-
# Persist tmux metadata
782-
state_update: dict = {
783-
"tmux_session": session_name,
784-
"tmux_main_pane": result["tmux_main_pane"],
785-
"tmux_sidebar_pane": result["tmux_sidebar_pane"],
786-
"persona_file": str(persona_file),
787-
"session_dir": str(session_dir),
788-
"agent": agent,
789-
"persona_hash": persona_hash,
790-
}
791-
if topic_config:
792-
state_update["topic_slug"] = topic_config.slug
793-
state_update["topic_config_name"] = topic_config.name
794-
write_session_state(state_update)
795-
796-
# ttyd is no longer spawned here (stage 2 of the ttyd retirement removed
797-
# the spawn entirely — see PLAN-retire-ttyd.md). _ttyd_credentials() and
798-
# app.state.lan_username/lan_password are now dead below this point;
799-
# stage 3 deletes them along with the rest of this legacy path.
800-
801-
return JSONResponse(
802-
{
803-
"study_session_id": study_id,
804-
"topic": body.topic,
805-
"energy": body.energy,
806-
"session_name": session_name,
807-
"agent": agent,
808-
},
809-
status_code=201,
810-
)

0 commit comments

Comments
 (0)