Repository navigation
feat: add MiniMax Code TUI and gated ACP Chat binding - #6520
nikhilachale wants to merge 47 commits into
Conversation
🏆 Review leaderboardOct 3–10, 2026 · UTC
Ranked by distinct external PRs reviewed, then review rounds, then PR comments. Self-activity and bot activity are excluded. Show 6 more reviewers
|
|
Preserved AO failure evidence, attached at AO attempt 001 visibly refused the initial turn because the original hook resolved the wrong profile from MiniMax's plugin cache. Capture record · provenance.
AO attempt 002 shows the successful initial proof and second edit, then a retained cancellation draft after exact restore. Its frozen result remains cancellation FAIL (45 seconds) and empty-input readiness BLOCKED. Gate summary and capture record · provenance.
Current head includes the reviewed composer detection fix; focused tests, pinned lint, and captured-output checks passed. A separately timestamped one-Ctrl+C control confirmed audit draft clearing on the same native session. Fresh complete lifecycle validation is pending. AO auth remains configured/unverified, with the strict authentication gate BLOCKED. |
|
Final live audit evidence at functional source AO004: 28 PASS, 1 authentication BLOCKED, no FAIL/NOT_RUN; strict overall BLOCKED, diagnostic-only. mcode 0.6.5 / Z.ai / glm-5.3-flash. AO session All functional gates passed: initial/second mutation, hidden/project instructions, activity, native identity, observed active-turn cancellation, guarded one-Ctrl+C draft cleanup with exact recovery-file presence/removal, confirmed termination, same-ID/workspace restore, initialized empty composer, continuation/history/file preservation, fresh restore-only instructions and model agreement. Provider success does not verify the auth probe. Cancellation is not proof that an already-running subprocess was reaped. Actual retained restored session captured in AO Electron on this VPS at 2026-10-10T14:53:58.011881Z using scrot on owned Xvfb; UI source matches functional source. PNG SHA-256
AO003 remains cancellation FAIL, cleanup BLOCKED without input, kill/restore NOT_RUN. Native Stopped/draft was visible but raw Ring.Tail omitted the unterminated footer; AO remained active. Final correction uses existing rendered-surface capability with current tail-fenced cues and no stale fallback. Captured 14:33:22.726970Z, functional/UI source
Both revision-pinned PNG URLs returned HTTP 200 and matched local SHA-256. Exact stored provider-key scan: 46 intended files, zero matches; git diff --check passed. Owned AO004 session termination confirmed through API, then exact verified daemon/Electron stopped. Original audit report was not overwritten. Sources, profiles, reports and captures retained. Final validation: exact evidence head The single final local |
i-trytoohard
added
comp/daemon
nikhilachale
left a comment
nikhilachale
left a comment
There was a problem hiding this comment.
Read-only review of head e83c478. Changes are warranted for the three P2 findings below; I do not recommend approval at this head. I traced the adapter through session launch/restore, hook projection, lifecycle precedence, terminal observation, model/install metadata, generated API declarations, and SQLite migration/reconciliation, and compared native contracts with the pinned MiniMax v0.6.5 source (309e28554ec091bb17b6c4f67111c8c01d688b94). No migration-number collision was found against current main fbce287. No local tests, builds, linters, or validation suites were run, as requested. This is a COMMENT review because the authenticated GitHub identity is the PR author, which cannot submit a formal request-changes/approval review on its own PR.
|
Harness-specific reports, JSON receipts, screenshots and provenance now live on this integration PR branch, rather than the skill PR #6509. Evidence index with embedded screenshots and JSON links. Historical successes, failures and scope limitations remain as recorded. Moving these files does not rerun native/AO testing or upgrade any verdict. This is evidence relocation only. Existing tested source/native hashes and failures are preserved; the documentation commit is not a fresh runtime pass. Applicable checks run in PR CI.
|
Preserve harness-owned evidence and merge current canonical API, storage, and native Chat updates. Runtime proof remains qualified by its original source.
|
Canonical main conflicts reconciled at Validation is PR CI only; current-head checks are pending until GitHub reports them. This merge does not constitute a new native/AO run or promote historical fresh/restore/provider-role proof. Harness evidence stays on this integration PR; PR6509 remains reusable guidance and linked summaries. |
Build the real daemon in PR CI for isolated VPS runtime verification without local validation suites or dependency installations. Record source revision, Go build info, and executable hash.
|
Canonical main conflicts reconciled at Validation is PR CI only; current-head checks are pending until GitHub reports them. This merge does not constitute a new native/AO run or promote historical fresh/restore/provider-role proof. Harness evidence stays on this integration PR; PR6509 remains reusable guidance and linked summaries. |
MiniMax Chat qualification — cleanup head Runtime evidence below was captured at Chat is implemented as a gated candidate, not enabled in the shipping registry. PR remains draft. The current pinned MiniMax Code 0.6.5 has a reproducible cancellation/readiness failure. The existing shared AO Chat renderer is reused; no provider-specific React screen or renderer-side ACP client was added. Source scope: base Native evidence at the pre-cleanup runtime revisionNative run at f631d336e1 · original log/format artifact (14-day retention; essential receipts retained below).
Essential f631d33 log receipts: The native cancelled result can arrive before its underlying active-turn lease releases. Pinned ACP completion/cancel handlers, native lock rejection and controller release explain the observed ordering. No authoritative root-idle fence was found in the advertised ACP methods. A text-matched retry would require discarding an uncommitted host failure receipt and create a crash-recovery ambiguity; no sleep, silent requeue or broad retry machinery was added. A corrected native contract/version must be independently qualified before enabling Chat. Newer published versions were not executed or declared compatible. Reproduce on an authorized Linux host with official0.6.5, Node24 and Python3 installed, from AO_NATIVE_MINIMAX_ACP=1 go test -race -count=1 -v -timeout=8m \
./internal/adapters/chatdriver/minimaxacp \
-run '^TestNativeMiniMaxACP(Conformance|ExactRestoreConformance)$'Required AO screenshots and live journeysNo current-head AO UI screenshots were captured. Local app/provider execution is excluded by the user's execution-host constraint; remote CI did not launch an actual AO UI. Chat admission remains blocked by the native failure. Earlier TUI images are not relabeled as current Chat proof.
Preserved earlier attemptsFirst checkpoint retains the initial workflow/model failures. Later attempts remain separate:
New runtime evidence remains in PR comments/CI attachments and owned temporary paths, not committed audit reports, logs, JSON receipts or screenshots. Source contract docs and reusable synthetic tests remain normal source. Legacy TUI evidence and duplicate contract docs were removed from the current diff after all 41 immutable source destinations were verified against the original bytes/hashes. Historical migration index preserves their original failure labels and limited scope. Cleanup targets only owned test sessions/profiles; this does not qualify the separate real AO cleanup/UI journey. Completed pre-cleanup CI receiptAll workflows below completed on
The source worktree is clean and the PR remains draft. No local execution or complete integration/production-readiness PASS is claimed. Native failure, registered AO/API, real authorization and actual UI/screenshot gaps remain explicit above. Cleanup-head CIHead
The cleanup-head native log repeats the original blocker after successful stream/context, approve/reject and exact tool cancellation: immediate continuation at |
nikhilachale
changed the title
|
Historical evidence relocation before PR cleanup This cleanup removes 41 PR-added files / 10,930 added text lines, including 8 historical PNGs, from the current diff:39 audit/evidence/index files and2 duplicate research/integration documents. Runtime behavior and tests are retained. The native0.6.5 cancellation/readiness blocker remains; Chat is still unregistered and the PR stays draft. The originals were already published in commit Current Chat qualification, original failures and UI gaps remains the runtime report. New evidence continues to belong in PR comments/attachments. Historical images already published in that commit are embedded below. These retain their original scope and are not claimed as current Chat/UI verification. Read the companion receipt/provenance files above for each original capture. Historical capture: fresh-stock-private-project-user.png
Historical capture: restored-distinct-target-empty-composer.png
Historical capture: restored-distinct-target-ready.png
Historical capture: restored-stock-empty-composer.png
Historical capture: minimax-ao-001-retained-failure.png
Historical capture: minimax-ao-002-retained-draft.png
Historical capture: minimax-ao-003-cancellation-failure-dark.png
Historical capture: minimax-ao-004-restored-success-dark.png
Cleanup appliedCommit |
MiniMax cancellation fixed — current head
|
| Step | Result | Proof |
|---|---|---|
| Persistent cancelled-result replay and admission | PASS, race-enabled transport regression | Replayed terminal, stop RPC/reply, duplicate suppression and exact ACK without blocking the reader. |
| Official native stream and private context | PASS | Actual model request carries initial context; multiple chunks before completion. |
| Native approve/reject | PASS | Offered decisions; protected side effect only after approval, absent after rejection. |
| Executing native tool Stop | PASS | Exact PID/kernel-start witness stops; native turn settles interrupted. |
| Immediate next message | PASS | Same-session follow-up completes after verified native release. |
| Delayed cancellation effect | PASS | Late artifact absent beyond the tool deadline. |
| Independent exact native restore | PASS | Same native ID/workspace, stable history, fresh post-load private context; missing/wrong-workspace refusal. |
| Registered settings and explicit Chat spawn | PASS, CI-only registry overlay | Actual AO settings/API, isolated worktree, native owner and initial context. |
| Idempotent API send and streamed projection | PASS | One durable turn, one native dispatch, one worktree mutation. |
| API approve/reject | PASS | Native offered decisions and protected side-effect boundary. |
| API Stop and immediate continuation | PASS | Exact executing native tool stops; same-session continuation succeeds. |
| Actual daemon replacement | PASS | Surviving exact detached host adopted; no duplicate dispatch or timeline loss. |
| AO Kill/archive → exact Restore | PASS, API scope | Confirmed native teardown and terminated record; exact identity/workspace/conversation restored. |
| Fresh context after AO restore | PASS, delivery scope | Token introduced only after confirmed Kill appears in actual post-restore model request; prior history and worktree edit retained. |
| Final isolation | PASS | No late cancellation artifact; project source checkout unchanged. |
| Actual Agent Harness/New Task/Chat/board UI, install/login, model toggles | NOT_RUN | CI did not launch AO UI; no screenshots or real provider authorization. Historical images are not relabeled. |
| Reviewer and TUI↔Chat handoff | Outside scope | No new support claimed. |
PASS TestNativeMiniMaxACPConformance (18.61s)
PASS TestNativeMiniMaxACPExactRestoreConformance (12.39s)
Native suite: PASS (31.988s)
PASS TestNativeMiniMaxRegisteredChatAPI (50.96s)
API suite: PASS (50.961s)
CI formatting diagnostic: empty patch
Shipping Chat registration remains disabled; candidate registration above is a CI-only source overlay, not a shipped feature flag. PR stays draft pending full integration/UI qualification. This green check does not fill the missing UI/auth rows.
Preserved failure progression
- Original pre-fix report, failing b75 native run: immediate continuation rejected with
-32603, active turn still held. e6d5bb3601run: native cancellation/continuation and exact restore PASS; newly reached API initial-context assertion FAIL. The fixture stored only the last label-matched request, allowing tool-free title/suggestion calls to overwrite the worker receipt. Current fixture separates worker receipts using the advertised Bash catalog; regression retains the private-context assertion. Initial formatting failures were corrected using CI-produced formatting deltas.adde537b95run: initial API private context PASS; idempotency assertion FAIL because it incorrectly expected duplicate responses to repeat a turn ID. Existing AO API deliberately returnsduplicate: trueand an empty turn ID. Current test honors that contract and still verifies one original durable turn and one native dispatch.a1066df789replay regression: expected RED before replay fix, timeout waiting for terminal event. Fixed in3886f0c39cby moving settlement off the reader with an atomic single-worker claim.3886f0c39crun: replay/native/API PASS; full Go lint reported error wrapping in the regression and G118 for background context on the replay goroutine. Both corrected explicitly in the final head; no lint suppression.- Current
ef3e216f0e48d078e2514ee7e44ea3d19c79e194: replay and both complete native/API qualification suites PASS as linked above; final-head lint also PASS.
Remaining CI and evidence ownership
Current-head Go and Frontend full workflows are still running at publication. Native/replay/API, lint, API/SQL drift, CLI native/container checks, release feeds and gitleaks have passed. Pending full suites are not called passed. Independent static review is complete with no remaining actionable findings.
No local build/tests/formatters/native/provider/UI execution ran. Only static local reads/edits/diff checks were used; all execution receipts above are from GitHub CI. Evidence stays in PR comments/CI attachments and temporary paths outside the checkout. Historical evidence migration remains unchanged in scope.
Prasad-D-Ware
left a comment
Prasad-D-Ware
left a comment
There was a problem hiding this comment.
Review of the MiniMax Code TUI/Chat integration at 1e8a81d. I've left five inline comments; I traced each one in the code but didn't run anything. The most important is the UserPromptSubmit generation check in hook.cjs: as written it can't detect a stale runtime. The credential-refresh comment can delete a working auth/ snapshot. The other three are a misleading recovery error on shutdown and two places where files pile up under ~/.ao.
Prasad-D-Ware
left a comment
Prasad-D-Ware
left a comment
There was a problem hiding this comment.
Re-reviewed the changes since my last review (1e8a81d → d8d1c16). All five comments are addressed, and I have no new blocking findings.
- hook.cjs: the exit-status guard is removed, and
ao hooksnow runs as best-effort reporting. A process from an earlier generation is now stopped by the prunedrouting.json(its hook fails closed) instead of by a check that never fired. - refreshProfileCredentials: a missing source
auth/keeps the private auth, a non-directory source is rejected, and a failed config write or a cancel after install rolls auth back. If the rollback itself fails, the backup is kept. - conversation.go: the fence now runs only for an accepted local stop or a native
cancelledresult. Theclosed/detachingcheck and the stored settlement cancel are guarded by the same lock ascloseProvider, so shutdown can't slip through and wait out the 10s timeout. The new tests cover shutdown, persistent detach and a context-cancelled request. - Launch pruning: this runs only after the new generation is fully written. It checks the ownership marker, works inside an
os.OpenRootroot, and skips names that aren't launch IDs. - Chat context file: there's now one stable
.ao-chat-context.md, replaced atomically, and per-launch copies left by earlier builds are swept.
Non-blocking note: because the private auth/ is now kept whenever the source has none, a user who logs out by deleting ~/.minimax/auth will still have the old credentials used by existing AO sessions. That seems to be an intended trade-off for config-only providers (d8d1c16). It may be worth a line in the docs or in AuthStatus.
CI: all 25 checks pass on d8d1c16.
|
Thanks for the merge with Before we approve, could you answer the two open review threads? Neither has a reply or a code change yet:
A fix, or an explanation of why either one should stay as it is, works for us. Once both are answered we'll do a final pass. |
This branch has not been deployed
3 participants
Code changes: +1753 -24
Tests: +3441 -18
Others: +109 -4
CI recovery for
a8a19e569: all 25 GitHub checks pass. The container download failed on aproxy.golang.orgHTTP/2INTERNAL_ERRORbefore compilation; the container retry passed on the same commit. The unchanged LAN tunnel and composer caret tests also passed on retry: backend gate, renderer smoke. No source or dependency changes were needed for this recovery, and no local checks/tests ran.Lint follow-up in
a8a19e569: the version-mismatch cache regression assertion useserrors.Is, addressing the singleerrorlintdiagnostic from GitHub lint. No dependencies installed or local checks/tests run. All 25 GitHub checks now pass on this head; recovery details are stated above.Review follow-up in
f59496f0f: TUI standing context is emitted only on the first rootUserPromptSubmitof each runtime launch. An exclusive launch-local marker prevents repeat injection; failed output releases it for retry, and restore prepares a fresh launch with updated context. Confirmed version mismatches are cached untilInvalidateBinaryResolution(harness refresh); probe failures, timeouts, and caller cancellation remain retryable. Added regression cases for these boundaries without running them locally.Installation deliberately requires the exact source-correlated MiniMax Code 0.6.5, installed manually. AO does not auto-install an unqualified npm-latest release for this adapter. After changing the installed version, refresh the harness to clear the cached resolution result. Shipping Chat registration remains disabled pending the stated qualification gaps.
The two remaining review concerns are fixed and all three already-addressed threads are closed. All 25 GitHub checks passed for the preceding merged head
b018cbfbd; that head subsequently reported the test-only lint issue corrected above. No dependencies installed and no local builds, checks, or tests run for this revision, as requested.Merged
mainatff84e3b53inb018cbfbd, preserving both MiniMax migration 201 and main migration 206 in the migration ledger. GitHub confirms the branch has no merge conflicts. All 25 GitHub checks passed for this merged head; current-revision CI status is stated above. No dependencies installed or local checks/tests run.Native-restore CI fix in
d8d1c1626: a source profile with credentials only inconfig.yamlcan resume after MiniMax creates its own privateauth/state. Missing source auth leaves that private state intact and atomically refreshes configuration. Auth replacement rollback protection remains in place. Updated TUI/Chat and native-created-auth regression cases; no dependencies installed or local checks/tests run. Pre-merge native-protocol CI passes, including interruption/replay, native exact restore, and registered AO daemon qualification against the controlled loopback provider. All 25 checks subsequently passed for merged head b018cbf; current-head CI is green as summarized above.Chat-context retention follow-up in
afa6f17c3: process launches atomically replace one private.ao-chat-context.mdfile; an empty standing prompt removes it. Legacy per-launch.ao-chat-context-*.mdfiles are cleaned up on subsequent launches without recursing into directories. Updated regression cases cover stable-path repeated launches, refreshed contents, empty prompts, and legacy/unrelated-file handling. No dependencies installed or local checks/tests run.Launch-retention follow-up in
45a89e48c: successful runtime preparation prunes older launch directories after publishing the current snapshots and hook configuration. Failed publication preserves the previous snapshot; cached callbacks for pruned generations are refused. Cleanup checks AO profile ownership and is confined to the profile through os.Root. Regression cases cover failed/abandoned generations, retained current context, and unaffected native history/neighboring sessions. No dependencies installed or local checks/tests run.Shutdown-settlement follow-up in
2a25e996b: native fencing requires an accepted local stop or a native cancelled result, including replay. Request-context cancellation alone does not trigger delegation-stop. Shutdown/detach cancels active settlement and suppresses recovery errors and receipt ACKs from the closing attachment. Added regression cases for cancellation classification, closed conversations, and shutdown/detach during settlement. No dependencies installed or local checks/tests run.Credential refresh preserves private native auth state when source auth is absent while refreshing provider configuration; config-only restores are supported. Auth installation and config-write failures roll back the previous auth state, with recovery backups retained if rollback fails. Added missing-source regression cases for TUI/Chat reuse and config-write rollback cases with/without previous auth. No dependencies installed or local checks/tests run.
Prompt-reporting follow-up in
a08646b62: activity callbacks are best effort and do not authorize prompt admission or prove that a runtime generation is current. Removed the callback-status refusal so a missing AO executable, nonzero exit, timeout, or reporting exception cannot block native input. Exact native conversation identity and bounded private-context validation remain enforced. Added regression cases for reporting failures and identity rejection; no dependencies installed or local checks/tests run.Review fixes in
1e8a81d68: accepts valid dotted AO session IDs in profile validation and hooks; confines styled-only activity reconciliation to MiniMax so Claude/Codex retain older-host support; carries the immutable TUI launch path as a base64url hook-command argument while preserving TMPDIR/TEMP/TMP; and refreshes source credentials/provider configuration on each TUI launch while retaining the AO permission selection and native history. Malformed refresh configuration preserves the previous snapshot; symlink errors identify the resource.Regression cases were added for those boundaries. No dependencies were installed, and no local builds, checks, or tests were run for this revision, as requested. Earlier qualification evidence below predates these fixes; pre-merge native-protocol verification is linked above; all 25 checks passed for the merged head b018cbf; current-head CI is green as summarized above.
Adds MiniMax Code TUI workers and a first-party ACP Chat candidate using shared AO Chat, private profiles, native permissions and live model variants. Stop now awaits the native delegation-stop receipt before committing interruption or announcing Ready; failed settlement preserves the durable receipt for recovery. Cancelled-result replay settles outside the transport reader with duplicate suppression and a bounded context. Reviewers and TUI↔Chat handoff remain unsupported.
Prior-head native and registered AO/API qualification passes against the official MiniMax Code 0.6.5 CLI and a controlled loopback model endpoint: streaming, approve/reject, executing-tool cancellation, immediate continuation, daemon replacement, archive and exact restore with refreshed private context. Real account authorization and actual AO UI screenshots remain NOT_RUN. Shipping Chat registration remains disabled. Full Go/Frontend CI status is recorded in the qualification comment; no local test/provider/UI execution ran.
TUI standing context uses supported
UserPromptSubmitadditionalContext from bounded immutable 0600 snapshots on the first root prompt of each runtime launch; later prompts do not repeat it. Restore creates a fresh launch and refreshes this context. Native identity, rules and permissions stay native-owned. Legacy identity-append histories are unchanged and refused for restore; authenticated wire-role proof and actual AO proof for the changed TUI hook route remain pending.Removed 41 evidence/duplicate-doc files (10,930 added text lines and 8 images). Historical originals and failure scope remain accessible through verified immutable links. New runtime evidence belongs in PR comments/attachments, not source files.