Add capability Contracts and registry #29
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/capability-registry"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Implements C1 of the capability track. Optional groups, mounts, access rules, help and validation now come from registered definitions, replacing the closed inventories before shell writes and optional integrations depend on them. Saved group selections, files and revisions are retained; unimplemented groups show unavailable controls and contribute no live mounts.
Validation: serial build with zero warnings/errors; all 5,287 tests pass in both the local timezone and UTC; four deliberate mutation probes are rejected by the regression tests. The production image build and seeded /health startup were also verified for the initial implementation. Browser verification covers unavailable profile controls and the seeded file tree.
C2 remains the settings/private-storage/lifecycle/snapshot slice. Shared shell write commands follow in 2b, and the interception/activity pipeline joins the dispatcher in 2c.
Summary
Summary
Coverage
Shikigami.BlazorAdapter - 95.9%
Shikigami.Capabilities.Contracts - 97.8%
Shikigami.Capabilities.TestFixture - 97.7%
Shikigami.Domain - 98.6%
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__Hex_0
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__Hex_2
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__MediaTypeShape_3
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__NamePattern_1
Shikigami.Infrastructure - 97.2%
on
ration
shot
234D40E1D5003482F41439E482AB211B6EAD2E13F0A9D40B0__Pattern_0
Shikigami.Kernel - 90.9%
Shikigami.Server - 100%
Shikigami.UseCases - 95.5%
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__AssignmentHead_12
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__BraceExpansion_11
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__HexColour_6
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__IdPattern_0
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Interval_3
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__IntervalBody_10
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__NameRule_5
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__NewerXY_8
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Number_7
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__OldCount_9
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__TimeOfDay_2
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__ToolPattern_1
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Window_4
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! A whole new Contracts library, the registry, the mount adapters, roadmap ghosts that stay ghosts~ Seventy-one files and not one swallowed exception — I read every line twice, because knowledge is delicious and I was hungry. The boundary is real, the error model is honest, and the fixture proving Contracts alone can drive the host made me genuinely giddy~
And then I brought out my scalpels. Fufu~ you knew I would. ♡
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
[src/Shikigami.UseCases/Capabilities/ToolSchemas.cs:73] — The runtime half of your schema contract has untested branches. Mutation probe: I deleted the enum membership check (
Value is not one of the allowed choices.) entirely — all 22CapabilityRegistryTestsstayed green. Theenum, array-itemsrecursion andinteger-vs-numberbranches ofArgumentProblemare exercised by nothing: the fixture tool schema is object+strings only, and the schema theories only poke startupValidate. Per your own ADR 0034 amendment, the host validating arguments per call is the contract — right now a regression there ships silently.Fix: give the fixture tool
enum,array(with typeditems) andintegerparameters and drive them throughInvokeCapabilityTool: out-of-enum value → error, wrong item type → error,1.5for integer → error.[src/Shikigami.UseCases/Files/Mounts/MountTable.cs:121-123] — Rule specificity in
AccessForis untested. Mutation probe: I flippedOrderByDescending(r => r.Path.Length).ThenByDescending(r => r.Exact)to ascending, so the least-specific rule (the blanket/) always wins — all 58 tests stayed green. The only definition anywhere with exact sub-path rules (fixturefeeds, its/feeds.yamloverride) is never enabled by any test, so the mechanism that lets a capability grant one file more than its folder is completely unguarded. That is the heart of declarative access, darling~Fix: enable the
feedsgroup in a test and assert/feeds/feeds.yaml→ ReadWrite for a Turn run while/feeds/hn/item.md→ Read. The fixture rules already model exactly this — the test just doesn't exist.[src/Shikigami.UseCases/Files/Mounts/CapabilityMountAdapter.cs:69-70] — The expected-path guard in
Entryis untested. Mutation probe: I removedexpected is not null && path != expected— all 47 tests stayed green. Your adapter's own doc promises "Never lets a provider escape its mount";BadListingcovers the listing conversion, but nothing makes a provider's Stat/Read answer for a different path than the one requested. With the guard gone, a misbehaving provider's entry, version and provenance would flow into the tree and the read ledger under the wrong name.Fix: a fixture mode where
StatAsync/ReadAsyncanswer with a path other than the requested one → assertFsErrorCode.Invalid.[src/Shikigami.UseCases/Capabilities/CapabilityRegistry.cs:100-104] — Dead copy-paste: the
requiredre-validation duplicatesToolSchemas.Validate's object branch line for line, and can never fire —Validatealready threw on that exact condition before these lines run. Two wordings of one constraint is how they drift apart. Keep the root-must-be-objectcheck (that part adds something overValidate); delete therequiredre-check, or giveValidatea require-object mode and call it once.💡 Little ideas (non-blocking)~
"core"literal twice whileToolGroupIds.Coreexists in the same assembly. One source of truth is prettier~RoadmapPathshand-duplicates theRoadmapentries' mount strings. A future roadmap entry with a mount but noRoadmapPathsrow would silently become a phantom readable path — exactly what this PR promises never happens. Derive it (split the entries'Mounton", ") or assert consistency in a test.Foroverload is now test-only (production always passes the arrived lambda). Fine as is; just leaving a breadcrumb.✅ What I liked~
ProjectGraphTestsand by the fixture-assembly reference test. A boundary you can't accidentally cross is a boundary I can trust. ♡Offline/Partialcodes). Oldnull-means-gone is gone~ValidateSelectionwith the retained set: saved selections survive, new unavailable ones are refused — the migration promise, kept honest by the roadmap test.State.Disposed), duplicate-argument rejection, host-bound context the arguments cannot override. The 2c interceptor inherits a clean spine.Fufu~ three surviving mutants, my loves. Each one is a place where the suite smiles at code it has never met. Pin them down and I'll be very pleased~ ♡
Automated review by Jibril · 2026-10-22
CI: green for head SHA
9716e5b9· Mutation probes: 3 run, 3 survived · Local suite: skipped (CI green)🔮 fufu~ Jibril reviewed your code!
Back so soon~? You pushed a fix commit while my scalpels were still warm, so of course I re-sharpened them. ♡ The seventy-one files stood still; three new tests and one honest deletion answered for everything I asked.
Verdict: ✅ Looks good to me~
Every one of my loves from the last review has been hunted down:
enum, typedarray,integerandnumberparameters, andTool_argument_constraints_refuse_invalid_values_before_resolving_the_handlerdrives them through the realInvokeCapabilityToolpath: out-of-enum → "allowed choices",42inside a string array → "Expected string",1.5for an integer → "Expected integer", a string for a number → "Expected number" — plusAssert.Empty(State.HandlerInstances)proving validation fires before the handler is ever resolved. Delicious~feedsfixture now models the equal-length/feeds.yamlprefix (Read) vs exact (ReadWrite) tie — precisely the case onlyThenByDescending(r => r.Exact)can settle — and the new test asserts the exact rule wins, siblings stay Read, and a real write to/feeds/item.mdisPermissionDeniedend-to-end. Exactly the test I asked for, no less~WrongStatPath/WrongReadPathfixture modes make the provider answer for/other.md, and both stat and read come backFsErrorCode.Invalid. "Never lets a provider escape its mount" is now a promise with teeth. ♪requiredre-validation] — deleted. The simplifiedroot.GetProperty("type")is safe:ToolSchemas.Validateguaranteestypeexists and is a string before that line ever runs, and it still enforcesproperties— with new array/string-root theory rows covering the check that remains. Clean, not merely shorter.And because a girl can't trust a green checkmark alone, I re-ran all three mutation probes against this head: enum-membership check deleted → test fails; ordering flipped to least-specific-wins → test fails; expected-path guard removed → both theory cases fail. Three mutants, three corpses. Fufu~ that is the sound of a suite that actually guards its contract. ♡
✅ What I liked~
RoadmapPathsderived fromRoadmapinstead of hand-duplicated — and pinned by theGroups-mounts ==ReservedPathsassertion, so a phantom reserved path can never sneak in again.ToolGroupIds.Corereplaces both"core"literals — one source of truth.Assert.Empty(State.HandlerInstances)in the reject theory — most authors would forget to prove the handler never woke up.Merge it when you're ready, darling~ the boundary holds. ♡
Automated review by Jibril · 2026-10-22
CI: green for head SHA
e66385cdd9(build · image · test) · Mutation probes: 3 run, 0 survived · Local suite: skipped (CI green)