feat(dsh): add native subagent workflow on beta - #548
Conversation
- AI_TOOLS: register dsh as class-2 pull-based platform (agentCapable, no hooks, supportsAgentSkills, cmdRefPrefix /trellis-) - configurator: shared .agents/skills/ via neutral resolver (byte-identical to Codex/Gemini/Pi/Kimi) + dsh-private .dsh/skills/ entry points (trellis-start/continue/finish-work) and agent prompts (implement/check with pull-based prelude, research standalone) - workflow.md: add DeepSeek Harness to class-2 platform groups and dispatch protocol prose; sync marketplace native workflow mirror - cli/init: --dsh flag + InitOptions entry (type-check enforced) - tests: dsh template/configurator/integration coverage; update pull-based marker assertions and tracking platform list Note: committed with --no-verify because the pre-commit pnpm test gate fails on this Windows checkout for pre-existing environment reasons (CRLF via autocrlf, chmod semantics, python3 subprocess) — verified identical failures on a clean baseline worktree. typecheck, lint, and all touched tests pass; CI on Linux remains the authoritative gate.
- active_task.py: accept DSH_TRELLIS_CONTEXT_ID (exported into agent shells by the dsh-trellis harness plugin) as a context-key override, so task.py start/create/current resolve a per-session pointer on dsh; add dsh to _KNOWN_PLATFORMS so pointer files record platform dsh - task_store.py: add .dsh to _SUBAGENT_CONFIG_DIRS so task.py create seeds implement.jsonl/check.jsonl for dsh projects
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Nice turnaround — you shipped Your scrub claim is confirmed, empiricallyThe comment on
The gap: this only protects the plugin-present path
Concretely, with no plugin installed and dsh launched from an active Trellis session: Unsetting Suggestion: your original framing generalizes betterYour very first message proposed preferring So the fallback path can use the same evidence the plugin path uses:
That closes the plugin-absent case without adding a second vendor-specific override, and it keeps I have not made this change on Minor
The rest of the PR looks well-formed from the outside: base branch, stable docs untouched, mirrors byte-identical, companion PRs opened against the upstream repos rather than forks. I will do a proper pass over the sub-agent dispatch and |
|
Reviewed the two companion PRs and validated all three together locally. Everything mechanical checks out; one wording regression is worth a look before you take this out of draft. Combined verificationChecked out this PR with So the three PRs are self-consistent as a set. Independently verified rather than taken from your report, and it matches your numbers modulo the tests added in marketplace#13Byte-identical to the bundled template in this PR — diffed directly, 714 lines, no drift, so All five block markers moved from the inline group to the sub-agent group in matched open/close pairs, and the class-2 list picked up DSH in both the prose line and the One thing to reconsider. This line was generalized: -Tools: `trellis-implement` / `trellis-research` are sub-agent types only (Task/Agent tool, NOT Skill; there is no skill by these names).
+Tools: `trellis-implement` / `trellis-research` name sub-agent roles, not main-session skills.The motivation is right — on DSH those names do exist as skills, so "there is no skill by these names" became false and had to go. But the original was doing operational work beyond describing the model: it named the tool to use and explicitly negated the wrong one, which is what stops a Claude Code session from calling Something like this keeps the guardrail while staying true on DSH:
Your DSH-specific paragraph below it already covers the docs#31Accurate, and scoped exactly as agreed — Documenting the I have opened #549 for the general problem, so it is tracked independently and does not block this PR. It carries the reproduction, why the Merge orderNothing blocking on our side beyond sequencing, which matches what you laid out:
Worth doing in that order even though I confirmed both SHAs are directly fetchable today: they are only reachable through PR refs, so a force-push or a closed PR would leave |
|
Thanks for the combined verification and for catching the Claude Code wording regression. I took both suggestions. 1. Restored the operational sub-agent guardrailThe workflow now says that The dogfood, bundled, and marketplace copies are byte-identical again, and a template assertion pins that clause. Marketplace head: 2. Closed the DSH plugin-absent nesting gap hereI kept #549 as the generic cross-platform problem, but used DSH's stronger managed-shell evidence in this PR:
There are positive and negative regressions for the plugin-absent path: a managed DSH shell rejects the inherited outer context, while I also tested the actual no-plugin path on DSH {"DSH_SESSION_ID":"session-02c083c6-fa6c-4d3c-8dc0-da70ea3e34a8","DSH_SHELL":"1","DSH_TRELLIS_CONTEXT_ID":null,"TRELLIS_CONTEXT_ID":"claude_outer-session","resolved":"dsh_session-02c083c6-fa6c-4d3c-8dc0-da70ea3e34a8"}So the plugin-absent path now isolates the inner DSH session without asking users to unset the outer variable. The beta EN/ZH docs and plugin README have been updated accordingly. Docs head: Verification at the new combined headFresh public clone, non-root
One preceding container run hit the unrelated I have kept all three PRs in draft. I will follow your merge order: marketplace#13, docs#31, repoint both submodules to commits reachable from |
|
All three review points landed, and I re-verified the combined state rather than re-reading it. Approved from our side — no blockers. The pluginless fix works, measured in a real dsh
Same working tree, same inherited The guard is also narrow enough not to hurt anything else:
Guardrail wording
Combined stateThis PR with One nit, non-blockingThe new comment says the paired sentinel and session id "cannot be forged by or inherited from an outer Trellis host." The inheritance half is solid — I confirmed dsh wipes and rebuilds the namespace. But forging is possible from an ordinary shell: Practically this is nothing: someone has to deliberately export both, and Ready when you areNothing left on our side. Whenever you are done with your remaining cleanup, mark the three PRs ready for review and I will merge in your order: marketplace#13 → docs#31 → repoint the submodules here → this into I have left them in draft rather than flipping them myself, since you are the one who knows whether anything else is still coming. |
Moves DeepSeek Harness from the inline-execution group to the class-2 sub-agent group across all five block markers, and documents the DSH dispatch path: load one `trellis-agent-<role>` skill per child, then synchronize via `trellis_wait` when the optional companion plugin is present, or an initial foreground dispatch when it is not. Never poll or sleep. Also restores the operational Claude Code guardrail on the shared Tools line — the sub-agent roles are dispatched through the host's sub-agent mechanism (on Claude Code: the Task/Agent tool, never the Skill tool) — while staying accurate on DSH, where those role instructions do ship as skills. Byte-identical to the bundled template in mindfold-ai/Trellis#548, verified by diff. Co-authored-by: SajoLuo <SajoLuo@users.noreply.github.com>
|
Both companion PRs are merged — we were each waiting on the other, my earlier "mark them ready and I will merge" read as if it covered all three. Your ordering was the correct one, so I have taken the first two steps. Squashed, so the SHAs are new. I verified the merged trees are byte-identical to the commits I reviewed ( Your no-plugin verification is a stronger construction than mine, incidentally — disabling the companion through a profile patch and poisoning both Over to you for the last two steps:
Then I will merge it into |
|
Thanks — the final two steps are complete at
Finalization checks are green: 3 focused DSH identity regressions, 9 DSH template tests, the bundled/marketplace workflow mirror assertion, and normalized One non-blocking observation for the proper Everything else is ready on my side. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7742ddab0f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| dsh_override = _string_value(os.environ.get("DSH_TRELLIS_CONTEXT_ID")) | ||
| if dsh_override: | ||
| return _sanitize_key(dsh_override) or _hash_value(dsh_override) |
There was a problem hiding this comment.
Do not let inherited DSH identity override an inner host
When Codex, Claude, or another supported host is launched from a DSH session, it inherits the outer process's DSH_TRELLIS_CONTEXT_ID along with its own session identity. This unconditional early return therefore selects the outer DSH context before inspecting the inner host's payload or native environment, causing task.py start/current/finish in the nested host to read or mutate the outer DSH active-task pointer. Scope this override to an execution known to be inside the current DSH managed shell, or otherwise allow an inner host's identity to outrank inherited DSH variables.
Useful? React with 👍 / 👎.
|
Did the sub-agent / Dispatch path, verified against the real tool surfaceThe workflow instructions depend on dsh's actual tool schema, so I checked the schema rather than trusting the prose:
Then the plugin-absent path end to end, no companion installed: So the capability check is answerable by the agent at runtime (not a flag it has to be told), and foreground dispatch actually settles. That is the half I could not confirm from the diff. Shared-script changes
Pre-existing bug your change happens to sidestepVerifying the marker filtering turned up something unrelated to this PR.
I confirmed the same on One wording point, take it or leave it
"initially" can be read as "the first child is foreground, later ones may be background" — which is exactly the state the paragraph forbids two sentences later. If you meant "at dispatch time" rather than "for the first dispatch", dropping the word or saying "dispatch each child with VerdictMerging now. Thanks for the turnaround on all of this — three review rounds inside two hours, each one with its own verification run, is not the normal experience on a platform PR. |
Brings 18 commits from main, including 0.6.15, the DSH minimal adapter (#547), and the marker-label routing fix. Beta stays on 0.7.0-beta.3 and keeps its own submodule pointers. Git merged three files cleanly whose *content* was contradictory, so those are the ones worth re-reviewing: - workflow.md: main puts dsh in the inline-execution group (#547 has no sub-agent surface), beta puts it in the class-2 sub-agent group (#548 added one). Both edits applied, leaving dsh in both groups. Resolved to beta's semantics — dsh removed from the inline markers in the template, the dogfood copy and the marketplace mirror. - dsh was registered twice: once by #547 on main and once by #548 on beta, in cli/index.ts, commands/init.ts and configurators/index.ts. Different lines each time, so no conflict was raised; tsc caught it. Deduped, keeping beta's wording. - opencode.test.ts: beta's layered-workflow tests fed the plugin a bare {type,text} part, which #524 on main no longer accepts — insertSyntheticTextPart requires an ordinary part with a persisted identity and now emits the breadcrumb as its own part instead of rewriting the user's text. The tests asserted on parts[0] and silently saw the unmodified prompt. Rewritten against the synthetic-part contract; they still verify the layered resolution they were written for. Declared conflicts resolved to beta for everything dsh-related and for the version files; workflow_phase.py took main's _PLATFORM_MARKER_LABELS table, which subsumes beta's dsh-only alias. Verified on the merged tree rather than assumed: - core 344 passed, cli 1825 passed, lint and tsc clean - all 22 platforms resolve a non-empty routing block - dsh resolves the sub-agent route, not the inline one - built the CLI and ran `init --dsh`: 86 files, with the three trellis-agent-* role skills beta expects - marketplace mirror byte-identical to the bundled template
Context
Follow-up to #547, which shipped the minimal DeepSeek Harness adapter in 0.6.15. This PR targets
feat/v0.7-betaand implements the native sub-agent workflow agreed in the coordination thread, including the nested-host identity findings tracked generically by #549.Summary
trellis-agent-{research,implement,check}role skills with pull-based task contextTask/Agent-not-SkillinstructionDSH_TRELLIS_CONTEXT_IDsupplied bydsh-trellisremains first, so a forwarded child identity may differ from the shell's own sessionDSH_SHELL=1together withDSH_SESSION_IDproves a DSH-managed shell and resolves through the canonical DSH env table before an inherited genericTRELLIS_CONTEXT_IDDSH_SESSION_IDwithout the managed-shell sentinel does not displace an explicit generic overrideTRELLIS_CONTEXT_IDwith the same DSH context keytrellis_waitcapability for one-shot, event-driven settlement when presenttrellis init --dshdsh --profile headlessinvocations are separate sessions unless explicitly resumedCompanion changes
The current root tip is
7742ddab, withdocs-siteat59d5b37andmarketplaceatcfb2f38. Both companion commits are reachable from their upstreammainbranches and are tree-identical to the reviewed PR heads.Verification
Windows
0.1.0-rc.6probe with the plugin disabled and a poisoned outer environment:TRELLIS_CONTEXT_ID=claude_outer-sessionDSH_TRELLIS_CONTEXT_IDandDSH_SESSION_IDwere scrubbedDSH_SESSION_ID, no plugin override, and resolved the matchingdsh_session-*keynpm pack --dry-runpassThe repository-wide Windows regression file still includes existing Windows-specific failures around the
python3Store alias and CRLF byte comparisons; the changed tests pass there, and the authoritative clean-Linux run below covers the complete suite.Clean Linux container
A fresh public clone on
node:22-bookworm, run as the non-rootnodeuser at roote3ba39f1, docs11e3e645, and marketplace08a258f0:python3: passOne unrelated
template-fetcher-cleanuptest produced a transientENOENTon the preceding run; it passed both an isolated rerun and the complete clean-clone rerun reported above.Finalization
At final root
7742ddab:docs-siteto mergedmaincommit59d5b37andmarketplaceto mergedmaincommitcfb2f38; both tree diffs against the reviewed heads are emptyReview notes
TRELLIS_CONTEXT_IDprecedence is unchanged outside a verified DSH-managed shell; TRELLIS_CONTEXT_ID is inherited across nested platform sessions, so the inner session adopts the outer session's active task #549 remains the venue for a platform-neutral nested-host mechanism.main, so this PR is ready for final review.