Skip to content

Commit 24f7cc0

Browse files
committed
fix: address review findings on the workflow and mission-control changes
reloadSkills refreshed the registry but not the rendered prompt, so a workflow saved mid-session was invocable as `/<name>` and still absent from the skill listing the model reads. Agent gains a prompt-only re-render that leaves the active tool set and turn limit alone, and remembers the profile its prompt was built from so a main agent on a custom profile is not re-rendered as the default. A member row recorded only the last line of a delta, so a provider that packed several closed lines into one chunk lost the rest. Every closed line is now its own event; the unclosed tail is shown but is not one. refreshSkillCommands applied whichever listing finished last. Several callers start it without awaiting, so a slow list for the session the user just left could land on the one they switched to. A generation guard drops stale results, and a failure now clears commands that belong to another session. Applying experimental flags rebuilt the command set before reloadSession, so it read the registry the reload was about to replace. Also: bump the SDK for the new public reloadSkills, note the auto-mode approval exception in the config reference, split the DynamicWorkflow reference into one idea per paragraph, and format changelog values as code.
1 parent 560c82f commit 24f7cc0

13 files changed

Lines changed: 252 additions & 44 deletions

File tree

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
---
22
"@pythoughts/pythinker-code": patch
3+
"@pythoughts/pythinker-code-sdk": minor
34
---
45

5-
Fix `/workflow save` leaving the saved workflow uncallable until the session was reloaded.
6+
Fix `/workflow save` leaving the saved workflow uncallable until the session was reloaded, and add `Session.reloadSkills()` to re-discover skills written while a session is open.

‎apps/pythinker-code/CHANGELOG.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@
5757

5858
- [#32](https://github.com/Pythoughts-labs/pythinker-code/pull/32) [`a504a82`](https://github.com/Pythoughts-labs/pythinker-code/commit/a504a820c4d9db14e213f4a021c86b048c4b916d) - Accept a provider's plain id for `--provider` at login, so a catalog provider no longer has to be named by its full display name, and stop a cancelled OpenAI Codex sign-in from holding the process open for the rest of its two-minute callback timeout. In the editor extension, signing in now shows one cancellable progress notification, a repeated sign-in joins the one already running instead of opening a second set of prompts, and a completed sign-in is no longer reported as failed when the status refresh behind it fails.
5959

60-
- [#32](https://github.com/Pythoughts-labs/pythinker-code/pull/32) [`a504a82`](https://github.com/Pythoughts-labs/pythinker-code/commit/a504a820c4d9db14e213f4a021c86b048c4b916d) - Save the thinking-effort level picked during login. Only an on/off flag was stored, so choosing low, medium, or xhigh reopened the session at high, and an OpenAI Codex login reopened at the model's maximum effort regardless of the choice.
60+
- [#32](https://github.com/Pythoughts-labs/pythinker-code/pull/32) [`a504a82`](https://github.com/Pythoughts-labs/pythinker-code/commit/a504a820c4d9db14e213f4a021c86b048c4b916d) - Save the thinking-effort level picked during login. Only an on/off flag was stored, so choosing `low`, `medium`, or `xhigh` reopened the session at `high`, and an OpenAI Codex login reopened at the model's maximum effort regardless of the choice.
6161

6262
- [#32](https://github.com/Pythoughts-labs/pythinker-code/pull/32) [`a504a82`](https://github.com/Pythoughts-labs/pythinker-code/commit/a504a820c4d9db14e213f4a021c86b048c4b916d) - Write the thinking effort picked at login to disk. The apply step recorded the level, but the patch that saved the result listed everything except it, so an API-key login still reopened at the default effort. Choosing `off` now also clears a level a previous login left behind, which a patch that only merges could not do by omitting the key.
6363

@@ -127,7 +127,7 @@
127127

128128
- [`c0f0976`](https://github.com/Pythoughts-labs/pythinker-code/commit/c0f09769e76c92002ca9b9a09d9cb820750f1046) - Remove the Pythinker Datasource plugin from the marketplace; its data gateway backend is not available, so every datasource query failed.
129129

130-
- [`c0f0976`](https://github.com/Pythoughts-labs/pythinker-code/commit/c0f09769e76c92002ca9b9a09d9cb820750f1046) - Enable automatic updates for native installs on Windows: /update now installs the new version in the background instead of printing a manual command, and the installer safely replaces the running executable.
130+
- [`c0f0976`](https://github.com/Pythoughts-labs/pythinker-code/commit/c0f09769e76c92002ca9b9a09d9cb820750f1046) - Enable automatic updates for native installs on Windows: `/update` now installs the new version in the background instead of printing a manual command, and the installer safely replaces the running executable.
131131

132132
### Patch Changes
133133

‎apps/pythinker-code/src/tui/commands/config.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1198,15 +1198,18 @@ export async function applyExperimentalFeatureChanges(
11981198
await host.harness.setConfig({ experimental });
11991199
const features = await host.harness.getExperimentalFeatures();
12001200
setExperimentalFeatures(features);
1201-
await host.refreshSkillCommands(host.session);
12021201
host.restoreEditor();
12031202
if (host.session !== undefined) {
12041203
await host.session.reloadSession();
1204+
// After the reload, never before: a flag can gate which skills exist, so
1205+
// rebuilding first read the registry the reload was about to replace.
1206+
await host.refreshSkillCommands(host.session);
12051207
await host.reloadCurrentSessionView(
12061208
host.session,
12071209
'Experimental features updated. Session reloaded.',
12081210
);
12091211
} else {
1212+
await host.refreshSkillCommands(undefined);
12101213
host.showStatus('Experimental features updated.', 'success');
12111214
}
12121215
host.track('experimental_features_apply', { changed: changes.length });

‎apps/pythinker-code/src/tui/components/messages/dynamic-workflow-mission-control.ts‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,6 @@ export class DynamicWorkflowMissionControlComponent implements Component {
281281
const member = this.findMemberByAgentId(input.agentId);
282282
if (member === undefined || isTerminalPhase(member.phase) || input.delta.length === 0) return;
283283
this.markStarted(input.agentId);
284-
const recordActivity = input.delta.includes('\n') || member.latest.length === 0;
285284
member.lastEventAtMs = Date.now();
286285
const combined = `${member.carry}${input.delta}`;
287286
// Only the text after the last newline is still being written. A delta that
@@ -291,7 +290,21 @@ export class DynamicWorkflowMissionControlComponent implements Component {
291290
const newlineIndex = combined.lastIndexOf('\n');
292291
const pending = newlineIndex < 0 ? combined : combined.slice(newlineIndex + 1);
293292
member.carry = clampLine(pending);
294-
this.setLatest(member, clampLine(latestNonEmptyLine(combined)), recordActivity);
293+
294+
// Every line the delta closed is an event of its own. Recording only the
295+
// last one dropped whole lines whenever a provider sent several in one
296+
// chunk, so the same agent showed less activity on a batching provider than
297+
// on one that streams a token at a time.
298+
if (newlineIndex >= 0) {
299+
for (const line of combined.slice(0, newlineIndex).split('\n')) {
300+
this.setLatest(member, clampLine(line), true);
301+
}
302+
}
303+
// The unclosed tail is shown but is not an event yet — except as the row's
304+
// first text, which would otherwise leave the row blank until a newline.
305+
if (pending.length > 0) {
306+
this.setLatest(member, clampLine(pending), member.latest.length === 0);
307+
}
295308
}
296309

297310
markSuspended(input: {
@@ -1065,14 +1078,6 @@ function elapsedSeconds(startedAtMs: number, endedAtMs: number): number {
10651078
return Math.floor(Math.max(0, endedAtMs - startedAtMs) / 1_000);
10661079
}
10671080

1068-
function latestNonEmptyLine(text: string): string {
1069-
for (const line of text.split(/\r?\n/).toReversed()) {
1070-
const normalized = normalizeText(line);
1071-
if (normalized.length > 0) return normalized;
1072-
}
1073-
return '';
1074-
}
1075-
10761081
/**
10771082
* Keeps the head of one streamed line. The row shows the head and clips the
10781083
* rest, so dropping the tail is invisible — and it is the only bound on a line

‎apps/pythinker-code/src/tui/pythinker-tui.ts‎

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import {
4747
sortSlashCommands,
4848
type PythinkerSlashCommand,
4949
type SkillListSession,
50+
type SkillSlashCommands,
5051
} from './commands';
5152
import {
5253
isExperimentalFlagEnabled,
@@ -259,6 +260,10 @@ export class PythinkerTUI {
259260
private readonly reverseRpcDisposers: Array<() => void> = [];
260261
private skillCommands: readonly PythinkerSlashCommand[] = [];
261262
readonly skillCommandMap = new Map<string, string>();
263+
/** Bumped per refresh so a slow one cannot apply over a newer one. */
264+
private skillCommandRefresh = 0;
265+
/** The session whose skills the commands on screen were built from. */
266+
private skillCommandSession: SkillListSession | undefined;
262267
private readonly imageStore = new ImageAttachmentStore();
263268
private fdPath: string | null = detectFdPath();
264269
private fdDownloadStarted = false;
@@ -438,23 +443,43 @@ export class PythinkerTUI {
438443
* never rebuilt from a skill list that has moved on.
439444
*/
440445
async refreshSkillCommands(session?: SkillListSession): Promise<void> {
446+
const refresh = (this.skillCommandRefresh += 1);
441447
if (session === undefined) {
448+
this.skillCommandSession = undefined;
442449
this.skillCommands = [];
443450
this.skillCommandMap.clear();
444451
this.setupAutocomplete();
445452
return;
446453
}
447454

455+
let built: SkillSlashCommands | undefined;
448456
try {
449-
const skillCommands = buildSkillSlashCommands(await session.listSkills());
450-
this.skillCommands = skillCommands.commands;
451-
this.skillCommandMap.clear();
452-
for (const [commandName, skillName] of skillCommands.commandMap) {
453-
this.skillCommandMap.set(commandName, skillName);
454-
}
457+
built = buildSkillSlashCommands(await session.listSkills());
455458
} catch {
456-
// Keep the skills already known. The builtin command set may still have
457-
// changed, so the autocomplete provider is rebuilt either way.
459+
// What to do about a failure depends on which session the commands on
460+
// screen came from, and that is only decided after the await below.
461+
}
462+
463+
// A refresh that started later has already applied. Several callers start
464+
// this without awaiting it, so a slow list for the session the user just
465+
// left would otherwise land on top of the one they switched to.
466+
if (refresh !== this.skillCommandRefresh) return;
467+
468+
if (built === undefined && this.skillCommandSession === session) {
469+
// A transient failure for the session already on screen: its commands are
470+
// still the right ones, so keep them. The builtin command set may still
471+
// have changed, so the autocomplete provider is rebuilt either way.
472+
this.setupAutocomplete();
473+
return;
474+
}
475+
476+
// Either it listed, or it failed for a session whose skills were never on
477+
// screen — keeping another session's commands would be worse than none.
478+
this.skillCommandSession = built === undefined ? undefined : session;
479+
this.skillCommands = built?.commands ?? [];
480+
this.skillCommandMap.clear();
481+
for (const [commandName, skillName] of built?.commandMap ?? []) {
482+
this.skillCommandMap.set(commandName, skillName);
458483
}
459484
this.setupAutocomplete();
460485
}

‎apps/pythinker-code/test/tui/commands/experiments.test.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,11 @@ describe('experimental feature command handlers', () => {
8888
expect(host.refreshSkillCommands).toHaveBeenCalled();
8989
expect(host.restoreEditor).toHaveBeenCalled();
9090
expect(host.session.reloadSession).toHaveBeenCalledOnce();
91+
// A flag can gate which skills exist, so rebuilding the command set before
92+
// the reload read the registry the reload was about to replace.
93+
expect(host.session.reloadSession.mock.invocationCallOrder[0]).toBeLessThan(
94+
host.refreshSkillCommands.mock.invocationCallOrder[0] ?? 0,
95+
);
9196
expect(host.reloadCurrentSessionView).toHaveBeenCalledWith(
9297
host.session,
9398
'Experimental features updated. Session reloaded.',

‎apps/pythinker-code/test/tui/components/messages/dynamic-workflow-mission-control.test.ts‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -943,6 +943,44 @@ describe('DynamicWorkflowMissionControlComponent', () => {
943943
expect(line).toContain("I've read the files");
944944
});
945945

946+
it('records every line a provider packs into one delta', () => {
947+
const component = createComponent();
948+
component.updateArgs({ items: ['Work'] });
949+
component.markInputComplete();
950+
register(component, 'agent-1');
951+
component.markStarted('agent-1');
952+
953+
// A provider that batches sends whole lines at once. Only the last one used
954+
// to reach the activity list, so the same run showed less on that provider.
955+
component.appendModelDelta({
956+
agentId: 'agent-1',
957+
delta: 'First line\nSecond line\nThird line\n',
958+
});
959+
960+
const output = renderText(component, 200);
961+
expect(output).toContain('First line');
962+
expect(output).toContain('Second line');
963+
expect(output).toContain('Third line');
964+
});
965+
966+
it('does not report an unfinished line as an event of its own', () => {
967+
const component = createComponent();
968+
component.updateArgs({ items: ['Work'] });
969+
component.markInputComplete();
970+
register(component, 'agent-1');
971+
component.markStarted('agent-1');
972+
component.appendModelDelta({ agentId: 'agent-1', delta: 'Closed line\nstill wri' });
973+
974+
// The row shows the tail as it arrives, but the activity list only carries
975+
// the line the delta actually closed.
976+
expect(memberLine(renderText(component, 200), 1)).toContain('still wri');
977+
const activity = renderText(component, 200).split('\n').filter(
978+
(line) => /^│\s*001 \+/u.test(line),
979+
);
980+
expect(activity.join('\n')).toContain('Closed line');
981+
expect(activity.join('\n')).not.toContain('still wri');
982+
});
983+
946984
it.each([64, 70, 80, 100, 200])(
947985
'keeps the task readable beside a long agent summary at width %i',
948986
(width) => {

‎docs/configuration/config-files.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ Fields in the config file fall into two categories: **top-level scalars** that d
7777
| --- | --- | --- | --- |
7878
| `default_model` | `string` | — | Default model alias; must be defined in `models` |
7979
| `default_thinking` | `boolean` | `false` | Whether new sessions enable Thinking (deep reasoning) mode by default; can be toggled from the model menu inside a session. Even when set to `true`, `[thinking].mode = "off"` will still force Thinking off |
80-
| `default_permission_mode` | `string` | `manual` | Default permission mode for new sessions; one of `manual` (prompt each time), `yolo` (auto-approve tool actions, but the agent may still ask questions), or `auto` (fully autonomous — the agent decides everything without asking) |
80+
| `default_permission_mode` | `string` | `manual` | Default permission mode for new sessions; one of `manual` (prompt each time), `yolo` (auto-approve tool actions, but the agent may still ask questions), or `auto` (fully autonomous — the agent decides everything without asking, except a `DynamicWorkflow` call, which still shows its plan for approval) |
8181
| `default_plan_mode` | `boolean` | `false` | Whether new sessions start in Plan mode (produce a plan before executing) by default |
8282
| `merge_all_available_skills` | `boolean` | `true` | Whether to merge Agent Skills from all available directories |
8383
| `extra_skill_dirs` | `array<string>` | — | Extra skill search directories, layered on top of the default directories |

0 commit comments

Comments
 (0)