Skip to content

fix(qodercli): keep child tools on the evaluation session #219

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.

Sign up for GitHub

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

Open
chiga0 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(qodercli): keep child tools on the evaluation session #219

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 7 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
40 changes: 36 additions & 4 deletions internal/agent/qodercli.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -28,6 +30,7 @@ type qoderCLIProfile struct {
binary string
credentialEnv string
exposeUsageEnv string
sessionEnv string
configDir string
installURL string
execPathProbe string
Expand Down Expand Up @@ -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 {
Expand All @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)

Expand Down
85 changes: 80 additions & 5 deletions internal/agent/qodercli_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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{})
Expand All @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down
Loading