Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions sei-agent-driver/cmd/sei-agent-driver/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ signal, so a caller never posts a stale file from a previous run.
| `OMNIGENT_ORIGIN` | `omnigent://internal` | Sent as the `Origin` header on every request to satisfy the server's trusted-origin CSRF guard on state-changing POSTs. This process is not a browser and sends no Origin of its own, so it announces this sentinel instead. |
| `SEIDROID_AGENT_ID` | `seidroid` | The agent **name** to resolve to an id. There is no lookup-by-name route server-side, so the driver pages the agent listing until this name matches. |
| `SEIDROID_MODEL` | *(empty)* | Substitutes for the model the agent spec names. Empty leaves the spec's own. It applies to the review's own agent only. A scout runs on another agent, so another harness and another provider. A scout therefore keeps its spec's model, and the driver leaves any override of its own alone. Forwarding a name one provider does not answer to costs the whole reading, not just the model. The driver applies the override at both ends of a session's life, because it lives on the session row rather than on the turn. A session this run opens carries it from create, so the first turn — the one that writes the review — already runs on it. For a session an earlier dispatch opened, the driver moves it on adopt. One limit applies there. The harness reads the row when it launches. A session whose harness is already up therefore answers this run on the model it launched with. The new value takes effect at the next launch. The log says which of the two happened instead of reporting a bare success. The reconcile compares before it writes, so the common case costs no request: nothing asked for, no override present. A failure logs a warning rather than failing the run, because the review still runs on the model the session already carries. That is the override it had, not the spec's model, so a failed clear leaves the stale one in place. The server forwards the value as-is and enumerates nothing, so it does not reject an unrecognised name here — that fails at turn start. |
| `SEIDROID_EFFORT` | *(empty)* | Substitutes for the reasoning effort the agent spec names, for example `high`. Empty leaves the spec's own. It applies to the review's own agent only, because a scout takes its effort from its own bundle. The driver sets it at create and moves it on adopt. On adopt, a session whose harness is already up answers this run at the effort it launched with. The server validates the value per provider at turn start. |

The driver also reads `GITHUB_RUN_ID` and `GITHUB_RUN_ATTEMPT` when
`--trigger-id` is not given. These now only label a dispatch in the logs — they
Expand Down
5 changes: 5 additions & 0 deletions sei-agent-driver/internal/driver/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ type Config struct {
// not recognise fails at turn start rather than here.
Model string

// Effort substitutes for the reasoning effort the agent spec names, e.g. "high".
// Empty leaves the spec's own. The server validates it per provider at turn start.
Effort string

// Token is the bearer credential, when one was minted elsewhere. Never
// logged, and never included in an error from this package.
//
Expand Down Expand Up @@ -235,6 +239,7 @@ func LoadConfig() (Config, error) {
Origin: envOr("OMNIGENT_ORIGIN", defaultOrigin),
Agent: envOr("SEIDROID_AGENT_ID", defaultAgent),
Model: strings.TrimSpace(os.Getenv("SEIDROID_MODEL")),
Effort: strings.TrimSpace(os.Getenv("SEIDROID_EFFORT")),
Comment thread
seidroid[bot] marked this conversation as resolved.
Token: resolveToken(),

MachineClientID: strings.TrimSpace(os.Getenv("OMNIGENT_MACHINE_CLIENT_ID")),
Expand Down
2 changes: 1 addition & 1 deletion sei-agent-driver/internal/driver/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import (
// exports -- a developer with OMNIGENT_BASE_URL set would otherwise fail the
// defaults case and pass the override cases for the wrong reason.
var configEnv = []string{
"OMNIGENT_BASE_URL", "OMNIGENT_ORIGIN", "SEIDROID_AGENT_ID", "SEIDROID_MODEL",
"OMNIGENT_BASE_URL", "OMNIGENT_ORIGIN", "SEIDROID_AGENT_ID", "SEIDROID_MODEL", "SEIDROID_EFFORT",
"OMNIGENT_API_TOKEN", "OMNIGENT_API_TOKEN_FILE",
"OMNIGENT_MACHINE_CLIENT_ID", "OMNIGENT_MACHINE_CLIENT_SECRET",
"SEIDROID_RUN_DEADLINE_S", "SEIDROID_REQUEST_TIMEOUT_S",
Expand Down
9 changes: 6 additions & 3 deletions sei-agent-driver/internal/driver/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,10 +308,13 @@ func (d *Driver) workFor(w Workload) Work {
// rejected at turn start by another, costing the reading and not just the
// model -- nor its to lose. A nil model leaves the session's own override
// untouched, which is the difference between not managing something and
// clearing it.
// clearing it. The configured effort is the review's for the same reason.
return Work{RunKey: w.RunKey(), Title: w.Title(), Agent: named}
}
}
model := d.cfg.Model
return Work{RunKey: w.RunKey(), Title: w.Title(), Agent: d.cfg.Agent, Model: &model}
model, effort := d.cfg.Model, d.cfg.Effort
return Work{
RunKey: w.RunKey(), Title: w.Title(), Agent: d.cfg.Agent,
Model: &model, Effort: &effort,
}
}
3 changes: 3 additions & 0 deletions sei-agent-driver/internal/driver/host.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ type Work struct {
// Per work rather than per run, because a configured model belongs to the agent it
// was configured for. See [Driver.workFor].
Model *string

// Effort is the reasoning-effort override, three-valued the same way as Model.
Effort *string
}

// Ask is one exchange: what to say, and how to know the answer is finished.
Expand Down
6 changes: 4 additions & 2 deletions sei-agent-driver/internal/omni/conversation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ type driverCreateReq struct {
// this omitempty, and whether the default path sends "model_override":"" or
// nothing at all is the difference between a server that validates the value
// rejecting every create and accepting it.
ModelOverride *string `json:"model_override"`
ModelOverride *string `json:"model_override"`
ReasoningEffort *string `json:"reasoning_effort"`
}

// driverPatchReq is the subset of a session-update body this file asserts on.
Expand All @@ -42,7 +43,8 @@ type driverCreateReq struct {
// leaves the field alone, and a value either sets or -- as a clear alias -- removes
// the override.
type driverPatchReq struct {
ModelOverride *string `json:"model_override"`
ModelOverride *string `json:"model_override"`
ReasoningEffort *string `json:"reasoning_effort"`
}

// driverEventReq is the subset of a POST .../events body this file asserts
Expand Down
72 changes: 72 additions & 0 deletions sei-agent-driver/internal/omni/effort_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
package omni

import (
"testing"

"github.com/sei-protocol/sei-internal-skills/sei-agent-driver/internal/driver"
)

// runWithEffort drives one review against fs with the configured reasoning effort.
func runWithEffort(t *testing.T, fs *driverFakeServer, work testWork, effort string) driver.Result {
t.Helper()

cfg := driverTestConfig(t, fs.URL)
cfg.Effort = effort
return newTestDriver(cfg, driver.Policy{}, driverTestLogger()).Run(t.Context(), work)
}

func TestCreateCarriesTheConfiguredEffort(t *testing.T) {
t.Parallel()

work := testWork{Repo: "sei-protocol/sandbox", PR: 31, Trigger: "first"}
fs := newDriverFakeServer(t, driverFakeServerConfig{
AgentPages: []string{driverAgentPage("ag_1", "seidroid", "", false)},
CreateResp: driverSessionResp("conv_new", "ag_1"),
SessionListResp: `{"data":[],"has_more":false}`,
StreamFrames: []string{
driverAckFrame(),
driverConsumedFrame("item_1"),
driverIdleFrame("resp_claude_a"),
driverDoneFrame(),
},
SessionResps: []string{
driverSessionResp("conv_new", "ag_1"),
driverSessionWithItems("conv_new", "ag_1",
driverReplyItem("item_reply", "resp_claude_a",
driverVerdict("Read the diff.", "comment"))),
},
})

runWithEffort(t, fs, work, "high")

created := fs.CreateReqs()
if len(created) != 1 {
t.Fatalf("created %d sessions %+v, want 1", len(created), created)
}
if created[0].ReasoningEffort == nil || *created[0].ReasoningEffort != "high" {
t.Errorf("create reasoning_effort = %v, want high", created[0].ReasoningEffort)
}
}

func TestAdoptedSessionIsPointedAtThisRunsEffort(t *testing.T) {
t.Parallel()

work := testWork{Repo: "sei-protocol/sandbox", PR: 32, Trigger: "again"}
fs := modelFakeServer(t, testRunKey(work.Repo, work.PR), "")

if result := runWithEffort(t, fs, work, "high"); result.SessionID != "conv_prior" {
t.Fatalf("SessionID = %q, want conv_prior — the adopt path did not run", result.SessionID)
}

patches := fs.PatchReqs()
if len(patches) != 1 {
t.Fatalf("sent %d session patches %+v, want 1", len(patches), patches)
}
if patches[0].ReasoningEffort == nil || *patches[0].ReasoningEffort != "high" {
t.Errorf("reasoning_effort = %v, want high", patches[0].ReasoningEffort)
}
if patches[0].ModelOverride != nil {
t.Errorf("model_override = %q, want it untouched: no model was configured to move",
*patches[0].ModelOverride)
}
}
101 changes: 62 additions & 39 deletions sei-agent-driver/internal/omni/host.go
Original file line number Diff line number Diff line change
Expand Up @@ -395,73 +395,97 @@ type adoption struct {
revivable bool
}

// modelOrEmpty reads a work's model override, treating "leave it alone" as "no
// override at create" -- a session being opened here carries nothing to leave alone.
func modelOrEmpty(model *string) string {
if model == nil {
// orEmpty reads one of a work's overrides, treating "leave it alone" as "no override
// at create" -- a session being opened here carries nothing to leave alone.
func orEmpty(value *string) string {
if value == nil {
return ""
}
return *model
return *value
}

// reconcileModel moves an adopted session's model override to what this work asks for.
// sessionOverride is one per-session setting that the driver moves on adopt.
type sessionOverride struct {
name string
current *string
set func(ctx context.Context, sessionID, value string) (*omnigent.SessionResponse, error)
clear func(ctx context.Context, sessionID string) (*omnigent.SessionResponse, error)
}

// reconcileOverrides moves an adopted session's model and reasoning-effort overrides to
// what this work asks for. A nil want leaves that override alone.
func (h *Host) reconcileOverrides(
ctx context.Context,
client *omnigent.Client,
session *omnigent.SessionResponse,
w driver.Work,
live bool,
) {
sessions := client.Sessions()
for _, o := range []struct {
want *string
sessionOverride
}{
{w.Model, sessionOverride{"model", session.ModelOverride,
sessions.SetModelOverride, sessions.ClearModelOverride}},
{w.Effort, sessionOverride{"reasoning effort", session.ReasoningEffort,
sessions.SetReasoningEffort, sessions.ClearReasoningEffort}},
} {
if o.want != nil {
h.reconcileOverride(ctx, session.ID, o.sessionOverride, *o.want, live)
}
}
}

// reconcileOverride moves one override of an adopted session to want.
//
// The override lives on the session row, not on the turn, so a session opened by an
// earlier dispatch answers on that dispatch's model until something changes it. A pull
// request that gained a model label between reviews would otherwise be answered by the
// old model with nothing in the output saying so.
// earlier dispatch answers on that dispatch's value until something changes it.
//
// The current value is read from the session rather than assumed, so the common case --
// nothing asked for and no override present -- costs no request.
//
// live is reported rather than acted on, and it is the limit of what this can promise.
// The server places the override on the row so that it is there *before the harness
// launches*, which is the SDK's stated reason for its create-time field. A session whose
// harness is already up has therefore already read its model, and this run's turn goes to
// that process: the row is correct for the next launch, not for the turn about to be
// sent. Saying so is the point -- an unqualified success here would have the log claim a
// change this run did not get.
// The harness reads the row when it launches, so a session whose harness is already up
// answers this run on the value it launched with: the row is correct for the next
// launch, not for the turn about to be sent.
//
// A failure is logged, not returned. The model is a preference, and the review still runs
// on the model the session already carries; refusing here would turn a preference into a
// pull request with no review on it. Not the agent spec's model -- this runs only when
// current and want already differ, so a failed write leaves the session on the override
// it had, which in the clear case is the very one the run was removing.
func (h *Host) reconcileModel(
// A failure is logged, not returned. The override is a preference, and refusing here
// would turn a preference into a pull request with no review on it. A failed write
// leaves the session on the override it had, not on the agent spec's value.
func (h *Host) reconcileOverride(
ctx context.Context,
client *omnigent.Client,
session *omnigent.SessionResponse,
sessionID string,
o sessionOverride,
want string,
live bool,
) {
current := ""
if session.ModelOverride != nil {
current = *session.ModelOverride
}
current := orEmpty(o.current)
if current == want {
return
}

var err error
if want == "" {
_, err = client.Sessions().ClearModelOverride(ctx, session.ID)
_, err = o.clear(ctx, sessionID)
} else {
_, err = client.Sessions().SetModelOverride(ctx, session.ID, want)
_, err = o.set(ctx, sessionID, want)
}
if err != nil {
h.log.Warn("could not move the adopted session's model, so it answers on the "+
h.log.Warn("could not move the adopted session's override, so it answers on the "+
"one it already had",
"session_id", session.ID, "want", want, "have", current, "error", err)
"session_id", sessionID, "override", o.name, "want", want, "have", current,
"error", err)
return
}
if live {
h.log.Warn("moved the adopted session's model, but its harness is already up "+
h.log.Warn("moved the adopted session's override, but its harness is already up "+
"and read the old one at launch, so this run still answers on that",
"session_id", session.ID, "want", want, "have", current)
"session_id", sessionID, "override", o.name, "want", want, "have", current)
return
}
h.log.Info("moved the adopted session's model before its harness launched",
"session_id", session.ID, "want", want, "have", current)
h.log.Info("moved the adopted session's override before its harness launched",
"session_id", sessionID, "override", o.name, "want", want, "have", current)
}

// createOrAdopt finds this work's session or opens one, and refuses to hand back a
Expand Down Expand Up @@ -495,9 +519,7 @@ func (h *Host) createOrAdopt(
h.log.Info("adopting the session an earlier dispatch created",
"run_key", w.RunKey, "session_id", existing.ID,
"live", live, "revivable", revivable)
if w.Model != nil {
h.reconcileModel(ctx, client, existing, *w.Model, live)
}
h.reconcileOverrides(ctx, client, existing, w, live)
return existing, adoption{continued: true, live: live, revivable: revivable}, nil
}
h.log.Warn("the session for this work cannot run a turn; replacing it",
Expand All @@ -518,7 +540,8 @@ func (h *Host) createOrAdopt(
// At create as well as on adopt, because the override has to be on the session
// row before the harness launches. Setting it afterwards would leave the first
// turn -- the one that writes the review -- on the spec's model.
ModelOverride: modelOrEmpty(w.Model),
ModelOverride: orEmpty(w.Model),
ReasoningEffort: orEmpty(w.Effort),
}

session, err := client.Sessions().Create(ctx, create)
Expand Down
8 changes: 7 additions & 1 deletion sei-agent-driver/internal/omni/model_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ func TestAdoptedScoutSessionIsLeftAtItsOwnModel(t *testing.T) {
// A review with nothing configured still carries a non-nil model — a pointer to the
// empty string, meaning "no override" — so it exercises the value branch. Only a
// workload on its own agent carries nil, and only its first dispatch creates. Without
// this, [modelOrEmpty] could return anything for nil and the suite would stay green
// this, [orEmpty] could return anything for nil and the suite would stay green
// while every first scout dispatch sent it.
func TestCreatedScoutSessionCarriesNoModel(t *testing.T) {
t.Parallel()
Expand Down Expand Up @@ -307,6 +307,7 @@ func TestCreatedScoutSessionCarriesNoModel(t *testing.T) {

cfg := driverTestConfig(t, fs.URL)
cfg.Model = "claude-opus-4-7"
cfg.Effort = "high"
newTestDriver(cfg, driver.Policy{}, driverTestLogger()).Run(t.Context(), scout)

created := fs.CreateReqs()
Expand All @@ -318,6 +319,11 @@ func TestCreatedScoutSessionCarriesNoModel(t *testing.T) {
"absent — the configured model belongs to the review's agent",
*created[0].ModelOverride)
}
if created[0].ReasoningEffort != nil {
t.Errorf("created a scout session with reasoning_effort = %q; want the field "+
"absent — a scout takes its effort from its own bundle",
*created[0].ReasoningEffort)
}
}

// TestReviewSurvivesAFailedModelMove is the fail-open promise, and it had no test.
Expand Down
Loading