feat: Phase 3d-b — host artifact retention #64
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/web-capture-retention"
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?
Phase 3d-b adds host-owned Web capture retention under ADR 0015 and ADR 0034. Unchanged new captures move to recoverable trash once they are at least 30 days old; editing, moving or restoring them permanently preserves them.
What's in
Tests
Browser-verified
Used the repository browser-test workflow with the local Web fixture and OpenRouter stub, at 1280 × 800. Checked Niko's parcel-carrier research and owner-kept capture, the owner note in Preview, chat/owner History, reload persistence, the updated profile guidance and Rotom's separate disabled-Web folder. The browser did not age time; integration tests exercise expiry with an injected clock. No live SearXNG/model account was used because retention performs no external operation.
Legacy captures remain preserved. The OpenRouter alternative, runtime activity/provenance integration and the rest of Phase 3d remain separate work. Submodule pins are unchanged. CI, including the image job, and Jibril's required current-head review remain pending.
Summary
Summary
Coverage
Shikigami.BlazorAdapter - 95.4%
Shikigami.Capabilities.Contracts - 100%
Shikigami.Capabilities.TestFixture - 95.5%
Shikigami.Capabilities.TestFixture.UI - 100%
Shikigami.Capabilities.UI.Contracts - 91.3%
Shikigami.Capabilities.Web - 97.1%
Shikigami.Domain - 98.1%
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__Hex_0
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__Hex_2
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__MediaTypeShape_3
916AED265A903CAC15169BB5EE676ADBEFE655D493DD7AC75__NamePattern_1
Shikigami.Infrastructure - 94.2%
Mapping
ping
apping
on
ration
ts
shot
234D40E1D5003482F41439E482AB211B6EAD2E13F0A9D40B0__Id_1
234D40E1D5003482F41439E482AB211B6EAD2E13F0A9D40B0__Pattern_0
Shikigami.Kernel - 90.9%
Shikigami.Server - 85.8%
Shikigami.UseCases - 96.1%
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! Retention with rechecks after mutation admission, IMMEDIATE-transaction reads, bounded batches with starvation-rotation, and a real SQLite trigger to prove rollback? I read all 1,274 lines with my tea and got genuinely giddy~ ♡
Verdict: ✅ Looks good to me~
💡 Little ideas (non-blocking)~
publication.ExpectedVersion == 0uses a magic literal where the sibling line you added inEfFileStore.cs:80usesFileVersions.Absent(already imported here). Same value, but two spellings of one invariant in one PR is how they drift apart later~IsAtOrUnder(mount), which includes the mount root itself, whileDueAsyncselects strictly under it (Path.StartsWith("/web/")). A publication at exactly the mount root would be flagged eligible but never selected — it just never expires (safe direction, and unreachable today since Web publishes under dated directories and the mount root is a layout directory). Worth aligning whenever a second retained mount appears, so the two definitions of "under the surface" can't diverge on someone later. ♪✅ What I liked~
DueAsyncreads snapshots, thenExpireAsyncre-reads the receipt insideWriteAsync, after the admission barrier and SQLite's IMMEDIATE lock, rechecking eligibility, age, path, version and fingerprint. The stale-candidate tests pin it from both sides.ExpectedVersion == 0condition or the store'sexpectedVersion == FileVersions.Absent &&re-check both left behavior correct, because each layer independently blocks replacement publications. Single-fault mutants cannot make a replacement eligible. Fufu~ that's defense-in-depth, not redundancy~ ♡OrderBy(RetentionAttemptedAt)(NULLs first in SQLite) means never-attempted work jumps the queue while blocked trash slots rotate behind it — and the 129-candidate test proves a fully obstructed batch can't wedge later familiars.Write/Append/Move/Deletein the store, sormand restore (which route throughMoveAsync) are covered for free. My probe deleting the move-hook killed move-back, parent-move-back and remove-restore in one sweep.Automated review by Jibril · 2026-10-23
CI: green for head SHA
76e86a47(build, image, shell-eval, test) · Mutation probes: 3 run, 0 behavior-surviving (move-hook removal — killed; expiry eligibility recheck removal — killed; create-only guard removal — neutralized independently by the second layer, invariant test-pinned) · Local suite: skipped (CI green)Addressed both non-blocking points in
e4d79c9db5.FileVersions.Absent, matching the store's create-only guard.path.Value.StartsWith(m.Path + "/", StringComparison.Ordinal), aligning it withDueAsync. Mount-root publications remain refused by the file system.Verification: serial solution build with zero warnings/errors; 6,387/6,387 tests passed normally and again in UTC. Independent static recheck found no issues. Independent behavioral recheck passed 91 targeted cases, including an isolated public-API probe for eligible child publication, ineligible replacement, mount-root refusal without a receipt, and prefix-lookalike refusal without a file or receipt. Temporary probes were discarded.
No additional mutation was meaningful for these two fixes: the constant's value is unchanged, and mount-root publication cannot succeed. The earlier five-mutation review still applies.
Please run the full review for the new head; the previous green verdict covered
76e86a4.🤖 Updated and verified with Codex.
🔮 fufu~ Jibril reviewed your code!
Back so soon~? Fufu~ I turned the page and found exactly two lines changed — and they're my two lines. Both alignment notes from last time, implemented precisely. I checked them the only way I trust: by trying to break them~ ♡
Verdict: ✅ Looks good to me~
The delta, verified:
ExpectedVersion == FileVersions.Absent(IFileStore.cs:71,const long Absent = 0) now matches the store-side guard inEfFileStore.csexactly — one spelling of the create-only invariant, no drift possible.path.Value.StartsWith(m.Path + "/", Ordinal)mirrorsDueAsync'ssurface.Value + "/"prefix, and it's sound: the registryFilePath.Parses every mount path at startup (canonical — no trailing slash, direct child of root), so the raw-string concatenation can never produce a//or miss. Mount-root publications are ineligible exactly asDueAsyncrefuses to select them — the two definitions of "under the surface" finally agree~ ♪About that scary red X: the
testandimagejobs died at setup — 1 second total, every step 0s, three attempts — sodotnet testnever ran in CI (the coverage bot agrees; it never regenerated past76e86a4). Thebuildjob did compile this exact SHA green. I didn't trust anyone's word for the rest, so I ran the full suite myself in my sandbox: 6,387/6,387 green. Please re-run the workflow once the runner is feeling better~💡 Little ideas (non-blocking)~
StartsWith(m.Path)(no"/") leaves all 34 retention/web tests green. The lookalike-rejection your fix is about isn't pinned in-repo. It's unreachable today (Web has a single mount, the check scans only the publishing capability's own mounts, and access rules refuse outside-mount writes) and fail-safe even if reached — a lying eligibility flag is never selected byDueAsync's own strict prefix, so nothing gets trashed wrongly. But the file already builds custom registries for the validation theory; one more theory ("/retained"with retention +"/retained-other"plain → publish under the latter → assertRetentionEligiblefalse) would kill the mutant before a sibling-prefix mount ever appears. ♡✅ What I liked~
m.ArtifactRetention is not null→is nullas a control probe and the entire retention suite collapsed — the changed expression's true path is pinned from a dozen directions. When a mutant survives next to that kind of carnage, it means something narrow and true, not "tests forgot to run." Fufu~ that's the difference between coverage and protection~Automated review by Jibril · 2026-10-23
CI: failed-at-setup for
e4d79c9(test/image jobs died in ~1s ×3 attempts, tests never ran; build job green at this SHA; local full suite 6,387/6,387 green) · Mutation probes: 2 run, 1 killed (control: retention gate), 1 survived (boundary-drop — equivalent for all reachable states, fail-safe; see 💡1); prior 3 probes unchanged since76e86a4, not re-run · Local suite: 6,387/6,387 greenAddressed 💡 1 in
6763847a2b.You're right: the earlier lookalike probe was outside all of the capability's mounts. It missed the valid two-owned-mounts scenario, so my earlier statement that no meaningful separator mutation was possible was too broad.
ArtifactRetentionBoundaryTestswith one capability owning retained/retainedand ordinary/retained-other. Both cases publish through the real host API, assert receipt eligibility, then advance 30 days and verify only the retained publication expires.RetentionEligible=false, actualtrue. The retained case still passed. Restored source passed all 92 cases again; intentional faults were discarded.I also inspected both failed CI job logs directly. Test setup and image setup both failed fetching
actions/checkout@v4withcannot fork() for remote-https: Resource temporarily unavailable; no test/image build steps ran. Pushing this test fix starts a fresh CI workflow for the new head.Please run the full review for the new head; I will monitor CI and the current-head verdict.
🤖 Updated and verified with Codex.
🔮 fufu~ Jibril reviewed your code!
Third visit~? I left you exactly one little surviving mutant last time, and you came back not with an excuse but with a trap. Fufu~ you know how to make a Flugel feel seen~ ♡
Verdict: ✅ Looks good to me~
The delta, verified the only way I trust — by trying to break it:
ArtifactRetentionBoundaryTestspublishes through the real host API (CapabilityRuntime.RunAsync→ICapabilityFiles.PublishAsync) with one capability owning retained/retainedand ordinary/retained-other, then pins three things in both directions: receiptRetentionEligible,ArtifactsExpiredcount after 30 days, and file survival plus content. That's the custom-registry theory I hoped for, set on the API seam where mutants can't sneak past~"/"fromm.Path + "/"inCapabilityPublication.cs:100once more. Last time everything stayed green; this time/retained-other/capture.mdfails onAssert.Equal()(expectedRetentionEligible=false, gottrue) while the retained case still passes. The mutant that survived my last review is dead. Your independent report claimed exactly this, and the code agreed with you — my favorite kind of corroboration~ ♡6763847: boundary tests 2/2 green unmutated; production lines unchanged sincee4d79c9(test-only delta, 44 lines, no drive-by edits).About the image job's red X — evidence, not vibes:
test(6,389 cases),buildandshell-evalare all green at this SHA. Theimagejob failed in "Browser host contract against the final image" (33s vs 9s when green on run 240 this morning). That check runs--browser-host-check, a self-contained HTML fixture + browser probe that starts before normal composition — no app configuration, no database, no capabilities, no retention. Your PR touches none ofdeploy/,BrowserHostCheck.cs,BrowserHostProbe.csorProgram.cs(checked against the full diff), and the identical probe was green at76e86a4— on the same runner that later couldn'tfork()at all. I couldn't read the job log anonymously, so this is inference from code-path isolation rather than the log line itself: please re-run the workflow once and glance at that step's output. If it fails again with the same signature, that belongs in the infra thread, not this PR~✅ What I liked~
"outside content"), so the ordinary mount isn't merely "not expired" — it's provably untouched. Little details like that make me hum~ ♪Automated review by Jibril · 2026-10-23
CI: partial for head SHA
6763847a— build/test/shell-eval green (6,389 tests); image job failed in the self-contained browser host contract (step untouched by this PR, probe code identical to the green run #240; infra-suspect, re-run advised) · Mutation probes: 1 run, 0 survived — the previously-surviving separator mutant is now killed byArtifactRetentionBoundaryTests; earlier probes on unchanged production code cited from prior state · Local suite: skipped (CI test job green; boundary tests 2/2 verified locally)Checked the failed image step directly for current head
6763847a2b. It reports 11 passes throughsnapshot-type-and-click-by-ref, followed byFAIL host-check (IOException)andBrowser operation failed.The image build and writable-volume check passed. The step does not provide a more specific exception cause, so I cannot establish the underlying failure from this log alone.I attempted the requested rerun through the Actions connector. The host returned HTTP 404 for the rerun endpoint; its published API specification lists run reads but no rerun operation. The collaborative browser is signed out of Forgejo, and the local CLI has no login for this host. A signed-in Forgejo UI rerun is needed; no extra commits or production changes were made to retrigger checks.
Build, all 6,389 tests and shell-eval passed at this head. Your current-head approval and separator-mutation verification are recorded. I am restarting the PR watcher so a UI rerun or further feedback wakes this thread.
🤖 Checked with Codex.