docs(example): add dual runtime tutorial - #96
ben-ranford wants to merge 7 commits into
Conversation
ben-ranford
added
area:core
bc629e1 to
ca66611
Compare
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: ca66611973
ℹ️ 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.
🟡 Changes recommended
Unresolved critical and moderate correctness issues remain in the tutorial and its tests.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds an unreleased local-checkout tutorial for running a counter application through line and JSONL agent hosts.
Changes:
- Adds the dual-runtime tutorial application and lifecycle tests.
- Adds the
stave-dual-runtimecommand. - Updates adoption documentation and generated inventories.
File summaries
| File | Summary and review findings |
|---|---|
scripts/rigor/generated/public-api.txt |
Updates the public API inventory. |
scripts/rigor/generated/dependency-inventory.json |
Updates the dependency inventory. |
examples/dualruntime/dualruntime.go |
Adds the tutorial implementation. Findings: critical (1) human input bypasses the registered action and authorization boundary; moderate (3) rendering may race event application, and the action is not idempotent; moderate (1) the reducer should match the action ID; nit (3) use typed action registration. |
examples/dualruntime/dualruntime_test.go |
Adds runtime parity and lifecycle tests. Moderate (1): concurrent agent requests can make the sequence assertion flaky. |
docs/client-adoption.md |
Adds adoption guidance. Nits: add source/output links (3), correct configuration guidance (2), add a changelog entry (3), link security and validation resources (1), and correct the one-session claim (1). |
cmd/stave-dual-runtime/main.go |
Adds the tutorial command entry point. |
Review details
Suppressed comments (4)
docs/client-adoption.md:27
- This refers to security and stale-target contract tests but does not link either resource, despite the tutorial's stated requirement to direct adopters to those contracts. Link the security guide and the relevant agent target-validation tests here.
and the existing security and stale-target contract tests before adapting the
docs/client-adoption.md:21
- This says the command shows one application session used by both hosts, but
mainselects exactly one branch with-agent, and the test constructshumanAppandagentAppseparately. As written it only demonstrates equal hashes from two sessions; either compose both hosts around oneApplication/session or revise this claim and the acceptance wording.
The unreleased `examples/dualruntime` package and `stave-dual-runtime` command
show one application session used by both hosts. From a local checkout run the
human line host with `printf 'inc\n' | go run ./cmd/stave-dual-runtime`, or the
JSONL host with `printf '{"jsonrpc":"2.0","id":1,"method":"stave.initialize"}\n{"jsonrpc":"2.0","id":2,"method":"stave.initialized"}\n{"jsonrpc":"2.0","id":3,"method":"stave.action.invoke","params":{"callId":"inc","actionId":"example.increment.v1"}}\n' | go run ./cmd/stave-dual-runtime -agent`.
examples/dualruntime/dualruntime.go:57
- This condition increments for every
ActionInvokedevent, regardless ofActionID; any additional action or directly submitted action event would mutate the counter. Match the payload's action ID toexample.increment.v1(and keep unrelated actions out of this reducer branch).
if ev.Kind == event.ActionInvoked || (ev.Kind == event.Text && ev.Payload.(event.TextPayload).Text == "inc") {
examples/dualruntime/dualruntime_test.go:67
- The agent server runs non-control requests concurrently, so the snapshot and action lines have no ordering guarantee. Because
invokeenqueuesActionInvokedasynchronously, the snapshot can observe sequence 2 and make the hard-coded"sequence":1assertion flaky; serve the handshake/snapshot separately before invoking or synchronize on the request-specific response.
in := strings.NewReader("{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"stave.initialize\"}\n{\"jsonrpc\":\"2.0\",\"id\":2,\"method\":\"stave.initialized\"}\n{\"jsonrpc\":\"2.0\",\"id\":3,\"method\":\"stave.snapshot\",\"params\":{\"mode\":\"full\"}}\n{\"jsonrpc\":\"2.0\",\"id\":4,\"method\":\"stave.action.invoke\",\"params\":{\"callId\":\"inc\",\"actionId\":\"example.increment.v1\"}}\n")
- Files reviewed: 4/6 changed files
- Comments generated: 7
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Also addressed the four suppressed findings in 11aef5c: the reducer matches the registered action ID, the test separates handshake/snapshot from invoke, the guide links security and target-validation contracts, and its wording describes alternative hosts using the same application definition with separate sessions for parity tests. Full |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11aef5cc6b
ℹ️ 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: b188c45757
ℹ️ 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: 06b107cb41
ℹ️ 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".
06b107c to
7d22f39
Compare
|
Summary
Adds a compiled local-checkout tutorial that runs one application session through real line and JSONL agent hosts.
Validation
printf 'inc\n' | go run ./cmd/stave-dual-runtimeGO=go go test -race ./examples/dualruntime -count=1GO=go make ci→ exit 0; log:/tmp/stave-issue64-bc629e1-make-ci.logRelease Notes
Unreleased local-checkout tutorial API and command; no published release contract changes.
Scope firewall
Only the dual-runtime tutorial command, example tests, adoption documentation, and generated inventories are changed. No transport, adapter, or framework changes.
Closes #64