Repository navigation
feat: allow AIO and Logs in default integrations - #1235
Conversation
🧙 Wizard CIRun the Wizard CI and test your changes against wizard-workbench example apps by replying with a GitHub comment using one of the following commands: Test all apps:
Test all apps in a directory:
Test an individual app:
Show more apps
Test against a Context Mill branch:
Add Results will be posted here when complete. |
|
merge this first before PostHog/context-mill#390 |
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 1 · PR risk: 0/10 |
| excludedTaskTypes: (flags) => | ||
| flags[WIZARD_DEFAULT_AIO_LOGS_FLAG_KEY] === 'false' | ||
| ? ['ai-observability', 'logs'] | ||
| : [], |
There was a problem hiding this comment.
we can make more generic, but we can have a list of special tasks that can be included or excluded by the planner agent using flags
There was a problem hiding this comment.
we'll come across this another day
| function excludedTypesNote(types: readonly string[]): string | null { | ||
| if (types.length === 0) return null; | ||
| return `The plan below may mention these task types, but this run excludes them: ${types.join( | ||
| ', ', | ||
| )}. enqueue_task will not accept them — do not queue them, do not retry them, and hang nothing off them.`; | ||
| } |
There was a problem hiding this comment.
if excluding tasks from planner agent, include this prompt
There was a problem hiding this comment.
Mannnn I can't wait for Jev. Jev would be the perfect enqueue agent and prompt assembler
| function excludedTypesNote(types: readonly string[]): string | null { | ||
| if (types.length === 0) return null; | ||
| return `The plan below may mention these task types, but this run excludes them: ${types.join( | ||
| ', ', | ||
| )}. enqueue_task will not accept them — do not queue them, do not retry them, and hang nothing off them.`; | ||
| } |
There was a problem hiding this comment.
Mannnn I can't wait for Jev. Jev would be the perfect enqueue agent and prompt assembler
| excludedTaskTypes: (flags) => | ||
| flags[WIZARD_DEFAULT_AIO_LOGS_FLAG_KEY] === 'false' | ||
| ? ['ai-observability', 'logs'] | ||
| : [], |
There was a problem hiding this comment.
we'll come across this another day
|
Wait for my agent to finish as wellll |
| flags[WIZARD_DEFAULT_AIO_LOGS_FLAG_KEY] === 'false' | ||
| ? ['ai-observability', 'logs'] | ||
| : [], | ||
|
|
There was a problem hiding this comment.
Here's the potential issue: The kill switch only controls orchestrated runs, so it doesn't disable integrations in the default setup path.
flag set to false -> user starts default PostHog setup -> prompt still requests AI Observability and Logs -> those integrations can still be installed
Suggested fix: Apply the flag when building default setup instructions, so all supported paths respect the same exclusions.
| ]; | ||
| const registry = await loadAgentRegistry(boot.skillsBaseUrl, flow, { | ||
| exclude: ciExcludedTaskTypes(), | ||
| exclude: excludedTaskTypes, |
There was a problem hiding this comment.
Here's the potential issue: The tests don't exercise the orchestration that applies this kill switch, so a future change can disconnect it without a failing test.
flow contains AI Observability and Logs -> flag hookup is removed -> flag set to false leaves both tasks available -> existing tests still pass
Suggested fix: Test an orchestrated flow containing both tasks with the flag off and at its default setting, checking the available tasks and planner instructions.
| } Could not load skill menu and halt. | ||
|
|
||
| Choose a skill from the \`integration\` category that matches this project's framework. Do NOT pick skills from other categories (llm-analytics, error-tracking, feature-flags, omnibus, etc.) — those are handled separately. | ||
| Choose a skill from the \`integration\` category that matches this project's framework. Start with this framework skill; load the AI Observability and Logs skills when its workflow calls for them, before verification and the setup report. Both are included by default where applicable; the skills define applicability and how to report skipped work. These three categories — \`integration\`, \`ai-observability\`, \`logs\` — are the only ones this run uses. Do NOT load or install skills from any other category (llm-analytics, error-tracking, feature-flags, audit, etc.) — those are handled separately. In particular, \`ai-observability\` is the category for AI Observability; do not substitute \`llm-analytics\`. |
There was a problem hiding this comment.
Here's the potential issue: The instructions don't explain how to complete multiple selected skills, so the agent must infer which work to run and in what order.
agent selects framework integration, AI Observability and Logs -> applies the singular install step to the framework skill -> the other integrations may be left uninstalled
Suggested fix: Repeat the workflow for every selected skill, explicitly completing framework integration first, then AI Observability, then Logs.
| Choose a skill from the \`integration\` category that matches this project's framework. Do NOT pick skills from other categories (llm-analytics, error-tracking, feature-flags, omnibus, etc.) — those are handled separately. | ||
| Choose a skill from the \`integration\` category that matches this project's framework. Start with this framework skill; load the AI Observability and Logs skills when its workflow calls for them, before verification and the setup report. Both are included by default where applicable; the skills define applicability and how to report skipped work. These three categories — \`integration\`, \`ai-observability\`, \`logs\` — are the only ones this run uses. Do NOT load or install skills from any other category (llm-analytics, error-tracking, feature-flags, audit, etc.) — those are handled separately. In particular, \`ai-observability\` is the category for AI Observability; do not substitute \`llm-analytics\`. | ||
| If no suitable integration skill is found, emit: ${ | ||
| AgentSignals.ERROR_RESOURCE_MISSING |
There was a problem hiding this comment.
Here's the potential issue: The expanded setup still has the old tool-call budget, so longer runs may stop before reporting what succeeded. Combined-run usage hasn't been measured.
combined setup needs a 251st tool call before final reporting -> the 250-call guard stops execution -> the user can be left with partial integration and no report
Suggested fix: Measure combined runs on real projects, then increase the budget if needed or reserve enough capacity for the report.
| inputs: args.inputs ?? {}, | ||
| dependsOn: args.dependsOn ?? [], | ||
| model: args.model, | ||
| optional: (ctx.optionalTypes ?? []).includes(args.type) || undefined, |
There was a problem hiding this comment.
Here's the potential issue: Optional work can become a prerequisite for other tasks, which undermines the guarantee that its failure is safe to ignore.
planner makes verification depend on optional AI Observability -> AI Observability exhausts its attempts -> verification runs without those results -> the run can still report success
Suggested fix: Apply the existing dependency restriction to all optional tasks, including tasks added by the planner.
| return `The plan below may mention these task types, but this run excludes them: ${types.join( | ||
| ', ', | ||
| )}. enqueue_task will not accept them — do not queue them, do not retry them, and hang nothing off them.`; | ||
| } |
There was a problem hiding this comment.
Here's the potential issue: The prompt tells the planner that task types were excluded even when they weren’t available in this flow. Overlapping exclusions can also list the same type twice.
flag excludes logs -> this flow has no logs task to remove -> prompt still tells the planner logs was excluded -> planner may describe work as skipped that was never available
Suggested fix: Build the note from task types actually removed from the flow, and list each type once.
| type: t.type, | ||
| status: t.status, | ||
| optional: t.optional === true, | ||
| })) satisfies TaskOutcome[]; |
There was a problem hiding this comment.
Here's the potential issue: Tests now depend on production bookkeeping that has no runtime consumer, so maintaining test assertions requires changes to real runs.
a test needs an extra detail about a failed task -> the production runner must record that test-only detail -> every real run carries the extra bookkeeping
Suggested fix: Have the test harness capture outcomes from the queue state before it is cleared, instead of adding test-only data to production runs.
| session: Pick<WizardSession, 'frameworkContext'>, | ||
| ): TaskOutcome[] { | ||
| const raw = session.frameworkContext[TASK_OUTCOMES_KEY]; | ||
| return Array.isArray(raw) ? (raw as TaskOutcome[]) : []; |
There was a problem hiding this comment.
Here's the potential issue: The tests cover reading outcome data but never prove that a real run writes it, so recording failures can pass unnoticed.
AI Observability runs but its outcome is not recorded -> the reader returns an empty result -> this looks like a run with no queued tasks -> tests using manually supplied outcomes still pass
Suggested fix: Test a real queued run through outcome recording, and distinguish missing data from a completed run with no tasks.
| * and no agent boots for it. The program owns the flag→type mapping; the | ||
| * runner only applies it. | ||
| */ | ||
| excludedTaskTypes?: (flags: Record<string, string>) => readonly string[]; |
There was a problem hiding this comment.
Here's the potential issue: The PR summary understates the behavior changes, so a reviewer could approve the prompt edits without examining the new failure semantics.
reviewer checks that the prompt requests all three integrations -> summary gives no reason to inspect optional-task failures -> changed success behavior can pass without review
Suggested fix: Describe the changes to task exclusions, optional-task failures and outcome reporting, including what happens when an optional integration fails.
| 'Add a task to the orchestrator queue. Use it to seed work and to enqueue follow-up work you discover. Keep tasks small and discrete.', | ||
| promptSnippet: | ||
| 'enqueue_task(type, label, dependsOn, reason) — add a task to the queue; returns its id', | ||
| 'enqueue_task({type, label, dependsOn, reason}) — add a task to the queue; the task type goes in the "type" field, and it returns the task id', |
There was a problem hiding this comment.
Here's the potential issue: The PR summary omits a shared tool-instruction change, so reviewers can miss that workflows beyond default setup are affected.
reviewer focuses on adding AI Observability and Logs -> shared guidance also changes to queue tasks using named fields -> every orchestrated flow receives that change without its scope being explained
Suggested fix: Explain the shared instruction change and its intended effect across flows, or split it into a separate commit for review.
|
👀 👀 👀 👀 👀 |
Brings in main (#1235, 2.77.0) through A2b. One conflict: the legacy adapter takes A3's @agent and @agent/types imports and adds TASK_OUTCOMES_KEY. The key and its TaskOutcome type join the public entry in the B2 group, and the e2e harness reads them from there instead of the orchestrator's queue. Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
Carries main's #1319 and #1235 through the callable host: - runProgram loads wizard flags before it resolves the run, so the integration prompt reads the AI Observability and Logs kill switch. artifacts.reportFile is now set just before the agent starts, so a pre-run failure leaves it absent. - One excludedIntegrationTaskTypes feeds both program registries. - The legacy adapter records task outcomes for the e2e harness, and TASK_OUTCOMES_KEY lives in runner/shared/types to stay out of the startup closure. Generated-By: PostHog Desktop Task-Id: d14e92bb-6ee1-49b5-8502-39cb80079589
3 participants
Problem
Default runs need to load AIO and Logs alongside the framework skill, but the current prompt restricts skill loading.
Changes
Allow the integration workflow to load both product skills before review and reporting. Allow one menu lookup per needed category.
Companion: PostHog/context-mill#390 adds the tasks and instructions. Release this Wizard change first.
Test plan
Build and 2,680 tests pass. Changed files pass lint and formatting. Full lint hits existing research-output errors. No live ingestion test.