feat(tools): remote_task replaces the cloudflare sandbox mode - #106
Closed
geminixiang wants to merge 2 commits into
Closed
geminixiang wants to merge 2 commits into
geminixiang wants to merge 2 commits into
Conversation
Git resolves its repository from the environment before it looks at the working directory. When the suite runs inside a git hook -- husky's pre-commit runs `npm test` -- git exports GIT_DIR to the hook, and every git a fixture spawns inherits it. With GIT_DIR set and no GIT_WORK_TREE, git treats the fixture's cwd as the work tree, so `git add -A` in a throwaway /tmp repo stages the deletion of every file tracked in *this* repository. That is not hypothetical. It is why `deploy/pm2/ecosystem.config.cjs` silently left the index during the gondolin work, and .gitignore's unanchored `ecosystem.config.cjs` then refused to let `git add -A` put it back, so the file looked deliberately deleted. It also made the hook unusable: the fixture's own commit re-enters this repo's pre-commit in a directory with no package.json and fails, which is why recent commits needed --no-verify. Eight test files spawn git, so the fix is one setup file that strips the ambient pointers rather than eight patches. Also anchors the two .gitignore patterns that made the damage stick: `/ecosystem.config.cjs` so the tracked deploy template at any depth is no longer matched, and `node_modules` without the trailing slash so a symlink -- which worktrees sometimes get -- is not committable. Un-ignoring the deploy template exposes it to knip, which has no reason to resolve a pm2 config, so deploy/** joins the knip ignore list. Verified two ways: pointing GIT_DIR at a decoy repo gives 3 failures and a wrecked index before, 23 passes and an untouched decoy after; and this commit itself passes the real pre-commit hook end to end -- lint, fmt:check, knip, build, 1390/1390 -- leaving a clean index.
Implements ADR 0002. Remote execution was never a sandbox runtime: it holds no workspace projection, so nothing it writes survives a turn and nothing from the conversation is visible to it. The type claimed otherwise -- ActorExecutionResolver resolved mounts and the Cloudflare branch discarded them, the bridge payload has no mount concept, and credentials.fileMounts was false -- so sessions and MEMORY.md were always on the mikan host and never on the remote side. So it becomes a tool. Each remote_task call gets a throwaway mikan-task-<uuid> sandbox, which is the point: parallel calls cannot see or disturb each other. A sticky per-conversation id would carry scratch state between calls, reintroducing the persistence this rejects, and would make one conversation's parallel calls collide. The tool is registered only when CLOUDFLARE_SANDBOX_URL is set, so a host without a bridge never advertises a capability it cannot fulfil. The task executor gets no vault credentials. The conversation vault belongs to the sandbox runtime; sending a user's credentials to a remote third party is not a default worth shipping, and nothing here needs it. --sandbox=cloudflare:* is rejected at startup with migration guidance, following the sandbox.gondolin.remote precedent. The consequence the ADR predicted lands too: allowsAmbientDefaultShared- Vault collapses to image alone, and its comment now states a trust rule instead of a topology one. The old "isolated sandboxes" framing is how agent-sandbox was added to that list in 19845f7, and it would have pulled in gondolin -- whose whole purpose is running untrusted code. Docs: a remote-task page replaces sandbox/cloudflare.md across all four locales, and the capability tables lose the column. Also corrects the gondolin lifecycle cell, still reading "worker-hosted" after #103.
Deploying mikan with
|
| Latest commit: |
d0a4fc8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://56afc41e.mikan-1wv.pages.dev |
| Branch Preview URL: | https://feat-remote-task-executor.mikan-1wv.pages.dev |
Owner
Author
|
Closing because this item has had no activity for more than 30 days. Please reopen it or create a fresh follow-up if the work is still relevant. |
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.
Implements ADR 0002, now moved from
proposedtoaccepted.The type was claiming something the runtime never did
Remote execution was never a sandbox runtime. It holds no workspace projection, and the code already agreed:
ActorExecutionResolverresolved mounts (execution-resolver.ts:65) and the Cloudflare branch discarded themcredentials.fileMountswasfalsegetPathContextreturned a host/runtime root pair with no translation between themSo sessions,
MEMORY.md, and skills were always on the mikan host and never on the remote side. No deployment loses data in this move — there was nothing out there to lose.What it becomes
A tool. Each
remote_taskcall gets a throwawaymikan-task-<uuid>sandbox, which is the point: parallel calls cannot see or disturb each other, so fan-out and dynamic workflows work.A sticky per-conversation id was the alternative and was rejected — it would carry scratch state between calls (exactly the persistence this decision rejects) and would make one conversation's parallel calls collide.
The tool is registered only when
CLOUDFLARE_SANDBOX_URLis set, so a host without a bridge never advertises a capability it cannot fulfil.One deliberate non-goal
The task executor receives no vault credentials. The conversation vault belongs to the sandbox runtime; shipping a default that sends a user's credentials to a remote third party is not something to do quietly. There is a regression test pinning it. Say the word if you want opt-in credential passing and I will design it separately.
The consequence ADR 0002 predicted
allowsAmbientDefaultSharedVaultcollapses toimagealone, and its comment now states a trust rule instead of a topology one. That framing matters: the old "isolated sandboxes" wording is exactly howagent-sandboxgot added to the list in19845f7by pattern-matching, and it would have pulled ingondolin— whose entire purpose is running code from people you do not trust. The test now assertsgondolinstays excluded, with the reason.Migration
--sandbox=cloudflare:*is rejected at startup with guidance, following thesandbox.gondolin.remoteprecedent. KeepCLOUDFLARE_SANDBOX_URLset to retain remote fan-out via the tool.Docs
A new
remote-taskpage replacessandbox/cloudflare.mdacross all four locales, and the mode tables lose the row, card, and column. Also fixes the gondolin lifecycle cell, which still read "worker-hosted" after #103.Verification
lint, fmt:check, knip, build, 1391/1391 — and this commit passed the real pre-commit hook with no
--no-verify, thanks to #105.Not exercised against a live bridge (no Worker deployed here); the tool's transport is covered by 7 unit tests including abort, non-zero exit, bridge error, and the no-credentials guard.