Preserve Codex subagent thread.started metadata#1
Merged
ratulsarna merged 2 commits intosubagent-support-codexfrom Mar 7, 2026
Merged
Conversation
📝 WalkthroughWalkthroughThe changes extend thread event handling infrastructure to support nested agent thread spawning. New type definitions introduce structured source metadata for thread events, parsing utilities normalize incoming event payloads, and comprehensive tests validate the enhanced functionality across the provider adapter and orchestration layers. Changes
Sequence DiagramsequenceDiagram
participant Codex as Codex Provider
participant Adapter as CodexAdapter
participant Parser as Payload Parser
participant Runtime as Runtime Layer
participant Orchestration as Orchestration
Codex->>Adapter: thread/started event<br/>(raw nested spawn data)
Adapter->>Parser: readThreadStartedPayload(raw event)
Parser->>Parser: normalizeThreadStartedSource()<br/>Validate & enrich metadata
Parser->>Parser: coalesceNullableString()<br/>Merge agent defaults
Parser-->>Adapter: ThreadStartedPayload<br/>(normalized structure)
Adapter->>Runtime: emit thread.started<br/>(structured payload)
Runtime->>Orchestration: thread.started<br/>(with parent/depth/agent info)
Orchestration->>Orchestration: Track nested thread hierarchy
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes The review requires understanding new type definitions, evaluating multiple parsing/normalization utility functions with edge case handling, verifying comprehensive test coverage across varied payload shapes, and confirming correct integration across three layers (contracts, adapter, orchestration). Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Owner
Author
|
@coderabbitai review |
Owner
Author
|
@codex review |
✅ Actions performedReview triggered.
|
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/contracts/src/providerRuntime.test.ts (1)
123-154: Assertpayload.statusin this regression too.
statusis part of the new contract surface, but this test never verifies the decoded value. It still passes if the decoder starts dropping or rewriting that field.Possible assertion
expect(parsed.payload.providerThreadId).toBe("provider-thread-1"); expect(parsed.payload.name).toBeNull(); expect(parsed.payload.preview).toBe(""); + expect(parsed.payload.status).toEqual({ + type: "active", + activeFlags: [], + }); expect(parsed.payload.source).toEqual({ kind: "subAgentThreadSpawn", parentProviderThreadId: "provider-parent-1", depth: 1,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/contracts/src/providerRuntime.test.ts` around lines 123 - 154, The test for decoding "thread.started" doesn't assert the new payload.status field, so regressions can drop or change it; update the test in providerRuntime.test.ts to assert parsed.payload.status equals the expected object (e.g., { type: "active", activeFlags: [] }) after the existing assertions for providerThreadId/name/preview/source, referencing the parsed variable and its payload.status property to ensure the decoder preserves that field.packages/contracts/src/providerRuntime.ts (1)
285-292: MakeThreadStartedSourcekind-specific.This flat shape accepts impossible combinations like
kind: "subAgentOther"withoutotherKind, orkind: "cli"withdepth. A discriminated contract here would keep malformed canonical events out of downstream ingestion instead of pushing that validation to consumers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/contracts/src/providerRuntime.ts` around lines 285 - 292, ThreadStartedSource is currently a flat Schema.Struct that allows impossible field combos (e.g., kind: "subAgentOther" without otherKind or kind: "cli" with depth); convert it into a discriminated union keyed on kind (use Schema.Sum/Schema.Discriminated/Schema.Union as available) where each branch enforces the correct shape: e.g., a "subAgent"/"subAgentOther" branch requiring parentProviderThreadId and for "subAgentOther" requiring otherKind, a "cli" branch disallowing depth, and the agentNickname/agentRole fields only present on the branches that allow them; update ThreadStartedSource to be this kind-specific summed schema while reusing existing primitives (ThreadStartedSourceKind, TrimmedNonEmptyStringSchema, NonNegativeInt) so downstream consumers get proper validation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/contracts/src/providerRuntime.test.ts`:
- Around line 123-154: The test for decoding "thread.started" doesn't assert the
new payload.status field, so regressions can drop or change it; update the test
in providerRuntime.test.ts to assert parsed.payload.status equals the expected
object (e.g., { type: "active", activeFlags: [] }) after the existing assertions
for providerThreadId/name/preview/source, referencing the parsed variable and
its payload.status property to ensure the decoder preserves that field.
In `@packages/contracts/src/providerRuntime.ts`:
- Around line 285-292: ThreadStartedSource is currently a flat Schema.Struct
that allows impossible field combos (e.g., kind: "subAgentOther" without
otherKind or kind: "cli" with depth); convert it into a discriminated union
keyed on kind (use Schema.Sum/Schema.Discriminated/Schema.Union as available)
where each branch enforces the correct shape: e.g., a "subAgent"/"subAgentOther"
branch requiring parentProviderThreadId and for "subAgentOther" requiring
otherKind, a "cli" branch disallowing depth, and the agentNickname/agentRole
fields only present on the branches that allow them; update ThreadStartedSource
to be this kind-specific summed schema while reusing existing primitives
(ThreadStartedSourceKind, TrimmedNonEmptyStringSchema, NonNegativeInt) so
downstream consumers get proper validation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e99a1385-0930-4a37-8110-329569b861c9
📒 Files selected for processing (5)
apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.test.tsapps/server/src/provider/Layers/CodexAdapter.test.tsapps/server/src/provider/Layers/CodexAdapter.tspackages/contracts/src/providerRuntime.test.tspackages/contracts/src/providerRuntime.ts
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
Scope
Verification
Summary by CodeRabbit
New Features
Tests