feat(plugins): expose activation plan reasons#70943
Conversation
Greptile SummaryThis PR introduces Confidence Score: 5/5Safe to merge — additive API, backward-compatible delegation, and well-covered by tests. The new resolveManifestActivationPlan function is purely additive. The refactored resolveManifestActivationPluginIds delegates cleanly to it and produces an identical sorted result. Logic for each trigger kind mirrors the old boolean checks and the exhaustiveness pattern is preserved. All remaining observations are P2 style suggestions. No files require special attention. Reviews (1): Last reviewed commit: "Merge branch 'main' into feat/plugin-act..." | Re-trigger Greptile |
🔒 Aisle Security AnalysisWe found 1 potential security issue(s) in this PR:
1. 🟡 Potential information disclosure: activation plan returns raw manifest registry diagnostics (includes local file paths and error details)
Description
If the activation plan object is ever surfaced to untrusted contexts (CLI output, UI, logs, API responses), this can leak internal directory layout and other environment details. Vulnerable behavior: return {
trigger: params.trigger,
pluginIds: [...new Set(entries.map((entry) => entry.pluginId))],
entries,
diagnostics: registry.diagnostics,
};Diagnostics can contain paths and detailed error strings, for example: diagnostics.push({
level: "error",
message: manifestRes.error,
source: manifestRes.manifestPath,
});RecommendationAvoid returning raw diagnostics to callers by default. Options:
Example (gated + sanitized): export function resolveManifestActivationPlan(params: ResolveManifestActivationPlanParams & { includeDiagnostics?: boolean }): PluginActivationPlan {
// ...
const diagnostics = params.includeDiagnostics
? registry.diagnostics.map(d => ({
level: d.level,
pluginId: d.pluginId,
// omit/relativize path-like values
source: d.source ? path.basename(d.source) : undefined,
// ensure message does not include absolute paths (apply your redaction rules)
message: redactPaths(d.message),
}))
: [];
return { trigger: params.trigger, pluginIds, entries, diagnostics };
}Also ensure any CLI/UI/API layer treats diagnostics as debug-only and never exposes them to untrusted users/tenants. Analyzed PR: #70943 at commit Last updated on: 2026-04-24T05:09:22Z |
Summary
resolveManifestActivationPlanbeside the existing ids-only activation planner APIactivationis planner metadata, not lifecycle API, and document compatibility/deprecation policyValidation
pnpm test src/plugins/activation-planner.test.tspnpm format:check src/plugins/activation-planner.ts src/plugins/activation-planner.test.ts src/plugins/manifest.ts docs/plugins/manifest.md docs/plugins/architecture.md docs/plugins/architecture-internals.md docs/plugins/sdk-migration.mdpnpm test:contracts:pluginspnpm tsgo:corecheck found no touched-file errorsKnown failures
pnpm check:changedfails insrc/agents/openai-transport-stream.tson existing OpenAI transport type errors (supportsStore,supportsReasoningEffort, etc.)pnpm buildreachesbuild:plugin-sdk:dtsand fails on the same unrelated OpenAI transport type errorsNotes