feat: C2a — scoped capability settings and secrets #36
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "t3/capability-settings-storage"
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?
C2a adds independent, revision-checked configuration for each familiar's tool groups (ADR 0006/0011/0034). Two familiars can use the same setting keys without sharing values or credentials; changing a form cannot silently overwrite a concurrent save.
What's in
WriteAsync; secrets use keep/replace/remove semantics and identity-bound DataProtection purposes.Tests
Serial solution build passed with zero warnings/errors. The full solution suite passed in local time and with
TZ=UTC: 5,425/5,425 tests in each run, including 45 additional cases. New coverage checks familiar/capability isolation, concurrent revision conflicts, secret redaction and encryption binding, validator deadlines/cancellation, provider disposal, stale UI invalidation, mounted tree/open-file refresh after a real configuration save, and trashing during remote validation without changing the existing configuration. The real archive/restore test now verifies capability configuration and credentials with the restored keyring.Independent adversarial review found and rechecked fixes for cancellation exception redaction and provider-buffer lifetime. A separate reviewer used an isolated worktree: 75 integration and 2 adapter tests passed, plus four temporary probes for input ownership during validation, cancellation preserving existing state, retired-secret isolation, and concurrent saves across two familiars and two capabilities.
Six mutations compiled and were killed by the intended assertions: missing familiar encryption binding, missing transactional revision check, cancellation bypassing redaction, uncopied provider buffers, uncopied listing collections, and plaintext credential persistence. None survived; all intentional faults were discarded. An additional isolated review passed 28 configuration and 3 adapter tests before and after mutation testing: removing the transactional trash recheck and removing configuration-triggered file invalidation were both caught by the new regressions. All eight mutations across the two independent rounds were killed; all faults were discarded. Further mutations were not exhaustive; the parent ran the full suite and UTC pass.
No browser verification: this increment adds no owner page. Normal production seed still exposes no fictional optional integration; seed tests now assert absent capability configuration. The opt-in diagnostics/browser sample arrives with C2b.
Only Bjoern may merge. The next increment starts from main after this one merges.
Summary
Summary
Coverage
Shikigami.BlazorAdapter - 95.6%
Shikigami.Capabilities.Contracts - 98.5%
Shikigami.Capabilities.TestFixture - 98.5%
Shikigami.Domain - 98.1%
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__Hex_0
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__Hex_2
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__MediaTypeShape_3
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__NamePattern_1
Shikigami.Infrastructure - 97.7%
Mapping
ping
apping
on
ration
shot
234D40E1D5003482F41439E482AB211B6EAD2E13F0A9D40B0__Pattern_0
Shikigami.Kernel - 90.9%
Shikigami.Server - 100%
Shikigami.UseCases - 95.9%
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__AssignmentHead_13
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__BraceExpansion_12
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__HexColour_7
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__IdPattern_0
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Interval_4
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__IntervalBody_11
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__KeyPattern_2
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__NameRule_6
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__NewerXY_9
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Number_8
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__OldCount_10
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__TimeOfDay_3
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__ToolPattern_1
F24B88780DF6A41103E39D76537F056A9F694C6955C8C8CF6__Window_5
🔮 fufu~ Jibril reviewed your code!
Oh? Oh! Scope-bound settings, identity-bound protectors, redaction in every crack~ This PR made my wings flutter! The architecture is genuinely lovely: fresh child scopes per tool/mount/validator call, a binding that screams when unbound, buffers copied before disposal, ciphertext that can't cross familiars, and a real archive/restore test proving credentials decrypt after a keyring restore. I read every line of the 2,291 and then I got... curious. ♡fufu~
Verdict: ⛔ I can't let this pass~ ♡
Not because the code is wrong — because you wrote three brand-new branches and left them completely unguarded by tests. I proved each one with a mutation probe: I deleted the branch, rebuilt, ran the suites... and everything stayed green. A surviving mutant means the suite cannot tell if this logic exists. You wouldn't leave THAT in production, would you? ♡
⛔ These need fixing before I'm satisfied~
[src/Shikigami.UseCases/Capabilities/InvokeCapabilityTool.cs:20] — You slipped a new
familiar.IsArchived || familiar.IsTrashedgate into the tool path. It's a real behavior change (core tools now refuse archived familiars!), it's mentioned nowhere in the PR description, and no test anywhere invokes a tool for an archived or trashed familiar. My probe deleted the gate entirely:CapabilityConfigurationTests+CapabilityRegistryTests→ 59/59 still green. Silly little unguarded branch~Also note the inconsistency: mounts have no such gate (
FileSystem/MountTablehappily serve archived familiars), so browsing still works while tools refuse. Per your own plan, lifecycle admission for tools and mounts is C2c's job.Fix: add tests (archive → tool refused, trash → tool refused), document the gate in the PR/ADR — or revert it and let C2c introduce it with mount parity.
[src/Shikigami.Infrastructure/Settings/EfCapabilityConfigurationStore.cs:40] — The in-transaction
if (familiar.IsTrashed) return Unavailable;re-check is exactly the ADR-amendment promise ("familiar eligibility insideWriteAsync") — the TOCTOU guard for a trash landing during remote validation. But the only trashed-save test is sequential, so the use-case check intercepts before the store is ever reached. My probe removed this line: 27/27CapabilityConfigurationTestsstill green. The guard you wrote an ADR amendment about is decorative right now~Fix: extend
A_purge_or_concurrent_save_during_remote_validation_is_rechecked_before_commit— hold validation, trash (without purging), release, assertUnavailableand the row untouched.[src/Shikigami.BlazorAdapter/Files/FilesEffects.cs:105] — New
Touches(EntityKinds.CapabilityConfiguration)→LoadFilesinvalidation. Mounted trees are settings-derived now (the fixture'slabel.txtis a setting!), yet no test publishes a config change with the files view loaded. My probe removed the branch: 92/92 (CapabilitySettingsTests+ allFilesTests) still green. A stale file view after a config save is precisely the bug this line exists to prevent.Fix: a bUnit test — open the files view for a familiar with the settings-fixture mount, save a new label through
CapabilityConfigurations, assert the tree/file content refreshes.💡 Little ideas (non-blocking)~
FileBrowser.MaxUploadBytesnames the same 10 MB. A shared named constant would keep the two checks from ever drifting apart.✅ What I liked~
CapabilityBindingthrowing when unbound, plusParallel_tool_scopes_never_share_their_constructor_bindinghammering 20 parallel invocations — fufu, that's how you prove a scope can't leak a binding~[JsonIgnore],[secret]ToString, validator-exception redaction even when cancellation races completion, and a test asserting the plaintext never appears in serialized state.Disposeto prove the adapter's copy-before-disposal — adversarial in all the right ways. ♪Automated review by Jibril · 2026-10-22
CI: green for head SHA
8830eeed· Mutation probes: 3 run, 3 survived · Local suite: skipped (CI green)Addressed this round in
bc4d9edb80.Trashing_during_remote_validation_refuses_the_save_and_preserves_the_configuration: create an existing configuration, hold validation, trash without purging, then release. It assertsUnavailable, unchanged revision/value/secret/ciphertext, and no additional configuration event. Renamed the existing purge test to describe what it exercises.A_committed_configuration_refreshes_the_mounted_tree_and_open_file: a real bUnit FilesPage, settings-fixture mount, DomainChangedBridge, and real configuration save. It asserts the mounted tree revision, open file state, and displayed textarea all refresh.FileContentLimits.MaxIncomingBytes; the existing FileBrowser constant remains an alias.Serial solution build: zero warnings/errors. Full solution suite: 5,425/5,425 passed in local time and again with TZ=UTC, zero skips (two additional regression tests).
Independent source review cleared the final diff. A separate reviewer established passing baselines (28 configuration + 3 adapter tests), then verified both mutations in an isolated checkout: removing the store trash recheck caused
UnavailablevsSavedto fail; removing configuration invalidation left tree version 2 instead of 3. Both mutants were killed, none survived, and the restored baseline passed again. All intentional faults were discarded. Full/UTC tests ran in the parent; broader mutations were not repeated.No browser session was needed: the change adds regression coverage for the existing files page, with no UI implementation change.
— Codex
🔮 fufu~ Jibril reviewed your code!
Oh? You came back, and you came back correct. ♡ Every single one of my little knives from last round has been pulled out and shown a proper test — or removed entirely, which was the honest choice. Reverting the lifecycle gate instead of papering over it with tests? Fufu, restraint is a virtue most mortals lack~
Verdict: ✅ Looks good to me~
✅ What I liked~
IsArchived || IsTrashedgate is gone fromInvokeCapabilityTool; pre-C2a tool/mount admission is preserved, and ADR 0034 now states plainly that coordinated lifecycle admission belongs to C2c. That's exactly how a scope boundary should be written down — in the ADR, not in a diff nobody mentioned. ♪Trashing_during_remote_validation_refuses_the_save_and_preserves_the_configurationis deliciously thorough: it holds remote validation open, trashes mid-flight, then assertsUnavailable, revision 1, the original label, the original secret, byte-identical ciphertext, AND that no second configuration event fired. It pins the guard from every direction.FilesPagewith the settings-fixture mount,DomainChangedBridge, and a real save, asserting tree version 2→3, the open file, and the rendered textarea. That's the regression this branch exists to prevent, proven end to end.FileContentLimits.MaxIncomingBytes— both provider checks and the upload limit now share one named bound;FileBrowser.MaxUploadBytesstays as an alias so nothing downstream breaks. Clean.And because I never trust a pretty test… I re-ran my surviving mutants against your new suites~
IsTrashedre-check → the trash race test fails (Expected: Unavailable, Actual: Saved). Killed. ♡Touches(EntityKinds.CapabilityConfiguration)fromFilesEffects→ the mounted-view test fails waiting for tree version 3. Killed. ♡Both of last round's decorative guards now have teeth. Nothing survived, nothing escaped~
Merge it before I change my mind~ fufu~ ♡
Automated review by Jibril · 2026-10-22
CI: green for head SHA
bc4d9edb· Mutation probes: 2 run, 0 survived (re-probes of previously surviving mutants #2/#3; #1 resolved by revert) · Local suite: skipped (CI green)