Conversation
|
Welp, first CI run failed, I'll get that sorted. |
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
|
OK fixed- I also had Codex sign all commits as co-author, hence the force push (I assume no one reviewed this in the past 15 minutes anyway). The commit fixing the CI problem also documents the issue in CONTRIBUTING.md. |
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
|
The previous fix involved platform-specific dependency installation issues. These further fixes ensure we don't have those issues in consumer repos, and updated documentation accordingly. |
|
Used AI to grok this rather big PR. Here is one item. I did, however, verify the below myself. src/schema/schema-publish.sh — unbound $version under set -u. In src mode, version is never assigned, but ⏺ Here's the walk-through of that finding in src/schema/schema-publish.sh. The setup: set -u at the top of the script Line 2 is set -euo pipefail. The -u part means: referencing a variable that was never assigned is a fatal error — bash aborts immediately Where version gets assigned — and where it doesn't The script has two modes, chosen by the first argument: if [ -z "$COMMAND" ]; then # no argument
if [[ $branch =~ ^v([0-9]+\.[0-9]+)-dev$ ]]; then
version="${BASH_REMATCH[1]}" # ← version IS set here
deploydir="./deploy/$SPEC_SLUG/${version}"
...
elif [ "$COMMAND" = "src" ]; then # "src" argument
deploydir="./deploy-preview" # ← version is NEVER set hereSo in src mode (which is exactly how consumers run it — the README tells them to wire build-src to oai-spec-publish-schemas src), the Where the unset variable gets used Inside publish_schema(), there's a branch that only runs when a schema directory has no Jekyll lander markdown file yet. In that case it if [ ! -z "$lastLander" ]; then That sed substitution references $version. In no-arg mode it works fine. In src mode, bash hits the unset $version, and because of set -u, |
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
|
@miqui thanks for taking a look, and great idea to use AI to analyze this. In addition to (via Codex) fixing the problem, I had it analyze this package for better testing opportunities, and particularly to add tests that served as examples of intended production use. The new commits include those, plus updates to the README explaining the testing approach. I am hoping that these example tests will help with reviewing and accepting this PR. I think when taking an AI-forward approach to software, focusing on the demonstrable behavior is the most effective use of human attention. And when humans notice that behavior is not entirely demonstrable, that is a gap that the responsible human author (me) should address. |
Co-authored-by: Codex <codex@openai.com>
|
The latest commit (pushed right after this comment) generalizes some things that are different for Arazzo so we can support that repository as well. It also adds another test case. I do not anticipate adding more commits unless/until more changes are requested. |
| @@ -0,0 +1,22 @@ | |||
| version: 2 | |||
| updates: | |||
| - package-ecosystem: npm | |||
There was a problem hiding this comment.
should we add a github actions package ecosystem as well so we keep GHA up to date
There was a problem hiding this comment.
Probably — I'll look into it.
|
|
||
| ```sh | ||
| npm update @oai/build-infra | ||
| npm test |
There was a problem hiding this comment.
Dependencies will be updated in the main branch of the consuming repository, and its schemas and schema tests live in the vX.Y-dev branches, so changes that break schema testing will only be detected after syncing the dependency update from main to the vX.Y-dev branches.
Maybe dependency updates need to be done in a dev branch, tested, discarded, and then done in the main branch?
Or maybe I am too pessimistic 😎
There was a problem hiding this comment.
I'd be fine with only managing dependencies on dev and the vX.Y-dev branches- one fewer place to sync. I don't think that changes anything in this repo unless you wanted the consuming branch strategy documented here instead of in the consuming repos?
There was a problem hiding this comment.
The respec build stuff is currently needed on main for publishing the released spec versions and on vX.Y-dev for previewing the work-in-progress versions, they all need dependency updates.
There was a problem hiding this comment.
@ralfhandl what are you requesting here? I'm feeling lost.
This repository centralizes all dependency management. Consuming repos then update their dependency on this repository, in whatever branches they need (presumably the same branches and sync patterns as before). When they install this package, it pulls in all of the dependencies formerly directly managed in the consuming repos.
I can't figure out what your comments here are asking, or whether you're talking about a dev branch here or in the consuming repos. Or some difference between what is updated on main vs dev in downstream repos (in which case, what is the behavior you think this repo needs to provide that it does not already)?
This PR (written by Codex with GPT 5.5; see last section below for details) factors the build system out of the OpenAPI-Specification repository so that it can be shared by other repositories, initially sig-security and sig-lifecycle (our existing repos that will produce SAF specifications). I'm not entirely sure how to review it, and am open to suggestions on any aspect of restructuring things in terms of commits or a series of PRs. (paging @ralfhandl )
Related PRs and work, in assumed merge order:
What this does
I'm not going to explain it in detail because I want reviewers to ensure that the documentation is sufficient for that. However, the following goals have (hopefully) been accomplished:
@oai/build-infra, which is installed from GitHub and should not ever be published to NPM. This eliminates the constant flood of dependabot PRs and their myriad downstream "sync from main/dev/etc." PRs that are constantly clogging the spec repo.scriptsdirectory in the OpenAPI-Specification have been replaced by this repository. The first commit in this repo shows where they moved, without any changes. Subsequent commits show what was changed.How to review it
I don't personally think we need to review this in line-by-line detail. We're not about to release anything, and we can merge the downstream PRs in sig-security and sig-lifecycle first and make sure the CI all passes there before merging in OpenAPI-Specification,
To whatever extent you do want to review details, I recommend looking at the diffs commit-by-commit as I had Codex break the steps down a bit into incremental chunks, although some are still quite large.
This is on a branch in this repo instead of on my fork so that draft PRs in downstream repos can access the commit for their CI.
Codex's role and how to manage it
This is the first time I've posted AI-written code as a public PR. I chose to do this with AI because (as @ralfhandl and I have discussed), we've wanted something like this but no one wants to do it by hand. Ralf put in heroic amounts of work to get it to its current state in the OpenAPI-Specification repo as it is.
I chose Codex because of the models I've used fairly extensively (Claude Code Sonnet and Opus, Codex 5.5) it has consistently handled complex, vague instructions the best (although it looks like Claude Fable will outdo it significantly).
I'm still relatively new to Codex, so I only just now noticed that, unlike Claude, it writes extremely terse commit messages. I can fix that if it is a concern. [EDIT: I previously noted the lack of Codex putting itself as a co-author on the commits, but I fixed that when I fixed the CI issue]