EPCIS V2 Reworked#37
Conversation
Adds OpenAI Codex-powered code review that runs on every PR and posts inline comments like a human reviewer. Uses structured output schema for reliable parsing and GitHub Review API for native inline comments. - .codex/review-prompt.md: Two-pass review (blockers then maintainability) with comment gate, deduplication, and uncertainty guard - .codex/review-schema.json: Strict output schema (additionalProperties: false) - .github/workflows/codex-review.yml: SHA-pinned actions, fork PR guard, paginated file listing, null-patch handling, overflow comments in summary Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
If Codex returns empty or non-JSON output, the workflow now posts a warning comment with a link to the logs instead of silently failing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tput Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
There are two breaking changes that need attention: the removal of the /epcis/asset/*ual endpoint and the MCP epcis-query response shape change. Both are API compatibility risks and should be handled with backward-compatible shims or versioning before merge. The validation/test additions look solid, but there are still some maintainability concerns around environment variable compatibility and test import paths.
| // GET /epcis/events/track - Track single EPC across full trace | ||
| api.get( | ||
| "/epcis/asset/*ual", | ||
| "/epcis/events/track", |
There was a problem hiding this comment.
🔴 Bug: This introduces /epcis/events/track but removes the existing /epcis/asset/*ual endpoint. That’s a breaking API change for clients that fetch by UAL. Consider keeping the old route as an alias or providing a deprecation/redirect path.
| { | ||
| summary, | ||
| count: resultCount, | ||
| events: resultData || [], |
There was a problem hiding this comment.
🔴 Bug: epcis-query now returns events as resultData (array) instead of the full graph.query response object. This changes the MCP tool response schema and can break existing clients. Consider preserving the previous shape (or adding a new field) or versioning the tool response.
| } | ||
|
|
||
| function getPublisherEndpoint(): string { | ||
| const publisherUrl = process.env.EXPO_PUBLIC_MCP_URL; |
There was a problem hiding this comment.
🟡 Issue: This switches the publisher base URL env var from PUBLISHER_URL to EXPO_PUBLIC_MCP_URL. Existing deployments that only set PUBLISHER_URL will fail at runtime. Consider supporting a fallback or keeping the old env var name for backward compatibility.
| import sinon from "sinon"; | ||
| import request from "supertest"; | ||
| import pluginEpcisPlugin from "../dist/index.js"; | ||
| import { EpcisQueryService } from "../src/services/epcisQueryService.js"; |
There was a problem hiding this comment.
🟡 Issue: Tests should import from ../dist/... per AGENTS; importing ../src/services/epcisQueryService.js bypasses the build and can hide packaging issues. Either export this from dist and import there, or test via the plugin surface.
2 participants
No description provided.