[CHORE] Retire capsule-wire into the Salvo review bucket - #430
Open
justin13888 wants to merge 5 commits into
Open
[CHORE] Retire capsule-wire into the Salvo review bucket#430justin13888 wants to merge 5 commits into
justin13888 wants to merge 5 commits into
Conversation
`capsule-wire` carried the framework-free response taxonomy `S-C27` extracted so the contract could outlive the transport. The transport swapped and the taxonomy did not come with it: `capsule-server::problem`, `::limits` and `::body` own it on Kynos, no live crate names `capsule_wire`, and a third of the crate is a `salvo_responses!` adapter for a framework that left the workspace with `S-C59`. Its only real consumers are the 42 macro call sites under `legacy-review/server-salvo/`, so the crate lands beside them at `legacy-review/server-salvo/wire/` with its manifest disabled, per the convention every other quarantined crate follows. `capsule-server` loses a path dependency it never imported; the module comment states where the taxonomy went instead of linking a crate that is no longer built. BREAKING CHANGE: `capsule-wire` is no longer a workspace member and `capsule-server` no longer depends on it. Nothing in the workspace imported it, so no public API moves.
`architecture-check` reads `cargo metadata`, so listing `capsule-wire` in `RETIRED_DEPENDENCIES` makes a re-added path dependency in any member a boundary violation rather than a review question. It joins the list under its retired arm, beside `capsule-media`. It stays out of `RETIRED_COMPONENT_NAMES` deliberately: `check_retired_references` is a substring scan over live `.md`/`.rs`/`.toml` and `ignored_path` excludes `legacy-review/` but neither `SLICES.md` nor `adr/`, so listing the name there would make the records of this retirement fail the check that enforces it. CI's `rust` paths filter loses the `capsule-wire/**` entry, which now names a directory outside the workspace, and the Salvo review notes say which of the moved crate's surfaces must not come back.
ADR-0004 flips from `proposed` to `accepted` and gains the `Contract:` line `adr/README.md` requires of a landed decision — without it the record is prose no reader can falsify. `S-C27` becomes `done`, by retirement rather than by completion. Its part 2 was owed to the Kynos port; the port declined it, because the 39 `ToSchema` derives the DTO move was waiting on retired with the Salvo tree instead of moving. The row's own "Done when" — `rg salvo capsule-api/*/src/models` empty plus a byte-identical `openapi.json` — is vacuous now that `capsule-api` does not exist and the SDK generates from the Kynos document, so leaving the row at `part 1 done` would name owed work nobody can do. The salvo→kynos register row says the same, and its claim that `architecture-check` reports 63 violations is put in the past tense it belongs in: the check is clean because the tree it counted is quarantined. `module-map.md`'s crate table and the `ROADMAP.md` package row drop `capsule-wire`: both enumerate what the workspace declares, and the roadmap check resolves rows against `[workspace] members` and the depth-one `legacy-review/*/` buckets, neither of which now names it. The three disabled Salvo manifests point at `../wire` so the quarantined tree stays internally consistent for whoever reads it.
Deploying capsule with
|
| Latest commit: |
be5a12a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b9b4a0e2.capsule-22k.pages.dev |
| Branch Preview URL: | https://chore-retire-capsule-wire-40.capsule-22k.pages.dev |
5 tasks
The Landed note said 41 `salvo_responses!` call sites. The count that produced it was taken with no `--include`, so it swept the same change's new `REVIEW.md` bullet, which names the macro in prose. Restricted to `.rs` outside the moved crate the tree has 40 invocations — which is what the salvo→kynos register row already says — plus two `use capsule_wire::…` lines, giving the 42 `capsule_wire` references measured separately.
…ire-capsule-wire-400 Resolution: `ROADMAP.md` takes the base's block — its rewordings of the `capsule-server` and `capsule-wasm` rows, which belong to that lane — with the single `capsule-wire` row dropped, since this branch retires that package.
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
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.
Description
capsule-wirewas extracted byS-C27so the Salvo server's response taxonomy could outlivethe transport. The transport swapped and the taxonomy did not come with it: on Kynos the status
is part of the return type,
capsule-server'sproblem,limitsandbodymodules carry thecoded-problem bodies, the size limits and the header census, and
tests/conformance.rsassertsboth directions of the agreement the extraction existed to keep. Nothing live imports
capsule_wire—capsule-serverdeclared it as a path dependency and used it zero times, onerustdoc intra-doc link aside — while a third of the crate is a
salvo_responses!adapter for aframework
S-C59removed from the workspace.So the crate leaves the active workspace and lands as review material beside its only real
consumers, at
legacy-review/server-salvo/wire/with its manifest disabled, per the conventionevery other quarantined Salvo crate follows.
Summary
Three commits, each leaving the tree coherent:
chore(wire)!: move capsule-wire into the Salvo review bucket—git mv capsule-wire legacy-review/server-salvo/wire,Cargo.toml→Cargo.toml.disabled; both[workspace]member entries and
capsule-server's path dependency dropped;capsule-server/src/lib.rs'smodule comment reworded so it no longer emits an intra-doc link to a crate that is no longer
built;
Cargo.lockregenerated (9 lines: the package stanza plus one edge).chore(xtask,ci): close the boundary capsule-wire left—capsule-wireadded toRETIRED_DEPENDENCIES, so a re-added path dependency in any member is a boundary violationrather than a review question; the
capsule-wire/**line removed from CI'srustpathsfilter; a "Do not reuse" bullet in
legacy-review/server-salvo/REVIEW.md.docs(slices,adr): record capsule-wire's retirement— ADR-0004proposed→acceptedwith the
Contract:lineadr/README.mdrequires;S-C27re-scoped to done-by-retirementwith a Landed note; the salvo→kynos migration-register row amended; the
module-map.mdcraterow and the
ROADMAP.mdpackage row removed; the three disabled Salvo manifests repointed at../wire.Two corrections to figures the issue and the tracker carried:
salvo_responses!sites. Measured: 40 macro invocations —.rsfilesunder
legacy-review/server-salvo/outside the moved crate — plus twouse capsule_wire::…lines, which is the 42
capsule_wirereferences counted separately. The Landed note says40, matching the figure the salvo→kynos register row already carried. (An earlier revision of
this pull request said 41: that count was taken with no
--include, so it swept this change'sown
REVIEW.mdbullet, which names the macro in prose. Corrected in393d78d6.)architecture-checkreports 63 boundary violations.It reports zero — the tree it counted is quarantined and
is_ignored_directoryskipslegacy-review/. The clause is put into the past tense it belongs in; no new figure isasserted.
Validation
Git hooks are not installed in this checkout (
.git/hooksholds only*.sample, nocore.hooksPath), sohk's pre-commit and pre-push steps did not run and every gate below wasinvoked explicitly as its
misetask. No--no-verifywas used.cargo metadata --offline --no-deps --format-version 1 | grep -c '"name":"capsule-wire"'grep -c capsule-wirereturns 1 in this checkout because the worktree directory is namedCapsule-chore-retire-capsule-wire-400; the"name":form is the same assertion without the path artefact.)grep -n 'capsule-wire' Cargo.lockcargo check -p capsule-server --offlinemise run architecture-checkSLICES.mdtext trips a retired-component name.cargo nextest run -p xtask --offlinemise run check-docs-truthmise run lint-check-mdmise run check-commitsmise run check-rustmise run test-rustcargo nextest run --workspace, then-p capsule-core --features ffi, then-p capsule-sdk --features ffi(160 tests run, 160 passed, 0 skipped)mise run check-docs-truthwas also run before theROADMAP.mdedit, deliberately, toestablish the mechanism rather than guess it: it reported
ROADMAP.md:44 capsule-wire is not declared by any manifest in the tree. A second deliberaterun with the row rewritten as
| legacy-review/server-salvo/wire | review-bucket | … |reportedthe same finding for that name —
reviewBucketsincheck-roadmap.mjsenumerateslegacy-review/<bucket>/at depth one only, so a crate inside the existingserver-salvobucketis not a package the check can key. That is the evidence behind decision 5.
CI on this pull request
Every failure below is classified against evidence, not asserted:
TypeError: fetch failedfromoven-sh/setup-bun@v2. No repository file is read before that step. The same gate passes locally on this tree (172 files, 0 issues) and passed on the base pull request.bun install --frozen-lockfileincapsule-webreportslockfile had changes, but lockfile is frozen.capsule-web/bun.lockandcapsule-web/package.jsonare byte-identical betweenmasterand this branch's base, and this change touches no web file. The job ran here only because thewebpaths filter includes.github/workflows/ci.yml, which this change edits, so it wasskippingon the base pull request and this branch is the first to exercise it.capsule-web/bun.lockfix and its Web job passes. Fixing it here would duplicate that lane's change and sits outside this lane's manifest.master(run 33572237154), andROADMAP.mdrecordscapsule-androidasblockedbecause the app references a DI layer that is not in the tree (#389). Nothing here touches Kotlin or Gradle.Risks and rollout
No persisted data, no deployed behaviour, no wire compatibility: nothing in the workspace
imported the crate, so no public API moves.
git revertof the merge commit restores it, andeach commit reverts independently.
The one operational effect is CI's
rustpaths filter losing a line; the directory it named isnow inside
legacy-review/, which no Rust gate descends into.Related Issues
Closes #400
Contributor Checklist
Decisions taken
Decisions taken inside the manifest during delivery, in the same shape:
Unresolved review notes
SLICES.md's row-count paragraph is now one row stale, deliberately untouched. FlippingS-C27frompart 1 donetodonemoves it out of thepartbucket, so**98 done / 57 done\* / 28 ready / 9 part / …**should read99 done / … / 8 part, and theparenthetical that follows still names
S-C27among the rows "the table spellspartandpart 1 done". That paragraph is outside this lane's manifest — it belongs to the tracker lanethis branch is stacked on — so it is left for whoever holds it rather than widened here. No gate
reads it:
check-docs-truthparses table cells and### S-XXXheadings only.Webjob's red mark on this pull request is issue ci: close the gate holes — test job outside required, missing path filters, pre-push parity, and the Kotlin smoke test #416'scapsule-web/bun.lockfix, notthis change; it turns green either when that lane lands or when this branch is rebased onto it.
It is listed here rather than fixed because the file is outside this lane's manifest.
capsule-wire/src/salvo_adapter.rs, a path this change moves. Left aswritten: an ADR's Context is by definition the state before the decision, and the ADR edit was
scoped to the Status flip and the
Contract:line.Blocker — this pull request is
CONFLICTING, so GitHub runs no CI on itThe base branch gained six commits after this branch forked from
1043a414(
99dd4bc8,0fe7e290,f0d5c3cc,adea7c2c,46d5559f,09b95705), and PR #430 is nowmergeable: CONFLICTING/mergeStateStatus: DIRTY. GitHub does not createpull_requestworkflow runs for a conflicting pull request, which is why
393d78d6carries only theCloudflare Pages check: no CI run exists for it, and none was cancelled — none was ever
created. A close/reopen of the pull request did not change that, as expected. The green CI
recorded above was observed on
b54dc818, the previous head; the one-line count correction in393d78d6has not been through CI. Every local gate was re-run on393d78d6and passed.The conflict is one file and one row.
git merge-treeagainst the current base reportsSLICES.mdandadr/0004-capsule-wire-is-retired.mdauto-merging and onlyROADMAP.mdconflicting, as a delete-vs-modify on the
capsule-wirerow: this branch deletes it (decision5), while
adea7c2creworded itsNotescell. The conflict block also picks up the base'srewordings of the adjacent
capsule-serverandcapsule-wasmrows.Resolution, verified rather than asserted: take the base's side of the block — so its
capsule-serverandcapsule-wasmrewordings are preserved, since those rows belong to thatlane — and drop the single
|capsule-wire| …line. Applied to the merge tree in a scratchcopy,
node capsule-docs/scripts/docs-truth.mjsthen reports cross-links 529/529,endpoint-census 84/84, module-paths 120/120 and roadmap 46 packages, all rows resolve.
This lane does not perform that merge. It would carry the base lane's
SLICES.mdprose,ROADMAP.mdrows,check-roadmap.mjsrewrite andbun.lockonto this branch — all outsidethis lane's manifest — and that base is still moving, so the merge would need redoing. The
integrator should land the base first, then merge or rebase this branch and resolve as above.
Decision 5 survives the base's gate rewrite.
f0d5c3ccmademanifestDirsrecurse onelevel for
Package.swift/package.json/pyproject.toml, but leftreviewBucketsenumeratinglegacy-review/<bucket>/at depth one, so a crate inside theserver-salvobucket is still nota package the check can key, and deleting the row is still the only green option.
The
Webfailure classified above is already fixed on the base.99dd4bc8 build(web): regenerate bun.lock for the package.json the v1 head changedis in the base now, so that jobshould pass once this branch is merged onto it.
Blocker cleared (
be5a12ab)The base was merged by merge commit, resolved exactly as verified above, and the pull request is
MERGEABLEagain. GitHub created workflow runs for the new head immediately — CI, Build Androidapp and Build Apple client — which confirms the conflict, not a runner outage, was what
suppressed run creation on
393d78d6. The base did not move again: it is still09b95705.mise run check-docs-truthon the merged tree reports cross-links 529/529, endpoint-census84/84, module-paths 120/120 and roadmap 46 packages all resolving;
mise run lint-check-mdreports 172 files, 0 issues;
mise run check-commitsaccepts the merge commit. The paragraphsabove are kept as the record of why the head sat without CI, not as a live claim.