feat: add Codexpert connector middleware#236
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39bb0fe531
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| ): CodexpertTaskResult { | ||
| const terminalError = error ?? (event?.type === 'error' ? event.message : null) | ||
| return { | ||
| status: terminalError ? 'failed' : 'success', |
There was a problem hiding this comment.
buildResult marks every run as success unless an explicit error was thrown, so a terminal done event with status like timeout, canceled, or backend-reported failure is currently reported as success. This can make the agent continue follow-up actions (for example PR publishing or “completed” messaging) on runs that actually failed or were interrupted.
Useful? React with 👍 / 👎.
| ...input, | ||
| updatedAt: new Date(), | ||
| }) | ||
| await this.repository.save(entity) |
There was a problem hiding this comment.
record lets repository write errors bubble up, but runCodexpertTask awaits this call in both normal and error paths. If the mapping table is unavailable (e.g., migration lag, permission issue, transient DB outage), the middleware can fail the whole tool call and even mask the original Codexpert outcome, although this table is intended for recovery/diagnostics metadata.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds a new @xpert-ai/plugin-codexpert-connector middleware package that proxies Codexpert MCP context tools and provides a synchronous runCodexpertTask tool with user-visible streamed output projection, plus a lightweight run-mapping persistence layer.
Changes:
- Introduces Codexpert connector middleware strategy, MCP/HTTP client, and visible markdown-safe projection utilities.
- Adds TypeORM entity + service to persist Xpert execution ↔ Codexpert session/task/thread/execution mappings.
- Adds new package scaffolding (tsconfigs, jest config, docs) plus lockfile + changeset updates.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| xpertai/pnpm-lock.yaml | Adds lock entries for the new middleware workspace package dependencies. |
| xpertai/middlewares/codexpert-connector/tsconfig.spec.json | Adds test tsconfig for the new package. |
| xpertai/middlewares/codexpert-connector/tsconfig.lib.json | Adds library build tsconfig for the new package. |
| xpertai/middlewares/codexpert-connector/tsconfig.json | Adds TS project references for lib/spec builds. |
| xpertai/middlewares/codexpert-connector/src/lib/types.ts | Defines config schema, tool input schema, events, and result types. |
| xpertai/middlewares/codexpert-connector/src/lib/markdown-format.ts | Adds markdown normalization + hard line-break preservation for visible projection. |
| xpertai/middlewares/codexpert-connector/src/lib/entities/codexpert-connector-run.entity.ts | Adds TypeORM entity for run mapping persistence. |
| xpertai/middlewares/codexpert-connector/src/lib/codexpert-visible-projector.ts | Implements event → user-visible chunk projection with dedupe + buffering. |
| xpertai/middlewares/codexpert-connector/src/lib/codexpert-visible-projector.spec.ts | Adds unit tests (currently covering markdown formatting behavior). |
| xpertai/middlewares/codexpert-connector/src/lib/codexpert-connector.module.ts | Registers plugin module, middleware, and run service; ensures schema at bootstrap. |
| xpertai/middlewares/codexpert-connector/src/lib/codexpert-connector.middleware.ts | Main middleware strategy: exposes MCP tools + runCodexpertTask, streams output, records runs. |
| xpertai/middlewares/codexpert-connector/src/lib/codexpert-connector.client.ts | Adds MCP client wrapper + connector HTTP streaming client with identity headers. |
| xpertai/middlewares/codexpert-connector/src/lib/codexpert-connector-run.service.ts | Adds schema bootstrap + record/upsert persistence. |
| xpertai/middlewares/codexpert-connector/src/index.ts | Adds plugin entrypoint + exports. |
| xpertai/middlewares/codexpert-connector/package.json | Declares the new package metadata and peer dependencies. |
| xpertai/middlewares/codexpert-connector/jest.config.ts | Adds Jest config for the new package. |
| xpertai/middlewares/codexpert-connector/eslint.config.mjs | Adds ESLint config placeholder for the new package. |
| xpertai/middlewares/codexpert-connector/README_zh.md | Adds Chinese documentation for config, tool flow, projection, and persistence model. |
| xpertai/middlewares/codexpert-connector/README.md | Adds English documentation for config, tool flow, projection, and persistence model. |
| xpertai/middlewares/codexpert-connector/.spec.swcrc | Adds SWC config for Jest (TS + decorators). |
| xpertai/.changeset/tidy-codexpert-connectors.md | Adds changeset entry for publishing the new package. |
Files not reviewed (1)
- xpertai/pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const terminalError = error ?? (event?.type === 'error' ? event.message : null) | ||
| return { | ||
| status: terminalError ? 'failed' : 'success', | ||
| codingSessionId: pickString( | ||
| event && 'codingSessionId' in event ? event.codingSessionId : null, | ||
| sessionId, | ||
| sessionSnapshot?.codingSessionId, | ||
| sessionSnapshot?.id, | ||
| ), | ||
| taskId: pickString(event && 'taskId' in event ? event.taskId : null, input.taskId), | ||
| threadId: pickString(event && 'threadId' in event ? event.threadId : null, input.threadId), | ||
| executionId: pickString(event && 'executionId' in event ? event.executionId : null), | ||
| repo: { | ||
| id: pickString(input.repoId, sessionSnapshot?.repoId), | ||
| name: pickString(sessionSnapshot?.repoName), | ||
| owner: pickString(sessionSnapshot?.repoOwner), | ||
| slug: pickString(sessionSnapshot?.repoSlug), | ||
| }, | ||
| branch: pickString(input.branchName, sessionSnapshot?.branchName), | ||
| environmentId: pickString(event && 'environmentId' in event ? event.environmentId : null, sessionSnapshot?.environmentId), | ||
| environmentReused: null, | ||
| summary: event?.type === 'done' ? pickString(event.summary, event.output) : null, | ||
| error: terminalError, | ||
| ...(event?.type === 'done' && event.prUrl ? { prUrl: event.prUrl } : {}), | ||
| } |
There was a problem hiding this comment.
buildResult always returns status: 'success' | 'failed' based only on whether an error message exists, ignoring event.status (and the timeout / canceled variants declared in CodexpertTaskResult). Also, if the stream ends without a terminal done/error event (event is null), this currently reports success, which can mask incomplete/aborted executions. Map done.status (and any timeout/canceled semantics) into the returned result and treat a missing terminal event as a failure/timeout so callers can react correctly.
| repo: { | ||
| id: pickString(input.repoId, sessionSnapshot?.repoId), | ||
| name: pickString(sessionSnapshot?.repoName), | ||
| owner: pickString(sessionSnapshot?.repoOwner), | ||
| slug: pickString(sessionSnapshot?.repoSlug), | ||
| }, | ||
| branch: pickString(input.branchName, sessionSnapshot?.branchName), |
There was a problem hiding this comment.
buildResult always returns a non-null repo object, even when neither the input nor the session snapshot has any repo fields (it becomes { id: null, name: null, ... }). Since CodexpertTaskResult allows repo: ... | null (and buildInitialResult already returns null when repo is unknown), consider returning null here too when no repo data is available to keep the API consistent and avoid downstream null-vs-empty-object ambiguity.
| subscriber.next({ | ||
| data: { | ||
| type: ChatMessageTypeEnum.MESSAGE, | ||
| data: text, | ||
| }, | ||
| }) |
There was a problem hiding this comment.
In emitSubscriberMessage, the fallback subscriber.next(...) inside the catch is not itself protected. If subscriber.next throws (e.g., subscriber is closed / backpressure errors), this will still bubble up and can fail the whole projection/tool call. Wrap the fallback subscriber.next in its own try/catch (or otherwise swallow/log) to keep projection best-effort.
| subscriber.next({ | |
| data: { | |
| type: ChatMessageTypeEnum.MESSAGE, | |
| data: text, | |
| }, | |
| }) | |
| try { | |
| subscriber.next({ | |
| data: { | |
| type: ChatMessageTypeEnum.MESSAGE, | |
| data: text, | |
| }, | |
| }) | |
| } catch { | |
| // Best-effort subscriber delivery; ignore fallback emission errors. | |
| } |
| data: { | ||
| type: 'text', | ||
| text: `Codexpert failed: ${message}`, | ||
| xpertName: 'Codexpert', | ||
| agentKey: 'codexpert', | ||
| } satisfies TMessageContentText, |
There was a problem hiding this comment.
projectImmediateError hard-codes xpertName: 'Codexpert' and agentKey: 'codexpert' even though the module defines CODEXPERT_XPERT_NAME / CODEXPERT_AGENT_KEY constants. Reuse the constants here to avoid drift if these identifiers change elsewhere.
| import { formatVisibleMarkdown, preserveMarkdownLineBreaks } from './markdown-format.js' | ||
|
|
||
| describe('preserveMarkdownLineBreaks', () => { | ||
| it('keeps visible single line breaks in markdown text', () => { | ||
| expect(preserveMarkdownLineBreaks('第一行\n第二行\n第三行')).toBe('第一行 \n第二行 \n第三行') | ||
| }) | ||
|
|
||
| it('keeps a trailing line break when a flush boundary ends after newline', () => { |
There was a problem hiding this comment.
This test file is named codexpert-visible-projector.spec.ts, but it only tests markdown-format.ts. Renaming it (or adding projector-specific tests here) would make test intent clearer and help keep coverage organized.
| "dependencies": {}, | ||
| "peerDependencies": { | ||
| "@langchain/core": "0.3.72", | ||
| "@metad/contracts": "^3.8.0", | ||
| "@modelcontextprotocol/sdk": "^1.17.5", | ||
| "@nestjs/common": "^11.1.6", | ||
| "@nestjs/typeorm": "^11.0.0", | ||
| "@xpert-ai/chatkit-types": "^0.0.12", | ||
| "@xpert-ai/plugin-sdk": "^3.8.0", | ||
| "chalk": "4.1.2", | ||
| "typeorm": "^0.3.20", | ||
| "zod": "3.25.67" | ||
| } |
There was a problem hiding this comment.
pnpm-lock.yaml lists tslib as a direct dependency for this workspace package, but package.json does not declare it in either dependencies or peerDependencies. This manifest/lock mismatch can break installs with --frozen-lockfile. Either add the missing tslib entry to package.json (matching the lock specifier), or regenerate the lockfile after aligning the manifest.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated 7 comments.
Files not reviewed (1)
- xpertai/pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| { name: 'id', type: 'uuid', isPrimary: true, generationStrategy: 'uuid', default: 'gen_random_uuid()' }, | ||
| { name: 'tenant_id', type: 'varchar', length: '64' }, | ||
| { name: 'organization_id', type: 'varchar', length: '64' }, | ||
| { name: 'user_id', type: 'varchar', length: '64' }, | ||
| { name: 'xpert_id', type: 'varchar', length: '64', isNullable: true }, | ||
| { name: 'conversation_id', type: 'varchar', length: '128', isNullable: true }, | ||
| { name: 'execution_id', type: 'varchar', length: '128', isNullable: true }, | ||
| { name: 'coding_session_id', type: 'varchar', length: '128', isNullable: true }, | ||
| { name: 'task_id', type: 'varchar', length: '128', isNullable: true }, | ||
| { name: 'thread_id', type: 'varchar', length: '128', isNullable: true }, | ||
| { name: 'codexpert_execution_id', type: 'varchar', length: '128', isNullable: true }, | ||
| { name: 'status', type: 'varchar', length: '32' }, | ||
| { name: 'last_error', type: 'text', isNullable: true }, | ||
| { name: 'metadata', type: 'jsonb', isNullable: true }, | ||
| { name: 'created_at', type: 'timestamp with time zone', default: 'now()' }, | ||
| { name: 'updated_at', type: 'timestamp with time zone', default: 'now()' }, | ||
| ], |
| @Column({ name: 'last_error', type: 'text', nullable: true }) | ||
| lastError?: string | null | ||
|
|
||
| @Column({ name: 'metadata', type: 'jsonb', nullable: true }) |
| let newlineIndex = buffer.indexOf('\n') | ||
| while (newlineIndex >= 0) { | ||
| const line = buffer.slice(0, newlineIndex).trim() | ||
| buffer = buffer.slice(newlineIndex + 1) | ||
| if (line) { | ||
| yield JSON.parse(line) as CodexpertConnectorEvent | ||
| } |
| export async function projectVisibleCodexpertEvent( | ||
| event: CodexpertConnectorEvent, | ||
| state: ProjectionState, | ||
| config?: RunnableConfig, | ||
| enableStatusEvents = true, | ||
| ) { | ||
| if (event.type !== 'text_delta') { | ||
| flushVisibleCodexpertProjection(state, config) | ||
| } | ||
|
|
||
| const text = resolveVisibleText(event, state, enableStatusEvents) | ||
| if (!text) { | ||
| return | ||
| } | ||
|
|
||
| if (event.type === 'text_delta') { | ||
| appendBufferedText(state, text) | ||
| if (shouldFlushBufferedText(state.pendingText)) { | ||
| flushVisibleCodexpertProjection(state, config) | ||
| } else { | ||
| scheduleBufferedTextFlush(state, config) | ||
| } | ||
| return | ||
| } | ||
|
|
||
| emitSubscriberMessage(config, text) | ||
| } | ||
|
|
| Compile the plugin: | ||
|
|
||
| ```bash | ||
| cd /Users/lilinhao/project/code-xpert/codexpertplugin/xpertplugins |
| @@ -0,0 +1 @@ | |||
| export default [] | |||
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Summary
Verification