You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
chore(herdr-core): type hc calls from hc's own OpenRPC schema - #2671
Ovis's herdr-core (hc) client was typed by hand (hc.call<{ terminals: HcTerminal[] }>(...)), while herdr's own API types are generated from herdr's schema. This PR generates the hc side too, from hc's own OpenRPC document, in the same bun run generate:types run and --check.
scripts/hc-schema.json: hc schema from hc 0.1.14, the version ci.yml's herdr-core-live job pins. herdr-core's schema is Apache-2.0 (its info.license), taken straight from the hc binary.
scripts/hc-types.ts emits server/herdr-core/hc-api.generated.ts: params and results for the 13 methods Ovis calls (HC_METHODS), the records they name, and an HcMethods map. String enums widen with (string & {}) as herdr's do.
HcConnection.call / open are typed by HcMethods, so a method name, a param or a result field hc does not have is now a type error. HcTerminal, HcStatus and HcForeground derive from the generated records.
Drift: generate:types --check (Fast checks) fails when the generated file is stale; server/herdr-core/live.test.ts (the herdr-core-live job) now fails when the pinned hc publishes a schema other than the snapshot. Refresh with OVIS_HC_BIN=<hc> bun run generate:types --refresh-hc.
Things the generated types surfaced:
terminal.get answers the Terminal record itself (checked against a real hc 0.1.14). The adapter accepted a { terminal } wrapper too, and only the fake cores in adapter.test.ts sent that shape. The fakes now answer like hc does, and the adapter's hedge is gone.
server/conventions.test.ts forbids the string "events.subscribe" outside two files. The generated types, their generator and the connection's HcStreamMethod type only name hc's method, so they are exempted, and the reason is in a comment.
Note: hc 0.1.17's hc schema is byte-identical to 0.1.14's (its info.version still says 0.1.14), although herdr-core's v0.1.17 tag has a larger hc.openrpc.json. That is why the snapshot is not tied to info.version and only the live job holds it to the binary.
Design idea credit: the "generate client types from hc's OpenRPC schema and fail CI on drift" idea came from a study of another project. No code was taken from it; this follows Ovis's existing generate-protocol-types.ts.
Validation
bun run check fast: all green (workflow syntax, generated types, generated themes, typecheck, build, unit tests: 10890 pass, 0 fail).
OVIS_TEST_HC_BIN=<hc 0.1.14> bun test ./server/herdr-core/live.test.ts: 4 pass, run against an isolated core (its own HC_SESSION, state, config and runtime dirs). With one method removed from the snapshot, the new schema test fails and the other 3 pass.
bun test scripts/hc-types.test.ts: the --check drift test fails with hc-api.generated.ts is STALE, then restores the file.
Not run: Integration and browser lanes, and Windows. Nothing in them changes here, and CI runs them.
Generate server/herdr-core/hc-api.generated.ts from a snapshot of `hc schema`
(scripts/hc-schema.json, hc 0.1.14, the version the live contract pins) in the
same generate:types run and --check as herdr's wire types. HcConnection.call and
open are typed by the generated HcMethods map, and the hand-written hc record
types derive from the generated ones. The herdr-core live job now fails when the
pinned hc publishes a schema other than the snapshot.
Reviewing files that changed from the base of the PR and between 4bc043d and c564175.
Files ignored due to path filters: 1
server/herdr-core/hc-api.generated.ts is excluded by !**/*.generated.*, !server/herdr-core/hc-api.generated.ts
Files selected for processing: 17
.coderabbit.yaml
AGENTS.md
CONTRIBUTING.md
changelog.d/hc-api-types.md
docs/development.md
scripts/generate-protocol-types.test.ts
scripts/generate-protocol-types.ts
scripts/hc-schema.json
scripts/hc-types.test.ts
scripts/hc-types.ts
server/conventions.test.ts
server/herdr-core/adapter.test.ts
server/herdr-core/adapter.ts
server/herdr-core/connection.ts
server/herdr-core/live.test.ts
server/herdr-core/snapshot.ts
server/session-history-routes.ts
Summary
Summary by CodeRabbit
Improvements
Calls to herdr-core are now checked against types generated from its schema, helping catch mismatches between the integration and the pinned hc version.
Generated type checks now cover both herdr and herdr-core.
Documentation
Updated contributor and development guidance for generating and refreshing both sets of types.
The protocol generator now produces and checks herdr and herdr-core types. The herdr-core connection, snapshot types, and adapter calls use schema-derived hc method and result types. Tests and development guidance cover schema validation and type refresh.
Changes: HC schema type generation
Layer / File(s)
Summary
Generate types from the HC schema scripts/hc-types.ts, scripts/hc-types.test.ts
A new generator creates deterministic TypeScript declarations for listed hc methods and referenced schemas. Tests cover schema shapes, required and optional parameters, missing methods, and deterministic output.
Integrate HC schema generation and checks scripts/generate-protocol-types.ts, scripts/hc-types.test.ts, server/herdr-core/live.test.ts, .coderabbit.yaml, AGENTS.md, CONTRIBUTING.md, changelog.d/hc-api-types.md, docs/development.md
The protocol generator now loads or refreshes the hc schema and generates or checks both type outputs. Tests compare the pinned binary schema with the snapshot and verify stale generated types are reported. Documentation and review configuration identify the generated file and refresh process.
HcConnection uses method-specific parameters and results, and snapshot aliases derive from generated hc records. Adapter calls and tests use inferred results; the convention test allows the additional events.subscribe references.
devswha/ovis#2442: Adds the herdr-core adapter and HcConnection that this change types from the hc schema.
devswha/ovis#2445: Adds the pinned hc live contract test and CI job that this change extends with schema snapshot comparison.
devswha/ovis#2449: Adds adapter operations for hc terminal and lifecycle methods whose result shapes this change generates.
Pre-merge checks: 4 · 1
Failed checks: 1 warning
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 4 passed
Check name
Status
Explanation
Title check
The title names the main change: hc calls are now typed from hc's own OpenRPC schema. The chore(herdr-core) prefix fits the change. The title is short, specific, and matches the raw summary and object…
Description check
The description covers the change, the generated files, the drift check, the refresh command, and the validation results. It follows the template's Change and Validation sections. It does not link an …
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Title check
Explanation
The title names the main change: hc calls are now typed from hc's own OpenRPC schema. The chore(herdr-core) prefix fits the change. The title is short, specific, and matches the raw summary and objectives.
Full details: Description check
Explanation
The description covers the change, the generated files, the drift check, the refresh command, and the validation results. It follows the template's Change and Validation sections. It does not link an issue, and the Change section does not mention the template's contract-test note for HTTP/WS changes. These gaps are minor, and the description is mostly complete.
Full details: Docstring Coverage
Explanation
Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 12 functions across 10 files. (5 skipped: 5 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches
Generate docstrings
Commit to this branch
Create a new PR
Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Review coverage is incomplete: 1 file could not be fully reviewed. Findings from completed review steps are included; see review info for details.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Reviewing files that changed from the base of the PR and between 4bc043d and 33aed87.
Files ignored due to path filters: 1
server/herdr-core/hc-api.generated.ts is excluded by !**/*.generated.*, !server/herdr-core/hc-api.generated.ts
Files selected for processing: 16
.coderabbit.yaml
AGENTS.md
CONTRIBUTING.md
changelog.d/hc-api-types.md
docs/development.md
scripts/generate-protocol-types.ts
scripts/hc-schema.json
scripts/hc-types.test.ts
scripts/hc-types.ts
server/conventions.test.ts
server/herdr-core/adapter.test.ts
server/herdr-core/adapter.ts
server/herdr-core/connection.ts
server/herdr-core/live.test.ts
server/herdr-core/snapshot.ts
server/session-history-routes.ts
Files not fully reviewed because their changes were unavailable: 1
scripts/hc-schema.json
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.
- A union with a variant that admits any value now renders as unknown,
instead of dropping that variant and narrowing the type.
- The hc drift test moves into generate-protocol-types.test.ts: both files
rewrite the generated outputs, and tests of one file run one at a time
while separate files may run side by side.
Adversarial review (Codex, gpt-6.1-terra, xhigh) at 33aed87. What it found and what changed in c564175:
MEDIUM, fixed: a schema anyOf with an any-value variant ({}) was narrowed to its typed variants (anyOf: [string, {}] gave string). I reproduced it with the reviewer's command, and such a union now renders as unknown. The generated file does not change, because hc 0.1.14's schema has no such union. A test covers it.
MEDIUM, fixed: the hc drift test in scripts/hc-types.test.ts wrote a sentinel into the generated file, and scripts/generate-protocol-types.test.ts rewrites both outputs, so the two files could race in parallel unit workers. The drift test now lives in generate-protocol-types.test.ts, whose tests run one after another.
MEDIUM, not changed: the schema-vs-binary check runs only in the herdr-core live contract job, which is not one of the three required checks. That job already guards the hc client this way, and this PR only adds a test to it. Making it required is a branch-protection decision for the maintainer.
LOW, not changed: enums widen with (string & {}) in request params too (terminal.send's if), so an invalid value still typechecks. This follows herdr's generator. Narrowing input-only enums can come later if wanted.
Checks: bun run check fast passed at c564175 (unit 10891 pass, 0 fail).
Merge note: CodeRabbit was rate-limited for the latest head (c564175), so this merges on the omo review (CLEAR receipt at c564175), the Codex Terra adversarial review (2 reproduced MEDIUMs fixed; 1 MEDIUM - the live drift job is not a required check - and 1 LOW left as notes) and green required checks.
…echeck (#2693)
## Summary
origin/main (12a7c63) fails `bun run typecheck` with four errors in
`server/herdr-core/adapter.ts`. #2671 typed `HcConnection.call` and
`open` by hc method name, generated from hc's own schema. #2674, merged
right after it, added a `hc.open<{ channel: number; terminal?:
HcTerminal }>("terminal.attach", ...)` and a `hc.call<{ text: string
}>("terminal.read", ...)` written for the old untyped signature, where
the type argument was the result type.
This drops those two type arguments. The method name now types both
calls: `terminal.attach` gives `AttachResult` (whose `terminal.rows` the
code reads) and `terminal.read` gives `ReadResult` (whose `text` it
reads). Nothing changes at runtime.
No changelog fragment: users notice nothing.
## Validation
- `bun run typecheck`: clean (four errors on main before).
- `HERDR_TEST_MODE=unit bun test server/herdr-core`: 90 pass, 0 fail.
- `bun run check fast`: not run locally. The heavy-run slots were all
held by other lanes, so the lead chose to let CI's Fast checks gate this
PR.
- Not run: Integration and browser lanes, and Windows. CI runs them.
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
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Summary
Ovis's herdr-core (
hc) client was typed by hand (hc.call<{ terminals: HcTerminal[] }>(...)), while herdr's own API types are generated from herdr's schema. This PR generates the hc side too, from hc's own OpenRPC document, in the samebun run generate:typesrun and--check.scripts/hc-schema.json:hc schemafrom hc 0.1.14, the version ci.yml'sherdr-core-livejob pins. herdr-core's schema is Apache-2.0 (itsinfo.license), taken straight from thehcbinary.scripts/hc-types.tsemitsserver/herdr-core/hc-api.generated.ts: params and results for the 13 methods Ovis calls (HC_METHODS), the records they name, and anHcMethodsmap. String enums widen with(string & {})as herdr's do.HcConnection.call/openare typed byHcMethods, so a method name, a param or a result field hc does not have is now a type error.HcTerminal,HcStatusandHcForegroundderive from the generated records.generate:types --check(Fast checks) fails when the generated file is stale;server/herdr-core/live.test.ts(theherdr-core-livejob) now fails when the pinned hc publishes a schema other than the snapshot. Refresh withOVIS_HC_BIN=<hc> bun run generate:types --refresh-hc.Things the generated types surfaced:
terminal.getanswers theTerminalrecord itself (checked against a real hc 0.1.14). The adapter accepted a{ terminal }wrapper too, and only the fake cores inadapter.test.tssent that shape. The fakes now answer like hc does, and the adapter's hedge is gone.server/conventions.test.tsforbids the string"events.subscribe"outside two files. The generated types, their generator and the connection'sHcStreamMethodtype only name hc's method, so they are exempted, and the reason is in a comment.Note: hc 0.1.17's
hc schemais byte-identical to 0.1.14's (itsinfo.versionstill says 0.1.14), although herdr-core'sv0.1.17tag has a largerhc.openrpc.json. That is why the snapshot is not tied toinfo.versionand only the live job holds it to the binary.Design idea credit: the "generate client types from hc's OpenRPC schema and fail CI on drift" idea came from a study of another project. No code was taken from it; this follows Ovis's existing
generate-protocol-types.ts.Validation
bun run check fast: all green (workflow syntax, generated types, generated themes, typecheck, build, unit tests: 10890 pass, 0 fail).OVIS_TEST_HC_BIN=<hc 0.1.14> bun test ./server/herdr-core/live.test.ts: 4 pass, run against an isolated core (its ownHC_SESSION, state, config and runtime dirs). With one method removed from the snapshot, the new schema test fails and the other 3 pass.bun test scripts/hc-types.test.ts: the--checkdrift test fails withhc-api.generated.ts is STALE, then restores the file.