Seller-tool onboarding: Proxy swap route, Holder route in the daemon, holder kit - #1004
Merged
Merged
Conversation
…d-policy mapping Paper artifacts only, no runtime changes. Anchored in plan v3 (1c35ba32...) and advisor verdict (44377fb0...).
Survey of upstream/main d7b94db: seller config, job lifecycle, MCP and credential/sandbox surfaces, with named gaps G-1..G-7. Paper only.
Walk A routes to rung 4, supported conditional on separable auth-file writes. Walk B routes to rung 3 and concludes deferred, with five named reasons the mapping contract does not transfer to HTTP bodies.
… register Completes the stage-0 paper contract: planned check map for plan v3 5.1-5.12, runtime negative-control mutants, evidence bundle layout with pre-recorded oracles, and gaps G-1..G-7 plus contract gaps C-1..C-7. Records the ten explicitly unsupported/deferred cases and the eight supported-profile requirements. No runtime changes.
Corrects two material errors found by the stage-0 verdict, both
re-verified first-hand against the base source:
- credential custody is NOT host-held injection: seller_exec.rs:2575
starts a per-job host credential proxy that forwards a placeholder and
fails closed rather than putting the real credential in the container.
G-6 narrowed to the genuinely absent pieces.
- timeout DOES reach Failed via fail_job_with_feedback -> fail_job; the
real hazard is that the write is best-effort and restart re-drives
non-terminal rows treating a missing deadline as live.
Adds the award-authority analysis (Awarded::{New,Duplicate,NoClaim},
suppression and ACCEPT reach the same store method) showing a
record_award hook is unsafe, the MCP attachment gap (mcp_servers is
Vec::new() at seller_exec.rs:2408), and the three named core adapters.
F1: authorization adapter moved off record_award to the owned-award / eligible-execution boundary in the Awarded::New arm after AwardMatch::Execute. Five positive preconditions, explicit no-grant list, arm-by-arm treatment, two-phase idempotent open, and a durable holder admission record supplying the service/grant binding the jobs table lacks. F2: durable monotonic admission state with close reason; fixed close ordering (commit closing -> deny -> reservations -> revoke/stop/remove -> commit closed) since termination is not transactional with SQLite; holder-enforced expiry independent of the marketplace process; boot reconciliation that fails closed; reservations forfeited without proof of non-occurrence; no write auto-replay. F6: party_scope defined as same-party same-vendor concurrent jobs only; no cross-party authority inferable from shared-holder.
…ests, gaps F4: replace the vendor-request-counter oracle with three independent observers (harness-owned child-start counter, vendor request counter, forbidden-effect markers). Reject-before-invocation now requires validation error AND zero child starts AND zero vendor effects. Adds a mutant that launches a child then rejects without contacting the vendor, and removes the unjustified cleanup dependent-skip. F5: holder-issued handles and slots immutably bound to holder/party/job with an admission membership check, plus the valid-cross-job-handle negative case. Effect maxima derived from bindings and enforceable, with input/output/network counted separately; page/item ceilings made consistent. Walk A classification conditional on A1-A5, not A5 alone. F6: removes the universal single-file-output requirement (plan requires private checked output slots); single-file stays walk A and the demo only. party_scope defined as same-party same-vendor. Splits router results into unsupported / deferred / manual-setup-required. Corrects C-1/C-4/C-7 to name missing enforcement rather than imply the plan permits weaker rules. Real-tool needs no longer read as blocking the authorized synthetic demo. Adds integration checks 13 (owned-award admission) and 14 (durable close and reconciliation) for F1/F2.
Per Petar 2026-09-09: the offering is per seller, not per job, and the tool is active together with the seller daemon. So tool-holderd enrols once at startup, reuses an existing session if one is already persisted, and stays available until the process stops. No award, payment, job start or job completion opens, closes, renews or revokes anything; detach_job explicitly reports that the tool stayed enrolled. Jobs are addressed, not entitled: attach_job creates a per-job Unix socket and records that job directory, so a connection carries its job identity from the listener it reached rather than from the request body. Reserved argument names (job_id, job_root, cwd, home) are refused rather than ignored. Five binaries: vendor-service (fake authenticated vendor, owns the request counters that serve as the independent oracle), vendor-cli (the tool being onboarded; persists its login in its own home and never takes a credential on argv), tool-holderd (the holder), holderctl (seller operator CLI) and tool-mcp-bridge (real MCP JSON-RPC over stdio inside a job container, proxying to the per-job socket). Credential containment is structural: vendor-cli runs as a child of the holder with a cleared environment pointing at holder-private state, so a job container is given only its own socket and its own directory. Dependency surface is serde plus std, nothing else, because the audit surface of a credential holder should be readable in one sitting.
fixture_suite (6): one enrolment serving two sequential jobs with the vendor login_count staying 1; the operation list identical on both job endpoints and on the control endpoint; a real MCP stdio session driving the tool through the bridge; daemon restart reusing the persisted session with zero further logins; availability following the daemon and nothing else; and the credential staying out of job directories (session file 0600 under holder state, job tree greppably free of secret and token). negative_controls (26): the parameter grammar and job-directory confinement, each asserting the reason for the refusal rather than that something failed, plus a positive control so a validator that rejected everything could not pass; live-endpoint refusals for reserved arguments, tool calls on the control endpoint, cross-job access through MCP by absolute path, traversal and output redirection; the output ceiling; and a vendor-side revocation becoming visibly unhealthy, failing closed, then recovering through re-enrolment. Claims about calls are checked against the vendor service counters, never the holder self-report. Two real defects the run surfaced, both fixed here rather than worked around: the holder runtime directory must be short because a Unix socket path is capped at SUN_LEN (104 bytes on macOS), which macOS temp paths silently exceed for per-job sockets while the control socket still fits; and a flag-like text probe longer than max_len trips the length rule first, so the test was asserting a rule it never reached.
Explicit dependency versions instead of workspace inheritance so the crate compiles from its own directory with no parent manifest. That lets the image build from a context containing only this crate, which keeps the build honest about the dependency surface: if the holder needed anything from maxplayer-core, this build would fail rather than quietly pick it up. One image carries all five binaries and serves every role in the demo. The roles differ by entrypoint and, far more importantly, by what is mounted into them. Verified: cargo test -p maxplayer-tool-kit still 32/32 after the manifest change, and docker build succeeds on colima/aarch64 (image maxplayer-tool-kit:demo, id 148697048a3a).
…kit binaries Cargo.lock records maxplayer-tool-kit as a workspace member. The Dockerfile builds all five binaries from the crate directory alone; NOT yet built or run here.
Each attached job now gets runtime/jobs/<job_id>/job.sock in its own 0700 directory, instead of a flat runtime/jobs/<job_id>.sock. The reason is the container demo: a job container must receive exactly its own endpoint. With a flat layout the only mountable unit is the directory holding every job socket, so per-job isolation could not be expressed as a mount boundary at all. One directory per job makes the isolation a property of the topology rather than a promise in a document. cargo test -p maxplayer-tool-kit: 32/32 after the change.
…p screen No binary in the kit handles --help: both CLIs print a usage line to stderr and exit non-zero on bad arguments, so CMD ["holderctl","--help"] made the image fail by default. The default command now names what the image carries, prints both usage lines and exits 0. Verified by rebuild: default run exits 0.
Runs the holder across two sequential job containers on Linux and writes an evidence directory: per-step verdicts, both MCP transcripts, holder and vendor logs, vendor counter snapshots, and a manifest. Two properties the in-process tests could not express: Job containers run with --network none and receive exactly two mounts, their own working directory and their own single-socket directory. A job container therefore could not reach the vendor even if it held the credential, and the per-job socket layout is what makes that mountable. The vendor port is published on host loopback, so every counter used as evidence is read from outside the system under test. The holder status output is recorded but is never the sole basis for a call-count claim. The manifest states mechanism_only with the limits spelled out: the CLI and the vendor were both written for this contract and cannot falsify it, so this is not independent real-tool acceptance. Not yet executed; committing before the run because the last three turns died mid-flight.
linux/arm64, docker 29.5.2 under colima. Full artefacts: both MCP transcripts, holder and vendor logs, vendor counter snapshots at four points, per-step verdicts, manifest. The load-bearing numbers, all read from the host rather than from the holder: login_count 1 across two sequential jobs and across a daemon restart, transform_count 2, auth_failures 0 until the revocation step deliberately caused one, login_count 2 only after explicit re-enrolment. Audited before committing: no credential, session token or secret-shaped string appears in any artefact. The per-run secret is generated on the host, mounted read-only, and removed on exit. The manifest records mechanism_only with its limits stated rather than implied.
The scope correction moved the tool from per-job grant to per-seller enrolment, which removes the premise of 04 Part I entirely: the award boundary is not the tool-lifecycle boundary, so the careful argument about which award edge to hook was answering a question that should not have been asked. Annotated rather than deleted. The reasoning is a record of how the wrong model was reached, and 04 Parts II onward — custody, containment, persistence, re-enrolment — were never about awards and are what the implementation honours. 00 also corrects its own status line: "paper contract only" was true when written and is not now. The epistemic caution survives the change and is restated, because a fake vendor and a CLI written for it still cannot establish third-party acceptance.
01 §2: the survey findings are verified and stand; what is withdrawn is the inference drawn from them. "The job record has no grant" was written as a gap to close and is in fact the correct shape. 02: the layering stack loses its third layer. A job endpoint and directory narrow where a call may act but are not a layer of the offering. grant_policy is marked withdrawn in place rather than cut, so the surrounding schema still reads. 05 Step 5 and 07 check 13: superseded outright, with what replaced them named — attach rather than mint, and validation plus confinement enforced on every call rather than a grant checked per call. 07 points at the tests that actually run. 06 Step 4: doubly inert, since walk B was already deferred at rung 3. Its tenant-isolation question is untouched and still open.
Two demo runs started twelve seconds apart and both were committed, which my previous commit message described as "the container run" as though there were one. The overlap was an accident of a turn killed mid-flight by a provider error, not a deliberate second run. Kept both rather than pruning one: deleting an inconvenient artifact is a worse habit than explaining it, and the two runs are separately networked and volumed, agree on all 27 checks, and so corroborate each other. The caveat is stated in the README rather than left implicit — sharing one Docker daemon means they are not independent in the strong sense.
The previous version blamed the duplicate bundle on a turn killed by a provider error. That was a guess written as fact. Two sessions were writing this worktree and branch at the time; the second was ordered to stop on 2026-09-09 15:06 PDT and this author made sole writer, which makes the other session the likelier source of the second run. Attribution is not recoverable from the artifacts: both sessions committed under the worktree identity and nothing records the launching process. The file now separates what the evidence supports (two runs, separate networks and volumes, same daemon, both 27/27) from what it does not (which session ran which), instead of filling the gap. Also verified this turn, since the branch carries two commits this author did not write: c4d02ba (Cargo.lock workspace entry) leaves the lock consistent under --locked --offline, and 2041c93 (image default command) does exit 0 on a fresh build, as it claimed. Both fixed real defects, 2041c93 in this authors own Dockerfile. Tests still 32/32 at HEAD.
… gates Advisor F5: the banner claimed "Parts II onward stand", but those parts carry admission rows, holder-enforced grant expiry, per-call token verification, per-job durable budgets, and a rule that only a newly authorized job may run after re-enrolment. Those contradict per-seller enrolment and same-job recovery, so the global withdrawal notice was not enough on its own. Each contradictory section is now marked in place, so a reader landing mid-document cannot mistake a withdrawn gate for a live requirement. What is retained is named exactly: trust boundary, credential custody, job-directory confinement, session persistence, re-enrolment. Per the fix order these are withdrawn requirements, not a backlog, and are not to be implemented. Two claims of my own are corrected rather than left standing: file confinement is NOT safe against a checked path being replaced between validation and use (F2), and the container evidence for credential absence and for the stop/restore lifecycle is under repair (F1, F3) and must not be cited as established. fixtures/README.md pointed at a scripts/demo.sh that does not exist, and blurred two different credential paths under "generated at run time". The demo derives a fresh random secret per run; the Rust fixture writes a fixed synthetic literal that lives in the test source. Stated without reproducing the value. Remaining F5 item, deliberately deferred to F1: demo.sh:88 and :306 still claim the credential is never on a command line. That claim is false today because of the -e NEEDLE probe, and it becomes true when F1 removes the injection. Correcting the prose before removing the defect would be the wrong order.
…captures Advisor F4: the image was built from mutable rust/debian tags with an unlocked cargo build, so the same Dockerfile produced different images on different days and no acceptance run could identify what it tested. Both stages are now pinned by digest (rust@sha256:ebd900ba…, debian@sha256:88200866…), resolved on linux/arm64 and recorded in the Dockerfile with the rustc version they carry. Updating a base image is now a visible edit rather than a silent drift. Cargo.lock is generated standalone for the crate and copied into the crate-only build context, so the image resolves no versions of its own and --locked fails rather than quietly updating a dependency. 11 packages. Verified, not assumed: docker build --no-cache with both pins and --locked exits 0; image sha256:0fece64074862f51d7b51a9547a8a340f67ff134ede2c350b6f c6409b7b263b7 carries all five binaries and the fixture config. .gitignore:18 (*.jsonl) had silently swallowed every raw MCP transcript the demo recorded, leaving summary PASS lines standing in for observations nobody could re-read. Evidence bundles are now excepted and the eight dropped captures from both 2026-09-09 runs are retained. They are the weaker, pre-repair evidence and are kept as superseded record, not as proof: F1 and F3 invalidate the credential-absence and lifecycle claims those runs made. Still owed on F4 and deliberately not claimed yet: the source-to-build receipt and recording the loaded image ID inside the demo manifest. That lands with the F1 rewrite of the same script.
… KNOWN DEFECT
WIP. Handoff stopped implementation mid-change on human order. This commit
is recorded so nothing is lost; it is NOT verified and MUST NOT be read as
a working fix.
KNOWN DEFECT, INTRODUCED BY THIS COMMIT AND NOT FIXED: $RUN_TMP is used in
the new code and is never defined anywhere in the script. demo.sh runs
under `set -euo pipefail`, so the first expansion aborts the run. The
script is broken-on-arrival until a scratch directory is defined. The
existing host scratch dir is $HOSTDIR ("$HOME/.mtk-demo/$RUN_ID", created
mode 0700 and removed by the EXIT trap); pointing RUN_TMP at a subdirectory
of it, or substituting $HOSTDIR, is the intended one-line repair. I found
this by grepping for the definition after writing the code, and stopped
before fixing it.
NOT DONE: bash -n passes, but that only proves syntax. The demo has NOT
been executed since this change. No check in it has been observed to pass.
The previously reported 27/27 belongs to the OLD script at a5e6880 and does
not describe this file.
What the change intends, and why (advisor F1). The old absence check passed
the live secret into the probe container as -e NEEDLE and grepped from
inside, so the credential was placed in the very container whose
cleanliness was the claim; `grep -rl ... || echo NOT_FOUND` printed
NOT_FOUND on any non-match exit including a scan that never ran; and a
trailing `|| true` swallowed docker failures, so an unstarted container
also read as absent. The check as written COULD NOT FAIL. This is my own
defect, conceded before review pressed on it.
The replacement exports the job container filesystem to the host with no
secret in its environment, scans on the host where the secret already
lives, treats capture failure as fatal instead of as absence, and adds a
negative control that plants the secret into a copy of the same archive and
asserts the SAME scanner reports 1 hit. Whether it behaves as intended is
unknown and untested.
Also still false and deliberately left alone: demo.sh:88 and the manifest
credential line still claim the secret is "never on a command line". That
prose was to be corrected once the injection was gone; the injection is
gone from the source but unverified, so the claim stays flagged rather than
quietly reworded.
Human order: stop implementing, hand off to another agent. This commit makes the branch self-contained so a stranger can continue without access to the authoring machine. docs/handoff/README.md states head/base/remote, the architecture and file map, F1-F5 status with the full review findings, build and run commands, image digests and lockfile facts, production integration gaps, the owed skill/templates/examples, and the seller-daemon scope with Petar routing decisions preserved but unactioned. It keeps one distinction sharp, because reports from here previously blurred it: tests actually executed (cargo 32/32, and the pinned --locked --no-cache build exiting 0) versus checks that were invalid or never ran. Named as invalid: the old credential-absence check, which passed the secret into the probe container, printed NOT_FOUND on any non-match exit and swallowed docker failures - it could not fail. Named as never run: everything in the F1 rewrite, and the demo since 11e6dee. The 27/27 belongs to the old script at a5e6880 and does not describe the current file. It also documents, prominently, that 11e6dee leaves demo.sh broken on arrival ($RUN_TMP used and never defined, under set -euo pipefail) and gives the one-line repair, rather than leaving the recipient to discover it. The five referenced documents are copied into docs/handoff/reference/ rather than cited by local path, since paths on this machine are unreachable to the recipient and therefore worthless: both advisor verdicts, the governing scope correction (sha256 verified 683da095... after copy), the implementation brief and plan v3. Each gets a provenance header; content is otherwise unmodified. A private seat memory file was excluded deliberately as a private log, with its load-bearing content restated in the handoff. Scanned before commit: no credentials, tokens, keys or private seat logs in any staged file.
The holder used to canonicalize a job-supplied path, check it, and return a string that the vendor CLI re-opened later. A job could swap the checked name, or a parent directory, for a symlink between the check and the open (advisor F2). The fix removes the re-open: - validate_call checks grammar and confinement by name only; it no longer touches the filesystem or returns a canonicalized path. - src/safeio.rs opens each file with openat + O_NOFOLLOW on every component, so a symlink on any component is refused rather than followed. This adds libc, the one dependency beyond serde, because std has no openat. - The holder copies an input into a private staging directory the job cannot reach, runs the CLI against the staged copy, and publishes the output with a no-follow create. Tests in negative_controls.rs prove the boundary: a symlink planted after validation is refused, at the unit level and at the live endpoint, and the outside file is neither read nor written. Also two clippy cleanups to keep the crate warning-clean. 35 tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
F1 - credential absence: - Define RUN_TMP, which was used but never set; the script aborted on its first expansion under set -euo pipefail. - capture_job_fs: exclude *.sock, so tar does not fail on the job's own socket. - scan_capture_for_secret: separate grep's clean no-match (exit 1) from a real scanner error (exit >=2). A no-match must not abort the run under pipefail, and an error must not read as "0 hits", which would manufacture a false absence. F3 - lifecycle: - Re-attach the job after each restart and prove call, loss, and restore on a live endpoint, not on an endpoint that was already detached. - Compare the full parsed tool list across both jobs and the seller control view, instead of a regex prefix that stopped at the first bracket. F4 - reproducibility: - Add a build receipt to manifest.json: source commit, crate tree object, lockfile and Dockerfile hashes, base-image digests, the built image id, and a hash of every raw capture. Demo run: 39 checks, 0 failures. The two F1 bugs above were latent in the earlier WIP and surfaced only once the demo could run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Deliverables owed by the ordering seat, for onboarding a new vendor tool. - .claude/skills/seller-tool-onboarding/SKILL.md: the repeatable procedure. It states the per-seller model, the steps, the safety invariants, how to test, and what is out of scope. - templates/seller-tool-config.template.json: a skeleton of the real config the holder loads (config.rs), with placeholders. - templates/README.md: a field-by-field guide, the safety rules the holder enforces, a runnable example (the text-transform config), and an illustrative example that maps a choice and a text parameter. Written in Simplified Technical English. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- docs/handoff/CONTINUATION-2026-09-10.md: continues the handoff README. It records how each finding was closed, the verification results, the owed kit, and a precise production-integration plan with confirmed insertion points. - 04-token-grant-contract.md: replace the "F2 not yet safe, F1/F3 under repair" status with the fixed state, and cite the safeio module and the new controls. Written in Simplified Technical English. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
evidence/20260910T110320Z is a run of the repaired demo: 39 checks, 0 failures. It demonstrates F1, F2, and F3 end to end. Its CAVEAT.md records that the image was built offline from rust:1-bookworm, not the pinned digests, because the Docker buildkit resolver was wedged on the pinned base metadata; the bundle stands for mechanism, and the F4 digest-pinned build must be re-run once registry resolution works. - README.md: mark the current bundle, and mark the two 2026-09-09 bundles superseded (pre-repair). The old ones are kept, not deleted. - .gitignore: keep the multi-MB filesystem-capture tar out of the repository; the scan summary and the transcript hashes are the evidence. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Petar's decision, 2026-09-10. Browser-based authentication is not supported for now. It fits the enroll-once holder model only when the vendor login persists and the holder can refresh it without a browser. A vendor that issues only short-lived, non-refreshable tokens would force a per-job re-login, which the enroll-once-per-daemon model cannot hold. It becomes supportable when a concrete vendor offers a refreshable browser session; the holder already accommodates that. Recorded in the continuation handoff, the onboarding skill, and 08-gaps C.2. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A code-grounded plan for wiring the holder into the product. Every site was read in the source on 2026-09-10. - Step 1: supervise tool-holderd from SellerNode::boot_with_lock (run.rs:3991) and the shutdown seam; hold the handle on the node. - Step 2: add an optional HeldToolConfig to SellerConfig (home.rs:193). - Step 3: mount the per-job socket via launch_with_mounts extra_mounts (seller_exec.rs:711) at /run/holder, nothing else. - Step 4: a JobToolEndpoint RAII guard that attaches on new and detaches on Drop, mirroring NetnsHolder and JobContainer. - Step 5: set mcp_servers to one tool-mcp-bridge McpServer (seller_exec.rs:2411); bake the bridge into the sandbox image. Includes the feature gate (held_tool = None behaves as today), the credential and egress boundary, a test plan, and open decisions. Linked from the continuation handoff and the spec index. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…-09-14 One holder container, two jobs on one enrolment (one under egress containment), an escape refused, a daemon-style restart that resumed the persisted login, and a real claude-agent-acp turn that called the seller's tool through tool-mcp-bridge over the job's socket - all through HeldTool::start/attach/shutdown, prepare_launch, launch_with_mounts and the cleanup capture, with the kit's fake vendor as the oracle. The synthetic secret appears in no file. The README states what each run proves, the facts, one thing learned about docker volumes, the limits, and how to rerun. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…et and one bridge per tool `[sandbox.held_tool]` becomes the list `[[sandbox.held_tools]]`. Each entry carries a required, unique `server_name`: the name the agent addresses (`mcp__<name>__<operation>`), the name of the holder container and its two volumes (`maxplayer-held-tool-<seat>-<name>`), and the name of the job's socket directory (`/run/holder/<name>`). Config resolution refuses a name that is not plain, a duplicate, or one a `[[sandbox.mcp_tools]]` entry also claims, because both lists land on the job's session. - `held_tool.rs`: `holder_names(seat, server_name)`; `job_socket_mount` / `job_socket_path` per tool; `JobToolEndpoint::attachments` mounts the tool's `jobs/<job>` subpath at its own path and gives the bridge `--socket /run/holder/<name>/job.sock` as an ARGUMENT - a stdio server's arguments reach the child on every harness, its environment only if the harness maps it. - `tool-mcp-bridge` (kit) gains `--socket <path>`; the environment form stays for the demo. - `seller_exec.rs`: `JobAttachments::merge` joins the per-tool attachments into the one list the launch takes; the validation of the list at config resolution. - `seller_node/run.rs`: the runner holds `Vec<Arc<HeldTool>>`; boot starts every holder concurrently, applies the fail posture per tool, and stops the holders that did start when a required one refuses the boot; attach and detach loop over the tools on both job paths; shutdown loops too. Tests: the unit tests follow the names; the config tests cover the list, the missing name, the duplicate and the cross-list clash; the live tests now run TWO tools - two holders enrolled once each, one job driving both through their own sockets, a contained job, a restart resuming both logins, and a real claude-agent-acp turn calling both tools. Both green on 2026-09-14 (bundle `evidence/20260914T103608Z-holder-route-two-tools/`, committed next). The sandbox image was rebuilt so its bridge understands the flag. Docs: the template comment, the quickstart, the skill, doc 09's table, doc 10's row, the kit's templates README, and CONTINUATION section 11. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Two holders enrolled once each; one job drove both tools through their own socket mounts; a job under egress containment; a restart that resumed both logins; and a real claude-agent-acp turn that called both tools, each vendor seeing one login and one transform. The kit's fake vendors are the oracle, so this is the mechanism through production code. Neither synthetic secret appears in any file. The evidence index names this the current Holder bundle and keeps the single-tool one as the record of its run. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
main brought the resolver file for contained jobs (`JobLaunch::resolv_conf`, `[sandbox] dns_servers`, the DNS and egress live gates) and the relay-push authority work. This branch brought the tool attachments (`JobLaunch::mcp_servers`, `[sandbox] mcp_tools`, `[sandbox] held_tools`, `ExtraMount`, `JobAttachments`). Four files conflicted, and every hunk was the same shape: both sides added a field, or a literal's line, at the same spot. Both were kept everywhere. Two follow-ups the merge needed beyond keeping both sides: the resolver field landed after the `JobLaunch` struct's closing brace and was moved back inside it; main's DNS live gate builds `ExtraMount::Bind` values for the launch, which takes typed mounts now. Verified on the merged tree: every CI feature set compiles; both routes ran live again under egress containment and passed (the Holder route with two tools, the Proxy swap against GitHub). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…FOs, stream SSE in the bridge Review round 2 of pull request #1004, findings 3, 4 and 8. Finding 3. A job connection resolved its job id through the holder's table at call time. A connection held across a detach and a re-attach of the same id read and wrote the new attachment's directory. Now each attach creates one immutable `Attachment` instance. The accept loop binds every connection to that instance. Each call checks the instance's `stop` flag before validation, before the tool runs, and before each publish. A live id cannot be attached a second time. Finding 4. A job-planted FIFO at an input or output name blocked a holder thread in `open`, and an output FIFO could receive bytes after detach. `safeio` now opens with `O_NONBLOCK`, checks the held descriptor with `fstat`, and refuses everything that is not a regular file before any read or truncation. Connections per attachment are bounded (16). An idle job connection ends 30 s after a detach. Finding 8. `mcp-http-bridge` read a response to EOF before it wrote anything, and read stdin only between responses. A server request sent in the middle of an SSE stream could never be answered. The bridge now runs one worker per stdin message, reads the response as it arrives (`http::request_streaming`, `SseSplitter`), and writes each JSON-RPC message as soon as its event is complete. A stdin line is classified as a request, a notification, or a response. A response's `202` with an empty body draws no error line. `vendor-mcp --server-request` and the `swap-proxy` test double stream too, so the suite can prove the exchange. Tests: 74 pass (`cargo test -p maxplayer-tool-kit --locked`), clippy clean. New: `tests/attachment_binding.rs` (six, against the real daemon), six `safeio` unit tests with `mkfifo`, two `proxy_swap_suite` tests, and unit tests for the streaming reader, the SSE splitter, and the message classifier. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ins, refuse tools under launcher Review round 2 of pull request #1004, findings 1, 2, 9 and 11. Finding 1 (DENY). The proxy substituted a placeholder in every request header. A job could put its placeholder in a header the vendor reflects into a response body, and a `\u`-escaped reflection returned the credential past the byte scrubber. `JobCredential` gains `substitute_in: HeaderScope`. An MCP tool registers with `HeaderScope::authorization()`: the placeholder is recognized and substituted in the `Authorization` header only, every other header goes to the vendor as the job wrote it, and a placeholder that appears only outside the scope is `NoKnownPlaceholder`. Env and file credentials keep `HeaderScope::Any`, the behavior they had. A scope that names no header is refused at registration. Finding 2. `allows_paired_redirect` compared authorities and treated `:443` and `:80` as equal, so a redirect from `https` to `http` on the same host was approved. It now compares origins: scheme, lowercased host, effective port. `same_authority` no longer equates two different explicit ports. Finding 9. Launcher mode accepted `mcp_tools` and `held_tools` and served neither. Both tables now require `mode = "docker"`. Finding 11. Live test A read the credential's absence off the redacted diagnostics capture, which proves the redactor, not the boundary. It now also reads `docker logs` of the job container raw, before the redacting capture, and asserts absence there. Tests: the stub vendor of the round-trip test records a reflected header and sees the placeholder; new proxy tests for the scope, the origin comparison, and the port rule; `launcher_mode_refuses_both_tool_tables`. Core gates on this tree: 449, 498, 1564 and 1629 pass across the four feature rows. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…it, bound every docker call Review round 2 of pull request #1004, findings 5, 6, 7 and 10. Finding 5. A start that failed after `docker run` (the status wait, the subpath probe) returned before any guard owned the container. An enrolled holder stayed running with no owner. `HeldTool::start` now arms a `StartGuard` before its first `docker` call and runs the rest in `start_owned`. A failure removes the container and the runtime volume on the async path; a cancellation removes them from the guard's drop. The state volume stays. Finding 6. `shutdown` set `stopped`, and `detach` set `detached`, before the docker call. A failure disabled the drop fallback, and `shutdown` reported a stop that had not happened. Both flags now follow a confirmed result. `remove_container` is `Ok` only when the container is gone. Both return `Result`, and the daemon logs an `Err`. Finding 7. Every `docker` call ran with no deadline. `run_bounded` spawns the child, drains both pipes on threads, polls for exit, and kills it at the deadline. Queries get 20 s, `holderctl` calls 30 s, `docker run` 120 s, `docker rm` 60 s. The drop fallbacks run through it too. Finding 10. A boot removed only the configured holders' stale containers. A holder whose tool was removed or renamed while the daemon was killed stayed enrolled. `reconcile_stale_holders` runs at boot on a docker seat: it lists the seat's holders by label, removes every one the config no longer names, and keeps their state volumes. Tests: `stale_holders_are_this_seats_unconfigured_holders_and_nothing_else`, `a_bounded_docker_call_is_killed_at_its_deadline`. The Holder live proof with two tools passed on this code and the rebuilt holder image on 2026-09-15. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…e reflection limit Review round 2 of pull request #1004 (a Codex agent, 2026-09-15): eleven findings, all agreed and fixed in the three commits before this one. Section 12 of the continuation document lists each finding, its fix, and the test that proves it, then the reruns on the fixed tree: the kit (74), the four core rows (449 / 498 / 1564 / 1629), the CLI rows, both rebuilt images, the Holder live proof with two tools, and the GitHub acceptance of the Proxy swap route with the streaming bridge. The routing document, the skill and the quickstart now say what the proxy does: it substitutes in the `Authorization` header only, for the vendor's host only, and it does not read a response body. The residual risk is a vendor that reflects its own `Authorization` header into a body. Such a vendor is not to be onboarded on this route. The GitHub evidence README separates the raw observations (the container's inspect, argv, session entry, transcript, and now its raw logs) from the redacted diagnostics capture, whose absence check proves the redactor and not the boundary. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
Author
|
Review round 2 (2026-09-15): all eleven findings are accepted and fixed in four commits on this branch.
Gates on this tree: kit 74; core 449 / 498 / 1564 / 1629; CLI 161 of 162 and 199 of 200. The one CLI failure is 🤖 Generated with Claude Code |
…n inspect as unknown, fail a held pipe Review round 3 of pull request #1004 (the re-review of round 2), findings 2, 3, 4, 7 and 8, and the cancelled-attach gap round 2 left open. Finding 2. `StartGuard` removed the holder's resources at once when the start future was dropped, while `docker run` was still creating the holder on the blocking pool; the container then appeared with no owner. `OwnedCall` replaces the guard: the blocking task records when the call ended, the future's owner records the abandonment, and whichever comes second runs the cleanup, so it runs exactly once and only after the effect exists. Every call in `start_owned` goes through it. `attach` owns its attachment the same way: a holder that attached the job after the daemon stopped waiting is told to detach it. Finding 3. `container_exists` read any non-zero `docker inspect` as "absent", so a docker daemon that was down let `shutdown` mark a running holder stopped and disarm its fallback. `inspect_outcome` now reads exit 0 as present, the daemon's own "no such" words as absent, and anything else as an error. Finding 4. `run_bounded` collected the child's pipes for two seconds and replaced a timeout with empty output and a success code; a descendant that held a pipe made `docker ps` read as empty and the reconcile remove nothing. The child now runs in its own process group, one deadline covers the run and the collection, a pipe held past it is an error, and the group is killed so the descendant does not outlive the call. Finding 7. The boot reconciled stale holders only on a docker seat. It now reconciles whatever the mode is, because the holders to remove are the previous configuration's; a host with no docker CLI is quiet. Finding 8. Live test A asserts that `docker logs` succeeded before it scans the raw logs. The evidence README says that run B has no raw observation of its own. Tests: `a_descendant_that_holds_the_pipes_fails_the_call_instead_of_emptying_its_output`, `inspect_words_decide_presence_and_an_unknown_answer_is_an_error`, `an_owned_call_dropped_in_flight_cleans_up_exactly_once_after_the_child_ends`, `an_owned_call_that_completes_cleans_up_only_when_it_stays_armed`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…rker at its answer, expire idle connections Review round 3 of pull request #1004 (the re-review of round 2), findings 1, 5 and 6. Finding 1. A call that passed the last detach check could pause before it wrote; detach completed, the same job id attached again at the same pathname, and the old call wrote into the new attachment's directory. `Attachment` gains a publish lock. Every staged output is read into memory first, outside the lock. The `stop` check and the writes are one unit under the lock. `stop_attachment` sets `stop`, then takes and releases the lock, so it waits for a publication in flight before it removes the socket and returns. After detach returns, no old call can write. Finding 5. A vendor that sent the final response and then kept its SSE stream open left the bridge's worker reading forever, and every request added a worker and a connection. A request worker now stops reading and drops its connection once its answer is written. Request workers are bounded (32): a request over the bound gets one immediate error line on its id. Notifications and responses are never bounded, so an answer to a server request always goes out. `vendor-mcp` gains `--hold-stream` and `--delay-ms` for the tests. Finding 6. The holder's read timeout only checked for a detach and continued, so silent connections on an attached job held every slot. `tool-mcp-bridge` opens one connection per message, so idle expiry is safe: a job connection with no complete request within the idle timeout is closed, attached or not. New daemon flag `--job-idle-timeout-secs` (default 30). Tests: 78 pass (was 74), clippy clean. New: `attachment_binding::a_call_that_pauses_before_publishing_cannot_write_after_the_detach`, `attachment_binding::silent_connections_expire_after_the_idle_timeout`, `proxy_swap_suite::a_request_worker_ends_when_its_answer_arrives_even_if_the_vendor_holds_the_stream`, `proxy_swap_suite::requests_over_the_in_flight_bound_get_one_error_line_and_the_rest_are_served`. The `HEAD` daemon fails both holder tests as the reviewer described. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…nt mode; record review round 3 Review round 3 of pull request #1004, finding 7, completed. The reconcile of stale holders at boot ran only on a docker seat, so a seat that left docker mode or dropped `[sandbox]` kept the holders of its previous configuration. The mode is no longer the test: a marker file in the seat's home (`held-tools-started`) is written when holders start and removed once a boot with no held tools has reconciled. The reconcile runs when the config names held tools or the marker exists, so a seat that never held a tool makes no `docker` call at boot (an unconditional call at every boot, including every test boot, was the first attempt, and it is not needed). The handoff document's section 12 gains the round-3 table (eight findings, all agreed and fixed), the reruns on the fixed tree, and the closed cancelled-attach item. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
Author
|
Review round 3 (2026-09-15): all eight findings on the round-2 fixes are accepted and fixed in three commits,
Gates on 🤖 Generated with Claude Code |
Review round 4 of pull request #1004, the one non-blocking follow-up. The attach error path ran a best-effort detach for every failure, so a duplicate attach that the holder refused ("already attached; detach it first") detached the attachment that already existed and belonged to someone else. `attach_answer` now classifies the call: the holder attached (exit 0), the holder refused (a non-zero exit: nothing was attached by this call, nothing to take back), or unknown (the call did not complete: killed at its deadline, or `docker` failed), and only the unknown case runs the best-effort detach. No normal daemon path issues a duplicate attach; the fix keeps the endpoint safe if one ever does. Test: `an_attach_the_holder_refused_is_not_taken_back_but_an_unknown_one_is`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…low-up, the red money-path check Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
Author
|
Review round 4 (2026-09-16): the one follow-up is fixed, and CI is green on the current head
The red Money-path check on 🤖 Generated with Claude Code |
pmilic021
added a commit
that referenced
this pull request
Sep 16, 2026
release: v0.5.9 notes — add #1004
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
1 participant
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.
What this delivers
A seller can offer third-party tools to its jobs without a credential entering a job container. Two automated routes ship, and a skill routes a seller's tool to the right one.
[[sandbox.mcp_tools]])mcp-http-bridge, baked into the sandbox image.claude-agent-acpturn.[[sandbox.held_tools]])tool-holderdfromcrates/maxplayer-tool-kitplus the vendor CLI). The holder enrols once and resumes its login across restarts. Each job gets its own Unix socket per tool, mounted as a volume subpath; the agent reaches it throughtool-mcp-bridge --socket …. Job end detaches the socket; the tool stays enrolled.claude-agent-acpturn that called both. The vendors in that proof are the kit's fakes.The routing decision tree is
docs/specs/seller-tool-onboarding/10-routing-and-options.md. The skill is.claude/skills/seller-tool-onboarding/SKILL.md.What changed in the product
crates/maxplayer-core/src/held_tool.rs(new): the holder container guard, per-job attach and detach, pure argv builders.seller_exec.rs:[[sandbox.mcp_tools]]registration on the proxy;ExtraMountandJobAttachments;run_agent_job_in_env; the docker alias pinhole opens when an MCP server entry names it; the capture redactor knows every real credential value a launch holds.seller_node/run.rs: holders start at boot and stop at shutdown; both job paths attach and detach; the container-delivery path carries the mounts and the session entries to the orchestrator.driver/acp.rs:McpServeris now the ACP wire shape the adapters read (a stdio entry with notypekey, or anhttpentry).docker/maxplayer-sandbox/Dockerfile:mcp-http-bridgeandtool-mcp-bridgeinstalled.crates/maxplayer-tool-kit: the holder kit (race-safe no-follow file consumption, staging, per-job sockets), the two bridges, the fake vendor and proxy test doubles, templates.[[sandbox.mcp_tools]]and[[sandbox.held_tools]]on[sandbox]. A seat without them is unchanged.How it is proven
Every claim has a bundle under
evidence/, each with a README that states what it proves and its limits. The token or synthetic secret of each run appears in no file.evidence/20260914T085619Z-github-proxy-swap/— Proxy swap against GitHub: the bridge in the sandbox image, uncontained and under egress containment, and a real agent turn. The token owner's login came back from GitHub; the token was absent from everything the container received; the placeholder got400at GitHub and502at the proxy; job end revoked it.evidence/20260914T103608Z-holder-route-two-tools/— Holder with two tools through the daemon code: two holders enrolled once each, one job driving both through their own sockets, a contained job, a restart that resumed both logins, and a real agent turn that called both tools.evidence/20260910T125544Z/— the kit's digest-pinned container demo, 39 checks, which cleared the five review findings (F1 to F5) recorded indocs/handoff/README.md.The live tests are
#[ignore]d and configured by environment:seller_exec::mcp_tool_tests::live_*andheld_tool::live_tests::live_*. Their READMEs say how to rerun them.Gates on the merged tree (this branch merged with
mainat1569010)cargo test -p maxplayer-core(default,acp,wallet,wallet,acp)cargo test -p maxplayer(default,acp,wallet)cargo test -p maxplayer-tool-kitTwo tests failed once each in the full run and are not related to this change:
seller_node::run::tests::the_receipt_sub_backfills_the_terminal_cache_on_bootfailed withrelay run: AddrInUse— a local relay fixture that predates this branch (gate open-pool claims on a relay-derived settlement receipt (#541) #548) and exists onmain. It passed three times in a row when run alone.doctor::tests::sandbox_image_check_is_wired_into_the_boot_gateneedsdocker manifest inspecton an unresolvable host to fail fast. It passed in the default row of the same run and failed in the featured row when Docker Desktop's registry client hung, which a direct probe confirmed. This branch's change to that file is one struct-literal field.Requirements and limits
Merge with
mainThe branch is merged with
mainat1569010(the resolver-file and relay-push work). The merge metfour conflict files, all of one kind: both sides added a field or a literal line at the same place,
this branch for tools and mounts (
mcp_servers,mcp_tools,held_tools),mainfor the resolverfile (
resolv_conf,dns_servers). Both were kept everywhere.main's DNS live test builds typedmounts now. After the merge, both routes ran live again under egress containment and passed.
Where to start reading
docs/handoff/CONTINUATION-2026-09-10.md, sections 10 and 11.docs/specs/seller-tool-onboarding/10-routing-and-options.md, then09-production-integration.md.crates/maxplayer-core/src/held_tool.rsand themcp_tool_*regions ofseller_exec.rs.🤖 Generated with Claude Code