diff --git a/client/src/components/cos/tabs/schedule/AppOverrideRow.jsx b/client/src/components/cos/tabs/schedule/AppOverrideRow.jsx index 2674772b77..c78222e5f4 100644 --- a/client/src/components/cos/tabs/schedule/AppOverrideRow.jsx +++ b/client/src/components/cos/tabs/schedule/AppOverrideRow.jsx @@ -8,7 +8,7 @@ import ToggleSwitch from '../../../ToggleSwitch'; import useFieldDraft from '../../../../hooks/useFieldDraft'; import { INTERVAL_LABELS, setMetadataOverride } from './scheduleConstants'; -const AppOverrideRow = memo(function AppOverrideRow({ app, taskType, globalIntervalType, globalTaskMetadata, managedAgentOptions, fileIssuesCapable, defaultFileIssues, inheritedProviderText, providers, override, onUpdate }) { +const AppOverrideRow = memo(function AppOverrideRow({ app, taskType, globalIntervalType, globalTaskMetadata, managedAgentOptions, fileIssuesCapable, defaultFileIssues, doWorkRequiresWorktree, inheritedProviderText, providers, override, onUpdate }) { const [updating, setUpdating] = useState(false); const [cronEditing, setCronEditing] = useState(false); const isEnabled = override?.enabled === true; @@ -61,6 +61,8 @@ const AppOverrideRow = memo(function AppOverrideRow({ app, taskType, globalInter : null; if (field === 'fileIssues' && nextFileIssues === true && taskMetadata) { taskMetadata = { ...taskMetadata, useWorktree: false, openPR: false, simplify: false }; + } else if (field === 'fileIssues' && nextFileIssues === false && doWorkRequiresWorktree && taskMetadata) { + taskMetadata = { ...taskMetadata, useWorktree: true }; } await onUpdate(app.id, taskType, { taskMetadata }).catch(() => {}); setUpdating(false); @@ -175,7 +177,9 @@ const AppOverrideRow = memo(function AppOverrideRow({ app, taskType, globalInter const effective = override?.taskMetadata?.[field] ?? globalTaskMetadata?.[field] ?? false; const hasOverride = override?.taskMetadata?.[field] !== undefined; const fileIssuesOn = (override?.taskMetadata?.fileIssues ?? globalTaskMetadata?.fileIssues ?? defaultFileIssues) === true; - const managed = managedAgentOptions?.includes(field) || (fileIssuesCapable && fileIssuesOn && ['useWorktree', 'openPR', 'simplify'].includes(field)); + const managed = managedAgentOptions?.includes(field) + || (fileIssuesCapable && fileIssuesOn && ['useWorktree', 'openPR', 'simplify'].includes(field)) + || (doWorkRequiresWorktree && !fileIssuesOn && field === 'useWorktree'); const titleText = managed ? `${label}: managed internally by ${taskType}` : `${label}: ${effective ? 'on' : 'off'}${hasOverride ? ' (app override)' : ' (inherited)'}`; diff --git a/client/src/components/cos/tabs/schedule/AppOverrideRow.test.jsx b/client/src/components/cos/tabs/schedule/AppOverrideRow.test.jsx index 8b7a2104f0..3957ec0783 100644 --- a/client/src/components/cos/tabs/schedule/AppOverrideRow.test.jsx +++ b/client/src/components/cos/tabs/schedule/AppOverrideRow.test.jsx @@ -12,6 +12,7 @@ function renderRow({ taskType = 'feature-ideas', fileIssuesCapable, defaultFileIssues, + doWorkRequiresWorktree, inheritedProviderText, providers, } = {}) { @@ -24,6 +25,7 @@ function renderRow({ managedAgentOptions={[]} fileIssuesCapable={fileIssuesCapable} defaultFileIssues={defaultFileIssues} + doWorkRequiresWorktree={doWorkRequiresWorktree} inheritedProviderText={inheritedProviderText} providers={providers} override={override} @@ -188,6 +190,22 @@ describe('AppOverrideRow — file issues only', () => { taskMetadata: { fileIssues: true, useWorktree: false, openPR: false, simplify: false }, }); }); + + it('restores required worktree isolation in a per-app do-work override', async () => { + const onUpdate = renderRow({ + taskType: 'module-hygiene', + fileIssuesCapable: true, + defaultFileIssues: true, + doWorkRequiresWorktree: true, + globalTaskMetadata: { fileIssues: true, useWorktree: false, openPR: false }, + }); + await act(async () => { + fireEvent.click(screen.getByRole('button', { name: /File issues only/i })); + }); + expect(onUpdate).toHaveBeenCalledWith('app-1', 'module-hygiene', { + taskMetadata: { fileIssues: false, useWorktree: true }, + }); + }); }); describe('AppOverrideRow — branch-reconcile batch size', () => { diff --git a/client/src/components/cos/tabs/schedule/GlobalConfigControls.jsx b/client/src/components/cos/tabs/schedule/GlobalConfigControls.jsx index 47c1942c94..027d186c9a 100644 --- a/client/src/components/cos/tabs/schedule/GlobalConfigControls.jsx +++ b/client/src/components/cos/tabs/schedule/GlobalConfigControls.jsx @@ -526,7 +526,11 @@ export default function GlobalConfigControls({ taskType, config, onUpdate, onTri onClick={async () => { setUpdating(true); await onUpdate(taskType, { - taskMetadata: toggleFileIssuesMetadata(config.taskMetadata, !fileIssuesEffective(config)) + taskMetadata: toggleFileIssuesMetadata( + config.taskMetadata, + !fileIssuesEffective(config), + config.doWorkRequiresWorktree, + ) }); setUpdating(false); }} diff --git a/client/src/components/cos/tabs/schedule/GlobalConfigControls.test.jsx b/client/src/components/cos/tabs/schedule/GlobalConfigControls.test.jsx index 479696cb67..cede4747c3 100644 --- a/client/src/components/cos/tabs/schedule/GlobalConfigControls.test.jsx +++ b/client/src/components/cos/tabs/schedule/GlobalConfigControls.test.jsx @@ -252,4 +252,16 @@ describe('GlobalConfigControls — file issues only', () => { taskMetadata: { useWorktree: false, openPR: false, simplify: false, fileIssues: true }, }); }); + + it('restores required worktree isolation when module-hygiene switches to do-work mode', () => { + const onUpdate = renderControls({ + taskType: 'module-hygiene', + taskMetadata: { useWorktree: false, openPR: false, simplify: false, fileIssues: true }, + config: { fileIssuesCapable: true, defaultFileIssues: true, doWorkRequiresWorktree: true }, + }); + fireEvent.click(screen.getByRole('button', { name: /File issues only/i })); + expect(onUpdate).toHaveBeenCalledWith('module-hygiene', { + taskMetadata: { useWorktree: true, openPR: false, simplify: false, fileIssues: false }, + }); + }); }); diff --git a/client/src/components/cos/tabs/schedule/PerAppOverrideList.jsx b/client/src/components/cos/tabs/schedule/PerAppOverrideList.jsx index 0a0a6d5632..b82d9e2bc5 100644 --- a/client/src/components/cos/tabs/schedule/PerAppOverrideList.jsx +++ b/client/src/components/cos/tabs/schedule/PerAppOverrideList.jsx @@ -55,6 +55,7 @@ export default function PerAppOverrideList({ taskType, config, apps, providers, managedAgentOptions={config.managedAgentOptions} fileIssuesCapable={config.fileIssuesCapable} defaultFileIssues={config.defaultFileIssues} + doWorkRequiresWorktree={config.doWorkRequiresWorktree} inheritedProviderText={inheritedProviderText} providers={providers} override={appOverrides[app.id]} diff --git a/client/src/components/cos/tabs/schedule/scheduleConstants.js b/client/src/components/cos/tabs/schedule/scheduleConstants.js index f438143ea7..42000d8c99 100644 --- a/client/src/components/cos/tabs/schedule/scheduleConstants.js +++ b/client/src/components/cos/tabs/schedule/scheduleConstants.js @@ -199,20 +199,26 @@ export const FILE_ISSUES_MANAGED_FIELDS = ['useWorktree', 'openPR', 'simplify']; export function managedAgentOptionsFor(config, overrideMetadata) { const managed = [...(config?.managedAgentOptions || [])]; - if (config?.fileIssuesCapable && fileIssuesEffective(config, overrideMetadata)) { + const fileIssues = fileIssuesEffective(config, overrideMetadata); + if (config?.fileIssuesCapable && fileIssues) { for (const field of FILE_ISSUES_MANAGED_FIELDS) { if (!managed.includes(field)) managed.push(field); } } + if (config?.doWorkRequiresWorktree && !fileIssues && !managed.includes('useWorktree')) { + managed.push('useWorktree'); + } return managed; } -export function toggleFileIssuesMetadata(metadata, next) { +export function toggleFileIssuesMetadata(metadata, next, doWorkRequiresWorktree = false) { const taskMetadata = { ...(metadata || {}), fileIssues: next }; if (next) { taskMetadata.useWorktree = false; taskMetadata.openPR = false; taskMetadata.simplify = false; + } else if (doWorkRequiresWorktree) { + taskMetadata.useWorktree = true; } return taskMetadata; } diff --git a/client/src/components/cos/tabs/schedule/scheduleConstants.test.js b/client/src/components/cos/tabs/schedule/scheduleConstants.test.js index d6c455470a..0824187248 100644 --- a/client/src/components/cos/tabs/schedule/scheduleConstants.test.js +++ b/client/src/components/cos/tabs/schedule/scheduleConstants.test.js @@ -42,15 +42,31 @@ describe('managedAgentOptionsFor', () => { it('leaves non-audit tasks alone', () => { expect(managedAgentOptionsFor({ managedAgentOptions: ['useWorktree'] })).toEqual(['useWorktree']); }); + + it('locks the worktree on for an isolation-required audit in do-work mode', () => { + expect(managedAgentOptionsFor({ + fileIssuesCapable: true, + defaultFileIssues: true, + doWorkRequiresWorktree: true, + }, { fileIssues: false })).toEqual(['useWorktree']); + }); }); describe('toggleFileIssuesMetadata', () => { - it('forces the no-code posture on and leaves it when turning off', () => { + it('forces the no-code posture on and otherwise leaves agent options alone', () => { expect(toggleFileIssuesMetadata({ useWorktree: true, openPR: true, simplify: true }, true)) .toEqual({ useWorktree: false, openPR: false, simplify: false, fileIssues: true }); expect(toggleFileIssuesMetadata({ fileIssues: true, useWorktree: false }, false)) .toEqual({ fileIssues: false, useWorktree: false }); }); + + it('restores required worktree isolation when do-work mode is selected', () => { + expect(toggleFileIssuesMetadata( + { fileIssues: true, useWorktree: false, openPR: false }, + false, + true, + )).toEqual({ fileIssues: false, useWorktree: true, openPR: false }); + }); }); describe('toggleMetadataField', () => { diff --git a/data.reference/prompts/skills/module-hygiene.md b/data.reference/prompts/skills/module-hygiene.md new file mode 100644 index 0000000000..801337849e --- /dev/null +++ b/data.reference/prompts/skills/module-hygiene.md @@ -0,0 +1,35 @@ +# Module Hygiene Audit Skill Template + +## Routing +**Use when**: The task is explicitly a module-hygiene audit or remediation run. +**Don't use when**: The task only removes dead code, fixes an unrelated defect, or asks for a broad rewrite without evidence. + +## Task-Specific Guidelines + +Improve structural maintainability without treating code size as a defect. + +### 1. Thresholds nominate; evidence decides +- Use complexity, function length, nesting, file size, churn, and fan-in to select candidates. +- Keep a finding only after tracing responsibilities, callers, tests, and history to a concrete change cost. +- Reject primarily declarative, generated, compatibility, mirror, and semantic-adapter false positives. + +### 2. Prove reuse discovery +- Search the repository's own catalogs, domain maps, public exports, naming variants, and importers. +- Record why an existing module should be extended or why a new public seam is necessary. +- Prefer consolidation that deletes a duplicate implementation over a wrapper that preserves both. + +### 3. Match discovery to the surface +- Curate and parity-check a catalog only for a genuinely reusable/public surface. +- Prefer lightweight ownership and placement guidance for broad implementation directories. +- Never require exhaustive catalogs or barrels merely because a directory is large. + +### 4. De-duplicate across history +- Search open and closed tracker work plus merged changes by file, symbol, and behavior. +- Link or reuse sibling simplification or code-quality work instead of filing it again under a new label. + +### 5. Mode +In file-issues mode, change no source and file only decision-complete findings. In implementation mode, make one behavior-preserving improvement in the isolated worktree and verify it at the highest practical public boundary. + +## Successful Outcome + +The run names its bounded slice and reuse searches, reports what it rejected, and produces zero to three high-confidence findings normally. A well-supported no-finding result is successful. diff --git a/server/lib/README.md b/server/lib/README.md index be192419f0..1f6ee3b1b8 100644 --- a/server/lib/README.md +++ b/server/lib/README.md @@ -61,7 +61,7 @@ The barrel `server/lib/index.js` is a machine-checkable enumeration of every pub | `agentValidation.js` | Social-bot agent schemas (personality, Moltbook/Moltworld accounts, automation schedules, agent tools + Moltworld payloads) and CoS Feature Agent definitions. | | `quotaBurnConfig.js` | Quota-burn plan shape: the provider families, the burn-job type alphabet + catalog the config page renders its form from, `QUOTA_BURN_BOUNDS` (the one bounds table the normalizer clamps to, the Zod schemas reject against, and the catalog descriptors publish as min/max), and total normalization (`normalizeQuotaBurnConfig`). Owns the dispatch-cap sentinel too (`QUOTA_BURN_UNLIMITED_DISPATCHES` / `isUnlimitedDispatchCap`) — -1 means the window is not counted, and is the default. Also owns the queued burn task's description shape (`burnTaskDescription` / `quotaBurnFamilyOfDescription`), shared with migration 225, and the `run once` vocabulary (`quotaBurnJobKey` / `jobIsSpent`, plus the two family predicates `familyIsConfigured` and `familyHasRunnableJobs`). Pure — no storage, no provider I/O. | | `quotaBurnPresets.js` | `QUOTA_BURN_PROMPT_PRESETS` — ready-made single-focus audit prompts for `agent-prompt` burn jobs (UX, a11y, mobile, failure paths, perf, test gaps, dead code, data safety, docs, security), each filing GitHub issues and changing no code. Templates: picking one COPIES its prompt into the job, so editing them never rewrites a configured job. `findQuotaBurnPreset(id)`. | -| `auditCatalog.js` | Shared catalog of scheduled AUDIT task types (`AUDIT_DEFINITIONS` / `AUDIT_TASK_TYPES`) that can either implement a fix or file tracker issues. `isFileIssuesMode`, `getAuditFilingPreset`, `modeContractFor`, `applyAuditModeWrapper`. Each quota-burn audit preset maps to a scheduled type here (guarded by `auditCatalog.test.js`). Pure — no I/O. | +| `auditCatalog.js` | Shared catalog of scheduled AUDIT task types (`AUDIT_DEFINITIONS` / `AUDIT_TASK_TYPES`) that can either implement a fix or file tracker issues. `isFileIssuesMode`, `auditDoWorkRequiresWorktree`, `getAuditFilingPreset`, `modeContractFor`, `applyAuditModeWrapper`. Each quota-burn audit preset maps to a scheduled type here (guarded by `auditCatalog.test.js`). Pure — no I/O. | | `quotaBurnValidation.js` | Zod schemas for the Quota Burn routes (partial config PUT, manual-run body, `run once` re-arm body). | | `quotaReset.js` | `parseHumanReset` turns a provider CLI's human reset string into ISO 8601 (call it from the adapter); `normalizeResetAt`/`hoursUntilReset` compute time remaining without treating unknown values as imminent resets; `parseObservedReset` pulls the reset a provider stated in its own refusal text, and `isObservedBlockActive` is the shared "is that refusal still holding?" predicate every observed-refusal ledger uses. | | `quotaWindows.js` | Classifies a quota window by PERIOD (`windowPeriodHours`); `classifyWindows` splits one card's windows into the weekly allowance that expires unused and the 5-hour one that refuses first, in one pass. `windowLabelOf` names a window. Pure. | diff --git a/server/lib/auditCatalog.js b/server/lib/auditCatalog.js index daf648d50c..332f2ba754 100644 --- a/server/lib/auditCatalog.js +++ b/server/lib/auditCatalog.js @@ -249,6 +249,20 @@ export const AUDIT_DEFINITIONS = Object.freeze({ noun: 'simplify finding(s)', }), }, + 'module-hygiene': { + quotaBurnId: null, + label: 'Module hygiene', + description: 'Module-hygiene audit — configurable: file issues (default) or implement one isolated refactor', + defaultFileIssues: true, + doWorkRequiresWorktree: true, + filing: filing({ + slugPrefix: 'module-hygiene-', + label: 'module-hygiene-audit', + issueLabel: 'code-quality', + labelDescription: 'Proposed from a module-hygiene audit', + noun: 'module-hygiene finding(s)', + }), + }, 'api-contract': { quotaBurnId: 'api-contract-audit', label: 'API & route contracts', @@ -329,6 +343,17 @@ export function defaultFileIssuesFor(taskType) { return AUDIT_DEFINITIONS[taskType]?.defaultFileIssues === true; } +/** + * Whether do-work mode for this audit must use an isolated managed worktree. + * The flag is catalog-owned so dispatch and schedule UI cannot drift. + * + * @param {string} taskType - Task type identifier + * @returns {boolean} True when live-checkout remediation is forbidden + */ +export function auditDoWorkRequiresWorktree(taskType) { + return AUDIT_DEFINITIONS[taskType]?.doWorkRequiresWorktree === true; +} + /** * Effective file-issues mode for a dispatch. An explicit `fileIssues` boolean * on the merged task metadata wins; otherwise the catalog default applies. diff --git a/server/lib/auditCatalog.test.js b/server/lib/auditCatalog.test.js index 58c496da6b..e6a6785819 100644 --- a/server/lib/auditCatalog.test.js +++ b/server/lib/auditCatalog.test.js @@ -7,6 +7,7 @@ import { DO_WORK_MODE_CONTRACT, isAuditTaskType, defaultFileIssuesFor, + auditDoWorkRequiresWorktree, isFileIssuesMode, getAuditFilingPreset, modeContractFor, @@ -36,10 +37,17 @@ describe('AUDIT_DEFINITIONS', () => { expect(defaultFileIssuesFor('ux')).toBe(true); expect(defaultFileIssuesFor('data-safety')).toBe(true); expect(defaultFileIssuesFor('simplify')).toBe(true); + expect(defaultFileIssuesFor('module-hygiene')).toBe(true); expect(defaultFileIssuesFor('security')).toBe(false); expect(defaultFileIssuesFor('accessibility')).toBe(false); expect(defaultFileIssuesFor('unknown')).toBe(false); }); + + it('declares isolation as a catalog capability only for audits that require it', () => { + expect(auditDoWorkRequiresWorktree('module-hygiene')).toBe(true); + expect(auditDoWorkRequiresWorktree('simplify')).toBe(false); + expect(auditDoWorkRequiresWorktree('unknown')).toBe(false); + }); }); describe('isFileIssuesMode', () => { @@ -99,6 +107,7 @@ describe('getAuditFilingPreset', () => { it('returns the preset for an audit type and null otherwise', () => { expect(getAuditFilingPreset('data-safety').slugPrefix).toBe('data-safety-'); expect(getAuditFilingPreset('simplify').issueLabel).toBe('code-quality'); + expect(getAuditFilingPreset('module-hygiene').slugPrefix).toBe('module-hygiene-'); expect(getAuditFilingPreset('claim-issue')).toBeNull(); }); diff --git a/server/services/agentPromptBuilder.test.js b/server/services/agentPromptBuilder.test.js index 455bc120ac..5969d4e756 100644 --- a/server/services/agentPromptBuilder.test.js +++ b/server/services/agentPromptBuilder.test.js @@ -64,11 +64,23 @@ assertProvider: (provider, { message, code, status = 503 } = {}) => { })); vi.mock('../lib/fileUtils.js', async (importOriginal) => { const actual = await importOriginal(); + const { readFile } = await import('fs/promises'); return { ...actual, // getAppWorkspace reads data/apps.json through this — mocked so the tilde // tests below never touch the real registry. readJSONFile: vi.fn(actual.readJSONFile), + // The production install copies shipped templates into data/. This test + // reads the committed module-hygiene template directly so the API-path + // assembly assertion stays independent of ignored runtime state. + tryReadFile: vi.fn(async (path, ...args) => { + const normalized = String(path).replace(/\\/g, '/'); + if (normalized.endsWith('/data/prompts/skills/module-hygiene.md')) { + return readFile(`${actual.PATHS.root}/data.reference/prompts/skills/module-hygiene.md`, 'utf8') + .catch(() => null); + } + return actual.tryReadFile(path, ...args); + }), }; }); vi.mock('../lib/slashdoLoader.js', async (importOriginal) => { @@ -102,6 +114,7 @@ import { getDigitalTwinForPrompt } from './digital-twin.js'; import { getToolsSummaryForPrompt } from './tools.js'; import { loadSlashdoFile, loadSlashdoLib, writeResolvedSlashdoBody } from '../lib/slashdoLoader.js'; // mocked above — control the inlined body import { SLASHDO_INLINE_BUDGET_CHARS } from '../lib/slashdoInvocation.js'; +import { DEFAULT_TASK_PROMPTS } from './taskPromptDefaults.js'; // The heading a task-type hook's prompt points at to locate the sentinel path. import { PROGRAMMATIC_OUTPUT_COMPLETION_HEADING } from '../lib/agentSentinel.js'; @@ -134,7 +147,10 @@ describe('composable skill template routing', () => { }))).toEqual(['security-audit', 'threejs-visual']); }); - it('routes data-safety and dead-code audits to their own skill templates', () => { + it('routes module-hygiene, data-safety, and dead-code audits to their own skill templates', () => { + expect(detectSkillTemplates(makeTask({ + description: '[Improvement] Module hygiene audit for shared components', + }))).toEqual(['module-hygiene']); expect(detectSkillTemplates(makeTask({ description: '[Improvement] Data and upgrade-safety audit', }))).toEqual(['data-safety']); @@ -143,6 +159,16 @@ describe('composable skill template routing', () => { }))).toEqual(['simplify']); }); + it.each(['analysisType', 'selfImprovementType'])( + 'prefers authoritative %s routing over broad description keywords', + (metadataKey) => { + expect(detectSkillTemplates(makeTask({ + description: 'Audit repository structure', + metadata: { [metadataKey]: 'module-hygiene' }, + }))).toEqual(['module-hygiene']); + }, + ); + it('joins templates in routing order and tolerates an unavailable domain guide', async () => { const loadTemplate = vi.fn(async (name) => ({ 'security-audit': 'Security lifecycle guidance', @@ -154,6 +180,48 @@ describe('composable skill template routing', () => { expect(loadTemplate).toHaveBeenNthCalledWith(1, 'security-audit'); expect(loadTemplate).toHaveBeenNthCalledWith(2, 'threejs-visual'); }); + + it('keeps the final module-hygiene mission generic across API and TUI/CLI paths', async () => { + const description = DEFAULT_TASK_PROMPTS['module-hygiene'] + .replaceAll('{appName}', 'Example App') + .replaceAll('{repoPath}', '/workspace/example-app') + .replace('{modeInstructions}', '## Mode: file issues, change nothing'); + const task = makeTask({ + description, + metadata: { analysisType: 'module-hygiene', noCodeOutput: true }, + }); + + const lightPrompt = buildLightContextPrompt( + task, + '/workspace/example-app', + null, + isTruthyMeta, + { providerId: 'codex-tui', providerCommand: 'codex' }, + ); + const apiPrompt = await buildAgentPrompt( + task, + {}, + '/workspace/example-app', + null, + isTruthyMeta, + { providerType: 'api' }, + ); + + for (const prompt of [lightPrompt, apiPrompt]) { + expect(prompt).toMatch(/crossing one is never a\s+finding by itself/); + expect(prompt).toContain('Reuse-search proof'); + expect(prompt).toContain('closed tracker items, and merged changes'); + expect(prompt).not.toContain('{appName}'); + expect(prompt).not.toContain('{repoPath}'); + expect(prompt).not.toContain('{modeInstructions}'); + expect(prompt).not.toContain('server/lib/README.md'); + expect(prompt).not.toContain('client/src/lib/README.md'); + expect(prompt).not.toContain('localhost:5555'); + } + expect(apiPrompt).toContain('## Task-Type Skill Guidelines'); + expect(apiPrompt).toContain('Thresholds nominate; evidence decides'); + expect(lightPrompt).not.toContain('## Task-Type Skill Guidelines'); + }); }); describe('reconcileSplitContext', () => { diff --git a/server/services/cosTaskGenerator.js b/server/services/cosTaskGenerator.js index 7ab8b30df6..a75de3760e 100644 --- a/server/services/cosTaskGenerator.js +++ b/server/services/cosTaskGenerator.js @@ -50,6 +50,7 @@ import { NON_ACTIONABLE_ISSUE_LABELS } from './perpetualWork.js'; import { isAuditTaskType, isFileIssuesMode, + auditDoWorkRequiresWorktree, modeContractFor, applyAuditModeWrapper, } from '../lib/auditCatalog.js'; @@ -3214,6 +3215,12 @@ export async function generateManagedAppImprovementTaskForType(taskType, app, st metadata.useWorktree = false; metadata.openPR = false; metadata.simplify = false; + } else if (auditDoWorkRequiresWorktree(taskType)) { + // Some structural audits are safe to remediate only in isolation. Enforce + // this after schedule/app defaults so a stale file-issues toggle transition + // cannot dispatch edits into the app's live checkout. PR creation remains + // independently configurable through metadata.openPR. + metadata.useWorktree = true; } // The app's per-app provider/model pin (#4783). Resolved through the shared // harness guard, so an api-typed pin falls back to the Schedule pin instead of diff --git a/server/services/cosTaskGenerator.referenceWatch.test.js b/server/services/cosTaskGenerator.referenceWatch.test.js index 1963348d91..a3e4c54ffe 100644 --- a/server/services/cosTaskGenerator.referenceWatch.test.js +++ b/server/services/cosTaskGenerator.referenceWatch.test.js @@ -323,4 +323,18 @@ describe('audit fileIssues toggle', () => { expect(task.description).toContain('Mode: implement the highest-value fix'); expect(task.description).not.toContain('[security-…]'); }); + + it('forces module-hygiene remediation into a worktree after an unsafe toggle transition', async () => { + const { getTaskInterval } = await import('./taskSchedule.js'); + getTaskInterval.mockResolvedValue({ + type: 'weekly', + taskMetadata: { fileIssues: false, useWorktree: false, openPR: false }, + }); + const task = await generate(makeApp(), 'module-hygiene'); + expect(task.metadata.fileIssues).toBe(false); + expect(task.metadata.useWorktree).toBe(true); + expect(task.metadata.openPR).toBe(false); + expect(task.metadata.noCodeOutput).toBeUndefined(); + expect(task.description).toContain('Mode: implement the highest-value fix'); + }); }); diff --git a/server/services/promptSections/instructions.js b/server/services/promptSections/instructions.js index ca7bacb08b..daafeeaebd 100644 --- a/server/services/promptSections/instructions.js +++ b/server/services/promptSections/instructions.js @@ -16,6 +16,10 @@ const SKILLS_DIR = join(PATHS.root, 'data/prompts/skills'); * Order matters — first match wins, so more specific patterns come first. */ const SKILL_MATCHERS = [ + { + skill: 'module-hygiene', + keywords: ['module-hygiene', 'module hygiene'] + }, { skill: 'data-safety', keywords: ['data-safety', 'upgrade-safety', 'schema parity', 'schemaversion', 'seed file', 'data.reference'] @@ -50,6 +54,28 @@ const SKILL_MATCHERS = [ } ]; +const SKILL_NAMES = new Set(SKILL_MATCHERS.map(({ skill }) => skill)); +const TASK_TYPE_SKILL_ALIASES = Object.freeze({ + security: 'security-audit', +}); + +const skillForTaskType = (task) => { + const taskTypes = [ + task?.metadata?.analysisType, + task?.metadata?.taskAnalysisType, + task?.metadata?.selfImprovementType, + ]; + for (const taskType of taskTypes) { + if (typeof taskType !== 'string') continue; + const normalized = taskType.trim().toLowerCase(); + if (Object.hasOwn(TASK_TYPE_SKILL_ALIASES, normalized)) { + return TASK_TYPE_SKILL_ALIASES[normalized]; + } + if (SKILL_NAMES.has(normalized)) return normalized; + } + return null; +}; + // Domain templates complement (rather than replace) the lifecycle template // selected above. Keep this list narrow: broad graphics terms would add prompt // weight to tasks that do not involve scene construction or rendering. @@ -71,6 +97,9 @@ const DOMAIN_SKILL_MATCHERS = [ * @returns {string|null} Skill template name or null if no match */ export function detectSkillTemplate(task) { + const taskTypeSkill = skillForTaskType(task); + if (taskTypeSkill) return taskTypeSkill; + const desc = (task?.description || '').toLowerCase(); for (const matcher of SKILL_MATCHERS) { if (matcher.keywords.some(kw => desc.includes(kw))) { diff --git a/server/services/taskPromptDefaults.test.js b/server/services/taskPromptDefaults.test.js index 0ed6b20a61..14a7bb7ba0 100644 --- a/server/services/taskPromptDefaults.test.js +++ b/server/services/taskPromptDefaults.test.js @@ -35,6 +35,25 @@ const SNAPSHOT = JSON.parse(readFileSync( )); describe('taskPromptDefaults integrity snapshot', () => { + it('module-hygiene v1 is generic, evidence-led, and bounded', () => { + const current = DEFAULT_TASK_PROMPTS['module-hygiene']; + + expect(PROMPT_VERSIONS['module-hygiene']).toBe(1); + expect(current).toContain('{appName}'); + expect(current).toContain('{repoPath}'); + expect(current).toContain('{modeInstructions}'); + expect(current).toMatch(/crossing one is never a\s+finding by itself/); + expect(current).toContain('Reuse-search proof'); + expect(current).toContain('Discoverability without catalog burden'); + expect(current).toContain('closed tracker items, and merged changes'); + expect(current).toContain('zero to three high-confidence findings'); + expect(current).toMatch(/highest\s+practical public boundary/); + expect(current).not.toContain('PortOS'); + expect(current).not.toContain('server/lib/README.md'); + expect(current).not.toContain('client/src/lib/README.md'); + expect(current).not.toMatch(/localhost|:\d{4}/); + }); + it('code-quality v3 inventories structural drift while preserving v2', () => { const current = DEFAULT_TASK_PROMPTS['code-quality']; const previous = PREVIOUS_DEFAULT_PROMPTS['code-quality'][0]; diff --git a/server/services/taskPromptDefaults/integrity.snapshot.json b/server/services/taskPromptDefaults/integrity.snapshot.json index 640508f763..c34d097551 100644 --- a/server/services/taskPromptDefaults/integrity.snapshot.json +++ b/server/services/taskPromptDefaults/integrity.snapshot.json @@ -13,6 +13,7 @@ "ux": "e03d03fc7c16faa5b5db93be2896b9ad", "data-safety": "86b07f51abdbcbe7016f840b5bb0cd9b", "simplify": "2a048214d1dd3b75bee9ae1a38fa1589", + "module-hygiene": "3b7671cd44f719d4b2116b96a279e6d0", "api-contract": "cf02b10a5993baf473e0d320771f26b6", "react-lifecycle": "14b3816d1bcc10f0d7d6357e55cc5e69", "observability": "fdb4d77df5891cfb653a5da2553ecf8a", @@ -74,6 +75,7 @@ "ux": 1, "data-safety": 1, "simplify": 1, + "module-hygiene": 1, "api-contract": 1, "react-lifecycle": 1, "observability": 1, diff --git a/server/services/taskPromptDefaults/prompts.js b/server/services/taskPromptDefaults/prompts.js index b82b6e7c09..f412c44dd4 100644 --- a/server/services/taskPromptDefaults/prompts.js +++ b/server/services/taskPromptDefaults/prompts.js @@ -555,6 +555,118 @@ Cross-version and cross-install compatibility code is NOT dead code, even when this install no longer hits it. Read the project's rules on migrations and version gates before proposing any such removal.`, + 'module-hygiene': `[Improvement: {appName}] Module hygiene audit + +Make {appName} easier to extend by improving responsibility boundaries, reuse, +ownership, and discovery of reusable code. A large codebase is not itself a +problem; the target is code whose organization makes correct changes harder. + +Repository: {repoPath} + +{modeInstructions} + +## Choose one bounded slice + +Start with a cheap repository-wide inventory, then audit one coherent slice. +Rank candidates using several signals together: size, recent churn, import +fan-in, mixed responsibilities, and whether prior audit history already covered +the area. Prefer a previously uncovered slice over repeatedly scanning the same +hotspot. Name the chosen slice before investigating it. + +These numeric thresholds generate candidates only; crossing one is never a +finding by itself: + +- cyclomatic complexity above 15 +- a function body above 50 lines +- nesting depth above 4 +- a file above 500 lines that appears to mix responsibilities + +For declarative UI, schemas, registries, and configuration, distinguish data or +markup volume from branching, state, side effects, and change coupling. + +## Prove structural maintenance cost + +Keep a candidate only when the code proves at least one concrete consequence: + +- one change repeatedly touches unrelated responsibilities +- high-churn behavior is concentrated behind an unstable or oversized boundary +- callers depend on internal details because ownership is unclear +- duplicated behavior has already drifted or makes fixes repeat across copies +- reusable code exists but agents or contributors cannot reliably discover it +- a public surface has no clear owner, placement rule, or compatibility policy + +Read the producer, consumers/importers, tests, repository instructions, and +recent history before deciding. A subjective preference for smaller files or a +different folder layout is not a finding. + +## Reuse-search proof + +Before proposing a new helper, hook, service, component, or primitive: + +1. Search the repository's catalogs, README/domain maps, barrels or public + exports, and likely shared directories. +2. Search semantically related terms as well as the proposed symbol name. +3. Inspect existing candidates and their importers to decide whether one should + be extended. +4. Record what was searched, why reuse is or is not appropriate, the intended + public owner, the internal seam, target location, and migration path. + +For duplication, cite both locations and prefer a deletion-oriented +consolidation. Do not propose a wrapper that leaves both implementations alive. + +## Discoverability without catalog burden + +Use the lightest durable discovery mechanism appropriate to the surface: + +- A genuinely reusable/public surface may need a curated catalog with a cheap, + mechanical parity check. +- A broad implementation directory usually needs a lightweight domain map, + placement rule, or ownership note rather than a barrel or exhaustive manual + inventory. +- Pages, routes, and application implementations do not need catalogs merely + because the directory is large. + +A proposed catalog must name its consumer and its parity mechanism. Prefer +clear naming and placement, or a generated index, when a handwritten inventory +would become a second maintenance burden. + +## Exclusions + +Do not file or implement shallow findings against generated, vendored, build, +snapshot, fixture, test, migration, or historical-default sources; primarily +declarative registries or schemas; documented compatibility facades or re-export +barrels; intentional cross-runtime mirrors; semantic adapters; or coherent large +modules with no proven change cost. Compatibility code is not dead code merely +because the current checkout no longer exercises it. + +## Ownership and prior-work deduplication + +This audit owns responsibility boundaries, module topology, reusable-surface +discoverability, and complexity caused by structural mixing. Pure dead-code +removal and direct copy-paste deletion belong to the repository's simplification +work; broader correctness and registry/generated-source drift belong to its +general code-quality work. Link or reuse a sibling finding instead of filing the +same work under a new category. + +Search current open work, closed tracker items, and merged changes by file, +symbol, and behavior—not only by this audit's name. Treat a previously shipped +split, compatibility facade, or deliberate adapter as history to understand, +not as proof the same refactor should be filed again. + +## Result quality + +Normally produce zero to three high-confidence findings; five is the hard mode +contract maximum, not a quota. Each finding must include exact file:line +evidence, maintenance impact, relevant producers/consumers/tests/history, +reuse-search and prior-work evidence, a decided destination and what remains +behind, compatibility obligations, and acceptance criteria at the highest +practical public boundary. + +If the evidence is insufficient, file or change nothing. In every outcome, +report the audited slice, the searches performed, the findings kept, and the +candidates deliberately rejected so a no-finding run still records useful +coverage.`, + 'api-contract': `[Improvement: {appName}] API and route-contract audit Audit {appName}'s API endpoints and route handlers for contract drift, diff --git a/server/services/taskPromptDefaults/versions.js b/server/services/taskPromptDefaults/versions.js index 795a740276..bfbb963ff7 100644 --- a/server/services/taskPromptDefaults/versions.js +++ b/server/services/taskPromptDefaults/versions.js @@ -45,6 +45,7 @@ export const PROMPT_VERSIONS = { 'ux': 1, // v1: walk the running UI with Playwright MCP against a 7-item named UX checklist and file ONE tracker item per finding via {trackerInstructions} — read-only on source, no branches/PRs. Mode (file-issues vs implement) is injected at dispatch via {modeInstructions} / applyAuditModeWrapper — no prompt bump required. 'data-safety': 1, // v1: data/upgrade-safety audit (migrations, schema parity, version gates). Mode injected at dispatch. 'simplify': 1, // v1: dead-code/duplication audit. Mode injected at dispatch. + 'module-hygiene': 1, // v1: evidence-led complexity, responsibility, reuse, and reusable-surface discoverability audit. Mode injected at dispatch. 'api-contract': 1, // v1: API/route-contract audit (validation gaps, client/server drift, status envelopes, asyncHandler). Net-new type (no PREVIOUS_DEFAULT_PROMPTS entry needed). Mode injected at dispatch. 'react-lifecycle': 1, // v1: React lifecycle/state audit (effect teardowns, stale closures, unmounted setState, derived state). Net-new type. Mode injected at dispatch. 'observability': 1, // v1: logging/observability audit (silent catches, log noise, missing error context, uninstrumented pipelines). Net-new type. Mode injected at dispatch. diff --git a/server/services/taskSchedule.js b/server/services/taskSchedule.js index aead9468d1..a9e4de115f 100644 --- a/server/services/taskSchedule.js +++ b/server/services/taskSchedule.js @@ -28,7 +28,7 @@ import { loadState, isImprovementEnabled } from './cosState.js'; import { getLocalParts } from '../lib/timezone.js'; import { getUserTimezone } from './userTimezone.js'; import { parseCronToNextRun, parseCronToPrevRun } from './eventScheduler.js'; -import { isAuditTaskType, defaultFileIssuesFor } from '../lib/auditCatalog.js'; +import { isAuditTaskType, defaultFileIssuesFor, auditDoWorkRequiresWorktree } from '../lib/auditCatalog.js'; import { DEFAULT_TASK_PROMPTS } from './taskPromptDefaults.js'; import { DEFAULT_PERPETUAL_RECHECK_MS, @@ -1321,6 +1321,9 @@ export async function getScheduleStatus() { if (isAuditTaskType(taskType)) { taskStatus.fileIssuesCapable = true; taskStatus.defaultFileIssues = defaultFileIssuesFor(taskType); + if (auditDoWorkRequiresWorktree(taskType)) { + taskStatus.doWorkRequiresWorktree = true; + } } // Perpetual tasks park PER-APP (parkPerpetual is called with the appId), so diff --git a/server/services/taskSchedule.test.js b/server/services/taskSchedule.test.js index 3ee0bb90b0..8b60088afb 100644 --- a/server/services/taskSchedule.test.js +++ b/server/services/taskSchedule.test.js @@ -1749,8 +1749,8 @@ describe('taskSchedule', () => { }) describe('audit file-issues types', () => { - it('registers data-safety and simplify as enabled on-demand file-issues audits', () => { - for (const taskType of ['data-safety', 'simplify']) { + it('registers data-safety, simplify, and module-hygiene as enabled on-demand file-issues audits', () => { + for (const taskType of ['data-safety', 'simplify', 'module-hygiene']) { expect(SELF_IMPROVEMENT_TASK_TYPES).toContain(taskType) expect(TASK_TYPE_DESCRIPTIONS[taskType]).toBeTruthy() const cfg = DEFAULT_TASK_INTERVALS[taskType] @@ -1761,6 +1761,10 @@ describe('taskSchedule', () => { expect(cfg.taskMetadata.openPR).toBe(false) expect(MANAGED_AGENT_OPTIONS[taskType]).toBeUndefined() } + expect(DEFAULT_TASK_INTERVALS['module-hygiene'].dataInputs).toEqual([ + 'open-issues', + 'open-pull-requests', + ]) }) it('surfaces fileIssuesCapable on audit types in getScheduleStatus', async () => { @@ -1771,6 +1775,10 @@ describe('taskSchedule', () => { expect(status.tasks['ux'].fileIssuesCapable).toBe(true) expect(status.tasks['ux'].defaultFileIssues).toBe(true) expect(status.tasks['data-safety'].fileIssuesCapable).toBe(true) + expect(status.tasks['module-hygiene'].fileIssuesCapable).toBe(true) + expect(status.tasks['module-hygiene'].defaultFileIssues).toBe(true) + expect(status.tasks['module-hygiene'].doWorkRequiresWorktree).toBe(true) + expect(status.tasks['simplify'].doWorkRequiresWorktree).toBeUndefined() expect(status.tasks['claim-issue'].fileIssuesCapable).toBeUndefined() }) diff --git a/server/services/taskScheduleRegistry.js b/server/services/taskScheduleRegistry.js index 957862e4a2..eece80a97d 100644 --- a/server/services/taskScheduleRegistry.js +++ b/server/services/taskScheduleRegistry.js @@ -43,6 +43,10 @@ export const SELF_IMPROVEMENT_TASK_TYPES = [ // copy-paste drift — distinct from `code-quality` (which is the broader DRY / // long-function / TODO pass). Defaults to file-issues. 'simplify', + // Structural-maintainability audit. Treats complexity thresholds as candidate + // signals, then proves responsibility, reuse, or discoverability impact before + // filing. Direct remediation is isolated in a managed worktree. + 'module-hygiene', // Quota-burn `api-contract-audit` counterpart. Route validation, client/server // drift, status envelopes, and missing `asyncHandler`. Defaults to file-issues. 'api-contract', @@ -357,6 +361,7 @@ export const DEFAULT_TASK_INTERVALS = { // defaults keep manual filing available without opting into scheduling. 'data-safety': { type: INTERVAL_TYPES.ON_DEMAND, enabled: true, providerId: null, model: null, prompt: null, taskMetadata: { fileIssues: true, useWorktree: false, openPR: false } }, 'simplify': { type: INTERVAL_TYPES.ON_DEMAND, enabled: true, providerId: null, model: null, prompt: null, taskMetadata: { fileIssues: true, useWorktree: false, openPR: false } }, + 'module-hygiene': { type: INTERVAL_TYPES.ON_DEMAND, enabled: true, providerId: null, model: null, prompt: null, dataInputs: ['open-issues', 'open-pull-requests'], taskMetadata: { fileIssues: true, useWorktree: false, openPR: false } }, 'api-contract': { type: INTERVAL_TYPES.ON_DEMAND, enabled: true, providerId: null, model: null, prompt: null, taskMetadata: { fileIssues: true, useWorktree: false, openPR: false } }, 'react-lifecycle': { type: INTERVAL_TYPES.ON_DEMAND, enabled: true, providerId: null, model: null, prompt: null, taskMetadata: { fileIssues: true, useWorktree: false, openPR: false } }, 'observability': { type: INTERVAL_TYPES.ON_DEMAND, enabled: true, providerId: null, model: null, prompt: null, taskMetadata: { fileIssues: true, useWorktree: false, openPR: false } }, @@ -522,6 +527,7 @@ export const TASK_TYPE_DESCRIPTIONS = { 'ux': 'UX/design audit — file issues (default) or implement fixes', 'data-safety': 'Data/upgrade-safety audit — file issues (default) or implement fixes', 'simplify': 'Dead-code/duplication audit — file issues (default) or implement removals', + 'module-hygiene': 'Module hygiene — complexity, reuse, ownership, and discoverability; file issues (default) or implement one refactor', 'api-contract': 'API/route-contract audit — file issues (default) or implement fixes', 'react-lifecycle': 'React lifecycle/state audit — file issues (default) or implement fixes', 'observability': 'Logging/observability audit — file issues (default) or implement fixes', diff --git a/server/services/workflow.js b/server/services/workflow.js index 832fa0e25f..295d88cb10 100644 --- a/server/services/workflow.js +++ b/server/services/workflow.js @@ -79,6 +79,7 @@ export const WORKFLOW_STAGES = [ 'ux', 'data-safety', 'simplify', + 'module-hygiene', 'api-contract', 'react-lifecycle', 'observability',