docs(locks): correct the kept-inline lock-open comments (#9267) - #9661
Merged
Conversation
Two comment-only fixes to the sites #9647 left inline: - session_pid._periodic_pid_sweep: delete the false clause claiming the with-scoped helper "serves exclusive acquisition only". open_lock_file only OPENS non-truncating; the lock mode is chosen at file_lock(fd, exclusive=...), and this sweep acquires shared read twelve lines below. The real reason it stays inline is fd lifetime -- the fd is held across the try/finally, not a `with` block -- which the original clause already stated; keep that, drop the added falsehood. - _McpFileLockSync.__enter__: shrink to a pointer at _McpFileLock above, whose kept-inline rationale already states the fd-lifetime reason fifty lines up. Repeating it is the duplication this item exists to reduce. No code change; the migrations #9647 landed are untouched. Refs #9267
github-actions
Bot
added
the
readiness: checking
Contributor
Opus 4.8 Review — ✅ no blocking findingsReviewed Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
Contributor
Design Review (Fable 5) — ✅ PASSDesign-level review of Both edits check out against the code: Design-Verdict: PASS Deletes a verifiably false claim about [DESIGN-REVIEWED] ba42c53 |
Contributor
First Principles Review (Fable 5) — ✅ PASSPremise-level review of Both claims verified against the repo: First-Principles-Verdict: PASS Verify the surviving claim too: the sweep fd never escapes What this change shipsInventory (2 items) — 2 justifiedIntent: delete a false clause and a duplicated rationale from two lock-open comments before they mislead a future migration — a FIX.
Both deletions have checkable provenance in the code itself: the deleted clause is contradicted twelve lines below it ( [FIRST-PRINCIPLES-REVIEWED] ba42c53 |
Contributor
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
github-actions
Bot
added
readiness: passed
Contributor
Author
|
Independently verified and certified. Head resolved once as Board. Collapsing check-runs by name and taking the highest id: 57 success, 0 failure, 0 timed_out, 0 action_required, 0 startup_failure, 0 cancelled, 0 pending of 63. The six non-success are all The empty red set is proven with a control rather than asserted. The identical expression returned Review lanes, read only after the board settled, since a body edit makes GPT re-read and it can post a new finding on an unchanged head. All four are green on this exact SHA, and both machine tokens match it:
Hygiene. One commit, two files, comment-only. No auto-close keyword, deliberately: Why this PR exists, stated plainly because the record should carry it. The comment it removes from That distinction is worth the round trip. A stale comment merely ages; this one was refuted by the code beneath it, and a reader who trusted it would conclude a shared-lock site could never use the helper and leave a correct migration undone. The second hunk shrinks the duplicated Certified by an automated pipeline. Merging remains a human decision — this pipeline never merges. |
iamwhatever
approved these changes
Sep 9, 2026
github-actions
Bot
removed
the
readiness: passed
jingchaodev
pushed a commit
to jingchaodev/KiroCrew
that referenced
this pull request
Sep 11, 2026
Operator-defined regex -> URL template rules (dashboard.link_patterns)
rewrite matching plain text into links at render time: prose matches
become markdown anchors, and an inline-code span whose whole text
matches renders as a link chip instead of the copy-only chip. Masking
keeps code blocks, existing links, autolinks and bare URLs untouched;
templates are http(s)-only and matched text is URL-encoded. Rules are
edited in Settings -> Chat and served through the dashboard config API,
so old transcripts linkify retroactively at display time.
feat(agents): note under the create modal's template picker that the choice can change later (#8497)
The Agent Template dropdown is the create modal's highest-stakes-looking
field: first-time users stall on it, assuming the pick is permanent. A
one-line note under the dropdown says the template can be switched anytime
after creating. Create-only via TemplateField's editLaterNote prop — the
editor renders the same field without it, since there the editable fields
are themselves the answer. Localized in all 13 catalogs.
Split out of #8497's original combined diff at the maintainer's request;
the Session Color removal now rides its own PR.
feat(dashboard): repeat code block actions at the bottom of tall blocks (#8784)
Chat code blocks pin their copy/edit buttons to the top-right corner
only. Once a block's rendered content grows past a fixed height
threshold, scrolling to grab those buttons costs a trip back to the
top -- exactly for the blocks most worth copying (long generated
configs, multi-file snippets).
Measure the block's content height with the existing
useMeasuredHeight hook and repeat the same action row (copy, plus any
caller-supplied headerActions such as the edit pencil) in a footer
once the block exceeds that threshold. The row markup is factored into
a small CodeBlockActions component shared by both call sites, so the
header and footer duplicate stay visually identical without a
copy-pasted JSX block.
Review findings addressed across this PR:
- max-two-buttons-per-row (blocking): the header's Run + Edit + Copy is
pre-existing (legacy status), but the footer is a NEW row, so
mirroring the header there put 3 siblings in a fresh row. CodeBlock
now takes a separate optional footerActions prop (defaults to
headerActions); EditableCodeBlock passes a trimmed Edit-only set to
the footer, keeping Run header-only.
- Footer copy reported success even when the clipboard write failed
(errors-use-error-notice, blocking): copy() now awaits copyCode and
only confirms on a true result, matching the established pattern in
TailnetMobileCard.
- Footer-triggered edit collapsed the block out from under the click
(the max-h-[480px] editor replacing a much taller block with no
scroll compensation). Fixed with scrollIntoView, gated on the
wrapper's top actually being above the viewport so a mid-viewport
HEADER edit doesn't also yank the page up for no reason the click
gave it.
- Inclusive Language (woke): "grandfathered" -> "legacy status".
Also: the footer's border was always painted, leaving a permanently
empty strip under every tall block at rest. It's border-transparent
until the same hover/focus reveal that shows the buttons, so the row
still reserves its height but paints nothing until there's something
to show.
Screenshots captured from an isolated CodeBlock render (short block vs.
a 30-line tall block) with the action row hovered, committed under the
repo-root temp-screenshots/ convention.
Closes #8227
test(pod): paginable long-history seed scenario (Refs #4100) (#9415)
No shipped seed scenario could paginate. The slot-detail fast path
answers has_more=false unless a session has size-rotated archive/
segments, and rotation only fires once a transcript outgrows a 10 MiB
budget, so every scenario built from a handful of messages leaves "load
earlier" structurally unreachable and a paging trace cannot be performed
through the real routes.
sessions-long-history seeds that state: one pinned session whose oldest
60 rows sit below a real rotation boundary. The initial slot load returns
the 8 live rows with has_more=true and next_before=60, and paging with
that cursor returns rows out of the archive.
Both shipped files are real ConversationLog output. The segment is
_maybe_rotate output at the production budgets, and the live transcript
is compaction output holding rows append wrote, so the fixture cannot
encode a rotation shape the writer does not produce. Only the rotate
segment and the live transcript are kept; the ~10 MiB compact segment the
recipe produces is discarded, which is the state a home reaches once
archive retention reclaims it. That keeps the fixture at ~20 KB of
package data, inside the 64 KB per-fixture cap.
The contract test pins the design point through the real readers and
pins the shipped segment's header and row shape against a rotation
driven in-test, so a change to the rotation format turns red instead of
leaving the fixture describing a shape the product no longer writes.
feat(members): dock the chat SidePanel beside the DM thread, first tab Crew summary (#9437)
The Members page's right column was a closable DetailPanel drawer — a
hand-built lookalike of the chat page's right column. It is now the chat
page's own tabbed SidePanel, permanent on wide windows (no close control,
no header toggle), whose first tab is the member's Crew summary carrying
what the drawer showed (identity + status, counters, driving sessions,
auto patrol, recent activity, wake sources, configuration, memory note,
edit exits). The + menu is the chat panel's (Files / Artifacts / Terminal
/ Browser / Side chat …) against the member's DM slot; the strip is
bucketed per member so it follows the roster selection.
SidePanel gains `leadingTab` (a host-owned, non-closable, non-draggable
tab ahead of the pinned block whose body the host renders), an optional
`onClose` (absent ⇒ no close control, Escape inert) and `extraReserveW`
(a sibling column's live width kept clear on drag). `usePanelTabs` takes
`opts.leadingId` so a fresh strip opens on the leading tab and an emptied
strip falls back to it. File / artifact open and file save move out of
useChatPageResourcesController into the shared `usePanelDocumentActions`
so both hosts run one implementation. Narrow windows (roster + thread +
panel cannot seat) keep the panel as an overlay the header button opens.
Closes #9432
Review round 1 (GPT): no agent hand-off on the panel read-error notice (the ChatPane below holds the DM draft as unsaved local state); Side chat withheld on this page until its composer draft persists or SidePanel keeps its body mounted (MEMBERS_WITHHELD_VIEWS, test-pinned).
Review round 2 (GPT): the panel binds to the member slot record only from the CURRENT snapshot (slotsLoaded) and only when it is the member's own (mode === member), so a record left over across a reconnect can never root Files / Terminal in a pre-restart project (test-pinned).
Design round: the unconfirmed-window withheld set is derived from VIEW_DATA_SOURCE (every classified view + terminal + app) instead of enumerated, so a new ViewKind is withheld there by construction; test pins it against the classification.
Rebased over #8947 (Quote / Ask on selected text): the drawer-hosted Side Chat main added is replaced by the panel's own Side tab — `openMemberSideChat` focuses it (revealing the overlay on narrow windows) for the confirmed slot only — and Side chat is offered again, since #8947 moved its composer draft into the per-slot chat-core store (the unmount-loss reason it was withheld for is gone). MembersPage.sideChat.test.tsx rewritten for the panel.
Fix-up: the Side tab is a dynamic tab (title as text, no aria-label) so the sideChat tests read it by accessible name; the drawer-only `side_chat_draft_waiting` key is removed from every catalog (dead-keys ratchet).
docs(spec): one paradigm for every monitoring loop (#9368)
Adds the umbrella spec the two existing monitoring specs sit under, so a
new monitored kind has a contract to implement rather than an existing
watch to copy.
Seven layers, one owner each, and six of them never learn what is being
watched. The consequential contracts:
- The probe signature is PLURAL from day one. A per-subject interface
cannot be batched later without changing every implementation and every
caller, and the difference is roughly 150 process invocations against
one query for fifty subjects. A probe that cannot batch loops internally
so the caller never encodes the difference.
- An observation is a NAMED entry with a severity and a reset scope, never
a bare fingerprint. A hash cannot be deduplicated per condition,
coalesced with a sibling, or re-asserted, because nothing can tell
whether two hashes describe the same condition.
- The decision layer is a pure function whose clock arrives as a value,
and it is level-triggered. Edge triggering loses any condition that
stayed true across a wake that did not happen. The re-alert window makes
that affordable and the budget makes it safe: a notification pipeline
aimed at humans needs no token budget because a paged human self-limits.
- Persisted state holds delivery bookkeeping only, versioned with a
migration per bump. Subject state belongs in the disposable evidence
file.
- An out-of-session driver is a detector, never a reactor. A cron turn has
no owning slot, so its tool calls hit deny-by-default and time out while
the job still records healthy.
Also records the rules that must be enforced by code rather than by prose,
including one the two current implementations already disagree on: the
status tool collapses superseded check attempts to the newest per identity
while the structured provider treats each row independently and maps
CANCELLED to failed, so it can wake on a failure that no longer exists.
Status is per layer, verified at 2f9ed9724, because most of this is the
target rather than a description of what runs today.
fix(dashboard): prevent empty mobile dialog (#9733)
refactor(apps): drop a shadow import and correct backend.py's comments (#9751)
`_capped_spill` re-imported `threading` inside the function, which the
module already imports unconditionally at line 24. The inner statement
re-bound the same `sys.modules` singleton, so removing it changes nothing
at runtime and removes the false hint that this function needs its own
import. No test patches `threading` on this module, so the local-to-global
name promotion is unobservable.
Two provably-equivalent structural edits alongside it:
- `_await_inflight_spawn`'s tail tested `cur is not None` twice and
re-read `.starting` to derive what the preceding arm's negation already
proved. It now returns early on `cur is None`, so each later test reads
once. `.starting` is written only at construction, and both reads were
inside the same `_lock` hold, so the single read cannot differ.
- `_start_app_backend_body` guarded `manifest.backend` against None when
computing `backend_type`, having already dereferenced
`manifest.backend.entryPoint` unguarded 326 lines earlier in the same
function. `AppManifest.backend` is a non-Optional `BackendConfig` with
no `__bool__`, so the else arm was unreachable.
The rest is comment hygiene in the same file, per AGENTS.md. A block above
`_PID_ANCESTRY_MAX_DEPTH` described "consecutive alive polls" that no
constant holds and `_survived_spawn` does not count; it is gone, and the
surviving block above those constants now states when a healthy backend
actually pays the full window (only where listener ownership cannot be
proven) instead of claiming it never does. Five docstring passages
narrated superseded code and now state current behaviour in present tense,
keeping the rationale. Two named the wrong thing: `_wait_for_pids` cited
`kill_pid` where the caller uses `kill_pid_pinned`, and
`_start_backends_concurrently` claimed boot costs one grace window
regardless of app count, which ignores the `_BOOT_SPAWN_MAX_WORKERS` cap.
`comment-history-baseline.json` drops this file from 7 markers to 6, which
`check_comment_history.py --write-baseline` records; the gate is a downward
ratchet and refuses a diff that retires a marker without lowering it. The
six remaining hits are present-tense statements about runtime state ("a
record that is no longer the tracked one"), not change history, so they
stay listed.
fix(chat): route a rejected 'auto' model to the picker and the default-model setting (#9099)
A partition that does not serve the `auto` sentinel rejects every session that
starts on it with "Your account does not have access to model 'auto'". The
generic entitlement message then advised "set agent.model to 'auto'", which is
the value that just failed, and the row offered Continue, which replays the
identical rejection. Reported from a region whose account is served only
gpt/deepseek/minimax/glm/qwen models (0.4.1).
- `_format_acp_error`: when the rejected id IS `auto`, say the automatic choice
is not offered in this region and route the user to the session model picker
and to Settings -> Chat -> Default Model (plus any per-agent pin). The non-auto
branch keeps its wording.
- `chat_runner`: the terminal error row carries `meta.kind = model_unentitled`
with `rejected_model` and `advertised`, decided from the exception's tags
(same evidence the formatter used), never from the prose.
- `ErrorCard`: a `model_unentitled` row offers "Choose a model" (opens the
composer's model picker) and "Change default model" (deep link to the
chat.default-model setting) instead of Continue. Panes without a picker render
the row as prose. Strings added to en.json + pseudolocale.
No automatic model substitution: the user picks, and the wire is unchanged.
Tests: new test_model_unentitled_meta.py (6), formatter wording test
(mutation-verified: fails on the pre-change formatter), ErrorCard tests (3).
Co-authored-by: Zejiang Guo <zejiangg@amazon.com>
feat: security-conductor golden-path corpus and verify_fix (#9500)
A security fix that makes the proof of concept stop reproducing has done
half a job. The other half is the half a security change actually gets
rejected for: the deny fence grows a rule that also refuses `gh pr view
--json`, or a guard is written against one host's spelling of a path, and
the tool the fix protected becomes one nobody can use.
This adds the corpus of legitimate operations that must survive a fix, and
the gate that re-checks them.
- ledger.py gains a fifth table, `golden_paths`, behind an additive
migration: every DDL statement is CREATE ... IF NOT EXISTS, so an
existing v1 database gains the table on the same pass and opens, and the
version bump is the last write -- an interrupted upgrade leaves the
version behind the tables, which retries, rather than ahead of them. The
bump is an UPDATE (the version row is a singleton) guarded by `<`, so a
database written by a newer checkout is left alone rather than walked
backwards. Every existing function signature is unchanged.
A corpus import is ONE transaction, so an interrupted one commits
nothing: a subset is a fence nobody chose and a silent one, since the
rows that never landed are invisible. That is why the import does not
call `add_golden_path` in a loop -- that function owns a transaction per
row -- and why the INSERT is split into a helper owning none.
`source_finding_id` rejects a bool explicitly, because `bool` subclasses
`int` and SQLite stores `true` as 1, which would silently attribute a
golden path to finding 1. Every text field that is present must BE a
JSON string: a list, number or null is refused, never str()-coerced or
defaulted, because "['gh', 'pr']" stored as a golden path makes the gate
classify a command nobody runs, and "platform": null widened to "any"
would gate every host with a row meant for one.
Rows carry the same propose/approve split as lessons, and approval is
write-once so the reviewer who admitted an operation stays on the
record. The table is the EDITING SURFACE, not the gate: an audit
proposes a row, a human approves it, and it reaches the gate when it
lands in the committed export through review. The CLI has three verbs
-- propose, approve, import -- mirroring the propose/approve pair the
RFC gives lessons and the committed-file relationship it gives
rules-of-engagement.json; no verb records one active, attributed row
from the command line, since that would be the approval without its
write-once record. There is deliberately NO
CLI verb that retires a golden path: the RFC gates deactivation exactly
like activation, and the flip stays the same human row edit rules and
lessons already use. Nothing reads the table as a gate, so the ledger
ships no host-filtered reader for it.
- verify_fix.py is the two-step gate. Step 1 runs verify_finding.py by
path -- never a copy of its judgement -- and requires `rejected`. Step 2
classifies every `shell` row of the COMMITTED golden-paths.json with the
tool gate's WHOLE deny composite -- the four checks hooks.on_tool_call applies to a shell
command, in its order (path fence, sensitive-command tier, exfiltration
auditor, deny-rule catalog), the same tier table scripts/deny_diff.py
declares -- and names the tier that refused. Measuring the catalog alone
would go green on a fix that tightened any of the other three. A tree
missing a tier is coverage lost and reported unavailable, not skipped.
A corpus file that is absent, does not load, or holds no row is 20, not
a pass: zero rows checked would fold to holds by construction, which is
the vacuous green a broken installation would report on every fix --
and the refusal scripts/deny_diff.py already makes of an empty corpus.
The gate reads the committed file and NOT the ledger's table, as the RFC
rules ("both gates read the file and nothing else"). The ledger is
per-host and invisible to CI, so a gate that read it could not be
enforced where it is declared blocking; and the table is mutable by
anything that can reach the database, so a gate that read it could be
steered by a row flip -- retire the one row the fix broke, and the gate
goes green with the fix unchanged. The file is read from beside the
skill, not from the worktree under review, so the change being judged
cannot rewrite the gate that judges it. And no ARGUMENT can shrink the
corpus: there is no flag naming another corpus file or another
platform -- the file is the one beside the script and the platform is
the host's own -- because a caller who could name either could name a
smaller check. Rows are named by their position in the file. The file
is validated by ledger.py's own loader, the same check
import-golden-paths applies, so the gate and the import cannot judge one
row by two rules -- and the loader accepts ONE shape, the object the
export is written in, so a second shape nothing writes is not a corpus.
The sibling ships in this same bundle, so an absent one is a broken
installation rather than a state to accommodate: still exit 20, because
a proof that was not re-run says nothing about the fix, and checked here
rather than left to the spawn, since the interpreter exists and a
nonexistent script argument makes the child exit 2 -- the verifier's own
code for "I rejected your input". The sibling `ledger.py` is the same
class: one that is absent or will not import is exit 20 through the same
JSON payload, never a traceback and exit 1, which is no verdict in the
contract. One printer, `emit()`, owns the payload shape and the stderr
lines for every path out of `main()`. Step 2 re-classifies every `shell`
golden path in the export whose platform matches the host against the
FIXED code; none may be refused.
NOTHING out of the corpus is executed, and that rule decides the design.
A row is text in a JSON file, and the same text once imported sits in a
table whose CLI ledger.py states plainly is not an authentication
boundary -- `--approved-by` is an unverified caller assertion -- so a
row is untrusted text written by whoever could edit the file or reach
the database. Every other consumer only READS such a row; running one as
argv would turn a file edit into command execution with the operator's
access, which no containment fixes because the escalation is in treating
the row as permission. So `shell` rows are CHECKED (classified, never run) and
`flow`/`cron` rows are RECORDED and reported for a human to exercise:
an MCP tool has no command line, a chat turn needs a gateway, and firing
a schedule has effects no deadline bounds. Parsing a schedule here would
settle nothing either -- a parse in this process never consults the
fixed code, so its answer is a constant no fix can change -- so the
corpus's own well-formedness is asserted in the test suite, at review
time, where a corpus-authoring mistake belongs.
Exit codes are the interface -- 0 holds, 10 the proof still reproduces,
30 a golden path is refused, 20 unverifiable -- and precedence is
10 > 30 > 20 > 0. The load-bearing property is the last one: 0 is
unreachable while any check this script OWNS went unsettled, because an
unclassified golden path is not a permitted one. A human row never moves
the exit code, because it was never claimed as this script's check.
The fence is read in a CHILD process with the worktree's own `src`
leading PYTHONPATH, because the point is to classify against the FIXED
code: an in-process import would bind whatever copy of the package the
interpreter already loaded, which for a test runner inside the
repository is the unfixed one. A leading PYTHONPATH entry is a
preference, not a guarantee -- a checkout without kiro_crew/security
would import the INSTALLED package -- so the probe proves where
kiro_crew.security came from and reports the composite unavailable
(exit 20) unless that file resolves beneath the worktree's src. A fence
borrowed from somewhere else is not a fence that agreed. The probe's payload is shape-checked
before any field is read off it, so a malformed answer is 20 rather than
a traceback exiting outside the contract.
Two containment details are deliberate. The ledger path is resolved ONCE
and passed to the verifier child explicitly -- the only thing this
script wants the ledger for: the child's HOME is the worktree and the
ledger's default path is HOME-relative, so a child left to its own
default would read a different database. And the deadline kills the
direct child only, matching verify_finding: a process-tree kill has no
portable spelling in the standard library, and a verifier whose own
teardown works on one host would be the platform lock-in this corpus
exists to catch. `--worktree` must be a git checkout, which is the
blast-radius bound the re-run proof depends on.
- golden-paths.json is the committed export the gate reads: 37 rows, each
with a reason. scripts/deny_diff.py's usage line and the fixture's
_comment name this file and the adopting change that retires the
fixture (#9678); wiring the gate into the fixer lane is #9707. The rows
cover read-only gh, git read/write/publish, the sanctioned test
invocation in both host spellings, the lint and type gates, the publish
preflight, the monitor-loop MCP tools, chat-start-one-turn via the
offline stub-ACP harness, and three shipped cron shapes. Two rows are
the read-only gh shapes that were false-positive refused while this was
being designed: two `gh pr view --json` calls joined by `;`, and a PR
read combined with a run list.
Tests
- test_security_conductor_verify_fix.py (73): the verdict ladder end to
end, driven as a subprocess with siblings staged beside it and the
corpus written one directory up, so the absent-verifier and absent-corpus
cases are testable at all; that the gate reads the committed file and
not the ledger, from both sides (a row only in the ledger does not gate;
a row retired in the ledger still does) and from the worktree (a
checkout shipping an emptier corpus is still judged by the skill's own;
no flag names another corpus or the other host); that a worktree without
the package does not borrow the installed fence, and that a fence under a
symlinked src is still the worktree's; that no corpus row is executed,
proved by a witness file each row would create if it ran, plus a
structural guard that the golden-path walk carries no spawn at all and
only two call sites spawn anything; the platform filter in both
directions; seven malformed probe payloads each pinned to the specific
guard it trips; the ledger path handed to the child; and one class that
asks the REAL `is_denied` about every shipped shell row, which turns the
corpus into a live regression gate at PR time.
- test_security_conductor_ledger.py: the new table's columns, its identity
(kind + command + platform, each part proved load-bearing), write-once
approval, retirement keeping its approver, corpus import that is
all-or-nothing on both validation and interruption, a racing identity
collision counted as skipped rather than fatal, and the v1 migration.
- Smoke-tested end to end against the real sibling verifier and the real
deny fence, with no stubs: a finding whose PoC no longer reproduces plus
a permitted `gh pr view --json; gh run list` row returns 0 and reports
the flow and cron rows for a human, and adding one genuinely refused row
returns 30 naming the rule that ate it.
- Red-before proven by reverting forty guards one at a time, including
the single accepted corpus shape, the string-typed text fields,
the file-not-ledger read, the skill's-own-copy read, the absence of a
corpus or platform flag, the fence-provenance check and its prefix
boundary, the absence of a one-step add verb, the unreadable-,
unloadable- and empty-corpus verdicts, the four-tier composite, the missing-tier
verdict, the absence of a retirement verb,
both exit-30 and exit-20 paths, the human-corpus separation, the
structural no-spawn guard, the platform filter, the migration bump, the
single import transaction, the bool rejection, and the live-fence gate;
every one turns its test red.
Refs #9195, #9332
Co-authored-by: Joe Guo <zejiangg@amazon.com>
fix(issue-radar): send Content-Type with glab api bodies (#9739)
glab api pipes JSON write bodies over stdin with --input - but sets no
Content-Type header. Strict self-hosted GitLab instances reject such a
request with HTTP 415 Unsupported Media Type, so Issue Radar write
operations (label add/remove, state changes) fail there. Add
--header "Content-Type: application/json" to the glab argv whenever a
body is present. Read paths without a body are unchanged, and the
GitHub transport is untouched.
Closes #9723
fix(work-ledger): retry a contended record read; make a test's ids salt-free (#9673)
Two reds on `Backend Tests (Windows) (4)`. Only one is a Windows fact.
`work_ledger._read_json_record` reads with a bare `read_text`, and a STRICT read
there is lock-free across writers: `_refuse_if_worker_holds_open_item` reads the
prior item under the WORKER's binding lock while that item's own conductor may be
replacing it under a different item lock. On Windows a read of a file another
handle holds open for write raises `PermissionError`, so one correct concurrent
writer turns a strict read into a bare `OSError`. The guard catches only
`WorkLedgerError`, so it escapes `apply_conductor_action`, and the dashboard route
maps `OSError` to a 503 `ledger_write_failed` "try again" -- telling a conductor to
retry a binding that is legitimately taken until the item closes. A permanent
refusal reported as transient is a product defect, so the fix is in the store:
route the read through `atomic_write.read_bytes_with_retry`, the read-side twin of
`replace_with_retry` that `members.py` already uses for this exact window. A
`PermissionError` that outlives the bounded retry still escapes, so the
fail-closed contract holds; on POSIX the helper re-raises on the first attempt.
The second red is not Windows-specific at all.
`test_reconcile_and_publish._finding` mints each PR url from
`abs(hash(fp)) % 9000 + 1`. `hash()` on a str is salted per interpreter process,
so roughly one process in 9000 gives two fingerprints the same url; the sweep then
skips both and the test reads `assert [] == ['aa']`. Each xdist shard is its own
process drawing its own salt, which is the whole reason it looks like one shard is
special. Reproduced on Linux with `PYTHONHASHSEED=17494`, where `aa` and `bb` both
map to `pull/1344`. A sha256 digest is salt-free, so the mapping is identical in
every process.
Tests: a new `test_a_contended_item_read_still_refuses_with_already_bound` drives
the guard through `windows_sim.read_sharing_violation` and asserts the refusal
stays `already_bound`; reverting only the retry line makes it fail with the
injected `PermissionError`. The four existing fail-closed tests move their fault
from `Path.read_text` to `Path.read_bytes` through one shared `_fault_record_read`
helper, since that is the call the record reader makes; their assertions are
unchanged and still pass on POSIX, where the helper does not retry.
feat(dashboard): add top-level /sessions page — bookmarkable session chooser (#6737)
A neutral, bookmarkable session list at /sessions inside the main dashboard
shell (nav rail + top bar stay available). Rows navigate to the full
/chat/<key> experience; unlike bare /chat the page never auto-selects or
auto-creates a session. Full-bleed rows on phone; schedule-style inset with
bordered group cards on desktop. Filter chips (All / Unread / status tags in
use), search, recency groups (Today / Yesterday / Earlier), New session.
Original work by Helena Stafford (@helenastafford).
Co-authored-by: Kiro Crew <noreply@kirodotdev.github.io>
feat: preserve opaque inbound attachments (#3754)
Video and unrecognized formats were rejected before download, so an inbound
file the agent could have acted on arrived as a note instead of bytes. Keep
them as byte-identical temporary files under a 50 MB cap, hand the agent the
local path plus the original name, type and size, and reuse the existing
per-turn cleanup ownership. Opaque bytes are never parsed or executed
automatically.
A sender picks filename and mimetype independently, so an opaque file could
arrive named "photo.png" while declaring application/octet-stream. The ACP
encoder types a prompt path by suffix alone, so such a path would reach the
image sink without the content-signature check the IMAGE branch enforces --
emitted as image/png on the strength of a sender-supplied name. An inlineable
image suffix is therefore stripped from the temporary path before ownership
transfers, mirroring the retype the IMAGE branch already performs, and a
rename that fails drops the attachment rather than emitting the original path.
A contract test pins the suffix set against the encoder's own table.
Co-authored-by: Jindong Hu <hjindong@amazon.com>
fix(acp): verify the agent the --agent flag selected actually loaded (#9668)
Guard (A) in create_session fails closed when a requested agent is absent
from the session's advertised modes, so the session never silently runs the
backend's own default in its place. But it only ever inspects the agent that
set_mode will activate -- an explicit override, or on KAS the injected
default. On kiro-cli, mode_agent is None for an ordinary session, so the
agent actually chosen by the --agent spawn flag reaches no check at all.
load_session has the same shape: its check reads `agent`, and its only
caller passes `agent=agent or None`.
That agent's spec can fail to load silently. A live probe of kiro-cli
settles what the session/new response says in each case: a spec that loads
is reported as `currentModeId` AND listed in `availableModes`, while a spec
the backend refuses is absent from the list and `currentModeId` names
kiro-cli's own default instead. The refusal is silent -- no JSON-RPC error,
a normal sessionId, the process runs on. Nothing downstream notices: the
set_mode response is never read back, currentModeId is never re-compared,
and mcp_session_report only logs (its docstring forbids reading a missing
report as "not mounted").
The user-visible result is a session with none of Kiro Crew's control plane,
while the global provider mcp.json -- which Kiro Crew pins off only on specs
it writes itself -- stays merged. So a third-party MCP server declared there
keeps working while every Kiro Crew tool the injected prompt names answers
"A tool with the name '<tool>' does not exist". For learn_add that reads to
the user as the agent reporting its memory is unavailable and the lesson was
not saved, when in fact the whole server is absent.
Guard (A2) applies the same check to the spawn-flag agent on both
session-start paths, before set_mode, terminating the session it created so
the backend does not hold an orphan. `currentModeId` is read as proof in
both directions: naming the spawn agent admits, naming anything else fails
closed even when no advertised list came back. The compatibility escape is
narrow -- only a response that names NO current mode falls back to the
advertised list -- so older kiro-cli and the offline fake backend are
unaffected. Scoped to the backend whose argv carries --agent, by a positive
identity test (harness-parity H5).
Six regression tests, mutation-verified: neutering the guard fails the three
fail-closed cases, and admitting a current-mode mismatch fails exactly the
substituted-current-mode test.
Co-authored-by: Zejiang Guo <zejiangg@amazon.com>
feat(chat): Quote / Ask on selected text in ChatPane (Members + split view) (#8947)
Selecting text in a Crew Members thread (or a split-view pane) offered Copy
only: AssistantMessage's selection toolbar draws the actions its host hands
it, and ChatPage was the only host passing onQuote/onAsk — quote's
FlyingQuote-into-composer and ask's open-/side-and-seed were ChatPage-local.
Extract both into one chat-core seam, chat-core/composer/selectionActions
(useSelectionQuoteAsk + quoteIntoDraft + seedSideChat), and make ChatPage and
ChatPane both consume it — no second implementation. Hosts differ only in
what they own: the composer draft, and how a Side Chat surface for a slot is
brought on screen (openSideChat). Quote is always offered; Ask exactly when
the host provides an opener (capability by omission, like onOpenFull).
- app-sdk: ChatMessageList/MessageRenderContext gain onQuote/onAsk; the
default assistant row passes them through.
- ChatPane: wires the seam to its own composer (FlyingQuote lands in the
pane) and takes an openSideChat prop.
- Split view: SessionGridView threads ChatPage's opener, which re-binds the
activity panel to the pane's slot (switchSlot) before opening the side tab.
- Members page: the detail drawer gains a Side Chat view bound to the
member's slot (reuses existing "Side Chat"/"Details" strings — no new copy).
- SideChat: the side-seed event now names its slot; a SideChat bound to
another slot ignores it, and the seed poll waits for the named slot's
composer (data-side-chat-slot) rather than any Side Chat's.
- ChatPage: only change is consuming the hook (behaviour unchanged) plus the
one-prop grid wiring.
Tests (mutation-oriented pins): selectionActions unit tests, ChatPane
selection-actions test, MembersPage Side Chat drawer test, SideChat
slot-aware seed, ChatPage Ask seed regression. Capture harness
website/scripts/capture-members-selection-quote-ask.mjs with three
asserted frames under temp-screenshots/members-selection-quote-ask/.
refactor(chat-core): route the scene popover and feedback sends through sendTurn; drop api.steerChat (#9593)
Chat-core RFC P2, batch C of #9570. The last two hand-rolled dashboard
senders call the transport's `sendTurn` and branch on its receipt, so
the deadline and the six-way classification are the core's:
- `hooks/useSceneInteraction.tsx` (scene agent popover composer): the
running/idle fork -- `api.steerChat` mid-turn, `api.sendChat` plus a
local `readSendReceipt` otherwise -- becomes ONE call,
`sendTurn({ message, slot, steer: !!src?.running })`; `steer` is a flag
of the same endpoint (#8689), not a different receipt shape.
refused -> report on the composer, hand the payload back
transport-error -> report on the composer, hand the payload back
unknown -> composer back to idle, nothing claimed (as before)
response-late -> report on the composer with the core's
delivery-unconfirmed copy, hand the payload back
(NEW: the old path awaited without bound; nothing
proves the gateway saw the request, so silence would
discard the text -- same policy as ChatPage)
dispatched/queued -> 'sent' tick + mini-thread echo, as before
A refused steer used to surface through the outer catch (steerChat
parsed via `j` and threw on a non-2xx); it is now a `refused` receipt
with the same user-visible result, and a `{ok:false}` steer inside a
200 -- which `j` passed through silently -- is now reported too.
- `App.tsx` feature-request send: `refused` -> error row with the
server's reason; `transport-error` -> error row; `unknown` and
`response-late` stay silent (the request was or may have been
accepted; the row is the pill's only signal).
- The composer's failed-send line migrates from a hand-written
`role="status"` div to `ErrorNotice variant="inline"` (same string),
`askAgent` off with a No hand-off comment naming the restored draft
(errors-use-error-notice: this diff touches its condition).
- `api.steerChat` had no remaining caller and is deleted; `sendChat`'s
comment now describes the steer flag it carries for the transport, and
`sendChat` runs `checkSessionExpired` on its raw response so a 403
auth challenge on any transport send keeps the silent-refresh / banner
recovery the `j`-parsed steer helper had.
Tests: the scene coverage file mocks the transport's wire (`api.sendChat`)
so the REAL classifier runs; it asserts `steer` is forwarded from the
running state, adds a refused-steer case and a deadline (`response-late`)
case. The feature-request test pins that the send rides the transport
(deadline signal on the wire). Dead `steerChat` stubs removed from four
test files. RFC 4.2 row split into the three #9570 batches.
Refs #9570
test: third side-effect audit (macOS): undo-proof floor, PTY deadlock (#9663)
Five full backend runs (92,261 tests each) and five full frontend runs on a
macOS host, from inside a Kiro Crew agent session, with an audit hook that
attributes every host write, spawn, connect and kill to the test that made
it and a per-test census of duration, RSS, threads and descriptors. Zero
backend flakes in 5 x 92k; two frontend flakes; 227 deterministic failures
and four 120-second hangs that every Linux run had been blind to; and host
writes the earlier audits could not see. Every fix carries a test that goes
red when it is reverted, and the classes are written into
testing-conventions "Side effects" so they cannot recur unnoticed.
Host state the suite touched, and what pins it now:
- monkeypatch.undo() in a test body unwound the whole floor (KIROCREW_HOME,
KIROCREW_PROFILE, KIROCREW_TELEMETRY, the service guard). #9614 moved the
pins onto the private _floor_monkeypatch; this adds the missing half: the
monkeypatch fixture is re-declared at the rootdir to depend on it, so the
floor is set up first and torn down last by construction rather than by
autouse ordering. test/conftest.py's autouse pins ride the same instance.
- A metric emitted at IMPORT (ToolHookResult.allow() as a default argument)
built the recorder from the operator's real config before any pin
existed and exported to the real ~/.kiro/crew/metrics every minute for
the life of each worker. KIROCREW_TELEMETRY=0 is now pinned for the
whole process in pytest_configure, and every module whose collection
builds the recorder is recorded and asserted empty.
- The mc-maint sweep resolved config_dir() when it RAN, after the queuing
test's pin was gone: 60+ mkdirs of the real home per run, plus a rmtree
and a marker aimed at it. cleanup_stale_sandbox_profiles takes the home
its caller resolved, with no default that could reopen the on-pool
resolution; SessionManager resolves it once at construction.
- Five tests dropped the override to test the default home and let
config_dir() create the real ~/.kiro/crew, and a resolver-only stand-in
still rewrote the real ~/.kirocrew.breadcrumb beside it. The teardown
hook reads paths._resolved_home before any fixture unwinds and fails the
test after the floor has torn down; the floor wraps the breadcrumb writer
to refuse while Path.home() is the operator's; each test fakes the host
home instead of only the resolver. A class- or module-scoped setup runs
outside the floor and now pins what it resolves itself.
- Fifteen __pycache__ trees per run from imports by path: sys.pycache_prefix
and PYTHONPYCACHEPREFIX point at ~/.cache/kirocrew/pycache.
- KIROCREW_PROFILE=amazon inherited from the agent session failed 150+
builtin-app tests closed because the pin lived in test/conftest.py,
which those testpaths never see. It is a rootdir floor now.
macOS, where the Linux-green suite had 227 failures and four hangs:
- _kill_session closed the PTY master before ending the shell; on
macOS/BSD close() waits for the reader's outstanding read, so four
terminal tests hit the 120 s timeout on every run. The process tree is
hung up (SIGHUP, the one signal an interactive shell honours) and
terminated before the master is closed; the tests run in under a second.
- Linux-shaped tests, one rule each: the frame recorder's 75 logic tests
pin its Linux-only ACL gate open; O_TMPFILE prompt tests skip on the
production capability flag; /dev/fd/N is compared by open+fstat identity;
unlink() on a directory is EPERM here, so _discard_untracked_files
recognises it; a simulated O_BINARY bit is derived from the live O_*
constants (landed in #9632); a --copies venv cannot relocate a
non-framework shared-lib CPython; the darwin workspace binding's pass_fds
entry is pinned away in the snapshot-descriptor test; a pinned
sys.platform needs a pinned start id; and codex-review.yml's two sed
expressions become one 1,8{} block, because BSD sed opens a numeric range
only on its exact line.
- test_handlers_system_macos_paths reached 8.8.8.8:80 through _local_ip();
stubbed at the seam the Linux siblings already use.
Frontend: AutoNudgePopover's watch list is three promise hops past
act(render), so the positive test flaked and every negative was vacuous;
the block now waits for the fetch to be answered. App's startup-video gate
read an interruption latch that an effect sets a commit after the
changelog shows, while changelogDecided lands a microtask later; the gate
reads the live conditions in the same commit too.
feat(dashboard): show a pre-boot failure panel, and gate chunk cycles in CI (#9656)
Two halves of one problem: a page that cannot boot should say so, and the one
config-reachable cause of a dead boot should not be able to ship.
VISIBLE FAILURE (closes #9543)
When a boot-critical module never arrives, no app code runs. No React error
boundary exists yet, and src/lib/staleShellHeal.ts cannot help either - it is a
boot-TIME probe, so it needs the boot it is meant to rescue. A top-level page in
that state showed index.html's dark empty shell and nothing else, which is the
mobile black screen behind #9518 and #9540.
index.html already detected the failure: a capture-phase error listener logs
"[boot] module script failed to load" and posts mc-embedded-boot stage=script-error
to the parent. But it returned early unless embedded, so a top-level page - the
phone - got nothing. Extend that listener rather than adding a second detector.
The panel is shell-owned and inline: no stylesheet, no bundle, no i18n, because
none of those loaded. "Did we boot" is answered by whether #root has children,
which needs no new signal from main.tsx - an entry chunk that failed leaves it
empty, and a LAZY chunk that failed after render leaves it populated, in which
case the running app owns that error and must not be covered. The reveal waits 4s
so it cannot fire while the service worker's own bounded retry is still winning.
Retry unregisters the worker and drops CacheStorage before reloading, because a
plain reload does not bypass the worker on iOS Safari - that is the escape hatch
users currently reach only by clearing site data by hand.
CYCLE GATE
Two chunks that statically import each other have no valid initialization order:
one body runs against a binding that is still uninitialized, and in this app that
lands on new QueryClient(...) throwing before React mounts - the same blank page,
from a different cause. Nothing in the pipeline could see it. check-bundle-size.mjs
measures bytes, tsc and eslint never look at the bundle, and the unit suite does
not load one. It is reachable from a config edit alone: rolldown's
includeDependenciesRecursively: false produced two cycles on a tree that had none
- a 71-chunk one spanning App, client, vendor-react and vendor-icons, and a
3-chunk one across the graph chunks - with every other gate green.
bundleReport.mjs now records each chunk's static imports, as the bundler resolved
them rather than by parsing minified output. dynamicImports is excluded: a dynamic
edge defers execution and cannot produce the failure. findChunkCycles reports
strongly connected components as data, matching checkChunkBudgets, and refuses a
report with no imports field instead of reading "no edges" as "no cycles".
check-chunk-cycles.mjs is the CLI.
It runs as a second step in the existing Bundle Size Gate job, reusing that job's
analyze build rather than paying for another one. The job keeps its name: it is a
required check, and renaming it to match the widened scope would silently stop
satisfying branch protection. The prepare-pr floor entry gains the same command,
so the profile still mirrors ci.yml.
Verified: chunk-cycle gate reports 675 chunks, 2058 static edges, no cycles on
this base; bundle-size gate still 675 within budget; i18n:render passes, which is
what proves the index.html edits did not break Vite's import-map relocation; 25
prepare-pr floor tests pass including test_ci_blocking_scans_are_covered_by_the_floor.
Co-authored-by: Bolin_Chen <17506219+bobbyfine@users.noreply.github.com>
fix(eval): save the eval report as utf-8, not the host code page (#9726)
format_results puts a ✅ or ❌ on every scenario, session, turn and
assertion, so the report `kirocrew eval` saves is never ASCII. It was
written with Path.write_text and no encoding=, which encodes with the
host's locale codec — cp1252, cp950, cp932 on Windows — and raises
UnicodeEncodeError on the first glyph.
The raise lands after the whole eval has run and takes the JSON summary
with it, so a run that printed its report to the terminal saves neither
artifact. The turn snippets the report embeds carry arbitrary agent text
as well.
The two writes move into write_eval_artifacts so the contract can be
exercised in a child process: the codec open() defaults to is fixed at
interpreter start and cannot be substituted in-process.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
fix(meetings): match a dictionary alias that starts or ends with punctuation (#9717)
Every alias was compiled as \b<alias>\b. \b asserts that a word character sits
on exactly one side of the position, so it delimits an alias only where the
alias's own edge character is itself a word character. On an alias edged with
punctuation it asserts the opposite: \b\.net\b requires a word character before
the dot, so the standalone ".net" the speaker said was skipped and "asp.net"
was rewritten instead.
"c++", "c#" and ".net" are ordinary entries in a speech-correction dictionary.
They were accepted by add_term, persisted, and listed in the editor as active
terms while correcting nothing — and silently rewriting text they did not name.
Each edge now takes \b when the alias's own character there is a word character
and a lookaround for a neighbouring word character when it is not. The two are
equivalent on a word-character edge, so an alphanumeric alias keeps exactly the
pattern it had.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
fix(ci): diff base to working tree for added lines in merge-ref ratchets (#9734)
The merge-ref gates (comment-history, subprocess-encoding, sync-io,
agent-sdk-boundary) scan violations from the working tree but took their
added-line numbers from git diff <base>...HEAD, which cannot see
uncommitted edits. On a dirty tree a pre-existing baselined line shifted
by an uncommitted insert could land on a line number the HEAD diff
counts as added, failing the gate locally on bytes that pass once
committed. added_lines() now diffs merge-base(<base>, HEAD) to the
working tree for the three-dot label, matching added_lines_at() and the
file state the gates actually scan. Clean trees (CI) are unaffected: the
two diffs are identical there. The merge-shape labels keep their
committed endpoints.
Closes #9719
fix(segmented-control): stop the active pill from tracking a mid-animation box (#9715)
The active-pill indicator is a layoutId motion.div, absolute inset-0 inside a
segment motion.button that carried layout. The segment label animates its width
0 -> auto on reveal (compact / iconOnly), growing the button box every frame, so
the shared-layout indicator was sized to an intermediate box instead of the
settled one (#9684).
Fix, both halves needed (measured 0px divergence together, ~6px each alone):
- drop layout from the button so it no longer re-projects its box every frame;
- give the indicator layout="position" so it springs only its cross-segment
travel and takes its size from CSS inset-0, matching the button box every
frame, settled or mid-reveal.
Closes #9684
docs(locks): correct the kept-inline lock-open comments (#9267) (#9661)
Two comment-only fixes to the sites #9647 left inline:
- session_pid._periodic_pid_sweep: delete the false clause claiming the
with-scoped helper "serves exclusive acquisition only". open_lock_file
only OPENS non-truncating; the lock mode is chosen at file_lock(fd,
exclusive=...), and this sweep acquires shared read twelve lines below.
The real reason it stays inline is fd lifetime -- the fd is held across
the try/finally, not a `with` block -- which the original clause already
stated; keep that, drop the added falsehood.
- _McpFileLockSync.__enter__: shrink to a pointer at _McpFileLock above,
whose kept-inline rationale already states the fd-lifetime reason fifty
lines up. Repeating it is the duplication this item exists to reduce.
No code change; the migrations #9647 landed are untouched.
Refs #9267
refactor(monitoring): type the probe boundary to a subject, not to GitHub (#9505)
Both Protocols at the external probe boundary -- `_Provider` in
monitoring/controller.py and `GitHubShadowProvider` in monitoring/shadow.py --
annotate their `probe` return as `GitHubPullRequestProbeResult`. That return type
is the defect. An abstraction typed to its one concrete implementation is not an
abstraction: a second monitored kind cannot satisfy either boundary whatever it
returns, because satisfying it requires producing a pull request's result type.
The two declarations being near-identical is a symptom of that, not the problem.
Introduce MonitorProbeResult in monitoring/models.py, naming no host: the
subject's canonical facts plus the generic observation the engine classifies.
GitHubPullRequestProbeResult becomes an implementation of it, keeping `response`
-- its own typed detail -- on itself rather than on the shared type. The two
Protocols collapse into one public MonitorProbe, and the service boundary
(`_Service.apply_monitor_probe`, AutoNudgeService.apply_monitor_probe) is retyped
to the shared record so no path names GitHub to describe a probe.
The signature is PLURAL from the start: it takes a sequence of subjects and
returns a mapping keyed by the subject string AS PASSED IN, not by any identity a
host derives from it -- a caller can only look up what it asked for. GitHub
answers for one pull request per call so its implementation loops, but arity is
the one thing that cannot be changed later without touching every implementation
and every caller, so it is settled now. Keying by the caller's own string is what
lets a host normalize a subject without reshaping the mapping its caller reads.
MonitorProbeResult is a RECORD rather than a bare sequence of per-check rows. A
host that publishes its own overall verdict, distinct from the rows a probe
enumerates, then has somewhere to put it as a defaulted field that reaches every
caller without changing this type or any signature naming it. No such field is
added here: nothing would read it, and this probe fetches no published aggregate
today, so populating one would mean a new request and a behaviour change.
A plural boundary lets a provider answer for a SUBSET of what it was asked, and
lets it answer with the wrong shape. Neither is a verdict: an absent subject
leaves no observation to decide from, and the decision engine reads attributes off
whatever it is handed, so an untyped value fails deep inside it rather than at the
boundary. Both consumers -- MonitorController.tick and run_shadow_probe -- resolve
through one shared `resolve_probe_result`, so the two cannot disagree about what
an unusable answer means; a guard in one and a bare KeyError in the other would be
the same hazard handled two ways.
Behaviour-preserving for a provider that answers its contract.
test_monitor_behaviour_golden.py's digest and all 28 of its per-group digests --
captured on kirocrew/main at 53987e756 before any edit in this stack -- are
unchanged across the boundary change.
feat(command-bar): file a contributed command's session in its own folder (#9573)
* feat(command-bar): file a contributed command's session in its own folder
A contributed row opens a NEW session every time it runs, and those sessions are
generated work rather than conversations the reader started. Left unfiled they
accumulate at the top level of the sidebar, push the reader's own chats down, and
interleave two different commands' runs with nothing separating them.
Each command now gets a folder of its own under one parent that says where all of
them came from: Command Bar Sessions / <the row's title>, created on first use.
Filed last and never awaited, so a folder API that is slow, capped or refused costs
the session its place in the sidebar and nothing else. Contributed rows only -- the
Ask row carries a sentence the reader wrote. Nothing is filed for a seed the reader
abandoned mid-create.
The folder list is READ from the sidebar's own ['chat-folders'] cache rather than
fetched: GET /api/chat/folders walks the on-disk session list synchronously to count
archived sessions per folder, so a fetch per run would pay for a filesystem scan that
scales with the reader's history to learn what is already cached. A cold cache falls
back to one fetch.
The parent name is a durable value the server stores and this code matches by name
later, so it is written as a literal with a scoped i18next/no-literal-string off block
in eslint.i18n.config.js, beside wireValues.ts's -- the same category, and where this
repository counts suppression.
The leaf name is clamped to the 100 characters chat_folders.py stores, because a
manifest title may be 120: without the clamp the create is silently shortened, the next
run's lookup for the full title misses, and every run makes another folder. When two
rows currently offered carry the same title, the contributing app's label is appended to
both, since a leaf keyed on the title alone would interleave two commands the reader
cannot tell apart.
* fix(command-bar): make the folder name safe under Unicode and guarantee a unique suffix
Two halves of one GPT finding, both real.
`slice` counts UTF-16 units, so a title whose 100th unit falls inside a surrogate pair
lost half a character and the stored name carried a lone surrogate -- an invalid string,
made durable. Truncation is by code point now, which is also what the server means:
Python slices its own strings by code point.
And the hash tier was only unlikely to collide, not unable to. The suffix now walks three
tiers -- readable tag, hash of the whole row id, hash plus the row's position in the
id-ordered colliding group -- each tried only when the one above fails to separate
anything, so uniqueness inside the group is guaranteed rather than probable.
feat(apps): default Library to enabled apps with a labelled show-all toggle (#9481)
Library listed every installed app, so the ~20 default-off builtins dominate the page a user visits to manage their own apps. Default the list to enabled apps and add a persisted, labelled 'Show N disabled' control that reveals the rest and switches back to 'Show enabled only'; the count tells an empty list apart from a filtered one.
The view participates in libraryView's per-visit 'listed' decision through the wasEnabled latch rather than filtering the rendered list: a row disabled mid-visit stays in place instead of vanishing under the cursor, while a never-enabled builtin is filtered. It is a filter, not a hide, so a default-off builtin with no Discover row stays enable-able. The empty enabled-only view keeps the reveal control so those builtins are never stranded.
Persisted with usePersistedBool under mc-apps-library-show-all (added to DURABLE_PREF_KEYS) so the choice survives reloads and follows the user across origins, like the other view toggles. New i18n keys across all catalogs plus the generated pseudolocale; plural base registered. Tests extended for the disable-then-stay sequence and the show-all reachability guard; the disabled-tile suites opt into the show-all view. Screenshot harness and two committed evidence images under temp-screenshots/.
Closes #9473
feat(diagnostics): add redacting kiro_cli_logs read tool (Refs #6023) (#9058)
Give the agent driving kiro-cli a sanctioned, redacted way to read
kiro-cli's own logs after a rejected turn. kiro-cli's data directory is
fenced by the command gate (it also holds SSO tokens) and that gate is
deliberately coarse and verb-independent, so naming a fenced path is
enough to end the turn. This is one reviewed read path instead of a
carve-out on a fail-safe gate.
Scope is the primary control, not redaction. The reader reads kiro-cli's
mcp/lsp protocol logs only. kiro-chat.log and the session transcripts are
each ONE fixed host path rather than a per-session file, so on a host
running one gateway with many concurrent sessions each interleaves every
session's traffic; both carry conversation prose, and the redaction stack
is a CREDENTIAL pass that does not narrow prose. collect_bundle still
includes the chat log: that path is user-to-user, this tool is
cross-session.
Measure the property that scope rests on, and enforce it. mcp/lsp logs are
safe to read here only while they record protocol traffic rather than frame
bodies, since they share the chat log's interleaved shape and an MCP
tools/call frame carries conversation-derived arguments. On kiro-cli 2.21.1
mcp.log is empty across a session of continuous MCP tool calls, every
lsp.log record is a single-line timestamped ERROR with no JSON-RPC envelope
and a longest line of 313 bytes, and sentinel strings passed as tool-call
arguments appear in neither file. Because that measures one version of a
component this repo does not pin, a source whose text carries serialized
frames is refused whole and visibly, so drift surfaces as a refusal rather
than a silent widening. The refusal decides only whether a file is the KIND
the tool assumes; it never claims to separate one session's frames from
another's, which is the pretend-scoped filter this module rejects.
Strip hidden characters before redacting, inside _scrub. redact_credentials
matches a secret's literal shape, so an invisible planted inside one
defeats it. Stripping afterwards is worse than not stripping, because the
strip REJOINS a credential that redaction was already asked about and
declined, and every consumer strips later: an MCP response goes through
validation.sanitize_response and a bundle member is normalized by whatever
reads the zip. strip_hidden_unicode states this ordering as its own
contract. Putting it in _scrub rather than the reader covers collect_bundle
by the same change.
Require the opened descriptor to be the file that was checked. lstat before
the open and fstat after must report the same st_dev/st_ino, refusing both
a final component that is a link and a swap landing between the two calls.
This is what defends Windows, where there is no O_NOFOLLOW and a junction
is a reparse point is_symlink does not report while satisfying S_ISREG.
Bound the read by max_bytes on every branch. size is an fstat snapshot, so
a file appended to faster than the loop drains never reaches EOF and a loop
bounded only by EOF grows until the process is OOM-killed.
Set O_NONBLOCK alongside O_NOFOLLOW. A validated regular file swapped for a
FIFO makes an O_RDONLY open block forever, because the S_ISREG guard only
sees a descriptor the open already returned.
Bound the whole response and trim from the FRONT. Every tool response
leaves through validation.build_tool_response, whose sanitize_response
truncates the TAIL -- for a log tail the worst end to lose, since the newest
lines are the reason the tool is called and the transport's marker reads as
if the oldest went. The reader keeps its output under a ceiling below the
transport's, divides that budget across sources, and labels which end it
dropped.
Return only the truncation marker when the tail window holds no newline. An
unterminated final line longer than the cap leaves no boundary to cut on,
and emitting that fragment strips the header token that redacts a value
further along the same line.
Bind a continuation line to the verdict of the event it belongs to, so an
event `since` rejects does not ship its payload without the header line
that identified it.
Keep one tail reader for the module, so the check-then-open guarantee
cannot be present in one path and absent in the other.
Start a cut window at a record boundary. The frame tripwire keys on a
frame's envelope tokens, which in a pretty-printed frame sit at the top,
while the byte cut runs from the front -- so an oversized multiline frame
can have its envelope fall outside the window while its trailing argument
lines survive, and those lines carry another session's conversation. A cut
window whose first line is not a timestamped record is sitting inside a
record whose beginning is gone, so those orphaned lines are discarded and
the window starts where a record does. A frame that starts INSIDE the
window still carries its envelope and the tripwire catches it, so the two
together cover both positions of the cut. This trims only a cut window, and
it lives in the agent-facing reader rather than the shared tail reader:
unlike the hidden-character strip it discards data, and the cross-session
boundary that justifies the loss is one collect_bundle does not cross.
Match the record prefix, not any leading digit, when finding that boundary.
A serialized frame can carry a JSON array of bare numbers, so a line like
" 1000000," begins with a digit and would pass as a record start --
ending the trim INSIDE the frame and returning the argument text after it.
The predicate requires an ISO date followed by T or a space, which no JSON
number can satisfy, and _filter_since shares it so the two cannot classify
a line differently.
feat(knowledge): cross-file cost ceiling for explicit imports (#9124) (#9222)
Co-authored-by: Kiro Crew Auto-Pipeline <kirocrew@users.noreply.github.com>
fix(auto-improvement): publish only the commit the pipeline committed (#8981)
`_direct_push` was handed the sha its finalizer produced -- the commit that was
measured, reproduced and written to the ledger -- and then pushed
`HEAD:refs/heads/<dest>`. Nothing compared the two, so whatever HEAD pointed at
when the push ran is what got published.
The window between them is not empty. `_prepush_review_clean` runs an agent in
that same clone with `Bash`/`Edit` for up to 30 turns, its prompt invites it to
"fix a trivial finding in the clone and re-review", and the runner's git
denylist covers only `push` and `remote set-url`, so `git commit --amend` is
permitted there.
The gate compares full object ids and fails closed when it has no valid one. It
sits after the review gate and before the credential scan and the push, and
deliberately not around the push itself: `_push_with_rebase` rewrites HEAD on
purpose and re-verifies the replayed tree.
The anchor is a full forty-hex object id, captured at the commit and never
re-derived. A short sha is an ambiguous NAME: git reads a revision as a ref name
before an abbreviated object i…
bolichen97
pushed a commit
that referenced
this pull request
Sep 12, 2026
Operator-defined regex -> URL template rules (dashboard.link_patterns)
rewrite matching plain text into links at render time: prose matches
become markdown anchors, and an inline-code span whose whole text
matches renders as a link chip instead of the copy-only chip. Masking
keeps code blocks, existing links, autolinks and bare URLs untouched;
templates are http(s)-only and matched text is URL-encoded. Rules are
edited in Settings -> Chat and served through the dashboard config API,
so old transcripts linkify retroactively at display time.
feat(agents): note under the create modal's template picker that the choice can change later (#8497)
The Agent Template dropdown is the create modal's highest-stakes-looking
field: first-time users stall on it, assuming the pick is permanent. A
one-line note under the dropdown says the template can be switched anytime
after creating. Create-only via TemplateField's editLaterNote prop — the
editor renders the same field without it, since there the editable fields
are themselves the answer. Localized in all 13 catalogs.
Split out of #8497's original combined diff at the maintainer's request;
the Session Color removal now rides its own PR.
feat(dashboard): repeat code block actions at the bottom of tall blocks (#8784)
Chat code blocks pin their copy/edit buttons to the top-right corner
only. Once a block's rendered content grows past a fixed height
threshold, scrolling to grab those buttons costs a trip back to the
top -- exactly for the blocks most worth copying (long generated
configs, multi-file snippets).
Measure the block's content height with the existing
useMeasuredHeight hook and repeat the same action row (copy, plus any
caller-supplied headerActions such as the edit pencil) in a footer
once the block exceeds that threshold. The row markup is factored into
a small CodeBlockActions component shared by both call sites, so the
header and footer duplicate stay visually identical without a
copy-pasted JSX block.
Review findings addressed across this PR:
- max-two-buttons-per-row (blocking): the header's Run + Edit + Copy is
pre-existing (legacy status), but the footer is a NEW row, so
mirroring the header there put 3 siblings in a fresh row. CodeBlock
now takes a separate optional footerActions prop (defaults to
headerActions); EditableCodeBlock passes a trimmed Edit-only set to
the footer, keeping Run header-only.
- Footer copy reported success even when the clipboard write failed
(errors-use-error-notice, blocking): copy() now awaits copyCode and
only confirms on a true result, matching the established pattern in
TailnetMobileCard.
- Footer-triggered edit collapsed the block out from under the click
(the max-h-[480px] editor replacing a much taller block with no
scroll compensation). Fixed with scrollIntoView, gated on the
wrapper's top actually being above the viewport so a mid-viewport
HEADER edit doesn't also yank the page up for no reason the click
gave it.
- Inclusive Language (woke): "grandfathered" -> "legacy status".
Also: the footer's border was always painted, leaving a permanently
empty strip under every tall block at rest. It's border-transparent
until the same hover/focus reveal that shows the buttons, so the row
still reserves its height but paints nothing until there's something
to show.
Screenshots captured from an isolated CodeBlock render (short block vs.
a 30-line tall block) with the action row hovered, committed under the
repo-root temp-screenshots/ convention.
Closes #8227
test(pod): paginable long-history seed scenario (Refs #4100) (#9415)
No shipped seed scenario could paginate. The slot-detail fast path
answers has_more=false unless a session has size-rotated archive/
segments, and rotation only fires once a transcript outgrows a 10 MiB
budget, so every scenario built from a handful of messages leaves "load
earlier" structurally unreachable and a paging trace cannot be performed
through the real routes.
sessions-long-history seeds that state: one pinned session whose oldest
60 rows sit below a real rotation boundary. The initial slot load returns
the 8 live rows with has_more=true and next_before=60, and paging with
that cursor returns rows out of the archive.
Both shipped files are real ConversationLog output. The segment is
_maybe_rotate output at the production budgets, and the live transcript
is compaction output holding rows append wrote, so the fixture cannot
encode a rotation shape the writer does not produce. Only the rotate
segment and the live transcript are kept; the ~10 MiB compact segment the
recipe produces is discarded, which is the state a home reaches once
archive retention reclaims it. That keeps the fixture at ~20 KB of
package data, inside the 64 KB per-fixture cap.
The contract test pins the design point through the real readers and
pins the shipped segment's header and row shape against a rotation
driven in-test, so a change to the rotation format turns red instead of
leaving the fixture describing a shape the product no longer writes.
feat(members): dock the chat SidePanel beside the DM thread, first tab Crew summary (#9437)
The Members page's right column was a closable DetailPanel drawer — a
hand-built lookalike of the chat page's right column. It is now the chat
page's own tabbed SidePanel, permanent on wide windows (no close control,
no header toggle), whose first tab is the member's Crew summary carrying
what the drawer showed (identity + status, counters, driving sessions,
auto patrol, recent activity, wake sources, configuration, memory note,
edit exits). The + menu is the chat panel's (Files / Artifacts / Terminal
/ Browser / Side chat …) against the member's DM slot; the strip is
bucketed per member so it follows the roster selection.
SidePanel gains `leadingTab` (a host-owned, non-closable, non-draggable
tab ahead of the pinned block whose body the host renders), an optional
`onClose` (absent ⇒ no close control, Escape inert) and `extraReserveW`
(a sibling column's live width kept clear on drag). `usePanelTabs` takes
`opts.leadingId` so a fresh strip opens on the leading tab and an emptied
strip falls back to it. File / artifact open and file save move out of
useChatPageResourcesController into the shared `usePanelDocumentActions`
so both hosts run one implementation. Narrow windows (roster + thread +
panel cannot seat) keep the panel as an overlay the header button opens.
Closes #9432
Review round 1 (GPT): no agent hand-off on the panel read-error notice (the ChatPane below holds the DM draft as unsaved local state); Side chat withheld on this page until its composer draft persists or SidePanel keeps its body mounted (MEMBERS_WITHHELD_VIEWS, test-pinned).
Review round 2 (GPT): the panel binds to the member slot record only from the CURRENT snapshot (slotsLoaded) and only when it is the member's own (mode === member), so a record left over across a reconnect can never root Files / Terminal in a pre-restart project (test-pinned).
Design round: the unconfirmed-window withheld set is derived from VIEW_DATA_SOURCE (every classified view + terminal + app) instead of enumerated, so a new ViewKind is withheld there by construction; test pins it against the classification.
Rebased over #8947 (Quote / Ask on selected text): the drawer-hosted Side Chat main added is replaced by the panel's own Side tab — `openMemberSideChat` focuses it (revealing the overlay on narrow windows) for the confirmed slot only — and Side chat is offered again, since #8947 moved its composer draft into the per-slot chat-core store (the unmount-loss reason it was withheld for is gone). MembersPage.sideChat.test.tsx rewritten for the panel.
Fix-up: the Side tab is a dynamic tab (title as text, no aria-label) so the sideChat tests read it by accessible name; the drawer-only `side_chat_draft_waiting` key is removed from every catalog (dead-keys ratchet).
docs(spec): one paradigm for every monitoring loop (#9368)
Adds the umbrella spec the two existing monitoring specs sit under, so a
new monitored kind has a contract to implement rather than an existing
watch to copy.
Seven layers, one owner each, and six of them never learn what is being
watched. The consequential contracts:
- The probe signature is PLURAL from day one. A per-subject interface
cannot be batched later without changing every implementation and every
caller, and the difference is roughly 150 process invocations against
one query for fifty subjects. A probe that cannot batch loops internally
so the caller never encodes the difference.
- An observation is a NAMED entry with a severity and a reset scope, never
a bare fingerprint. A hash cannot be deduplicated per condition,
coalesced with a sibling, or re-asserted, because nothing can tell
whether two hashes describe the same condition.
- The decision layer is a pure function whose clock arrives as a value,
and it is level-triggered. Edge triggering loses any condition that
stayed true across a wake that did not happen. The re-alert window makes
that affordable and the budget makes it safe: a notification pipeline
aimed at humans needs no token budget because a paged human self-limits.
- Persisted state holds delivery bookkeeping only, versioned with a
migration per bump. Subject state belongs in the disposable evidence
file.
- An out-of-session driver is a detector, never a reactor. A cron turn has
no owning slot, so its tool calls hit deny-by-default and time out while
the job still records healthy.
Also records the rules that must be enforced by code rather than by prose,
including one the two current implementations already disagree on: the
status tool collapses superseded check attempts to the newest per identity
while the structured provider treats each row independently and maps
CANCELLED to failed, so it can wake on a failure that no longer exists.
Status is per layer, verified at 2f9ed9724, because most of this is the
target rather than a description of what runs today.
fix(dashboard): prevent empty mobile dialog (#9733)
refactor(apps): drop a shadow import and correct backend.py's comments (#9751)
`_capped_spill` re-imported `threading` inside the function, which the
module already imports unconditionally at line 24. The inner statement
re-bound the same `sys.modules` singleton, so removing it changes nothing
at runtime and removes the false hint that this function needs its own
import. No test patches `threading` on this module, so the local-to-global
name promotion is unobservable.
Two provably-equivalent structural edits alongside it:
- `_await_inflight_spawn`'s tail tested `cur is not None` twice and
re-read `.starting` to derive what the preceding arm's negation already
proved. It now returns early on `cur is None`, so each later test reads
once. `.starting` is written only at construction, and both reads were
inside the same `_lock` hold, so the single read cannot differ.
- `_start_app_backend_body` guarded `manifest.backend` against None when
computing `backend_type`, having already dereferenced
`manifest.backend.entryPoint` unguarded 326 lines earlier in the same
function. `AppManifest.backend` is a non-Optional `BackendConfig` with
no `__bool__`, so the else arm was unreachable.
The rest is comment hygiene in the same file, per AGENTS.md. A block above
`_PID_ANCESTRY_MAX_DEPTH` described "consecutive alive polls" that no
constant holds and `_survived_spawn` does not count; it is gone, and the
surviving block above those constants now states when a healthy backend
actually pays the full window (only where listener ownership cannot be
proven) instead of claiming it never does. Five docstring passages
narrated superseded code and now state current behaviour in present tense,
keeping the rationale. Two named the wrong thing: `_wait_for_pids` cited
`kill_pid` where the caller uses `kill_pid_pinned`, and
`_start_backends_concurrently` claimed boot costs one grace window
regardless of app count, which ignores the `_BOOT_SPAWN_MAX_WORKERS` cap.
`comment-history-baseline.json` drops this file from 7 markers to 6, which
`check_comment_history.py --write-baseline` records; the gate is a downward
ratchet and refuses a diff that retires a marker without lowering it. The
six remaining hits are present-tense statements about runtime state ("a
record that is no longer the tracked one"), not change history, so they
stay listed.
fix(chat): route a rejected 'auto' model to the picker and the default-model setting (#9099)
A partition that does not serve the `auto` sentinel rejects every session that
starts on it with "Your account does not have access to model 'auto'". The
generic entitlement message then advised "set agent.model to 'auto'", which is
the value that just failed, and the row offered Continue, which replays the
identical rejection. Reported from a region whose account is served only
gpt/deepseek/minimax/glm/qwen models (0.4.1).
- `_format_acp_error`: when the rejected id IS `auto`, say the automatic choice
is not offered in this region and route the user to the session model picker
and to Settings -> Chat -> Default Model (plus any per-agent pin). The non-auto
branch keeps its wording.
- `chat_runner`: the terminal error row carries `meta.kind = model_unentitled`
with `rejected_model` and `advertised`, decided from the exception's tags
(same evidence the formatter used), never from the prose.
- `ErrorCard`: a `model_unentitled` row offers "Choose a model" (opens the
composer's model picker) and "Change default model" (deep link to the
chat.default-model setting) instead of Continue. Panes without a picker render
the row as prose. Strings added to en.json + pseudolocale.
No automatic model substitution: the user picks, and the wire is unchanged.
Tests: new test_model_unentitled_meta.py (6), formatter wording test
(mutation-verified: fails on the pre-change formatter), ErrorCard tests (3).
Co-authored-by: Zejiang Guo <zejiangg@amazon.com>
feat: security-conductor golden-path corpus and verify_fix (#9500)
A security fix that makes the proof of concept stop reproducing has done
half a job. The other half is the half a security change actually gets
rejected for: the deny fence grows a rule that also refuses `gh pr view
--json`, or a guard is written against one host's spelling of a path, and
the tool the fix protected becomes one nobody can use.
This adds the corpus of legitimate operations that must survive a fix, and
the gate that re-checks them.
- ledger.py gains a fifth table, `golden_paths`, behind an additive
migration: every DDL statement is CREATE ... IF NOT EXISTS, so an
existing v1 database gains the table on the same pass and opens, and the
version bump is the last write -- an interrupted upgrade leaves the
version behind the tables, which retries, rather than ahead of them. The
bump is an UPDATE (the version row is a singleton) guarded by `<`, so a
database written by a newer checkout is left alone rather than walked
backwards. Every existing function signature is unchanged.
A corpus import is ONE transaction, so an interrupted one commits
nothing: a subset is a fence nobody chose and a silent one, since the
rows that never landed are invisible. That is why the import does not
call `add_golden_path` in a loop -- that function owns a transaction per
row -- and why the INSERT is split into a helper owning none.
`source_finding_id` rejects a bool explicitly, because `bool` subclasses
`int` and SQLite stores `true` as 1, which would silently attribute a
golden path to finding 1. Every text field that is present must BE a
JSON string: a list, number or null is refused, never str()-coerced or
defaulted, because "['gh', 'pr']" stored as a golden path makes the gate
classify a command nobody runs, and "platform": null widened to "any"
would gate every host with a row meant for one.
Rows carry the same propose/approve split as lessons, and approval is
write-once so the reviewer who admitted an operation stays on the
record. The table is the EDITING SURFACE, not the gate: an audit
proposes a row, a human approves it, and it reaches the gate when it
lands in the committed export through review. The CLI has three verbs
-- propose, approve, import -- mirroring the propose/approve pair the
RFC gives lessons and the committed-file relationship it gives
rules-of-engagement.json; no verb records one active, attributed row
from the command line, since that would be the approval without its
write-once record. There is deliberately NO
CLI verb that retires a golden path: the RFC gates deactivation exactly
like activation, and the flip stays the same human row edit rules and
lessons already use. Nothing reads the table as a gate, so the ledger
ships no host-filtered reader for it.
- verify_fix.py is the two-step gate. Step 1 runs verify_finding.py by
path -- never a copy of its judgement -- and requires `rejected`. Step 2
classifies every `shell` row of the COMMITTED golden-paths.json with the
tool gate's WHOLE deny composite -- the four checks hooks.on_tool_call applies to a shell
command, in its order (path fence, sensitive-command tier, exfiltration
auditor, deny-rule catalog), the same tier table scripts/deny_diff.py
declares -- and names the tier that refused. Measuring the catalog alone
would go green on a fix that tightened any of the other three. A tree
missing a tier is coverage lost and reported unavailable, not skipped.
A corpus file that is absent, does not load, or holds no row is 20, not
a pass: zero rows checked would fold to holds by construction, which is
the vacuous green a broken installation would report on every fix --
and the refusal scripts/deny_diff.py already makes of an empty corpus.
The gate reads the committed file and NOT the ledger's table, as the RFC
rules ("both gates read the file and nothing else"). The ledger is
per-host and invisible to CI, so a gate that read it could not be
enforced where it is declared blocking; and the table is mutable by
anything that can reach the database, so a gate that read it could be
steered by a row flip -- retire the one row the fix broke, and the gate
goes green with the fix unchanged. The file is read from beside the
skill, not from the worktree under review, so the change being judged
cannot rewrite the gate that judges it. And no ARGUMENT can shrink the
corpus: there is no flag naming another corpus file or another
platform -- the file is the one beside the script and the platform is
the host's own -- because a caller who could name either could name a
smaller check. Rows are named by their position in the file. The file
is validated by ledger.py's own loader, the same check
import-golden-paths applies, so the gate and the import cannot judge one
row by two rules -- and the loader accepts ONE shape, the object the
export is written in, so a second shape nothing writes is not a corpus.
The sibling ships in this same bundle, so an absent one is a broken
installation rather than a state to accommodate: still exit 20, because
a proof that was not re-run says nothing about the fix, and checked here
rather than left to the spawn, since the interpreter exists and a
nonexistent script argument makes the child exit 2 -- the verifier's own
code for "I rejected your input". The sibling `ledger.py` is the same
class: one that is absent or will not import is exit 20 through the same
JSON payload, never a traceback and exit 1, which is no verdict in the
contract. One printer, `emit()`, owns the payload shape and the stderr
lines for every path out of `main()`. Step 2 re-classifies every `shell`
golden path in the export whose platform matches the host against the
FIXED code; none may be refused.
NOTHING out of the corpus is executed, and that rule decides the design.
A row is text in a JSON file, and the same text once imported sits in a
table whose CLI ledger.py states plainly is not an authentication
boundary -- `--approved-by` is an unverified caller assertion -- so a
row is untrusted text written by whoever could edit the file or reach
the database. Every other consumer only READS such a row; running one as
argv would turn a file edit into command execution with the operator's
access, which no containment fixes because the escalation is in treating
the row as permission. So `shell` rows are CHECKED (classified, never run) and
`flow`/`cron` rows are RECORDED and reported for a human to exercise:
an MCP tool has no command line, a chat turn needs a gateway, and firing
a schedule has effects no deadline bounds. Parsing a schedule here would
settle nothing either -- a parse in this process never consults the
fixed code, so its answer is a constant no fix can change -- so the
corpus's own well-formedness is asserted in the test suite, at review
time, where a corpus-authoring mistake belongs.
Exit codes are the interface -- 0 holds, 10 the proof still reproduces,
30 a golden path is refused, 20 unverifiable -- and precedence is
10 > 30 > 20 > 0. The load-bearing property is the last one: 0 is
unreachable while any check this script OWNS went unsettled, because an
unclassified golden path is not a permitted one. A human row never moves
the exit code, because it was never claimed as this script's check.
The fence is read in a CHILD process with the worktree's own `src`
leading PYTHONPATH, because the point is to classify against the FIXED
code: an in-process import would bind whatever copy of the package the
interpreter already loaded, which for a test runner inside the
repository is the unfixed one. A leading PYTHONPATH entry is a
preference, not a guarantee -- a checkout without kiro_crew/security
would import the INSTALLED package -- so the probe proves where
kiro_crew.security came from and reports the composite unavailable
(exit 20) unless that file resolves beneath the worktree's src. A fence
borrowed from somewhere else is not a fence that agreed. The probe's payload is shape-checked
before any field is read off it, so a malformed answer is 20 rather than
a traceback exiting outside the contract.
Two containment details are deliberate. The ledger path is resolved ONCE
and passed to the verifier child explicitly -- the only thing this
script wants the ledger for: the child's HOME is the worktree and the
ledger's default path is HOME-relative, so a child left to its own
default would read a different database. And the deadline kills the
direct child only, matching verify_finding: a process-tree kill has no
portable spelling in the standard library, and a verifier whose own
teardown works on one host would be the platform lock-in this corpus
exists to catch. `--worktree` must be a git checkout, which is the
blast-radius bound the re-run proof depends on.
- golden-paths.json is the committed export the gate reads: 37 rows, each
with a reason. scripts/deny_diff.py's usage line and the fixture's
_comment name this file and the adopting change that retires the
fixture (#9678); wiring the gate into the fixer lane is #9707. The rows
cover read-only gh, git read/write/publish, the sanctioned test
invocation in both host spellings, the lint and type gates, the publish
preflight, the monitor-loop MCP tools, chat-start-one-turn via the
offline stub-ACP harness, and three shipped cron shapes. Two rows are
the read-only gh shapes that were false-positive refused while this was
being designed: two `gh pr view --json` calls joined by `;`, and a PR
read combined with a run list.
Tests
- test_security_conductor_verify_fix.py (73): the verdict ladder end to
end, driven as a subprocess with siblings staged beside it and the
corpus written one directory up, so the absent-verifier and absent-corpus
cases are testable at all; that the gate reads the committed file and
not the ledger, from both sides (a row only in the ledger does not gate;
a row retired in the ledger still does) and from the worktree (a
checkout shipping an emptier corpus is still judged by the skill's own;
no flag names another corpus or the other host); that a worktree without
the package does not borrow the installed fence, and that a fence under a
symlinked src is still the worktree's; that no corpus row is executed,
proved by a witness file each row would create if it ran, plus a
structural guard that the golden-path walk carries no spawn at all and
only two call sites spawn anything; the platform filter in both
directions; seven malformed probe payloads each pinned to the specific
guard it trips; the ledger path handed to the child; and one class that
asks the REAL `is_denied` about every shipped shell row, which turns the
corpus into a live regression gate at PR time.
- test_security_conductor_ledger.py: the new table's columns, its identity
(kind + command + platform, each part proved load-bearing), write-once
approval, retirement keeping its approver, corpus import that is
all-or-nothing on both validation and interruption, a racing identity
collision counted as skipped rather than fatal, and the v1 migration.
- Smoke-tested end to end against the real sibling verifier and the real
deny fence, with no stubs: a finding whose PoC no longer reproduces plus
a permitted `gh pr view --json; gh run list` row returns 0 and reports
the flow and cron rows for a human, and adding one genuinely refused row
returns 30 naming the rule that ate it.
- Red-before proven by reverting forty guards one at a time, including
the single accepted corpus shape, the string-typed text fields,
the file-not-ledger read, the skill's-own-copy read, the absence of a
corpus or platform flag, the fence-provenance check and its prefix
boundary, the absence of a one-step add verb, the unreadable-,
unloadable- and empty-corpus verdicts, the four-tier composite, the missing-tier
verdict, the absence of a retirement verb,
both exit-30 and exit-20 paths, the human-corpus separation, the
structural no-spawn guard, the platform filter, the migration bump, the
single import transaction, the bool rejection, and the live-fence gate;
every one turns its test red.
Refs #9195, #9332
Co-authored-by: Joe Guo <zejiangg@amazon.com>
fix(issue-radar): send Content-Type with glab api bodies (#9739)
glab api pipes JSON write bodies over stdin with --input - but sets no
Content-Type header. Strict self-hosted GitLab instances reject such a
request with HTTP 415 Unsupported Media Type, so Issue Radar write
operations (label add/remove, state changes) fail there. Add
--header "Content-Type: application/json" to the glab argv whenever a
body is present. Read paths without a body are unchanged, and the
GitHub transport is untouched.
Closes #9723
fix(work-ledger): retry a contended record read; make a test's ids salt-free (#9673)
Two reds on `Backend Tests (Windows) (4)`. Only one is a Windows fact.
`work_ledger._read_json_record` reads with a bare `read_text`, and a STRICT read
there is lock-free across writers: `_refuse_if_worker_holds_open_item` reads the
prior item under the WORKER's binding lock while that item's own conductor may be
replacing it under a different item lock. On Windows a read of a file another
handle holds open for write raises `PermissionError`, so one correct concurrent
writer turns a strict read into a bare `OSError`. The guard catches only
`WorkLedgerError`, so it escapes `apply_conductor_action`, and the dashboard route
maps `OSError` to a 503 `ledger_write_failed` "try again" -- telling a conductor to
retry a binding that is legitimately taken until the item closes. A permanent
refusal reported as transient is a product defect, so the fix is in the store:
route the read through `atomic_write.read_bytes_with_retry`, the read-side twin of
`replace_with_retry` that `members.py` already uses for this exact window. A
`PermissionError` that outlives the bounded retry still escapes, so the
fail-closed contract holds; on POSIX the helper re-raises on the first attempt.
The second red is not Windows-specific at all.
`test_reconcile_and_publish._finding` mints each PR url from
`abs(hash(fp)) % 9000 + 1`. `hash()` on a str is salted per interpreter process,
so roughly one process in 9000 gives two fingerprints the same url; the sweep then
skips both and the test reads `assert [] == ['aa']`. Each xdist shard is its own
process drawing its own salt, which is the whole reason it looks like one shard is
special. Reproduced on Linux with `PYTHONHASHSEED=17494`, where `aa` and `bb` both
map to `pull/1344`. A sha256 digest is salt-free, so the mapping is identical in
every process.
Tests: a new `test_a_contended_item_read_still_refuses_with_already_bound` drives
the guard through `windows_sim.read_sharing_violation` and asserts the refusal
stays `already_bound`; reverting only the retry line makes it fail with the
injected `PermissionError`. The four existing fail-closed tests move their fault
from `Path.read_text` to `Path.read_bytes` through one shared `_fault_record_read`
helper, since that is the call the record reader makes; their assertions are
unchanged and still pass on POSIX, where the helper does not retry.
feat(dashboard): add top-level /sessions page — bookmarkable session chooser (#6737)
A neutral, bookmarkable session list at /sessions inside the main dashboard
shell (nav rail + top bar stay available). Rows navigate to the full
/chat/<key> experience; unlike bare /chat the page never auto-selects or
auto-creates a session. Full-bleed rows on phone; schedule-style inset with
bordered group cards on desktop. Filter chips (All / Unread / status tags in
use), search, recency groups (Today / Yesterday / Earlier), New session.
Original work by Helena Stafford (@helenastafford).
Co-authored-by: Kiro Crew <noreply@kirodotdev.github.io>
feat: preserve opaque inbound attachments (#3754)
Video and unrecognized formats were rejected before download, so an inbound
file the agent could have acted on arrived as a note instead of bytes. Keep
them as byte-identical temporary files under a 50 MB cap, hand the agent the
local path plus the original name, type and size, and reuse the existing
per-turn cleanup ownership. Opaque bytes are never parsed or executed
automatically.
A sender picks filename and mimetype independently, so an opaque file could
arrive named "photo.png" while declaring application/octet-stream. The ACP
encoder types a prompt path by suffix alone, so such a path would reach the
image sink without the content-signature check the IMAGE branch enforces --
emitted as image/png on the strength of a sender-supplied name. An inlineable
image suffix is therefore stripped from the temporary path before ownership
transfers, mirroring the retype the IMAGE branch already performs, and a
rename that fails drops the attachment rather than emitting the original path.
A contract test pins the suffix set against the encoder's own table.
Co-authored-by: Jindong Hu <hjindong@amazon.com>
fix(acp): verify the agent the --agent flag selected actually loaded (#9668)
Guard (A) in create_session fails closed when a requested agent is absent
from the session's advertised modes, so the session never silently runs the
backend's own default in its place. But it only ever inspects the agent that
set_mode will activate -- an explicit override, or on KAS the injected
default. On kiro-cli, mode_agent is None for an ordinary session, so the
agent actually chosen by the --agent spawn flag reaches no check at all.
load_session has the same shape: its check reads `agent`, and its only
caller passes `agent=agent or None`.
That agent's spec can fail to load silently. A live probe of kiro-cli
settles what the session/new response says in each case: a spec that loads
is reported as `currentModeId` AND listed in `availableModes`, while a spec
the backend refuses is absent from the list and `currentModeId` names
kiro-cli's own default instead. The refusal is silent -- no JSON-RPC error,
a normal sessionId, the process runs on. Nothing downstream notices: the
set_mode response is never read back, currentModeId is never re-compared,
and mcp_session_report only logs (its docstring forbids reading a missing
report as "not mounted").
The user-visible result is a session with none of Kiro Crew's control plane,
while the global provider mcp.json -- which Kiro Crew pins off only on specs
it writes itself -- stays merged. So a third-party MCP server declared there
keeps working while every Kiro Crew tool the injected prompt names answers
"A tool with the name '<tool>' does not exist". For learn_add that reads to
the user as the agent reporting its memory is unavailable and the lesson was
not saved, when in fact the whole server is absent.
Guard (A2) applies the same check to the spawn-flag agent on both
session-start paths, before set_mode, terminating the session it created so
the backend does not hold an orphan. `currentModeId` is read as proof in
both directions: naming the spawn agent admits, naming anything else fails
closed even when no advertised list came back. The compatibility escape is
narrow -- only a response that names NO current mode falls back to the
advertised list -- so older kiro-cli and the offline fake backend are
unaffected. Scoped to the backend whose argv carries --agent, by a positive
identity test (harness-parity H5).
Six regression tests, mutation-verified: neutering the guard fails the three
fail-closed cases, and admitting a current-mode mismatch fails exactly the
substituted-current-mode test.
Co-authored-by: Zejiang Guo <zejiangg@amazon.com>
feat(chat): Quote / Ask on selected text in ChatPane (Members + split view) (#8947)
Selecting text in a Crew Members thread (or a split-view pane) offered Copy
only: AssistantMessage's selection toolbar draws the actions its host hands
it, and ChatPage was the only host passing onQuote/onAsk — quote's
FlyingQuote-into-composer and ask's open-/side-and-seed were ChatPage-local.
Extract both into one chat-core seam, chat-core/composer/selectionActions
(useSelectionQuoteAsk + quoteIntoDraft + seedSideChat), and make ChatPage and
ChatPane both consume it — no second implementation. Hosts differ only in
what they own: the composer draft, and how a Side Chat surface for a slot is
brought on screen (openSideChat). Quote is always offered; Ask exactly when
the host provides an opener (capability by omission, like onOpenFull).
- app-sdk: ChatMessageList/MessageRenderContext gain onQuote/onAsk; the
default assistant row passes them through.
- ChatPane: wires the seam to its own composer (FlyingQuote lands in the
pane) and takes an openSideChat prop.
- Split view: SessionGridView threads ChatPage's opener, which re-binds the
activity panel to the pane's slot (switchSlot) before opening the side tab.
- Members page: the detail drawer gains a Side Chat view bound to the
member's slot (reuses existing "Side Chat"/"Details" strings — no new copy).
- SideChat: the side-seed event now names its slot; a SideChat bound to
another slot ignores it, and the seed poll waits for the named slot's
composer (data-side-chat-slot) rather than any Side Chat's.
- ChatPage: only change is consuming the hook (behaviour unchanged) plus the
one-prop grid wiring.
Tests (mutation-oriented pins): selectionActions unit tests, ChatPane
selection-actions test, MembersPage Side Chat drawer test, SideChat
slot-aware seed, ChatPage Ask seed regression. Capture harness
website/scripts/capture-members-selection-quote-ask.mjs with three
asserted frames under temp-screenshots/members-selection-quote-ask/.
refactor(chat-core): route the scene popover and feedback sends through sendTurn; drop api.steerChat (#9593)
Chat-core RFC P2, batch C of #9570. The last two hand-rolled dashboard
senders call the transport's `sendTurn` and branch on its receipt, so
the deadline and the six-way classification are the core's:
- `hooks/useSceneInteraction.tsx` (scene agent popover composer): the
running/idle fork -- `api.steerChat` mid-turn, `api.sendChat` plus a
local `readSendReceipt` otherwise -- becomes ONE call,
`sendTurn({ message, slot, steer: !!src?.running })`; `steer` is a flag
of the same endpoint (#8689), not a different receipt shape.
refused -> report on the composer, hand the payload back
transport-error -> report on the composer, hand the payload back
unknown -> composer back to idle, nothing claimed (as before)
response-late -> report on the composer with the core's
delivery-unconfirmed copy, hand the payload back
(NEW: the old path awaited without bound; nothing
proves the gateway saw the request, so silence would
discard the text -- same policy as ChatPage)
dispatched/queued -> 'sent' tick + mini-thread echo, as before
A refused steer used to surface through the outer catch (steerChat
parsed via `j` and threw on a non-2xx); it is now a `refused` receipt
with the same user-visible result, and a `{ok:false}` steer inside a
200 -- which `j` passed through silently -- is now reported too.
- `App.tsx` feature-request send: `refused` -> error row with the
server's reason; `transport-error` -> error row; `unknown` and
`response-late` stay silent (the request was or may have been
accepted; the row is the pill's only signal).
- The composer's failed-send line migrates from a hand-written
`role="status"` div to `ErrorNotice variant="inline"` (same string),
`askAgent` off with a No hand-off comment naming the restored draft
(errors-use-error-notice: this diff touches its condition).
- `api.steerChat` had no remaining caller and is deleted; `sendChat`'s
comment now describes the steer flag it carries for the transport, and
`sendChat` runs `checkSessionExpired` on its raw response so a 403
auth challenge on any transport send keeps the silent-refresh / banner
recovery the `j`-parsed steer helper had.
Tests: the scene coverage file mocks the transport's wire (`api.sendChat`)
so the REAL classifier runs; it asserts `steer` is forwarded from the
running state, adds a refused-steer case and a deadline (`response-late`)
case. The feature-request test pins that the send rides the transport
(deadline signal on the wire). Dead `steerChat` stubs removed from four
test files. RFC 4.2 row split into the three #9570 batches.
Refs #9570
test: third side-effect audit (macOS): undo-proof floor, PTY deadlock (#9663)
Five full backend runs (92,261 tests each) and five full frontend runs on a
macOS host, from inside a Kiro Crew agent session, with an audit hook that
attributes every host write, spawn, connect and kill to the test that made
it and a per-test census of duration, RSS, threads and descriptors. Zero
backend flakes in 5 x 92k; two frontend flakes; 227 deterministic failures
and four 120-second hangs that every Linux run had been blind to; and host
writes the earlier audits could not see. Every fix carries a test that goes
red when it is reverted, and the classes are written into
testing-conventions "Side effects" so they cannot recur unnoticed.
Host state the suite touched, and what pins it now:
- monkeypatch.undo() in a test body unwound the whole floor (KIROCREW_HOME,
KIROCREW_PROFILE, KIROCREW_TELEMETRY, the service guard). #9614 moved the
pins onto the private _floor_monkeypatch; this adds the missing half: the
monkeypatch fixture is re-declared at the rootdir to depend on it, so the
floor is set up first and torn down last by construction rather than by
autouse ordering. test/conftest.py's autouse pins ride the same instance.
- A metric emitted at IMPORT (ToolHookResult.allow() as a default argument)
built the recorder from the operator's real config before any pin
existed and exported to the real ~/.kiro/crew/metrics every minute for
the life of each worker. KIROCREW_TELEMETRY=0 is now pinned for the
whole process in pytest_configure, and every module whose collection
builds the recorder is recorded and asserted empty.
- The mc-maint sweep resolved config_dir() when it RAN, after the queuing
test's pin was gone: 60+ mkdirs of the real home per run, plus a rmtree
and a marker aimed at it. cleanup_stale_sandbox_profiles takes the home
its caller resolved, with no default that could reopen the on-pool
resolution; SessionManager resolves it once at construction.
- Five tests dropped the override to test the default home and let
config_dir() create the real ~/.kiro/crew, and a resolver-only stand-in
still rewrote the real ~/.kirocrew.breadcrumb beside it. The teardown
hook reads paths._resolved_home before any fixture unwinds and fails the
test after the floor has torn down; the floor wraps the breadcrumb writer
to refuse while Path.home() is the operator's; each test fakes the host
home instead of only the resolver. A class- or module-scoped setup runs
outside the floor and now pins what it resolves itself.
- Fifteen __pycache__ trees per run from imports by path: sys.pycache_prefix
and PYTHONPYCACHEPREFIX point at ~/.cache/kirocrew/pycache.
- KIROCREW_PROFILE=amazon inherited from the agent session failed 150+
builtin-app tests closed because the pin lived in test/conftest.py,
which those testpaths never see. It is a rootdir floor now.
macOS, where the Linux-green suite had 227 failures and four hangs:
- _kill_session closed the PTY master before ending the shell; on
macOS/BSD close() waits for the reader's outstanding read, so four
terminal tests hit the 120 s timeout on every run. The process tree is
hung up (SIGHUP, the one signal an interactive shell honours) and
terminated before the master is closed; the tests run in under a second.
- Linux-shaped tests, one rule each: the frame recorder's 75 logic tests
pin its Linux-only ACL gate open; O_TMPFILE prompt tests skip on the
production capability flag; /dev/fd/N is compared by open+fstat identity;
unlink() on a directory is EPERM here, so _discard_untracked_files
recognises it; a simulated O_BINARY bit is derived from the live O_*
constants (landed in #9632); a --copies venv cannot relocate a
non-framework shared-lib CPython; the darwin workspace binding's pass_fds
entry is pinned away in the snapshot-descriptor test; a pinned
sys.platform needs a pinned start id; and codex-review.yml's two sed
expressions become one 1,8{} block, because BSD sed opens a numeric range
only on its exact line.
- test_handlers_system_macos_paths reached 8.8.8.8:80 through _local_ip();
stubbed at the seam the Linux siblings already use.
Frontend: AutoNudgePopover's watch list is three promise hops past
act(render), so the positive test flaked and every negative was vacuous;
the block now waits for the fetch to be answered. App's startup-video gate
read an interruption latch that an effect sets a commit after the
changelog shows, while changelogDecided lands a microtask later; the gate
reads the live conditions in the same commit too.
feat(dashboard): show a pre-boot failure panel, and gate chunk cycles in CI (#9656)
Two halves of one problem: a page that cannot boot should say so, and the one
config-reachable cause of a dead boot should not be able to ship.
VISIBLE FAILURE (closes #9543)
When a boot-critical module never arrives, no app code runs. No React error
boundary exists yet, and src/lib/staleShellHeal.ts cannot help either - it is a
boot-TIME probe, so it needs the boot it is meant to rescue. A top-level page in
that state showed index.html's dark empty shell and nothing else, which is the
mobile black screen behind #9518 and #9540.
index.html already detected the failure: a capture-phase error listener logs
"[boot] module script failed to load" and posts mc-embedded-boot stage=script-error
to the parent. But it returned early unless embedded, so a top-level page - the
phone - got nothing. Extend that listener rather than adding a second detector.
The panel is shell-owned and inline: no stylesheet, no bundle, no i18n, because
none of those loaded. "Did we boot" is answered by whether #root has children,
which needs no new signal from main.tsx - an entry chunk that failed leaves it
empty, and a LAZY chunk that failed after render leaves it populated, in which
case the running app owns that error and must not be covered. The reveal waits 4s
so it cannot fire while the service worker's own bounded retry is still winning.
Retry unregisters the worker and drops CacheStorage before reloading, because a
plain reload does not bypass the worker on iOS Safari - that is the escape hatch
users currently reach only by clearing site data by hand.
CYCLE GATE
Two chunks that statically import each other have no valid initialization order:
one body runs against a binding that is still uninitialized, and in this app that
lands on new QueryClient(...) throwing before React mounts - the same blank page,
from a different cause. Nothing in the pipeline could see it. check-bundle-size.mjs
measures bytes, tsc and eslint never look at the bundle, and the unit suite does
not load one. It is reachable from a config edit alone: rolldown's
includeDependenciesRecursively: false produced two cycles on a tree that had none
- a 71-chunk one spanning App, client, vendor-react and vendor-icons, and a
3-chunk one across the graph chunks - with every other gate green.
bundleReport.mjs now records each chunk's static imports, as the bundler resolved
them rather than by parsing minified output. dynamicImports is excluded: a dynamic
edge defers execution and cannot produce the failure. findChunkCycles reports
strongly connected components as data, matching checkChunkBudgets, and refuses a
report with no imports field instead of reading "no edges" as "no cycles".
check-chunk-cycles.mjs is the CLI.
It runs as a second step in the existing Bundle Size Gate job, reusing that job's
analyze build rather than paying for another one. The job keeps its name: it is a
required check, and renaming it to match the widened scope would silently stop
satisfying branch protection. The prepare-pr floor entry gains the same command,
so the profile still mirrors ci.yml.
Verified: chunk-cycle gate reports 675 chunks, 2058 static edges, no cycles on
this base; bundle-size gate still 675 within budget; i18n:render passes, which is
what proves the index.html edits did not break Vite's import-map relocation; 25
prepare-pr floor tests pass including test_ci_blocking_scans_are_covered_by_the_floor.
Co-authored-by: Bolin_Chen <17506219+bobbyfine@users.noreply.github.com>
fix(eval): save the eval report as utf-8, not the host code page (#9726)
format_results puts a ✅ or ❌ on every scenario, session, turn and
assertion, so the report `kirocrew eval` saves is never ASCII. It was
written with Path.write_text and no encoding=, which encodes with the
host's locale codec — cp1252, cp950, cp932 on Windows — and raises
UnicodeEncodeError on the first glyph.
The raise lands after the whole eval has run and takes the JSON summary
with it, so a run that printed its report to the terminal saves neither
artifact. The turn snippets the report embeds carry arbitrary agent text
as well.
The two writes move into write_eval_artifacts so the contract can be
exercised in a child process: the codec open() defaults to is fixed at
interpreter start and cannot be substituted in-process.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
fix(meetings): match a dictionary alias that starts or ends with punctuation (#9717)
Every alias was compiled as \b<alias>\b. \b asserts that a word character sits
on exactly one side of the position, so it delimits an alias only where the
alias's own edge character is itself a word character. On an alias edged with
punctuation it asserts the opposite: \b\.net\b requires a word character before
the dot, so the standalone ".net" the speaker said was skipped and "asp.net"
was rewritten instead.
"c++", "c#" and ".net" are ordinary entries in a speech-correction dictionary.
They were accepted by add_term, persisted, and listed in the editor as active
terms while correcting nothing — and silently rewriting text they did not name.
Each edge now takes \b when the alias's own character there is a word character
and a lookaround for a neighbouring word character when it is not. The two are
equivalent on a word-character edge, so an alphanumeric alias keeps exactly the
pattern it had.
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
fix(ci): diff base to working tree for added lines in merge-ref ratchets (#9734)
The merge-ref gates (comment-history, subprocess-encoding, sync-io,
agent-sdk-boundary) scan violations from the working tree but took their
added-line numbers from git diff <base>...HEAD, which cannot see
uncommitted edits. On a dirty tree a pre-existing baselined line shifted
by an uncommitted insert could land on a line number the HEAD diff
counts as added, failing the gate locally on bytes that pass once
committed. added_lines() now diffs merge-base(<base>, HEAD) to the
working tree for the three-dot label, matching added_lines_at() and the
file state the gates actually scan. Clean trees (CI) are unaffected: the
two diffs are identical there. The merge-shape labels keep their
committed endpoints.
Closes #9719
fix(segmented-control): stop the active pill from tracking a mid-animation box (#9715)
The active-pill indicator is a layoutId motion.div, absolute inset-0 inside a
segment motion.button that carried layout. The segment label animates its width
0 -> auto on reveal (compact / iconOnly), growing the button box every frame, so
the shared-layout indicator was sized to an intermediate box instead of the
settled one (#9684).
Fix, both halves needed (measured 0px divergence together, ~6px each alone):
- drop layout from the button so it no longer re-projects its box every frame;
- give the indicator layout="position" so it springs only its cross-segment
travel and takes its size from CSS inset-0, matching the button box every
frame, settled or mid-reveal.
Closes #9684
docs(locks): correct the kept-inline lock-open comments (#9267) (#9661)
Two comment-only fixes to the sites #9647 left inline:
- session_pid._periodic_pid_sweep: delete the false clause claiming the
with-scoped helper "serves exclusive acquisition only". open_lock_file
only OPENS non-truncating; the lock mode is chosen at file_lock(fd,
exclusive=...), and this sweep acquires shared read twelve lines below.
The real reason it stays inline is fd lifetime -- the fd is held across
the try/finally, not a `with` block -- which the original clause already
stated; keep that, drop the added falsehood.
- _McpFileLockSync.__enter__: shrink to a pointer at _McpFileLock above,
whose kept-inline rationale already states the fd-lifetime reason fifty
lines up. Repeating it is the duplication this item exists to reduce.
No code change; the migrations #9647 landed are untouched.
Refs #9267
refactor(monitoring): type the probe boundary to a subject, not to GitHub (#9505)
Both Protocols at the external probe boundary -- `_Provider` in
monitoring/controller.py and `GitHubShadowProvider` in monitoring/shadow.py --
annotate their `probe` return as `GitHubPullRequestProbeResult`. That return type
is the defect. An abstraction typed to its one concrete implementation is not an
abstraction: a second monitored kind cannot satisfy either boundary whatever it
returns, because satisfying it requires producing a pull request's result type.
The two declarations being near-identical is a symptom of that, not the problem.
Introduce MonitorProbeResult in monitoring/models.py, naming no host: the
subject's canonical facts plus the generic observation the engine classifies.
GitHubPullRequestProbeResult becomes an implementation of it, keeping `response`
-- its own typed detail -- on itself rather than on the shared type. The two
Protocols collapse into one public MonitorProbe, and the service boundary
(`_Service.apply_monitor_probe`, AutoNudgeService.apply_monitor_probe) is retyped
to the shared record so no path names GitHub to describe a probe.
The signature is PLURAL from the start: it takes a sequence of subjects and
returns a mapping keyed by the subject string AS PASSED IN, not by any identity a
host derives from it -- a caller can only look up what it asked for. GitHub
answers for one pull request per call so its implementation loops, but arity is
the one thing that cannot be changed later without touching every implementation
and every caller, so it is settled now. Keying by the caller's own string is what
lets a host normalize a subject without reshaping the mapping its caller reads.
MonitorProbeResult is a RECORD rather than a bare sequence of per-check rows. A
host that publishes its own overall verdict, distinct from the rows a probe
enumerates, then has somewhere to put it as a defaulted field that reaches every
caller without changing this type or any signature naming it. No such field is
added here: nothing would read it, and this probe fetches no published aggregate
today, so populating one would mean a new request and a behaviour change.
A plural boundary lets a provider answer for a SUBSET of what it was asked, and
lets it answer with the wrong shape. Neither is a verdict: an absent subject
leaves no observation to decide from, and the decision engine reads attributes off
whatever it is handed, so an untyped value fails deep inside it rather than at the
boundary. Both consumers -- MonitorController.tick and run_shadow_probe -- resolve
through one shared `resolve_probe_result`, so the two cannot disagree about what
an unusable answer means; a guard in one and a bare KeyError in the other would be
the same hazard handled two ways.
Behaviour-preserving for a provider that answers its contract.
test_monitor_behaviour_golden.py's digest and all 28 of its per-group digests --
captured on kirocrew/main at 53987e756 before any edit in this stack -- are
unchanged across the boundary change.
feat(command-bar): file a contributed command's session in its own folder (#9573)
* feat(command-bar): file a contributed command's session in its own folder
A contributed row opens a NEW session every time it runs, and those sessions are
generated work rather than conversations the reader started. Left unfiled they
accumulate at the top level of the sidebar, push the reader's own chats down, and
interleave two different commands' runs with nothing separating them.
Each command now gets a folder of its own under one parent that says where all of
them came from: Command Bar Sessions / <the row's title>, created on first use.
Filed last and never awaited, so a folder API that is slow, capped or refused costs
the session its place in the sidebar and nothing else. Contributed rows only -- the
Ask row carries a sentence the reader wrote. Nothing is filed for a seed the reader
abandoned mid-create.
The folder list is READ from the sidebar's own ['chat-folders'] cache rather than
fetched: GET /api/chat/folders walks the on-disk session list synchronously to count
archived sessions per folder, so a fetch per run would pay for a filesystem scan that
scales with the reader's history to learn what is already cached. A cold cache falls
back to one fetch.
The parent name is a durable value the server stores and this code matches by name
later, so it is written as a literal with a scoped i18next/no-literal-string off block
in eslint.i18n.config.js, beside wireValues.ts's -- the same category, and where this
repository counts suppression.
The leaf name is clamped to the 100 characters chat_folders.py stores, because a
manifest title may be 120: without the clamp the create is silently shortened, the next
run's lookup for the full title misses, and every run makes another folder. When two
rows currently offered carry the same title, the contributing app's label is appended to
both, since a leaf keyed on the title alone would interleave two commands the reader
cannot tell apart.
* fix(command-bar): make the folder name safe under Unicode and guarantee a unique suffix
Two halves of one GPT finding, both real.
`slice` counts UTF-16 units, so a title whose 100th unit falls inside a surrogate pair
lost half a character and the stored name carried a lone surrogate -- an invalid string,
made durable. Truncation is by code point now, which is also what the server means:
Python slices its own strings by code point.
And the hash tier was only unlikely to collide, not unable to. The suffix now walks three
tiers -- readable tag, hash of the whole row id, hash plus the row's position in the
id-ordered colliding group -- each tried only when the one above fails to separate
anything, so uniqueness inside the group is guaranteed rather than probable.
feat(apps): default Library to enabled apps with a labelled show-all toggle (#9481)
Library listed every installed app, so the ~20 default-off builtins dominate the page a user visits to manage their own apps. Default the list to enabled apps and add a persisted, labelled 'Show N disabled' control that reveals the rest and switches back to 'Show enabled only'; the count tells an empty list apart from a filtered one.
The view participates in libraryView's per-visit 'listed' decision through the wasEnabled latch rather than filtering the rendered list: a row disabled mid-visit stays in place instead of vanishing under the cursor, while a never-enabled builtin is filtered. It is a filter, not a hide, so a default-off builtin with no Discover row stays enable-able. The empty enabled-only view keeps the reveal control so those builtins are never stranded.
Persisted with usePersistedBool under mc-apps-library-show-all (added to DURABLE_PREF_KEYS) so the choice survives reloads and follows the user across origins, like the other view toggles. New i18n keys across all catalogs plus the generated pseudolocale; plural base registered. Tests extended for the disable-then-stay sequence and the show-all reachability guard; the disabled-tile suites opt into the show-all view. Screenshot harness and two committed evidence images under temp-screenshots/.
Closes #9473
feat(diagnostics): add redacting kiro_cli_logs read tool (Refs #6023) (#9058)
Give the agent driving kiro-cli a sanctioned, redacted way to read
kiro-cli's own logs after a rejected turn. kiro-cli's data directory is
fenced by the command gate (it also holds SSO tokens) and that gate is
deliberately coarse and verb-independent, so naming a fenced path is
enough to end the turn. This is one reviewed read path instead of a
carve-out on a fail-safe gate.
Scope is the primary control, not redaction. The reader reads kiro-cli's
mcp/lsp protocol logs only. kiro-chat.log and the session transcripts are
each ONE fixed host path rather than a per-session file, so on a host
running one gateway with many concurrent sessions each interleaves every
session's traffic; both carry conversation prose, and the redaction stack
is a CREDENTIAL pass that does not narrow prose. collect_bundle still
includes the chat log: that path is user-to-user, this tool is
cross-session.
Measure the property that scope rests on, and enforce it. mcp/lsp logs are
safe to read here only while they record protocol traffic rather than frame
bodies, since they share the chat log's interleaved shape and an MCP
tools/call frame carries conversation-derived arguments. On kiro-cli 2.21.1
mcp.log is empty across a session of continuous MCP tool calls, every
lsp.log record is a single-line timestamped ERROR with no JSON-RPC envelope
and a longest line of 313 bytes, and sentinel strings passed as tool-call
arguments appear in neither file. Because that measures one version of a
component this repo does not pin, a source whose text carries serialized
frames is refused whole and visibly, so drift surfaces as a refusal rather
than a silent widening. The refusal decides only whether a file is the KIND
the tool assumes; it never claims to separate one session's frames from
another's, which is the pretend-scoped filter this module rejects.
Strip hidden characters before redacting, inside _scrub. redact_credentials
matches a secret's literal shape, so an invisible planted inside one
defeats it. Stripping afterwards is worse than not stripping, because the
strip REJOINS a credential that redaction was already asked about and
declined, and every consumer strips later: an MCP response goes through
validation.sanitize_response and a bundle member is normalized by whatever
reads the zip. strip_hidden_unicode states this ordering as its own
contract. Putting it in _scrub rather than the reader covers collect_bundle
by the same change.
Require the opened descriptor to be the file that was checked. lstat before
the open and fstat after must report the same st_dev/st_ino, refusing both
a final component that is a link and a swap landing between the two calls.
This is what defends Windows, where there is no O_NOFOLLOW and a junction
is a reparse point is_symlink does not report while satisfying S_ISREG.
Bound the read by max_bytes on every branch. size is an fstat snapshot, so
a file appended to faster than the loop drains never reaches EOF and a loop
bounded only by EOF grows until the process is OOM-killed.
Set O_NONBLOCK alongside O_NOFOLLOW. A validated regular file swapped for a
FIFO makes an O_RDONLY open block forever, because the S_ISREG guard only
sees a descriptor the open already returned.
Bound the whole response and trim from the FRONT. Every tool response
leaves through validation.build_tool_response, whose sanitize_response
truncates the TAIL -- for a log tail the worst end to lose, since the newest
lines are the reason the tool is called and the transport's marker reads as
if the oldest went. The reader keeps its output under a ceiling below the
transport's, divides that budget across sources, and labels which end it
dropped.
Return only the truncation marker when the tail window holds no newline. An
unterminated final line longer than the cap leaves no boundary to cut on,
and emitting that fragment strips the header token that redacts a value
further along the same line.
Bind a continuation line to the verdict of the event it belongs to, so an
event `since` rejects does not ship its payload without the header line
that identified it.
Keep one tail reader for the module, so the check-then-open guarantee
cannot be present in one path and absent in the other.
Start a cut window at a record boundary. The frame tripwire keys on a
frame's envelope tokens, which in a pretty-printed frame sit at the top,
while the byte cut runs from the front -- so an oversized multiline frame
can have its envelope fall outside the window while its trailing argument
lines survive, and those lines carry another session's conversation. A cut
window whose first line is not a timestamped record is sitting inside a
record whose beginning is gone, so those orphaned lines are discarded and
the window starts where a record does. A frame that starts INSIDE the
window still carries its envelope and the tripwire catches it, so the two
together cover both positions of the cut. This trims only a cut window, and
it lives in the agent-facing reader rather than the shared tail reader:
unlike the hidden-character strip it discards data, and the cross-session
boundary that justifies the loss is one collect_bundle does not cross.
Match the record prefix, not any leading digit, when finding that boundary.
A serialized frame can carry a JSON array of bare numbers, so a line like
" 1000000," begins with a digit and would pass as a record start --
ending the trim INSIDE the frame and returning the argument text after it.
The predicate requires an ISO date followed by T or a space, which no JSON
number can satisfy, and _filter_since shares it so the two cannot classify
a line differently.
feat(knowledge): cross-file cost ceiling for explicit imports (#9124) (#9222)
Co-authored-by: Kiro Crew Auto-Pipeline <kirocrew@users.noreply.github.com>
fix(auto-improvement): publish only the commit the pipeline committed (#8981)
`_direct_push` was handed the sha its finalizer produced -- the commit that was
measured, reproduced and written to the ledger -- and then pushed
`HEAD:refs/heads/<dest>`. Nothing compared the two, so whatever HEAD pointed at
when the push ran is what got published.
The window between them is not empty. `_prepush_review_clean` runs an agent in
that same clone with `Bash`/`Edit` for up to 30 turns, its prompt invites it to
"fix a trivial finding in the clone and re-review", and the runner's git
denylist covers only `push` and `remote set-url`, so `git commit --amend` is
permitted there.
The gate compares full object ids and fails closed when it has no valid one. It
sits after the review gate and before the credential scan and the push, and
deliberately not around the push itself: `_push_with_rebase` rewrites HEAD on
purpose and re-verifies the replayed tree.
The anchor is a full forty-hex object id, captured at the commit and never
re-derived. A short sha is an ambiguous NAME: git reads a revision as a ref name
before an abbreviated …
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is the problem?
Two kept-inline lock-open comments that #9647 landed are wrong or redundant:
session_pid._periodic_pid_sweepsays the with-scoped helper "servesexclusive acquisition only" -- but
try_acquire_lock(lock_fd.fileno(), exclusive=False)sits twelve lines below it, so the comment is refuted bythe code directly beneath it.
_McpFileLockSync.__enter__repeats the kept-inline rationale that_McpFileLockalready states fifty lines above.Why this issue matters to the user
A comment outlives the PR. The sweep comment does not merely go stale; it states
a false fact about
platform_compat.open_lock_filein the same module. A readerwho trusts it concludes the helper cannot serve a shared lock and leaves a correct
migration undone. The duplicated
_McpFileLockSynccomment is exactly the kind ofcopy-drift this consolidation item exists to reduce.
How our fix solves it
open_lock_fileonly OPENS non-truncating; the lock mode is aseparate choice at
file_lock(fd, exclusive=...). The real reason the sweepstays inline is fd LIFETIME -- the fd is held across the try/finally, not a
withblock. The original comment already said that; the fix deletes the addedfalse clause and keeps the correct reason.
_McpFileLockSync: shrink to a pointer at_McpFileLockabove.No code change; the migrations #9647 landed are untouched.
What tests we did
One file at a time,
-n0,</dev/null:timeout 900 python3 -m pytest -n0 test/test_platform_compat.py -x -q </dev/null-- 275 passed, 23 skipped (contract scan included)timeout 900 python3 -m pytest -n0 test/test_pid_lifecycle.py -x -q </dev/null-- 172 passedtimeout 900 python3 -m pytest -n0 test/test_mcp_core_coverage.py -x -q </dev/null-- 164 passedComment-history gate passes; the deletions moved no baseline entry.
Any other suggestions on the work
The lesson from the false clause:
open_lock_fileis an opener, not a lock.Whether a site can adopt it turns on the fd lifetime it needs, never on the lock
mode -- the mode is chosen at the acquire. The census of remaining siblings lives
on #9267 (still open because #9647 used
Refs, notCloses).Refs #9267