Repository navigation
feat(agents): add first-class DeepSeek Harness (dsh) support - #22468
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe changes add DeepSeek Harness as a supported TUI agent. They add command and process recognition, terminal-title and prompt-readiness detection, session resume handling, and commit-message agent configuration. They also add DSH hook event routing and status normalization, scrub-safe environment aliases, and local and remote managed-hook installation. Desktop, mobile, README, and supported-agent documentation now include DSH entries and assets. Priority: ➖ Normal Merge Risk: 🔵 Low · up to DeepSeek Harness support looks ready to merge. One test that checks owner-only file permissions will fail when the suite runs on Windows, even though the product behavior is correct. Skip that test on Windows so cross-platform test runs stay green. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 26183221-55bd-4761-a0e8-968ab7bcb06c
⛔ Files ignored due to path filters (1)
src/shared/agent-icons/dsh.pngis excluded by!**/*.png
📒 Files selected for processing (83)
README.mdconfig/tsconfig.cli.jsondocs/readme/README.es.mddocs/readme/README.fr.mddocs/readme/README.ja.mddocs/readme/README.ko.mddocs/readme/README.pt.mddocs/readme/README.zh-CN.mddocs/reference/agent-status-store.mddocs/reference/antigravity-readiness-evidence.mddocs/reference/ssh-execution-boundary.mddocs/site/content/docs/agents/supported.mdxmobile/src/components/mobile-agent-icon-assets.tsmobile/src/tasks/mobile-tui-agents.tssrc/main/agent-hooks/managed-agent-hook-registry.tssrc/main/agent-hooks/managed-hook-command-contract.test.tssrc/main/agent-hooks/remote-hook-service-installers.test.tssrc/main/agent-hooks/remote-managed-hook-installer-coverage.test.tssrc/main/agent-hooks/remote-managed-hook-installers.tssrc/main/agent-hooks/server-opencode-binder.test.tssrc/main/agent-hooks/server-retired-pane-new-turn.test.tssrc/main/daemon/pty-subprocess/spawn-environment.tssrc/main/dsh/dsh-home-patch.test.tssrc/main/dsh/dsh-home-patch.tssrc/main/dsh/hook-service.test.tssrc/main/dsh/hook-service.tssrc/main/dsh/hook-settings.tssrc/main/pi/agent-status-extension-source.tssrc/main/pi/titlebar-extension-service.test.tssrc/main/pi/titlebar-extension-service.tssrc/main/providers/local-pty-spawn-environment.tssrc/main/runtime/__fixtures__/dsh-tui-ready-no-key.meta.jsonsrc/main/runtime/__fixtures__/dsh-tui-ready-no-key.txtsrc/main/runtime/dsh-readiness-transcript.test.tssrc/main/runtime/orca-runtime-controller-knows-pty-is-live.tssrc/main/runtime/orca-runtime-terminal-handle-incarnation.test.tssrc/main/runtime/orca-runtime-write-terminal-agent-prompt.tssrc/main/runtime/rpc/methods/orchestration-worker-release-incarnation-fallback.test.tssrc/main/runtime/rpc/methods/orchestration-worker-release-reap-fixed.func.test.tssrc/main/text-generation/commit-message-model-discovery-policy.tssrc/renderer/src/components/terminal-pane/pty-connection-pi-synchronized-input.test.tssrc/renderer/src/i18n/locales/en.jsonsrc/renderer/src/lib/agent-catalog.tsxsrc/renderer/src/lib/agent-favicon-assets.tssrc/renderer/src/lib/agent-status.tssrc/renderer/src/runtime/agent-resume-host-authority-capability.test.tssrc/renderer/src/runtime/agent-resume-host-authority-capability.tssrc/renderer/src/runtime/web-session-tabs-sync/mirrored-status-tab-retractions.test.tssrc/renderer/src/runtime/web-session-tabs-sync/mirrored-status-tab-retractions.tssrc/shared/agent-headless-command.tssrc/shared/agent-hook-listener/provider-dispatch.tssrc/shared/agent-hook-listener/provider-event-routing.tssrc/shared/agent-hook-listener/providers/dsh-events.test.tssrc/shared/agent-hook-listener/providers/dsh-events.tssrc/shared/agent-hook-listener/source-routing.tssrc/shared/agent-hook-relay.tssrc/shared/agent-hook-scrub-safe-env.test.tssrc/shared/agent-hook-scrub-safe-env.tssrc/shared/agent-hook-types.tssrc/shared/agent-kind.tssrc/shared/agent-node-entrypoint-identities.tssrc/shared/agent-node-package-entrypoints.tssrc/shared/agent-process-recognition.tssrc/shared/agent-session-resume.tssrc/shared/agent-title-core.tssrc/shared/agent-title-status.tssrc/shared/agent-type-label.tssrc/shared/commit-message-agent-spec.test.tssrc/shared/commit-message-agent-specs-secondary.tssrc/shared/draft-paste-ready-scanner.tssrc/shared/dsh-launch-command.test.tssrc/shared/dsh-launch-command.tssrc/shared/dsh-terminal-title.test.tssrc/shared/protocol-version.tssrc/shared/skills-cli-agent-keys.tssrc/shared/source-control-ai-action-recipes.test.tssrc/shared/telemetry-property-schemas.tssrc/shared/terminal-title-agent-type.tssrc/shared/tui-agent-config.tssrc/shared/tui-agent-display-names.tssrc/shared/tui-agent-selection.tssrc/shared/tui-agent.tstests/tools/omp-source-control-runtime-smoke.mjs
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.
| | `tabId` | `structuredAgentSessionTabId(sessionId)` | | ||
| | `worktreeId` | `summary.workspaceId` (a folder workspace id is a valid value) | | ||
| | `state` | `structuredAgentSessionAgentStatus(summary).state`: the lead's own status folded with its live `backgroundTasks`, so a settled lead whose subagent still runs reads `working` | | ||
| | `workingMode` | `'monitoring'` from the same fold when watch loops are the only live child work; omitted otherwise, which clears it on the row | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Describe all child work that produces monitoring.
The status tests expect workingMode: 'monitoring' when an idle lead has only a backgrounded shell or only a workflow. Neither case requires a watch loop. Replace “when watch loops are the only live child work” with wording that includes these cases.
| <a href="https://github.com/QwenLM/qwen-code"><kbd><img src="https://www.google.com/s2/favicons?domain=qwenlm.github.io&sz=64" alt="Qwen Code logo" width="16" valign="middle" /> Qwen Code</kbd></a> | ||
| <a href="https://support.atlassian.com/rovo/docs/install-and-run-rovo-dev-cli-on-your-device/"><kbd><img src="https://www.google.com/s2/favicons?domain=atlassian.com&sz=64" alt="Rovo Dev logo" width="16" valign="middle" /> Rovo Dev</kbd></a> | ||
| <a href="https://dev.meta.ai/docs/muse-code"><kbd><img src="https://www.google.com/s2/favicons?domain=dev.meta.ai&sz=64" alt="Muse logo" width="16" valign="middle" /> Muse</kbd></a> | ||
| <a href="https://deepseek-harness.github.io/deepseek-harness/"><kbd><img src="https://www.google.com/s2/favicons?domain=deepseek.com&sz=64" alt="DeepSeek Harness logo" width="16" valign="middle" /> DeepSeek Harness</kbd></a> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove the duplicate DeepSeek Harness badge.
This list already adds the same label and link at Line 182. Keeping both entries renders DeepSeek Harness twice.
| promptDelivery: 'argv', | ||
| buildArgs: ({ prompt }) => ['--profile', 'headless', prompt], |
There was a problem hiding this comment.
🔒 Security & Privacy | 🛡️ Detected with Advanced Tier | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
DeepSeek Harness dsh --profile headless read prompt from stdin
💡 Result:
Inspection citation: inspection_6eb391c43fc1e99786d5aceb2dcdfca3
<source_evidence>
Citations:
- 1: https://deepseekplugins.org/plugins/deepseek-ai/deepseek-harness~23packages~2Fbundle~2Fheadless
- 2: GitHub discussion 1368 in deepseek-ai/deepseek-harness (link omitted to avoid creating a cross-reference)
- 3: https://github.com/deepseek-ai/deepseek-harness/blob/master/apps/cli/reference/README.md
- 4: https://github.com/deepseek-ai/deepseek-harness/blob/master/packages/bundle/headless/README.md
🏁 Script executed:
#!/bin/bash
set -e
file=src/shared/commit-message-agent-specs-secondary.ts
git diff --unified=35 51c3434851bdf8cb4c563f0b38cff257b413ef16 07531ed6ec2751ea554b1886332c6c0d053126cd -- "$file"
printf '\n--- target file size and outline ---\n'
wc -l "$file"
ast-grep outline "$file"
printf '\n--- delivery references ---\n'
rg -n -C 5 'promptDelivery|buildArgs|spawn|stdin|dsh' src/shared/commit-message-agent-spec.ts src/shared/tui-agent.ts "$file"
printf '\n--- exact reviewed lines ---\n'
nl -ba "$file" | sed -n '125,165p'
printf '\n--- dsh config and caller references ---\n'
rg -n -C 3 'commit-message-agent-specs-secondary|promptDelivery|buildArgs' srcRepository: stablyai/orca
Length of output: 45611
🏁 Script executed:
#!/bin/bash
set -e
file=src/shared/commit-message-agent-specs-secondary.ts
printf '%s\n' '--- PR diff for target file ---'
git diff --unified=24 51c3434851bdf8cb4c563f0b38cff257b413ef16 07531ed6ec2751ea554b1886332c6c0d053126cd -- "$file"
printf '%s\n' '--- target code ---'
nl -ba "$file" | sed -n '105,165p'
printf '%s\n' '--- spec definitions and delivery hooks ---'
nl -ba src/shared/commit-message-agent-spec.ts | sed -n '1,240p'
printf '%s\n' '--- tui agent declarations ---'
nl -ba src/shared/tui-agent.ts | sed -n '1,240p'
printf '%s\n' '--- focused argv and spec references ---'
rg -n -C 3 "promptDelivery: 'argv'|promptDelivery.*argv|buildArgs|commit-message-agent-specs-secondary" src/shared src/main | head -n 240Repository: stablyai/orca
Length of output: 36005
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- consumers of delivery selector ---'
rg -n -C 4 "spec\\.promptDelivery|promptDelivery === 'stdin'|promptDelivery.*stdin|stdin.*promptDelivery" src
printf '%s\n' '--- agent specification dispatch ---'
rg -n -C 4 "getCommitMessageAgentSpec|COMMIT_MESSAGE_AGENT_SPECS|commitMessageAgentSpec|commit-message-agent-spec" src/main
printf '%s\n' '--- repo-wide exact promptDelivery selector references ---'
rg -n -F "promptDelivery === 'stdin'" srcRepository: stablyai/orca
Length of output: 14172
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- stdinPayload consumers ---'
rg -n -C 5 'stdinPayload' src
printf '%s\n' '--- child process spawn and stdin write sites ---'
rg -n -C 4 'stdio:|\\.stdin\\.end|stdin\\.write|spawn\\(' src/main | head -n 220Repository: stablyai/orca
Length of output: 42007
Information Disclosure
Reachability: Internal
Exploitability: Difficult
CWE: CWE-214
Pipe the headless task through stdin. The prompt includes the staged diff, so passing it in argv exposes repository content to local process inspection and can exceed OS command-line limits. The headless profile accepts a task from stdin when no positional task is provided.
Deliver the task via stdin
- promptDelivery: 'argv',
- buildArgs: ({ prompt }) => ['--profile', 'headless', prompt],
+ promptDelivery: 'stdin',
+ buildArgs: () => ['--profile', 'headless'],📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| promptDelivery: 'argv', | |
| buildArgs: ({ prompt }) => ['--profile', 'headless', prompt], | |
| promptDelivery: 'stdin', | |
| buildArgs: () => ['--profile', 'headless'], |
Source: Learnings
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Remote (SSH relay) DSH panes will not report status: the scrub-safe aliases are only produced on the local spawn paths. Details in the inline comment. Everything else is minor.
Reviewed changes
- Agent registration. Adds
dshacross the TuiAgent tables — catalog (dsh-tui), display name,agent-kind, telemetry, mobile icon/favicon, auto-pick order, headless command (--profile headless), commit-message spec, andagent-session.dsh-resume.v1. - Detection & launch. New
dsh-launch-command.tsreads only the launcher's own tokens to separate the interactivedsh-tuiprofile fromweb/headless/sdk/acp/plugin;agent-node-package-entrypoints.tsextracts the node-shim data out ofagent-process-recognition.tsand adds thedsh-tuipackage path. - Managed hooks.
dsh-home-patch.tsmanages a marker-delimitedinsertrow in$DSH_HOME/cordis.patch.yml, preserving everything outside the markers and failing closed on a truncated region;hook-service.tswrites the hook script plus a Claude-shaped managed hooks file and implementsinstallRemoteover SFTP. - Status bridge.
dsh-events.tsmaps DSH's Claude-shaped events to states (SessionStart/Stop→ done,UserPromptSubmit/PostToolUse→ working,PreToolUseonask_user_question→ waiting; subagent/Notification/PermissionRequestignored). - Env-scrub workaround.
agent-hook-scrub-safe-env.tsmirrorsORCA_PANE_KEY/ORCA_AGENT_LAUNCH_TOKENontoORCA_AGENT_PANE/ORCA_AGENT_LAUNCH; the hook script restores the canonical names. - Title & readiness. DSH's whale outranks the Gemini classifier and status detector (DSH's resting
✦is Gemini's working glyph); adsh-composer-promptsignal watches for the composer inside the alternate screen. - Docs. README plus localized readmes, the supported-agents table, and reference-doc touch-ups.
ℹ️ Nitpicks
- The PR body says a reference doc is included at
docs/reference/dsh-harness-integration.md, but that file is not in the diff and nothing in the tree references it. Either add it or drop the claim. - About a dozen files carry reformatting-only churn unrelated to DSH — e.g.
src/main/runtime/rpc/methods/orchestration-worker-release-incarnation-fallback.test.ts(relocatedoxlint-disablecomments),src/main/text-generation/commit-message-model-discovery-policy.ts,src/main/pi/titlebar-extension-service.ts,src/renderer/src/runtime/web-session-tabs-sync/mirrored-status-tab-retractions.ts,tests/tools/omp-source-control-runtime-smoke.mjs. It widens the reviewed surface and invites merge conflicts; worth splitting out. isDshNonInteractiveCommand's first loop returnsfalseat the first non-flag token after index 1, so an interpreter invocation whose script path is not at index 1 (e.g.node --no-warnings …/@deepseek-ai/dsh/lib/bin.js --profile acp) is read as interactive. Only the plainnode <script> --profile …shape is covered by the tests.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
35f497f to
805bada
Compare
|
Thanks — worked through all seven. Five were real; two I'm pushing back on with evidence. Fixed
Not changing
|
There was a problem hiding this comment.
ℹ️ No critical issues — the fix commit closes the remote-status gap cleanly. Two minor suggestions inline.
Reviewed changes
- Marker-region pairing hardened.
findManagedDshPatchRegionnow pairs each end marker with the nearest preceding start, so an orphan start above user rows can no longer swallow them when a later block is installed — with a regression test that fails on the old logic. - Remote status gap closed.
buildSpawnEnvin the SSH relay now applies the scrub-safe pane-identity aliases, so a remote DSH hook can recoverORCA_PANE_KEYafter DSH's credential scrub. - Source Control AI prompt moved off argv. DSH's headless one-shot now delivers the diff over stdin (
--profile headless -) instead of placing it in the process table. - TUI launcher recognized as interactive.
dsh-tui/dstshort-circuitisDshNonInteractiveCommand, so a workspace target namedweb/pluginno longer marks a live pane as a headless one-shot. - Test isolation. The DSH hook-service test also isolates
USERPROFILE; a duplicated README badge was dropped.
ℹ️ Nitpicks
commit-message-agent-spec.test.tsassertsdshis in the spec list but nothing pins the newpromptDelivery: 'stdin'/buildArgs() === ['--profile', 'headless', '-']contract this commit changed; a one-line assertion would lock it in.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| @@ -0,0 +1,5 @@ | |||
| { | |||
There was a problem hiding this comment.
This file is an npm install prefix generated by .github/workflows/pi-owner-runtime.yml (npm install --prefix .cache/pi-owner …), not source. It looks like it was swept into the commit by a bulk git add; nothing in the repo references it as a tracked input. Please delete it and add /.cache/ to .gitignore so a future git add -A cannot re-commit the install tree.
Technical details
# Accidental generated npm prefix committed
## Affected sites
- `.cache/pi-owner/package.json:1` — new file added by `805bada0fd`; declares only `@earendil-works/pi-coding-agent: ^0.83.0`.
- `.github/workflows/pi-owner-runtime.yml:28` — `npm install --prefix .cache/pi-owner --ignore-scripts … @earendil-works/pi-coding-agent@0.83.0` is what produces it. The pinned version there, not the committed `^0.83.0`, is the source of truth.
- `.gitignore` — has no repo-relative `.cache/` rule (existing `.cache` entries are all `$HOME/.cache`-style).
## Required outcome
- The generated prefix is not part of the product and must not be tracked; the runtime workflow should continue to create it on demand.
805bada to
6316e63
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 599de5c8-41c2-43e5-87b4-1999199a348f
⛔ Files ignored due to path filters (1)
src/shared/agent-icons/dsh.pngis excluded by!**/*.png
📒 Files selected for processing (7)
src/main/dsh/dsh-home-patch.test.tssrc/main/dsh/dsh-home-patch.tssrc/main/dsh/hook-service.test.tssrc/relay/pty-handler.tssrc/shared/commit-message-agent-specs-secondary.tssrc/shared/dsh-launch-command.test.tssrc/shared/dsh-launch-command.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.
There was a problem hiding this comment.
ℹ️ No new issues in this delta — the shared managed-hooks reader extraction and module cleanups are behavior-preserving.
Reviewed changes
- Extracted a shared managed-hooks event reader — new
managed-hooks-json-events.ts; DSH's and Muse's near-identical readers now delegate to it and their duplicated private helpers are gone. - Consolidated the DSH hook-service file I/O — one
readTextOrAbsentreplaces theexistsSync+read pair (dropping a TOCTOU) and onestatus()builder replaces four inline status literals. - Simplified the DSH patch transforms —
isEmptyFlowDocument/withoutTrailingBlanksreplace the index-juggling empty-flow scan and the manual trailing-blankpoploops;rmSync(..., { force: true })replaces exists-then-unlink. - Tightened launcher-token detection —
flagNamenormalizes--profile=web, and the two bare-word subcommands share one set.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 943367ce-2815-44ee-8644-2d84fc8c81a3
📒 Files selected for processing (7)
config/tsconfig.cli.jsonsrc/main/agent-hooks/managed-hooks-json-events.tssrc/main/dsh/dsh-home-patch.tssrc/main/dsh/hook-service.tssrc/main/dsh/hook-settings.tssrc/main/muse/hook-settings.tssrc/shared/dsh-launch-command.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.
There was a problem hiding this comment.
ℹ️ No critical issues — the flow-style refusal, mode preservation, and relay identity drop are correct and covered. One minor suggestion inline.
Reviewed changes
This run re-reviewed commit b56b147d10 — the delta since pullfrog review 5296419149 (at 7c50d79925).
- Refuses an unappendable flow-style patch.
applyManagedDshPatchnow returnsnullfor a non-empty[…]body, andinstall()/installRemote()report an actionable error instead of appending a block entry DSH could not parse. - Handles
[] # comment.isEmptyFlowDocumentstrips a trailing comment, so DSH's commented empty sequence is recognized and only the[]token is dropped. - Preserves patch-file permissions. The rewrite goes through
writeHooksJson(..., { preserveMode: true }), so an owner-onlycordis.patch.ymlno longer widens to the umask default. - Stops the relay inheriting a pane.
buildSpawnEnvdrops inheritedORCA_PANE_KEY/ORCA_AGENT_LAUNCH_TOKENbefore mirroring the scrub-safe aliases, so a relay launched from an Orca pane cannot attribute hooks to the wrong pane. - Tests. New
dsh-home-patch.test.tsandhook-service.test.tscases cover the flow refusal, the[] # commentshape, and mode preservation; the 65 targeted tests pass locally.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
b56b147 to
f5148e0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: 2b8908ef-e0bc-4b18-b9d6-a9351ff2d8b0
⛔ Files ignored due to path filters (1)
src/shared/agent-icons/dsh.pngis excluded by!**/*.png
📒 Files selected for processing (5)
src/main/dsh/dsh-home-patch.test.tssrc/main/dsh/dsh-home-patch.tssrc/main/dsh/hook-service.test.tssrc/main/dsh/hook-service.tssrc/relay/pty-handler.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 3 remain after this review.
| it('keeps an owner-only patch file owner-only', () => { | ||
| // CWE-732: the temp+rename replacement must not widen the file to the umask default. | ||
| const userRows = '- id: llm-deepseek\n config: {}\n' | ||
| mkdirSync(join(home, '.dsh'), { recursive: true }) | ||
| writeFileSync(configPath(), userRows, 'utf-8') | ||
| chmodSync(configPath(), 0o600) | ||
|
|
||
| expect(new DshHookService().install().state).toBe('installed') | ||
| expect(statSync(configPath()).mode & 0o777).toBe(0o600) | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Skip the owner-only mode test on Windows.
This file explicitly supports Windows runs (Line 59 selects dsh-hook.cmd). On Windows, chmodSync(path, 0o600) changes only the read-only attribute. statSync().mode & 0o777 then reports 0o666 for a writable file. The assertion on Line 168 fails on every Windows run, even when preserveMode works correctly. POSIX permission bits are what this test checks, so run it only on POSIX.
🧪 Proposed fix
- it('keeps an owner-only patch file owner-only', () => {
+ it.skipIf(process.platform === 'win32')('keeps an owner-only patch file owner-only', () => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('keeps an owner-only patch file owner-only', () => { | |
| // CWE-732: the temp+rename replacement must not widen the file to the umask default. | |
| const userRows = '- id: llm-deepseek\n config: {}\n' | |
| mkdirSync(join(home, '.dsh'), { recursive: true }) | |
| writeFileSync(configPath(), userRows, 'utf-8') | |
| chmodSync(configPath(), 0o600) | |
| expect(new DshHookService().install().state).toBe('installed') | |
| expect(statSync(configPath()).mode & 0o777).toBe(0o600) | |
| }) | |
| it.skipIf(process.platform === 'win32')('keeps an owner-only patch file owner-only', () => { | |
| // CWE-732: the temp+rename replacement must not widen the file to the umask default. | |
| const userRows = '- id: llm-deepseek\n config: {}\n' | |
| mkdirSync(join(home, '.dsh'), { recursive: true }) | |
| writeFileSync(configPath(), userRows, 'utf-8') | |
| chmodSync(configPath(), 0o600) | |
| expect(new DshHookService().install().state).toBe('installed') | |
| expect(statSync(configPath()).mode & 0o777).toBe(0o600) | |
| }) |
Register DSH as a supervised Orca agent: catalog entry and detection for its dsh-tui profile, status/question hooks through DeepSeek's own Claude-Code hook bridge, composer-ready prompt delivery, session resume, headless Source Control AI, and title identity that no longer collides with Gemini's.
…its title as Gemini DSH runs command hooks through its own shell executor, which drops every env var whose name contains KEY, TOKEN, SECRET or PASSWORD — taking ORCA_PANE_KEY and ORCA_AGENT_LAUNCH_TOKEN with it, so every hook exited without posting. Mirror both onto scrub-safe aliases at spawn and restore them at the top of the DSH hook script. Its title collided too: DSH rests on the same glyph Gemini works on, so a resting DSH pane was relabelled Gemini CLI and reported working forever. Defer both the Gemini classifier and the title status detector on DSH's whale, in the base module both copies of that classifier read.
…, keep the diff off argv - findManagedDshPatchRegion paired an orphan start marker with a later block's end, so a truncated write made install/remove delete the user's own rows. Pair each end with the nearest preceding start; regression test fails without the fix. - The relay PTY env builder never applied the scrub-safe aliases, so remote DSH status silently never appeared even with the remote hook installed. - Source Control AI sent the whole diff on argv; send it over stdin with DSH's '-' marker. - dsh-tui/dst already chose the interactive profile, so a workspace folder named 'web' or 'plugin' no longer marks a live agent pane non-interactive. - Isolate USERPROFILE as well as HOME so a Windows run cannot edit the real home. - Drop the duplicate README badge and revert an incidental doc reformat.
…ules Reuse before reimplementing: readManagedDshHookEvents was a near-verbatim copy of Muse's, with byte-identical private helpers. Both now call one readManagedHookEventsFromJson. Also: one readTextOrAbsent instead of two spellings of the same read (dropping an existsSync TOCTOU), one status() builder instead of four inline literals, rmSync(force) instead of exists-then-unlink, and a redundant empty-string guard before JSON.parse. The patch-file transforms lose their index juggling for a predicate plus a filter.
… relay inheriting a pane - applyManagedDshPatch matched only an exact `[]`, so `[] # keep empty` or a non-empty flow sequence got a block entry appended after it — invalid YAML that would leave DSH unable to load the user's own patch layer either. It now strips the token from an empty sequence (keeping a trailing comment) and returns null for a non-empty one; install reports that and changes nothing. - The patch rewrite dropped an owner-only file to the umask default (CWE-732); pass preserveMode. - The relay PTY env never dropped inherited pane identity the way the local and daemon builders do, so a spawn that specified none could inherit the relay's own and every agent's hook would report against that pane.
… the mode test to POSIX A refused patch file carries no managed region, so getStatus() fell through to a bare not_installed with detail null — the actionable 'rewrite it as a block sequence' message only ever reached the one-shot install() return. Export the predicate and check it first, behind one shared message constant. The owner-only mode assertion cannot hold on Windows, where chmod only toggles the read-only attribute and mode & 0o777 reads 0o666 for any writable file.
Measured, not derived: 4220 without the DSH icon entry, 4221 with it. Two of the three modules above main's 4218 pin are not this change's — they arrived with the mobile work after #22570 and were never repinned; the changelog records that split explicitly.
f5148e0 to
a916199
Compare
|
Second review round — both new comments were right, and both are fixed. Flow-style refusal now survives a status read ( Owner-only mode test scoped to POSIX ( Also rebased onto current
Verification: |
There was a problem hiding this comment.
ℹ️ No new issues in this delta. The flow-style refusal now persists into status, which closes the prior review thread; this is not an approval because two earlier threads (relay alias test coverage,
/.cache/gitignore) are still open and untouched by these commits.
Reviewed changes
This run re-reviewed the delta since pullfrog review 5297947652 (at b56b147d) — commits 28652cd, 279be4c, a916199.
- Flow-style refusal now reported on every status read.
isNonEmptyFlowDocumentbecame the exportedisDshPatchFileUnappendable(text), andbuildStatuschecks it before the pointer comparison, so a refused[…]patch file keeps returning the actionable error fromgetStatus()instead of decaying to a barenot_installed. - One refusal message for install, remote install, and status. The text moved into a shared
FLOW_STYLE_DETAILconstant, so the one-shot and polled reads cannot drift. - Mode-preservation test scoped to POSIX.
it.skipIf(process.platform === 'win32'), sincechmodonly toggles the read-only bit on Windows and the mode assertion cannot mean what it says there. - Flow-refusal test now re-reads status. The
hook-servicecase asserts a latergetStatus()still reports the refusal, which would fail against the pre-fix pointer path. - Mobile session-route closure repinned to 4221, with the drift from 4218 documented inline as pre-existing inputs rather than folded into the DSH delta.
Technical details
# DeepSeek Harness increment review
## Delta verified
- `src/main/dsh/dsh-home-patch.ts:133` — predicate exported; same flow-sequence-only semantics as before.
- `src/main/dsh/hook-service.ts:140` — flow check sits after the managed-file read and before
`readManagedDshHooksConfigPath`, so the refused file's detail survives.
- `src/main/dsh/hook-service.test.ts:186` — re-read assertion fails without the fix.
- Tests run locally: `dsh-home-patch` (17) + `dsh hook-service` (12) → 29 green.
## Carried-forward risks (not new; tracked in the PR summary snapshot)
- Relay alias mirroring still has no test in the relay spawn-env suite.
- `/.cache/` remains absent from `.gitignore`.
- PR body references `docs/reference/dsh-harness-integration.md`, which is not in the tree.
- Windows/Linux and a live SSH remote were not exercised.DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
ℹ️ No new issues in this delta — the DSH
tui-idlelane is correct and covered, and theorigin/mainmerges preserve every DSH registration. This is not an approval because two earlier threads (relay alias test coverage,/.cache/gitignore) are still open and untouched by these commits.
Reviewed changes
This run re-reviewed the delta since pullfrog review 5298516087 (at a916199) — the authored tui-idle commit 36ad7b7 plus the two origin/main merges (ead7e14, d49efbf).
- DSH settles
tui-idleon its own hook.hasFreshDoneFirstPartyStatusadds a first-party lane toisTuiIdleSatisfied: a freshdonerow for adshpane satisfies the wait, since DSH's title deliberately carries no idle status (its✦rest glyph is Gemini's working glyph). Scoped todsh, because agents whose hooks report child turns can emit a mid-turndone. - Merge integration.
origin/main's ZCode agent and extracted entrypoint helpers were reconciled with this branch's splits; DSH remains registered in the managed and remote hook registries, the shared coverage ratchet, and the node entrypoint tables, with no duplicate entries.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
Adding the dsh-tui capture to __fixtures__ enrolled it in the serialize replay sweep, where it reports 10 I2 divergences and failed the unlisted-transcript default of 0. Every one is the same shape — visible-grid row=0, a 24-bit background the round trip does not restore to default — which is DSH's whale intro painting whole rows of true colour. Verified as an upstream limitation rather than a regression by replaying against the previous build (build-serialize-addon-at-ref.mjs --ref origin/main): I1 and I3 both hold.
|
Thanks — this is a careful report, and the caution at the end is the most useful part of it. Re-confirmed against current
So the misattribution is in the label path, not the agent-type resolver — the resolver still returns That is the same bug class this PR hit with DSH's own working frames ( Your caution is right, and it is the same reasoning behind the whale. Adding OSC 9999 dropping unregistered On whether Orca would take a One practical note: |
Only conflict was the serialize known-I2 table, where main added the Codex 0.157 fixtures beside this branch's DSH entry; both sets kept.
Same single conflict as before: the serialize known-I2 table, where main added the Claude trust-dialog fixture beside this branch's DSH entry. Both kept.
Main refactored isTuiIdleSatisfied into evaluateTuiIdle, which returns a verdict rather than a boolean. The DSH lane still returned `true`; it is tier-1 positive evidence, so it returns READY_STRONG like the title/body lane above it. Re-verified the regression test still fails without the lane.
There was a problem hiding this comment.
ℹ️ No new issues in this delta — the
origin/mainmerge and the one-line DSH adaptation are correct and the affected suites pass. Not an approval: the two earlier threads (relay alias test coverage,/.cache/gitignore) remain open and untouched by these commits.
Reviewed changes
This run re-reviewed the delta since pullfrog review 5324067086 (at d49efbf892) — the origin/main merges up to base 78771646af, plus the authored commit 3d45c3b09c.
- DSH
tui-idlelane adapted to the new verdict API.origin/main(#22927) replaced the booleanisTuiIdleSatisfiedwithevaluateTuiIdlereturning aTuiIdleVerdict; the DSH first-party-donelane now returnsREADY_STRONGinstead of a baretrue, keeping its position after the title lane and before the working veto.doneand a freshworkingare mutually exclusive on one status row, so the ordering is sound. - New DSH transcript pinned in the serializer replay.
dsh-tui-ready-no-keyjoinsKNOWN_PREEXISTING_I2_FAILURESwith a count of 10, documented as true-colour background divergences verified against the previous build (build-serialize-addon-at-ref.mjs --ref origin/main). Test-only, and the fixture is new in this PR. - Merge reconciliation. Main's Claude trust-dialog / terminal-wait work integrates without disturbing the DSH registrations; the two touched test files keep their DSH entries.
Verified locally on this delta: tui-idle-evidence.test.ts + dsh-readiness-transcript.test.ts (26 tests) and serialize-grid-transcript-replay.test.ts (28 tests, including the new DSH fixture) pass.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
…awn path The relay builds a remote pane's env itself, so the alias mirroring there had no test: removing the call left every suite green while remote DSH status silently vanished. Both cases fail without it.
The reference doc had no inbound link from anywhere in the repo.
There was a problem hiding this comment.
ℹ️ No critical issues — the relay alias regression tests close the prior gap, and one new doc pointer dangles.
Reviewed changes
This run re-reviewed the two commits since pullfrog review 5334146983 (at 3d45c3b09c).
- Pinned the relay's scrub-safe alias mirroring with regression tests.
pty-handler-spawn-environment.test.tsnow spawns with explicitORCA_PANE_KEY/ORCA_AGENT_LAUNCH_TOKENand asserts both aliases land oncallArgs.env, plus a second case that seeds the relay's own environment and asserts no alias survives a spawn claiming no identity. Both fail when theapplyScrubSafeAgentEnvAliases(result)call is stubbed. - Pointed
DshHookServiceat the integration reference. A JSDoc comment now links todocs/reference/dsh-harness-integration.mdfor the profile/patch-layer and env-scrub model.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| : status(configPath, 'partial', `Managed hook missing for events: ${missing.join(', ')}`, true) | ||
| } | ||
|
|
||
| /** Installs Orca's status hooks into DSH. See `docs/reference/dsh-harness-integration.md` |
There was a problem hiding this comment.
docs/reference/dsh-harness-integration.md does not exist anywhere in the tree or in this PR's diff, so this pointer dangles and the commit's premise — that the reference doc simply needed an inbound link — cannot hold. Either commit the reference doc or drop this pointer and the matching claim in the PR body.
Linux: first-class DSH integration not detected, pane misclassified as Gemini CLIEnvironment
SummaryThe DSH agent entry never appears in the catalog on Linux, and a manually launched Repro
Actual
Expected
Evidence that PATH is not the cause
and Orca was launched from a shell where Suspected causes
WorkaroundSettings → Agents → Custom agents → Add
This yields a correctly-labelled, interactive DSH pane. Hook-based status, question notifications, and resume are still unavailable, since those depend on the built-in bridge mount. |
|
Environment: Orca How it presents here — every dsh worktree reports no agent at all, while a $ orca worktree ps --json # condensed
# /home/…/Projects/agentic-engineering status=working agents=[{"agentType":"pi","state":"working"}]
# /home/…/workspaces/playdate-app/Inbox status=active agents=[]
# … six more dsh panes the same: agents=[]Corroborating the mislabel in the "Before" section: one of these panes reports One finding relevant to the "needs your answer" half of this PR: dsh-tui already attempts a waiting-for-you signal — Orca just can't see it. // OSC 9 desktop notification — same mechanism Claude Code's own CLI uses
this.tui.terminal.write(`\x1b]9;${message}\x07`);Orca registers OSC handlers for That may be worth an explicit decision in this PR: teach the terminal OSC 9 → attention/notification (it would also cover anything else using the Claude Code convention), or have the dsh hook publish the waiting state itself. It can't be derived from turn boundaries, because a dsh turn stays open while it waits on a question — What I'm running meanwhile (not a request — just data, and it's the only reason my panes show anything at all): Orca's bell path is agent-agnostic, so a BEL on the pane's pty calls |
…nother tabbed worktree A tmux server started from an Orca pane freezes that pane's environment, including the scrub-safe pane alias ORCA_AGENT_PANE (stablyai#22468). Sessions a user later creates detached in that server inherit the alias, and since an agent of a different worktree posts its hooks under the creating pane's key. The claimed pane key and worktree id are inherited claims, while the conversation folder the agent reports in its hook payload is not, so a record written from such a hook made the next Orca restart type `cd <other worktree> && claude --resume <other session>` into the wrong tab, and the tab's own conversation was not restored (stablyai#37799). Guard the resume-record boundary on the execution host: a hook status or capture whose provider session cwd lies inside another worktree that has its own tabs in the same window session never writes, replaces, clears, or restores that pane's record. A cwd outside every tabbed worktree keeps today's behavior (stablyai#16960), and the pane's own record stays kept so a later hook from the pane's own conversation still refreshes it. - cwdInsideOtherTabbedWorktree: shared predicate over the session's tabbed worktree paths, boundary-aware and spelling-normalized; when the pane's own worktree nests below another tabbed one, the longest matching worktree path owns the folder so a cwd inside the pane's tree is not misread as foreign (coderabbit Major PRRT_kwDORpCz1s6zN5uJ) - hookStatusResumeCommand and quitResumeRecords: skip foreign statuses - applyWorktreeSleepResumeRecords: skip foreign sleep captures - resolvePaneOwnedResumeStartup: veto restore of a stored record or a hook-store read naming another tabbed worktree; a stored record stays kept and the pane reopens as a plain shell Fixes stablyai#37799 Generated-by: GLM-5.3-Flash (ZCode)
ELI5
DeepSeek's coding agent, DeepSeek Harness, can now be picked from Orca's agent menu like Claude or Codex. Orca launches it, knows it by name, shows whether it is thinking, finished, or waiting on you, and can pick the conversation back up later.
What Changed
Before: DSH only ran if you typed
dsh-tuiinto a blank Orca terminal. It was absent from the agent menu, and once running Orca mislabelled the pane "Gemini CLI" and showed it as permanently working — no status, no notification, no "needs your answer", no resume.After:
DeepSeek Harnessis in the agent menu, launches into a pane, and reports real status.The pieces:
dsh, that boots a profile; only thedsh-tuiprofile paints a composer, whileweb/headless/sdk/acp/pluginare alldshin the process table. Detection reads the profile out of the launcher's own tokens so a server or a one-shot is never mistaken for an agent pane.@deepseek-ai/dsh-hooks-claude-codebridge rather than a bespoke protocol. Orca mounts it from a marker-delimited block it owns in$DSH_HOME/cordis.patch.yml— one layer above every profile, so a pane the user started themselves still reports, and Orca never edits a profile the user owns.KEY,TOKEN,SECRETorPASSWORDbefore a hook starts — which tookORCA_PANE_KEYandORCA_AGENT_LAUNCH_TOKENwith it, so every hook exited without posting. Orca now mirrors both onto scrub-safe aliases at spawn and the DSH hook script restores the canonical names first.<prefix> 🐋 <session>, and its idle prefix✦is Gemini CLI's working glyph. That is why the pane read as a working Gemini. The whale now outranks the Gemini classifier and the title status detector, in the base module both copies of that classifier read.dsh-composer-promptreadiness signal watches for the❯glyph inside the alternate screen instead.dsh-tui --resume <id>, using thesession_idthe hook reports.dsh --profile headlessis registered for Source Control AI text generation. The prompt goes over stdin with DSH's-marker, not argv: it carries the whole diff, which on argv would sit in the process table and would eventually hit the argv limit. Measured against 0.1.5-rc.1 — omitting the positional entirely is rejected (a task is required) even when stdin is a pipe, so the-is required.Also included: icon, labels, telemetry, mobile registry, SSH/remote installer, and a reference doc at
docs/reference/dsh-harness-integration.md.Why
The issue asks for first-class detect/launch, and the thread asks for status/notifications/resume rather than waiting on a second official CLI. DSH already publishes a first-party Claude-Code hook bridge, so the honest integration is to mount that instead of inventing a protocol or shipping a wrapper — which is also why this keeps working as DSH's own surface evolves.
Mounting at the home layer (rather than passing
--patchat launch) was chosen so status does not depend on Orca having been the one to start the process, and so install/remove/status match how every other agent's managed hooks behave.Two deliberate limits, both documented:
working, notwaiting. DSH's bridge has noNotificationorPermissionRequestevent. The ask-the-user tool (ask_user_question) is a hook, so real questions surface correctly; an approval prompt has no signal, andworkingis the honest answer since the harness is mid-tool.Linked Issue
Relates to #14323
This adds the official
dshharness, launched through thedsh-tuiprofile that selects it. It does not close the broader tracking request.Visual Proof
All captured from a hidden Electron renderer over CDP against the real
@deepseek-harness-tui/dsh-tui0.10.2 on official@deepseek-ai/dsh0.1.5-rc.1. Model traffic is served by a local scripted endpoint so the demo needs no DeepSeek account; every hook, status row and title below is real.1. DeepSeek Harness in the agent menu
2. Agent identity — before and after
The same pane, same app, before and after the title fix in this PR. Before, the tab and sidebar read Gemini CLI; after, they read DeepSeek Harness and carry DSH's own title.
3. Question status — the pane is waiting on you
DSH's
ask_user_questioncard is open; the sidebar row shows the attention state, and Orca's status store reportswaitingwithtoolName: ask_user_question.4. Answered and finished
The answer lands, DSH reports the questionnaire answered, and the row returns to done.
5. The live status rows behind those screens
Sampled from
window.api.agentStatus.getSnapshot()while the turn ran:origin: "hook"is the point: these come from DSH's bridge, not from guessing at the screen. Thesessionid is the resume target.6. Orchestration reader sees the pane
7. The managed hook block preserves user rows
$DSH_HOME/cordis.patch.ymlafter Orca's install — the user's own row is untouched:Testing
Platform tested: macOS arm64, Node 24, against
@deepseek-harness-tui/dsh-tui0.10.2 on@deepseek-ai/dsh0.1.5-rc.1.Manual, end to end in the app: detect → launch → SessionStart idle row → prompt → working →
ask_user_question→ waiting → answer → done, with the resume session id carried throughout. The hook bridge was additionally verified directly againstdsh --profile headless, and DSH's env scrub rule was measured with a probe hook that printed its own environment.Automated:
src/shared/agent-hook-listener/providers/dsh-events.test.ts— the event→state mapping, including the subagent events that must be ignored.src/main/dsh/hook-service.test.ts,src/main/dsh/dsh-home-patch.test.ts— install/remove/status, user-row preservation, the[]empty-document case, and the truncated-region fail-closed path.src/shared/dsh-launch-command.test.ts— profile-aware process recognition.src/shared/dsh-terminal-title.test.ts— the Gemini/Claude title collisions.src/shared/agent-hook-scrub-safe-env.test.ts— the alias contract.src/main/runtime/dsh-readiness-transcript.test.ts— readiness and identity read from a committed PTY transcript (dsh-tui-ready-no-key.txt), captured perdocs/reference/agent-pty-transcript-capture.mdand secret-scanned clean.pnpm tcclean;pnpm run check:code-quality:changedpasses; the affected suites run 14,695 tests green.Not tested on Windows or Linux. The Windows path follows the existing
.cmd+ PowerShell shape Muse uses, and DSH binds its shell to PowerShell on Windows, which is the same splitwrapPosixHookCommand/wrapWindowsHookCommandalready encode. The SSH installer mirrors the local one over SFTP and is covered by the existing remote-installer ratchet, but was not exercised against a live remote host.AI Disclosure
Implemented with Claude Code (Opus).
Review
Review round 1 (all seven comments addressed; see the PR comment for the full reply):
findManagedDshPatchRegionpaired an orphan start marker with a later block's end, so a truncated write made install/remove delete the user's own rows. Now paired with the nearest preceding start, with a regression test verified to fail without the fix.dsh-tui web— a workspace folder namedweborplugin— no longer marks a live agent pane non-interactive.USERPROFILEas well asHOME, so a Windows run cannot edit the real home.Follow-up pass, no behaviour change:
readManagedDshHookEventswas a near-verbatim copy of Muse's with byte-identical private helpers, so both now share onereadManagedHookEventsFromJson(src/main/agent-hooks/managed-hooks-json-events.ts). Plus one reader instead of two spellings of the same read, one status builder instead of four inline literals, and simpler patch-file transforms. Net −79 lines.Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
installRemotemirrors the local install over SFTP and is registered in the remote installer ratchet. Review caught that the scrub-safe aliases were applied only in the local and daemon PTY env builders — a remote pane's env is built on the relay side, so remote DSH status would have silently never appeared despite the remote hook installing fine.relay/pty-handler.tsnow applies them too, and the reference doc states all three builders must.agent-session.dsh-resume.v1runtime capability so an older host answers adshsession request safely instead of failing the launch. The canonical env var names are unchanged — the aliases are additive.String.includes.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)