diff --git a/CHANGELOG.md b/CHANGELOG.md index 96bfc61..23c5fad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 artifact lookup. `QODER_CN_ACCESS_TOKEN` is accepted as a local secret-manager input alias and forwarded to qodercn as `QODERCN_PERSONAL_ACCESS_TOKEN`. +### Fixed +- The built-in `qodercli` engine now assigns one native Qoder session ID per + evaluation run and exports it as `QODER_SESSION_ID` (or + `QODERCN_SESSION_ID`). Qoder and its Bash tool subprocesses therefore share + the same conversation identity, while resumed turns retain that identity and + independent cases cannot inherit a stale parent session. + ## [0.9.1] - 2026-08-20 ### Fixed diff --git a/internal/agent/qodercli.go b/internal/agent/qodercli.go index 3adc35d..ef6306f 100644 --- a/internal/agent/qodercli.go +++ b/internal/agent/qodercli.go @@ -9,6 +9,8 @@ import ( "strings" "time" + "github.com/google/uuid" + "github.com/alibaba/skill-up/internal/credential" "github.com/alibaba/skill-up/internal/logging" "github.com/alibaba/skill-up/internal/observability" @@ -28,6 +30,7 @@ type qoderCLIProfile struct { binary string credentialEnv string exposeUsageEnv string + sessionEnv string configDir string installURL string execPathProbe string @@ -55,6 +58,8 @@ const ( qoderCNExposeTokenUsageEnv = "QODERCN_EXPOSE_TOKEN_USAGE" //nolint:gosec // environment variable name, not a credential qoderExposeTokenUsageEnabled = "true" qoderJSONOutputFlag = " --output-format json" + qoderSessionIDEnv = "QODER_SESSION_ID" + qoderCNSessionIDEnv = "QODERCN_SESSION_ID" ) func qoderProfileForKwargs(kwargs map[string]string) qoderCLIProfile { @@ -66,6 +71,7 @@ func qoderProfileForKwargs(kwargs map[string]string) qoderCLIProfile { binary: "qodercli", credentialEnv: credential.EnvQoderPersonalAccessToken, exposeUsageEnv: qoderExposeTokenUsageEnv, + sessionEnv: qoderSessionIDEnv, configDir: ".qoder", installURL: "https://qoder.com/install", execPathProbe: qoderExecPathProbeCmd, @@ -76,6 +82,7 @@ func qoderProfileForKwargs(kwargs map[string]string) qoderCLIProfile { binary: "qodercn", credentialEnv: credential.EnvQoderCNPersonalAccessToken, exposeUsageEnv: qoderCNExposeTokenUsageEnv, + sessionEnv: qoderCNSessionIDEnv, configDir: ".qoder-cn", installURL: "https://static.qoder.com.cn/qoder-cli-cn/install.sh", execPathProbe: qoderCNExecPathProbeCmd, @@ -155,13 +162,22 @@ func (a *QoderCLIAgent) Run(ctx context.Context, rt Runtime, opts ExecOptions, m }, err } - envVars := a.qoderRunEnvVars() + sessionID := uuid.NewString() + envVars := a.qoderRunEnvVars(sessionID) opts = a.mergeExecOptionsEnv(ctx, opts, envVars, a.buildAgentObservabilityAttrs(nil)) + a.pinQoderSessionEnv(&opts, sessionID) ctx = observability.ContextWithConfiguredAgentSpanAttributes(ctx, opts.Env) result, err := rt.Exec(ctx, cmd, opts) sessionResult := a.buildSessionResult(ctx, rt, opts, instruction, start, result) if sessionResult != nil { + if sessionResult.SessionID != "" && sessionResult.SessionID != sessionID { + return sessionResult, fmt.Errorf( + "%s returned session %q after launcher assigned %q", + a.profile.binary, sessionResult.SessionID, sessionID, + ) + } + sessionResult.SessionID = sessionID sessionResult.PromptDelivery = promptDelivery } if err != nil { @@ -192,14 +208,26 @@ func (a *QoderCLIAgent) appliedModelName(_ context.Context) string { return "" } -func (a *QoderCLIAgent) qoderRunEnvVars() map[string]string { +func (a *QoderCLIAgent) qoderRunEnvVars(sessionID string) map[string]string { envVars := a.credentialEnvVars("", "") if _, configured := envVars[a.profile.exposeUsageEnv]; !configured { envVars[a.profile.exposeUsageEnv] = qoderExposeTokenUsageEnabled } + if sessionID != "" { + envVars[a.profile.sessionEnv] = sessionID + } return envVars } +func (a *QoderCLIAgent) pinQoderSessionEnv(opts *ExecOptions, sessionID string) { + if opts.Env == nil { + opts.Env = make(map[string]string) + } + // Pin the launcher-assigned ID after all user/runtime env layers have + // merged so the Qoder process and its child tools observe one session. + opts.Env[a.profile.sessionEnv] = sessionID +} + func buildQoderRunCmd(instruction, model string) string { return buildQoderRunCmdForBinary("qodercli", instruction, model) } @@ -231,7 +259,11 @@ func buildQoderResumeCmd(instruction, model, sessionID string) string { } func buildQoderResumeCmdForBinary(binary, instruction, model, sessionID string) string { - cmd := binary + " --permission-mode=bypass_permissions" + qoderJSONOutputFlag + // A resumed Qoder process selects the session through -r. Inherited Qoder + // session variables map to --session-id and conflict with --resume unless + // --fork-session is also used, so remove both editions before launching. + cmd := "unset " + qoderSessionIDEnv + " " + qoderCNSessionIDEnv + "; " + + binary + " --permission-mode=bypass_permissions" + qoderJSONOutputFlag if model != "" { cmd += " --model " + shellQuote(model) } @@ -334,7 +366,7 @@ func (a *QoderCLIAgent) RunTurn(ctx context.Context, rt Runtime, opts ExecOption instruction := message.Content cmd := buildQoderResumeCmdForBinary(a.profile.binary, instruction, a.appliedModelName(ctx), sessionID) - envVars := a.qoderRunEnvVars() + envVars := a.qoderRunEnvVars("") opts = a.mergeExecOptionsEnv(ctx, opts, envVars, a.buildAgentObservabilityAttrs(nil)) ctx = observability.ContextWithConfiguredAgentSpanAttributes(ctx, opts.Env) diff --git a/internal/agent/qodercli_test.go b/internal/agent/qodercli_test.go index 510b379..a3a94fa 100644 --- a/internal/agent/qodercli_test.go +++ b/internal/agent/qodercli_test.go @@ -215,12 +215,14 @@ func TestQoderCLIRun_MergesConfiguredEnvVars(t *testing.T) { ag := NewQoderCLIAgent(Config{ EnvVars: map[string]string{ "QODER_TEST_FLAG": "cfg-flag", + qoderSessionIDEnv: "stale-parent-session", }, }) _, err := ag.Run(context.Background(), rt, ExecOptions{ Env: map[string]string{ - "EXTRA_FLAG": "1", + "EXTRA_FLAG": "1", + qoderSessionIDEnv: "runtime-session-override", }, }, []transcript.Message{{ Role: transcript.RoleUser, @@ -239,6 +241,33 @@ func TestQoderCLIRun_MergesConfiguredEnvVars(t *testing.T) { if rt.lastExecEnv[qoderExposeTokenUsageEnv] != qoderExposeTokenUsageEnabled { t.Fatalf("expected %s to default to true, got %q", qoderExposeTokenUsageEnv, rt.lastExecEnv[qoderExposeTokenUsageEnv]) } + if resultID := rt.lastExecEnv[qoderSessionIDEnv]; resultID == "" || + resultID == "stale-parent-session" || resultID == "runtime-session-override" { + t.Fatalf("expected %s to be assigned", qoderSessionIDEnv) + } +} + +func TestQoderCLIRun_AssignsDistinctSessions(t *testing.T) { + t.Parallel() + + rt := &qoderTestRuntime{ + workspace: t.TempDir(), + execResult: runtime.ExecResult{Stdout: "ok\n", ExitCode: 0}, + } + ag := NewQoderCLIAgent(Config{}) + message := []transcript.Message{{Role: transcript.RoleUser, Content: "hello", Turn: 1}} + + first, err := ag.Run(context.Background(), rt, ExecOptions{}, message) + if err != nil { + t.Fatalf("first run: %v", err) + } + second, err := ag.Run(context.Background(), rt, ExecOptions{}, message) + if err != nil { + t.Fatalf("second run: %v", err) + } + if first.SessionID == "" || second.SessionID == "" || first.SessionID == second.SessionID { + t.Fatalf("session IDs = %q, %q; want distinct non-empty IDs", first.SessionID, second.SessionID) + } } func TestQoderCLIRun_CNEditionUsesCNCommandAndEnv(t *testing.T) { @@ -272,6 +301,12 @@ func TestQoderCLIRun_CNEditionUsesCNCommandAndEnv(t *testing.T) { if got := rt.lastExecEnv[qoderCNExposeTokenUsageEnv]; got != qoderExposeTokenUsageEnabled { t.Fatalf("%s = %q, want true", qoderCNExposeTokenUsageEnv, got) } + if got := rt.lastExecEnv[qoderCNSessionIDEnv]; got == "" { + t.Fatalf("expected %s to be assigned", qoderCNSessionIDEnv) + } + if _, exists := rt.lastExecEnv[qoderSessionIDEnv]; exists { + t.Fatalf("global %s must not be injected for CN", qoderSessionIDEnv) + } if _, exists := rt.lastExecEnv[qoderExposeTokenUsageEnv]; exists { t.Fatalf("global %s must not be injected for CN", qoderExposeTokenUsageEnv) } @@ -312,7 +347,7 @@ func TestQoderCLIRun_ParsesJSONUsage(t *testing.T) { rt := &qoderTestRuntime{ workspace: t.TempDir(), execResult: runtime.ExecResult{ - Stdout: `{"type":"result","subtype":"success","result":"OK.","usage":{"input_tokens":100,"cache_read_input_tokens":20,"cache_creation_input_tokens":3,"output_tokens":7},"session_id":"qoder-session-json"}`, + Stdout: `{"type":"result","subtype":"success","result":"OK.","usage":{"input_tokens":100,"cache_read_input_tokens":20,"cache_creation_input_tokens":3,"output_tokens":7},"session_id":"${QODER_SESSION_ID}"}`, }, } ag := NewQoderCLIAgent(Config{}) @@ -331,8 +366,8 @@ func TestQoderCLIRun_ParsesJSONUsage(t *testing.T) { if result.FinalMessage != "OK." { t.Fatalf("FinalMessage = %q, want OK.", result.FinalMessage) } - if result.SessionID != "qoder-session-json" { - t.Fatalf("SessionID = %q, want qoder-session-json", result.SessionID) + if result.SessionID == "" || result.SessionID != rt.lastExecEnv[qoderSessionIDEnv] { + t.Fatalf("SessionID = %q, launcher env = %q", result.SessionID, rt.lastExecEnv[qoderSessionIDEnv]) } if got := result.Transcript.FinalAssistantMessage(); got != "OK." { t.Fatalf("final transcript message = %q, want OK.", got) @@ -478,6 +513,40 @@ func TestQoderCLIRunTurn_ResumeUsesCorrectFlag(t *testing.T) { if rt.lastExecEnv[qoderExposeTokenUsageEnv] != qoderExposeTokenUsageEnabled { t.Fatalf("expected %s to default to true, got %q", qoderExposeTokenUsageEnv, rt.lastExecEnv[qoderExposeTokenUsageEnv]) } + if got := rt.lastExecEnv[qoderSessionIDEnv]; got != "" { + t.Fatalf("%s = %q, want no resume-time session selection env", qoderSessionIDEnv, got) + } + if !strings.HasPrefix(rt.agentCommand, "unset QODER_SESSION_ID QODERCN_SESSION_ID; qodercli ") { + t.Fatalf("resume command must clear inherited session selection env: %q", rt.agentCommand) + } +} + +func TestQoderCLIRunTurn_ResumeClearsInheritedSessionEnv(t *testing.T) { + t.Parallel() + + rt := &qoderTestRuntime{ + workspace: t.TempDir(), + execResult: runtime.ExecResult{Stdout: "resumed answer\n", ExitCode: 0}, + } + ag := NewQoderCLIAgent(Config{ + EnvVars: map[string]string{ + qoderSessionIDEnv: "stale-global-session", + qoderCNSessionIDEnv: "stale-cn-session", + }, + }) + + _, err := ag.RunTurn(context.Background(), rt, ExecOptions{ + Env: map[string]string{qoderSessionIDEnv: "runtime-override"}, + }, transcript.Message{Role: transcript.RoleUser, Content: "follow up", Turn: 2}, "resume-target") + if err != nil { + t.Fatalf("RunTurn (resume): %v", err) + } + if !strings.HasPrefix(rt.agentCommand, "unset QODER_SESSION_ID QODERCN_SESSION_ID; qodercli ") { + t.Fatalf("resume command must clear inherited session selection env: %q", rt.agentCommand) + } + if !strings.Contains(rt.agentCommand, "-r 'resume-target'") { + t.Fatalf("resume command must select only the explicit resume target: %q", rt.agentCommand) + } } func TestQoderCLIAppliedModelName_AllowsSupportedModel(t *testing.T) { @@ -744,7 +813,13 @@ func (r *qoderTestRuntime) Exec(_ context.Context, command string, opts runtime. strings.Contains(command, "qoder.com.cn/qoder-cli-cn/install.sh") { r.lastExecEnv = mapsClone(opts.Env) } - return r.execResult, nil + result := r.execResult + for _, sessionEnv := range []string{qoderSessionIDEnv, qoderCNSessionIDEnv} { + if sessionID := opts.Env[sessionEnv]; sessionID != "" { + result.Stdout = strings.ReplaceAll(result.Stdout, "${"+sessionEnv+"}", sessionID) + } + } + return result, nil } func (r *qoderTestRuntime) Workspace() string { return r.workspace } func (r *qoderTestRuntime) RequiresProcessSandbox() bool {