feat(octto): add browser-based brainstorming system #11
Conversation
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
The context-injector hook was using snake_case 'file_path' but OpenCode tools use camelCase 'filePath', causing directory context injection to silently fail for all file read/edit operations. - Change input.args?.file_path to input.args?.filePath - Add tests for context injection behavior
Wrap spawn() call in try-catch to provide descriptive error messages when command not found or other spawn failures occur. - Add try-catch around bun-pty spawn() call - Throw descriptive error with command name and original message - Add test for command not found scenario - Note: bun-pty silently ignores invalid workdir (library limitation)
Add src/utils/config.ts with compile-time constants organized by domain: - compaction: threshold, cooldown, timeout settings - contextWindow: warning/critical thresholds - tokens: estimation settings - paths: ledger dirs, context files, patterns - timeouts: btca, toast durations - limits: file sizes, PTY buffer, cache settings Add comprehensive tests in tests/utils/config.test.ts (29 tests)
When a user configures an agent with an unavailable provider/model in micode.json, instead of crashing at runtime, we now: - Validate models at plugin startup against available providers - Log a warning for invalid models - Remove the invalid model from config, letting the agent use its default Handles edge cases: empty strings, malformed models (no slash), and models with multiple slashes in the ID. Closes #10
Update hooks and tools to use the new utility modules: - auto-compact: config.* for thresholds/timeouts, extractErrorMessage - context-injector: config.paths.* and config.limits.* - context-window-monitor: config.contextWindow.* - ledger-loader: config.paths.ledgerDir/ledgerPrefix - token-aware-truncation: config.tokens.* - artifact-auto-index: log.error instead of console.error Part of tech debt remediation (Phase 3).
Integrate octto (interactive browser-based brainstorming) into micode as a separate primary agent alongside the existing brainstormer. Infrastructure (src/octto/): - session/: HTTP server + WebSocket for browser communication - state/: Brainstorm state persistence to thoughts/brainstorms/ - ui/: Self-contained HTML/CSS/JS bundle for browser UI - types.ts: 15+ question type configurations Tools (src/tools/octto/): - brainstorm.ts: create_brainstorm, await_brainstorm_complete, end_brainstorm - questions.ts: 15 question type tools (pick_one, confirm, slider, etc.) - responses.ts: get_answer, get_next_answer, list_questions - session.ts: start_session, end_session - processor.ts: Answer processing with probe agent integration Agents (src/agents/): - octto.ts: Primary orchestrator for brainstorm sessions - bootstrapper.ts: Creates 2-4 exploration branches from request - probe.ts: Evaluates branch Q&A and generates follow-up questions Config: - Added octto section to src/utils/config.ts with timeouts and limits - ID generators moved to octto/session/utils.ts (single source of truth)
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
20 issues found across 53 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/octto/ui/bundle.ts">
<violation number="1" location="src/octto/ui/bundle.ts:733">
P1: Hard-coding `ws://` makes the UI fail to connect when served over HTTPS because browsers block mixed-content WebSocket calls. Derive the protocol from `window.location.protocol` and use `wss://` when the page is loaded via HTTPS.</violation>
<violation number="2" location="src/octto/ui/bundle.ts:1263">
P2: Rate answers are recorded incorrectly: `setRating` marks every star up to the chosen value as `.selected`, but `submitRate` only reads the first `.selected` button, so higher ratings are never captured. Store the chosen value explicitly (e.g., via a data attribute) or select the highest `.selected` element before submitting.</violation>
</file>
<file name="src/octto/state/store.ts">
<violation number="1" location="src/octto/state/store.ts:155">
P1: `deleteSession` bypasses the per-session lock, so concurrent writes can recreate a session immediately after it is deleted.</violation>
</file>
<file name="src/octto/session/sessions.ts">
<violation number="1" location="src/octto/session/sessions.ts:208">
P1: `getAnswer` marks the entire question as TIMEOUT when a single wait call times out, so later user responses are silently dropped because `handleWsMessage` only processes PENDING questions.</violation>
</file>
<file name="src/config-loader.ts">
<violation number="1" location="src/config-loader.ts:122">
P2: `override.model` is assumed to be a string, but malformed configs can make it a non-string, causing `.trim()` to throw during validation.</violation>
</file>
<file name="src/octto/types.ts">
<violation number="1" location="src/octto/types.ts:184">
P2: `ShowPlanConfig.sections` is required even though `markdown` is documented as an alternative; make `sections` optional so callers can supply only `markdown`.</violation>
</file>
<file name="src/octto/constants.ts">
<violation number="1" location="src/octto/constants.ts:11">
P2: `DEFAULT_MAX_QUESTIONS` is hard-coded to 15 instead of being re-exported from `config.octto`, contradicting the stated single source of truth and risking divergence when the config changes.</violation>
</file>
<file name="src/octto/session/waiter.ts">
<violation number="1" location="src/octto/session/waiter.ts:60">
P2: Ensure the waiter is removed even when the callback throws; otherwise a failing waiter stays registered and blocks future notifications for that key.</violation>
<violation number="2" location="src/octto/session/waiter.ts:77">
P2: Wrap the notification loop in a `try/finally` so the waiter list is always cleared even when a callback throws; otherwise the key remains registered forever after an exception.</violation>
</file>
<file name="src/tools/octto/session.ts">
<violation number="1" location="src/tools/octto/session.ts:18">
P2: The `start_session` schema blocks valid octto question types (rank, rate, ask_image, etc.), so sessions containing those questions cannot be started even though the system supports them.</violation>
</file>
<file name="src/tools/milestone-artifact-search.ts">
<violation number="1" location="src/tools/milestone-artifact-search.ts:20">
P1: `searchMilestoneArtifacts` is never implemented on `ArtifactIndex`, so this tool throws before returning any results.</violation>
</file>
<file name="src/agents/octto.ts">
<violation number="1" location="src/agents/octto.ts:43">
P2: Design documents are instructed to be written to `thoughts/shared/plans/...` even though brainstorming outputs must live under `thoughts/shared/designs/`, so this prompt would misfile every design artifact.</violation>
</file>
<file name="src/octto/session/server.ts">
<violation number="1" location="src/octto/session/server.ts:19">
P2: Bind the brainstorming server to the loopback interface (e.g., hostname 127.0.0.1) so brainstorming sessions are not exposed on all network interfaces.</violation>
</file>
<file name="thoughts/shared/designs/2026-01-16-milestone-artifact-indexing-design.md">
<violation number="1" location="thoughts/shared/designs/2026-01-16-milestone-artifact-indexing-design.md:34">
P3: The metadata section is duplicated (“Metadata Fields” and “Metadata Fields (Explicit)”) with identical content, which makes the design doc harder to read and suggests there might be a missing distinction.</violation>
</file>
<file name="src/index.ts">
<violation number="1" location="src/index.ts:117">
P2: `octtoSessionsMap` is never populated, so the new session-deletion cleanup never ends octto sessions, leading to leaked browser/WebSocket sessions for each user session.</violation>
</file>
<file name="src/tools/octto/formatters.ts">
<violation number="1" location="src/tools/octto/formatters.ts:9">
P2: `escapeXml` does not escape quotes, so attribute values containing `"` or `'` will break the generated XML and allow injection. Extend the helper to encode quotes before reusing it for attributes.</violation>
</file>
<file name="src/tools/octto/questions.ts">
<violation number="1" location="src/tools/octto/questions.ts:122">
P2: rate tool omits the `labels` configuration defined in `RateConfig`, preventing callers from supplying the min/max labels the UI supports.</violation>
<violation number="2" location="src/tools/octto/questions.ts:193">
P2: ask_image ignores the `accept` whitelist from `AskImageConfig`, so callers cannot restrict allowed image types and the frontend can’t enforce MIME/extension safety.</violation>
<violation number="3" location="src/tools/octto/questions.ts:451">
P2: slider tool never accepts the `labels` object defined in `SliderConfig`, so callers can’t provide min/mid/max labels and the UI cannot display them.</violation>
</file>
<file name="tests/utils/logger.test.ts">
<violation number="1" location="tests/utils/logger.test.ts:72">
P2: `require` is not available in this ESM test file, so deleting `require.cache[...]` will throw and break the debug test. Use an ESM-friendly cache-busting approach instead (e.g., dynamic import with a query string).</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| </div> | ||
|
|
||
| <script> | ||
| const wsUrl = 'ws://' + window.location.host + '/ws'; |
There was a problem hiding this comment.
P1: Hard-coding ws:// makes the UI fail to connect when served over HTTPS because browsers block mixed-content WebSocket calls. Derive the protocol from window.location.protocol and use wss:// when the page is loaded via HTTPS.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/octto/ui/bundle.ts, line 733:
<comment>Hard-coding `ws://` makes the UI fail to connect when served over HTTPS because browsers block mixed-content WebSocket calls. Derive the protocol from `window.location.protocol` and use `wss://` when the page is loaded via HTTPS.</comment>
<file context>
@@ -0,0 +1,1599 @@
+ </div>
+
+ <script>
+ const wsUrl = 'ws://' + window.location.host + '/ws';
+ let ws = null;
+ let questions = [];
</file context>
Suggested change
| const wsUrl = 'ws://' + window.location.host + '/ws'; | |
| const wsProtocol = window.location.protocol === 'https:' ? 'wss://' : 'ws://'; | |
| const wsUrl = wsProtocol + window.location.host + '/ws'; |
Comment on lines
+155
to
+157
| async deleteSession(sessionId: string): Promise<void> { | ||
| await persistence.delete(sessionId); | ||
| }, |
There was a problem hiding this comment.
P1: deleteSession bypasses the per-session lock, so concurrent writes can recreate a session immediately after it is deleted.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/octto/state/store.ts, line 155:
<comment>`deleteSession` bypasses the per-session lock, so concurrent writes can recreate a session immediately after it is deleted.</comment>
<file context>
@@ -0,0 +1,159 @@
+ return Object.values(state.branches).every((b) => b.status === BRANCH_STATUSES.DONE);
+ },
+
+ async deleteSession(sessionId: string): Promise<void> {
+ await persistence.delete(sessionId);
+ },
</file context>
Suggested change
| async deleteSession(sessionId: string): Promise<void> { | |
| await persistence.delete(sessionId); | |
| }, | |
| async deleteSession(sessionId: string): Promise<void> { | |
| return withSessionLock(sessionId, async () => { | |
| await persistence.delete(sessionId); | |
| }); | |
| }, |
|
|
||
| timeoutId = setTimeout(() => { | ||
| cleanup(); | ||
| question.status = STATUSES.TIMEOUT; |
There was a problem hiding this comment.
P1: getAnswer marks the entire question as TIMEOUT when a single wait call times out, so later user responses are silently dropped because handleWsMessage only processes PENDING questions.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/octto/session/sessions.ts, line 208:
<comment>`getAnswer` marks the entire question as TIMEOUT when a single wait call times out, so later user responses are silently dropped because `handleWsMessage` only processes PENDING questions.</comment>
<file context>
@@ -0,0 +1,383 @@
+
+ timeoutId = setTimeout(() => {
+ cleanup();
+ question.status = STATUSES.TIMEOUT;
+ resolve({ completed: false, status: STATUSES.TIMEOUT, reason: STATUSES.TIMEOUT });
+ }, timeout);
</file context>
| execute: async (args) => { | ||
| try { | ||
| const index = await getArtifactIndex(); | ||
| const results = await index.searchMilestoneArtifacts(args.query, { |
There was a problem hiding this comment.
P1: searchMilestoneArtifacts is never implemented on ArtifactIndex, so this tool throws before returning any results.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tools/milestone-artifact-search.ts, line 20:
<comment>`searchMilestoneArtifacts` is never implemented on `ArtifactIndex`, so this tool throws before returning any results.</comment>
<file context>
@@ -0,0 +1,48 @@
+ execute: async (args) => {
+ try {
+ const index = await getArtifactIndex();
+ const results = await index.searchMilestoneArtifacts(args.query, {
+ milestoneId: args.milestone_id,
+ artifactType: args.artifact_type,
</file context>
| const ratings = {}; | ||
| for (const opt of (q.config.options || [])) { | ||
| const container = document.getElementById('rate_' + questionId + '_' + opt.id); | ||
| const selected = container.querySelector('.rate-star.selected'); |
There was a problem hiding this comment.
P2: Rate answers are recorded incorrectly: setRating marks every star up to the chosen value as .selected, but submitRate only reads the first .selected button, so higher ratings are never captured. Store the chosen value explicitly (e.g., via a data attribute) or select the highest .selected element before submitting.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/octto/ui/bundle.ts, line 1263:
<comment>Rate answers are recorded incorrectly: `setRating` marks every star up to the chosen value as `.selected`, but `submitRate` only reads the first `.selected` button, so higher ratings are never captured. Store the chosen value explicitly (e.g., via a data attribute) or select the highest `.selected` element before submitting.</comment>
<file context>
@@ -0,0 +1,1599 @@
+ const ratings = {};
+ for (const opt of (q.config.options || [])) {
+ const container = document.getElementById('rate_' + questionId + '_' + opt.id);
+ const selected = container.querySelector('.rate-star.selected');
+ if (selected) {
+ ratings[opt.id] = parseInt(selected.dataset.value);
</file context>
| }), | ||
| }); | ||
|
|
||
| interface SliderConfig { |
There was a problem hiding this comment.
P2: slider tool never accepts the labels object defined in SliderConfig, so callers can’t provide min/mid/max labels and the UI cannot display them.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tools/octto/questions.ts, line 451:
<comment>slider tool never accepts the `labels` object defined in `SliderConfig`, so callers can’t provide min/mid/max labels and the UI cannot display them.</comment>
<file context>
@@ -0,0 +1,488 @@
+ }),
+ });
+
+ interface SliderConfig {
+ session_id: string;
+ question: string;
</file context>
| options: optionsSchema.describe("Items to rate"), | ||
| min: tool.schema.number().optional().describe("Minimum rating value (default: 1)"), | ||
| max: tool.schema.number().optional().describe("Maximum rating value (default: 5)"), | ||
| step: tool.schema.number().optional().describe("Rating step (default: 1)"), |
There was a problem hiding this comment.
P2: rate tool omits the labels configuration defined in RateConfig, preventing callers from supplying the min/max labels the UI supports.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tools/octto/questions.ts, line 122:
<comment>rate tool omits the `labels` configuration defined in `RateConfig`, preventing callers from supplying the min/max labels the UI supports.</comment>
<file context>
@@ -0,0 +1,488 @@
+ options: optionsSchema.describe("Items to rate"),
+ min: tool.schema.number().optional().describe("Minimum rating value (default: 1)"),
+ max: tool.schema.number().optional().describe("Maximum rating value (default: 5)"),
+ step: tool.schema.number().optional().describe("Rating step (default: 1)"),
+ },
+ validate: (args) => {
</file context>
| }), | ||
| }); | ||
|
|
||
| interface ImageConfig { |
There was a problem hiding this comment.
P2: ask_image ignores the accept whitelist from AskImageConfig, so callers cannot restrict allowed image types and the frontend can’t enforce MIME/extension safety.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/tools/octto/questions.ts, line 193:
<comment>ask_image ignores the `accept` whitelist from `AskImageConfig`, so callers cannot restrict allowed image types and the frontend can’t enforce MIME/extension safety.</comment>
<file context>
@@ -0,0 +1,488 @@
+ }),
+ });
+
+ interface ImageConfig {
+ session_id: string;
+ question: string;
</file context>
Comment on lines
+72
to
+73
| delete require.cache[require.resolve("../../src/utils/logger")]; | ||
| const { log } = await import("../../src/utils/logger"); |
There was a problem hiding this comment.
P2: require is not available in this ESM test file, so deleting require.cache[...] will throw and break the debug test. Use an ESM-friendly cache-busting approach instead (e.g., dynamic import with a query string).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/utils/logger.test.ts, line 72:
<comment>`require` is not available in this ESM test file, so deleting `require.cache[...]` will throw and break the debug test. Use an ESM-friendly cache-busting approach instead (e.g., dynamic import with a query string).</comment>
<file context>
@@ -0,0 +1,78 @@
+ process.env.DEBUG = "1";
+ // Need fresh import to pick up env change
+ // Clear module cache
+ delete require.cache[require.resolve("../../src/utils/logger")];
+ const { log } = await import("../../src/utils/logger");
+ log.debug("my-module", "Debug info");
</file context>
Suggested change
| delete require.cache[require.resolve("../../src/utils/logger")]; | |
| const { log } = await import("../../src/utils/logger"); | |
| const { log } = await import(`../../src/utils/logger?cacheBust=${Date.now()}`); |
| - created_at | ||
| - tags | ||
|
|
||
| # Metadata Fields (Explicit) |
There was a problem hiding this comment.
P3: The metadata section is duplicated (“Metadata Fields” and “Metadata Fields (Explicit)”) with identical content, which makes the design doc harder to read and suggests there might be a missing distinction.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At thoughts/shared/designs/2026-01-16-milestone-artifact-indexing-design.md, line 34:
<comment>The metadata section is duplicated (“Metadata Fields” and “Metadata Fields (Explicit)”) with identical content, which makes the design doc harder to read and suggests there might be a missing distinction.</comment>
<file context>
@@ -0,0 +1,65 @@
+- created_at
+- tags
+
+# Metadata Fields (Explicit)
+- milestone_id (stored in metadata for filtering and retrieval)
+- artifact_type
</file context>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Integrates octto (interactive browser-based brainstorming) into micode as a separate primary agent alongside the existing brainstormer.
thoughts/brainstorms/Changes
Infrastructure (
src/octto/)session/: HTTP server + WebSocket for browser communicationstate/: Brainstorm state persistenceui/: Self-contained HTML/CSS/JS bundle for browser UItypes.ts: 15+ question type configurationsTools (
src/tools/octto/)brainstorm.ts: create_brainstorm, await_brainstorm_complete, end_brainstormquestions.ts: 15 question type toolsresponses.ts: get_answer, get_next_answer, list_questionssession.ts: start_session, end_sessionprocessor.ts: Answer processing with probe agent integrationAgents (
src/agents/)octto.ts: Primary orchestrator for brainstorm sessionsbootstrapper.ts: Creates 2-4 exploration branches from requestprobe.ts: Evaluates branch Q&A and generates follow-up questionsConfig
octtosection tosrc/utils/config.tswith timeouts and limitsoctto/session/utils.ts(single source of truth)Test plan
bun run buildpassesbun testpasses (200 tests, 1 pre-existing failure unrelated to octto)Summary by cubic
Add octto: a browser-based brainstorming agent with branch-based exploration, interactive Q&A, and state saved to thoughts/brainstorms/. Also adds a milestone artifact search tool plus safer startup through model validation and error handling.
New Features
Bug Fixes
Written for commit 4db82b6. Summary will update on new commits.