Repository navigation
refactor(tooling): share repository Git adapters - #1203
Conversation
Repository fixtures and hydration tools duplicate Git process setup. Share the existing invocation patterns with their actual consumers.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The narrow refactor preserves existing Git execution behavior, with no unresolved blocking issues.
Review effort: Balanced
Findings: None
What changed in this PR
Consolidates Git invocation helpers across repository tooling and fixtures, providing the shared fixture dependency for the SDK follow-up.
Changes:
- Adds a typed Git text adapter for MCP and SDK consumers.
- Reuses Git execution in release and hydration tooling while preserving output, stdin, stderr, and failure behavior.
| File | Description |
|---|---|
| sdk/typescript/tests-ts/support/shell.ts | Re-exports the Git adapter. |
| sdk/typescript/tests-ts/release-pr.test.ts | Uses the adapter for blob creation. |
| sdk/typescript/tests-ts/git-fixture.ts | Shares fixture Git execution. |
| sdk/typescript/scripts/release-pr.mjs | Uses the adapter for release operations. |
| plugins/codex-security/mcp-app/tests/test_mcp_app_smoke.mjs | Uses the adapter to read commit IDs. |
| plugins/codex-security/mcp-app/tests/test_compact_artifact_server.mjs | Shares fixture Git execution. |
| plugins/codex-security/mcp-app/scripts/git.mjs | Adds the UTF-8 Git adapter. |
| plugins/codex-security/mcp-app/scripts/git.d.mts | Declares adapter types. |
| evals/triage-finding/scripts/hydrate-calibration-repos.js | Reuses the hydration Git helper. |
| evals/triage-finding/sastbench/scripts/hydrate-sastbench-repos.js | Exports the helper with configurable stderr. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ 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". |
alandelong-oai
left a comment
There was a problem hiding this comment.
Reviewed 3807571306e706884d831acc5825ae4a707d6810 against its own base 3642a76a53173e012f579737d1b6cf4096379af2 with exactly three independent high-reasoning full-diff reviews, including every test diff, plus root verification. No serious introduced code defect identified.
73 focused SDK tests and both complete affected MCP test scripts passed; fresh native host build, plugin/SDK builds, types, formatting, Ruff and portable source checks passed.
Local validation was on macOS arm64. No model execution, local Windows run or full installed-package smoke. Reviewed sources remained unchanged. This review covers this exact head; it does not establish combined-stack validation against current main.
Summary
Repository fixtures and hydration tools duplicate Git process setup. Share the existing invocation patterns with their actual consumers.
Changes
Testing
Release-PR tests passed (73 cases), along with hydration and Git fixture checks. Release-automation warning failures passed after disabling ambient proxy warnings. All five portable source checks and SDK/MCP typechecks passed.
Risk and rollout
Git output, exit failures and raw diagnostics retain their current behavior. This supplies the fixture dependency used by the runtime follow-up.
This patch has no source prerequisite within the series. Squash-merge it into
mainafter review and checks pass. Before retargeting dependent PRs tomain, rebase them ontomainexcluding this branch’s old pre-squash commits and rerun their checks.Direct follow-ups: #1207.
When integrating with #1206, retain both the Git adapter and temporary-directory helper imports in
test_compact_artifact_server.mjs.Public disclosure review