Repository navigation
Feat: Pick an agent's inference server with S in agentop - #1336
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (33)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe changes add inference-host data to session summaries and expose pipeline error policies through the API. agentop uses this information to display server routing, warn about eligible sessions during server removal, and let users change an agent’s server in the TUI. ChangesInference server routing and visibility
Priority: ➖ Normal Estimated code review effort: 4 (Complex) | ~60 minutes Change: Feature Sequence Diagram(s)sequenceDiagram
participant User
participant AgentopTUI
participant ConfigWriter
participant Cortex
User->>AgentopTUI: Select a server for an agent
AgentopTUI->>ConfigWriter: Write verified routing configuration
ConfigWriter->>Cortex: Reload configuration
Cortex-->>ConfigWriter: Return reload outcome
ConfigWriter-->>AgentopTUI: Return write and reload result
AgentopTUI->>Cortex: Refetch pipeline when the local target matches
Suggested reviewers: Merge Risk: ⚪ Minimal · up to No actionable merge-blocking issue was established; the change is mergeable after normal checks. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
SessionSummary gains inferenceHost: the host of the session's latest request that carried an inference parse, folded at append time by the store and by the archive's fold through one rule, persisted in session.json, and left alone by tunnel rows, MCP calls and responses, which interleave with turns. After a redirect it is the server that answered. agentop names a session's inference server from it. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Verify, how a server is shown, the change that routes an agent, and which server a host belongs to move from package main into cmd/agentop/servers, so the TUI's S picker writes with the same check agentop server uses and shows servers the same way. Behaviour is unchanged. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Counted from the running proxy's resident sessions whose inferenceHost is on the server: each of them gets an error asking for a new session from its next request. The remove still goes ahead. add's line now names S on the agents pane beside the use command. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
A SERVER column, while the proxy runs the inference-router: the server an agent's new sessions go to, or "own choice" for one it leaves alone. Read from /v1/pipeline, the configuration the proxy runs, which agentop now refetches with the pane's rows so a switch made in a shell shows. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
S opens a picker over the pane, its cursor on what the agent has now: its own choice, then each server with its host and model mapping. Enter writes the change agentop server use or reset makes, with the same check, and waits for the proxy's reload; the footer shows it in flight, then one line naming the server and how many running sessions stay where they are, or the proxy's error. Offered only on this machine's Cortex with the router running; elsewhere S says why. Each way the write can end says where it left the config file, as agentop server use does: put back after a refusal or a proxy that stopped answering, left as written after a timeout, untouched when the file already held the choice. The pipeline is refetched after every outcome, so the SERVER column settles what a flash cannot. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
A plugin under on_error: observe runs and records, but what it would reject, rewrite or redirect is dropped, and /v1/pipeline served its config with nothing to say so: a reader saw an observing plugin as one that acts. Each plugin's entry now carries onError when its policy is not the default, so in practice only observe appears; an off plugin is not built and is not listed. Pipeline.PolicyAt, the lookup Run already used, is exported for it, and the session API reads the plugins and their policies from one Load so a reload between the two cannot pair a plugin with another's policy. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Under on_error: observe the router stays in the pipeline and keeps its config, but moves nothing. agentop read only the config, so the agents pane's SERVER column named servers no request went to, and S wrote a route and flashed "New X sessions →" for a change that did nothing. The TUI now reads the policy /v1/pipeline serves. A router under observe is no router to the column, the footer or S, and S says why in the words agentop server uses, now shared from the servers package. S asks again on enter, since the pipeline can change while the picker is open. With no router on the wire, which is also how an on_error: off router looks, S names both fixes. A config agentop cannot decode is treated as no router rather than read in part. The no-User-Agent row's SERVER cell is blank, like Other's, since the router refuses to route it. Server names are sanitised like every other label the proxy serves. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
The result of S's write was a three-second flash after thirty columns of connection state, cut from the right: at 80 columns a refusal lost the proxy's error and that the file was put back, a timeout the URL to check, a success its count. It is now the footer's whole line until the next key, cut from the left so what the reader acts on survives, with shorter lead-ins. Why S opens nothing is shown whole in a panel over the pane: the observe reason is two sentences and a file path, which no footer line holds. The count is taken when Enter is pressed, and the pipeline refetched only while this machine's Cortex is still on screen, so leaving for a pod meanwhile neither counts its sessions nor paints its pipeline. The picker and the panel close when the pane is entered, so one a message moved the pane out from under does not come back. [S] is shown only on a row where S opens. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
A read-only SERVER column, while the proxy's inference-router has more than one server: the server on the host the session's inference last went to, a host no server has in parentheses, or a dash. It follows the AGENT column's rule: added only when it fits without narrowing another. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
agentop's README gains the agents pane's S and SERVER column, the sessions table's SERVER column, the key, and remove's warning; Claude Code's page points at both; CLAUDE.md lists inferenceHost on /v1/sessions. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
interleaved() gains a denial carrying an inference parse, as a live one does: the parser runs before the plugin that turns the request away. It went nowhere, so it must not move the session's host, and now every inferenceHost test checks that. Dropping the phase check from inferenceHostOf fails three of them. The comments that said otherwise are fixed: foldFixture's host per agent does not pin latest-wins (its first and latest turns are the same agent's); InferenceHost is the server a request was sent to, whether or not it answered; and absent means unknown, not "no inference traffic", for a session whose archived history predates the field. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
After a restart a resumed session reports the archive's inferenceHost until its own next inference request, but the inference-router's pins live in memory and did not survive: its next request is decided as a new one's and goes to its agent's current server. S counted it as staying where it is, and agentop server remove as one that would get the "no longer configured" error. Neither is true. /v1/sessions now says when that is the case: inferenceHostFromHistory is true when inferenceHost came only from the archive's fold, because no request the proxy's entry holds set it. inferenceHost itself is unchanged. Both counts leave such a row out. After an eviction alone the pin can survive, so there the count errs low, the quiet side. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
The warning said every session on the removed server gets an error from its next request, and the success line that a session that started on it now does. Live, only one the router pinned there does, and only on a request addressed to a server that is left: a request addressed to the removed server's own host is no longer an inference server's, so the router skips it and it goes there with the agent's own key. Both lines now say which is which, and that adding the server back routes them to it again. The count also leaves out the default and pending: buckets. The router never pins them; their requests follow the agent's current server, which remove refuses to take away, so none of them can get the error. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Three places said everything a plugin under on_error: observe would change is dropped: apiclient.PipelinePlugin.OnError, Pipeline.PolicyAt and CLAUDE.md's /v1/pipeline row. The framework mutes a Reject, SetBody, SetResponseBody and Redirect; a header a plugin writes still goes out, token-exchange's Authorization among them. Each now says so, as does the session API's own field comment. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
baseURLCheck and serverAdd's same-host refusal each kept their own loop over the servers, comparing hostnames as servers.ForHost does. They agreed today; with one rule they cannot come to disagree with the sessions table's SERVER column and the session counts, which already use ForHost. No change in what either says: the existing case-and-port tests for both pass unchanged, and dropping the same-name exception or feeding ForHost the wrong host fails them. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
S's observe and no-router reasons named this machine's config file by its absolute path, where agentop server prints it under the home directory as ~/.cortex/config.yaml, and the README says S uses the same words. A small homeTilde in the tui package, the rule package main's uses, so the panel and the command now say the same thing about the same file. It reads $HOME and nothing under it; the test points it at a temporary directory with t.Setenv. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
Four things about what S shows once ↵ is pressed. A switch that does not go through — refused, unreachable, not put back, or stopped before the write — now opens the wrapped panel S's refusals use. A refusal's line is its lead-in and the proxy's whole error, and a real reloader error alone runs past what 80 columns leave it, so the left-cut footer line lost "refused" and "put back" first. Success and a timeout stay the sticky line. A failure landing off the AGENTS pane, where the panel is not drawn, is the sticky line too. The result closes any panel up when it lands: S on a switch in flight opens one saying so, and the key that closed it also dismissed the sticky result underneath. ↵ on the entry the picker opened on no longer short-cuts to "already go to". The route can change while the picker is up — the rows poll refetches /v1/pipeline, a shell's `agentop server use` reaches the file first — and the shortcut flashed "already go to ete" over a proxy routing to glm, writing nothing. edit.WritePluginConfig decides: it writes and polls nothing when the file already holds the choice, and that reads as already in force when the router ↵ read agrees. Every served string S draws is sanitised where it is drawn: server names in the picker, the in-flight marker and every result line, hosts and model mappings in the picker, the errors a switch reports, and the panel's whole text. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
The README's S section now matches the code it describes: a switch that does not go through is a panel, whole, closed with Esc, q, Ctrl+C or ↵, and the sticky line only when it lands off the agents pane or is a timeout; a choice the config file already holds writes nothing, the file deciding rather than where the cursor opened; and the reasons S opens nothing include the three the list left out — the unknown row, which the router refuses to route, a pipeline agentop has not read yet, and a router config this agentop cannot decode. The SERVER column's blank cells name unknown beside All agents and Other. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
The sessions table's AGENT and SERVER columns came and went as the window widened: at 90 they fitted in the room COST and SAVED leave while TITLE holds them out, at 93 those returned and took it back, and near 113 the columns fitted again. They are now offered only where COST and SAVED render, so each appears once (AGENT at 113, SERVER at 114, both at 127) and stays. TITLE no longer grows into the room an asked-for column has yet to claim, which had it collapse from 23 to 11 the moment AGENT arrived. A sweep from 40 to 250 columns over every combination pins both, and growSessionsTitle's "presence is monotonic" is true again. The agents pane keeps its SERVER column at 80 columns, squeezing AGENT from 30 to 16, deliberately: it is the one place the pane shows where each agent's new sessions go, and the agent's name and minor version survive, which is what SERVER and S act on. A test pins that floor. Also fixes sessionsColumns' TITLE comment, which said the id is read off row[0]; it is read from sessionRowIDs. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
huang195
force-pushed
the
feat/agentop-server-picker
branch
from
October 8, 2026 17:03
e9941c8 to
8f1d79c
Compare
Resolves the conflicts with rossoctl#1335 (model mapping). The replacement question in agentop server add keeps rossoctl#1335's model-mapping wording and calls this branch's moved helpers, servers.Host and servers.Mapping. claude-code.md names both the SERVER column and the model line. Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Hai Huang <huang195@gmail.com>
mrsabath
approved these changes
Oct 8, 2026
mrsabath
left a comment
mrsabath
left a comment
Member
There was a problem hiding this comment.
Reviewed across core/session, core/sessionapi, core/pipeline, cmd/agentop (CLI, the new servers package, the TUI) and the docs. The dual-path checks all hold: the store fold and the archive's SummaryFold share the one inferenceHostOf rule, the CLI's runningOn and the TUI's sessionsStaying agree on what counts as a pinned running session, and both surfaces word an inactive router through servers.Inactive — so the two cannot drift apart. Denied requests are covered too: a denial appends only the SessionDenied row, so the phase check keeps it from moving the host.
The sanitization discipline is 滴水不漏 (dī shuǐ bù lòu — watertight; not a drop leaks): served names, mappings and errors are sanitized, an inferenceHost is drawn only when it passes a hostname's alphabet, and a URL the router refuses shows nothing of itself. The docs claims check out against the code — the CLAUDE.md field-order note, the README anchors, the 80-column AGENT squeeze, and the column-presence monotonicity tests.
One nit below; nothing blocking.
Areas reviewed: Go (core/session, core/sessionapi, core/pipeline, cmd/agentop CLI + servers + TUI), tests, docs
Commits: 19, all signed-off
CI status: all checks passing
| return "the change to " + msg.agent + " was not confirmed: the proxy stopped answering before it reported the reload, " + | ||
| "so the config file was put back; check the proxy is running and try again" | ||
| case edit.WriteReloadTimedOut: | ||
| return fmt.Sprintf("wrote the change to %s, but the proxy reported no reload within %s; check %s/reload/status", |
Member
There was a problem hiding this comment.
This is the one flash string that interpolates something unsanitized: msg.statsURL. Every other served string in serverSwitchedText goes through sanitizeLabel, and renderServerNotice's comment sets the bar at "a path from a config file" — which is where the stats URL comes from (stats.address, via localEditTargets). The sticky flash renders raw, so against this PR's own invariant ("nothing a cell, flash or panel shows can carry … a control character") this line is the gap. Practical risk is near nil — the address has to survive dialURL and a live /reload/status probe — so consistency only: sanitizeLabel(msg.statsURL) would close it.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Third PR in the series that lets a team choose, from agentop, which LiteLLM server an agent's new sessions use. #1330 gave the command line
agentop server use. This PR puts the same choice in the TUI. PressSon the agents pane, pick a server, and new sessions of that agent go there. A SERVER column on the agents and sessions panes shows where each agent and session is going.Based on main, now that #1329 and #1330 have merged. It is independent of #1335 (model mapping), but both edit
cmd/agentop/cmd_server*.go, so whichever merges second gets rebased.What a user sees
Son an agent's row opens a picker listing the agent's own choice first, then each server with its host and mapping. The cursor starts on what the proxy runs now, and ↵ applies the highlighted row.Success:
New claude-code sessions → glm. 3 running sessions stay where they are.A running conversation never moves; that is the router's pin rule from Feat: Route an agent's new sessions to a chosen inference server #1330.Failure: shown in a wrapped panel, so the proxy's whole error is readable at 80 columns. What happened to the config file is stated plainly:
Whether the choice is "already" in force is decided from the file, not from what the picker showed when it opened.
Refusals:
Srefuses in a small panel, with a reason, on:on_error: observerouter, which routes nothing (same wording asagentop server);The sessions SERVER column names each session's server from the host its inference requests went to. A host that is no server's shows in parentheses. The column appears only with more than one server, and only from about 114 columns, together with COST and SAVED, so widening the terminal never takes a column away.
agentop server removenow warns how many running sessions are still on the server it removes, andaddmentionsS. The warning counts only sessions the router has pinned there. Its wording now says exactly which ones get an error: a session pinned to the removed server, on a request addressed to a server that remains.What it adds
coreis consumed outside this repo; nothing is removed):session.SessionSummary.InferenceHost(inferenceHostonGET /v1/sessions,omitempty): the host of the session's latest outbound inference request. The store and the archive'sSummaryFoldfold it with one shared rule, so tunnel rows, MCP calls, responses and denials never move it. It survives a trim, a rekey and a resume; a resumed session's own value wins. The archive'ssession.jsongains the same field, and an older file decodes to unknown.inferenceHostFromHistory(omitempty) is set when that host came only from the archive: a session resumed after a restart that has sent no inference since, which no pin holds.Sandremoveleave such sessions out of their counts.GET /v1/pipelinegainsonErrorper plugin, sent only when it is not the default; in practice onlyobserve, since anoffplugin is never built.(*pipeline.Pipeline).PolicyAtis exported from the formerpolicyAt, anddescribePipelinereads plugins and policies from oneLoad.cmd/agentop/servers, the router pieces the CLI and the TUI share:Verify,AgentChange,Host,Mapping,ForHost,Inactive./v1/pipelinewith keys already redacted, never the file. It refetches with each agents poll, so a switch made in a shell shows.edit.WritePluginConfigwith the sameVerifyasagentop server use. It writes only when this machine's Cortex is on screen, and lets the writer decide whether the file already holds the choice.Testing
core/session: latest wins; untouched by tunnel, MCP, response and denied rows; absent when there is none; survives a trim; the two folds agree at every split point; an oldersession.json.core/sessionapi:inferenceHoston resident, resumed and archive-only rows, andonErroron/v1/pipeline, both asserted on the raw body of a real server.cmd/agentop/servers: each helper, including that a URL the router refuses shows nothing of itself.cmd/agentop/tui:cmd/agentop:remove's warning, counted through a real sessionapi server.expectagainst an isolated Cortex on 127.0.0.1:47711–47714 from a scratch HOME:[S], and the picker opening on the current server;remove's warning, and the 503 that follows;[S].The shared proxy on
:47600was never touched.go vet,go testandgo mod tidy -diffpass in core, cmd/agentop, cmd/cortex (full) and cmd/cortex-envoy (envoy), andgofmt -lis clean on every touched directory.Known gaps and deferred minors
offrouter looks like none to the TUI, since/v1/pipelinenever lists anoffplugin. The refusal names both fixes.agentop server's settings check is where that is caught, as in Feat: Route an agent's new sessions to a chosen inference server #1330.claude-code/2.1.270truncates.remove's count still includes an unrouted agent's sessions on the removed host. They get no error, and nothing on the summary tells them apart.Assisted-By: Claude (Anthropic AI) noreply@anthropic.com
Summary by CodeRabbit
New Features
Bug Fixes