Repository navigation
claude protocol spike - #177
Merged
Merged
Conversation
…t implicit ADR-0037's protocol-first control plane needs a third adapter mode alongside Tui and Headless: an interactive, long-lived vendor process driven over its own structured protocol rather than a PTY, with the terminal reduced to a self-rendered view. Reusing Headless was wrong twice over -- it is already retired-and-rejected, and this is not the deleted batch/non-interactive shape either. Added AdapterMode::Protocol to both the config-layer and dispatch-layer enums, with the two exhaustive matches this broke (conformance's fixture dispatch, recovery's resume-mode check) given honest "not built yet" rejections rather than a silent fallthrough. Protocol mode has no conformance suite to run at all yet. Passing its adapter's raw declared capabilities into gate_profile's existing authorization path unchanged would have looked identical, by type, to a suite that actually ran -- a landmine for the day AdapterAuthorization::authorize grows its first real capability check (it reads zero capability fields today, which is exactly why this was safe to leave unaddressed until now, and exactly why it would not be forgiven forever). Introduced GatedCapabilities (Proven/Unproven) so authorize() itself is a call site that must say which one it has, threaded through its one production call site and all four implementations. A test proves the distinction is load-bearing rather than aspirational: the same authorization checker denies an Unproven protocol-mode submission and allows a Proven TUI-mode one, so the difference in outcome can only come from the marker. gate_profile's own new Protocol branch rejects every kind but claude (nothing else implements it) and otherwise proceeds with a new, deliberately minimal declared_capabilities() -- committed scope only, matching what this mode's first real adapter is actually built to demonstrate, not an aspirational full set. Also closed a related gap the same review surfaced: crewd conformance's own --mode flag rejected Headless specifically rather than allowing only Tui, so a future mode value would have reached run_fixture_conformance's own "the caller must reject this first" contract from a live command line instead of failing before it. Inverted to an allowlist so the guarantee holds by construction.
…he drop-injection seam absent from release builds The live control-channel stream carries no sequence number a dropped message would be missed against, so "the stream went quiet" and "nothing happened" are indistinguishable from the stream alone -- reconciling against the vendor's own durable transcript, read as a file rather than through the same in-process channel already consumed, is what makes a gap detectable at all. Added RuntimeEvent::AdapterReconciliationEvent: examined/gapsFound/ gapsRepaired counts, no free text. examined == 0 is itself a finding, not a clean pass -- a reconciliation that never looked at anything is indistinguishable from one that never ran, and a boolean "did it run" flag cannot tell those apart the way a count can. Deliberately not reusing the existing needsReconciliation RunFlags boolean, which already means something narrower and unrelated (a human typed directly into an attached pane) and is itself write-only today -- folding a second meaning onto it would be exactly the kind of ambiguity this mechanism exists to get away from. The comparison itself (crate::adapter::claude_protocol::reconcile) is a pure function: parse the transcript, diff its entry ids against what the caller already knows was journaled, report the difference as gaps. The caller owns repairing each gap and counting how many attempts actually succeeded, since only it holds the sink and the run's identity. The drop-injection seam a test needs to actually exercise reconciliation catching a dropped event -- rather than merely asserting it in prose -- is modeled directly on TerminalGrid::mark_unsupported's cfg(test)/ cfg(not(test)) split: the test build's parse wraps the real parse with a thread-local seam a test can arm once; the non-test build has no such function at all, no thread-local, and no branch checking one -- an absent code path, not a no-op one. The `#[cfg(test)]` attribute is what guarantees that absence, a compile-time property; a new external integration test (crates/runtime/tests/claude_protocol_reconcile.rs) cannot prove the attribute is honored (a seam left compiled-in but simply never armed would pass it unchanged) but does prove the real parse path itself works correctly in a build where `cfg(test)` is off for this crate's own code -- something no test living inside crates/runtime/src/ can do, since cfg!(test) is true throughout that binary's own compilation regardless. The entry schema (a `uuid` field, a `type` of `user`/`assistant`) is provisional, not yet checked against a live transcript capture -- flagged in the module's own doc comment, on the same basis the control channel's own exact framing is still to be confirmed against source when it is built.
…val ledger No adapter has ever actually called into crew's own approval service before this one: `Adapter::respond_to_approval` is `capability_unsupported` everywhere else today, and `ApprovalRequest`/`ApprovalDecision` already carry everything a durable, replayable approval needs (request, decision, who decided, when). This is a bridge, not a new ledger. The callback ApprovalService::decide invokes once a decision is recorded is a one-shot channel captured at request time, not a `worker_id -> Arc<dyn Adapter>` registry lookup: an approval belongs to a specific in-flight request, not to an adapter instance, and a registry lookup would happily deliver a decision to a NEW instance after a restart that has no pending tool call and no idea what the decision refers to. The channel makes that coupling explicit and lets it die with the request. A dropped receiver -- the run cancelled, the adapter torn down, or claude's own process already exited before a decision arrived -- deliberately never errors the callback: the decision is already durable in the ledger by the time this callback runs, and the vendor call the reply would have unblocked is already moot. Proven directly, not just asserted: acknowledging an approval id nobody registered, and acknowledging one whose receiver was already dropped, both return Ok. The full round trip is tested against a real ApprovalService (not a fake): parse a can_use_tool request, submit it, decide it exactly the way a human answering a dialog would through `approval/decide`, and assert the control-channel reply bytes this adapter would write back are correct -- proving the mechanism end to end, not its two halves in isolation. The control-channel JSON shapes are provisional, flagged in the module's own doc comment on the same basis as `claude_protocol::reconcile`'s own transcript schema -- confirming them against source is part of this spike's own job, not something to guess past. `parse_permission_request`/`build_permission_response`/ `handle_permission_request`/`ProtocolApprovalCallback` have no caller yet -- the control-channel reader that would call them while a real claude process runs does not exist. Flagged with the same `#[allow(dead_code)]` and comment `reconcile.rs` already carries, owed a real caller alongside it.
…two bridges built ahead of it Builds the actual protocol-mode adapter: spawns claude with the settled launch posture (plain -p, both stream-json formats, an explicit --setting-sources naming every source claude would already consult, no --bare or hook-suppressing flag), checks claude's own one-time workspace-trust record before ever spawning it (protocol mode has no terminal to observe a trust gate on, so this has to happen first, by reading the record directly rather than pattern-matching painted text), delivers the initial prompt as the first stream-json input message, and drives the whole turn through a new NDJSON reader loop modeled on the coordination broker's own client-side framing. The reader is the first real caller for two modules built ahead of it: approval_bridge (previously #![allow(dead_code)]) now answers a live can_use_tool request through crew's own approval ledger, and reconcile::find_gaps now runs once a turn ends, against claude's own durable transcript file, journaling either a normal reconciliation count or -- when nothing could be read at all -- a distinguishable protocol-health failure rather than a count of zero that would read as a clean pass. No repair mechanism exists yet: a detected gap is reported, not yet re-journaled, which corrects an earlier, premature doc comment claiming the two counts should always agree. The workspace-trust check gets its own third EscalationRaised reason, distinct from the terminal path's vendorFirstRunGate: this run has no live pane a human could type an answer into, so it fails the run outright rather than leaving it parked, and says so in its own remediation text. Reaching the existing escalation projection from this new construction site meant teaching the shared event-recording path to open an escalations row for any EscalationRaised event it sees, not only the one shape it already special-cased. The approval bridge needed one piece of daemon-wide wiring to actually work end to end: crew has always run every approval decision through a single process-wide callback, defaulted to a no-op. That callback is now the one this bridge already built and proved safe for an unregistered or dropped receiver, so replacing the default costs nothing for every other adapter and gives this one a real path from a decided approval back to the request that is still blocked on it. --model is threaded through as a plain, already-resolved parameter this adapter does not itself resolve -- the run-specific-over-boot-config precedence a correct value needs belongs to the fix already in flight for the shared adapter-construction path, and this adapter is built to inherit that fix once it lands and is merged in, not to duplicate it ahead of time. Explicitly out of scope, same as previously scoped for this piece: a renderer beyond bare legibility, resume/--continue, the other three vendors, and performance work. Also not yet built, and flagged rather than quietly assumed: an init-time check that the actually-launched process matches the intended posture (system/init's own reported fields are still unconfirmed against a live capture, the same provisional caution already on this module's transcript and control-channel shapes), and the pane renderer.
… shared model-resolution fix has landed Merged main in first: the fix threading a run's own resolved model into build_tui_adapter (rather than a stale boot-loaded snapshot) landed while this adapter was mid-build. This adapter already sourced its own model from the same profile.model field that fix threads through as run_model, so nothing about the value it launches with was actually wrong -- but nothing proved it either, and the whole point of that fix's own reproduction (assert against the real launched argv, not a configured value) is that a configured-looking value can be wrong at the one point that matters. So: made the spawned binary and the trust-record path plain, constructor-injected fields, the same shape ClaudeTuiVendor's own cfg.bin already gives it, rather than a PATH-shadowing trick or a cfg(test) branch in the method that actually spawns. A new test then drives Adapter::start for real against a fake claude script that dumps its own received argv before doing anything else, and asserts --model against that file -- proving the wiring reaches a real subprocess, not just that the two ends were built to agree. Also corrected two comments this adapter's own doc carried while that fix was still in flight, both stale now that it has landed: the three-row boot-config-vs-run-model precedence that fix establishes has nothing to apply to here, since protocol mode has no boot-loaded per-vendor config in the first place. cargo fmt, cargo clippy --all-targets --all-features -- -D warnings, the full cargo test --workspace, and the full bun run check (markers, generate --check, format, typecheck, extension build, bun test) all clean.
Closes the one blocking finding from review: the earlier commit claimed "behaviorally a no-op for every other adapter" and cited a test that proves ProtocolApprovalCallback::acknowledge returns Ok for an approval nobody registered. That is a claim about a function; the claim actually made was about every other adapter in the daemon, and nothing exercised a TUI adapter with the real callback installed at all. Checked directly rather than argued: TuiAdapter carries no ApprovalService/ApprovalCallback field, build_tui_adapter's own call sites never pass one, and ApprovalService::request has exactly one caller in the whole tree (the bridge this spike built). So the claim was always going to hold -- but "was asserted, not proven" is the exact substitution this spike exists to catch, in the one commit whose blast radius reaches every adapter. New test drives a full TUI run through AdapterRegistry with the REAL daemon-shaped wiring installed (an ApprovalService backed by ProtocolApprovalCallback, sharing the run's own db, via set_protocol_support -- the identical shape lifecycle.rs uses for the whole daemon), through a real tool call (the fixture script's own tool_use entry), and asserts three things afterward: respond_to_approval still refuses with capability_unsupported, no row was ever inserted into approvals for the run, and no EscalationRaised reached the journal as a side effect of the tool call either. Making this test possible from crates/runtime/tests/ (an ordinary external dependency of the crate, unable to see pub(crate) items) meant widening ProtocolApprovalCallback, the approval_bridge module, and ProtocolSupport/set_protocol_support from pub(crate) to pub -- the same "widen specifically so an external test can prove a production property" precedent claude_protocol::reconcile's own module already set, applied here to a different property. cargo fmt, cargo clippy --all-targets --all-features -- -D warnings, the full cargo test --workspace, and the full bun run check all clean.
The ADR does not exist yet -- by our own sequencing it is written after
this spike reports, and citing it as settled throughout the
implementation assumes the spike's own conclusion. Eleven files carried
the citation, two of them shipped schema text: a protocol doc comment
on AdapterMode (crates/runtime/src/config/crew.rs) and one on
AdapterReconciliationEvent (crates/protocol/src/event.rs) both regenerate
into crew-config.schema.json and packages/protocol-ts/schema/crew.schema.json,
putting a dangling citation plus spike-status narrative into public
schema output.
Every citation is replaced with either the reasoning it stood for
(inline, so it survives whatever the ADR ends up numbered, or whether
it lands at all) or an existing path (docs/adr/0026-headless-retirement.md
for the decision being superseded). Spike-status phrasing ("spike-scoped",
"once the ADR is accepted") is removed from the two doc comments that
ship, not just reworded around the citation -- a schema consumer should
never see internal review-cycle status.
Caught the same stale claim a second time while in event.rs's doc
comment for other reasons: the shipped description for
AdapterReconciliationEvent still said gapsFound and gapsRepaired "are
expected equal," the exact premature claim already corrected in this
crate's own internal event_sink.rs doc comment two commits ago, never
carried over to the shipped copy. Fixed identically here.
Regenerated crew-config.schema.json and
packages/protocol-ts/schema/crew.schema.json via bun run generate and
the documented crewd config print --schema command; no TypeScript
binding file actually changed (description-only edits, no field/type
change).
cargo fmt, cargo clippy --all-targets --all-features -- -D warnings,
the full cargo test --workspace (including the three shipped-description
guard tests in crew-protocol), and the full bun run check (markers,
generate --check, format, typecheck, extension build, bun test) all
clean.
…de-protocol-spike
…isting attach/pane machinery unchanged Adds the pane renderer: when a TuiSupport bundle was ever supplied to the registry, a protocol-mode Claude run now opens a pane the same way a TUI run does -- an AttachServer bound at this run's own socket, a PaneCoordinator::attach call, DisplayPaneAttached/Detached journaled -- except the pane's target is a new, non-PTY AttachTarget that feeds a broadcast sender directly, fed one formatted line per normalized event as the reader classifies it, rather than raw PTY bytes. Absent tui support the adapter runs with no pane at all rather than refusing to start: unlike a TuiAdapter, this adapter's pane is a convenience view, not its control surface, so attach failures (no tui bundle, or AttachServer::start itself failing) are logged and the turn proceeds unaffected either way. The pane states plainly, as its own first line, what it is: a crew-authored report, not claude's own screen. This matters because the whole hazard of a crew-rendered pane is that it looks like evidence -- a human who does not know the difference could cite it as though they watched the vendor directly. The module doc also states the pane's other departure from the TUI path's own precedent: this adapter's lines are unredacted, exactly like the TUI pane's raw PTY bytes are, and for the identical reason (a live view is not the durable record) -- recorded so the next person asking is not left to re-derive it. write_input on the new AttachTarget is a documented no-op: this adapter has no path from a viewer's keystroke back into claude mid- turn. A keystroke is never silently discarded, though -- the attach server's own on_user_input callback writes an immediate, local notice back into the same pane, naming plainly that nothing was sent and this run cannot be steered. Deliberately not RuntimeEvent::OutOfBandInput: that event means a human bypassed the adapter, so the durable journal may not reflect everything the vendor received -- untrue here by construction, since nothing the viewer types ever reaches claude at all. Journaling it anyway would raise a flag asserting a condition that cannot have occurred. The PaneCoordinator construction TuiAdapter's own build already had is extracted into one shared helper, called from both the TUI and protocol-mode branches, rather than maintaining two copies of the same sequence -- the same drift hazard this branch already named for two separately-maintained AdapterMode enums, avoided here rather than repeated. Proven end to end, not just built to agree: a new test drives a real turn (the same fake-claude-script harness used for the model-argv proof) with a real PaneCoordinator and a fake pane backend wired in, and asserts a real DisplayPaneAttached/Detached pair was journaled with the fake backend's own pane ref -- not that the pieces were built to agree, but that wiring them together actually reaches the journal. Root-caused a real failure while writing it: the test's own attach socket, nested under the default system temp directory, overflowed the platform's Unix-socket path-length limit exactly the way an existing comment elsewhere in the tree already warns about; fixed by binding the test's tempdir under /tmp directly, the same override an existing sibling test already uses for the identical reason. Self-caught before commit: extracting the shared PaneCoordinator helper left its own #[allow(too_many_arguments)] attribute attached to the wrong function (the new, five-argument helper, not the thirteen-argument one that still needs it) -- moved to the right one. cargo fmt, cargo clippy --all-targets --all-features -- -D warnings, the full cargo test --workspace, and the full bun run check all clean.
…ffect, and stop reconciliation over-reporting gaps Three fixes from a live-capture round (seven real claude -p calls against claude_code_version 2.1.268, two permission modes, two forcing conditions), reported and ruled on before any of this landed. Permission mode was never pinned by this adapter's own launch argv -- a bare -p launch inherited whatever the operator's personal ~/.claude.json/settings default happened to be, including a value that would make can_use_tool never reach this adapter's own bridge at all. Confirmed live, not inferred: launching with no --permission-mode reported permissionMode: "plan", traced to this machine's own personal defaultMode setting. Load-bearing for permissions by the standing rule that crew passes anything load-bearing explicitly rather than inherits it -- the maintainer ruled on this departure from the original launch-posture wording directly: pin --permission-mode to "auto" (a permissive value, its own semantics otherwise unmeasured by this spike), never the --allow-dangerously-skip-permissions flag, which only makes bypass available without pinning anything. --permission-prompts is pinned too, to its own already-default "host": the flag that decides whether the host is consulted about a permission decision at all is exactly as load-bearing, on the same ground, not a second decision. The init-time posture assertion now checks both, and aborts the turn immediately on either mismatch rather than merely reporting it afterward: claude_code_version against a tested range (system/init carries no settings-sources field at all, so this is the only assertion available for the binary's own compatibility, not the fallback half of a choice), and permissionMode against the pinned value directly -- a real check that the pin took effect, not a proxy inferred from a version being in range. Separately, the same capture found a real transcript carries non-conversational bookkeeping entries (a SessionStart hook's own error, observed as type: "attachment") that do carry a uuid but never appear on the live control-channel stream at all -- so reconciliation was flagging every one of them as a phantom gap. Fixed with a denylist, not an allowlist: a confirmed never-streams kind is excluded from the gap set specifically, while an unrecognized future kind stays gappable. Noisy-but-visible is the correct failure direction for a mechanism whose whole purpose is catching silence; an allowlist would make a genuinely new non-streaming kind's future absence invisible by construction, which is the exact failure this mechanism exists to prevent. The same round also tried, twice, to force a genuine can_use_tool round trip onto this reader's own stdout stream. Neither attempt produced a control_request-shaped frame, but that result does not settle anything about the channel, for two reasons found after the fact: an empty permission_denials on both ask-rule attempts is itself consistent with the ask rule never having matched at all (the documented hostless behaviour denies and reports a match, it does not silently allow one), and separately, no call in the round passed --permission-prompt-tool stdio -- understood only afterward, from SDK source rather than CLI documentation, to be the actual enabler of control-frame permission requests. Absent that flag, a null result here cannot distinguish "no host was asked" from "the mechanism was never turned on." No conclusion about the approval bridge's own integration shape is drawn in this commit; a corrected experiment (a verified forcing condition, the sentinel flag present) is pending separately, reported as it happens rather than assumed here. Two small fixtures added, condensed and scrubbed (not a raw byte-for-byte capture): the one system/permission_denied frame this reader does not yet parse, and one recorded manual-mode turn. Neither is evidence for or against the approval bridge's own premise -- both kept for the real wire shapes captured, with what each does and does not establish written down in fixtures/adapters/claude-protocol/README.md rather than left for a reader to assume from the filename. Neither carries a username, home path, or session id from the machine that captured it. cargo fmt, cargo clippy --all-targets --all-features -- -D warnings, the full cargo test --workspace, the full bun run check, and bun scripts/check-markers.ts all clean.
…ile denials three-way against crew's own ledger, and bound process teardown Wires the mechanism the live capture confirmed (calls 8/9): the bare CLI's can_use_tool control channel needs --permission-prompt-tool stdio, not SDK embedding -- that earlier hypothesis is withdrawn. - launch.rs: pins --permission-prompt-tool stdio, same ground as the mode/prompts pins already there. - reader.rs: drive_turn now sends an initialize control_request first and withholds the actual prompt until its control_response arrives, processing whatever interleaves (hooks, system/init) through the same dispatch either way. A stream that closes before that handshake completes is now a protocol error, not a silent empty turn. - reader.rs/approval_bridge.rs: ledger reconciliation, three-way. Every can_use_tool request this reader bridges into ApprovalService is tracked by tool_use_id, and every real system/permission_denied notice observed on the stream is tracked separately by the same id. The closing result message's permission_denials is checked against both sets: explained by either is expected (the host was asked and answered, or an operator's own rule denied without asking anyone); explained by neither fails the turn closed -- the one case nothing on the stream accounts for. permission-denied-frame.jsonl stops being a documented gap and becomes this check's own test fixture. - adapter.rs: child.wait() after a completed turn is no longer unbounded -- it waits out a grace window, then escalates SIGINT -> SIGTERM -> SIGKILL on the same timings every other supervised process in this daemon uses, so the untested assumption that dropping stdin makes claude exit promptly is never load-bearing. - supervisor/mod.rs: its own module doc comment claimed every adapter spawns through Supervisor, which claude_protocol's bare tokio::process::Command does not -- documented as the exception it is, with the reason, rather than left as a true-sounding invariant with a silent hole in it. Fixture: can-use-tool-round-trip.jsonl adds the reply side, live, against the real CLI -- the two existing fixtures only ever proved the request side or its absence. README updated throughout: the earlier "SDK-embedding" caveat is resolved (confirmed cause: the missing sentinel flag), and the other caveat (an ask rule that never matched) is resolved by the same call's own hard-deny positive control, not left open.
# Conflicts: # crates/runtime/src/adapter/claude_protocol/adapter.rs # crates/runtime/src/adapter/claude_protocol/approval_bridge.rs # crates/runtime/src/adapter/claude_protocol/launch.rs # crates/runtime/src/adapter/claude_protocol/mod.rs # crates/runtime/src/adapter/claude_protocol/reader.rs # crates/runtime/src/adapter/claude_protocol/reconcile.rs # crates/runtime/src/adapter/registry.rs
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.
Continues the protocol control-plane spike after #174 (which this branch's own earlier commits already shipped): the pane renderer, the init-time posture assertion, and the final wiring for what a live capture against the real
claudeCLI confirmed.Pane renderer. When a
TuiSupportbundle is available, a protocol-mode run now gets a crew-authored, read-only pane -- a provenance banner up front, one formatted line per normalized event,write_inputa documented no-op.build_pane_coordinatorextracted once and shared by both the TUI and Protocol adapter-construction paths rather than copied.Live capture, and what it settled. Nine real
claude -pcalls (billed, in a trusted repo) answered the open questions from #174:system/initnever reports which--setting-sourcestook effect, and the operator's own personal settings were silently governingpermissionMode-- both--permission-modeand--permission-promptsare now pinned explicitly, andsystem/init's own reported values are asserted against those pins at turn start, aborting immediately on a mismatch (a CLI-version range check runs the same way).can_use_toolcontrol-channel requests do NOT appear on stdout without--permission-prompt-tool stdio-- an undocumented sentinel flag found in the SDK's own source, not--help. Confirmed live: the same settings and prompt, with only this flag added, produced a realcan_use_toolrequest; a reply built fromapproval_bridge::build_permission_response's own unmodified shape then let the tool actually execute. This reverses an earlier working hypothesis that approval delegation needed SDK embedding rather than the bare CLI.initializecontrol request the SDK always sends first (even though it declares nothing about permissions) is now sent bydrive_turnbefore the actual prompt, which is withheld until the matchingcontrol_responsearrives.resultmessage is now reconciled three ways against what the turn actually saw: explained by acan_use_toolrequest this reader bridged into crew's own approval ledger (asked and answered), explained by a realsystem/permission_deniednotice observed on the stream (an operator's own rule denying without asking anyone), or explained by neither -- the one case nothing on the stream accounts for, which fails the turn closed.child.wait()after a completed turn is now bounded and escalates SIGINT -> SIGTERM -> SIGKILL on the same timings every other supervised process in this daemon uses, rather than relying untested on stdin-drop causing a prompt exit.type: "attachment"transcript entries (SessionStart hook error records) never stream live and no longer get reported as phantom gaps by reconciliation.Three scrubbed fixtures under
fixtures/adapters/claude-protocol/capture the real wire shapes from this round (see that directory's own README for provenance and what each one does and does not establish).Not in scope:
resume/--continue, the other three vendors, performance work.cargo fmt,cargo clippy --all-targets --all-features -- -D warnings,cargo test --workspace, andbun run checkall clean.