fix(issue-radar): open lock sidecars non-truncating before the acquire by chenmingwei23 · Pull Request #9275 · kirodotdev/KiroCrew

chenmingwei23 · 2026-09-07T17:55:27Z

Problem / Motivation

Eighteen lock context managers in issue_radar's backend open their lock sidecar
with open(lock_path, "w") and only then hand the descriptor to
platform_compat.file_lock(..., exclusive=True). "w" truncates the file at
open -- before the lock is held.

Why it matters

On Windows the acquire routes to msvcrt.locking. A truncating open of a lock
file whose first byte another holder already locked raises a sharing violation
instead of waiting. The contending acquirer crashes before it reaches the lock,
so the critical section it was serialising runs unserialised -- a silent loss of
mutual exclusion, not a visible error. POSIX flock tolerates the truncate,
which is why the class is invisible on Linux and shows up only as intermittently
red Windows shards. The same class fixed one-write-short concurrency assertions
elsewhere in the sweep (test_work_ledger assert 2 == 3, test_sel
assert 39 == 40). issue_radar's store carries connect/settings/label/tag caches
and the whole crew ledger behind these locks, so a lost acquire can drop a config
write or a crew-record update on Windows.

What changed (motivation -> approach -> change)

Symptom: on Windows a second process contending for one of these locks fails
instead of waiting, and the guarded read-modify-write is not serialised.

Root cause: the lock file is opened truncating ("w"). The truncate lands before
the lock, and on Windows a truncating open of an already-locked file is itself
what raises the sharing violation.

Change: each of the 18 sites now ensures the parent dir exists,
lock_path.touch(exist_ok=True), then open(lock_path, "r+") -- writable
(msvcrt.locking needs a writable handle, so "r" is not an option) but NOT
truncating -- acquiring on that handle exactly as before. This is the same shape
and rationale as work_ledger._open_lock (#9237) and session_pid.py (#9250).
No shared helper is introduced: consolidating this preamble into one public
opener is #9267's job, which the reporter deliberately kept separate. store.py
takes 7 sites, crew_store.py 11. The lock directories are already ensured by
data_dir / repo_data_dir / crews_dir; the added touch makes each site
self-sufficient and matches the reference fix.

eval/bench/safepath.open_write_nofollow is not the tool here: its load-bearing
flag is O_EXCL (atomic exclusive CREATE, refuse an existing name) for
write-once/temp files, so it cannot open a persistent lock sidecar that is reused
across acquires.

Tests

src/kiro_crew/apps/builtins/issue_radar/tests/test_lock_no_truncate.py -- pins
the property the sweep uses (#9237/#9250): seed the lock file with bytes, acquire
and release the lock, assert the bytes survive. Five cases cover both modules and
all three path expressions the sweep touched -- a bound lock_path variable
(_config_lock), a .with_suffix(".json.lock") cache sidecar
(issues_cache_lock), a per-item .lock name (issue_write_lock), the
inline-expression _skip_lock, and a real read-modify-write consumer
(write_settings). Each fails on every platform under the old truncating open
and passes under touch + "r+".

Proven non-vacuous: reverting _config_lock alone back to open(lock_path, "w")
reddens its case with b'' != b'held by a prior acquirer\n'; the skill's
prove.py reports PROVEN with the production hunks reverted.

Manual verification

N/A -- unit coverage sufficient. The change is a mechanical open-mode fix on
Linux (dev host); the Windows sharing-violation path is what the truncation
property stands in for, and it is the same property the two merged sibling PRs
pinned.

Related Issues

Closes #9263
Refs #9248

Coordination note for the reviewer: on #9248, @jeeshofone claimed these exact
issue_radar sites (2026-09-07 13:57Z) with a shared-helper plan and a "PR to
follow shortly"; that PR has not appeared, and open PR #9269 covers a different
file set (deploy/mcp/pod), not issue_radar. This PR takes the per-subsystem local
route the #9263 body prescribes and leaves the shared-opener consolidation to
#9267. If #9267 lands first and consolidates these 18 sites, this PR should
defer to it.

Pattern harvest

Defect class: a lock file opened truncating (open(path, "w")) before the
descriptor is handed to a lock acquire. Truncation happens before the lock is
held; on Windows a truncating open of an already-locked file raises a sharing
violation instead of waiting, silently losing mutual exclusion, while POSIX
flock hides it.

Rule candidate: review-prompt / semgrep -- flag open(<x>, "w") (or
os.open(..., O_TRUNC)) whose descriptor is passed to a file_lock /
flock_exclusive / msvcrt.locking acquire on the same handle. The fix is
always touch(exist_ok=True) + open(..., "r+").

chenmingwei23 · 2026-09-07T17:55:43Z

Intent: Stop the 18 issue_radar lock sidecars from being opened truncating (open(path, "w")) before their platform_compat.file_lock acquire, so a contending acquirer on Windows waits instead of hitting a sharing violation and the guarded critical section is actually serialised. Fix each site in place with touch(exist_ok=True) + open(..., "r+"), matching work_ledger._open_lock (#9237).
Not a goal: Introducing a shared non-truncating lock-opener helper -- that is #9267's consolidation across ~30 sites in 12 files, deliberately kept separate. No behaviour change to the guarded read-modify-write logic; no new public API.

github-actions · 2026-09-07T18:00:50Z

Design Review (Fable 5) — ✅ PASS

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

Design-Verdict: PASS

Root-cause fix (truncate-before-acquire) applied uniformly to all 18 sites, matching the established #9237/#9250 pattern, with consolidation explicitly tracked in #9267.

[DESIGN-REVIEWED] 25f6e25

github-actions · 2026-09-07T18:01:06Z

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 25f6e25e1b4fa3a75cfa477f147caef85d271c2d — 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 claims verified. The diff contains exactly the 18 open-mode changes (7 in store.py, 11 in crew_store.py) plus one test file — nothing else. The reference helper work_ledger._open_lock (src/kiro_crew/work_ledger.py:532) has the identical shape and Windows rationale, and is module-private, so reusing it would mean importing a private symbol cross-module; the description defers consolidation to #9267, a split the reporting issue prescribed. The safepath.open_write_nofollow disqualification is accurate (its O_EXCL refuses an existing name — unusable for a reused sidecar). My repo-wide grep for open(*lock*, "w") handed to a lock acquire found 12 remaining production siblings: deploy/pending.py (3), deploy/profiles.py (2), mcp_tools/control.py (1), pod/runtime.py (1) — covered by PR #9269 per the description — plus aws_control backend (4: library.py:196, backup.py:177, shares.py:202, 222) and webhooks.py:189, which no named PR claims.

First-Principles-Verdict: PASS

Cause-level fix at all 18 issue_radar sites — the truncating open is itself the Windows failure — with nothing riding along and the sweep split explicitly recorded.

What this change ships

Intent: stop a contending Windows process from crashing (and silently losing mutual exclusion) when acquiring any issue_radar file lock — a FIX.

A second Windows acquirer of any issue_radar lock now waits instead of crashing (18 sites) — justified
Lock sidecar contents survive each acquire on every platform — justified, same mechanism
Each site now creates its own lock file (touch) instead of relying on directory helpers — declared, required by "r+"
New regression test: seed sidecar bytes, acquire, assert survival (5 representative cases) — justified

Watch

Grep for open(*lock*, "w") → lock acquire leaves 12 siblings repo-wide. Seven (deploy ×5, mcp_tools ×1, pod ×1) are claimed by fix: avoid truncating lock files before acquiring lock on Windows #9269 per the description; the 5 in aws_control/backend (library.py:196, backup.py:177, shares.py:202, 222) and webhooks.py:189 are claimed by no PR named anywhere — confirm the Lock files opened truncating before the acquire: five sites still carry the Windows defect fixed in 9237 #9248 sweep tracks them, or they stay broken after the sweep closes.
18 inlined copies of the same 3-line preamble now exist pending Consolidate the non-truncating lock-open preamble into one shared helper #9267's shared opener; if Consolidate the non-truncating lock-open preamble into one shared helper #9267 stalls, work_ledger._open_lock's shape is the consolidation target.

[FIRST-PRINCIPLES-REVIEWED] 25f6e25

github-actions · 2026-09-07T18:02:04Z

Opus 4.8 Review — ✅ no blocking findings

Reviewed 25f6e25e1b4fa3a75cfa477f147caef85d271c2d — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 25f6e25

Verdict parsed from the review's SHA-scoped output markers for commit 25f6e25e1b4fa3a75cfa477f147caef85d271c2d.

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

github-actions · 2026-09-07T18:02:50Z

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 25f6e25e1b4fa3a75cfa477f147caef85d271c2d and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 25f6e25

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

The 18 lock context managers in issue_radar's store.py and crew_store.py opened the lock sidecar with open(lock_path, "w") and only then handed the descriptor to platform_compat.file_lock(..., exclusive=True). "w" truncates at open, before the lock is held. On Windows the acquire routes to msvcrt.locking, and a truncating open of a lock file whose first byte another holder already locked raises a sharing violation instead of waiting, so the contending acquirer fails before it reaches the lock and the mutual exclusion silently does not happen. POSIX flock tolerates it, which is why it is invisible on Linux. Each site now ensures the parent dir, lock_path.touch(exist_ok=True), then open(lock_path, "r+") -- writable (msvcrt.locking needs it) but not truncating -- acquiring on that handle exactly as before. Same shape and rationale as work_ledger._open_lock (#9237) and session_pid.py (#9250). Refs #9248

chenmingwei23 · 2026-09-07T18:08:36Z

register temporary-directory cleanup immediately span=4c7bce175910

self-added: yes

Fixed in 25f6e25. setUp now calls self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True) on the line right after mkdtemp, and tearDown is removed. addCleanup callbacks run even when a later setUp statement raises, so a temp dir can no longer outlive a failed setup; tearDown does not run in that case, which was the leak.
Class-level ruling: any "a resource acquired in setUp leaks if setUp raises before tearDown is reached" finding in this file is answered by the same construction -- register cleanup with addCleanup at the point of acquisition rather than relying on tearDown.

dwu96

Tier 1 auto-approve: fix (3 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: lock sidecars were opened with mode "w", which truncates at open; the fix touches the path then opens "r+" so the writable handle msvcrt.locking needs no longer destroys the lock file, with a new no-truncate regression test.

iamwhatever

Tier 1 auto-approve: fix (3 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: issue-radar lock sidecars were opened with "w", truncating the lock file at acquire; every acquire site in crew_store.py and store.py now touches then opens "r+", with a regression test pinning non-truncation.

#9275) The 18 lock context managers in issue_radar's store.py and crew_store.py opened the lock sidecar with open(lock_path, "w") and only then handed the descriptor to platform_compat.file_lock(..., exclusive=True). "w" truncates at open, before the lock is held. On Windows the acquire routes to msvcrt.locking, and a truncating open of a lock file whose first byte another holder already locked raises a sharing violation instead of waiting, so the contending acquirer fails before it reaches the lock and the mutual exclusion silently does not happen. POSIX flock tolerates it, which is why it is invisible on Linux. Each site now ensures the parent dir, lock_path.touch(exist_ok=True), then open(lock_path, "r+") -- writable (msvcrt.locking needs it) but not truncating -- acquiring on that handle exactly as before. Same shape and rationale as work_ledger._open_lock (#9237) and session_pid.py (#9250). Refs #9248 (cherry picked from commit 81d5751)

A lock file opened with open(path, "w") is truncated before any lock is held; on Windows the acquire is msvcrt.locking on the already-truncated file, so a contending process can observe or produce an empty lock file and crash out of the critical section (kirodotdevGH-9248). Adds platform_compat.open_lock_file — a create-or-open (O_RDWR|O_CREAT, never truncating) opener yielding a raw fd for file_lock/flock_exclusive — and converts the four aws_control lock sites (backup, library, shares x2), the last unclaimed offenders after kirodotdev#9250/kirodotdev#9275/kirodotdev#9279/kirodotdev#9237. Pins the property fleet-wide: a contract test scans the source tree for truncating opens handed to a lock acquire, so a new offender fails with its file and line. deploy/pending.py and deploy/profiles.py are exempted while PR kirodotdev#9269 (which owns those sites) is in flight. Fixes kirodotdev#9248 (remaining sites).

…9316) A lock file opened with open(path, "w") is truncated before any lock is held; on Windows the acquire is msvcrt.locking on the already-truncated file, so a contending process can observe or produce an empty lock file and crash out of the critical section (GH-9248). Adds platform_compat.open_lock_file — a create-or-open (O_RDWR|O_CREAT, never truncating) opener yielding a raw fd for file_lock/flock_exclusive — and converts the four aws_control lock sites (backup, library, shares x2), the last unclaimed offenders after #9250/#9275/#9279/#9237. Pins the property fleet-wide: a contract test scans the source tree for truncating opens handed to a lock acquire, so a new offender fails with its file and line. deploy/pending.py and deploy/profiles.py are exempted while PR #9269 (which owns those sites) is in flight. Fixes #9248 (remaining sites).

agent_skill_globs re-read and re-security-validated every agent spec (77 files, ~115 ms) on every call, and it sits on the session-context build path — paid at every session start, subagent spawn, and full-context cron/monitor wake, plus the dashboard prompts listing. Cache the result per (agents_dir, agent) against the same stat-only directory signature the list_agents cache already uses, caching [] misses too; clear_list_agents_cache() drops both caches. Measured on a 77-agent dir: 115 ms -> 0.5 ms per call; warm build_session_context 87 ms -> ~14 ms. fix(bench): warm real embedder before KB evaluation (#9295) Wait for the configured in-process embedding backend in one-shot KB benchmark runs instead of refusing during a cold model load. Preserve non-blocking availability for normal Knowledge traffic and cover blocking and fallback backend contracts. fix(acp): derive fallback spellings when the model config-option push is rejected (#9281) The claude adapter advertises model variants under spellings its 'model' config option does not accept (prefixed provider ids with a [1m] window marker), and the fold in resolve_wire_model_id only works against a warm advertised-model cache. On a cold cache the raw id reached the wire, the adapter rejected it, and the session reset onto the default with no explanation. Derive fallback candidates from the id itself at both config-option wire sites and let the adapter judge each: verbatim, then prefix-stripped, then window-stripped. An explicit switch (set_model) raises the typed AcpModelUnavailable instead of a generic AcpError when every spelling is refused; a startup application of an inherited value keeps the session on the backend default, mirroring the withhold contract and the effort push's value-rejection ladder. fix(usage): surface refused-transcript count so UNC home stops reading zero (#9254) On a Windows roaming-profile (UNC) home the usage page counted zero sessions silently: every transcript is refused by validate_file_path and the count was only logged server-side. Carry refused_transcripts in the /api/usage/kiro payload and render a warning banner on the usage page so the zero is shown as incomplete rather than as a fact. Does not admit the sessions dir to the UNC gate (that trust decision is open issue #8079); the refusal itself is unchanged. Closes #6733 fix(dashboard): mark inherited default agent on composer chips (#8770) (#9253) fix(mcp): expand header value references in the remote probe (#9249) * fix(mcp): expand header value references in the remote probe The dashboard's remote MCP probe sent configured headers verbatim, so a static auth header whose value carries a documented ${VAR}/${env:VAR} runtime reference reached the server as literal text and was correctly rejected. Because _needs_authorization treats any Authorization key as a supplied credential, the 401 rendered the row Error / HTTP 401 with a hint advising the user to delete a header that authenticates fine in every session. The probe now resolves header value references through the gateway rewriter's existing declared-env expander (same regex, same credential-filtered source view, unresolved stays literal for kiro-cli parity), uses the expanded map for the request and for _needs_authorization's sent-headers view, and threads it into both redact_mcp_error call sites so an error echoing a resolved secret is scrubbed by the exact-value layer. An Authorization value still carrying an unresolved reference supplied nothing, so it no longer suppresses needs_auth. Fixes #9206 * fix(mcp): scrub individually resolved placeholder values in probe errors A partially expanded header value (${TOKEN}${MISSING}) sends '<resolved>${MISSING}', so neither the full sent value nor the Authorization suffix in redact_mcp_error's scrub set matches a server echoing only the resolved fragment. _expand_header_placeholders now also returns each individually resolved placeholder value, and both redaction call sites pass them as a new extra_values leg, folded in under the same credential length regimes (below the minimum length a value is skipped, since no boundary rule separates it from prose words). Adopts the GPT 5.6 review lane's blocking finding on mcp_discovery.py:680. --------- Co-authored-by: dwu96 <dwu96@amazon.com> perf(dev-fleet): skip npm ci and rebuild on a backend-only sync (#9227) A non-edition Pull + Build appended npm ci and npm build + stage unconditionally, so a sync that moves only Python still deleted and reinstalled website/node_modules from an unchanged lockfile and re-ran a full vite build reproducing a byte-identical bundle. The skip decision is made post-fetch by the preflight step -- the only window where the incoming ref is pinned and the worktree has not yet been merged to it. The preflight signals the verdict by exiting a reserved code (EXIT_FRONTEND_SKIP) that the runner trusts ONLY from the preflight step's label; the runner then suppresses the npm ci and build+stage steps, holding the verdict in its own state. A worktree-run step (a pip lifecycle script) exiting the same code is demoted to a plain failure, so it cannot forge a skip and ship stale assets. Skipping a step also skips its node_modules transaction, which must not run for a no-op or it would delete the tree. feat(chat): Formatted/Raw toggle on markdown content cards (#9224) Adds a Formatted | Raw segmented toggle to markdown content cards in the chat transcript, matching the control tool detail cards carry. Formatted renders through the transcript's markdown pipeline; Raw is the verbatim source with the edit affordance. Formatted is the default, overridable via a new Chat setting. Closes #9196. fix(dashboard): swipe navigation for multi-image viewer (#9190) The image viewer opens with a SET -- dispatchLightbox() collects every img[data-lightbox-image] in the nearest [data-image-scope] -- but the only way through it was ArrowLeft/ArrowRight. On a phone there is no keyboard, so every image after the first was unreachable and nothing on screen said the set existed. A horizontal drag now pages through the set. The one-finger overlay gesture locks an AXIS once it crosses the slop, then either pulls down to dismiss (unchanged) or sideways to page, so a diagonal drag can no longer do both. Four product decisions, each matching an existing decision in the same component rather than inventing one: * Commit on DISTANCE only, like the dismiss path, and at a shorter threshold (64px vs 96px): paging is reversible, a dismiss destroys the viewing context, so paging can commit on less travel. * Rubber-band at index 0 and at the last image, reusing the divisor the upward dismiss drag already used -- a silent no-op at the ends reads as a broken gesture. * Show position in the set ("2 of 4") on an aria-live pill. Without an affordance the gesture is invisible, and this is also the first thing to announce the image change to a screen reader. * Own the gesture only at fit zoom, exactly as dismiss does; above fit a horizontal drag is already pan, handled on the <img>. Owning the horizontal axis here is safe because the app-wide nav drawer yields to any element whose computed touch-action is none, which the overlay already set to take page zoom. The keyboard path, the pinch, the double-tap and the desktop click-to-close are untouched. Closes #5799 fix(security): never waive exfil redaction for model-authored URLs (#7820) (#9189) A GitHub issue-prefill link is redacted in chat as `[REDACTED: suspicious URL to github.com]`, which #7820 reports as a false positive. Two earlier rounds of this change waived the aggregate query-LENGTH signal for that shape -- first on the validated shape alone, then additionally pinned to this project's own tracker. Both are withdrawn. Neither was safe, and the difference between them was only who reads the payload. What reaches `redact_exfiltration_urls` is MODEL-AUTHORED text: injected content steers the model into emitting a prefill URL whose `body` carries percent-encoded private context; the waiver skips the length check; the link renders as the familiar "file an issue" affordance; the user submits it; and the issue is PUBLIC, so the attacker reads it. Pinning the repository does not close that -- this project's tracker is world-readable by design. A URL's shape says nothing about who authored it, and a marker placed IN the text travels in the channel the injection controls. So no validation performed on a model-emitted URL can establish provenance, and that is a property of the class rather than of any one spelling of the check. - Remove the frontend waiver: `isPrefilledIssueUrl` and the `EXFIL_ISSUE_*` constants in `website/src/utils/sanitize.ts` (they landed for #7824). The aggregate-length signal there is unconditional again. This is the production change in this commit. - The BACKEND needed no deletion after rebasing onto #9183, which split `security.py` into a package: `security/exfil.py` was re-derived from a base that never carried the waiver, so its length gate is already unconditional. What lands there is the reason it must stay that way, recorded at the gate so the next person with a long legitimate URL narrows the heuristic for every host instead of carving out a shape. - Deliver #7820's feature through the TRUSTED STRUCTURED CHANNEL the product already ships, rather than through the redactor. `diagnostics._issue_url` assembles the prefill query from structured fields (version, channel, what-happened, context, install) with `urlencode`; it travels as `BundleResult.github_issue_url` on a JSON response no redactor scans; and `ReportProblemModal` / `ReportProblemCard` render their own `<a href={result.github_issue_url}>`. Nothing on that path is text a model wrote, so nothing on it needs a waiver. `diagnostics.py` already documented this split, with `terminal_issue_url` as the bounded variant for paths that DO get relayed through prose. - Repoint the parity guard from pinning a carve-out to pinning its ABSENCE on both halves, reading each enforcing source at run time. The backend sweep runs over the whole `security/` package rather than one module, because a waiver reintroduced in any of the nine modules or in the facade would be just as live -- the single-file path this replaces was left pointing at a deleted file by #9183, which is exactly the failure mode. The length comparison is asserted at its own site on each side, so a waiver spelled with fresh identifiers still fails, and the structured seam is asserted present because deleting the waiver is only free while that seam exists. Red-first against the pre-fix head: a prefill link to this project's own tracker with an encoded payload rendered as a live link. The withdrawn allow-list held GitHub's classic prefill keys, while the trusted builder uses issue-FORM keys, which that list never contained -- so the waiver only ever served links the model typed, the exfiltration surface and nothing else. Refs, not Closes: #7820 asks for a narrower heuristic and also reports `monitorportal.amazon.com`. Both are still redacted in chat, and a test pins them as getting the same verdict. Narrowing aggregate query length is a per-host security-ceiling decision on its own merits, and a per-shape escape hatch is not a substitute for it. Refs #7820 feat(connections): default-on launch with explicit card set (#9149) The Connections gallery was merged on main but held behind an opt-in `connections_ui: true`, so a shipped build reached it only after a hand-edit of the instance's config.json. Flip that default: the gallery is now on for every install that never touched the flag, while `connections_ui: false` remains the escape hatch that empties the Services panel and puts chat back as the only authorize prompt. The key discovery is that holding GitHub back cost NO new mechanism. A per-provider launch gate already exists on both sides of the seam -- `launch_gate_passed` in src/kiro_crew/connections/registry.json, filtered by `get_visible_providers()` in registry.py and by `CONNECTION_PROVIDERS` in website/src/pages/connections/registry.ts -- and GitHub is already `launch_gate_passed: false` there because its OAuth-app registration under the kirodotdev org is outstanding. So the launch set is the six providers those filters already produce (Notion, Linear, Atlassian, Stripe, Vercel, GitLab), and the flip adds zero production lines for per-provider gating. `test_only_gated_launch_services_are_visible` already pins GitHub's exclusion on the backend; this change adds the matching assertion on the frontend's exported list. The predicate keeps the old "never guess from a sloppy value" rule and points it at the safe direction, which is now off. An absent key is ON, an exact `true` is ON, and everything else present -- including a hand-edited `"false"` -- is OFF, so an opt-out cannot silently fail to take. A config that has not been read yet (undefined, and a failed fetch) stays OFF: the opt-out lives in that config, so opening before reading it would flash a gallery the user disabled and fire its status queries on their behalf. That preserves today's loading behavior exactly rather than introducing a new transient. KNOWN DEPENDENCY, not fixed here: on a live gateway the flag never reaches the browser at all, because GET /api/config/kirocrew strips unknown top-level keys (`_masked_config_dict` pops `_extra_sections`). The default-on half of this change works regardless -- it is the absent-key path. The `connections_ui: false` escape hatch only becomes functional once fix/connections-ui-flag-roundtrip makes the key schema-known. Landing this before that fix therefore ships a gallery with no working opt-out, which is why the flip is merge-gated on a human re-test. Prose that asserted the feature was unreleased is updated in the same pass so the code no longer contradicts itself: the hook docstring, CapabilitiesPage's flag docstring, ConnectionsPage's `servicesEnabled` docstring and its empty-list comment, and McpTab's mint-engine comment. ConnectionsPage's parameter default stays `false` so a caller that omits the prop cannot open a gallery by omission. Tests: ConnectionsUiGate.test.ts is rewritten for the new default (absent key opens, explicit false closes, unread config closes, sloppy values close) and gains a launch-set block asserting GitHub is absent and every offered provider passed its gate. McpTab.test.tsx re-points its baseline mock at the shipped default, converts the chat-prose fallback case to an explicit `connections_ui: false`, and adds a case proving the in-place sign-in reaches a managed row with no flag key at all. No user-facing strings were added, so no i18n catalog changes. test(issue-radar): pin auto-nudge crew turns resolve their own identity (#9071) An Issue Radar crew driven by an auto-nudge cycle reaches its ledger tools through the same _run_chat entry a human turn uses, which resolves identity as effective_session_key(slot) and hands it to the one shared publish_turn_identity writer. So a nudge turn and a direct turn on the same slot resolve the SAME session identity, and the strict gate lets the crew through -- the property #5905 assumed was broken. This is the positive twin of the existing subagent refusal test: a subagent resolves to its PARENT slot and is refused; an auto-nudge crew is the same principal re-entered on a timer and is admitted. Nothing pinned that the nudge fire path keeps routing through the shared writer, so a future refactor of _fire_dashboard_nudge could silently reintroduce the confusing refusal. These regression tests assert the property (same identity, one shared writer), not the call ordering. Also prunes the file from the black shrinking-baseline now that it is black-clean. fix(portability): keep the export archive inside the crew directory (#9070) * fix(portability): keep the export archive inside the crew directory `create_export_zip` walks `workspace`, `plan_memory` and `skills` with `rglob("*")` and skips an entry that `is_symlink()`, so that an export never carries bytes from outside the crew directory. That skip is blind to the escape that costs the most: `rglob` DESCENDS a directory link, and the file on the far side is an ordinary one — `is_symlink()` false — so it is written straight into the archive. On Windows the link is typically a junction, which `is_symlink` does not report at all, and a junction needs none of the privilege a directory symlink needs there. The two filters below the skip do not catch it either. `_is_excluded` and `is_sensitive_path` both read the LEXICAL path, which runs through the link's own name and therefore looks like ordinary crew content — so a sensitive file behind a link passes the sensitivity check that exists to stop exactly this. Measured on unpatched main with a real junction: `workspace/memory/linked/ not-ours.md` appeared in the export's namelist, sourced from outside the crew directory. The consequence class is content the user then hands to someone else, since an export archive is made to be moved off the host. Fixed by requiring the RESOLVED path to stay under the resolved crew directory — the repo's existing idiom (`apps/backend.py:651`). Resolving is what covers both shapes at once: it follows every reparse point on the way down and answers where the bytes actually live. `mc` is resolved once so a legitimately linked `$KIROCREW_HOME` does not reject the whole export. The `is_symlink()` skip is kept as-is; this adds a layer rather than replacing one. The import side of this module is untouched. `test_export_skips_symlinks` cannot cover this — it plants a link that IS the entry, and it `pytest.skip`s where symlinks cannot be created, i.e. on the platform where the junction spelling lives. The new tests use `conftest.make_dir_link` and need no privilege. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(portability): archive the descriptor that was validated `create_export_zip` walked `workspace/`, `plan_memory/` and `skills/` with `rglob("*")`, which DESCENDS a directory link while the file on the far side is an ordinary one, so the walk's own `is_symlink()` skip never fired for it. Neither by-name filter below that skip is a containment test either: `_is_excluded` is a rule about the archive name, and `is_sensitive_path` asks whether a path is a PROTECTED location -- it does resolve links, so a linked `~/.aws` was caught -- never whether it is inside the crew directory. An ordinary file of the user's, reached through a link dropped in the workspace, was sensitive to none of them and went into an archive the user hands on. A resolve-and-compare would answer that for an instant only: `ZipFile.write` takes a NAME and opens it again, so the file that was checked and the file that is read are two separate lookups, and a running gateway gives agent tools write access to that workspace while an export can be triggered. `_open_verified` inverts the order -- open first, then ask the kernel where the open thing is via `pinned_fs.fd_real_path` -- and `_add_from_fd` streams the entry from that descriptor, so check and use address one object. A hardlink alias, which no path-based guard can see, is refused on the descriptor's link count; `force_zip64=True` keeps a source over `ZIP64_LIMIT` archiving as `ZipFile.write` used to. That settles what may be ARCHIVED. It does not settle what may be TOUCHED, and on Windows those are different questions: resolving a path whose component is a link to a UNC share IS an outbound SMB authentication, so a refusal computed afterwards has already paid the cost it exists to prevent. No pathname check fixes it -- every by-name check is a check-to-open window and an adversary that can plant the link chooses when. The names have to be held rather than inspected. The enumeration was the last place that still inspected them. Pinning the candidate's ancestors closed the swap race, but `rglob` had already descended a PRE-PLANTED junction, and `is_file()` and `is_sensitive_path()` had already resolved what it yielded, before any of it ran. Measured on this module, exporting a workspace holding one planted junction: 14 resolving calls went out through it -- `os.path.realpath` and `ntpath._getfinalpathname` -- while the export correctly archived nothing from behind it. Nothing being archived was never the question. So the walk descends and verifies in one motion instead. `_walk_pinned` pins a directory with `platform_compat.pin_directory` before listing it -- the handle omits `FILE_SHARE_DELETE`, so a held directory can be neither renamed nor deleted, nor can anything above it, and the open refuses to follow a reparse point, so a junction at the name fails there instead of being traversed -- then classifies each child from the listing Windows already returned, where the attributes and reparse tag arrive inline and cost no lookup THROUGH the child. A child directory is descended only by pinning it in turn, so the path the kernel walks to reach any component runs entirely through components already opened and verified, starting at the crew root itself. This is the pattern `aws_control/backend/storage.py` already uses to hold the path a sandboxed CLI writes through. The leaf goes through the new `platform_compat.open_file_no_reparse`, which opens a reparse point AS ITSELF rather than following it, so the refusal and the open are one operation; POSIX keeps the `O_NOFOLLOW` open it already had, named. `_pin_ancestors` and `_open_inside` are gone rather than kept alongside this: the walk holds the same chain, for the whole subtree instead of re-walking it per file, so keeping them would be two mechanisms doing one job. The walk starts at the RESOLVED root, since `$KIROCREW_HOME` may itself be a link and `pin_directory` refuses a reparse point at a name -- pinning the configured spelling would return an empty export on such a host. The exported set is unchanged: on a tree covering nesting, `EXCLUDE_DIRS`, `EXPORT_EXCLUDE`, `.pid` and `skills/auto`, the archive is byte-identical to the one the previous walk produced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> feat(knowledge): scope local_knowledge_search to a namespace (#9032) The knowledge store already models a namespace label on items (with an index), and the dashboard already lists, assigns and browse-filters by it. Search was the one verb the label never reached: HybridRetriever and local_knowledge_search scoped only by source_id. Add an optional namespace filter to the search seed legs (keyword + vector), mirroring the existing source_id scoping. namespace is a plain items.namespace column match -- a relevance/organisation filter, not a security boundary -- and composes with source_id. The graph leg stays unfiltered, as it is for source_id. Refs #8266 Co-authored-by: gh-autofix#2887 <chenmingwei23@users.noreply.github.com> fix: persist the deferred-note hold with the slot so it survives a restart (#4093) (#8982) POST /api/chat/slots/{slot}/note replies 200 with visibleDeferred: true for a note held during a running turn, but both halves of the hold lived only in _ChatSlot._deferred_notes — a gateway restart between the 200 and the next turn silently voided the delivery promise. The hold now persists through the slot's own metadata line under one invariant: retirement is ROW-DERIVED. Every note carries an id; the flush stamps each delivered inject row with it (meta.noteId) and records rebind-dropped ids on the slot; the full save — reading the on-disk and live holds UNDER the history lock — retires exactly the entries whose rows are in the window it writes (or whose ids were dropped) and keeps everything else. Row and retirement land in one atomic file replace; the drop records are consumed only AFTER that write commits, since a dropped note's row never exists and the record is its only retirement path. - enqueue (durable-before-200): asyncio.to_thread + update_metadata_if, everything read inside the lock-time guard; the write MERGES by note id (union_deferred_notes), pins the posted note (ensure=) so a racing turn-end flush cannot yield a 200 with no durable copy, and pins the TARGET to the history key authorized at enqueue, re-verified under the store lock — a cron/workflow rebind in the persist window is refused (uniform not-found shape) instead of writing app content into a foreign transcript's metadata. The union never evicts: at the 2x ceiling the NEW note is refused (429). On EVERY failure branch, a rollback that finds the note already drained means a flush DELIVERED it — the 200 stands, because any error would make the caller re-post a line the user already saw. Other failures roll back BY IDENTITY and answer a retryable 503 (including an UNREADABLE record) - deferred notes are bounded at the ENQUEUE boundary (413 over 4000 chars); the durable copy is persisted VERBATIM, never truncated - both restore paths replay the hold up to the durable CEILING (2x the live cap — every durable entry is a 200-acknowledged note); restored notes are sanitized fail-closed (no session stamp -> dropped; over-bound content -> dropped; malformed context half dropped alone) - the flush never writes metadata; a crash between flush and save re-delivers on restore (at-least-once) - docs updated in the same commit (App Kit api-reference, session.md, endpoint docstring): do not re-post after a restart; 503 = retry, 413/429 = boundary refusals, 404 = ownership/rebind refusal _pending_context (the /context queue) stays memory-only: only the context halves embedded in held notes ride the same metadata shape naturally. Closes #4093 Co-authored-by: Nick Bowers <nrb@amazon.com> feat: speak replies with the host's own engine, no install needed (#8957) Text-to-speech needed an install before it could say anything: Piper wants a binary and a voice model on disk, Polly wants AWS credentials, so turning on auto-speak on a fresh machine produced silence and a settings panel with two paths that both start with homework. Add a third provider, `system`, and make it the default. It drives the speech engine the operating system already ships -- `say` on macOS, System.Speech through Windows PowerShell 5.1, `espeak-ng` on Linux when present -- so auto-speak works with nothing downloaded and nothing configured. Linux is the one platform where the engine can be absent; that is reported as unavailable with the package to install rather than failing silently at synthesis time. The engine is resolved through `trusted_system_bin`, not PATH, so a shim in an agent-writable directory cannot be handed model output, and the spoken text never reaches argv: `say` and `espeak-ng` read it on stdin, and the Windows path writes it to a temp file that a constant base64 -EncodedCommand script reads, so no quoting decision is ever made about model output. This provider deliberately does NOT route through `wrap_argv`. Windows has no sandbox backend, so a wrap fails closed on the one platform whose built-in engine users depend on, and there is nothing to confine in an OS component resolved from a directory the user cannot write. Piper keeps its wrap: it runs a downloaded model through a third-party binary. A test pins the distinction. Also fixes two things the new provider surfaced: `_resolve_piper_binary` looked for `~/piper-venv/bin/piper` on every platform, so a correct Windows install was never found (`Scripts\piper.exe`), and the module documented Piper as having no Windows build, which the `piper-tts` wheel has published since 1.3.0. fix(config): hold the sidecar advisory lock in KiroCrewConfig.save() and offload async callers (#4767) (#8906) KiroCrewConfig.save() was an unlocked whole-document replace: its rename could land inside an update_config_locked holder's read-modify-write (CLI writer, boot refresh, second gateway), silently discarding the other writer's change -- every field in the document exposed. Two halves, landing together per the recorded failure modes of the first attempt (#4371): HALF 1 -- lock the writer. The sidecar acquire in update_config_locked is extracted into _config_write_lock() and save() now writes under it: the SAME <config>.lock sidecar, beside the resolved symlink target, so both writer families contend on one lock and the lifecycle stays one shared file (no new residue -- the Windows orphan concern from #4371). The write itself stays atomic + mode-preserving via write_config_atomically, and the overlay-subtraction content is untouched. save()'s docstring now states the staleness contract plainly: it publishes an in-memory snapshot, so it is ONLY for single-flow callers with no suspension between load and save (CLI one-shots, the boot default-config write); read-modify-write flows belong on update_config_locked. HALF 2 -- dashboard writers become DELTA read-modify-writes, off-loop. Every .save() call site was enumerated and classified. Coroutine callers no longer call save() at all: each persists the keys it owns as a delta mutate through update_config_locked -- read, re-check, and write inside ONE flock hold -- dispatched off the loop via run_config_write (the transaction shape its docstring prescribes) or, where the asyncio config lock is already held, via the module's cancellation-draining _drained_to_thread: - dashboard/handlers/updates.py api_log_level: delta on agent.log_level - dashboard/handlers/files.py workspace create/update/delete: delta on the one workspaces entry, with the state-dependent preconditions (name/dir collision, default-workspace and agent references) re-run against the document as read inside the lock and surfaced as 4xx conflicts. The copy_from tree is staged only after ALL path validation passes, installed inside the locked mutate (atomic os.replace, or a merge copy when the destination pre-exists), and every cleanup of a staged tree runs in a worker, never inline on the loop - dashboard/handlers/agents.py agents-sync + agent-create: delta on the agents entries (adds re-checked, package/aim prunes re-derived, name conflicts re-checked against the in-lock document), via _drained_to_thread under the held asyncio lock - dashboard/handlers/onboarding_import.py: both former manual-lock sites route through run_config_write; the state PUT is a delta on dashboard.import_onboarded; the unused local _get_config_lock wrapper removed - cli_server.py gateway-boot default-config write: asyncio.to_thread (file absent, nothing to lose, no loop-side writers yet) Sync CLI/wizard callers (cli_commands, cli_config, cli_cloud, cloud/wizard) stay on save(): single-flow, no suspension between load and save. crew_chat.py st.save()/self.save() are CrewStore, and mochi stats_service self.save() is its own persistence -- not KiroCrewConfig. Tests: red-before-green concurrency pair (save waits on a held sidecar; a locked writer landing inside save's critical window survives), sidecar lifecycle (only the one shared .lock beside config.json; symlinked config locks beside the target), in-lock precondition re-check suite for the workspace handlers (stale snapshot passes, fresh on-disk document refuses, unrelated entries survive the delta), staging-lifecycle tests (no destination mutation or staged residue on conflict or failed validation), a runtime off-loop probe on the log-level handler, and a self-tested AST ratchet forbidding any coroutine from calling KiroCrewConfig.save() inline. Closes #4767 Co-authored-by: Nick Bowers <nrb@amazon.com> fix(members): dispatch server carries the gateway home override (#8611) The crew member DM session's kirocrew-dashboard MCP entry carried only the session key (and, since #8837, the bound port). The server resolves *which gateway* to call from its data home, so on any install where KIROCREW_HOME is set -- a pod, a second profile -- it presented the member's identity to the default home's gateway, which has no such member slot and refused every verb as caller_unidentified while tools/list looked healthy. Carry the same home override every managed Crew server already carries (_managed_mcp_env), through the same helper so the two cannot drift. On a default install the helper returns {} and the emitted entry is unchanged. The per-member conversation index (#8612) no longer rides in this PR; it travels with its consumer, #8613, which is stacked on this branch. feat(members): open the last member on arrival, mirror it in the URL (#8546) The Crew Members page landed on an empty column every visit and forgot which member was open when the user navigated away and back. Now: - A visit that names no member opens the remembered one, else the first row in display order — never the empty column. - The last member opened is remembered per browser (mc-members-last-member, via safeStorage), so leaving for Sessions/Settings and returning, or a reload, lands on the same conversation. A remembered member that was deleted or renamed falls back to the first row without an error. - The open member rides the URL (?member=<name>): a reload keeps it and a link lands on one. Switching members REPLACES the entry — the page holds one history entry however many members are visited, so Back leaves it in one press, the Sessions sidebar's rule; only the below-md roster->thread step pushes. The value is the exact crew name, not the slug — the slug is lossy and would misroute Oncall to oncall. A link naming a member that is gone takes the same fallback and says so in a warn-toned status above the thread, leading with the swap ("Showing X — “Y” is no longer on the roster."; 13 catalogs + en-XA): the user asked for someone specific, and a silently mounted other thread is the misroute this page exists to prevent. The stand-in opened by that fallback is the page's choice, not the user's, so it does NOT become the memory — one dead link cannot overwrite the member the user had chosen. - Below md the page stays a two-level list->detail navigation: no ?member= IS the roster, so no auto-open there (same rule as SidePanelLayout's remembered tab). A stale ?member= returns to the roster with the same notice above the list. The header back pops the entry the roster pushed (no duplicate roster entry in history) and drops the param in place for a deep link, which has no roster behind it. Tests cover default, restore, stale fallback (memory: silent; URL: with notice on both sides of md, retired on the next open, memory preserved), URL-over-memory, click writes URL+memory, a driven-history walk over two members that leaves the page in one Back, and the below-md back semantics (the useNavigate mock records AND performs against the MemoryRouter). fix(pod): scope pod kiro-cli oauth grants to the pod home (#8528) A pod isolates KIROCREW_HOME and KIRO_HOME but deliberately keeps the real HOME, and kiro-cli derives its MCP OAuth artifact directory from the spawned process's own $HOME. So a pod REUSED the operator's machine-level grants (a provider card read Connected from a grant minted on the real machine) and a grant minted INSIDE a pod outlived pod down as a real, durable credential. Both break test fidelity and the destroy-grants-after-each-smoke-test requirement. Two halves, one shared resolver: - mcp_grant.kiro_oauth_cache_dir() -- the single default all four callers (mint, status, disconnect, mcp_discovery's remote probe) reach with no explicit cache_dir -- now resolves through the new config.paths.kiro_oauth_cache_home(), which honours KIROCREW_OS_HOME and rejects the same unsafe targets as kiro_home()'s KIRO_HOME. This moves the pod gateway's own grant reads onto the pod's tree. - acp.client._apply_pod_home_remap(), applied identically at both spawn transports, remaps a pod-spawned kiro-cli child's HOME/USERPROFILE to that same directory. kiro-cli offers no env override for just its OAuth cache, so remapping the child's HOME is the only way to move its own WRITES. Gated on KIROCREW_POD=1 plus positive membership in ACP_BACKENDS_POD_HOME_REMAP -- its own set, not a reuse of the internal-sandbox one, because "carries its own OS sandbox" and "relocating HOME moves its credential store" are different questions (harness-parity H6). Sign-in still works: pod.runtime._seed_pod_os_home create-only stages the AGENT RUNTIME's own identity store -- the per-platform paths derived from identity_stores.store_mappings(), which is where the harness actually resolves its access token -- into the pod tree at boot. No host .aws/sso/cache contents are copied: an earlier revision staged those and it was deleted, so a pod's .aws/sso/cache starts EMPTY and holds only grants that pod itself mints. A pod therefore cannot be pre-authorized to a provider nobody consented to inside it, by construction rather than by a filename glob. A boot viability probe (agent_sdk.pod_child_probe) is the last gate before serving: it spawns the child through the same confinement production uses (env scrub, sandbox wrap, cgroup scope, rlimit preexec) and routes an unbootable child through _refuse, so a signed-out or dead child makes pod up fail loudly with a terminal exit code instead of serving a broken pod. AWS_CONFIG_FILE / AWS_SHARED_CREDENTIALS_FILE are deliberately NOT exported into the pod child. An earlier revision pinned both at the real home so a pod turn could still reach the operator's profiles; that made each name an alias for a path the sensitive-path keystone fences, and three review rounds each closed one spelling of the retrieval. The alias was deleted at its source instead -- security.py records the full reasoning, including why a name-based deny rule is defensible for the variables that hold a SECRET and not for the ones that hold a PATH. The real passwd home stays fenced: security.py's matchers run in the gateway process against its own Path.home(), and the remap only ever mutates the dict handed to the child spawn, never os.environ. The os-home tree nests under the pod home, so cleanup_home's existing pod down sweep reclaims a pod's grants with everything else. feat(chat): add inline paste token composer (#8310) Add a default-off Lexical composer path with atomic inline paste nodes, editor-neutral selection and focus controls, shared clipboard policy, lazy-load rollback, and focused regression coverage while preserving the textarea fallback. fix(slack): report the real dashboard link-click window, not the constant (#7265) `generate_token` mints `exp = now + min(LINK_WINDOW_SECS, session_ttl)` so a link can never authenticate past the session it grants. Both Slack surfaces that report that window still printed the unclamped constant, so `/kirocrew dashboard 1m` promised "Click within 5m" about a link that dies in one — a 5x overstatement of a window the user has to act inside. Report the clamped value at both reporters: the DM built in `allowlist.send_dashboard_link` and the ephemeral block built in `events._handle_dashboard`. For a full-length caller both strings are byte-identical to before, because the clamp only ever tightens. Original change authored by Leon (@leonlaiyc). Co-authored-by: Leon <276034861+leonlaiyc@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Kiro Crew <noreply@kirodotdev.github.io> feat(cron): allow assigning a Schedule-page folder when creating a cron (#7249) cron_add/cron_update (MCP) accept a 'folder' argument (name or id; a missing name is created through POST /api/cron-folders, the dashboard's own endpoint, so the create shares the Schedule page's lock and in-memory list). 'kirocrew cron add' gains --folder (existing folders only). App manifests can set 'folder' on a cron entry (a NAME, resolved at registration; unresolved degrades to ungrouped with a warning and is re-applied on the next enable, so the assignment survives the disable/enable delete-and-recreate cycle). Shared read-only resolver in cron.py (load_cron_folders / lookup_cron_folder_id): cron_folders.json stays dashboard-owned; no non-dashboard surface writes it directly. Co-authored-by: Aurélien Andrey <aurand@amazon.fr> feat(session): recover warm-pool hits for effort-override sessions (#7096) Apply reasoning effort overrides post-claim on warm-pool processes via provider.change_effort, keeping the warm-pool hit path active for slots carrying reasoning effort overrides. Fixes #6248 feat(meetings): import a recording into a meeting (#5741) POST …/{id}/import takes a host path to an audio file, transcribes it with the gateway's own batch speech-to-text, and dispatches each line through the same admission transaction live speech uses, so an imported recording is persisted to the transcript and reaches the agents exactly as if spoken. Every dispatched line requires the SESSION OBJECT admitted at import start — a meeting id is a name, not an identity — so a meeting stopped, deleted, and recreated under the same id mid-import gets a 410 (meeting_session_replaced) instead of the old recording's lines. A recording that would split into more than MAX_IMPORT_LINES lines is refused whole with 413 (recording_too_long) rather than silently truncated: a capped import that returns success while the recording's tail is missing is data loss the user cannot see. Original feature authored by Kai Mitsuzawa (kaizawa97). Rebase onto main, locale conflict resolution, the session-identity fix, and the overflow rejection by Kiro Crew. Co-authored-by: Kiro Crew <noreply@kirodotdev.github.io> fix(issue-radar): open lock sidecars non-truncating before the acquire (#9275) The 18 lock context managers in issue_radar's store.py and crew_store.py opened the lock sidecar with open(lock_path, "w") and only then handed the descriptor to platform_compat.file_lock(..., exclusive=True). "w" truncates at open, before the lock is held. On Windows the acquire routes to msvcrt.locking, and a truncating open of a lock file whose first byte another holder already locked raises a sharing violation instead of waiting, so the contending acquirer fails before it reaches the lock and the mutual exclusion silently does not happen. POSIX flock tolerates it, which is why it is invisible on Linux. Each site now ensures the parent dir, lock_path.touch(exist_ok=True), then open(lock_path, "r+") -- writable (msvcrt.locking needs it) but not truncating -- acquiring on that handle exactly as before. Same shape and rationale as work_ledger._open_lock (#9237) and session_pid.py (#9250). Refs #9248 fix(apps): read the answer to the stop the Job SDK already performs (#9262) A runner that submits to a pool and returns the unwaited concurrent.futures.Future hands the work back, so the SDK holds the only reference to it for one instant. _close_quietly already asked that reference to cancel -- as hygiene -- and discarded the answer, while _undriven_result had already worded the record as work that 'may still be running ... and cannot stop'. So a run whose work provably never started was recorded as work a retry could overlap. _stop_returned_work now makes that stop deliberate, takes it before the verdict is worded, and answers three ways: True only for a concurrent.futures.Future whose cancel() returned True, which that class documents as the call never running; False when the work is already executing; None when no guarantee is available. An asyncio.Task can answer cancel() with True and finish anyway, so a truthy cancel is never read as a stop. Refs #7814 Co-authored-by: Kiro Crew <noreply@kiro.dev> fix(hooks): stop the hooks.json lock opens from truncating the lock file (#9279) The two writers of hooks.json — webhooks.locked() and the register_hook MCP tool (mcp_tools/control.py) — both opened their shared lock sidecar with open(lock_path, "w") and only then acquired the lock. "w" truncates AT OPEN, so the mutation happens before the lock that is supposed to guard it is held. On Windows the acquire routes to msvcrt.locking, and a truncating open of a lock file whose first byte another holder already locked raises a sharing violation instead of waiting — the contending acquirer fails before it ever reaches the lock, and the mutual exclusion the lock exists to provide silently does not happen. POSIX flock tolerates the truncate, which is why the defect is invisible on Linux. These two sites are ONE unit: both resolve to the SAME <data-home>/hooks.json.lock from two different modules, which is exactly the cross-process contention the defect needs — webhooks.locked() is documented public precisely because register_hook must share its lock discipline. Fixing one without the other would leave the pair half-locked and the Windows failure fully live. Both sites now touch(exist_ok=True) and open "r+" — writable, which msvcrt.locking requires, but not truncating. Same shape as work_ledger._open_lock (PR #9237), session_pid.py (PR #9250), and dashboard/handlers/mcp.py's _McpFileLock, which was written this way from the start. Truncation is the direct, platform-independent observable, so the tests seed the lock file, take and release the lock, and require the bytes to have survived: they fail on every platform if a truncating open comes back. Part of the #9248 sweep (one subsystem per PR, per the issue); the remaining subsystems get their own follow-up issues. Co-authored-by: dwu96 <dwu96@amazon.com> Co-authored-by: Kiro Crew <noreply@kiro.dev> revert: restore windows startup by reverting #8117 This reverts commit 259db3970e91f4bf6842456b713ad031eed4902f. Restore the Windows Kiro delegation contract: the macOS settings probe must not prevent an explicitly classified official Kiro backend from starting on a fresh Windows install. Kiro CLI remains a trusted runtime dependency; its internal sandbox is outside this Crew-side validation. Revert the whole PR for immediate recovery, including its UI-stream deadline and debug-only precise-memory-info switch. Remove the obsolete stalled-stream test together with its subsequent determinism changes. Validation on this branch: 140 backend tests passed, 5 platform skips; 23 Electron native-logging tests passed. Changed Python files pass isort and flake8; the staged diff passes the whitespace check. revert: put the AI review lanes back on Opus 4.8 and Fable 5 This reverts commit ecf50cadd4336f40d2166c835bc780118372ec57 (#9158). Fable 5.1 is listed ACTIVE on Bedrock in this account and its us. inference profile exists, but it is not actually invokable from these lanes: every first call is rejected at zero tokens. The three advisory lanes hid that behind --fallback-model, so they reported success while modelUsage named us.anthropic.claude-opus-4-8, and the GPT lanes' adjudication pass carries no fallback, so it failed outright and turned the required GPT 5.6 Review check red on four pull requests inside sixteen minutes. Listing a model as available was not evidence that the role may invoke it. Re-landing the bump needs a real InvokeModel check first, and the advisory lanes need it too: their green was the fallback, not Fable 5.1. The two corrected labels for the Opus-4.8-running claude-review.yml lane go back to saying Fable 5 with this revert. That is pre-existing wording, wrong but harmless, and it returns with the retry rather than holding up an urgent revert. fix(instances): journal every warm-set write and the handshake drops behind a stuck pane (#9141) A remote crew pane can sit on "loading pane" with nothing in gateway-launch.log to explain it. The pane journal added in #8274/#8813 covers the viewport's own warm paths, but the one warm-writer it does not own -- connectInstanceInto, shared by tab clicks and the auto-connect fan-out -- was silent, and three state changes that put a pane into that bucket left no line at all. connectInstanceInto now journals `warm` / `warm-declined` / `warm-failed` tagged `via=select|auto-connect`, in the same shape the viewport uses. A connect that answers `connected` with no port or no token leaves the OLD warm entry standing (a dead port), so the tab still renders an iframe and the user sees only "loading"; that is now the one `warm-declined` line it was missing. InstancesViewport journals three more moments: - `evict`: the K-cap LRU eviction, the only teardown the user never asked for. The next click re-warms the pane as a brand-new load, so an evicted pane that then fails reads, without this line, as a pane that was fine until it suddenly was not. - `watchdog-armed`: the 15s load watchdog starting. A pane that spins forever WITHOUT ever producing `load-timeout` is a pane whose watchdog never armed, and only the absence of this line says so. - `ready-unattributed`: an `mc-embedded-ready` from a loopback origin the parent cannot map to a warm pane -- the handshake dropped because the origin map has not caught up with a port change or an eviction. Journaled only for this message type and only for a loopback origin; the child sends it at most six times per load, so it cannot flood. No token material reaches the log: `paneLog` redacts secret-named fields and the new test pins that the token string never appears. Co-authored-by: Joe Guo <zejiangg@amazon.com> fix(slack): stop a refused progress card from splitting the reply (#9260) A task card is progress decoration and the 30s elapsed-time refresh re-sends it for as long as a tool runs, so on a several-minute tool phase it is the only thing appending to the stream. Both Slack pipelines rotated the stream when Slack refused that append: the message the reader was watching was stopped mid-answer and the rest of the reply continued in a new one, which reads as a reply that failed followed minutes later by an unexplained second reply. _append_task now skips the refused card and keeps the stream. _append_stream still rotates when real text is refused, which is the moment a rotation is worth its price, and that replacement stream now opens with a continuation marker so the two messages read as one answer. test(work-ledger): name the cause when the binding race test shortfalls (#9257) test_two_conductors_binding_one_worker_at_once_yield_exactly_one_binding fails on Backend Tests (Windows) (4) intermittently and reports only "assert 2 == 3". Its bind helper caught only WorkLedgerError, so anything else (a bare OSError from a Windows sharing violation, per #9250's own docstring) killed the thread silently, appended nothing, and shortened the count -- discarding the one fact that identifies the cause. Record one outcome per thread keyed by conductor, catch BaseException and keep the type + message, and flag a thread still alive after join(timeout). Put the per-thread outcomes in both assertion messages so the next Windows failure names its exception instead of a count. The == 1 and == 3 assertions are unchanged: this adds diagnosis, not tolerance. fix(session-pid): stop the PID lock opens from truncating the lock file (#9250) The three PID lock helpers opened their lock file with `open(path, "w")` and only then acquired the lock. `"w"` truncates AT OPEN, so the truncation happens before the lock is held. On Windows the acquire routes to `msvcrt.locking`, and a truncating open of a lock file whose first byte another holder already locked raises a sharing violation instead of waiting -- so the contending acquirer crashes with a bare OSError before it ever reaches `file_lock`, and the serialisation the lock exists to provide never happens. POSIX `flock` tolerates the truncate, which is why this was invisible on Linux and reddened only the Windows shards. Issue #9248, which named `session_pid.py` as the site most likely to bite in practice: PID files are read by other processes precisely while they contend. It named two sites and asked whoever picked it up to re-run the grep rather than trust the list; doing so found a third, `_periodic_pid_sweep`, which is the worst of them because it runs on a timer while `_track_session_pid` contends for the same lock. All three now `touch(exist_ok=True)` and open `"r+"` -- writable, which `msvcrt.locking` requires, but not truncating. Same shape as `dashboard/handlers/mcp.py`'s `_McpFileLock`, which was already written this way, and as `work_ledger._open_lock`. Truncation is the direct, platform-independent observable, so the tests seed the lock file, take and release the lock, and require the bytes to have survived: they fail on every platform if the truncating open comes back, rather than only on Windows. `webhooks.py` carries the same pattern and is deliberately left alone -- issue #9248 asks for one change per subsystem so each gets its own review. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> fix(work-ledger): stop the lock-file open from truncating on Windows (#9237) The store's _open_lock opened its lock file with open(path, "w"), which truncates on open. On Windows a truncating open of a lock file whose first byte another holder already locked under msvcrt.locking raises a sharing violation instead of waiting, so a second contending acquirer crashes with a bare OSError before it reaches file_lock -- the bind it was serialising is never mutually excluded. POSIX flock tolerates the truncate, so the defect was Windows-only. Fix mirrors dashboard/handlers/mcp.py: touch + open r+ (writable, no truncate). fix(acp): give session/set_mode the session-start budget on Windows (#9225) session/set_mode boots the switched-to agent's MCP servers, the same (re-)initialization that gives session/new and session/load their 90s budget, but both set_mode call sites used the generic 30s _REQUEST_TIMEOUT. A switched-to server pending OAuth holds the response for its full 30s wait, so the 30s budget races it exactly as it would session start. Pass the already-resolved session-start budget to both set_mode calls. Closes #9185 feat: add contributes.panelTabs app-manifest contribution (#7975) test(acp): re-record frame-replay snapshots for the wire_title field (#9232) The replay corpus recorded by #9161 predates the wire_title field #9073 added to every tool_call event 3 minutes later, so main's own corpus no longer matches main's own render. Regenerated with scripts/update_acp_frame_snapshots.py; the only delta is the new wire_title key. refactor(electron): share typed preload bridge contracts (#9228) Co-authored-by: DeryFerd <DeryFerd@users.noreply.github.com> fix(messaging): keep prose that merely mentions [STEERING visible (#9209) `_SteeringMarkerFilter` held any buffered tail that began `[STEERING` until it found a `]`, and deleted it at flush if none arrived. That is a locate-by-substring decision, not a grammar one: the filter already hands on a CLOSED frame that fails `_STEER_MARKER_RE` as ordinary text, so the same sentence survived when the writer happened to type a `]` later and vanished when they did not. Measured on main -- `"Use the [STEERING protocol to redirect"` reaches the channel as `"Use the "`, and `"text [STEERING nonsense] tail"` arrives whole. The filter runs in `TurnDriver` upstream of every renderer, so an agent explaining the steering protocol loses the rest of its message on Discord, Telegram and WhatsApp. It also negates `test_unfinished_marker_prefix_grammar.py`'s `test_prose_mentioning_steering_is_left_visible`, which pins that exact string as visible one layer down. The unterminated tail is now judged by the grammar the marker actually has: a tail that can still extend into `[STEERING steer-<id>]` is held as before, and one that has already diverged is prose and is handed on. The probe is compiled from `constants._STEERING_TAIL_PREFIX_RE`'s pattern -- the same closure `split_trailing_protocol_suffix` uses downstream -- so the grammar keeps one source, with `IGNORECASE` added to match this module's own recognizer, since here a tail judged prose is EMITTED and a probe stricter than the recognizer beside it would leak part of a real frame. Fail-closed behaviour is unchanged: a tail that was still a viable marker when the stream ended is dropped, not emitted, and the 16 KiB ceiling is checked before the grammar probe so an adversarial buffer cannot make it re-scan a growing string. Co-authored-by: Claude Opus 5 <noreply@anthropic.com> fix(command-bar): attribute contributed rows to the validated app name (#9226) * fix(command-bar): attribute contributed rows to the validated app name A contributed command's attribution came from `app.displayName`, which is free text the app chooses, so it could not do the one job it has. An app declaring `displayName: "Kiro Crew"` put that string where the row states its owner, and the row then read as native while its prompt goes to an agent with tools. In the other direction `||` is not the emptiness test it looks like: the manifest validator refuses only a falsy `displayName`, so `" "` or a zero-width `\u200b` is "present", wins over the name, and renders the empty label that `kindLabel` shows as a bare kind -- character for character what a builtin row shows. The label now comes from `name`, the install identity. `KEBAB_RE` limits it to `[a-z0-9]` plus single hyphens, so it can neither be blank nor spell a host word with a space or a capital, and it is unique per installed app. The clip path already used `name`; the unclipped path now agrees with it. * docs(command-bar): add before/after evidence for the attribution fix Captured with the repo's own scripts/capture-command-bar-contributed.mjs harness, with the external app's displayName set to "Kiro Crew" so the BEFORE frame shows the impersonation the fix removes rather than merely a friendly name. fix(autonudge): make an armed auto-nudge loop readable via monitor_inspect (#9221) fix: treat a serve config naming only other ports as a free 443 (#7999) A serve status document that keys mappings by port (TCP by bare number, Web/AllowFunnel by host:port) and names no key for 443 is positive evidence that 443 is vacant, not an unknown. Previously any unrelated serve config -- another project on port 80 -- made the Phone access card and `kirocrew tailnet up` refuse to publish with "could not identify what is on port 443", on exactly the developer workstations most likely to run other tailscale-serve projects. serve_state() now sets a port_free reading via _has_port_shaped_keys(), and publish() proceeds when the port is provably free; a shared _key_port() parser keeps the port-evidence read and the 443 detector from ever disagreeing (a leading-zero key must not read as evidence while hiding the 443 mapping it names). Documents with serve content but no port-shaped keys keep refusing; a stranger's handler at 443/ keeps refusing and the Phone access card derives `occupied` for it up front instead of offering a publish that fails one click later. unpublish() leaves other ports' config untouched. Co-authored-by: Antigravity AI <ai@gotad.net> fix(connections): honest transport-error copy on the needs-attention card (#9148) Gate the provider-verdict headline on auth-shaped evidence via errorIndicatesProviderRejection; transport failures (timeout, DNS, reset, unknown) now render connection_unreachable instead of claiming the provider invalidated the grant. Default is no-verdict. New connection_unreachable key in all 13 locale catalogs plus regenerated pseudolocale. fix(protocol): tolerate a Markdown wrapper around the [OPTIONS:] marker (#9174) A model sometimes wraps the whole trailing marker line in inline code or emphasis -- `[OPTIONS: A | B]` in backticks, or **[OPTIONS: A | B]**. The wrapper character lands after the closing bracket, which broke the end-of-line anchor: the marker was neither parsed nor stripped, the turn lost its follow-up pills, and the raw marker leaked to the user as literal code-styled text. Every surface shares the grammar, so dashboard, Slack, previews, and the Discord/Telegram trailer split all lost their buttons on the same input. The grammar already absorbs the same class of model tic (a stray `](OPTIONS)` link close), so this widens it symmetrically and tightly: - a LEADING wrapper run (` / * / _, at most 3) matches only at line start after optional indent, so emphasis belonging to preceding prose is never eaten; - a TRAILING run only when it abuts the closer (or its `(...)` tic) AND the marker is line-anchored: a mid-line marker keeps the pre-widening grammar byte for byte, so an inline code span quoting a marker (`Use [OPTIONS: A | B]` in backticks) never loses its closing character. Python spells the condition as a `(?(lwrap)...)` conditional group; JS, which has no conditionals, as a two-branch alternation with (S, labels) group pairs (1,2)/(3,4); - the Python `labels` group is named (the conditional group precedes it), and the group-reading consumers (slack, messaging renderer, dashboard) read `group("labels")` / branch pairs accordingly; - frontend regex and both backend mirrors change in one commit so the grammars cannot drift; - the partial-marker stripper and the backend trailer splitter pull a line-leading wrapper along with a still-streaming marker, so the wrapper is never stranded in the visible half; - ReDoS linearity is preserved: the wrapper class shares no character with the indent or trailing whitespace classes, each alternation branch is the proven linear shape, and both wrapper positions are anchored by the required [OPTIONS: literal. Tests in both languages cover backtick-wrapped, emphasis-wrapped, leading-only, trailing-only, the negative rows that must stay negative (trailing period, trailing prose, non-abutting wrapper, 4+ runs, prose-owned emphasis, and the mid-line wrapped/quoted forms including the inline-code corruption case), streaming-prefix behaviour, group contracts in both languages, and adversarial wrapper runs against the linearity guards. Closes #9110 Co-authored-by: Joe Guo <zejiangg@amazon.com> fix(directive): claim the out-of-band record by the call's input, not the result (#9073) The out-of-band session-directive path parked the tool's validated payload on the gateway and let the consumer claim it -- but the consumer learned WHICH record to claim by reading the directive marker back out of the tool RESULT text (`session_directive.peek`). That text is whatever the backend chooses to put on the wire, and KAS reshaped it four ways in as many weeks: the envelope re-serialised with every quote escaped (#8182), the result copied into both `response` and `message` (#8841, kiro-agent bc5906adf), one field swapped for an offload reference above a size threshold, and every string capped at 30k chars with the tail-anchored marker falling off the end. Each shape was one more repair branch in `acp/_dispatch.py`, which every backend shares, and the next kiro-agent release could add another. Conductor frames were already at 22k against the 30k cliff. The tool call's INPUT reaches both sides through no envelope at all. The MCP server receives it as the `arguments` of `tools/call`; the consumer sees it as the `rawInput` of the ACP `tool_call` frame, which kiro-agent emits uncapped. So the record is now keyed by `session_directive.call_input_digest` of the tool NAME plus those raw arguments (the name because every no-argument tool hashes `{}` alike; a planted `reset_conversation({})` was otherwise claimable by any `{}` call): * `mcp_core._call_tool` digests the raw args BEFORE validation (validation adds schema defaults the model never typed) into a ContextVar; `control._emit_directive` posts it beside the payload as `input_digest`. * `api_session_directive` requires it (400 without -- a record nothing can claim is worse than an honest refusal) and `directive_queue.publish` stores it. `directive_queue.claim` matches on `(session_key, input_digest)` inside the claiming turn; the `(kind, args)` correlation is gone. * `chat_runner` records the digest of `raw_tool_params` on every `EVENT_TOOL_CALL`, refreshes it from a `tool_call_update` carrying rawInput (claude-agent-acp streams an empty input first), and claims by it on the final result frame. The claim is attempted when the frame carries a marker OR a record is parked for the session and the call has a digest, so a result whose marker was capped off entirely still arms. `_meta` is dropped before hashing: kiro-agent strips it before `callTool`, so the server never sees the block the frame carries. Same trust shape as the marker it replaces: model-controlled content, bound to the session by arriving on its own event stream in the call the tool served. A caller who can park a record for another session still needs THAT session's model to make a call with identical arguments in the same turn. The applied payload is always the record's; the digest only picks which record. Deleted: `_repair_escaped_marker`, `_marker_bearing_text`, `_elide_marker_value`, `_dumps_elided_siblings`, `ELIDED_MARKER_VALUE`, `session_directive.peek`, `peek_failure_reason`, and the marker-envelope branches in both result builders (`_dispatch` and the legacy `AcpClient`). The kiro-cli path (`_meta.kiro` identity + `decode` from the marker) is unchanged; the messaging TurnDriver never used the queue a…

chenmingwei23 requested a review from a team as a code owner September 7, 2026 17:55

chenmingwei23 requested a review from pepmach September 7, 2026 17:55

github-actions Bot added the readiness: checking Automated validation is still running label Sep 7, 2026

github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 7, 2026

chenmingwei23 force-pushed the fix/issue-radar-lock-open-no-truncate-9263 branch from 28da0e8 to 25f6e25 Compare September 7, 2026 18:07

github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Sep 7, 2026

github-actions Bot added readiness: passed Eligible automated validation passed for the current revision and removed readiness: checking Automated validation is still running labels Sep 7, 2026

chenmingwei23 mentioned this pull request Sep 7, 2026

deploy lock sidecars are opened truncating before the acquire (5 sites, #9248 sweep) #9265

Closed

dwu96 enabled auto-merge (squash) September 7, 2026 19:04

dwu96 approved these changes Sep 7, 2026

View reviewed changes

dwu96 merged commit 81d5751 into main Sep 7, 2026
64 checks passed

dwu96 deleted the fix/issue-radar-lock-open-no-truncate-9263 branch September 7, 2026 19:05

iamwhatever approved these changes Sep 7, 2026

View reviewed changes

github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Sep 7, 2026

Navigation Menu

Uh oh!

fix(issue-radar): open lock sidecars non-truncating before the acquire - #9275

fix(issue-radar): open lock sidecars non-truncating before the acquire#9275
dwu96 merged 1 commit into
mainfrom
fix/issue-radar-lock-open-no-truncate-9263

chenmingwei23 commented Sep 7, 2026

Uh oh!

chenmingwei23 commented Sep 7, 2026

Uh oh!

github-actions Bot commented Sep 7, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 7, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 7, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 7, 2026 •

edited

Loading

Uh oh!

chenmingwei23 commented Sep 7, 2026

Uh oh!

dwu96 left a comment

Uh oh!

Uh oh!

iamwhatever left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

chenmingwei23 commented Sep 7, 2026

Problem / Motivation

Why it matters

What changed (motivation -> approach -> change)

Tests

Manual verification

Related Issues

Pattern harvest

Uh oh!

chenmingwei23 commented Sep 7, 2026

Uh oh!

github-actions Bot commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Design Review (Fable 5) — ✅ PASS

Uh oh!

github-actions Bot commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

First Principles Review (Fable 5) — ✅ PASS

What this change ships

Watch

Uh oh!

github-actions Bot commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Opus 4.8 Review — ✅ no blocking findings

Uh oh!

github-actions Bot commented Sep 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

GPT 5.6 Review — ✅ no blocking findings

Uh oh!

chenmingwei23 commented Sep 7, 2026

Uh oh!

dwu96 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

iamwhatever left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Sep 7, 2026 •

edited

Loading

github-actions Bot commented Sep 7, 2026 •

edited

Loading

github-actions Bot commented Sep 7, 2026 •

edited

Loading

github-actions Bot commented Sep 7, 2026 •

edited

Loading