fix(sessions): surface working_directory and agent_profile on list_sessions

[tedswinyar]

A single cao-server serves every session from one flat namespace filtered only by the cao- prefix, so list_sessions returns the sessions of every orchestrator using that server, with no way to tell them apart. When more than one orchestration runs against the same server (for example, two people driving CAO from different project directories, or one person running separate sessions per repo), a caller cannot distinguish its own sessions from another's. This surfaced in practice: an external orchestrator listed the sessions, saw sessions it did not recognize, and nearly shut them down as orphans while they were doing live work.

This change adds ownership metadata to each list_sessions entry so a caller can identify which sessions are its own. Each entry now carries a working_directory and an agent_profile. The working directory is the strongest signal, since a given orchestrator normally drives one project directory.

How the working directory is resolved. A new nullable working_directory column is added to the terminals table and populated with the launch-time directory when a terminal is created. list_sessions prefers that persisted value, falls back to the live tmux pane's current path when it is absent, and returns null when neither can be resolved. Persisting the launch-time value matters because the pane's current path drifts if the agent changes directory during its run, so the pane path alone is an unreliable ownership marker.

Scope and safety. The change is additive. The cao- prefix filter is unchanged, the new fields are optional, and enrichment is best-effort per session: if one session's metadata cannot be resolved, that entry gets null fields and the rest of the list is returned normally rather than the whole call failing. The database migration follows the existing caller_id column pattern and is idempotent. Existing callers of list_sessions (the web UI, cao session list, the HTTP /sessions endpoint) keep working, and the new SessionListEntry model allows extra fields so nothing the backend already returned is dropped.

This is a tactical fix, not the whole story. Surfacing the working directory lets a caller identify ownership by convention, but it does not enforce anything: any caller can still see and shut down any session. The deeper question is whether the shared server should scope or namespace sessions per orchestrator at all, and how that should interact with the web UI and CLI, which also list every session. That belongs in a design discussion with the maintainers, and I will open a separate issue and PR for it rather than fold a larger behavioral change into this one. This PR is the low-risk, immediately useful piece.

Tests. Drives list_sessions against a backend with a known working directory and asserts the persisted value is returned; asserts the pane-path fallback fires when the persisted value is null; asserts an orphan session (tmux present, no database row) lists with null fields rather than crashing; asserts one unresolvable session does not blank the whole list; and covers migration idempotency against both a fresh database and one that predates the column. Relevant suites pass (session service, database, ops MCP server).

[codecov-commenter]

Codecov Report

❌ Patch coverage is 93.10345% with 4 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (main@752be53). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
...cli_agent_orchestrator/services/session_service.py	89.47%	4 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #497   +/-   ##
=======================================
  Coverage        ?   91.14%           
=======================================
  Files           ?      179           
  Lines           ?    23321           
  Branches        ?        0           
=======================================
  Hits            ?    21256           
  Misses          ?     2065           
  Partials        ?        0           

Flag	Coverage Δ
unittests	91.14% <93.10%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[fanhongy]

Summary

The additive response shape is compatible and the focused tests pass, but two lifecycle cases make the new ownership metadata unreliable. Both can cause an orchestrator to associate a live session with the wrong project.

Findings

P2: Persist the effective launch directory, not the raw optional argument

/tmp/cao-pr-review-awslabs-cli-agent-orchestrator-pr-497/checkout/src/cli_agent_orchestrator/services/terminal_service.py:308

The new database write stores working_directory exactly as received even though the backend resolves a different effective value. POST /sessions and ops MCP launch_session both allow the argument to be omitted; tmux/herdr then launch in os.getcwd(), but this line stores NULL. After the pane changes directory, list_sessions falls back to that drifting live path, which is the behavior persistence was intended to prevent. A relative input such as . is also persisted literally while tmux launches in a canonical absolute path; enrichment prefers the persisted . and never consults the pane. This makes sessions launched with the default or a relative path fail as stable, comparable ownership signals.

Resolve the effective canonical directory once before backend creation and pass that same value to both the backend and db_create_terminal, or read the newly created pane's directory immediately and persist it. Add coverage for omitted and relative launch paths, including a later pane-CWD change.

P2: Stale terminal rows misattribute a reused session name

/tmp/cao-pr-review-awslabs-cli-agent-orchestrator-pr-497/checkout/src/cli_agent_orchestrator/services/session_service.py:102

Enrichment accepts the first persisted profile/directory for a session without checking that the terminal row belongs to the current backend session incarnation. If a tmux session is killed outside CAO, its DB rows remain until retention cleanup. Relaunching the same custom session name creates another row but does not remove the old rows; the current SQLite query returns the older row first, and this loop immediately reports its directory/profile. A focused reproduction with /old/project followed by /new/project returned /old/project and the old profile for the live cao-reused session.

When creating a new backend session after session_exists is false, purge stale terminal rows for that session name before recording the new terminal, or otherwise validate terminal rows against live windows/session incarnation before using them. Add a regression test for external backend deletion followed by same-name relaunch.

P3: Remove the new changed-source mypy error

/tmp/cao-pr-review-awslabs-cli-agent-orchestrator-pr-497/checkout/src/cli_agent_orchestrator/services/session_service.py:114

next((...), None) introduces Argument 2 to "next" has incompatible type "None"; expected "dict[str, Any]" [arg-type]. Repository-wide mypy is non-blocking, but changed-source checking isolates this as the only error in the four modified source files. Use an empty-dict sentinel, an explicitly typed helper, or a small loop so the added code remains type-clean.

Validation

• uv run pytest test/services/test_session_service.py test/clients/test_database.py test/ops_mcp_server/test_server.py test/services/test_terminal_service_full.py -q - 144 passed
• uv run black --check src/ test/ - passed
• uv run isort --check-only src/ test/ - passed
• git diff --check 77befe84e69c0785c25ec024bb7e6391202cf305...HEAD - passed
• Changed-source mypy - failed only at session_service.py:114
• Focused reproductions confirmed raw . resolves to an absolute launch path and stale-first rows produce the old directory/profile

[gutosantos82]

PR Review: #497 — fix(sessions): surface working_directory and agent_profile on list_sessions

Summary

This PR addresses a real multi-orchestrator hazard (an external orchestrator nearly shut down another caller's live sessions as "orphans") by enriching list_sessions entries with working_directory and agent_profile. The design is additive and careful: a nullable DB column following the caller_id migration pattern, best-effort per-session enrichment that never blanks the list, a pane-cwd fallback, and a SessionListEntry model with extra="allow" so existing fields survive. All 144 change-selected tests pass in the PR worktree and CI is fully green. However, two lifecycle gaps — independently verified here — mean the persisted "launch-time directory" (the PR's own headline design point) is absent or wrong in exactly the common cases: default launches persist NULL (degrading to the drifting pane path the PR says persistence exists to prevent), and stale terminal rows from an externally killed session misattribute a relaunched same-name session to the old project. Recommend one round of changes.

Blocking (must fix before merge)

• [correctness] src/cli_agent_orchestrator/services/terminal_service.py:308 — the DB write persists the raw optional argument, not the effective launch directory. Verified structurally: TmuxClient.create_session calls _resolve_and_validate_working_directory() internally (tmux.py:135), which resolves None to a default and canonicalizes relative paths — but db_create_terminal receives the pre-resolution value. Consequences: (a) any launch that omits working_directory (the default for cao launch without --working-directory, and for handoff/assign unless CAO_ENABLE_WORKING_DIRECTORY=true) persists NULL, so list_sessions falls back to the live pane path — which drifts when the agent cds, which is precisely the unreliability the PR body says persistence was designed to prevent; (b) a relative input like . is persisted literally and, being truthy, is preferred over the pane path, so it never self-corrects into a comparable absolute path. Fix: resolve the effective canonical directory once and pass the same value to both the backend and db_create_terminal (or read the newly created pane's cwd immediately after creation and persist that). Add coverage for omitted and relative launch paths.
• [correctness] src/cli_agent_orchestrator/services/session_service.py:102 (_enrich_session_ownership) — stale terminal rows misattribute a reused session name. list_terminals_by_session (database.py:597) has no ORDER BY and no liveness/incarnation check; if a tmux session is killed outside CAO, its rows persist until retention cleanup, and relaunching the same custom session name adds a new row without purging the old ones. SQLite returns oldest-first, and the enrichment loop takes the first truthy profile/directory — so the live session reports the previous project's directory and profile. For a feature whose entire purpose is letting an orchestrator decide "is this session mine?" before acting (including shutdown), silently reporting another incarnation's identity is the failure mode that caused the motivating incident. Fix: purge stale rows for a session name when creating a new backend session, or filter rows against live windows before use. Add a regression test for external kill + same-name relaunch. (The maintainer-side review reports a focused reproduction returning /old/project for the live relaunched session; the code structure confirms the mechanism.)

Important (should fix)

• [types] src/cli_agent_orchestrator/services/session_service.py:114 — next((t for t in terminals if t.get("tmux_window")), None) introduces the only changed-source mypy error: Argument 2 to "next" has incompatible type "None"; expected "dict[str, Any]". Verified: mypy on the worktree flags exactly this line (the other errors are pre-existing in unrelated files). Repo-wide mypy is non-blocking, but don't add new errors — a small loop or a typed helper keeps it clean.

Nits (optional)

• [correctness] src/cli_agent_orchestrator/services/session_service.py:131 — the new comment claims the .get("id", "") filter hardens against a backend dict that "must not blank the entire list", but it only covers a missing key. Verified dynamically: a backend returning {"id": None} raises AttributeError in the comprehension, is swallowed by the outer except, and returns [] — blanking the list, the exact failure class the comment says it prevents. (s.get("id") or "") closes it for one character of code.
• [performance] session_service.py list_sessions — now N+1: one DB query per session, plus one tmux subprocess (get_pane_working_directory) per session whose persisted cwd is NULL. Given the Blocking finding above, that is every default-launched and every pre-migration session, on every poll of /sessions (the web UI polls it). Fine at local scale, but worth a note — fixing Blocking #1 also mostly eliminates the subprocess churn.
• [design] _enrich_session_ownership merge semantics — agent_profile and working_directory can come from different terminals (verified dynamically: terminal-1's directory + terminal-2's profile merge into one entry). For supervisor+worker sessions this is usually harmless (first row is the supervisor), but the field description "the session's first known terminal" is not quite what the code does. Either take both fields from the first terminal that has either, or adjust the description.
• [conventions] CHANGELOG.md — no [Unreleased] entry; sibling fixes in this area carry hand-curated entries. Convention-by-precedent.

Tests

Strong for the paths it covers: persisted-value preference (with an AssertionError-armed fake proving the pane is not consulted), pane fallback, per-field failure isolation, orphan sessions, per-session enrichment failure isolation, migration idempotency against fresh and legacy databases, and full round-trip through a real SQLite engine. The ops MCP test pins extra="allow" field preservation (session_name, terminal_count). What's missing maps exactly to the blocking findings: no test launches with an omitted or relative working_directory and asserts what gets persisted, and no test covers external kill + same-name relaunch. The suite tests the read path thoroughly but assumes the write path stored the right thing.

Verification

Dynamic verification ran in a fresh worktree at the PR head (683d78b), PYTHONPATH=<worktree>/src + main-checkout venv (Python 3.12).

• ✓ VERIFIED — change-selected suites (test_session_service.py, test_database.py, test_server.py (ops), test_terminal_service_full.py): 144 passed in 6.6s. Matches the maintainer-side review's count. CI green on 3.10/3.11/3.12 + security scans.
• ✓ VERIFIED — get_pane_working_directory exists on TerminalBackend base, tmux backend, and herdr backend — the fallback works on both backends.
• ✓ VERIFIED — {"id": None} from the backend blanks the entire session list (log: Failed to list sessions: 'NoneType' object has no attribute 'startswith') — see nit.
• ✓ VERIFIED — enrichment does not mutate the backend's session dict (copies first); mixed-terminal merge takes profile and directory from different rows — see nit.
• ✓ VERIFIED — mypy flags session_service.py:114 as the only new changed-source error.
• ✓ VERIFIED (structurally) — TmuxClient.create_session resolves/canonicalizes working_directory after the raw value is already persisted to the DB; nothing writes the resolved value back. Confirms Blocking #1.
• ✓ VERIFIED (structurally) — list_terminals_by_session has no ORDER BY and no liveness filter; no purge of same-name rows on session creation. Confirms the mechanism of Blocking #2 (maintainer-side review reports a live reproduction).

Verdict

Request changes — the direction, API shape, and test discipline are all good, but the persisted launch directory must be the effective resolved directory (not NULL/relative passthrough), and stale same-name rows must not misattribute a live session. Both are cheap, well-localized fixes with clear regression tests, and both sit on the exact code path the feature exists to make trustworthy.

[Copilot]

Pull request overview

This PR adds best-effort ownership metadata to session listings so callers can distinguish “their” sessions from other orchestrators sharing the same cao-server, by surfacing each session’s working_directory and agent_profile.

Changes:

• Persist working_directory in the terminals DB table and plumb it through terminal creation and terminal metadata reads.
• Enrich services.session_service.list_sessions() results with working_directory (persisted preferred, tmux-pane cwd fallback) and agent_profile.
• Update ops MCP server models and expand test coverage across session service, database, terminal service, and ops MCP server.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
test/services/test_terminal_service_full.py	Updates terminal creation test to pass the new working_directory parameter.
test/services/test_session_service.py	Adds coverage for session list enrichment (persisted cwd, pane fallback, orphaned sessions, partial failures).
test/ops_mcp_server/test_server.py	Extends ops MCP list-sessions test to assert the new field is present in dumped session output.
test/clients/test_database.py	Extends DB tests for working_directory read/write and schema migration idempotency.
src/cli_agent_orchestrator/services/terminal_service.py	Persists working_directory into the terminals metadata record during terminal creation.
src/cli_agent_orchestrator/services/session_service.py	Enriches list_sessions() entries with working_directory/agent_profile via DB metadata and tmux fallback.
src/cli_agent_orchestrator/ops_mcp_server/models.py	Introduces SessionListEntry model (extra fields allowed) and updates SessionListResult sessions typing.
src/cli_agent_orchestrator/clients/database.py	Adds working_directory column, migration, and includes it in terminal CRUD/list APIs.

Comments suppressed due to low confidence (1)

src/cli_agent_orchestrator/services/session_service.py:122

• Same as above: this exception is intentionally swallowed, but the log message omits the traceback (and uses eager f-string formatting). Including exc_info=True makes pane-cwd resolution failures actionable in production logs.

            try:
                enriched["working_directory"] = backend.get_pane_working_directory(
                    session_name, terminal["tmux_window"]
                )
            except Exception as e:
                logger.warning(f"Failed to resolve working directory for {session_name}: {e}")

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[tedswinyar]

Thanks @fanhongy and @gutosantos82 — both blocking findings were real and are fixed, along with the mypy and {"id": None} nits. Rebased onto main (4cc40b1) to pick up #513.

Blocking 1 — persist the effective launch directory (terminal_service.py)
The raw optional arg is no longer what gets stored. A new _resolve_working_directory() resolves the cwd once — via the same resolve_and_validate_path the tmux backend uses, defaulting to os.getcwd() when omitted — and that single resolved value is threaded to both the backend create_session/create_window call and db_create_terminal. Omitted launches now persist an absolute path instead of NULL, and . is canonicalized rather than stored literally, so the persisted value no longer loses to (or drifts with) the pane path. Tests assert what actually lands in the DB, not what the read path returns.

Blocking 2 — stale rows misattribute a reused session name (terminal_service.py)
New-session creation now calls delete_terminals_by_session(session_name) immediately after the backend session is confirmed created, so a same-name relaunch after an external kill cannot inherit the previous incarnation's directory or profile. Regression test covers external-kill + same-name relaunch.

Important — mypy: the next(..., None) line is gone; the lookup is now an explicit loop, and mypy reports no new errors in the changed files.

Nit — {"id": None}: fixed with enriched.get("id") or "". A backend returning an explicit None id no longer blanks the whole list.

Nit — merge semantics: agent_profile and working_directory now come from a single ownership_terminal (the first row having either field), so the two can no longer be reported from different terminals. The field description matches the behavior.

Verification

Because the rebase brought in #513 — which adds a second launch path (initial_message / defer_init) through the same create_terminal this PR modifies — the write-path invariant was re-verified on both paths against the real SQLite column (raw typeof(), not the read path). All six cases store the identical canonical absolute path:

working_directory arg	synchronous	deferred-init (#513)
omitted (None)	absolute canonical	absolute canonical
"."	absolute canonical	absolute canonical
absolute	absolute canonical	absolute canonical

Never NULL, never a literal ".". Also confirmed by execution: the stale-row purge fires on the deferred path too (external kill → same-name relaunch reports the new directory/profile, not the old); the purge is an exact tmux_session match, so unrelated and unprefixed rows are untouched; concurrent same-name creates leave the winner's row intact (the loser raises before any purge runs); and a failed backend create_session raises before the purge is reachable. Pre-migration rows with NULL working_directory still fall back to the pane cwd correctly.

Change-selected suites: 157 passed (including #513's new tests). black/isort clean; mypy clean on changed files.

Two items deliberately not changed here

• N+1 in list_sessions (@gutosantos82's perf nit, also raised by Copilot): fixing Blocking 1 removes most of the per-session subprocess churn, since persisted cwd is now populated on every new launch and the pane fallback only fires for pre-migration rows. Collapsing the remaining per-session DB query into one list_all_terminals call is a behavior change I'd rather do as a follow-up than fold into a fix PR — happy to do it here if you'd prefer.
• exc_info=True on the two swallowed-exception logs (Copilot): happy to add if you want it in this PR.

One thing the review surfaced that is not introduced by this PR: session_exists() conflates "absent" with "lookup query failed" on both backends (tmux.py bare except, herdr_backend.py on returncode != 0), so a transient error reads as "session gone". This PR only inherits that; it's the same sentinel class as the lookup-error-as-absence finding on #498. Tracked separately rather than widened into this change.

[fanhongy]

lgtm, merge if @gutosantos82 agree.

[sujoydc]

Verified the round-2 fixes at 876a9c8 against @gutosantos82's original review rather than
the summary. All seven items are accounted for: both blocking findings, the mypy next()
error, the {"id": None} list-blanking nit, the two-terminal merge nit, and the missing
[Unreleased] CHANGELOG entry are closed; the N+1 is openly deferred. Reproduced the test
claim: 157 passed on session service + database + terminal service + ops MCP server, up
from the 144 at the previous head.

Spot-checking the two blocking fixes rather than taking them on faith:

• Blocking 1 is properly closed. _resolve_working_directory passes allow_create=False, allow_file=False, description="Working directory" with the same None -> os.getcwd()
  default as clients/tmux.py::_resolve_and_validate_working_directory, and the single
  resolved value goes to both backend.create_session and db_create_terminal. The
  resulting double resolution is idempotent: /tmp/x canonicalizes to /private/tmp/x on
  both passes, so the tmux client's second pass can't re-transform what was persisted.
• Blocking 2 is properly closed and correctly scoped. delete_terminals_by_session is an
  exact tmux_session == filter, and the session_exists guard raises at line 244 before
  create_session, so the purge at line 260 can only ever touch rows for a name the
  backend reported absent. It can't delete a live incarnation's rows.

One new finding and one scope note.

The change is additive for tmux but not for herdr. herdr_backend does no path
validation of its own, so hoisting resolution into terminal_service.create_terminal adds
a gate on that path rather than relocating one. Details inline on terminal_service.py:152.
I think hoisting is right, and centralizing beats one backend enforcing it — the ask is
just a caveat in the body and CHANGELOG, since "The change is additive" is currently stated
without qualification and a herdr operator will see new ValueErrors from a change that
doesn't look related.

Scope note worth a sentence in the body. Blocking 1 fixed NULL-versus-drift, but when
working_directory is omitted the resolved value is the server's os.getcwd() — the same
string for every caller sharing that server. working_directory is an explicit optional
param on every launch path (api/main.py POST /sessions and /sessions/{name}/terminals,
ops MCP launch_session), and nothing forwards the caller's own cwd automatically. So a
populated working_directory is only an ownership signal when the caller passed one
explicitly. The body frames the directory as "the strongest signal," which holds for
explicit launches but not defaults, and a downstream orchestrator reading a populated field
as proof of ownership is the exact failure this change exists to prevent. Worth stating so
consumers treat the field as a hint rather than an assertion.

[tedswinyar]

Rebased onto current main (0ac5cc9) — this was showing as CONFLICTING, and it's now MERGEABLE again. No code changes from the review round above; this is purely a rebase.

The conflicts were in clients/database.py, services/terminal_service.py and test/services/test_terminal_service_full.py, from #524 (typed memory relationship store) and #470 (kiro v2/KAS engine selection). Both upstream features and this PR's changes are preserved — resolved by keeping both sides.

All five commits are unchanged in content. Verification after the rebase:

• Change-selected suites: 180 passed (up from 157 — upstream has added tests since; the delta is theirs, not new here)
• Baseline on plain main for the same selection is 170, so this PR's 10 tests are present and passing
• The one additional failure in a wider run (test_config_service::test_assembles_full_typed_config) is a pre-existing order-dependent flake — it passes standalone and fails the same way on plain main

@gutosantos82 — your review is pinned to 683d78b, which is two heads behind now (876a9c8, then this rebase). Everything you raised is addressed in the comment above. A re-review when you have a moment would unblock this.

@fanhongy — thanks for the approval; flagging that the head moved for the rebase in case you want another look.

[gutosantos82]

PR Review: #497 — fix(sessions): surface working_directory and agent_profile on list_sessions

Summary

Delta review against the previously reviewed head b8f2bd48 (verdict there: Request changes).
The author's claim that this push is "purely a rebase" is verified: git range-diff shows
the same five commits replayed onto current main (752be53f, the v2.4.1 release); four are
byte-identical and the fifth differs only in CHANGELOG conflict placement. The rebase resolves
the previous round's CONFLICTING gate — the branch is MERGEABLE again and CI is fully green.
Substance is unchanged: this remains good, well-motivated work (ownership metadata on
list_sessions to prevent cross-orchestrator session shutdown), with all of the first round's
blocking findings still fixed. However, the one live blocker from the prior round — the
test-isolation regression introduced by the unmocked delete_terminals_by_session call —
reproduces identically at this head (18 failed / 34 passed on a clean environment, all
sqlite3.OperationalError: no such table: terminals, while upstream main passes 52/52 under
the same conditions). The maintainer's CHANGES_REQUESTED also still stands. One small,
well-scoped fix away from approval.

Blocking (carried forward — re-verified at this head)

• [tests] test/services/test_terminal_service_full.py (TestCreateTerminal, TestCreateTerminalEnvVars) —
  re-verified at eed78037 in a fresh worktree with a clean temp HOME: 18 failed, 34
  passed, every failure sqlite3.OperationalError: no such table: terminals. Baseline
  control: the same file on upstream main (752be53f) under the same clean HOME passes
  52/52, so this is unambiguously a PR regression, not environmental. Mechanism unchanged:
  the new delete_terminals_by_session(session_name) call in create_terminal
  (terminal_service.py, after tmux session creation) is not in the @patch stacks of these
  pre-existing DB-free tests, so it falls through to the real SQLite engine at
  $HOME/.aws/cli-agent-orchestrator/db/. Green CI at this head remains an ordering artifact
  (an earlier test in the full-suite run initializes the DB), and local runs on a developer
  machine silently mutate the developer's real CAO database. Fix remains small: add
  delete_terminals_by_session to the affected @patch stacks, or add an autouse per-test DB
  fixture. Repro: H=$(mktemp -d); HOME=$H python -m pytest test/services/test_terminal_service_full.py -q.

Resolved since last review

• [merge] CONFLICTING gate cleared — the branch is rebased onto current main and reports
  mergeable: MERGEABLE. Verified main is an ancestor of the PR head and the PR diff vs main
  matches the advertised 9 files / +561 −25.

Important (carried forward — unchanged at this head)

• [consistency] terminal_service.py (_resolve_working_directory) — resolution still runs
  for every backend before creation (confirmed at this head: called unconditionally at
  create_terminal step 2), changing the herdr backend's contract (must-exist +
  blocked-system-dir checks it previously didn't apply). Arguably desirable hardening, but
  still undocumented and untested on the herdr path. A one-line statement of intent in the PR
  body/CHANGELOG would close this.
• [security] GET /sessions disclosure — confirmed at this head the endpoint still carries
  no scope dependency (@app.get("/sessions") with no Depends), while this PR adds absolute
  filesystem paths (embedding OS usernames/project names) and agent profiles to its response.
  Missing auth is pre-existing; the disclosure is new. Cheap mitigation: gate with the same
  require_any_scope dependency as /events/history, here or immediately after.

Process note

The maintainer (gutosantos82) review state is still CHANGES_REQUESTED, so
reviewDecision=CHANGES_REQUESTED gates merge regardless of this review. fanhongy has
approved conditional on the maintainer's agreement. Once the test-mocking fix lands, this PR
is in good shape: the first round's substantive blockers (persist effective launch directory,
stale-row purge, mypy, id=None handling) all remain fixed at this head.

[tedswinyar]

Thanks @gutosantos82 — the test-isolation regression was real and is fixed, and your review surfaced a second instance of it that I'd missed. Seven commits, all test-only (src/ untouched, verified across the whole range).

Reproduced your finding first. On eed7803 with a clean HOME: 18 failed / 34 passed, every failure sqlite3.OperationalError: no such table: terminals. Same file on main: 52/52. Confirmed as a PR regression, and confirmed that green CI was the ordering artifact you described.

The fix (b12022f) adds delete_terminals_by_session to the 15 affected @patch stacks in TestCreateTerminal/TestCreateTerminalEnvVars, mocking at the terminal_service import site — matching how test/services/test_flow_service.py already isolates the identical call in flow_service.py. I chose that over an autouse DB fixture because it follows existing convention in this repo and keeps the isolation visible per-test.

Verifying it isolates rather than silences. A fix that works by adding mocks can convert a real failure into a false green, so I mutation-tested it: removing the delete_terminals_by_session(session_name) call from create_terminal makes test_session_service.py::TestSessionOwnershipIntegration::test_same_name_relaunch_purges_stale_terminal_metadata fail. The purge is also covered directly by test/clients/test_database.py. So the feature stays guarded; only the DB-free unit tests got isolated.

The regression was wider than either of us spotted. An adversarial review pass found the same unmocked-call regression in three sibling files that neither of us had checked:

File	Tests
test_terminal_service_coverage.py	4
test_kiro_engine_phase0.py	2
test_plugin_event_emission.py	2

Measured: 8 failed / 28 passed on the PR head vs 36 passed / 0 failed on main — so #497 introduced these too. Fixed in f553114 / 0c44b31 / f641552, each following that file's own convention (test_kiro_engine_phase0.py uses with (patch(f"{_MODULE}.X"), ...) context managers rather than decorator stacks). These ride along rather than becoming a follow-up because they're the same defect and would break CI on any fresh environment.

Two commits are pre-existing cleanup, not this PR's regression — flagged separately so you can weigh them independently: 040039e and b20c3fc close real-DB touches that were silently swallowed rather than failing (an exception-cleanup db_delete_terminal, an unmocked status_monitor cascading into a real metadata query, and a real MemoryService construction). Both are present identically on main — verified by control run. I included them because commit b12022f alone makes the tests pass while the file still creates a real .db file; only with these does the suite stop touching a real database at all. Happy to drop them into a separate PR if you'd rather keep this one strictly to the regression.

A second review round found one more hole, fixed in b0495f1. Mocking each call individually left test_send_input_does_not_dispatch_on_failure passing the "no real DB" bar only because a parametrized sibling ran first and poisoned the module-global _memory_injected_terminals cache (terminal_service.py:89,114). Selecting that test alone still reached the real MemoryService — and with no HOME override that means the developer's live CAO database. It also hits under pytest --lf, any single-nodeid selection, and pytest-xdist --dist load. Sequential CI runs were unaffected, which is why it stayed hidden.

Fixed both narrowly (the missing patch) and generically: all four modules now carry pytestmark = pytest.mark.usefixtures("isolated_memory_db"), which is already the idiom here (test/conftest.py:174, used by six modules). That makes these files immune to the next unmocked DB call rather than only the ones enumerated today. The per-call mocks stay, since several tests assert on them.

Verification

• All 4 touched files: 88 passed, no .db file created
• Every one of the 25 collected nodeids run individually on a fresh HOME: no .db file (was 1)
• test/services/ excluding agui: 1649 passed, 6 skipped
• The one test_fifo_reader.py failure is a pre-existing environmental flake — reproduces intermittently on this branch (1 of 3 runs), passes on main, and touches no code in this change
• The ~43 agui failures are pre-existing/environmental — identical set on origin/main

Note this branch is not rebased onto the latest main (4 commits landed while I was working); the branch still reports MERGEABLE and I'd rather not rebase under an existing approval unless you want me to.

Your two carried "Important" items are still open and I'd like your read on them: the herdr _resolve_working_directory contract change (I'll document the intent in the CHANGELOG if you agree that's the right level), and the GET /sessions disclosure now that this PR adds absolute paths to that response — happy to gate it with the same require_any_scope dependency as /events/history here rather than deferring.

[gutosantos82]

PR Review: #497 — fix(sessions): surface working_directory and agent_profile on list_sessions

Summary

Delta review against previously reviewed head eed78037 (verdict there: Request changes, with
exactly one live blocker — the test-isolation regression from the unmocked
delete_terminals_by_session call). The author's claim that this push is seven test-only
commits is verified: git diff eed78037..b0495f16 --name-only touches only
test/services/test_terminal_service_full.py, test_terminal_service_coverage.py,
test_kiro_engine_phase0.py, and test_plugin_event_emission.py (+118 −6); zero src/ files
change. The blocker is fixed and re-verified empirically (details below). With that
cleared, the substance of the PR — ownership metadata (working_directory, agent_profile)
on list_sessions entries so a caller on a shared cao-server can tell its own sessions
apart, persisted launch-time directory with live-pane fallback, best-effort per-entry
enrichment, idempotent migration — stands as previously assessed: well-motivated, additive,
and carefully scoped, with all of the first round's substantive blockers (persist effective
launch directory, stale-row purge, mypy, id=None handling) still fixed at this head.

Blocker resolution — verified at this head

• [tests] test-isolation regression: FIXED. Re-ran the exact prior repro in a fresh
  worktree at b0495f16 with a clean temp HOME:
  test/services/test_terminal_service_full.py now passes 52/52 (prior head: 18 failed
  with sqlite3.OperationalError: no such table: terminals), and no .db file is created
  under the clean HOME. All six suites the PR touches pass together under the same clean-HOME
  conditions: 141/141 (test_terminal_service_full, test_terminal_service_coverage,
  test_kiro_engine_phase0, test_plugin_event_emission, test_session_service,
  test_database, test_server). The fix is the right shape, not a bandage: a file-wide
  pytestmark = pytest.mark.usefixtures("isolated_memory_db") routes any residual DB touch to
  a per-test SQLite engine (shared test/conftest.py fixture, monkeypatched SessionLocal,
  disposed on teardown), and delete_terminals_by_session is added explicitly to every
  affected @patch stack — so the DB-free tests are DB-free again by construction, not by
  test ordering. The author also closed pre-existing real-DB touches surfaced by the same
  check (e.g., db_delete_terminal in the session-not-found path) and a second instance the
  maintainer's review surfaced in other files.
• One .db file does still appear when running test/clients/test_database.py under a clean
  HOME — verified this is pre-existing on upstream main (same behavior at 2a6f20cb),
  by design for the DB-client suite, and not a PR regression.
• CI: 19/19 checks pass at this head.

Non-blocking (carried forward — src/ unchanged since last review, both re-confirmed)

• [consistency] terminal_service.py (_resolve_working_directory) — resolution still runs
  unconditionally for every backend before creation, tightening the herdr backend's contract
  (must-exist + blocked-system-dir checks it previously didn't apply). Reasonable hardening,
  still undocumented/untested on the herdr path. A one-line statement of intent in the PR body
  or CHANGELOG closes this.
• [security] GET /sessions disclosure — re-confirmed at this head: @app.get("/sessions")
  (api/main.py:2012) carries no auth/scope dependency, and this PR adds absolute filesystem
  paths (embedding OS usernames/project layouts) and agent profiles to its response. Missing
  auth is pre-existing; the added disclosure is new. Cheap mitigation now or as fast-follow:
  gate with the same scope dependency used by /events/history.

Process note

The maintainer (gutosantos82) submitted CHANGES_REQUESTED on Aug 5, i.e. before the
Aug 6 test-only fix push, so reviewDecision=CHANGES_REQUESTED still gates merge but is
addressed-pending-re-review, not outstanding. fanhongy has an APPROVED review on record.
The finding the maintainer raised (the test-isolation regression) is exactly what this push
fixes, and the author credits the review for surfacing a second instance. The ball is in the
maintainer's court to re-review; nothing further is owed by the author beyond the optional
nits above.

(b) adds a new information-disclosure surface to an unauthenticated endpoint — an operator
should read the /sessions note before acking, and publishing an approve while the
maintainer's CHANGES_REQUESTED is unresolved should be a deliberate choice.

Verification performed

• Fetched PR head b0495f16 and upstream main (2a6f20cb); worktrees at both.
• git diff eed78037..b0495f16 --name-only: 4 test files only, zero non-test files.
• Clean-HOME repro (HOME=$(mktemp -d)) of the prior blocker: 52/52 pass at this head
  (18F/34P at prior head); no stray .db created by the terminal-service suite.
• All 6 touched suites under clean HOME: 141 passed.
• Per-suite DB-touch bisection: only test_database.py writes a DB file; same on main.
• gh pr checks: 19 pass, 0 fail. mergeable: MERGEABLE.
• Inspected fixture implementation (test/conftest.py:174) and patch-stack additions.

[haofeif]

@tedswinyar can you please help to resolve the conflicts.