-
Notifications
You must be signed in to change notification settings - Fork 697
feat(skill): harden runtime execution #1343
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| # Skill Runtime Hardening Plan | ||
|
|
||
| ## Data Model | ||
| - Add `SkillExtensionConfig` with `version`, `env`, `runtimePolicy`, and `scriptOverrides`. | ||
| - Add `SkillScriptDescriptor` generated from `scripts/**/*.{py,js,mjs,cjs,sh}` and merged with sidecar overrides. | ||
| - Store sidecars in `<skillsDir>/.deepchat-meta/<skillName>.json`. | ||
|
|
||
| ## Runtime Flow | ||
| - `SkillPresenter` owns sidecar read/write and script discovery. | ||
| - `SkillExecutionService` validates active skill access, resolves scripts, merges env, selects runtime, and executes scripts. | ||
| - `skill_run` becomes the preferred skill-local execution entrypoint. | ||
|
|
||
| ## Read Guardrails | ||
| - Add shared binary-read helpers for ACP and main agent reads. | ||
| - ACP rejects non-text files through `fs/read_text_file`. | ||
| - Main agent keeps image OCR fallback, but rejects unsupported binary reads with guidance. | ||
|
|
||
| ## Process Output Reliability | ||
| - Move foreground and background completion semantics from `exit` to `close`. | ||
| - Await output flush before returning completed process results. | ||
| - Offload large foreground output to session files when possible. | ||
|
|
||
| ## UI | ||
| - Extend the skill editor with runtime policy, env rows, and discovered scripts. | ||
| - Show script/env/runtime summary badges on skill cards. | ||
|
|
||
| ## Tests | ||
| - Presenter: sidecar lifecycle, script discovery, overwrite/uninstall behavior. | ||
| - Runtime: active skill enforcement, runtime fallback, script path validation. | ||
| - Agent tooling: prompt injection, `skill_run`, binary read rejection. | ||
| - ACP: `read_text_file` binary rejection. | ||
| - Process handling: `close`-based flush behavior. | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| # Skill Runtime Hardening | ||
|
|
||
| ## Summary | ||
|
|
||
| Add runtime-aware skill extensions without changing external `SKILL.md` formats. DeepChat stores skill-only runtime settings in a sidecar directory and uses them to execute bundled scripts safely and predictably. | ||
|
|
||
| ## User Stories | ||
|
|
||
| ### US-1: Configure skill environment variables | ||
| - As a user, I can define env vars for a skill in Settings. | ||
| - Acceptance: | ||
| - Env vars are stored outside the skill folder as plaintext sidecar metadata. | ||
| - Editing a skill does not rewrite external `SKILL.md` frontmatter to persist env vars. | ||
|
zerob13 marked this conversation as resolved.
|
||
|
|
||
| ### US-2: Run skill scripts reliably | ||
| - As an agent, I can run scripts bundled in an active skill without guessing relative paths. | ||
| - Acceptance: | ||
| - Only scripts under `<skillRoot>/scripts/` can be executed. | ||
| - The runtime picks system `uv`/`node` first, then falls back to DeepChat bundled runtimes. | ||
| - Python scripts run from the skill root and honor `pyproject.toml` via `uv run --project`. | ||
|
|
||
| ### US-3: Prevent prompt pollution from binary file reads | ||
| - As an agent, reading an image or binary file through text-file APIs does not inject raw bytes into prompt context. | ||
| - Acceptance: | ||
| - ACP `fs/read_text_file` rejects image/PDF/common binary files with remediation guidance. | ||
| - Main agent `read` keeps image OCR fallback, but rejects unsupported binary formats instead of returning raw bytes. | ||
|
|
||
| ### US-4: Guide the model toward stable skill execution | ||
| - As an agent, active skill instructions clearly include absolute paths, script inventory, and the preferred execution tool. | ||
| - Acceptance: | ||
| - Active skill prompt includes `skillRoot`, recommended `base_directory`, runnable scripts, and explicit guardrails against inline `python -c` / `node -e`. | ||
|
|
||
| ### US-5: Keep process output after process exit | ||
| - As an agent, command output is still available when a child process writes a large payload right before exiting. | ||
| - Acceptance: | ||
| - Foreground exec waits for child `close`. | ||
| - Background exec sessions are considered complete only after `close` and log flush. | ||
| - Large foreground output is offloaded to a session log file instead of being silently truncated away. | ||
|
|
||
| ## Non-Goals | ||
| - Secret encryption or OS keychain storage for skill env vars. | ||
| - Extending external skill formats with DeepChat-only frontmatter fields. | ||
| - General workflow orchestration across multiple skills. | ||
|
|
||
| ## Constraints | ||
| - Keep existing skills compatible. | ||
| - Ignore `.deepchat-meta` in skill discovery and sync/export flows. | ||
| - Reuse the existing `process` tool for background session management. | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| # Skill Runtime Hardening Tasks | ||
|
|
||
| 1. Add shared types for skill runtime config and script descriptors. | ||
| 2. Extend `SkillPresenter` with sidecar persistence and script discovery. | ||
| 3. Add `SkillExecutionService` and wire `skill_run` into agent tools. | ||
| 4. Harden binary read behavior in ACP and main agent reads. | ||
| 5. Switch exec completion logic from `exit` to `close` and await output flush. | ||
| 6. Extend skills settings UI to edit runtime config and show summaries. | ||
| 7. Add or update tests for presenter, ACP, agent tooling, and process output behavior. |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,108 @@ | ||
| import path from 'path' | ||
| import { detectMimeType, isLikelyTextFile } from '@/presenter/filePresenter/mime' | ||
|
|
||
| const TEXT_LIKE_MIMES = new Set([ | ||
| 'application/json', | ||
| 'application/xml', | ||
| 'application/javascript', | ||
| 'application/x-javascript', | ||
| 'application/typescript', | ||
| 'application/x-typescript', | ||
| 'application/x-sh' | ||
| ]) | ||
|
|
||
| const DOCUMENT_MIMES = new Set([ | ||
| 'application/pdf', | ||
| 'application/msword', | ||
| 'application/vnd.openxmlformats-officedocument.wordprocessingml.document', | ||
| 'application/vnd.ms-powerpoint', | ||
| 'application/vnd.openxmlformats-officedocument.presentationml.presentation', | ||
| 'application/vnd.ms-excel', | ||
| 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', | ||
| 'application/vnd.oasis.opendocument.spreadsheet' | ||
| ]) | ||
|
|
||
| const ALWAYS_BINARY_MIMES = new Set([ | ||
| 'application/zip', | ||
| 'application/x-zip', | ||
| 'application/gzip', | ||
| 'application/x-gzip', | ||
| 'application/x-7z-compressed', | ||
| 'application/x-rar-compressed', | ||
| 'application/wasm' | ||
| ]) | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| export function isTextLikeMime(mimeType: string): boolean { | ||
| return mimeType.startsWith('text/') || TEXT_LIKE_MIMES.has(mimeType) | ||
| } | ||
|
|
||
| export function isDocumentMime(mimeType: string): boolean { | ||
| return DOCUMENT_MIMES.has(mimeType) | ||
| } | ||
|
|
||
| export async function shouldRejectAcpTextRead(filePath: string): Promise<{ | ||
| reject: boolean | ||
| mimeType: string | ||
| }> { | ||
| const mimeType = await detectMimeType(filePath) | ||
|
|
||
| if (isTextLikeMime(mimeType)) { | ||
| return { reject: false, mimeType } | ||
| } | ||
|
|
||
| if (mimeType === 'application/octet-stream') { | ||
| const likelyText = await isLikelyTextFile(filePath) | ||
| return { reject: !likelyText, mimeType } | ||
| } | ||
|
|
||
| return { reject: true, mimeType } | ||
| } | ||
|
|
||
| export async function shouldRejectAgentBinaryRead( | ||
| filePath: string, | ||
| mimeType: string | ||
| ): Promise<boolean> { | ||
| if (mimeType.startsWith('image/')) { | ||
| return false | ||
| } | ||
|
|
||
| if (isTextLikeMime(mimeType) || isDocumentMime(mimeType) || mimeType === 'text/csv') { | ||
| return false | ||
| } | ||
|
|
||
| if ( | ||
| ALWAYS_BINARY_MIMES.has(mimeType) || | ||
| mimeType.startsWith('audio/') || | ||
| mimeType.startsWith('video/') | ||
| ) { | ||
| return true | ||
| } | ||
|
|
||
| if (mimeType === 'application/octet-stream') { | ||
| return !(await isLikelyTextFile(filePath)) | ||
| } | ||
|
|
||
| return false | ||
| } | ||
|
|
||
| export function buildBinaryReadGuidance( | ||
| filePath: string, | ||
| mimeType: string, | ||
| mode: 'agent' | 'acp' | ||
| ): string { | ||
| const fileName = path.basename(filePath) | ||
| const shared = `Cannot read "${fileName}" as plain text (detected MIME: ${mimeType}).` | ||
|
|
||
| if (mode === 'acp') { | ||
| return [ | ||
| shared, | ||
| '`fs/read_text_file` only supports text files.', | ||
| 'Use OCR/image tooling for images, and convert or extract PDFs/binary formats before reading them as text.' | ||
| ].join(' ') | ||
| } | ||
|
|
||
| return [ | ||
| shared, | ||
| 'Use image OCR/summary for images, or a dedicated conversion/extraction tool or skill script for binary formats.' | ||
| ].join(' ') | ||
| } | ||
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.