feat(dashboard): opt-in per-session project directory

[chenmingwei23]

What is the problem?

Every new chat session lands in the same shared project directory. A user running
several unrelated pieces of work concurrently gets their notes, scratch files and
session context mixed together, because nothing about session creation is
per-session: the project a new slot resolves to comes from
dashboard.default_project, else from the workspace directory.

The only workaround today is to open a session and manually repoint it at another
directory through the project endpoint, every single time.

Why this issue matters to the user

The reporter's case is the common one, not an edge case: concurrent unrelated
activities. Manual repointing is easy to forget and easy to get wrong, and the
failure is silent - you do not find out that two pieces of work shared a
directory until their files are already interleaved. Forgetting it once is enough
to lose the separation for that session's whole lifetime.

How our fix solves it

THIS DEFAULTS OFF. dashboard.new_project_per_session is False, so installing
this change alters nothing: a config that never mentions the key resolves a new
session's project through exactly the same chain as before.

Chaining from the symptom to the root cause:

• Symptom: unrelated sessions share files.
• Because: a new slot's project is resolved from global config, so every new
  session resolves to the same directory.
• Because: there was no per-session directory in that resolution at all. A slot
  carries two identities - slot.workspace (a NAME, derived from the agent
  binding, whose name-to-directory mapping is global) and slot.project (a real
  directory) - and only the second is per-session.
• So: the fix adds a derivation keyed on slot.project, which is already
  per-session, already has a validated setter, and is already what becomes the
  agent's cwd.

Why the config keys are project-named rather than workspace-named. The request
says "workspace", but in this codebase a workspace is a distinct concept with its
own name-to-directory mapping and its own memory and knowledge base. This change
sets slot.project. Naming the keys after what they actually set avoids shipping a
name that would need a compatibility alias later, and a config key is one of the
few things here that is genuinely expensive to rename once it is in user config
files.

Answering the four questions this change has to answer, all from existing code
rather than invented:

WHERE the directory goes. Under an existing configured root, never anywhere
new. dashboard.session_project_root names it; empty falls back to a .sessions
folder inside the workspace directory the session already resolves to, created
through the same pinned routine the session directories use. It is one level down
rather than the workspace directory itself because the session name becomes the
directory name, so creating at that top level lets a session called
package.json put a directory on that path. A CONFIGURED root must ALREADY
EXIST, matching how dashboard.default_project is treated in
the sibling branch, so a typo disables the feature instead of scattering
directories. Nothing is written outside the root: the derived path is asserted to
be an immediate child of the resolved root, and mkdir is called without
parents.

HOW it is named, and why an existing directory is never taken over. From
slot.key - the MINTED key, not the request body's name - via the existing
_safe_dir_name. Two properties matter, and the second is the one that makes the
feature safe:

• _safe_dir_name is a sanitizer, not a validator. It maps separators to _ but
  does not reject .., so the containment assertion is what stops an escape, not
  the name.
• The directory is created EXCLUSIVELY. slot.project is persisted on the
  transcript's metadata line and rehydrated from there, so a restored session is
  served from metadata and never re-derives. A candidate path that already exists
  therefore belongs to an EARLIER session whose key was reused - a caller-supplied
  name, a channel key, or a minted key reproduced by a closed session plus a
  same-second restart - and adopting it would hand a fresh session that session's
  files. It is refused, and the caller falls back to the shared default.

Exclusive creation is also what lets this ship with NO delete path: nothing is
cleared, reused, or overwritten.

WHAT happens when creation FAILS. The session still opens, degraded. Every
failure mode - a root that is empty, missing, not a directory or sensitive; a name
that would escape the root; an existing candidate; an OSError from the mkdir on
a read-only or full disk - returns "", which the caller reads as "no per-session
directory" and falls through to the shared default. This is not a new posture: the
sibling configured-default branch already skips an ineligible path to fall back
"instead of wedging every new slot".

WHO cleans up. Nobody, and that is a real cost this PR names rather than
solves. A directory per session is unbounded growth. This change deliberately
contains NO delete path - a reaper written in the same change as a create path is
how data loss ships - so cleanup is proposed below instead.

Deliberate deviations from the issue's literal request, each with its reason:

• No Settings UI toggle. A new label needs a new i18n key, and
  catalogParity.test.ts requires every locale (14 of them). I will not fabricate
  translations for 13 languages, and en.json is generated and must not be
  hand-edited (website/docs/i18n-catalog.md). The setting is config-file only for
  now, exactly like its closest sibling dashboard.default_project, which also
  ships with no UI control.
• The root is config-file only rather than a Settings directory picker. Measured:
  every key the dashboard config PUT accepts is a bool, an int or a small enum, and
  that handler's write branch does no path handling at all. This change does not
  make itself the first dashboard-writable path setting.
• No memory or knowledge-base isolation. That is the open product question the
  reporter flagged, and it lives on slot.workspace, which is agent-derived -
  repointing it per session would collide with agent bindings. The reporter's own
  view was that shared memory is the safer default.

config-baseline.json is regenerated because the repo commits a schema snapshot
and asserts byte-parity with its generator, so adding two config fields invalidates
it. That is a consequence of the change, which is why the diff touches a fifth
file.

What review changed, and one thing I got wrong twice

Four rounds of blocking findings, all inside this diff:

1. The handler derived the directory from the request body's name. The
   dashboard's own new-chat path sends no name, so name was None,
   _safe_dir_name(None) raised, the broad except swallowed it, and with the
   toggle ON behaviour was identical to OFF on the primary path. Now derives from
   slot.key.
2. os.geteuid() was evaluated at collection time, which aborts the whole shard on
   Windows where the attribute does not exist. Guarded with os.name != "posix".
   Separately, SAST flagged a hardcoded 0o700; the test now captures and restores
   the mode pytest created, which removes the finding and is more correct.
3. A reused key could inherit a deleted session's files. I gated on the key's SHAPE
   via _slot_index_from_key, which only checks that the second segment is a digit
   and never validates the trailing timestamp - so worker-1-stable passed an
   "auto-minted only" test while being entirely reusable. That is relying on what a
   predicate is called instead of reading what it validates.
4. The real fix, and the correction of my own error. I claimed slot.project is not
   persisted, having checked open_slots.json (which carries session keys only) and
   concluded a global absence. It is persisted - on the transcript's metadata line,
   rehydrated in chat_persistence, with project in history's owned-field list.
   I verified one persistence mechanism and treated that as verifying the
   inference. Because restore comes from metadata rather than re-derivation,
   refusing an existing directory is safe, and exclusive creation replaces the
   key-classifying gate entirely - covering every reusable-key case without this
   call site classifying keys at all.
5. A 500 from this endpoint, found by CI and not by my own tests. Reading the new
   field by bare attribute access raised AttributeError inside the request
   handler, so POST /api/chat/slots returned 500 whenever cfg.dashboard was a
   partial stand-in. Measured: 36 sites across 19 test files patch
   KiroCrewConfig.load with a minimal dashboard=SimpleNamespace(...) carrying
   only the fields the surrounding code reads. That is why the read now uses
   getattr with the dataclass defaults, and why the fix belongs in the handler
   rather than in 19 test doubles - a missing field reading as OFF is the
   documented default. I had attributed this red to a flaky event-loop class for
   three board cycles; see the harvest below.
6. A sensitive path was CREATED before being validated. is_sensitive_path was
   applied to the root before the mkdir and to the result after it, but never to
   the candidate before it - so a session key of security_policy.json under the
   data home minted a DIRECTORY on a governance trust root, and the post-check then
   returned "" having already done the damage. Nothing here deletes, so the
   directory would stay and block policy loading. I had applied the
   pre-check/post-check reasoning to containment only, never to sensitivity. Now
   rejected before the mkdir; verified that is_sensitive_path matches the path
   while it still does not exist, which is the case that matters. This is also
   downstream of dropping the key-shape gate in round 4: a caller-supplied name
   becomes the slot key, so the name reaching that line is caller-steerable, and I
   widened who could steer it without re-examining the guards that depend on it.
7. I tried to apply the setting on the second creation path and reverted it, which
   is the most useful thing in this history. POST /api/chat also creates a slot,
   so the setting applied on the create endpoint and not on a send to an unknown
   slot. Adding it there produced two defects in two rounds. First it broke six
   fail-closed tests: test_slot_create_default_agent patches
   KiroCrewConfig.load to raise OSError, and my unguarded await propagated it
   as a 500 from an endpoint contracted to answer 400/409 - and separately, the
   round-5 hardening of new_project_per_session had left cfg.dashboard itself
   bare, which the send path exposed because it sees config stand-ins with no
   dashboard attribute at all. I had fixed the leaf and assumed the chain. Then,
   with those fixed, the reviewer found the derivation was using the wrong
   workspace: an auto-created slot's workspace is "default" regardless of the
   agent the request names, so with an unconfigured root the directory landed under
   the default workspace's tree and that wrong location persisted.
8. That last defect is not fixable there, and the reason is a documented invariant
   rather than my judgement. Deriving needs the workspace; for an auto-created slot
   the workspace comes from the requested agent's bindings; but
   resolve_agent_bindings requires the project directory it is given to be "the
   same directory Kiro Crew passes as the kiro-cli cwd", because passing a
   directory the session does not run in reintroduces the silent
   agent-substitution bug that lookup exists to prevent. Bindings need the final
   project, and the project needs the bindings' workspace. So the send path now
   deliberately does NOT derive, with that reasoning at the call site and a test
   pinning both the absence and the reason. It degrades to the shared default,
   which is today's behaviour. The create endpoint, where the workspace is resolved
   before the derivation, is unaffected.
9. The commit was a bare assignment past two awaits. _apply_per_session_project
   awaits the derivation and the conflict scan without holding a lock across them,
   so a concurrent project POST could set an explicit selection while it waited,
   and the unconditional slot.project = per_session at the end would overwrite
   that selection with the derived default - the weaker value clobbering the
   stronger. Now a compare-and-set: re-read the field after the awaits and commit
   only if it is still the empty value the derivation started from; a concurrent
   writer wins. This is the ROOT of the whole class - findings 1, 5, 7 and this one
   are all a value trusted at commit time that was only valid before an await - and
   unlike the earlier per-instance fixes it closes the class at the commit point.

Two blocking findings I am not acting on, each with the measurement rather than a
preference. The first is the send path above: the choice was between a directory
under the wrong workspace root and the setting not applying there, and the second
degrades to current behaviour while the first persists a wrong location.

The second is preserving the per-session project across agent and workspace
switches. The workspace-switch handler refuses outright once a session has messages

• if slot.total_messages > 0 returns 409 "Cannot change workspace after messages
  have been sent. Open a new session instead." - so the switch is only reachable on a
  session that has written nothing, and there are no files to re-share. The residual is
  an empty orphaned directory and the setting no longer applying after a switch: disk
  cost and incompleteness rather than the data loss the finding is anchored to.
  Re-deriving under the new workspace is also a design decision about what a workspace
  switch MEANS, and belongs in its own change. Both are filed as follow-ups below.

What tests we did

test/test_session_project_dir.py, 30 tests, run as
python3 -m pytest -n0 test/test_session_project_dir.py: 30 passed. black, isort
and flake8 clean on all four Python files changed. mypy reports 2 errors, both in
transcribe.py, a file this change does not touch.

Three handler-level tests drive the real api_chat_slot_create, because several of
the findings above were in what the call site passed or read, which no unit test of
the helper can see: a nameless create must get its own directory named from the
minted key; a create whose directory already exists must NOT adopt it - parametrized
over reused-name and worker-1-stable, the second being the name that defeated the
earlier gate, with a pre-seeded file asserted to survive; and a create with a PARTIAL
config must still return 200 with the feature reading as OFF.

Two further tests cover what those cannot. One asserts structurally that the send
path does NOT derive and that the reason is recorded at the call site, so a later
reader does not close the apparent gap by reintroducing the wrong-workspace bug; it
also asserts the create endpoint still does apply it. The other calls the helper
directly with a config object that has no dashboard attribute, the shape that
turned into a 500 in finding 7.

Mutation-verified, classified by reading the runner's own failure line:

mutation	result
revert getattr to bare attribute access	reddens the partial-config test with assert 500 == 200, and reproduces the original CI failure
revert exclusive create to exist_ok=True	reddens 4 tests, including both handler cases, on the adopted path
revert slot.key to name	reddens the nameless test, assert '' == '.../chat-1-...'
non-deterministic name	reddens 7 tests on real assertions
remove except degradation	reddens with PermissionError, the correct observable for a guard whose absence crashes
drop the config parse line	reddens assert '' == '.../roots'
flip the default to True	reddens assert True is False
delete the PRE-mkdir sensitive check	reddens assert [PosixPath('.../security_policy.json')] == []
make the send path derive again	reddens the deliberately-does-not-derive test
bare cfg.dashboard instead of the guarded chain	reddens with the exact AttributeError, and reproduces the CI regression
bare commit instead of compare-and-set	reddens the concurrent-selection test with the clobbered path
delete either or BOTH containment sites	reddens NOTHING - see the survivors below

Three survivors are disclosed rather than hidden, because each says something about
the code:

• Deleting EITHER containment site, or BOTH, reddens nothing. I previously claimed
  deleting both surfaced an escape; that was wrong, and measuring it is what showed
  why. _safe_dir_name cannot emit a path separator (it maps /, \ and : to
  _), so the join is an immediate child by construction; and mkdir does NOT
  follow a symlink at the final component - dangling or not it raises
  FileExistsError, which exclusive creation already refuses. So containment is
  now depth against a change to either of those facts, not an active guard. It stays
  because a sanitizer that starts passing separators through, or a move away from
  exclusive creation, would make it load-bearing again with no other signal.
• Deleting the isdir(base) guard reddens nothing, because mkdir without
  parents fails on a missing or non-directory root anyway.
• Deleting the non-string key guard reddens nothing, because the broad except
  returns the same "".

All three share one cause worth stating plainly: the broad except Exception is the
backstop that makes individual guards unobservable, and it is also what let finding
1 above ship as a silent no-op. It is kept because the sibling default_project_dir
uses the same breadth for the same reason, and because a session must open even on
an unanticipated failure - but every guard it masks now carries a comment recording
that it was measured unobservable, so a later reader neither deletes it as dead nor
trusts it as pinned.

On the CI reds, split by measurement rather than by guess. Backend Tests (3.12, 3)
and (Windows) (3) were MINE: the 500 above, established by reverting only my three
source files and re-running the named test class (3 failed with my change, 3 passed
without, 190 pass in that whole file now). Backend Tests (3.12, 4) is NOT mine:
its failure is test_snapshot.py::TestNotificationCopyWhenNoLiveFileExists
asserting a concurrency ordering, it does not reproduce serially, and it passes
25/25 both with my change and with my change reverted - so nothing in this diff
moves it. I am not fixing that one here.

Any other suggestions on the work

• Cleanup is owed and is proposed, not implemented. The reporter's own
  suggestion was to ask on session delete with a Yes / No / Never answer, and he
  called the zip-to-archive idea scope creep himself. Any reaper should be its own
  change, so a delete path is never introduced alongside a create path. Note that
  exclusive creation means a stale directory is never silently reused, so the cost
  of not having a reaper is disk growth rather than cross-session leakage.
• The Settings toggle is a clean follow-up once the 14 locale strings can be
  produced properly. No backend change is needed beyond adding the key to the
  dashboard config GET and PUT.
• A per-session workspace that also isolates memory and the knowledge base is a
  larger, separate decision about slot.workspace and agent bindings.
• Applying the setting to sessions auto-created by a send to an unknown slot needs the
  workspace-versus-bindings circularity broken first - most likely by having that
  endpoint resolve the workspace explicitly before the slot is used, rather than
  relying on get_or_create_slot's default. Worth doing, but not as a side effect of
  this change.
• What a per-session project should do across an agent or workspace switch is its own
  decision, and is the one blocking finding I declined above. Today a workspace
  switch repoints the project at the new workspace's directory, which is defensible;
  the switch is also refused with a 409 once the session has messages. Worth settling
  deliberately rather than inside this change.
• Dashboard UI to manage workspaces (create/update/delete), not just switch #8265 asks for a dashboard UI to create, update and delete workspaces. Different
  mechanism, but adjacent, and whoever builds that UI is the natural owner of the
  toggle above.

Pattern harvest

Rule candidate: a call inserted into a long handler must sit after every input it
reads is FINAL, and "final" is found by locating where each input is last assigned -
not by reading the surrounding lines. My insertion read a workspace that the handler
never assigns and a config the handler deliberately loads late and tolerantly. Two
separate defects, one ordering cause.

Rule candidate: when adding a call into a shared request handler, derive the
regression surface from the ROUTE rather than from which files look related. I picked
two files that had failed recently, both passed, and CI then failed six tests in
three files I had not run. Grepping the route name across the test tree produced 28
files in one command - that list is the sweep, and it is cheap enough that guessing
was never worth it.

Rule candidate: when hardening an attribute read against partial stand-ins, guard the
whole chain, not the leaf. I replaced cfg.dashboard.new_field with
getattr(cfg.dashboard, "new_field", default) and left cfg.dashboard itself bare,
so the same class of AttributeError came back one level up on a different call path.
A helper reached from more than one entry point sees more shapes of its arguments than
any single call site does.

Rule candidate: when a function validates a path and then creates it, every
validation must run BEFORE the side effect, not merely somewhere in the function. I
had a pre-check/post-check pair for containment and only a post-check for
sensitivity, so the sensitive path was created and then rejected - and with no delete
path, rejecting after the fact leaves the damage in place. List the checks against
the single line that mutates the filesystem and ask of each one which side of it it
falls on.

Rule candidate: before calling a CI red flaky or inherited, read WHICH test failed
and check whether it exercises a surface this change touches. I dismissed a failing
shard as a flaky event-loop class for three board cycles; its annotation named a 500
on /api/chat/slots, the exact endpoint being modified. The evidence I had cited was
real but about different files, so it never bore on this failure. "Does not reproduce
serially" is only evidence once it is the failing test that was re-run.

Rule candidate: checking one persistence mechanism does not establish that a field
is unpersisted. open_slots.json carrying only session keys was true and did not
support "slot.project is not persisted" - the per-slot fields ride the transcript
metadata line. Before building on an absence, enumerate the write sites for the
field (grep every assignment and every serializer) rather than reading the one store
you happened to open.

Rule candidate: a broad except on a resolution helper converts caller errors into
its own "unavailable" return value, so a wrong argument at the call site ships as a
silent no-op and every guard inside the helper stops being observable to mutation.
When a helper degrades by returning a sentinel, validate the caller's argument at the
boundary and test the call site through its real entry point.

Refs #8432

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of 82abfc308119f1ad1ae3634eb3b75e59280495ad — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All the helper symbols the diff leans on (pinned_fs.pin_parent/fd_real_path/supports_pinned_walk, platform_compat.pin_directory/first_linked_ancestor/is_link_or_junction, _safe_dir_name) pre-exist the change, and the new surface counts one real consumer each (chat_handlers.py:297,321,323). The one item without a derivable requirement is the second config key.

First-Principles-Verdict: CONCERNS

dashboard.session_project_root is a flexibility knob with no named user: the .sessions fallback alone removes the reported harm.

Not justified as shipped

• Item 3, dashboard.session_project_root — inherited. Its only stated support is shape-symmetry with dashboard.default_project ("exactly like its closest sibling"); no reporter, issue sentence, or failure names a need for a non-default root, and the description's own words make it one-way-door surface ("a config key is one of the few things here that is genuinely expensive to rename"). Grepped session_project_root: 1 read site (chat_handlers.py:323) plus definitions/baseline.

What this change ships

Inventory (9 items) — 8 justified

Intent: let a user running unrelated concurrent work stop new chat sessions from sharing one project directory — an ADDITION (opt-in feature).

1. Each newly created chat session can get its own project directory — justified
2. New config key dashboard.new_project_per_session, default off — justified
3. New config key dashboard.session_project_root — inherited: symmetry-justified, no named user needing a non-default root
4. A .sessions container is auto-created inside the workspace when no root is set — justified
5. A session whose derived directory already exists silently falls back to the shared default — justified
6. Sessions auto-created by the message-send path never get a private directory — justified
7. A plain create now persists the slot immediately (previously only metadata-carrying creates saved) — justified
8. The shared-default project assignment now waits on a new per-slot lock — justified
9. config-baseline.json regenerated for the two keys — justified

Watch

• session_project_root (item 3): defer the key; the fixed .sessions container under the workspace is the smallest honest version and already ships as its fallback. Clears when: a linked report names someone who needs session directories outside the workspace, or the key is dropped.

Subtractions

• Drop session_project_root from sections.py/loader.py/config-baseline.json and make the .sessions container the only location — 1 consumer (chat_handlers.py:323); reintroduce the root when a real request for it exists.
• Delete the two containment asserts in session_project_dir (loader.py, the pair the comment itself calls "measurably unobservable — deleting either or both reddens nothing"); their stated purpose is guarding a future change, which is "so we can later" by the author's own measurement.

[FIRST-PRINCIPLES-REVIEWED] 82abfc3

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 82abfc308119f1ad1ae3634eb3b75e59280495ad — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

Real harm, opt-in default-off, built on existing seams (slot.project, _safe_dir_name, pinned_fs, transcript persistence), with failure modes and deferred scope explicitly reasoned.

Suggestions

• The toggle silently not applying to send-created slots is documented only in code comments; add one line to the new_project_per_session help text (config-baseline is the only user-facing surface) so a user who flips it on knows sessions auto-created by a send still share the default.

[DESIGN-REVIEWED] 82abfc3

[github-actions]

GPT 5.6 Review — ✅ human override accepted

Human judgment by @chenmingwei23 overrides the GPT 5.6 finding for 82abfc308119f1ad1ae3634eb3b75e59280495ad; the recorded reason is authoritative for this commit.

This comment is updated in place on each push.

The model was not re-run because an authorized human decision supersedes it.

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 82abfc308119f1ad1ae3634eb3b75e59280495ad: <one-sentence reason>

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 82abfc308119f1ad1ae3634eb3b75e59280495ad — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 82abfc3

Verdict parsed from the review's SHA-scoped output markers for commit 82abfc308119f1ad1ae3634eb3b75e59280495ad.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 82abfc308119f1ad1ae3634eb3b75e59280495ad: <one-sentence reason>

[chenmingwei23]

Stopping this PR: converting to draft

This PR is being converted to draft and stood down. The reason is a design finding about the mechanism, recorded here so the follow-up starts from it rather than rediscovering it.

The opt-in per-session project directory is a compound operation: validate a configured root, create a directory under it, assign that directory to the slot, and persist the assignment. Those four steps have to hold together atomically, but they run from a request handler that owns none of them as a unit -- there is no single owner holding a lock across all four.

Across twelve review rounds the mechanism drew six blocking findings, all inside roughly 220 lines of directory derivation (session_project_dir in config/loader.py and _apply_per_session_project in dashboard/chat_handlers.py). Each finding is a different pair of the four steps failing to hold together:

1. Config load on the derivation path was not failure-tolerant (a create-time surface returned the wrong status).
2. The config attribute chain was read without guarding the whole chain.
3. The derivation read a session input before that input was final.
4. The assignment could be overwritten by a concurrent writer between the derivation's awaits.
5. Directory creation is not symlink-safe.
6. The assignment is not durably persisted at the point it is made.

Findings 1 through 4 were each fixed at the call site, and each fix revealed the next -- the signature of one unwritten contract rather than six independent defects. Findings 5 and 6 are the two currently open, and neither is closable at the call site, because the call site is not where atomicity lives.

The settings surface and the config keys are sound; the plumbing carries none of the six findings. But shipping the plumbing alone lands a config key with zero consumers, which reads as dead code and is behaviourally identical to not shipping it. So the honest resolution is not a partial merge. The derivation needs to be rebuilt behind a single owner that performs validate-create-assign-persist as one unit on the slot layer. That is tracked as a separate design issue, linked below.

Refs #8432

[chenmingwei23]

Rebuild tracked as #9218 (labeled decision -- no needs-design label exists on this repo).

[chenmingwei23]

Retracting the stand-down: both remaining findings are closed at the call site

I stood this PR down and converted it to a draft on the strength of one claim: that GPT's last two findings -- symlink-safe directory creation, and durable persistence of the assignment -- were not closable at the call site, and that the derivation therefore had to be rebuilt behind a single atomic owner. That claim was wrong. Both are closed here, in the two functions that already existed, and each fix is pinned by a test that fails when the fix is reverted. Retracting on the record rather than quietly re-opening, because the stand-down was public.

Head is now 18c9484a0, one commit, and #9218 is resolved against this PR rather than left describing a rebuild that is not happening.

Finding 1 -- root-symlink race in session_project_dir (src/kiro_crew/config/loader.py)

The create resolved the root's PATH a second time. Every component is re-resolved at mkdir time, so replacing the validated root with a symlink between the validation and the create redirected the create to wherever that symlink pointed -- under a name the caller steers, since the session key becomes the directory name. The checks after the create would reject the RESULT, but nothing here deletes, so the escaped directory already existed.

Closed by creating the child through a descriptor instead of through the path:

• os.open(base_resolved, O_RDONLY | O_DIRECTORY | O_NOFOLLOW) refuses a root that is a symlink at open time, so the pre-open swap returns "" with no side effect at all.
• os.mkdir(name, dir_fd=dir_fd) resolves the child against that opened inode, so a swap landing AFTER the open cannot move it either -- the directory appears inside the validated root, or nowhere. _safe_dir_name maps /, \ and : to _, so name is a single component by construction, which is what makes it safe to resolve relative to a descriptor.
• An fstat/stat identity check then confirms the pinned inode is still what base_resolved names, and refuses if it is not: the child stayed contained, but the path this function would return no longer refers to it.

Evidence, not assertion. test_root_swapped_for_a_symlink_after_validation_creates_nothing_outside times the swap off the last validation call before the create, so the interleaving is deterministic rather than a race the test hopes to hit. Reverting to the path-based candidate.mkdir() fails it with:

AssertionError: created a directory outside the configured root
assert [PosixPath('/.../outside/chat-9')] == []

That is the finding reproduced: a caller-named directory created outside the configured root. With the descriptor in place the same test passes and outside/ stays empty.

Finding 2 -- assignment not persisted (src/kiro_crew/dashboard/chat_handlers.py)

The handler's durable save was conditional on metadata the REQUEST supplied -- a folder id, a pinned title, a peer binding. A plain create or a reopen supplies none of those, so the assignment lived only in memory: a crash before the next periodic flush lost it, while the directory and anything the session had written into it stayed on disk with nothing pointing at it. Nothing re-derives the directory (a restored session is served its project from the transcript metadata line), so the assignment IS the only record of the association.

Closed as GPT prescribed: _apply_per_session_project now returns whether THIS call assigned slot.project, and the forced save is conditioned on that alongside the three metadata fields. Every other exit returns False, including the compare-and-set losing to a concurrent writer -- that writer owns its own persistence, and reporting True there would attribute its value to this call.

test_the_assignment_is_saved_even_with_no_other_metadata drives the real create handler with an empty body, which is exactly the shape that supplies no folder, title or peer key. Dropping the new term from the save condition fails it with:

AssertionError: the assignment was never persisted

It asserts the forced save rather than file contents, because the defect is that the write never happens, and force=True is what distinguishes it from the dirty-flag flush that may or may not follow.

Why this lands rather than gets rebuilt

The stand-down argued that findings 1 through 4 were each fixed at the call site and each revealed the next, and read that as one unwritten contract needing a single atomic owner. The pattern was real; the conclusion did not follow. Findings 1 through 4 were an ordering class -- a value trusted at commit time that was only valid before an await -- and the compare-and-set at the commit point closed that class. These last two are not members of it. One is filesystem-level TOCTOU, whose standard remedy is a directory descriptor, and the other is a missing save. Neither needed the lock the stand-down was reaching for, and rebuilding roughly 220 lines to reach the same two remedies is strictly more work than landing them here.

The atomic-owner design is still the better long-term seam and the reasoning for it is preserved on #9218's history. It is not a prerequisite for this change.

Local gates

Green on 18c9484a0: black, isort, flake8, mypy, sync-io-in-async, loop-bound-locks, subprocess-encoding, agent-SDK-boundary, lockdown-before-publish, testpaths-coverage, docs-lint, vendor-manifest, brand-name, harness-parity, feature-map, changelog-history, focus-cue, builtin-skill-scope. The feature's own suite is 32/32 (two new tests above) and the config suite is 501/501.

Refs #8432

[chenmingwei23]

Round on f2c2cf994: three findings closed, including one I introduced

The Windows red was mine, not a flake, and it came from the fix I made for the previous round's symlink finding. os.O_DIRECTORY and os.O_NOFOLLOW do not exist on Windows CPython, so naming them raised AttributeError straight into session_project_dir's catch-all, which returns "" -- and "" is the caller's "no per-session directory". That is why every failing assertion in test_session_project_dir.py showed an empty string where a path belonged: the code was not choosing a wrong directory, it was silently disabling the whole opt-in on every Windows session. GPT flagged the same thing independently as F3.

All three of its findings on 18c9484a0 are real and closed. Each fix uses machinery this repo already has rather than a new one, and each is pinned by a test that fails when the fix is reverted.

F1 -- an ancestor swap escaped the configured root

Correct, and my previous fix could not have caught it. O_NOFOLLOW guards the LAST component only, so a directory ABOVE the root replaced with a link is followed silently, and the post-create identity check I had added could not see it either: re-resolving the path walks the swapped ancestor too, so it agrees with itself and the escape looks contained.

Closed with pinned_fs.pin_parent, the component-by-component pinned walk GPT pointed at -- one openat per component of the ALREADY-resolved root, each with O_NOFOLLOW, and the child created relative to the descriptor that walk produces. base_resolved is passed as-is and never re-resolved inside, which is that function's stated contract: resolving within the walk would re-follow whatever an ancestor points at by now.

create_and_open_dir_pinned was the obvious first reach and is wrong here: it re-realpaths the parent internally, so the swapped ROOT is followed by that resolution. My existing root-swap test caught that immediately, which is a fair argument for having written it.

New test test_an_ancestor_swapped_for_a_symlink_creates_nothing_outside swaps the root's parent timed off the last validation before the create. Reverting to a leaf-only O_NOFOLLOW open fails it with AssertionError: created through a swapped ancestor, while the earlier root-swap test still passes -- the two cover different halves, which is the point.

F2 -- concurrent creates defeated session isolation

Also correct. Two creates naming one slot key both passed the "no project yet" check and both derived. The loser's exclusive create met the winner's directory, got nothing back, and its caller fell through to the shared default; if that fallback committed first, the compare-and-set correctly declined to overwrite it, and the slot ended on the shared directory with the winner's private one orphaned. A double submit silently cost the isolation the opt-in exists for.

Closed by serialising the transaction on a new per-slot _project_init_lock, following the convention its siblings in state.py establish -- _fork_lock, _model_pick_lock, _remote_pick_lock each own one transaction, and deliberately not slot._lock, which guards message-window edits and must not be held across the two thread hops this makes. The follower now waits, finds the project assigned, and reuses it by no-oping.

The compare-and-set stays: the lock serialises this helper against itself, but a concurrent project POST does not take it.

New test test_two_concurrent_creates_for_one_slot_derive_only_once asserts the DERIVATION COUNT rather than the final project, because the final project looks identical either way in a unit -- the orphaning needs the caller's fallback to interleave. Dropping the lock fails it with AssertionError: derived 2 times; the follower re-derived.

F3 -- Windows always disabled the feature

Closed with platform_compat.pin_directory on the branch taken when the platform cannot pin by descriptor. On Windows that opens the root without FILE_SHARE_DELETE, and a directory held by such a handle -- along with every directory above it -- can be neither renamed nor deleted while the handle lives, so the swap is PREVENTED rather than detected, and the open refuses to follow a reparse point. Creating by name under that pin is therefore safe. The capability probe is the same shape dashboard/handlers/prompts.py already uses: pinned_fs.supports_pinned_walk() and os.mkdir in os.supports_dir_fd.

Not done by dropping O_NOFOLLOW, which would have reopened the finding closed last round.

New test test_the_directory_is_still_created_without_the_posix_open_flags DELETES both attributes with monkeypatch.delattr, matching how the repo's other Windows tests simulate this. Deleting them is what reproduces the AttributeError; a test that only cleared os.supports_dir_fd would have passed against the broken code. Naming the flags unconditionally again fails it with AssertionError: assert '' == '/.../sessions/chat-13' -- the same empty string the Windows shard reported.

The teardown lines

ValueError: I/O operation on closed pipe and RuntimeError: Event loop is closed at the end of that log were not investigated as separate defects, on the reading that they are teardown noise downstream of the real failures. I will confirm they are gone on this head rather than assume it.

Local gates on f2c2cf994

Green: black, isort, flake8, mypy, sync-io-in-async, loop-bound-locks, agent-SDK-boundary, lockdown-before-publish, testpaths-coverage, docs-lint, brand-name, harness-parity, feature-map, changelog-history, focus-cue. The feature's suite is 35/35 (three new tests above), the config surface 536/536, and 281 tests across the slot/state/create surface pass. test/test_transcribe.py fails to collect here, and does so identically on bare main with no diff, so it is not this branch's.

Refs #8432

[chenmingwei23]

Round on 58ff96819: a Windows hole my own test caught, plus GPT's live-slot finding

Two of the three items this round were mine. Head is 58ff96819.

The second Windows failure was mine, and it was a real hole

Shard 3 reported TWO failures, not one. The skill-contract one is main-owned (tracked as #9363). The other was mine:

FAILED test/test_session_project_dir.py::TestSessionProjectDirRefusals::test_an_ancestor_swapped_for_a_symlink_creates_nothing_outside - AssertionError: created through a swapped ancestor

My previous round closed GPT's ancestor-swap finding on POSIX with the component-by-component pinned walk, and left the non-pinned branch holding only pin_directory. That pin refuses a reparse point at the root's own NAME and, once held, forbids renaming or deleting the root and everything above it -- but an ancestor swapped BEFORE the pin is traversed, so the pin lands on whatever that ancestor points at and the create escapes. The same defect GPT flagged as F1, arriving by a different route on the platform that cannot walk.

Closed by re-reading the ancestor shape while the pin is HELD, with platform_compat.first_linked_ancestor, and refusing when the answer differs from what validation saw. Order carries the property: a swap before the pin gives a different answer, and a swap after the pin cannot happen because the handle blocks it. The same comparison made BEFORE the pin would leave the window open. Comparing the shape rather than refusing any linked ancestor keeps a root under a redirected profile or a mapped drive working -- what matters is that the answer changes mid-call, not that a junction exists.

The lesson is the test gap, not the mechanism: I had pinned the non-pinned branch's HAPPY PATH and none of its refusals, and nothing on POSIX exercises that branch. New test test_an_ancestor_swap_is_refused_without_the_posix_open_flags deletes the two flags so the capability probe reports False, then runs the same swap at the same timing. It fails with created through a swapped ancestor when the re-read is removed, and it reproduces the Windows failure on Linux, which is what should have existed before the last push.

GPT F1 on 4560ac140 -- an existing live slot keeps its old working directory

Real, and the adjudication is right that the precondition is plausible. An existing slot can already have a live provider running in the shared directory. Assigning slot.project changes in-memory metadata only, so the provider keeps the cwd it started with: the session advertises a private directory while its files keep landing in the shared one. Isolation reported and not delivered, and the mixed files cannot be sorted out afterwards. The projectless-existing-slot precondition is exactly what the message-send path leaves behind, since that path deliberately does not derive.

Taken GPT's first option -- gate on is_new_slot -- rather than resetting the live session. A reset is correct on an explicit workspace switch, which is why _reset_slot_session_or_warn lives there, and wrong on a reopen the user did not ask to restart: it would kill a running provider as a side effect of opening a tab. A newly minted slot has no provider yet (schedule_eager_spawn runs at the end of the handler, after the project is final), so on that path the cwd and the metadata agree by construction. An existing projectless slot keeps the shared default, which is this feature's documented degrade posture.

New test test_a_reopen_of_an_existing_slot_derives_nothing. Worth naming how the first version of it was useless: it reopened a slot whose directory still existed, so the never-adopt rule refused the second derivation for an unrelated reason and the test passed with the gate removed. The precondition has to be a slot with NO directory of its own -- the send-path shape -- and with that, removing the gate fails it with the reopen derived a directory.

Backend Lint & Type Check -- the comment-history ratchet

Six markers, all mine: three (#8432) issue references and three history phrases (used to). Rewritten in present tense with the issue numbers dropped; the history belongs in git and in this thread.

One mechanical note for anyone hitting this gate locally: it reads file content from disk but computes the added-line set from origin/main...HEAD, so with uncommitted changes the two disagree and it reports a pre-existing marker in a shifted comment block as newly added. Commit first, then run it -- the run that looked like a real offender was that mismatch.

Local gates on 58ff96819

Green: black, isort, flake8, mypy, comment-history, sync-io-in-async, loop-bound-locks, subprocess-encoding, agent-SDK-boundary, lockdown-before-publish, testpaths-coverage, docs-lint, changelog-history, feature-map, focus-cue, brand-name, harness-parity, builtin-skill-scope. Feature suite 37/37, config surface 542/542. Every new test above is mutation-verified: each fails with a named assertion when its fix is reverted.

Refs #8432

[chenmingwei23]

Round on fe2d4efdc: the is_new_slot gate reopened the concurrency finding

GPT is right, and this one is a regression I introduced last round rather than a re-roll. Head is fe2d4efdc.

Gating the derivation on is_new_slot fixed the live-provider problem and broke the concurrency fix in the same move. Two overlapping creates on one name split into a winner that mints the slot and a follower that finds it existing. The follower is now excluded from the derivation, so it never takes _project_init_lock -- it goes straight to the shared fallback, reads a field the winner has not committed yet, and installs the shared default. The winner's compare-and-set then correctly declines to overwrite it, and the slot runs in the shared directory with the private one it exclusively created orphaned. Exactly the harm the lock was added to prevent, arriving through the path the gate opened.

The mistake was treating the lock as guarding the derivation. It guards DECIDING this slot's project, and the shared fallback is the other half of that decision. So the fallback's commit now runs under the same lock, with a re-read inside it:

async with slot._project_init_lock:
    if not slot.project:
        slot.project = cfg_proj or default_project_dir(workspace)

The value is still computed outside the lock -- it depends on config and the workspace, never on the slot, so two callers compute the same answer and only the commit needs ordering. state.py's comment on the lock now says both halves rather than just the derivation.

On the test, and what I could not prove

Two concurrent POSTs do not reproduce this. I wrote that test first, and it passed with the fix reverted; instrumenting the handler showed why -- the second request only reaches the project block after the first has committed, on a single client and on two separate clients alike. The requests observably serialise somewhere between get_or_create_slot and the project decision. I did not chase down which mechanism does it, and I am not claiming the interleaving is impossible in production: nothing in this code path forbids it, the adjudication traced it from the code, and a security-class finding I cannot disprove is not one to override.

So the test pins the property the fix actually needs. It holds _project_init_lock with the project cleared -- the state a follower observes mid-derivation -- then issues the follower's real request through the handler and asserts two things: the request does not finish while the decision is held, and once the decision lands it accepts that value instead of replacing it. Reverting the fix fails it with committed past a held project decision.

Naming the weaker link honestly: this pins that the fallback waits on a held decision, not that two real requests ever interleave that way. A two-POST test would read stronger and prove less.

Local gates on fe2d4efdc

Green: black, isort, flake8, mypy, comment-history, sync-io-in-async, loop-bound-locks, changelog-history, feature-map, focus-cue, brand-name, testpaths-coverage, docs-lint. Feature suite 38/38, config surface 543/543.

Also confirmed from the previous head's logs: all four Backend Tests (Windows) shards went green, so the ancestor-swap fix holds on the runner that caught it.

Refs #8432

[chenmingwei23]

Round on 18a877503: a session name could squat a project file

GPT's finding is real, and I reproduced it before fixing. Head is 18a877503.

With no session_project_root configured, the base was the workspace directory itself, and the session name becomes the directory name. A supplied name reaches that path, so:

POST /api/chat/slots {"name": "package.json"}

put a DIRECTORY on <workspace>/package.json. Measured against the real function:

returned: .../ws/package.json
workspace root now holds: ['README.md', 'package.json']
package.json is a DIR: True
writing the real file FAILED: IsADirectoryError [Errno 21] Is a directory

Caller-steerable, silent, and it breaks a file the session never owned. Worth being plain about the scope: it is not only the malicious name. Every session was creating a directory at the top level of the workspace, so ordinary use scattered them among the user's project files and the collision was waiting for a session named after one.

Fixed as GPT prescribed, with a fixed container: <workspace>/.sessions/<name>. The name is never derived from a session key and is dot-prefixed, so it cannot collide with a project file, and inside the container the only names a session can collide with are other session names -- which exclusive creation already refuses. That removes the class rather than filtering names, which is the part that matters: a denylist of file names to reject would have to know every file the user might later create.

The container is the one directory this function creates for itself, and the asymmetry with a CONFIGURED root is deliberate. A configured root that does not exist is a typo, and creating it would scatter directories wherever the typo pointed, so that case still refuses. The container is a fixed name under a directory that already validated, so there is nothing to guess at. Created owner-only through platform_compat.make_owner_only_dir, since a session's project directory holds that session's files.

Tests: test_a_session_name_cannot_squat_a_project_file_in_the_workspace asserts the workspace path stays free AND that the real file can still be written there afterwards, and the existing empty-root test now pins the container level. Reverting the base back to the workspace directory fails both.

Local gates on 18a877503

Green: black, isort, flake8, mypy, comment-history, sync-io-in-async, loop-bound-locks, changelog-history, feature-map, focus-cue, brand-name, testpaths-coverage, docs-lint, lockdown-before-publish. Feature suite 39/39, config surface 544/544.

Everything else on the previous head was green: 55 checks passing, with GPT the only red.

Refs #8432

[chenmingwei23]

Round on 438dcb254: the container I added last round was itself an unhardened create

GPT is right, and this is the second round in a row where the finding is a regression from my own previous fix. Head is 438dcb254.

The .sessions container closed the squat class by adding a directory level, and created it with make_owner_only_dir, which is mkdir(parents=True, exist_ok=True). An exist_ok create accepts a name that already exists, and a symlink planted at that name is such a name -- so a link at <workspace>/.sessions was followed and realpath handed back its target, putting every session directory wherever it pointed. That is the class the session create had already closed, reintroduced one level up by the fix for a different finding.

The pattern is the point, so I fixed the pattern rather than the second site. There is now ONE create routine, _pinned_dir_create, used for both directories this module makes:

• POSIX: pin_parent walks the already-resolved parent one component at a time with O_NOFOLLOW, and the child is created against that descriptor.
• Windows: the parent is held open without FILE_SHARE_DELETE, the ancestor shape is re-read while the pin is held, and a link at the child's own name is refused.
• exclusive=True for a session directory refuses an existing name outright. exclusive=False for the container accepts one only after an O_NOFOLLOW open proves it is a real directory and not a link -- accepting the name is not the same as accepting whatever currently answers to it.

Two directories created by two routines is what let the softer one drift; two directories created by one routine cannot.

The first version of this test proved nothing, and why that matters here

I also added a containment check to _default_session_root, and it alone refuses a container link pointing OUTSIDE the workspace. So my first symlink test passed with the pinned create reverted -- the containment check was doing the work, and I would have shipped an untested hardening while believing otherwise.

The case that separates them is a link pointing to a SIBLING inside the workspace: containment accepts it, because the resolved parent really is the workspace, while the session's files still land somewhere the user never chose. The test now uses that, and reverting to the exist_ok create fails it with the derived path resolving into notes/chat-17.

On the Windows red

Backend Tests (Windows) (4) failed on the previous head with exactly one failure, test/test_work_ledger.py::test_two_conductors_binding_one_worker_at_once_yield_exactly_one_binding -- outside this diff, and its own assertion message names a Windows sharing violation or an unfinished thread as the expected shortfall. That is the shard-4 flake, not this branch. All four Windows shards were green on the head before it, which is where the ancestor fix was proven.

Local gates on 438dcb254

Green: black, isort, flake8, mypy, comment-history, sync-io-in-async, loop-bound-locks, changelog-history, feature-map, focus-cue, brand-name, testpaths-coverage, docs-lint, lockdown-before-publish, agent-SDK-boundary. Feature suite 41/41, config surface 546/546.

Refs #8432

[chenmingwei23]

The board went green, and one advisory finding was a real defect in my own text

438dcb254 reached readiness: passed with 56 checks green, zero failures, and both hard lanes clean -- GPT 5.6 and Opus 4.8 no blocking findings, no [BLOCK-MERGE], PR Readiness success. I pushed anyway, because two advisory CONCERNS independently named the same thing and one of them is not a judgment call.

Head is now fb0d85ccc, rebased onto e7db5f5b8.

Fixed: the shipped help text said the wrong place

Design Review and First Principles both caught that sections.py promised "Empty = the workspace directory" while the code creates <workspace>/.sessions/<key>. That divergence is mine: the .sessions container arrived to stop a session named package.json squatting a project file, and I did not update the string that tells the user where their files go. A setting that misstates a path is worse than a re-rolled review lane, and main had moved anyway, so the rebase carried the fix at no extra cost.

The help text now reads "Empty = a '.sessions' folder inside the workspace directory, created for you", the PR description above says the same and explains why it is one level down, and config-baseline.json is regenerated.

Accepted as a declared residual: the fallback is only a debug log

Design Review is right that every degrade path -- reused key, ineligible root, OSError, workspace conflict, send-path creation -- drops the session onto the shared default while an opted-in user believes it is isolated, and that this is the same silent-sharing shape the PR exists to fix.

Not fixed here, and not because it is wrong. Surfacing it is a new user-visible surface -- a slot field or a raised log level -- rather than a correction to a shipped path, and the degrade posture deliberately matches the sibling default_project branch. It is recorded on #9218, whose second item is exactly this, so it is tracked rather than left implicit. slot.project is visible in the dashboard, so a user CAN see where a session landed; what is missing is being told, which is the part the follow-up owes.

Declined here, and it needs a human: drop session_project_root

First Principles argues the key has no named needer -- its only support is symmetry with default_project, and the .sessions container already serves everyone. The reasoning is sound and I am not rebutting it: a config key is permanent surface, and one real consumer plus a parse plus a baseline entry is thin support.

I am not dropping it unilaterally, because that subtracts a config key from a PR that is otherwise settled, and whether the repo wants the symmetry is a maintainer's call rather than mine. Both paths are cheap from here: the key can go in a two-line follow-up if the answer is no, or stay if the symmetry with default_project is the point. Flagging it for whoever merges.

Not this branch's

Backend Tests (Windows) (4) failed one head back on test/test_work_ledger.py::test_two_conductors_binding_one_worker_at_once_yield_exactly_one_binding -- outside this diff, and the assertion text names a Windows sharing violation as the expected shortfall. All four Windows shards were green on the head before it.

Local gates on fb0d85ccc

Green: black, isort, flake8, mypy, comment-history, sync-io-in-async, loop-bound-locks, changelog-history, feature-map, focus-cue, brand-name, testpaths-coverage, docs-lint. Feature suite 41/41, config surface 551/551.

Refs #8432

[chenmingwei23]

Board green on fb0d85ccc, and the GPT refusal cleared on a re-run

readiness: passed, 56 checks green, zero failures, PR Readiness success, no [BLOCK-MERGE] on this head, and all four lane verdicts name fb0d85cccd9aa7d8409160915322c573fc1cbb7b. No override was needed.

One thing worth leaving on the record, because the lane's own error text says the opposite.

Attempt 1 of GPT 5.6 Review on this head did not produce a verdict -- the provider declined to review, the job logged REFUSED: true, and Gate on findings failed with "the provider declined the request because of the diff's own content, and re-runs have not been observed to clear this class of refusal". A plain re-run of that same run concluded success on attempt 2 with REFUSED: false and a real verdict. So for this diff at least, the "re-runs have not been observed to clear this class" claim does not hold, and re-running is worth one attempt before a human is asked to adjudicate.

The trap in between is worse than the refusal. The codex-ai-review comment written by the refused attempt read "GPT 5.6 Review -- no blocking findings", three seconds before the gate failed. Reading the comment alone says the lane is clean; only the job log distinguishes a verdict from a declined review. Anyone triaging a red GPT lane whose comment looks clean should check REFUSED: in the log rather than trusting the marker comment.

This PR is a plausible trigger for that class: it is path validation, O_NOFOLLOW, and symlink-race handling, which can read to a content classifier as sandbox-escape material. It reviewed clean on 438dcb254 and refused starting at 58ff96819, so the refusal is not intrinsic to the whole diff.

Nothing further from me. The two advisory CONCERNS are dispositioned in the comment above: the debug-log-only degrade is a declared residual tracked on #9218, and whether to drop dashboard.session_project_root is a maintainer's call I have deliberately left open rather than decided.

Refs #8432

[bolichen97]

@chenmingwei23 Audited at 25d6120. The head has since moved to fb0d85c and now also touches src/kiro_crew/dashboard/state.py, so please ignore anything below that the new head already handles.

Nothing here has landed on main. new_project_per_session, session_project_root, session_project_dir and _apply_per_session_project have zero hits on origin/main, and no other open PR adds them, so there is no redundant scope to drop. The remaining scope is the whole feature.

Two rebase costs appeared since the audited merge base:

1. Merged fix(chat): allow a mid-conversation workspace switch #9084 removed the if slot.total_messages > 0 409 in api_chat_slot_workspace. Your description declines "preserve the per-session project across agent and workspace switches" on the premise that the switch refuses once a session has messages. That premise no longer holds on main, so after a rebase a mid-conversation workspace switch can re-point slot.project while the earlier per-session directory keeps its files. That is real cross-session file sharing, not an empty orphaned directory.

2. _apply_per_session_project commits slot.project with a bare if not slot.project re-read. main now uses the _CommitToken identity-gated convention in src/kiro_crew/dashboard/chat_handlers.py at lines 5792, 7566 and 7790. Please adopt it.

Please also reconcile with open #7161, which rewrites slot.project in api_chat_slot_agent and api_chat_slot_workspace. The hunks sit in different functions so both can land, but it addresses the same switch preservation gap you declined.

config-baseline.json will conflict textually with #8862, #8302 and #9315. Those are mechanical.

If the plan is still the rebuild tracked in Issue #9218, please note that here so we can follow it there.

Posted from the 2026-09-08 open-PR relationship audit (read-only, one auditor per PR); reply here if any of this is wrong.

[chenmingwei23]

Round on d4fa9e1d8: the created directory is no longer reopened by name

GPT's finding is real and reachable, and the fix turned out to be smaller than its wording suggested because half of it already exists in the repo. Head is d4fa9e1d8.

The defect

After the exclusive create, the function turned its new directory back into a path with os.path.realpath(str(candidate)). That resolves the NAME again, and between the create and that call a process running as this user -- an agent, in this product -- can drop the new child and put a link to another session's directory in its place. The checks that follow then validate whatever the name now means, and a link aimed at a SIBLING under the same parent satisfies every one of them: it is a real directory, it is not sensitive, and its parent is the configured root. So the new session was handed the other session's files.

That is precisely the adoption the exclusive create exists to refuse, reached one step later. The exclusive create closes "the directory already exists"; re-resolving the name reopened it as "the directory exists by the time I look again".

The fix

_pinned_dir_create now returns the descriptor of the directory it created, and _validated_dir_from_fd performs every post-create check against that descriptor:

• the path comes from pinned_fs.fd_real_path, which is the kernel's own answer for the inode already held open, so it carries no symlink component left to swap;
• os.fstat on the descriptor decides "is a directory", not os.path.isdir on a name;
• the sensitive-path and parent checks then run on a path that cannot have been redirected.

Every arm fails closed. fd_real_path returns None rather than falling back to a mutable pathname when the kernel cannot answer, and None becomes "".

Applied to BOTH directories, not just the flagged one. The container is created by the same routine and had the same re-resolution, so fixing only the session directory would have left the identical substitution one level up -- which is how the previous two rounds went, and the reason this module has exactly one create routine now.

chdir_fd was the part I expected to be hard. It is already done: acp/client.py binds _bound_workspace_fd alongside _spawn_work_dir and passes chdir_fd to the spawn, so the cwd is entered through a descriptor rather than a pathname. The gap was only between this function's create and its return.

The test, and the two versions of it that proved nothing

test_a_sibling_swap_after_creation_cannot_be_adopted creates a victim session directory with a file in it, swaps the new child for a link to that sibling, and asserts the result is not the victim's path and the victim's file survives.

My first version timed the swap off the sensitive-path check, and it passed against the defect. realpath runs BEFORE that check, so the name was resolved while it was still honest and the swap landed too late to matter. The swap has to be timed off the create's own return -- the window the attack actually has, and the only point both a descriptor-validating and a name-resolving version pass through. With that timing, restoring the realpath version fails with adopted a sibling session's directory.

Two rounds ago a test passed on the mutant because a containment check was doing the work; this time it was the hook's position. Worth stating as its own lesson: a mutation that survives means the test is not measuring what its name claims, and the next step is finding which line actually decides, not strengthening the assertion.

Not this branch's

Backend Tests (Windows) (4) on the previous head failed twice:

• test/test_workflows_app.py::test_main_boots_platform_before_binding_server -- worker 'gw2' crashed, tracked as ci: test_main_boots_platform_before_binding_server hangs/crashes shard 4 on every PR since #9066 #9675, hanging shard 4 on every PR since fix(workflows): bootstrap the platform context #9066 and observed on three other unrelated diffs.
• test/test_session_pool.py::TestDiscardReaping::test_wedged_shutdown_is_bounded_and_hard_killed -- TimeoutError, which only this PR showed. This diff does not touch the session pool or its shutdown path; it creates and validates a directory. This head is the second sample, and if it recurs here I will investigate it as real rather than assume timing.

Local gates on d4fa9e1d8

Green: black, isort, flake8, mypy, comment-history, sync-io-in-async, loop-bound-locks, changelog-history, feature-map, focus-cue, brand-name, testpaths-coverage, docs-lint. Feature suite 42/42, config surface 552/552.

Refs #8432

[chenmingwei23]

/ai-review override gpt 82abfc3: Same-user TOCTOU on the per-session project directory create path -- an actor able to rename directories there already holds the user's privileges, and the Opus lane assessed this same candidate family on this same path and judged it not blocking.

Recording the reasoning so the override is auditable.

The finding is that the create/open sequence for a session's project directory can be raced so the opened directory is not the one that was created. The mechanism is real and the write-up is accurate. What it requires is a process running as the same user, racing a directory it can already rename, in a tree that user owns. Such an actor does not gain anything from this path that it does not already have, which is the bar this repository applies to a reachable-trigger claim.

The second lane reviewed the same surface and reached the opposite verdict, describing the candidates as same-user races against the per-session project directory create path and declining to block on them. This override records a judgement between two disagreeing lanes rather than dismissing an uncontested finding.

The remedy the lane asks for is a contract change: the validated identity has to survive across persistence so a managed configured root can be re-pinned to its original canonical parent after a restore from stored metadata. That is a real design, and it is the right one if this class ever needs closing -- it is written up in the local backlog. It does not belong in this branch, whose subject is an opt-in per-session project directory and which is otherwise green.

An earlier attempt to satisfy the lane locally, by re-pinning at the bind site, introduced two descriptor-lifecycle defects of its own and took the blocking count from one to three. That attempt is reverted; this head carries the original, smaller surface.

[github-actions]

Human judgment recorded

@chenmingwei23 marked the gpt AI finding as false positive, not applicable, or explicitly accepted for 82abfc308119f1ad1ae3634eb3b75e59280495ad.

Same-user TOCTOU on the per-session project directory create path -- an actor able to rename directories there already holds the user's privileges, and the Opus lane assessed this same candidate family on this same path and judged it not blocking.

This decision applies only to this commit. A new push requires a new judgment.

[chenmingwei23]

@iamwhatever answering your review question:

Could you please check how it influences session teleporting?

Short answer: a per-session directory does not travel, and the one path where that
could bite is a pre-existing behaviour this feature widens rather than introduces.

First, a caveat on terms, because I do not want to answer a different question than
you asked. There is no teleport anywhere in the product: the only occurrences are
placeholders for an INVALID name in tests (apply_conductor_action(..., "teleport")
asserts CODE_INVALID_ACTION, and computer_teleport is an unknown-tool example),
and there is nothing in the frontend. So I took "teleporting" to mean moving a live
session somewhere else and checked the three mechanisms that actually do that. If
you meant something else, say which and I will redo this against it.

Turns executed on a peer crew (executor == "remote"). The project directory is
never sent. remote_relay._PEER_CONTROL_SEGMENTS forwards exactly agent, model,
workspace and reasoning_effort to the peer slot, and project is not among them,
so the peer resolves its own working directory from its own workspace binding and
never sees a local path. There is therefore no "path missing on the far side" failure
mode here -- the far side is never told the path.

What does happen locally: a newly created remote-bound slot is a new slot, so it does
derive a directory, and that directory scopes local file search, @-mention completion
and <project>/.kiro/steering resolution via active_project_dir. It does not become
the agent's cwd, because the agent runs on the peer. So the effect is a local search
scope pointing at an empty directory, not a broken turn.

Resume from history. The field is persisted (session_control and
slot_projection write project, chat_persistence and channel_slots read it
back) and rehydrate applies it as-is: if meta.get("project"): slot.project = meta["project"]. There is no isdir check on that path, unlike
POST /api/chat/slots/{slot}/project, which validates before accepting. So a session
whose directory was deleted -- or whose metadata was carried to a machine where that
path does not exist -- comes back naming a directory that is not there, and each
consumer decides what that means downstream.

This is worth being precise about: that asymmetry is not introduced here. A project
derived from dashboard.default_project restores through the same unvalidated
assignment today. What this feature changes is how MANY sessions carry a project, so
it makes an existing gap easier to reach rather than creating one. I would rather fix
it as its own change than fold an unrelated validation into this one, but if you want
the isdir check on the rehydrate path added here, say so and I will.

Fork. chat_fork copies the parent's value (new_slot.project = slot.project),
so a fork and its parent share one directory. That is deliberate -- the fork inherits
the parent's working context -- but it does mean "one directory per session" is not
literally true across a fork. Worth knowing if you were expecting isolation there.

Two related things already on record, so you can see where the edges are: the
remaining GPT finding on this head is a same-user create/open race in
_pinned_dir_create that a maintainer override dispositioned, and the broader
"slot.project is a name without an identity" problem -- which is the same family as
the unvalidated restore above -- is captured for separate work rather than being
attempted here.

Nothing in the diff changed for this answer. Could you re-review when you have a
moment?

[chenmingwei23]

@iamwhatever this is out of draft now, which is probably why it did not show up as
something waiting on you.

Your question about session teleporting is answered in
#8997 (comment) -- short
version: the project directory is never forwarded to a peer crew, so it does not
travel and the far side is never handed a path that could be missing there; a resumed
session does restore the field without an isdir check, but that same unvalidated
restore already applies to a project derived from dashboard.default_project, so this
feature widens an existing gap rather than adding one, and I left it for its own
change unless you want it here.

The diff has not moved since that answer. Head is 82abfc3, with 56 checks green and
none red.