Skip to content

feat: add Grok Build runtime (plain-mode adapter fields + registry entry)#27

Open
tynamite wants to merge 9 commits into
OpenCoven:mainfrom
tynamite:feat/grok-build-runtime
Open

feat: add Grok Build runtime (plain-mode adapter fields + registry entry)#27
tynamite wants to merge 9 commits into
OpenCoven:mainfrom
tynamite:feat/grok-build-runtime

Conversation

@tynamite

@tynamite tynamite commented Jul 17, 2026

Copy link
Copy Markdown

Adds Grok Build to the runtime registry as a plain-mode one-shot
adapter, plus the spec fields it needs — matching the recipe-only
adapter in OpenCoven/coven#410 exactly.

Spec additions (coven-runtime-spec)

  • ContinuityArgs — one-shot session continuity (init/resume prefix
    args, session_id_flag, resume_flag), mirroring coven's type.
  • prompt_flag / interactive_prompt_flag — flag-bound prompts
    (--single=<prompt>) for runtimes with no positional prompt slot.
  • Validation: continuity args must declare a usable init or resume
    launch; blank prompt flags are rejected; preassigned-session-id may be
    satisfied by the continuity path.
  • Forward compatibility: parsing tolerates unknown fields from newer
    spec versions; authoring (conjure) stays strict via
    unknown_manifest_fields / unknown_index_fields. Mutating registry
    flows load strictly; read-only registry list loads tolerantly so one
    newer entry can't make every runtime unlistable.

Registry

  • registry/runtimes/grok/1.0.0.json + examples/grok.json: the Grok
    Build adapter in plain output mode (--output-format plain, Grok's
    own headless default — final response text on stdout, errors on
    stderr, no translation layer), with sandbox argv mapping and
    continuity args identical to the coven trusted recipe.
  • Canonical index rebuilt via conjure registry build.

Deliberately not included

No event_protocol field or enum: the coven-side event bridge was
withdrawn in favor of the recipe-only adapter per maintainer review on
OpenCoven/coven#410. The generic mechanism belongs to a follow-up design
issue and can re-enter the spec with that discussion.

Verification

cargo fmt --check, cargo clippy --workspace --all-targets -- -D warnings, cargo test --workspace --locked (92 tests) all pass.
Schema and examples validated in lockstep (schema_examples suite).

claude added 7 commits July 17, 2026 13:43
…untimes)

Extends the manifest spec with the fields a finite one-shot headless
runtime needs — prompt_flag / interactive_prompt_flag (flag-bound
prompts), continuity_args (cold-start session init/resume), and
event_protocol (grok-headless-v1) — and registers Grok Build 1.0.0.

Spec changes are additive with snake_case + camelCase aliases; legacy
manifests deserialize unchanged. New validation mirrors coven's loader:
event_protocol and capabilities.stream are mutually exclusive,
continuity_args must declare a usable init or resume launch, its
session_id_flag requires preassigned_session_id, and the
preassigned-session rule now accepts a continuity session flag. JSON
Schema updated in lockstep (schema_examples drift guard extended with a
grok-shaped adapter and examples/grok.json).

The manifest matches the trusted recipe reviewed in tynamite/coven#1
byte-for-token: --single prompts, streaming-json output, --session-id /
--resume continuity, and argv-list sandbox mapping driving both
--permission-mode and --sandbox.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ct8hD3ixehKLeaSu7jGwnA
… install hint

Normalize the preassigned-session ValidationError field to the canonical
snake_case key (capabilities.preassigned_session_id) so error fields map
onto manifest JSON consistently, and reword the Grok Build install_hint
to lead with npm and the official install guide instead of embedding a
pipe-to-shell command, matching the other registry entries.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ct8hD3ixehKLeaSu7jGwnA
Per Codex review: a streaming adapter receives the pre-assigned id
through stream_args, so a continuity-only session_id_flag validated but
could never reach the streaming process. Streaming adapters now require
stream_args.session_id_flag; the continuity flag satisfies the rule only
for non-stream adapters (the Grok Build shape). Regression test added;
docs updated to match.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ct8hD3ixehKLeaSu7jGwnA
…conjure

A registry index written by a newer spec version used to fail parsing
entirely on older consumers: deny_unknown_fields on the manifest types
meant one new field poisoned every runtime in the document, and the
notify-downstream sync makes that scenario real, not hypothetical.

Parsing is now tolerant — unknown fields are ignored and an unrecognized
event_protocol value degrades to a per-adapter Unknown marker instead of
failing the index — while authoring strictness moves to the new pure
unknown_manifest_fields checker, which conjure's shared manifest loader
enforces (with a friendlier hint when the input is actually a registry
index). validate_manifest rejects Unknown protocols so authored
manifests must name a protocol their target spec knows; the sandbox
object keeps strict field matching because its untagged forms are
disambiguated by field sets. Docs cover the two-layer rule and the
v0.1.3 live-index caveat; drift-guard and forward-compat tests added.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ct8hD3ixehKLeaSu7jGwnA
Per Copilot review: stream_args and continuity_args now validate against
their own allowlists so a key nested in the wrong block is reported by
the authoring typo guard, and the adoption doc anchors the v0.1.4
behavior split to the release that first contains tolerant parsing
instead of implying the unbumped in-repo version.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ct8hD3ixehKLeaSu7jGwnA
Per Codex review, tolerant parsing had two conjure-side holes: validate
--registry silently blessed typo'd entries (the strict check only
guarded load_manifest), and load-rewrite flows like registry yank would
destructively strip newer-spec fields and collapse future protocol
values to 'unknown' when an older conjure touched a newer index.

The spec's unknown-field checker is refactored into a shared
unknown_adapter_fields (which also flags event_protocol values that
parse to Unknown), the registry crate gains unknown_index_fields
covering index/entry/adapter levels, and load_registry now refuses any
unrecognized content — typos fail validation, and mutation of a
newer-spec index fails with an upgrade hint instead of losing data.
Tolerant parsing remains the read-only consumer path.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ct8hD3ixehKLeaSu7jGwnA
…tocol schema

Per Copilot review: unknown_index_fields iterates borrowed objects
instead of cloning the runtimes map and entry arrays, the CLI loaders
deserialize from the already-parsed Value instead of parsing twice
(load_registry had the same pattern), and the schema's eventProtocol
definition accepts explicit null like every other optional scalar field.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ct8hD3ixehKLeaSu7jGwnA

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3ee734af0d

ℹ️ 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".

Comment on lines +77 to +78
let unknown = coven_runtime_registry::unknown_index_fields(&value);
if !unknown.is_empty() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep registry listing tolerant of newer indexes

When an older conjure runs registry list against an index containing a newly added adapter field or protocol, run_list calls this shared loader and the unknown-field check rejects the entire index. Listing is read-only, so this defeats the tolerant parsing introduced for read-only consumers and prevents even known runtimes from being displayed; use tolerant RegistryIndex parsing for registry list while retaining this strict check for validation and mutation flows.

Useful? React with 👍 / 👎.

Comment on lines +178 to +179
#[serde(default, alias = "promptFlag", skip_serializing_if = "Option::is_none")]
pub prompt_flag: Option<String>,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject blank prompt flags during validation

If an authored manifest sets prompt_flag (or interactive_prompt_flag) to "" or whitespace, the schema and validate_adapter both accept it, but consumers distinguish Some(flag) from the positional-prompt fallback and will construct a malformed flag-bound prompt. This makes conjure validate bless an adapter whose launches fail; validate these new optional flags as non-blank when present, as is already done for session and sandbox flags.

Useful? React with 👍 / 👎.

Per upstream Codex review on OpenCoven#27:

registry list is read-only, so it now loads the index tolerantly — one
entry written by a newer spec must not make every runtime unlistable.
Mutating and authoring flows (validate --registry, registry build,
registry yank) keep the strict loader.

prompt_flag and interactive_prompt_flag reject blank values in both the
Rust rules and the JSON Schema (minLength) — a present-but-blank flag
would make consumers emit a malformed '=<prompt>' argument instead of
falling back to a positional prompt. Regression tests for both.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ct8hD3ixehKLeaSu7jGwnA

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 85e9b45649

ℹ️ 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".

/// with no positional prompt slot (e.g. Copilot's `--prompt`, Grok Build's
/// `--single`). `None` means the prompt is the final positional argument.
#[serde(default, alias = "promptFlag", skip_serializing_if = "Option::is_none")]
pub prompt_flag: Option<String>,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Publish the public struct expansion under a breaking version

Adding this field and the other new RuntimeAdapter fields breaks every downstream struct literal and exhaustive destructuring pattern, even though the fields are optional for serde. The compatibility documentation explicitly identifies the next release as v0.1.4, but a patch update within the 0.1 series is expected to remain source-compatible; consumers can therefore receive compilation failures from an ostensibly compatible upgrade. Release this API expansion as 0.2.0, or provide a construction strategy that does not require callers to enumerate fields.

Useful? React with 👍 / 👎.

Comment on lines +279 to +280
if let Some(continuity) = &adapter.continuity_args {
if !continuity.has_init_launch() && !continuity.has_resume_launch() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Reject a blank continuity resume flag

When continuity_args contains usable init data, a present but blank resume_flag passes this aggregate check because has_init_launch() is already true and there is no per-field validation. The serialized adapter still contains Some(" "), so consumers that distinguish presence from absence can construct a malformed flag-bound resume invocation instead of using the positional/absent fallback. Reject resume_flag when present but blank, as is now done for the prompt flags.

Useful? React with 👍 / 👎.

The companion coven PR landed as a pure manifest recipe running Grok
Build's plain output mode — no event-protocol bridge in core. Align
this registry PR with that outcome:

- Drop the EventProtocol enum, its two validation rules, its schema
  definition, and the unknown-protocol-value scan; the generic
  event_protocol mechanism moves to a maintainer design issue and can
  return with it.
- Re-point the grok manifest, example, and canonical index entry at the
  shipped plain-mode contract (`--output-format plain` prefix args,
  official-installer install hint, protocol-free description).
- Keep everything the recipe actually exercises: ContinuityArgs, prompt
  flags, blank-flag validation, unknown-field strict/tolerant layers.
  Forward-compat tests that used an unrecognized protocol value as
  their fixture now exercise the same tolerance through unknown fields.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ct8hD3ixehKLeaSu7jGwnA
@tynamite

Copy link
Copy Markdown
Author

Heads-up on the rescope just pushed (cd92006): the companion coven PR
(OpenCoven/coven#410) landed its revision as a pure manifest recipe
running Grok Build's plain output mode — no event-protocol bridge in
core — per the maintainer review there. This PR now matches that
outcome:

  • Removed the EventProtocol enum, its validation rules, and its
    schema definition. The generic event-protocol mechanism moves to a
    maintainer design issue and can return with whatever shape that
    discussion settles on.
  • Realigned the grok manifest/example/index entry with the shipped
    recipe: --output-format plain prefix args, official installer hint,
    no protocol field. The canonical index entry was regenerated via
    conjure registry build.
  • Unchanged: ContinuityArgs, prompt flags, blank-flag validation,
    and the strict-authoring / tolerant-parsing unknown-field layers
    (including the earlier review fixes) — all of which the recipe-only
    adapter actually exercises. Forward-compat tests that previously used
    an unrecognized protocol value now exercise the same tolerance
    through unknown fields.

Diff is −204/+57 versus the previous head. fmt / clippy -D warnings /
full workspace tests pass.

@tynamite tynamite changed the title feat: accept Grok Build into the canonical registry (event-protocol runtimes) feat: add Grok Build runtime (plain-mode adapter fields + registry entry) Jul 18, 2026
@tynamite

Copy link
Copy Markdown
Author

@BunsDev Update now that the companion PR has landed: OpenCoven/coven#410 merged
(308f3d2) as the pure manifest-recipe variant — Grok Build running in
its plain output mode via coven adapter install grok, no
event-protocol bridge in coven core.

This PR is already rescoped to match that outcome (cd92006), removing
everything that described the withdrawn approach:

  • Dropped the EventProtocol enum, its two validation rules
    (stream-mutual-exclusion, unknown-value rejection), and the schema's
    eventProtocol definition. That generic mechanism is deferred to a
    design issue rather than living in this spec version.
  • Re-pointed the grok manifest/example/canonical-index entry at the
    shipped plain-mode contract — --output-format plain prefix args,
    official-installer install hint — field-for-field identical to what
    merged in #410.
  • Kept everything the shipped recipe actually exercises and that
    already passed review here: ContinuityArgs, prompt_flag/
    interactive_prompt_flag, blank-flag validation, the strict-authoring
    / tolerant-parsing unknown-field layers.

cargo fmt --check, cargo clippy --workspace --all-targets -- -D warnings, and cargo test --workspace --locked (92 tests) all pass.
Ready for another look whenever you have time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants