-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(opencode-go): normalize tool catalogs and stateless continuation #3986
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f41f4be
8e7cfc7
89b69a0
9b42c1a
d1f61e9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| # Six-item manual integration stack | ||
|
|
||
| ## Loop specification | ||
|
|
||
| - Archetype: satisfy the six named bug contracts, with a docs-only roadmap cycle followed by six implementation cycles and one integration cycle. | ||
| - Trigger: owner request on 2026-09-08 to use one stack, repeated PABCD, Astra high delegates, no local suites, no-verify pushes, and merge through dev. | ||
| - Goal: Go/xAI child-result compatibility, separated V2 guidance and server-owned presets, and correctly scoped reset-credit recovery. | ||
| - Non-goals: releases, main/preview, account changes, real credit consumption, unrelated cleanup, native GitHub stacks, local product tests/install/typecheck/build. | ||
| - Verifier: independent source audits and GitHub `ci.yml` at each candidate head; final dispatch `lane=all`. Docs-only verification checks numbered artifacts and whitespace without running product code. Every activation fixture and observable result is specified in the phase designs. | ||
| - Stop: every named item has a fresh terminal disposition, all nonempty layers have landed through reviewed PRs, final hosted gates pass, and fetched ancestry plus landed-tree comparison prove integration. | ||
| - Artifacts: this numbered unit, ignored `.tmp/bug6-01a07e9d/`, and session-bound `.codexclaw` ledger/receipts. New unpublished security analysis stays in scratch only. | ||
| - Outcomes: DONE requires all evidence; NOOP requires proof the full named contract already landed; unresolved work remains pending; genuine external blockers are reported without inventing proof. | ||
| - Escalation: main reclaims failed delegate slices after two distinct failures. Delegation is preplanned below. No further user approval is needed for the explicitly authorized pushes/merges. Permission/ruleset changes remain outside scope. | ||
| - Resources: existing repository/GitHub access, synthetic fixtures, gpt-6-astra high leaf agents within host concurrency. No user-set token/cost/time limit; no extra resource budget is invented. Bounded external polls and owned-job cleanup. | ||
|
|
||
| ## Baseline and ownership | ||
|
|
||
| Initial integration base: `9e1468d4b7a41b498ed2aca98507ada2c741afea`. | ||
| The session remains in its managed worktree and adopts branch `codex/bug6-01a07e9d-roadmap` in place. Thirty pre-existing document files are fingerprinted in ignored scratch; they are excluded from every commit. Main owns Git operations, plan/FSM, CI decisions and merges. Source investigation lanes are disjoint Go/xAI, V2, and credit pairs. B-stage workers receive only the current audited phase scope, never speculative product-write authority. | ||
|
|
||
| ## Work-phase map | ||
|
|
||
| | Cycle | Source | Deliverable | Delivery dependency | | ||
| | --- | --- | --- | --- | | ||
| | wp0 | This roadmap | Audited full file/behavior map and verification contracts | none | | ||
| | wp1 | PR #3838 | Remaining Go private-input compatibility and regression coverage | wp0 | | ||
| | wp2 | Issue #3907 | Strict xAI child-result continuation | wp1 normalization contract | | ||
| | wp3 | PR #3944 | Separate proxy V2 guidance from native policy | wp2, owner-requested cumulative chain | | ||
| | wp4 | PR #3951 | Server-owned proactive preset and dashboard semantics | wp3 policy vocabulary | | ||
| | wp5 | PR #3965 | Canonical reset operation alias settlement | wp4, owner-requested cumulative chain | | ||
| | wp6 | Issue #3973 | Account/scope/generation-bound post-reset recovery | wp5 operation identity | | ||
| | wp7 | All six | Exact-head CI, UI artifact observation, bottom-up landing and original dispositions | wp1–wp6 | | ||
|
|
||
| The owner explicitly chose a single chain across otherwise independent domains. Each PR contains only its own layer. No empty product PR is created for already-landed work; its verified NOOP record remains in the chain. Revalidate each decade design at its P boundary. Preserve contributor commit identity/trailers on every carried implementation. | ||
|
|
||
| ## Verification and landing contract | ||
|
|
||
| `ci.yml` accepts all pull-request bases (`pull_request: {}`); manual child PRs therefore receive product CI. Its gates job runs typecheck, dashboard tests, privacy scan and the relevant build. Windows/control coverage is dispatched explicitly. A green hygiene or enforce-target check is not product evidence, and skipped/cancelled jobs are not passing tests. Runtime validation stays hosted; local checks are NOT RUN by owner instruction. | ||
|
|
||
| Before each merge refresh head/base, membership, reviews, required checks and actor. Authenticated actor `lidge-jun` has admin permission (live preflight); MAINTAINERS.md permits recorded maintainer integration into dev without self-approval, while retaining outstanding maintainer objections and security review. Merge bottom-up, retarget the next owned child to dev, preserve branches while referenced, and revalidate the resulting base/head. Do not alter source authors' branches. Close only fully resolved source items after landing. | ||
|
|
||
| ## Continuity ledger | ||
|
|
||
| - wp0 P: live source intake and complete decade designs in progress; no product changes. | ||
| - wp0 A: independent Astra high reviewer returned PASS, zero blockers. Full source appendices remain in ignored scratch. An absent REST stack field means unknown membership, not proven absence; inspect the stacks endpoint before delivery. | ||
| - wp0 B/C handoff: all eight numbered roadmap documents are complete. Structural validation passed with 30 pre-existing user files preserved. Next cycle is wp1 Go residual implementation. Candidate cycles c1–c6 require their scoped audited delta and matching-head PR CI; c7 retains all six terminal dispositions and final integration proof. | ||
|
Comment on lines
+44
to
+46
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win Use one authoritative
🤖 Prompt for AI Agents |
||
| - Remote documentation verification uses isolated `macmini-cf` scratch, not the deploy-docs workflow. Existing Node 24.20.0 is available under the remote user's nvm tree; select the repository-pinned Bun in that scratch environment and record actual versions. No live service or account state is touched. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,40 @@ | ||
| # wp1: Go compatibility residuals | ||
|
|
||
| Depends on wp0. C3 with independent boundary review. Source PR #3838 head `d84e5a80a5e40a65462a0466d82cdcec463a847e`; baseline dev `9e1468d4b7a41b498ed2aca98507ada2c741afea`. #3942 already landed the generic array agent-message normalizer. Reimplement the remaining Go behavior on current owners; do not restore the removed `opencode-go.ts` or duplicate namespace logic. | ||
|
|
||
| ## Main decisions | ||
|
|
||
| Carry Go additional-tools placement and the canonical preset's stateless intent. Retain current all-parts readable/fail-closed agent-message behavior; the old lossy mixed-content hunk is deliberately declined because successful transport alone does not prove complete task content. Original PR disposition must name that decision rather than claim every historical hunk landed. Custom renamed providers retain explicit configuration semantics; no destination-based configuration migration is added. | ||
|
|
||
| ## File changes | ||
|
|
||
| - NEW `src/adapters/opencode-go-additional-tools.ts`: export a small immutable placement helper taking body and base URL. Match HTTPS `opencode.ai`, standard port, exact `/zen/go/v1` (optional terminal slash); reject credentials/query/fragment and other paths. For valid `additional_tools` array wrappers append their already-normalized tools to top-level tools and remove the wrapper. Preserve unrelated input and supported nameless hosted tools. Non-array malformed wrappers remain unchanged; no valid wrappers returns the original body. Existing namespace/custom owners perform identity lowering and dedupe before this pass. | ||
| - MODIFY `src/adapters/openai-responses.ts`: import helper; invoke only inside non-forward dispatch after existing namespace/custom/search lowering around baseline line 2455, before code-mode/compaction and later hosted-tool pruning. Response alias maps stay owned by prior normalization. | ||
| - MODIFY `src/providers/registry.ts`: canonical `opencode-go` entry gains `statelessResponses: true`. Existing derive logic seeds/backfills only absent values; explicit false remains authoritative. | ||
| - MODIFY `tests/providers/opencode-go-grok46-responses.test.ts`: replace the old expected private wrapper with promoted tools; cover duplicate containers, distinct namespace same-name children, custom/function handling, hosted Luna search versus Go Grok denial, tool_choice none/allowed list, tool_search_output activation, forward/Zen/lookalike/wrong-port exclusion and immutable replay. | ||
| - MODIFY `tests/providers/opencode-go-luna-wire.test.ts`: cover seed/backfill/false, full-history continuation with synthetic reasoning and paired tool results; assert previous_response_id removed, store false, call pairing/history retained. Cover stateless orphan and reasoning-summary interactions through existing focused suites. No new test file is required if these current owners remain reviewable. | ||
| - MODIFY `docs-site/src/content/docs/reference/configuration/providers.md` and `structure/04_transports-and-sidecars.md`: record Go wrapper placement and canonical stateless default with explicit override and full-history limits. Update only contradicting translated statements. | ||
|
|
||
| ## Before / after flow | ||
|
|
||
| Before: namespace normalization leaves valid declarations inside `input.additional_tools`; strict Go receives a private wrapper. After: the same normalized declarations appear in `tools`, and valid wrappers are removed. Before: canonical Go may forward previous_response_id with replay history. After: existing stateless normalization strips the stored-continuation parameters and sends complete history. | ||
|
|
||
| ## Activation and observable coverage | ||
|
|
||
| Use production adapter fixtures, not a duplicate normalizer oracle. Namespace alpha.lookup and beta.lookup must both remain callable; duplicate wire identities follow the existing canonical owner. Nameless hosted tools must survive placement until provider/model pruning. Malformed wrapper, unapproved destination and forward controls stay byte-identical. Seed false must differ from default true. A two-turn synthetic continuation must preserve meaningful reasoning/tool history while removing stored-state references. Inspect existing stateless orphan repair and summary tests; extend any missing Go model coverage without weakening assertions. | ||
|
|
||
| Hosted verification: PR CI covers changed runtime and provider suites, with final full dispatch before integration. Local product tests/install/typecheck/build are NOT RUN. Preserve original PR account-linked Co-authored-by credit; resolve identity from GitHub before commit. The full source investigation is in ignored `.tmp/bug6-01a07e9d/go-xai-plan.md`; it is not public implementation proof. | ||
|
|
||
| ## wp1 P refresh | ||
|
|
||
| Previous wp0 D directs Go residual implementation. During live refresh dev advanced to c15662855 (#3975), changing only tests/codex-integration/codex-prompt-text-probe.test.ts. Hook-disabled merge incorporated that unrelated probe fixture correction before B; Go owners and this design are unchanged. The initial A narrative said unchanged dev based on the pre-fetch snapshot; this entry corrects it. | ||
|
|
||
| ## C audit foldback and repair plan | ||
|
|
||
| Independent review at 9b42c1a80 found two blockers. F1 accepted: the stateless flag enables content-to-summary output normalization, but the continuation cache records original output; full-history overlap then fails. The adapter-only full-history fixture bypassed the affected server boundary. F2 accepted: baseUrl-only matching misses split/endpoint-inclusive configurations and can affect an overridden non-Go resource. Neither finding conflicts with preserving opaque items or existing fail-closed policy. | ||
|
|
||
| Repair F1: MODIFY `src/server/responses/core.ts` at `rememberPassthroughResponseChecked` only. After current namespace/custom/function restoration, apply existing `rewriteReasoningSummaryInJson` under the same `hideThinkingSummary !== true && routeUsesContentChannelReasoning(provider, model)` condition as client output, then record that representation. Preserve item content and IDs under the existing opaque-item rule; do not weaken overlap comparison or use ID-only matching. This aligns stored output with the actual client serialization for SSE and JSON. Extend the current Go server fixture to send actual full-history plus previous_response_id and assert each prior call/message occurs exactly once; retain delta replay and hiding/opaque controls. The shared callback is an explicit narrow scope expansion required by this newly activated path, not unrelated state refactoring. | ||
|
|
||
| Repair F2: the helper now accepts the final resolved Responses request URL already built by the adapter. Match exact origin and `/zen/go/v1/responses`, rejecting userinfo/query/fragment. Positive fixtures cover normal base, endpoint-inclusive base and split custom path; negative fixtures cover an override resolving to Zen/non-Go and assert both actual request URL and body. Update destination wording in docs and preserve all prior host/port/immutability controls. | ||
|
|
||
| Re-review the repaired diff with the same implementation auditor; retain CI failures and repair evidence. No local product commands are authorized. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| # wp2: xAI string child-result continuation | ||
|
|
||
| Depends on wp1 current outbound placement and full-history regression controls. C3. Issue #3907 posts string `agent_message.content`; #3942 already implements arrays for all non-forward destinations. Scope is the string residual only. | ||
|
|
||
| ## File changes | ||
|
|
||
| - MODIFY `src/adapters/routed-agent-messages.ts`: extend `normalizeRoutedAgentMessages` with optional `{ allowStringContent?: boolean }`, default false. If enabled and content is a nonblank string, create one input_text part containing the exact original string. Existing attribution and array handling continue. Whitespace-only, unknown, malformed and ciphertext shapes remain unchanged; never trim the forwarded text or mutate the input. | ||
| - MODIFY `src/adapters/openai-responses.ts`: reuse `isXaiResponsesDestination` from `src/providers/xai-transport.ts`, pass its result as allowStringContent inside the existing `!forward` call. Existing array behavior stays available for other non-forward destinations. No custom-forward exception. | ||
| - MODIFY `tests/adapters/routed-agent-messages.test.ts`: exercise both exact xAI hosts, API-key and OAuth, exact text/newlines, attribution, missing transport item ID and input immutability. String controls: native/custom forward, other providers, lookalike hosts and blank content stay unchanged; existing all-parts array/ciphertext tests remain. | ||
| - MODIFY `tests/server/server-xai-responses-streaming.test.ts`: extend the synthetic server fixture with parent request, child request, then parent continuation containing string child result plus genuine paired tool history. Upstream stub rejects surviving private agent_message with 422; assert user-message child text, ordinary response completion, paired calls preserved and no repeated incompatible dispatch. This exercises the wire boundary, not the actual Codex scheduler. | ||
| - MODIFY `docs-site/src/content/docs/reference/adapters.md`, `docs-site/src/content/docs/reference/configuration/providers.md`, and the contradicting Russian adapters paragraph: describe existing non-forward array conversion and xAI string extension, preserving forward/encrypted exclusions. Sync `structure/04_transports-and-sidecars.md` without broadening the passive manifest claims. | ||
|
|
||
| ## Before / after | ||
|
|
||
| Before the raw-body outbound normalizer requires array content and leaves the issue's string item on the strict xAI wire. After it produces `{type: message, role: user, content: [{type: input_text, text: originalText}]}` through the existing attribution rules, only for an approved non-forward xAI destination. No tool result is synthesized and no encrypted message is partially discarded. | ||
|
|
||
| ## Verification | ||
|
|
||
| Pin parent/child fixtures to synthetic input. The strict upstream stub must reject the pre-fix request shape and accept the normalized one; destination-negative controls prove the guard is active. Hosted PR CI and final full dispatch execute adapter/server regressions. Local tests/install/typecheck/build remain NOT RUN. Source audit checks raw-body call placement and all consumers of the added option. There is no serialized configuration field or migration: option creation and consumption are both in-memory adapter calls. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Require the docs-site build in the verification contract.
docs-site/AGENTS.mdrequirescd docs-site && bun install --frozen-lockfile && bun run buildand forbids passing documentation validation without a successful build. Line 9 only requires artifact and whitespace checks..github/workflows/ci.ymldoes not builddocs-site, and its expensive jobs skip docs-only changes. Add the required command and require its success before reporting documentation validation as passed.🤖 Prompt for AI Agents