Skip to content

Conversation

@marlon-costa-dc
Copy link

@marlon-costa-dc marlon-costa-dc commented Feb 10, 2026

Summary

This PR introduces native agent-teams orchestration and an optional integration layer for MCB (Memory Context Bus).

Key Features

  • Native Agent Teams: Direct orchestration of sub-agents without external dependencies.
  • Legacy Artifact Support: Restores artifact detection and ingestion pipeline for backward compatibility.
  • Compat Shims: Migration shims for older context formats.
  • Optional MCB Layer:
    • MCB integration is disabled by default.
    • Enabled via mcb.enabled: true in config.
    • Granular control over MCB tools (search, memory, etc.).
    • Zero runtime impact when disabled.

Additional Hooks

  • Wisdom Capture: Captures learning patterns (error corrections, successful approaches) into MCB when enabled.
  • Ambiguity Detector: Detects ambiguous user instructions to clarify intent before execution.

Fixes

  • Addressed review comments from @cubic-dev-ai:
    • Added null/non-object guard for JSON parsing in hooks-config-adapter.ts.
    • Added try/finally block in teammate-tools.ts to ensure task reset happens even if inbox cleanup fails.

Verification

  • All tests passing (including new MCB config gate tests).
  • Typecheck clean.
  • Schema updated.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 10, 2026

All contributors have signed the CLA. Thank you! ✅
Posted by the CLA Assistant Lite bot.

Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 52 files

Confidence score: 3/5

  • Some risk due to two moderate issues that can affect runtime behavior and cleanup reliability, so this isn’t a no-brainer merge.
  • In src/tools/agent-teams/teammate-tools.ts, if clearInbox throws, resetOwnerTasks is skipped, potentially leaving tasks owned by a removed teammate.
  • In src/features/compat-shims/hooks-config-adapter.ts, parsing valid JSON like "null" can cause a TypeError when accessing parsed.disabled_hooks/parsed.hooks without guarding for non-object/null.
  • Pay close attention to src/tools/agent-teams/teammate-tools.ts and src/features/compat-shims/hooks-config-adapter.ts - error handling around cleanup and JSON parsing assumptions.
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/features/compat-shims/hooks-config-adapter.ts">

<violation number="1" location="src/features/compat-shims/hooks-config-adapter.ts:38">
P2: Valid JSON like "null" parses to null, but the code assumes an object and immediately accesses parsed.disabled_hooks/parsed.hooks, which will throw a TypeError at runtime. Add a guard for non-object/null parsed values before calling normalizeRawHooks.</violation>
</file>

<file name="src/tools/agent-teams/teammate-tools.ts">

<violation number="1" location="src/tools/agent-teams/teammate-tools.ts:53">
P2: resetOwnerTasks is skipped if clearInbox throws, which can leave tasks owned by a removed teammate. Wrap the inbox cleanup so task reset still runs even when clearInbox fails.</violation>
</file>

Since this is your first cubic review, here's how it works:

  • cubic automatically reviews your code and comments on bugs and improvements
  • Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
  • Add one-off context when rerunning by tagging @cubic-dev-ai with guidance or docs links (including llms.txt)
  • Ask questions if you need clarification on any suggestion

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

@marlon-costa-dc
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

1 similar comment
@marlon-costa-dc
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

@marlon-costa-dc marlon-costa-dc changed the title feat: native agent teams + MCB integration + legacy artifact detection feat: native agent-teams & optional MCB integration Feb 10, 2026
@marlon-costa-dc
Copy link
Author

I've addressed the review comments from @cubic-dev-ai in commit 6e2a9b7:

  • : Added null/non-object guard for JSON parsing.
  • : Wrapped inbox cleanup in try/finally to ensure task reset happens even if inbox cleanup fails.

Additionally, I've implemented the requested optional MCB layer (commit 1392f59):

  • MCB integration is now strictly config-gated via .
  • Zero runtime impact when disabled (default).
  • Config schema updated and validated.

@marlon-costa-dc
Copy link
Author

Correction:
I've addressed the review comments from @cubic-dev-ai in commit 6e2a9b7:

  • hooks-config-adapter.ts: Added null/non-object guard for JSON parsing.
  • teammate-tools.ts: Wrapped inbox cleanup in try/finally to ensure task reset happens even if inbox cleanup fails.

Additionally, I've implemented the requested optional MCB layer (commit 1392f59):

  • MCB integration is now strictly config-gated via mcb.enabled: true.
  • Zero runtime impact when disabled (default).
  • Config schema updated and validated.

@marlon-costa-dc marlon-costa-dc force-pushed the feat/native-agent-teams-mcb-integration branch 4 times, most recently from 8acf006 to 192cbf4 Compare February 10, 2026 18:49
@marlon-costa-dc
Copy link
Author

recheck

github-actions bot added a commit that referenced this pull request Feb 10, 2026
- McbAvailabilityStatus with per-tool tracking and TTL cache
- withMcbFallback<T>() wrapper: try operation → degraded on failure
- Auto-marks tools unavailable on failure, short-circuits repeat calls
- 9 BDD tests (availability + graceful-wrapper)
- hooks-config-adapter: reject null/array/primitive JSON with warning
- teammate-tools: wrap clearInbox in try/finally so resetOwnerTasks always runs
- Add MCB optional layer config schema (src/config/schema/mcb.ts)
- Add config gate logic to disable MCB by default (src/features/mcb-integration/config-gate.ts)
- Add 'mcb' config field to OhMyOpenCodeConfigSchema
- Wire initialization in src/index.ts to lock MCB availability based on config
- Add wisdom-capture hook (uses MCB fallback) and ambiguity-detector hook
- Fix type errors in wisdom-capture tests
@marlon-costa-dc marlon-costa-dc force-pushed the feat/native-agent-teams-mcb-integration branch from 192cbf4 to 9c1471c Compare February 10, 2026 18:53
@marlon-costa-dc
Copy link
Author

recheck

@marlon-costa-dc marlon-costa-dc marked this pull request as draft February 11, 2026 16:15
Copy link

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 20 files (changes from recent commits).

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=".gitignore">

<violation number="1" location=".gitignore:42">
P2: The global ignore pattern `mcb.*` is overly broad and matches existing source files (e.g., `src/config/schema/mcb.ts`), which can cause new MCB-related files to be silently ignored by Git.</violation>
</file>

<file name="src/tools/agent-teams/send-message-tool.ts">

<violation number="1" location="src/tools/agent-teams/send-message-tool.ts:183">
P2: Plan approval responses are delivered to the inbox but never resume the recipient teammate, unlike other message types. This can leave agents waiting for approval without being woken to process the response.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

…y hooks

- Register oc-mcb builtin skill via SkillMcpManager (mcb serve stdio)
- Add command, args, env, data_dir fields to McbConfigSchema (additive)
- Create session-lifecycle handler: resetWarningState + fire-and-forget recovery
- Wire handleMcbSessionCreated in event.ts on session.created
- Add 20 integration tests (skill registration, config, availability, real binary)
- Update docs to reflect stdio-first architecture and recovery mechanism
…ate on plan approval

- Change mcb.* to /mcb.* in .gitignore to avoid ignoring source files like src/config/schema/mcb.ts
- Add resumeTeammateWithMessage() call after plan_approved and plan_rejected paths in send-message-tool.ts
- Fixes PR review comments code-yeongyu#3 and code-yeongyu#4 from cubic-dev-ai
@marlon-costa-dc
Copy link
Author

All 4 review comments from cubic-dev-ai have been addressed:

# File Issue Fix
1 hooks-config-adapter.ts:38 null guard for JSON parse ✅ Fixed in ac3cac6
2 teammate-tools.ts:53 clearInbox/resetOwnerTasks try/finally ✅ Fixed in 6e2a9b7
3 .gitignore:42 mcb.* pattern too broad — matches source files ✅ Fixed in 5c4e4f6 — changed to /mcb.* (root-anchored)
4 send-message-tool.ts:183 Plan approval doesn't resume teammate ✅ Fixed in 5c4e4f6 — added resumeTeammateWithMessage() after both plan_approved and plan_rejected paths

All fixes verified: typecheck clean, 59 agent-teams tests passing, 0 failures.

- add MCP stdio client helper and default argument builders for strict MCB schemas
- add tiered E2E suites that exercise real mcb serve tool listing and core tool calls
- align mcb integration types and exports with discovered v0.2.1-dev tool contracts
@marlon-costa-dc
Copy link
Author

MCB Roundtrip Evidence (v0.2.1-dev)

Pushed commit 2cad9302 with real write/read E2E roundtrip coverage.

Write/Read operations verified in tests

Flow Write operation Read operation Verification
Session roundtrip session create session get + session list Created session_id appears in get/list payload
Index/search roundtrip index start over temp dir (calculator.ts with calculateTotalRoundtripMarker) search (resource: code) Search returns marker or non-zero results
Memory known bug memory store (resource: observation) N/A Confirmed isError: true + internal error

Command evidence

  • bun test src/features/mcb-integration/e2e-roundtrip-session.test.ts2 pass, 0 fail
  • bun test src/features/mcb-integration/e2e-roundtrip-index.test.ts1 pass, 0 fail
  • bun test src/features/mcb-integration/80 pass, 0 fail
  • bun run typecheckpass
  • bun run buildpass
  • PATH=\"/usr/bin:/bin\" bun test src/features/mcb-integration/e2e-roundtrip-*.test.ts0 pass, 7 skip, 0 fail (graceful skip without mcb)
  • pgrep -af \"mcb serve\" → no orphan mcb serve process

Notes

  • PR is still DRAFT (unchanged), base branch dev.
  • Full repo bun test currently reports existing unrelated failures outside this change set; MCB integration suite is green.

- Add test-mcb.toml: local-only providers (fastembed, edgevec, Moka, tokio)
- mcb-client-helper: support --config path and env vars for StdioClientTransport
- mcb-roundtrip-helpers: use all 11 valid MCB AgentType enum values
- e2e-roundtrip-session: 3 tests with inline bun:sqlite DB verification
  - session create → agent_sessions table (FAILS: MCB persistence bug)
  - session get → cross-reference DB state (FAILS: no session ID returned)
  - memory store → observations table (PASSES: MCB persists observations)
- e2e-roundtrip-index: 2 tests with inline bun:sqlite DB verification
  - index start → collections/file_hashes tables (FAILS: 0 rows persisted)
  - search → cross-reference file_hashes (FAILS: 0 rows persisted)
- Remove all silent if(hasTable) guards: tests FAIL explicitly per constraint
- All DB paths isolated via MCP__AUTH__USER_DB_PATH to /tmp/ with cleanup
…ct verification

Memory test now verifies: MCB response contains observation_id, row exists
with exact content='e2e-memory-test', observation_type='code', and
project_id='test-project' — not just row count > 0.
…ssion_summary_id

- tier2 memory: update error assertion (MCB now returns proper validation
  'Project ID cannot be empty' instead of generic 'internal error')
- tier2 validate: update to expect 'domain_crate' config error (MCB bug:
  list_rules should not need project config variables)
- session create: add session_summary_id + model to data payload per
  MCB create.rs requirements (still fails: FK constraint on session_summaries)
- tier2: 7/7 pass (was 5/7)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant