fix(conpty): stop pinning --effort on hand-driven interactive sessions
Task, ad-hoc, planning, handoff and resume sessions now inherit the CLI default / the user's /effort setting. List-handler sessions keep their per-role effort.
This commit is contained in:
@@ -15,9 +15,9 @@ Autonomous queue tasks are **not** covered here — they stay on the stream-json
|
||||
|
||||
| Kind | Hub spec method | Notes |
|
||||
|---|---|---|
|
||||
| Task session | `GetInteractiveLaunchSpec` | Effort from the task/list model preset |
|
||||
| Ad-hoc | `GetAdHocLaunchSpec` | Effort from the global default |
|
||||
| Planning | (planning start/resume) | Effort from `PlanningAlias`; uses `--permission-mode default`, **not** `plan` |
|
||||
| Task session | `GetInteractiveLaunchSpec` | No `--effort` — CLI default |
|
||||
| Ad-hoc | `GetAdHocLaunchSpec` | No `--effort` — CLI default |
|
||||
| Planning | (planning start/resume) | No `--effort`; uses `--permission-mode default`, **not** `plan` |
|
||||
| List handler | `GetMergeHelperLaunchSpec` / `GetMergeHelperHandoffLaunchSpec` | Model + effort fixed per role (`ModelRegistry.HandlerTriageAlias`/`HandlerWaitAlias`/`HandlerMergeAlias`), **not** from list config; `--permission-mode` via `PermissionModeResolver` (unattended) |
|
||||
|
||||
## ⚠️ Gotcha: never pass task free-text as a CLI argument
|
||||
@@ -39,9 +39,10 @@ description skips the file **and** the positional arg entirely — it still gets
|
||||
|
||||
## Argument ordering
|
||||
|
||||
Every spec passes `--effort <level>` from the relevant model's preset. It **leads** the args —
|
||||
except for a fresh task session with a brief, where `--add-dir <sessionDir>` must come first so
|
||||
`--effort` (a single-value flag) can sit directly before the positional kickoff.
|
||||
Only the list-handler specs pass `--effort` (from the role model's preset). Hand-driven sessions
|
||||
(task, ad-hoc, planning, handoff, resume) deliberately pass none, so the CLI default / the user's
|
||||
`/effort` setting applies. A positional kickoff must follow a single-value flag (`--session-id`,
|
||||
`--append-system-prompt-file`), never a variadic one (`--add-dir`, `--allowedTools`).
|
||||
|
||||
`--model` is deliberately **NOT** forced on an interactive session — the user can still switch
|
||||
models in the TUI.
|
||||
@@ -87,7 +88,7 @@ own transcript:
|
||||
`TaskRepository.SetInteractiveSessionIdAsync` — **before** the `LaunchSpec` is returned, i.e.
|
||||
before the ConPTY host ever spawns `claude`. `BuildFreshTaskArgsAsync` then passes it as
|
||||
`--session-id <guid>`, placed as the single-value flag directly before the positional kickoff
|
||||
(or, with no brief, right after `--effort`).
|
||||
(or, with no brief, as the only arg).
|
||||
3. **Fresh worktree wins.** `isFreshWorktree` forces `run` to `null` *and* is checked before
|
||||
reading `task.InteractiveSessionId`, so a recreated worktree never resumes a stale id from
|
||||
either source — it always takes the fresh path, which overwrites the stale
|
||||
@@ -355,7 +356,7 @@ Autonomous and interactive sessions do **not** share a system prompt. Per start
|
||||
|---|---|---|
|
||||
| Autonomous run/continue/retry | `TaskRunner.ResolveConfigAsync` → `ClaudeArgsBuilder.Build` | `--append-system-prompt <text>`, recomputed and re-sent on **every** invocation including a `--resume` continue (`PromptKind.System` + improvement/list/task overrides) |
|
||||
| Interactive task session, fresh | `InteractiveLaunchSpecService.BuildForTaskAsync` → `BuildFreshTaskArgsAsync` | none — no `--append-system-prompt(-file)` at all |
|
||||
| Interactive task session, resume | `InteractiveLaunchSpecService.BuildForTaskAsync` → `WindowsTerminalLauncher.BuildResumeArgs` | none — only `--resume <id>` (+ `--effort`) |
|
||||
| Interactive task session, resume | `InteractiveLaunchSpecService.BuildForTaskAsync` → `WindowsTerminalLauncher.BuildResumeArgs` | none — only `--resume <id>` |
|
||||
| Ad-hoc directory session | `InteractiveLaunchSpecService.BuildForDirectoryAsync` | none |
|
||||
| Planning session start | `InteractiveLaunchSpecService.BuildPlanningStart` → `WindowsTerminalLauncher.BuildPlanningStartArgs` | `--append-system-prompt-file <path>` (`PromptKind.Planning`) |
|
||||
| Planning session resume | `InteractiveLaunchSpecService.BuildPlanningResume` → `WindowsTerminalLauncher.BuildPlanningResumeArgs` | none — only `--permission-mode default --allowedTools <planning allowlist> --resume <id>` |
|
||||
|
||||
+1
-1
@@ -32,7 +32,7 @@ Der Sichtprüfungs-Block dieses Batches (Ctrl+K/`#`, Titel-Edit, ConPTY-/Refine-
|
||||
Interactive-Chip, Diff-Viewer-Abstände, Manual-Tasks, Vorgaben-pro-Modell-Tabelle) ist von Mika
|
||||
verifiziert und deshalb hier entfernt. Es bleiben die Entscheidungen, die daran hängen:
|
||||
|
||||
- Interaktive ConPTY-Sessions bekommen `--effort`, aber **kein** `--model` — die Session läuft weiter unter dem Modell aus Mikas Claude-Config, der Effort kommt aus dem Preset des Modells, das ClaudeDo für die Task auflösen würde. Falls ClaudeDo auch interaktiv das Modell erzwingen soll, ist das ein Folge-Task.
|
||||
- Interaktive (handgesteuerte) ConPTY-Sessions bekommen weder `--model` noch `--effort` — beides kommt aus Mikas Claude-Config bzw. dem CLI-Default. Nur die List-Handler-Sessions pinnen Modell + Effort pro Rolle.
|
||||
- `AppSettings.DefaultMaxTurns` ist jetzt tatsächlich verdrahtet (`ModelPresets.For(..., global.DefaultMaxTurns)` in `TaskRunner.ResolveConfigAsync`): reiner Fallback für ein Modell, das auch nach `ModelRegistry.TryNormalizeAlias` auf keine Preset-Zeile trifft — vorher war das Feld tot (hartkodierte 30). Hat weiterhin keinen eigenen Editor mehr (nur die Preset-Tabelle pro Modell); Spalte könnte später entfallen, falls das nie zutrifft.
|
||||
|
||||
## Beobachtung (offen — Entscheidung Mika)
|
||||
|
||||
@@ -170,11 +170,6 @@ public sealed class InteractiveLaunchSpecService
|
||||
var resolvedClaude = WindowsTerminalLauncher.Resolve(_claudePath)
|
||||
?? throw new InvalidOperationException($"claude executable not found: {_claudePath}");
|
||||
|
||||
// Start the hand-driven session at the effort configured for the model this task would run
|
||||
// under, instead of inheriting whatever the user's global Claude Code config happens to be.
|
||||
// The model itself is deliberately NOT forced here — the user can still switch it in the TUI.
|
||||
var effort = EffortFor(globalSettings, task.Model ?? listConfig?.Model);
|
||||
|
||||
// Resume this task's own last interactive conversation when it has one -- it takes
|
||||
// precedence over the latest autonomous run's session, since an interactive session is a
|
||||
// distinct conversation from an autonomous run even against the same worktree. Fall back
|
||||
@@ -193,13 +188,13 @@ public sealed class InteractiveLaunchSpecService
|
||||
if (resumeSessionId is { Length: > 0 })
|
||||
{
|
||||
sessionId = resumeSessionId;
|
||||
args = WithEffort(WindowsTerminalLauncher.BuildResumeArgs(resumeSessionId), effort);
|
||||
args = WindowsTerminalLauncher.BuildResumeArgs(resumeSessionId);
|
||||
}
|
||||
else
|
||||
{
|
||||
sessionId = Guid.NewGuid().ToString();
|
||||
await new TaskRepository(ctx).SetInteractiveSessionIdAsync(taskId, sessionId, ct);
|
||||
args = await BuildFreshTaskArgsAsync(task, effort, sessionId, ct);
|
||||
args = await BuildFreshTaskArgsAsync(task, sessionId, ct);
|
||||
}
|
||||
|
||||
if (paneId is not null)
|
||||
@@ -242,8 +237,7 @@ public sealed class InteractiveLaunchSpecService
|
||||
|
||||
return new LaunchSpec(
|
||||
ctx.WorkingDir, resolvedClaude,
|
||||
WithEffort(WindowsTerminalLauncher.BuildPlanningStartArgs(ctx),
|
||||
EffortFor(ReadSettings(), ModelRegistry.PlanningAlias)),
|
||||
WindowsTerminalLauncher.BuildPlanningStartArgs(ctx),
|
||||
env);
|
||||
}
|
||||
|
||||
@@ -262,8 +256,7 @@ public sealed class InteractiveLaunchSpecService
|
||||
|
||||
return new LaunchSpec(
|
||||
ctx.WorkingDir, resolvedClaude,
|
||||
WithEffort(WindowsTerminalLauncher.BuildPlanningResumeArgs(ctx.ClaudeSessionId),
|
||||
EffortFor(ReadSettings(), ModelRegistry.PlanningAlias)),
|
||||
WindowsTerminalLauncher.BuildPlanningResumeArgs(ctx.ClaudeSessionId),
|
||||
env);
|
||||
}
|
||||
|
||||
@@ -280,15 +273,13 @@ public sealed class InteractiveLaunchSpecService
|
||||
?? throw new InvalidOperationException($"claude executable not found: {_claudePath}");
|
||||
|
||||
await using var ctx = await _dbFactory.CreateDbContextAsync(ct);
|
||||
var settings = await new AppSettingsRepository(ctx).GetAsync(ct);
|
||||
|
||||
var env = new Dictionary<string, string>
|
||||
{
|
||||
["MCP_TOOL_TIMEOUT"] = McpToolTimeoutMs,
|
||||
};
|
||||
|
||||
// No task and no list here — the global default model's preset decides the effort.
|
||||
var args = WithEffort(Array.Empty<string>(), EffortFor(settings, settings.DefaultModel));
|
||||
IReadOnlyList<string> args = [];
|
||||
if (paneId is not null)
|
||||
{
|
||||
// Pre-assigned so the pane entry can carry it -- a handoff records it as the brief's origin.
|
||||
@@ -322,16 +313,13 @@ public sealed class InteractiveLaunchSpecService
|
||||
?? throw new InvalidOperationException($"claude executable not found: {_claudePath}");
|
||||
|
||||
await using var ctx = await _dbFactory.CreateDbContextAsync(ct);
|
||||
var settings = await new AppSettingsRepository(ctx).GetAsync(ct);
|
||||
var sessionId = Guid.NewGuid().ToString();
|
||||
|
||||
var model = settings.DefaultModel;
|
||||
TaskEntity? task = null;
|
||||
if (taskId is not null)
|
||||
{
|
||||
var taskRepo = new TaskRepository(ctx);
|
||||
task = await taskRepo.GetByIdAsync(taskId, ct) ?? throw new KeyNotFoundException($"Task not found: {taskId}");
|
||||
model = task.Model ?? (await new ListRepository(ctx).GetConfigAsync(task.ListId, ct))?.Model;
|
||||
await taskRepo.SetInteractiveSessionIdAsync(taskId, sessionId, ct);
|
||||
}
|
||||
|
||||
@@ -352,7 +340,6 @@ public sealed class InteractiveLaunchSpecService
|
||||
var args = new List<string>(await PaneSessionArgsAsync(paneId, sessionId, ct))
|
||||
{
|
||||
"--add-dir", Path.GetDirectoryName(briefPath)!,
|
||||
"--effort", EffortFor(settings, model),
|
||||
"--session-id", sessionId,
|
||||
$"Read the file {briefPath} first. It is the handoff brief from the session that worked here " +
|
||||
"before you. After reading it, continue the work as it describes.",
|
||||
@@ -379,12 +366,11 @@ public sealed class InteractiveLaunchSpecService
|
||||
|
||||
var resolvedClaude = WindowsTerminalLauncher.Resolve(_claudePath)
|
||||
?? throw new InvalidOperationException($"claude executable not found: {_claudePath}");
|
||||
var settings = await new AppSettingsRepository(ctx).GetAsync(ct);
|
||||
|
||||
IReadOnlyList<string> args =
|
||||
[
|
||||
.. await PaneSessionArgsAsync(paneId, sessionId, ct),
|
||||
.. WithEffort(WindowsTerminalLauncher.BuildResumeArgs(sessionId), EffortFor(settings, settings.DefaultModel)),
|
||||
.. WindowsTerminalLauncher.BuildResumeArgs(sessionId),
|
||||
];
|
||||
var env = new Dictionary<string, string>
|
||||
{
|
||||
@@ -836,21 +822,6 @@ public sealed class InteractiveLaunchSpecService
|
||||
private static string EffortFor(AppSettingsEntity settings, string? model)
|
||||
=> ModelPresets.For(settings.ModelPresets, model ?? settings.DefaultModel).Effort;
|
||||
|
||||
// Prepends `--effort <level>`. It has to lead: a positional kickoff prompt must stay last, and
|
||||
// it may only follow a single-value flag — a variadic flag would swallow it.
|
||||
private static IReadOnlyList<string> WithEffort(IReadOnlyList<string> args, string effort)
|
||||
{
|
||||
var result = new List<string>(args.Count + 2) { "--effort", effort };
|
||||
result.AddRange(args);
|
||||
return result;
|
||||
}
|
||||
|
||||
private AppSettingsEntity ReadSettings()
|
||||
{
|
||||
using var ctx = _dbFactory.CreateDbContext();
|
||||
return new AppSettingsRepository(ctx).GetAsync().GetAwaiter().GetResult();
|
||||
}
|
||||
|
||||
// The task's brief (title + description, unmodified/multi-line) must never travel as a CLI
|
||||
// argument: the ConPTY host flattens Args into one command line to spawn the process, and
|
||||
// claude re-splits that line on whitespace, so any token in the description that starts with
|
||||
@@ -858,18 +829,18 @@ public sealed class InteractiveLaunchSpecService
|
||||
// the brief at its first line even if it didn't. So it's written to a file instead --
|
||||
// same pattern as BuildForMergeHelperAsync / WindowsTerminalLauncher.BuildPlanningStartArgs --
|
||||
// and claude is pointed at it with a single-line kickoff. --add-dir exposes the session dir to
|
||||
// Read; --effort (single-value) must sit directly before the positional kickoff so the
|
||||
// Read; --session-id (single-value) must sit directly before the positional kickoff so the
|
||||
// preceding variadic --add-dir doesn't swallow the kickoff as another directory.
|
||||
// No brief (task has neither a title nor a description) -> no positional arg at all.
|
||||
// `--session-id` pre-assigns the claude session id the caller already persisted (see
|
||||
// BuildForTaskAsync) so this fresh conversation is resumable from its very first turn --
|
||||
// it's a single-value flag, so it may sit directly before the positional kickoff.
|
||||
private static async Task<IReadOnlyList<string>> BuildFreshTaskArgsAsync(
|
||||
TaskEntity task, string effort, string sessionId, CancellationToken ct)
|
||||
TaskEntity task, string sessionId, CancellationToken ct)
|
||||
{
|
||||
var brief = BuildTaskBrief(task);
|
||||
if (string.IsNullOrEmpty(brief))
|
||||
return new[] { "--effort", effort, "--session-id", sessionId };
|
||||
return new[] { "--session-id", sessionId };
|
||||
|
||||
var sessionDir = Path.Combine(Paths.AppDataRoot(), "task-sessions", task.Id);
|
||||
Directory.CreateDirectory(sessionDir);
|
||||
@@ -879,7 +850,6 @@ public sealed class InteractiveLaunchSpecService
|
||||
return new[]
|
||||
{
|
||||
"--add-dir", sessionDir,
|
||||
"--effort", effort,
|
||||
"--session-id", sessionId,
|
||||
$"Read the file {briefPath} first. It contains the task you must work on. " +
|
||||
"Then, before changing anything, reply with a one-line restatement of what you intend to do " +
|
||||
|
||||
@@ -215,7 +215,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable
|
||||
var secondSpec = await svc.BuildForTaskAsync(taskId, CancellationToken.None);
|
||||
|
||||
Assert.Equal(_tempDir, secondSpec.Cwd);
|
||||
Assert.Equal(new[] { "--resume", firstSessionId }, ArgsAfterEffort(secondSpec));
|
||||
Assert.Equal(new[] { "--resume", firstSessionId }, HandDrivenArgs(secondSpec));
|
||||
|
||||
using var readCtx = _db.CreateContext();
|
||||
Assert.Null(await new WorktreeRepository(readCtx).GetByTaskIdAsync(taskId));
|
||||
@@ -352,7 +352,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable
|
||||
|
||||
Assert.Equal(_worktreeDir, spec.Cwd);
|
||||
Assert.Equal(_claudeStubPath, spec.Exe);
|
||||
Assert.Equal(new[] { "--resume", "sess-123" }, ArgsAfterEffort(spec));
|
||||
Assert.Equal(new[] { "--resume", "sess-123" }, HandDrivenArgs(spec));
|
||||
Assert.Equal(InteractiveLaunchSpecService.McpToolTimeoutMs, spec.Env["MCP_TOOL_TIMEOUT"]);
|
||||
}
|
||||
|
||||
@@ -368,7 +368,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable
|
||||
var svc = BuildService();
|
||||
var spec = await svc.BuildForTaskAsync(taskId, CancellationToken.None);
|
||||
|
||||
Assert.Equal(new[] { "--resume", "sess-kept" }, ArgsAfterEffort(spec));
|
||||
Assert.Equal(new[] { "--resume", "sess-kept" }, HandDrivenArgs(spec));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
@@ -427,7 +427,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable
|
||||
var svc = BuildService();
|
||||
var spec = await svc.BuildForTaskAsync(taskId, CancellationToken.None);
|
||||
|
||||
Assert.Equal(new[] { "--resume", "interactive-session-1" }, ArgsAfterEffort(spec));
|
||||
Assert.Equal(new[] { "--resume", "interactive-session-1" }, HandDrivenArgs(spec));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
@@ -447,7 +447,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable
|
||||
// start a new conversation.
|
||||
var secondSpec = await svc.BuildForTaskAsync(taskId, CancellationToken.None);
|
||||
|
||||
Assert.Equal(new[] { "--resume", firstSessionId }, ArgsAfterEffort(secondSpec));
|
||||
Assert.Equal(new[] { "--resume", firstSessionId }, HandDrivenArgs(secondSpec));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
@@ -544,7 +544,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable
|
||||
var spec = await svc.BuildForTaskAsync(taskId, CancellationToken.None);
|
||||
|
||||
// no brief -> no --add-dir, no positional arg -- but a session id is still pre-assigned
|
||||
var rest = ArgsAfterEffort(spec);
|
||||
var rest = HandDrivenArgs(spec);
|
||||
Assert.Equal("--session-id", rest[0]);
|
||||
Assert.Equal(rest[1], await ReadInteractiveSessionIdAsync(taskId));
|
||||
}
|
||||
@@ -582,7 +582,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable
|
||||
|
||||
Assert.Equal(_tempDir, spec.Cwd);
|
||||
Assert.Equal(_claudeStubPath, spec.Exe);
|
||||
Assert.Empty(ArgsAfterEffort(spec));
|
||||
Assert.Empty(HandDrivenArgs(spec));
|
||||
Assert.Equal(InteractiveLaunchSpecService.McpToolTimeoutMs, spec.Env["MCP_TOOL_TIMEOUT"]);
|
||||
Assert.Empty(_seeder.Calls);
|
||||
}
|
||||
@@ -761,33 +761,28 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable
|
||||
|
||||
// ── Merge helper ──
|
||||
|
||||
/// Every ConPTY spec now leads with `--effort <level>` from the per-model preset (the seeded
|
||||
/// settings row has no overrides, so the shipped default for the default model applies).
|
||||
/// Asserts that pair and returns the rest of the args for the test's own comparison.
|
||||
private static string[] ArgsAfterEffort(LaunchSpec spec)
|
||||
/// Hand-driven ConPTY specs never pass `--effort` -- the CLI's own default (or the user's
|
||||
/// /effort setting) applies. Asserts that and returns the args for the test's own comparison.
|
||||
private static string[] HandDrivenArgs(LaunchSpec spec)
|
||||
{
|
||||
var args = spec.Args.ToList();
|
||||
Assert.Equal("--effort", args[0]);
|
||||
Assert.Equal(ModelPresets.For(ModelPresets.Defaults, ModelRegistry.DefaultAlias).Effort, args[1]);
|
||||
return args.Skip(2).ToArray();
|
||||
Assert.DoesNotContain("--effort", spec.Args);
|
||||
return spec.Args.ToArray();
|
||||
}
|
||||
|
||||
/// A fresh (never-resumed) task with a title/description produces
|
||||
/// `--add-dir <sessionDir> --effort <level> --session-id <guid> <kickoff>` -- the brief
|
||||
/// `--add-dir <sessionDir> --session-id <guid> <kickoff>` -- the brief
|
||||
/// travels via the file at <sessionDir>/brief.md, never as a raw CLI argument. Asserts that
|
||||
/// exact shape and returns (sessionDir, briefPath, kickoff, sessionId) for the test's own
|
||||
/// checks. Cleanup happens via `_seededTaskIds` in Dispose (the session dir is keyed by task id).
|
||||
private static (string SessionDir, string BriefPath, string Kickoff, string SessionId) ParseFreshTaskArgs(LaunchSpec spec)
|
||||
{
|
||||
var args = spec.Args.ToList();
|
||||
Assert.Equal(7, args.Count);
|
||||
Assert.Equal(5, args.Count);
|
||||
Assert.Equal("--add-dir", args[0]);
|
||||
var sessionDir = args[1];
|
||||
Assert.Equal("--effort", args[2]);
|
||||
Assert.Equal(ModelPresets.For(ModelPresets.Defaults, ModelRegistry.DefaultAlias).Effort, args[3]);
|
||||
Assert.Equal("--session-id", args[4]);
|
||||
var sessionId = args[5];
|
||||
var kickoff = args[6];
|
||||
Assert.Equal("--session-id", args[2]);
|
||||
var sessionId = args[3];
|
||||
var kickoff = args[4];
|
||||
|
||||
var briefPath = Path.Combine(sessionDir, "brief.md");
|
||||
return (sessionDir, briefPath, kickoff, sessionId);
|
||||
@@ -1562,7 +1557,7 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable
|
||||
|
||||
var spec = BuildService().BuildPlanningResume(ctx);
|
||||
|
||||
var args = ArgsAfterEffort(spec);
|
||||
var args = HandDrivenArgs(spec);
|
||||
Assert.Equal("--permission-mode", args[0]);
|
||||
Assert.Equal("default", args[1]);
|
||||
Assert.Equal("--allowedTools", args[2]);
|
||||
|
||||
Reference in New Issue
Block a user