Skip to content

End a Kiro Crew session when Crew is done with it - #1181

Merged
realtonyyoung merged 6 commits into
mainfrom
tonyyoung/ai-3192-kiro-crew-session-end
Sep 27, 2026
Merged

realtonyyoung merged 6 commits into
mainfrom
tonyyoung/ai-3192-kiro-crew-session-end

Conversation

@realtonyyoung

@realtonyyoung realtonyyoung commented Sep 26, 2026 •

Copy link
Copy Markdown
Collaborator

No GitHub issue — AI-3192

What & why

Kiro has no session-end hook, so the watcher ends a Kiro session when its kiro-cli process exits. Kiro Crew keeps one process running for a whole chat and, by default, runs its sub-agents inside that same process, so a finished sub-agent, or a chat Crew had moved to a new session, stayed active until Crew itself stopped. For a Kiro session on a machine with Crew, the watcher now also polls Crew's files once connected, and ends the session as crew_finished when Crew writes the sub-agent's tombstone.json, or when the session map shows the chat now names another session.

Where to look

  • False ends are guarded:
    • a session current in any chat is live;
    • an unreadable map ends nothing;
    • a missing chat entry must persist across three readable checks;
    • a session Crew does not know is never ended.
  • Sub-agent lookup: anchored at the session's own creation time rather than a count of recent directories, and skipped for chat sessions.
  • Short sessions: a crew_finished session below the buffering threshold still flushes its lines and posts its end.

Verification

  • Real Crew state: the finished sub-agent and a replaced chat session read as finished; both current chat sessions and a plain Kiro session do not.
  • Tests: tombstones, busy and late-started searches, chats moving, resuming elsewhere or dropping, and interleaved unreadable reads.

🤖 Generated with Claude Code

Crew keeps one kiro-cli process for a chat and its in-process sub-agents;
state.json's status is never updated at the end, so tombstone.json is the signal.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@linear-code

linear-code Bot commented Sep 26, 2026

Copy link
Copy Markdown

AI-3192

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

End Kiro sessions when Crew finishes or replaces them

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 20-40 Minutes

Grey Divider

AI Description

• Polls Crew tombstones and session maps to detect completed hosted Kiro sessions.
• Ends detected sessions as crew_finished while preserving parent-exit precedence.
• Shares concurrent JSON parsing and tests malformed, moved, dropped, and aged records.
Diagram

sequenceDiagram
    actor Crew as Kiro Crew
    participant Watch as WatchCommand
    participant EndWatch as Crew End Watch
    participant Records as Crew Records
    participant Files as Crew State Files
    participant Server as Session API
    Crew->>Files: Write tombstone or session map
    Watch->>EndWatch: Create for Kiro session
    loop Every 5 seconds
        EndWatch->>Records: Read session state
        Records->>Files: Open shared JSON files
        Files-->>Records: Current or absent record
        Records-->>EndWatch: Parsed state
    end
    alt Crew finished
        EndWatch-->>Watch: Cancel as crew_finished
        Watch->>Server: Post session end
    else Session remains active
        Watch->>Watch: Continue watching
    end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Filesystem event subscription
  • ➕ Could react immediately instead of polling every five seconds.
  • ➕ Avoids repeated reads while Crew sessions remain active.
  • ➖ Filesystem events can be duplicated, missed, or raised before writes complete.
  • ➖ Requires more recovery and synchronization logic across supported platforms.
  • ➖ The session map still needs stateful interpretation after each event.

Recommendation: Keep the bounded periodic polling approach. Kiro provides no session-end hook, and resilient reads already treat missing or half-written records as inconclusive; polling integrates cleanly with the existing cancellation lifecycle and is safer than relying on filesystem notification delivery.

Files changed (7) +326 / -51

Bug fix (2) +134 / -4
KiroCrewSessionEndWatch.csDetect Crew-owned session completion +90/-0

Detect Crew-owned session completion

• Adds a stateful watcher that recognizes sub-agent tombstones and chat session replacement or removal. Unknown sessions and unreadable records remain active, while discovered sub-agent directories are cached beyond the bounded initial scan.

src/Capacitor.Cli.Core/Harness/Kiro/KiroCrewSessionEndWatch.cs

WatchCommand.csPoll Crew state and emit crew_finished +44/-4

Poll Crew state and emit crew_finished

• Starts a five-second Crew completion poll for top-level Kiro watchers when Crew is present. Cancels completed watchers with 'crew_finished' and centralizes end-reason precedence so parent exit remains authoritative.

src/Capacitor.Cli/Commands/WatchCommand.cs

Refactor (2) +52 / -46
KiroCrewParentResolver.csReuse shared Crew record parsing +12/-46

Reuse shared Crew record parsing

• Replaces private JSON reading and conversion helpers with 'KiroCrewRecords', preserving parent and child resolution behavior while removing duplicated parsing code.

src/Capacitor.Cli.Core/Harness/Kiro/KiroCrewParentResolver.cs

KiroCrewRecords.csCentralize resilient Crew JSON reads +40/-0

Centralize resilient Crew JSON reads

• Adds size-bounded, shared-access JSON reading for files Crew may rewrite concurrently. Also centralizes string, GUID, and epoch conversions used by Crew integrations.

src/Capacitor.Cli.Core/Harness/Kiro/KiroCrewRecords.cs

Tests (2) +139 / -0
KiroCrewSessionEndWatchTests.csCover Crew completion state transitions +106/-0

Cover Crew completion state transitions

• Tests tombstone detection, cached sub-agent discovery, replaced and dropped chats, malformed maps, already-replaced sessions, and sessions unknown to Crew.

test/Capacitor.Cli.Core.Tests.Unit/Harness/Kiro/KiroCrewSessionEndWatchTests.cs

WatcherCrewEndDecisionTests.csVerify Crew watcher eligibility and end precedence +33/-0

Verify Crew watcher eligibility and end precedence

• Confirms only eligible Kiro watchers monitor Crew, validates the 'crew_finished' reason, and ensures process exit outranks simultaneous Crew completion.

test/Capacitor.Cli.Tests.Unit/Commands/WatcherCrewEndDecisionTests.cs

Documentation (1) +1 / -1
README.mdDocument Crew-aware Kiro session completion +1/-1

Document Crew-aware Kiro session completion

• Explains that Crew-hosted sub-agents end when tombstoned and chat sessions end when Crew replaces them, rather than waiting for the shared process to exit.

README.md

@qodo-code-review

qodo-code-review Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Dropped chats await process exit ✓ Resolved 🐞 Bug ☼ Reliability
Description
The polling task waits five seconds before its first IsFinished call, so ChatMovedOn has not
cached _chatKey when monitoring begins. If Crew drops the chat from session_map.json during that
interval, the method sees neither the key nor the session and keeps returning false, leaving process
exit as the only remaining end signal.
Code

src/Capacitor.Cli/Commands/WatchCommand.cs[R497-500]

+            _ = Task.Run(async () => {
+                while (!cts.Token.IsCancellationRequested) {
+                    try {
+                        await Task.Delay(TimeSpan.FromSeconds(5), time, cts.Token);
Relevance

●●● Strong

Reliability findings addressing missed initial polling state are typically accepted; this directly
strands dropped chats.

PR-#1149

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The integration delays before its first state check, while ChatMovedOn can recognize a removed
entry only after a prior successful read has populated _chatKey. With an empty map on that first
read, the _chatKey is null branch returns false and has no information it can cache on later
polls.

src/Capacitor.Cli/Commands/WatchCommand.cs[494-505]
src/Capacitor.Cli.Core/Harness/Kiro/KiroCrewSessionEndWatch.cs[66-88]
test/Capacitor.Cli.Core.Tests.Unit/Harness/Kiro/KiroCrewSessionEndWatchTests.cs[75-83]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A Crew chat removed before the watcher's first delayed poll cannot be associated with its session, so Crew completion is never detected.

## Fix Focus Areas
- src/Capacitor.Cli/Commands/WatchCommand.cs[494-505]
- src/Capacitor.Cli.Core/Harness/Kiro/KiroCrewSessionEndWatch.cs[66-88]

## Recommended Fix
Perform an initial synchronous `IsFinished` check before entering the delayed polling loop so the watcher caches the current chat key immediately. Reuse the same completion-and-cancellation handling for both the initial check and subsequent polls, and add a test where the map entry is removed before the first delayed poll.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. Busy crews miss finished agents ✓ Resolved 🐞 Bug ☼ Reliability
Description
FindSubagentDir repeatedly examines only the 64 newest directories and caches a directory only
after finding the watched session. When at least 64 newer directories precede the first successful
scan, the target remains outside every subsequent scan even if it contains a valid tombstone, so
Crew completion is never reported.
Code

src/Capacitor.Cli.Core/Harness/Kiro/KiroCrewSessionEndWatch.cs[R47-50]

+            dirs = new DirectoryInfo(crew.SubagentsDir).EnumerateDirectories()
+                .OrderByDescending(d => d.LastWriteTimeUtc)
+                .Take(SubagentScanLimit)
+                .ToList();
Relevance

●●● Strong

Recent precedent accepted removing bounded scans that miss older Crew records.

PR-#1149

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The target directory is memoized only after discovery, while every miss rebuilds the same
mtime-ordered newest-64 list. The existing many-newer-directories test primes the cache before
adding those directories, so it does not exercise the initial-miss case; the parent resolver also
documents that Crew histories can exceed a bounded live window and uses a larger 256-record limit.

src/Capacitor.Cli.Core/Harness/Kiro/KiroCrewSessionEndWatch.cs[35-63]
test/Capacitor.Cli.Core.Tests.Unit/Harness/Kiro/KiroCrewSessionEndWatchTests.cs[39-53]
src/Capacitor.Cli.Core/Harness/Kiro/KiroCrewParentResolver.cs[18-25]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The bounded scan always revisits the same newest 64 sub-agent directories, so an older target can remain permanently invisible.

## Fix Focus Areas
- src/Capacitor.Cli.Core/Harness/Kiro/KiroCrewSessionEndWatch.cs[35-63]
- test/Capacitor.Cli.Core.Tests.Unit/Harness/Kiro/KiroCrewSessionEndWatchTests.cs[39-54]

## Recommended Fix
Make unsuccessful scans progress through older directories across polling iterations, while retaining a bounded amount of work per check and caching the target once found. Add a test where more than 64 newer directories already exist before the watcher's first check, rather than only after the target has been cached.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

3. A test comment embeds run metadata ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
KiroCrewSessionEndWatchTests describes its fixture as coming from a real run and embeds the
run-specific identifiers chat-2 and 65eed35b. When fixture values or Crew record shapes change,
this historical provenance can become stale without documenting any behavior-critical constraint for
the test.
Code

test/Capacitor.Cli.Core.Tests.Unit/Harness/Kiro/KiroCrewSessionEndWatchTests.cs[5]

+/// <summary>Crew state shaped like a real run: dashboard chat <c>chat-2</c> and its sub-agent <c>65eed35b</c>.</summary>
Relevance

● Weak

Recent precedent rejected removing detailed operational context from comments when it documents
observed behavior.

PR-#978

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The added summary explicitly says the state is shaped like a real run and names identifiers from
that run. Rule 2897915 prohibits execution or run metadata in comments, while rule 2762993 permits
comments only when they explain a current non-obvious, behavior-critical constraint.

Rule 2897915: Avoid time-sensitive or process-reference metadata in code comments
Rule 2762993: Restrict comments to documenting non-obvious, behavior‑critical constraints
test/Capacitor.Cli.Core.Tests.Unit/Harness/Kiro/KiroCrewSessionEndWatchTests.cs[5-5]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test class summary records historical real-run provenance and run-specific identifiers instead of an active testing constraint.

## Fix Focus Areas
- test/Capacitor.Cli.Core.Tests.Unit/Harness/Kiro/KiroCrewSessionEndWatchTests.cs[5-5]

## Recommended Fix
Remove the summary comment, or replace it with a concise statement of a non-obvious fixture invariant without referring to a real execution or embedding run-specific identifiers.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 64 rules
✅ Cross-repo context — repo relationships
  Explored: repo: kurrent-io/kcap-server (sha: 5f4fb8ef) — View relationship
Review mode: ⚖️ Balanced: Downgraded extended -> standard: change is below the extended eligibility bar (hunks 17/18, lines 377/200; both must reach the floor). Router rationale: This adds substantial concurrent watcher logic across multiple Kiro state paths, file-read semantics, lifecycle/end-reason precedence, and integration points, creating several independent opportunities for subtle session-ending defects.

Grey Divider

Tip of the day
💡 Did you know, you can describe a rule in plain language on the Rules page and Qodo drafts it for you

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread src/Capacitor.Cli/Commands/WatchCommand.cs Outdated
Comment thread src/Capacitor.Cli.Core/Harness/Kiro/KiroCrewSessionEndWatch.cs
realtonyyoung and others added 3 commits September 26, 2026 11:49
Crew rewrites the session map in place, so a missing or unreadable entry
must persist before it counts; a replaced entry naming another session is proof.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A count cap let a busy Crew keep a sub-agent's directory out of every scan;
its directory is created at spawn, just before the session starts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@realtonyyoung

Copy link
Copy Markdown
Collaborator Author

Qodo finding 3 (test comment embeds run metadata): fixed in e1fc668 — the fixture summary now says only that the records are in the shapes Crew writes, with no run-specific identifiers.

realtonyyoung and others added 2 commits September 26, 2026 11:59
A cancel before the hub connects skips the final drain and end POST, and a
watcher restarted later must still find a sub-agent spawned long before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@realtonyyoung
realtonyyoung merged commit 165ff4e into main Sep 27, 2026
8 checks passed
@realtonyyoung
realtonyyoung deleted the tonyyoung/ai-3192-kiro-crew-session-end branch September 27, 2026 23:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant