feat: seed the SDK from its contribution to the Omnigent repository - #1
Merged
Conversation
The Omnigent maintainers asked that this live in our own organisation rather than upstream, and that turns out to fix a real problem rather than only a political one. As a fork pull request against their monorepo it could never pass CI: every substantive check — pytest, integration, end-to-end, the Docker build — skipped behind a security gate that a fork touching CI cannot clear, so nothing meaningful had ever run against this code. Here the checks are the gate. It also removes an indirection that was a small lie. A consumer declared a dependency on the upstream module path at v0.0.0 and rewrote it with a `replace` onto a pseudo-version of a personal fork. That collapses into an ordinary dependency on a tagged module. What moved: the hand-written client (client, session, stream, event, errors, list, doc) with its tests, the two generated files, the generator, and the check script. The module path is rewritten throughout and nothing references the old one. 300 tests pass, `-race` included, and bin/check.sh — the same script a developer runs locally, invoked by CI rather than reimplemented so the two cannot drift — is green. Licensing, which is the one part that is not mechanical. The repository was scaffolded MIT. That is wrong here: models.gen.go and events.gen.go are generated from a vendored copy of the Omnigent OpenAPI specification and reproduce its schema descriptions as doc comments, so the generated half is a derivative of an Apache-2.0 input owned by Databricks. This is therefore Apache-2.0 with a NOTICE carrying that attribution. The NOTICE is scoped rather than copied verbatim: upstream's lists its Python runtime dependencies, none of which this module contains, and section 4(d) provides for excluding notices that do not pertain to a derivative work. A test caught that. TestModuleShipsItsOwnLicence was written when this module was nested inside the Apache-2.0 repository, to keep the licence in the module zip; it failed on the move and is why the relicensing did not happen silently. Its rationale is rewritten for the new layout rather than left describing a nesting that no longer exists. The vendored spec is a snapshot, and the README says so where a reader will meet it. Several defects in the first consumer of this SDK came from reasoning about server behaviour off a stale or simply wrong copy of that specification, so refreshing it is a deliberate act with a diff to read, not a background sync.
Removing the working-directory that scoped every step into the nested module left `defaults:` with no value. PyYAML reads that as null and accepts it; GitHub's workflow schema rejects it, and the rejection is a startup_failure with no jobs and no annotation — so there is nothing in the run to read. The block existed only to enter sdks/go-client. The module is the repository root now, so the key goes rather than getting an empty body.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Seeds this repository with the Go client that was offered upstream as omnigent-ai/omnigent#4010, at the maintainers' suggestion that it live in our organisation instead.
Why this is better than upstreaming, concretely
As a fork PR against their monorepo it could never pass CI. Every substantive check — pytest, integration, end-to-end, the Docker build — skipped behind a security gate that a fork touching CI cannot clear. Nothing meaningful had ever run against this code in the four rounds of review it went through. Here, the checks are the gate.
It also removes an indirection that was a small lie: a consumer declared a dependency on the upstream module path at
v0.0.0and rewrote it with areplaceonto a pseudo-version of a personal fork. That becomes an ordinary dependency on a tagged module.What moved
The hand-written client (
client,session,stream,event,errors,list,doc) with its tests, the two generated files, the generator, andbin/check.sh. Module path rewritten throughout; nothing references the old one.300 tests pass,
-raceincluded.bin/check.shis green — CI invokes that same script rather than reimplementing its steps, so the two cannot drift.Licensing — the one non-mechanical part
The repository was scaffolded MIT. That's wrong here.
models.gen.goandevents.gen.goare generated from a vendored copy of the Omnigent OpenAPI specification and reproduce its schema descriptions verbatim as Go doc comments. The generated half is therefore a derivative of an Apache-2.0 input owned by Databricks, Inc. So this is Apache-2.0, with aNOTICEcarrying that attribution.The NOTICE is scoped rather than copied verbatim: upstream's lists its Python runtime dependencies — pyyaml, fastapi, thirty others — none of which this module contains. Apache-2.0 §4(d) provides for excluding notices that don't pertain to a derivative work.
A test caught this.
TestModuleShipsItsOwnLicencewas written when the module was nested inside the Apache-2.0 repository, to keep the licence inside the module zip. It failed on the move to the MIT scaffold, which is the only reason the relicensing didn't happen silently. Its rationale is rewritten for the new layout rather than left describing a nesting that no longer exists.One thing for reviewers to know
The vendored spec at
spec/openapi.jsonis a snapshot, and the README says so where a reader will meet it. Several defects in the first consumer of this SDK came from reasoning about server behaviour off a stale — or simply the wrong — copy of that specification. Refreshing it should be a deliberate act with a diff to read, not a background sync.