feat(acp): harness descriptors, registry, and capability gates as config data - #8255
feat(acp): harness descriptors, registry, and capability gates as config data#8255billygerhard wants to merge 1 commit into
Conversation
48b055f to
29760a7
Compare
29760a7 to
5e310f6
Compare
5e310f6 to
5bdda35
Compare
5bdda35 to
e866930
Compare
Design Review (Fable 5, fork) — 🟡 CONCERNSDesign-level review of Design-Verdict: CONCERNS Sound core design — data descriptors, fail-closed capabilities, config-can't-select-code — but live undescribed loader machinery and a docstring-only cross-PR contract need eyes. Watch
[DESIGN-REVIEWED] ac78f9b |
GPT 5.6 Review (fork) — ✅ no blocking findingsReviewed Review detailsFINDING -- src/kiro_crew/security.py:7068 -- |
First Principles Review (Fable 5, fork) — 🟡 CONCERNSPremise-level review of All verification done. The reused mechanisms ( First-Principles-Verdict: CONCERNS The core earns its stacked, consumer-less landing — but What this change shipsIntent: let an operator add any standard-ACP harness as config instead of a code PR — an ADDITION, part 1 of a declared 3-PR stack (zero consumers is derived from the 1 MB review-lane limit).
Watch
Subtractions
[FIRST-PRINCIPLES-REVIEWED] ac78f9b |
Opus 4.8 Review (fork) — ✅ no blocking findingsReviewed Review detailsThe single candidate depends on a live consumer of the registry's availability/resolve path. Confirmed: every caller of No further grounded defect surfaced in the changed lines. No findings. [OPUS-REVIEWED] ac78f9b |
e866930 to
6bbdec2
Compare
|
Dispositions for the GPT 5.6 review of e866930 (addressed in 6bbdec2): BLOCKING — cli_config.py protected-key deletion check races the locked write: FIXED. The guard is now evaluated a second time INSIDE each lock-held mutate callback ( FINDING — validation.py accepts a Also in 6bbdec2: no other code changes; the diff delta vs e866930 is exactly the cli_config fix, its test, and the validation.py removal. |
b53d7c4 to
1b9d9f1
Compare
1b9d9f1 to
104ba08
Compare
|
Disposition for the Design Review BLOCK on 1b9d9f1 (addressed in 104ba08): ACCEPTED and fixed with the relocation the review prescribed. Operator harness descriptors no longer live in
New/updated tests: the shell-fence grid for |
104ba08 to
c4d5598
Compare
|
Dispositions for the GPT 5.6 review of 104ba08 (addressed in c4d5598): BLOCKING — check_harness_parity.py:106 suppression matched inside string literals: ACCEPTED, fixed. Confirmed: BLOCKING — :241 VOCABULARY_PATHS exemption: no change, per the adjudication's downgrade, whose reasoning matches the design: the two exempted modules are themselves vocabulary homes ( |
c4d5598 to
3b3eab4
Compare
|
Disposition for the GPT 5.6 review of c4d5598 (addressed in 3b3eab4): BLOCKING — security.py: cd-relative shell writes bypass the harnesses.json fence: ACCEPTED, fixed as prescribed. Confirmed: every Tests: the existing bare-token grid is tuple-driven, so all five probe families (cd-relative redirect, bare-name-any-verb, subdir-relative, Windows-relative, unrelated-names-stay-allowed) now run against harnesses.json automatically; the harness fence test additionally pins tuple membership and the exact cd-relative shape from the finding. 2,474 tests green across the security/registry/alias/denied-commands suites. |
…fig data Part 1 of 3 (core): author any ACP provider as a config-authored HarnessDescriptor; the registry validates and serves descriptors, protocol profiles pin each wire dialect, spawn-side attestation refuses unvetted executables, and code-only capabilities are refused from config - with a key-level config-write guard for agent.harnesses. No consumer is wired yet - session selection and serving are part 2; the UI is part 3.
3b3eab4 to
ac78f9b
Compare
|
Dispositions for the First Principles BLOCK and GPT 5.6 review of c4d5598 (addressed in ac78f9b): FP BLOCKER 1 — spec + gate assert a retirement this commit does not perform: ACCEPTED, fixed by the prescribed subtraction. FP BLOCKER 2 — description describes a mechanism the diff replaced: ACCEPTED, fixed. The PR body is rewritten: the stale FP subtraction (black reformat rides along): ACCEPTED, fixed. FP subtractions on GPT BLOCKER — suppression marker matched inside string literals: fixed at the source, ships with the gate in part 2. The quote-aware fix (suppression read only from the true trailing comment via Also in ac78f9b: rebased onto current main. 2,350 tests green on the slice; flake8/isort/mypy/black-gate/docs-lint/parity(main's gate)/boundary all pass. |
|
Attempted a maintainer rebase onto main
Also resolved mechanically once you rebase: |
Problem / Motivation
Stack: part 1 of 3 (supersedes #7984) — part 2 wires the registry into
session serving; part 3 lands the Settings UI, docs, and screenshot evidence.
Each part opens after its predecessor merges.
Adding an AI provider to Kiro Crew today means a code PR: a bespoke adapter,
capability sets edited by hand, and a spawn branch per backend. An operator who
runs an ACP-speaking server that Kiro Crew has never heard of — a vendor
adapter binary, an internal gateway, a house-built agent — cannot use it at
all, even though the conversation protocol on the wire is the same standard
ACP the named backends already speak.
Why it matters
The set of ACP-capable providers is growing faster than any hand-written
adapter roster can. Making a provider a ROW OF CONFIG DATA instead of a code
change turns "support my provider" from a feature request into an operator
action, while keeping the security posture explicit: a descriptor declares
what its harness may do, and everything it does not declare stays off.
What changed (motivation → approach → change)
Goal: any ACP provider becomes a row in a dedicated
harnesses.jsonbesidethe config. Approach: a validated descriptor object rather than free-form
config — every behavioural difference between harnesses becomes a declared
field with a pinned vocabulary, so the serving code (part 2) can be generic.
This part lands the core, with no consumer wired yet (behaviour of every
existing session path is unchanged — that is part 2):
acp/harness_descriptor.py: theHarnessDescriptordataclass — executable,argv template, capability set, MCP delivery mode, model source — with
field-level validation. Code-only capabilities (
internal_sandbox,acp_runtime_pool,session_sharing,kiro_identity_store) are REFUSEDfrom config with per-key reasons: an operator row can never grant itself a
sandbox waiver.
acp/harness_registry.py: bundled descriptors for the named backends plusoperator rows read from
~/.kiro/crew/harnesses.json— a dedicated file,deliberately NOT a
config.jsonkey; invalid rows are excluded from everylisting with a recorded reason, never silently trusted.
acp/protocol_profile.py: each wire dialect pinned as data (protocolversion, permission-option style, model/effort channel).
security.py:harnesses.jsonis write-protected from agent tools on BOTHwrite paths — the file-edit register and the shell register (a descriptor
row names an executable the gateway spawns as itself; nothing downstream
clamps that). Because the file is effectively an execution grant, its leaf
is additionally matched anchor-independently (
_BARE_TOKEN_PROTECTED_LEAVES),so a
cd-relative redirect cannot walk around the home-anchored spellings.Reads stay allowed on both paths: the file holds no secret and the registry
reads it on every listing. Same relocation pattern as
playwright-cli-config.json, the computer-use policy, and the OMC rotationrecord.
acp/harness_adapters.pyspawn-side trust attestation: the resolvedexecutable must be a runnable, non-zero-byte file, and the rendered argv is
refused unless
argv[0]execs exactly the attested path.The parity-gate extension (new CI rules that steer added lines onto
descriptor-declared capabilities) and the harness-parity spec updates ship
with part 2, where the capability-view retirement they describe actually
happens.
Tests
test_harness_descriptor.py— field validation, capability refusal fromconfig (per-key reasons), delivery-mode vocabulary.
test_harness_registry.py— bundled + operator rows, invalid-row exclusionwith reasons, the
harnesses.jsonfile read (valid, malformed, non-object,unparseable), and the write-fence grid: file-edit tier membership, shell
writes refused across every home-prefix spelling AND the bare
cd-relativeform, file-READ tool ungated, Settings-PATCH allowlist pinned.
test_harness_generic_adapter.py— executable resolution (absolute,PATH-augmented bare name, relative refused, zero-byte refused,
platform-correct executability).
test_harness_attestation.py— the spawn-side trust seam: attested launchpath, zero-byte and non-executable refusals, and the check that
argv[0]execs exactly the attested bytes.
test_harness_capability_views.py,test_protocol_profile.py,test_security.py— leaf/descriptor agreement, dialect profiles, and thebare-token fence grid (which is tuple-driven, so
harnesses.jsonrunsthrough all five probe families automatically).
Manual verification
N/A for this part — nothing consumes the registry yet, so there is no
user-visible behaviour change; the full stack was hands-on verified in a dev
pod (all three named providers plus a config-authored stub provider serving
real sessions) and that evidence ships with part 3, where the UI lands.
No linked issue: this implements the provider-extensibility work discussed in
#7984, which this stack supersedes.