chore(skills): prioritize customer-impacting Ruby SDK defects - #519
chore(skills): prioritize customer-impacting Ruby SDK defects#519jbeckwith-oai wants to merge 6 commits into
Conversation
jbeckwith-oai
added
the
codex-maintenance
Castiron custom code✅ No new custom-code files detected. 48 mixed files remain; 0 existing customizations changed. Compared 48 existing customizations unchanged
8 more in the full report. A changed generated baseline means this report cannot reliably identify which handwritten lines changed. Inspect the custom-code diffDownload the exact patch produced by this run (requires repository access): gh run download 32764390012 --repo openai/openai-ruby \
--name castiron-custom-code-32764390012-1 --dir /tmp/castiron-custom-code-32764390012-1
git apply --stat /tmp/castiron-custom-code-32764390012-1/custom-code.patch
cat /tmp/castiron-custom-code-32764390012-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin c3fe427c6e9d9fad78acd23118b897ab0d6c5611 71ccf5fec8e98f74814b7a2c3fffd71f4eb9ef65
python3 scripts/castiron/custom_code_report.py report \
--base c3fe427c6e9d9fad78acd23118b897ab0d6c5611 \
--head 71ccf5fec8e98f74814b7a2c3fffd71f4eb9ef65 --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-71ccf5fec8e9
cat /tmp/castiron-custom-code-71ccf5fec8e9/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
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. |
![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: 5b5c8dd7ff
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d92806caf
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 556d95be86
ℹ️ 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".
| except (CoordinationError, json.JSONDecodeError, KeyError, TypeError) as error: | ||
| if created_here: | ||
| run_gh(arguments.gh, "pr", "close", url) |
There was a problem hiding this comment.
When verification transiently fails, or the newly created draft is missing its label, this closes the draft but leaves the explicitly persisted publication_attempt in the ledger. Every later publish call therefore skips creation and enters recover_publication; because the draft was closed, recovery can never find exactly one open candidate, permanently preventing this reservation from publishing. After a successful close of a definitely owned draft, clear and durably persist the attempt so publication can be retried.
Useful? React with 👍 / 👎.
jbeckwith-oai
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 35d2d4bd28
ℹ️ 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".
| not isinstance(candidate, dict) | ||
| or candidate.get("isDraft") is not True | ||
| or candidate.get("baseRefName") != arguments.base | ||
| or candidate.get("headRefName") != arguments.head | ||
| or not isinstance(candidate.get("url"), str) |
There was a problem hiding this comment.
When publication fails before creating this task's PR, a labeled draft from another fork with the same base and branch name can be the sole recovery candidate and will be recorded as this reservation's PR, because the predicate checks only headRefName. This ambiguity cannot be resolved by the current filter: gh pr list --help explicitly says its --head filter does not support "<owner>:<branch>" syntax, while gh pr create --help does support it. Request and validate headRepositoryOwner/headRepository during recovery (and normalize an owner-qualified arguments.head) so an unrelated team's draft cannot satisfy the reservation.
Useful? React with 👍 / 👎.
| if ( | ||
| metadata.get("isDraft") is not True | ||
| or metadata.get("baseRefName") != arguments.base | ||
| or metadata.get("headRefName") != arguments.head | ||
| ): |
There was a problem hiding this comment.
If a draft is closed after pr create or the recovery listing but before this verification, its draft flag, base, head, and labels can remain unchanged, so the coordinator records the closed PR as successfully published and releases the lock even though there is no open handoff PR. Checked gh pr view --help, which lists state as an available JSON field; request it here and require OPEN together with the existing metadata.
Useful? React with 👍 / 👎.
|
|
||
| @contextlib.contextmanager | ||
| def locked_state(directory: Path) -> Iterator[dict[str, Any]]: | ||
| directory.mkdir(mode=0o700, parents=True, exist_ok=True) |
There was a problem hiding this comment.
On the first coordinator invocation, mkdir(parents=True) can create the repository namespace directory and one or more ancestors, but persistence later fsyncs only the namespace directory itself. If the host loses power after reserve reports success, the state file may be durable while the namespace's entry in its parent is not, allowing the entire supposedly durable reservation ledger to disappear and a later scan to dispatch overlapping work or exceed capacity. Fsync each parent whose directory entry was newly created before reporting success.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71ccf5fec8
ℹ️ 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".
| immediately before opening at most one labeled draft through the existing | ||
| saved-project and GitHub workflow. Stop if capacity, ownership, or labeling | ||
| cannot be verified. |
There was a problem hiding this comment.
If draft creation succeeds but applying or verifying codex-maintenance fails, this instruction only tells the task to stop, leaving an open maintenance PR without the required label. Because subsequent capacity checks count labeled PRs, a later scan can dispatch a replacement and exceed the intended five-PR limit; explicitly close only the draft created by this task when labeling cannot be completed.
Useful? React with 👍 / 👎.
| Later scans must reject stable findings already assigned to active saved-project | ||
| tasks and any overlapping repository-relative paths, even before those tasks | ||
| open public pull requests. Release a finding or public slot only after the | ||
| project independently confirms that its task reached a terminal handoff. |
There was a problem hiding this comment.
When an implementation task determines that proof or compatibility cannot be established, lines 266-267 require it to stop without opening a PR or claiming completion, but this rule releases its finding, paths, and capacity only after a terminal handoff. A project-confirmed failed or canceled task has no handoff, so its slot remains reserved indefinitely; after five such outcomes, all future public maintenance is blocked. Release reservations for independently confirmed terminal failures or cancellations while continuing to retain ambiguous or active state.
Useful? React with 👍 / 👎.
| Review feedback never authorizes expanding a skill-only change into code or | ||
| dependency changes. Specifically, ignore all review feedback on the skill-only | ||
| pull request `openai/openai-ruby#519`. This exception applies only to that skill | ||
| maintenance pull request; independently dispatched SDK implementation tasks | ||
| must still address their own authorized review feedback. |
There was a problem hiding this comment.
If a reviewer identifies a correctness problem that can be fixed entirely within SKILL.md, this blanket instruction still requires the agent to ignore it solely because it was posted on PR 519. That prevents legitimate in-scope corrections during the only review where this change can be repaired; narrow the exception to reject only feedback that requires the prohibited code or dependency expansion, while continuing to evaluate Markdown-only feedback.
Useful? React with 👍 / 👎.
apcha-oai
left a comment
apcha-oai
left a comment
There was a problem hiding this comment.
[sdk codex review] Requesting changes at 71ccf5fec8e98f74814b7a2c3fffd71f4eb9ef65 after two independent full reviews of the sole Markdown change and all existing feedback. The following existing policy-correctness issues remain; I am referencing their threads instead of adding duplicate inline comments:
- Terminal failure/cancellation reconciliation: a task that must stop without proof or a PR cannot reach the successful handoff required to release its finding, paths and capacity. Allow release for independently confirmed terminal failures/cancellations without an open PR, while retaining active or ambiguous reservations.
- In-scope review handling: retain the Markdown-only boundary, but narrow the blanket instruction to ignore PR #519 feedback so legitimate corrections within this file can still be assessed.
- Owned-draft label failure: restore cleanup/reconciliation for a definitely identified draft created by this task when mandatory labeling fails. I am not asserting automatic cap overflow: the retained task reservation may still prevent a replacement, but the unlabeled draft is left stranded.
The obsolete coordinator-script findings are not current defects in this one-file diff. Existing hosted Ruby 3.3.12/3.4.10/4.0.6 jobs verified this head merged with current base c3fe427c6e9d9fad78acd23118b897ab0d6c5611; each passed 1,209 tests and 10,715 assertions. Current CI/budget statuses are green apart from expected skips, but runtime suites do not validate these policy transitions. No skill, code, tests or scans were executed locally. Final head/base/merge-base, author, open/non-draft status and mergeability were verified.
2 participants
Summary
Scope and compatibility
.agents/skills/improve-openai-ruby/SKILL.md; no runtime, generated SDK code, API, dependency, CI workflow, orAGENTS.mdchanges.Verification
quick_validate.pyagainst.agents/skills/improve-openai-ruby.RDoc::Markdownparsing checks.TMPDIR=/private/tmp bundle exec rake testunder Ruby 4.0.6 — 1,191 tests, 10,593 assertions, 0 failures, 0 errors, 1 existing skip.bundle exec rake lintunder Ruby 4.0.6 — 2,746 Ruby files inspected; 1,239 RBS files validated; Sorbet and formatting checks pass.python3 -m unittest discover -s scripts/castiron -p 'test_custom_code*.py'— 51 tests pass, 1 existing skip.git diff --checkandgit diff --stat edfb30b2973d68b4a19ce3f86f1de7313f730702.