fix(sdk): address remaining tooling review findings

packages/sdk/helper/src/documents/cordis-yaml-file.ts

@@ -135,9 +135,11 @@ export class CordisYamlFile extends ProjectFile {
   }
 
   /** Add one new top-level entry, rejecting duplicate ids. */
-  addEntry(entry: CordisConfigEntry): void {
+  addEntry(entry: CordisConfigEntry, commentedExample?: string): void {
     if (this.entryNode(entry.id)) throw new Error(`Cordis config entry already exists: ${entry.id}`)
-    this.sequence().items.push(this.document.createNode(entry))
+    const node = this.document.createNode(entry)
+    if (commentedExample) node.comment = commentedExample.split('\n').map(line => ` ${line}`).join('\n')
+    this.sequence().items.push(node)
   }
 
   /** Remove an entry by id and report whether it existed. */

packages/sdk/helper/src/documents/pnpm-workspace-file.ts

@@ -4,73 +4,93 @@
  * @module @deepseek-ai/dsh-helper/documents/pnpm-workspace-file
  */
 
-import { parse, stringify } from 'yaml'
+import {
+  isMap, isScalar, isSeq, parseDocument,
+  type Document, type Scalar, type YAMLMap, type YAMLSeq,
+} from 'yaml'
 import { ProjectFile, withTrailingNewline } from './project-file.ts'
 
+function parseYaml(text: string): Document.Parsed {
+  const document = parseDocument(text, { keepSourceTokens: true, prettyErrors: true })
+  if (document.errors.length > 0) {
+    throw new Error(`invalid pnpm-workspace.yaml: ${document.errors.map(error => error.message).join('; ')}`)
+  }
+  if (!isMap(document.contents)) throw new Error('pnpm-workspace.yaml root must be an object')
+  return document
+}
+
 /** Generated pnpm-workspace.yaml model. */
 export class PnpmWorkspaceFile extends ProjectFile {
-  private readonly packages = new Set<string>()
-  private autoInstallPeers = true
+  private readonly document: Document.Parsed
 
-  private constructor(originalText?: string) {
+  private constructor(document: Document.Parsed, originalText?: string) {
     super('pnpm-workspace.yaml', originalText)
+    this.document = document
   }
 
   /** Create a pnpm workspace document. */
   static create(): PnpmWorkspaceFile {
-    return new PnpmWorkspaceFile()
-  }
-
-  /** Parse the workspace fields the SDK owns. */
-  static parse(text: string): PnpmWorkspaceFile {
-    const value: unknown = parse(text)
-    if (value === null || Array.isArray(value) || typeof value !== 'object') {
-      throw new Error('pnpm-workspace.yaml root must be an object')
-    }
-    const input = value as Record<string, unknown>
-    if (!Array.isArray(input.packages) || input.packages.some(item => typeof item !== 'string')) {
-      throw new Error('pnpm-workspace.yaml packages must be an array of strings')
-    }
-    const document = new PnpmWorkspaceFile(text)
-    for (const pattern of input.packages) document.packages.add(pattern as string)
-    if (input.autoInstallPeers !== undefined && typeof input.autoInstallPeers !== 'boolean') {
-      throw new Error('pnpm-workspace.yaml autoInstallPeers must be boolean')
-    }
-    document.autoInstallPeers = input.autoInstallPeers !== false
+    const document = new PnpmWorkspaceFile(parseYaml('{}\n'))
+    document.mapping().set('packages', document.document.createNode([]))
+    document.mapping().set('allowBuilds', document.document.createNode({ esbuild: true }))
     return document
   }
 
-  /** Clone workspace globs and peer-install policy. */
+  /** Parse the workspace fields the SDK owns while retaining all other YAML. */
+  static parse(text: string): PnpmWorkspaceFile {
+    const document = new PnpmWorkspaceFile(parseYaml(text), text)
+    document.packageSequence()
+    const autoInstallPeers = document.mapping().get('autoInstallPeers')
+    if (autoInstallPeers !== undefined && typeof autoInstallPeers !== 'boolean') {
+      throw new Error('pnpm-workspace.yaml autoInstallPeers must be boolean')
+    }
+    return document
+  }
+
+  /** Clone the complete comment-preserving workspace document. */
   override clone(): PnpmWorkspaceFile {
-    const clone = new PnpmWorkspaceFile(this.originalText)
-    for (const pattern of this.packages) clone.packages.add(pattern)
-    clone.autoInstallPeers = this.autoInstallPeers
-    return clone
+    return new PnpmWorkspaceFile(parseYaml(this.serialize()), this.originalText)
   }
 
   /** Add one package workspace glob. */
   addPackage(pattern: string): void {
-    this.packages.add(pattern)
+    const packages = this.packageSequence()
+    if (packages.items.some(item => item.value === pattern)) return
+    packages.add(this.document.createNode(pattern))
   }
 
   /** Disable registry peer auto-installation for live-link projects. */
   disableAutoInstallPeers(): void {
-    this.autoInstallPeers = false
+    this.mapping().set('autoInstallPeers', false)
   }
 
   /** Validate workspace globs. */
   override validate(): void {
-    for (const pattern of this.packages) {
+    for (const pattern of this.packageValues()) {
       if (pattern.trim().length === 0) throw new Error('pnpm workspace pattern must not be empty')
     }
   }
 
-  /** Serialize pnpm's workspace and peer policy. */
+  /** Serialize the workspace while retaining unknown settings and comments. */
   override serialize(): string {
-    return withTrailingNewline(stringify({
-      packages: [...this.packages].sort(),
-      ...this.autoInstallPeers ? {} : { autoInstallPeers: false },
-      allowBuilds: { esbuild: true },
-    }, { lineWidth: 0 }))
+    return withTrailingNewline(this.document.toString({ lineWidth: 0 }))
+  }
+
+  private mapping(): YAMLMap {
+    /* v8 ignore next -- parseYaml and create both establish a mapping root */
+    if (!isMap(this.document.contents)) throw new Error('pnpm-workspace.yaml root must be an object')
+    return this.document.contents
+  }
+
+  private packageSequence(): YAMLSeq<Scalar<string>> {
+    const packages = this.mapping().get('packages', true)
+    if (!isSeq(packages) || packages.items.some(item => !isScalar(item) || typeof item.value !== 'string')) {
+      throw new Error('pnpm-workspace.yaml packages must be an array of strings')
+    }
+    return packages as YAMLSeq<Scalar<string>>
+  }
+
+  private packageValues(): string[] {
+    return this.packageSequence().items.map(item => item.value)
   }
 }

packages/sdk/helper/src/features/builtin/index.ts

@@ -55,7 +55,15 @@ export function createBuiltinRegistry(profile: ProjectProfile): FeatureRegistry
           label: 'Sandboxed executor',
           resources: [
             { kind: 'npm-cordis-config-entry', id: 'sandbox', package: '@deepseek-ai/dsh-sandbox-local' },
-            { kind: 'npm-cordis-config-entry', id: 'bash', package: '@deepseek-ai/dsh-bash-sandbox' },
+            {
+              kind: 'npm-cordis-config-entry',
+              id: 'bash',
+              package: '@deepseek-ai/dsh-bash-sandbox',
+              commentedExample: `Uncomment to allow writes under the project workspace.
+config:
+  mode: workspace-write
+  workspaceRoot: !!js process.cwd()`,
+            },
           ],
         },
       ],

packages/sdk/helper/src/features/define-feature.ts

@@ -31,6 +31,7 @@ interface NpmCordisConfigEntrySpec {
   package: string
   config?: Readonly<Record<string, unknown>>
   ownedConfigKeys?: readonly string[]
+  commentedExample?: string
 }
 
 /** Relative or absolute file Cordis config entry with no NPM dependency. */
@@ -40,6 +41,7 @@ interface FileCordisConfigEntrySpec {
   path: string
   config?: Readonly<Record<string, unknown>>
   ownedConfigKeys?: readonly string[]
+  commentedExample?: string
 }
 
 /** Static complete file owned by one feature option. */
@@ -147,6 +149,7 @@ function resourcesFromSpec(spec: FeatureResourceSpec): ProjectResource[] {
             ...config ? { config } : {},
           },
           ownedConfigKeys: spec.ownedConfigKeys ?? Object.keys(config ?? {}),
+          ...spec.commentedExample ? { commentedExample: spec.commentedExample } : {},
           ...validateConfig ? { validateConfig } : {},
         },
       ]

packages/sdk/helper/src/features/resources.ts

@@ -31,6 +31,7 @@ export interface CordisConfigEntryResource {
   key: ResourceKey
   entry: CordisConfigEntry
   ownedConfigKeys: readonly string[]
+  commentedExample?: string
   validateConfig?: (config: Readonly<Record<string, unknown>>) => readonly string[]
 }
 

packages/sdk/helper/src/project/project-edit-session.ts

@@ -19,7 +19,7 @@ import { EnvFile } from '../documents/env-file.ts'
 import { PackageJsonFile, type PackageManifest } from '../documents/package-json-file.ts'
 import { ProjectFile } from '../documents/project-file.ts'
 import { TsConfigFile } from '../documents/tsconfig-file.ts'
-import type { FeatureId, ResourceKey } from '../ids.ts'
+import { featureId, type FeatureId, type ResourceKey } from '../ids.ts'
 import { LinkWorkspace } from '../package-managers/link-workspace.ts'
 import type { LocalPluginBlueprint } from '../plugins/local-plugin-blueprint.ts'
 import type { FeatureSelection, ProjectProfile } from './types.ts'
@@ -390,7 +390,7 @@ export class ProjectEditSession implements FeatureProjectView {
       }
       case 'cordis-config-entry': {
         const current = this.cordis().entry(resource.entry.id)
-        if (!current) this.cordis().addEntry(resource.entry)
+        if (!current) this.cordis().addEntry(resource.entry, resource.commentedExample)
         else {
           if (current.name !== resource.entry.name) {
             throw new Error(`Cordis config entry ${resource.entry.id} is owned by ${current.name}, not ${resource.entry.name}`)
@@ -486,9 +486,17 @@ export class ProjectEditSession implements FeatureProjectView {
 
   private validateFinalState(): void {
     for (const document of this.documents.values()) document.validate()
+    const profile = this.finalProfile()
+    const view = this.projectView(profile)
     for (const feature of this.registry.all()) {
-      if (!feature.isApplicable(this.profile)) continue
-      const installation = feature.inspect(this)
+      const state = this.states.get(feature.id)
+      if (!feature.isApplicable(profile)) {
+        if (state?.state === 'enabled') {
+          throw new Error(`feature ${feature.id} is not available for ${profile.runInterface}`)
+        }
+        continue
+      }
+      const installation = feature.inspect(view)
       /* v8 ignore next 3 -- public domain commands assert feature consistency before final validation */
       if (installation.state === 'inconsistent') {
         throw new Error(`feature ${feature.id} is inconsistent: ${installation.diagnostics.join('; ')}`)
@@ -499,7 +507,7 @@ export class ProjectEditSession implements FeatureProjectView {
       }
       if (installation.state !== 'enabled' || !installation.selection) continue
       for (const requirement of feature.requirements(installation.selection)) {
-        const required = this.registry.get(requirement.id).inspect(this)
+        const required = this.registry.get(requirement.id).inspect(view)
         /* v8 ignore next 3 -- ensureRequirements establishes enabled requirements before contributions change */
         if (required.state !== 'enabled') {
           throw new Error(`feature ${feature.id} requires enabled ${requirement.id}`)
@@ -522,6 +530,22 @@ export class ProjectEditSession implements FeatureProjectView {
     }
   }
 
+  private finalProfile(): ProjectProfile {
+    const runInterface = this.states.get(featureId('app'))?.selection?.options[0]
+    if (runInterface !== 'acp' && runInterface !== 'stdio' && runInterface !== 'embed') return this.profile
+    return { ...this.profile, runInterface }
+  }
+
+  private projectView(profile: ProjectProfile): FeatureProjectView {
+    return {
+      profile,
+      cordisConfigEntries: () => this.cordisConfigEntries(),
+      packageManifest: () => this.packageManifest(),
+      hasDocument: path => this.hasDocument(path),
+      readEnvironment: (path, name) => this.readEnvironment(path, name),
+    }
+  }
+
   private async assertUnchanged(paths: readonly string[]): Promise<void> {
     for (const path of paths) {
       const source = this.source.document(path)