Skip to content

Opened against wrong repo — please ignore #3686

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
SergeSerb2 wants to merge 6 commits into from
Closed

Opened against wrong repo — please ignore #3686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ccb4506
ci: require Codex review before merge via status-check gate
SergeSerb2 Jul 4, 2026
089b1da
ci: address Codex review findings on review gate
SergeSerb2 Jul 4, 2026
18a160e
ci: detect clean Codex reviews and poll instead of instant-failing
SergeSerb2 Jul 4, 2026
b6c0e06
ci: relax gate to one Codex review per PR
SergeSerb2 Jul 4, 2026
abe850d
Merge pull request #1 from SergeSerb2/ci/codex-review-gate
SergeSerb2 Jul 4, 2026
b16c18b
Add cross-provider sub-agent spawning via product MCP toolkit
SergeSerb2 Jul 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions .github/workflows/codex-review-gate.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
name: Codex review gate

on:
pull_request:
types: [opened, synchronize, reopened, ready_for_review]
pull_request_review:
types: [submitted]

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Recheck the gate when Codex reviews are dismissed

The workflow explicitly ignores dismissed reviews in the jq filter, but it never runs for the pull_request_review.dismissed action. If a Codex review is dismissed after this check has passed, the required status remains green until some unrelated PR event reruns it, so the gate can still report that a non-dismissed Codex review exists when it no longer does; include dismissed in this trigger.

Useful? React with 👍 / 👎.


permissions:
pull-requests: read
Comment on lines +9 to +10

@macroscopeapp macroscopeapp Bot Jul 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Medium workflows/codex-review-gate.yml:9

The workflow grants only pull-requests: read, but line 35 calls GET repos/$REPO/issues/$PR/comments, which requires issues: read. The GITHUB_TOKEN lacks that scope, so gh api fails (or returns empty) and the clean-review fallback path never detects a Codex review. Add issues: read to the permissions block.

 permissions:
   pull-requests: read
+  issues: read
🤖 Copy this AI Prompt to have your agent fix this: 
In file @.github/workflows/codex-review-gate.yml around lines 9-10:

The workflow grants only `pull-requests: read`, but line 35 calls `GET repos/$REPO/issues/$PR/comments`, which requires `issues: read`. The `GITHUB_TOKEN` lacks that scope, so `gh api` fails (or returns empty) and the clean-review fallback path never detects a Codex review. Add `issues: read` to the `permissions` block.


jobs:
codex-review-required:
name: codex-review-required
runs-on: ubuntu-latest
timeout-minutes: 20
steps:
- name: Require one Codex review on this PR
env:
GH_TOKEN: ${{ github.token }}
REPO: ${{ github.repository }}
PR: ${{ github.event.pull_request.number }}
run: |
codex_reviewed() {

@macroscopeapp macroscopeapp Bot Jul 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟠 High workflows/codex-review-gate.yml:24

codex_reviewed() counts any non-dismissed Codex review or issue comment on the PR without checking that it targets github.event.pull_request.head.sha. After a synchronize push, the previous Codex review/comment is still present on the PR, so the function returns success for a stale review and the gate lets unreviewed new commits merge. Consider filtering reviews/comments by the current head SHA (e.g. matching Reviewed commit: <sha> for issue comments and the review's commit_id for PR reviews) so only a review of the latest commit satisfies the check.

🤖 Copy this AI Prompt to have your agent fix this: 
In file @.github/workflows/codex-review-gate.yml around line 24:

`codex_reviewed()` counts any non-dismissed Codex review or issue comment on the PR without checking that it targets `github.event.pull_request.head.sha`. After a `synchronize` push, the previous Codex review/comment is still present on the PR, so the function returns success for a stale review and the gate lets unreviewed new commits merge. Consider filtering reviews/comments by the current head SHA (e.g. matching `Reviewed commit: <sha>` for issue comments and the review's `commit_id` for PR reviews) so only a review of the latest commit satisfies the check.

local n
n=$(gh api "repos/$REPO/pulls/$PR/reviews" --paginate | jq -s '
[add[] | select(
(.user.type == "Bot")
and (.user.login | ascii_downcase | contains("codex"))
and (.state != "DISMISSED")
)] | length')
[ "$n" -ge 1 ] && return 0
# A clean Codex review ("no major issues") arrives as an issue
# comment carrying "Reviewed commit: `<sha>`", not as a PR review.
n=$(gh api "repos/$REPO/issues/$PR/comments" --paginate | jq -s '
[add[] | select(
(.user.type == "Bot")
and (.user.login | ascii_downcase | contains("codex"))
and (.body | test("Reviewed commit:"))

@chatgpt-codex-connector chatgpt-codex-connector Bot Jul 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Require Codex review for the current head

This accepts any old clean Codex comment because it only tests for the marker, and the review path above similarly counts all non-dismissed Codex reviews without checking the review commit_id. On a synchronize run after new commits are pushed, an earlier Codex review/comment for the previous head still makes the required check pass, so the gate no longer proves the code being merged was reviewed; compare the review/comment SHA with github.event.pull_request.head.sha.

Useful? React with 👍 / 👎.

)] | length')
[ "$n" -ge 1 ]

@cursor cursor Bot Jul 4, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Gate accepts stale Codex reviews

Medium Severity

The Codex review gate treats any historical bot comment containing Reviewed commit: as sufficient. After new commits are pushed, an review tied to an older SHA can still satisfy the check without re-reviewing the current head.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b16c18b. Configure here.

}

for i in $(seq 1 30); do
if codex_reviewed; then
echo "Codex has reviewed this PR."
exit 0
fi
echo "Waiting for Codex review ($i/30)..."
sleep 30
done
echo "::error::No Codex review on this PR after 15 minutes. Comment '@codex review' on the PR, then re-run this check."
exit 1
11 changes: 10 additions & 1 deletion apps/server/src/mcp/McpHttpServer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import {
PreviewSnapshotToolkit,
PreviewStandardToolkit,
} from "./toolkits/preview/tools.ts";
import { SubAgentToolkitHandlersLive } from "./toolkits/agents/handlers.ts";
import { SubAgentToolkit } from "./toolkits/agents/tools.ts";

const unauthorized = HttpServerResponse.jsonUnsafe(
{
Expand Down Expand Up @@ -208,10 +210,17 @@ export const PreviewToolkitRegistrationLive = Layer.mergeAll(
PreviewSnapshotRegistrationLive,
);

export const SubAgentToolkitRegistrationLive = McpServer.toolkit(SubAgentToolkit).pipe(
Layer.provide(SubAgentToolkitHandlersLive),
);

const McpTransportLive = McpServer.layerHttp({
name: "T3 Code",
version: packageJson.version,
path: "/mcp",
}).pipe(Layer.provide(McpAuthMiddlewareLive));

export const layer = PreviewToolkitRegistrationLive.pipe(Layer.provideMerge(McpTransportLive));
export const layer = Layer.mergeAll(
PreviewToolkitRegistrationLive,
SubAgentToolkitRegistrationLive,
).pipe(Layer.provideMerge(McpTransportLive));
2 changes: 1 addition & 1 deletion apps/server/src/mcp/McpInvocationContext.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
import * as Context from "effect/Context";
import * as Effect from "effect/Effect";

export type McpCapability = "preview";
export type McpCapability = "preview" | "agents";

export interface McpInvocationScope {
readonly environmentId: EnvironmentId;
Expand Down
2 changes: 1 addition & 1 deletion apps/server/src/mcp/McpSessionRegistry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ const makeWithOptions = Effect.fn("McpSessionRegistry.make")(function* (
threadId: ThreadId.make(request.threadId),
providerSessionId,
providerInstanceId: ProviderInstanceId.make(request.providerInstanceId),
capabilities: new Set(["preview"]),
capabilities: new Set(["preview", "agents"]),
issuedAt,
expiresAt,
};
Expand Down
Loading
Loading