feat: bound Dynamic Workflow fan-out and add multi-provider login - #32
Merged
Conversation
Dynamic Workflow can now be turned off entirely through the `disableWorkflows` config key or the PYTHINKER_CODE_DISABLE_WORKFLOWS environment variable, with the environment winning so an operator can force it off without editing config. When disabled the DynamicWorkflow builtin is never registered on an agent, and the TUI drops /workflow from autocomplete and refuses to run it.
The Dynamic Workflow guidance tells the model to decompose work as finely as possible, which is the right default for a power user and the wrong one for someone paying per token. The new `workflowSizeGuideline` config key (and PYTHINKER_CODE_WORKFLOW_SIZE_GUIDELINE) picks an advisory subagent-count target: small, medium (default), large, or unrestricted. The target is wording only — the hard cap of 128 subagents and the scheduler are untouched. It reaches the model through both the tool description and the Dynamic Workflow mode reminder, and says out loud that it supersedes the decompose-freely guidance so the model is not left with two conflicting targets.
The batch scheduler's ramp explicitly did not cap on active tasks, so a 128-item workflow could put 128 subagents in flight at once. Nothing bounded how many subagents a session accumulated over its lifetime, and nothing stopped a subagent from spawning a subagent indefinitely. Adds MAX_CONCURRENT_WORKFLOW_SUBAGENTS (20), enforced in both the normal ramp and the rate-limit phase, plus MAX_SUBAGENTS_PER_SESSION (200) and MAX_SUBAGENT_SPAWN_DEPTH (3) enforced on spawn. The scheduler re-arms its launch timer while the cap blocks the ramp so a freed slot is always picked up, and the two spawn guards throw text written for the model to act on, which the batch turns into one failed task rather than a failed batch.
Every DynamicWorkflow call now generates one run id, carries it to each queued subagent through the same route model and effort already take, and reports it back on the result's root element as run_id. The id is generated at the tool rather than in the batch scheduler so non-workflow batches are never stamped with one, and its charset is locked to lowercase alphanumerics because later work uses it as a filename segment and accepts it back as tool input. The validator ships next to the generator for that code to reuse.
Every `subagent.*` event now carries the `workflowRunId` of the Dynamic Workflow that launched it, and `subagent.spawned` additionally carries the run's user-facing `workflowName`. A consumer watching a session with two concurrent workflows could not previously tell which subagent belonged to which; it can now. The run id also reaches telemetry as `workflow_run_id` on `subagent_created`. Adds `workflow.warning`, an advisory event emitted once before a run whose agent count exceeds the operator's size guideline. It is advisory only: nothing is blocked or cancelled. The threshold is the configured guideline's target, falling back to 25 for `unrestricted` so even an unrestricted operator hears about a very large fan-out. The warning carries `parentToolCallId` — the same key the `subagent.*` events use — so a client can attach it to the workflow that raised it rather than rendering it as a detached global message.
`workflow.warning` was reaching no user. All three client event switches end in `default:`, so an unhandled event type is dropped silently and the compiler says nothing — the wiring had to be added and verified by hand in each client. TUI: the warning lands on the Dynamic Workflow mission-control card for its tool call, as a SYS activity row alongside "Workflow cancelled" and "Workflow failed". When no card is live (the tool call was retired) it falls back to a status line rather than being dropped. Headless `-p` mode ignores it, matching how the existing `warning` event is treated there. Web: projected through the existing agent-warning path, so it joins the warnings the UI already renders. VS Code: a new `WorkflowWarning` wire event carries the run to the webview, where it is stored on the workflow's tool item and rendered as an advisory line above the lanes. Its `parent_tool_call_id` is scoped with the orchestrating agent's id, the same scoping `subagent.spawned` uses, so the warning and the lanes resolve to the same card in a nested workflow. Adding a case to the TUI session-event switch also requires an entry in the parity matrix: `feature-parity.test.ts` scans that switch's source and asserts the matrix covers exactly the cases it finds.
DynamicWorkflow gains an optional `output_schema`. When set, every subagent in the run returns its result by calling the existing StructuredOutput tool instead of writing a prose summary, and the child's result becomes the JSON-serialized structured output. The schema rides the existing turn path: `turn.prompt` already accepts one and already enforces it through StructuredOutputState, so this is plumbing rather than a second validator. One schema per workflow, not per item — `items` is a plain string array and every item runs the same prompt template, so a per-item schema is not expressible in the tool contract. Two interactions this creates are handled explicitly. The short-summary continuation is skipped when a schema is in effect, since the structured output rather than the prose is the deliverable and the follow-up turn would run without a schema. And a child that never produces conforming output is reported as its own `schema_error` outcome rather than a generic failure: it is classified from a typed StructuredOutputMaxRetriesError, not by string-matching an error message, it is counted separately in the run summary because such a child did run, and it never aborts its siblings.
The multi-provider login experience — Kimi device code, OpenAI Codex OAuth, open platforms, catalog providers — lived inside the TUI slash command and took a SlashCommandHost, so it was reachable only from the TUI. Both other surfaces that offer login, `pythinker login` and the VS Code extension, call `harness.auth.login(undefined, ...)` instead, which is Kimi-only by construction and offers the user no choice at all. Move the flows to a LoginUi port so a second renderer can exist without duplicating the glue. `handleLoginCommand` keeps its signature and becomes an adapter that builds a LoginUi from the host; logout is untouched and stays TUI-only, since it is the only part that needs app state. Keep the port renderer-neutral where it counts: the platform-value vocabulary moves out of the TUI platform selector, which reaches the choice picker and the stateful theme and would drag both into every non-TUI caller, and the default provider name comes from the oauth package rather than hopping through a TUI constants re-export. Also fix a bug the move made visible: the Codex flow removed the configured provider before fetching models and before the model picker, so failing to list models, getting an empty list, or simply cancelling the picker signed the user out of a working setup for nothing. The removal now happens once the replacement is certain, matching the other two flows.
`pythinker login` went straight to a Kimi device code and offered no choice, because the CLI called `harness.auth.login(undefined, ...)` — a path that is Kimi-only by construction. The picker the TUI has always had (OAuth and API-key flows, plus every catalog provider) now backs the CLI too, via the LoginUi port, so there is one flow and two renderers rather than two flows. The terminal renderer is built on @clack/prompts and follows the reference CLI's argument design: `--provider <id|name>` matches by id then by display name and skips the picker, and an unknown value fails loudly with the valid ids instead of quietly falling back to a default. `--method` is deliberately absent: this codebase's platform list is flat, so each row already is a provider-and-method pair, and grouping them would invent structure the domain does not have. Without a TTY the command refuses to guess and exits non-zero. Success is now reported by `runLogin` returning whether credentials were written, rather than inferred from a telemetry call — an exit code must not depend on an observability side effect, and the Kimi flow's catch block fell through to a success return, so a failed login exited 0. Two further fixes the port made visible: the Codex flow disarmed its cancel handle before fetching models, leaving SIGINT nothing to abort during that request and the abort check downstream dead; and the login tests asserted exit codes with `toHaveBeenCalledWith`, which always passes because the mocked `process.exit` throws and the catch branch then exits 1. They now pin the first exit call, and API-key login — the path every provider but one uses — has its own coverage. The advertised ACP terminal-auth method described a device-code flow that no longer exists; it now describes provider selection.
The flows sat in apps/pythinker-code, so the VS Code extension could not use them — it depends on @pythoughts/pythinker-code-sdk alone, and still calls harness.auth.login(undefined, ...), which offers no provider choice at all. The SDK is the right home rather than the oauth package: oauth has no @pythoughts dependencies and must keep none, while the flows need fetchCatalog and applyCatalogProvider, which live in the SDK. Putting them in oauth would have inverted that edge into a cycle. Nothing new is added to any package.json. Two couplings dissolved on inspection rather than being carried across. The port declared showStatus(message, level) against a TUI theme token, but no flow ever passes a level, so the parameter and the theme dependency are gone. The remaining app types (spinner handle, api-key prompt options, platform selection) are now declared in the SDK and matched structurally. Generic error formatting moves out of a TUI util into the SDK, re-exported so its existing importers are untouched. Browser opening becomes a port member instead of a helper the flows call directly. It was duplicated byte-for-byte into the SDK during the move, and it was never renderer-agnostic to begin with: a terminal shells out, while an editor extension has to use its own host API. Owning it in the port removes the copy and is what lets a VS Code renderer exist. Also fixes a flaky test rather than leaving it annotated: the skill telemetry assertion awaited a skill.activated event and then read telemetry records that are written independently of it, so a loaded machine lost the race.
Sign-in in the extension called the managed-provider login directly, so it offered no choice of provider at all -- the same bug the CLI had. It now builds a LoginUi from VS Code's own widgets and runs the shared flows, so both surfaces present the same providers. The effort-level rule moves into the SDK rather than being restated per renderer: the two copies had already diverged on an empty effort list and, on the managed path, on whether the three-state thinking declaration outranks the legacy reasoning boolean. Carry a model's declared reasoning efforts into its alias. Without them the picker offered the low/medium/high fallback while the config written straight afterwards recorded the real list, so the two disagreed for OpenAI Codex. Time the catalog fetch out. The editor's progress notification is not cancellable, so unlike the terminal a hung request stranded login with no way out; the bundled catalog is a working fallback. Render a dismissed picker as idle instead of "Login failed". That branch was unreachable while login could only succeed or throw. Drop the hardcoded vendor subscription link from the login screen.
A model building the item list routinely emits a trailing empty string. The per-item minimum length made that fail argument validation, which rejects the whole call before the tool runs -- so no subagent started and every prompt had to be sent again. The tool's own error handling never saw it, because nothing reached the tool. Blank items are now dropped and counted. The results report how many went, so a quietly shorter workflow cannot pass for one the caller sized correctly, and the too-few-items error says the same thing rather than claiming a list of three had fewer than two entries. The launch panel counted raw items, so it advertised subagents that were never going to run -- which is why a rejected call looked like one that started and died. It now counts what will actually launch.
A running row showed a static periwinkle dot -- the same colour the panel uses for row ids and the RUN label, and the same shape a finished row shows. Nothing about it said "this agent is working right now". It is now a dim grey braille spinner, so activity reads as motion and the periwinkle stays for identity. All running rows share the workflow clock, so they spin in step rather than drifting apart by whenever each agent started. The Orchestrating label shimmered a periwinkle highlight across a grey base, so the sweep washed out. Pairing primary with primaryShimmer keeps it periwinkle throughout, in both the dark and light palettes. Only existing palette tokens are used, so the custom-theme schema, docs, and skill token tables are unchanged.
…picked effort Two defects a review swarm surfaced, both verified against source and both reproduced by a failing test before the fix. A workflow subagent lost its output schema whenever a provider rate limit forced its turn to be retried: the retry path never forwarded the schema, so the model was not offered the StructuredOutput tool, answered in prose, and the batch recorded it as completed rather than as a schema failure. The structured-output contract was void on exactly the path most likely to be taken under load, with no trace. Existing coverage only exercised the spawn path, which is why this stayed green. Login stored only an on/off thinking flag, so picking low, medium, or xhigh reopened the session at high, and an OpenAI Codex login reopened at the model's maximum effort no matter what was chosen. The level now travels with the boolean, and a level the model does not declare is still ignored rather than persisted. The TUI's own model picker also built its aliases by hand and dropped the declared efforts, so it now shares the one converter. Also adds the changesets this branch was missing: the size guideline and kill switch, the fan-out caps, run correlation, and the ACP method rename.
…st event The bar was a three-stage ratchet: 20% started, 50% model text, 75% any tool call, 100% finished. Because it only ever advanced, every agent that touched a tool sat at 75% until it ended -- so an agent ten seconds in, one twenty minutes in, and one wedged for good all rendered identically. The number implied a completion fraction nothing can know, since no one knows how many steps an agent will take before it takes them. Each row now shows its tool-call count and how long it has been silent. Both are observed facts rather than estimates, and together they answer the question the bar could not: is this agent still working? Silence past a minute turns amber and past three minutes red, so a stalled row separates itself from a busy one at a glance. No new events were needed -- every tool call and streamed delta already reached the component, which was discarding them into the ratchet.
…re meant to save Three defects, all fallout from dropping blank items rather than rejecting the call, and all in the scenario that change exists to handle. The note explaining the drop was prepended to the tool output, but consumers match the result document anchored at the start of that output. A run that dropped an item therefore parsed as unsupported and rendered as failed despite every subagent succeeding. The note now follows the results, and both sides pin the ordering: the producer asserts the envelope comes first, the renderer asserts it still parses with the note attached. The transcript counted blank entries when sizing the panel, so a dropped item left a row queued forever and held the header below its total. The schema still capped the raw item count, so 128 real prompts plus one blank was rejected whole -- reopening the hole at the boundary. The cap now applies to the items that survive, and going over it fails inside the tool with a readable message instead of discarding every prompt.
Four defects in the login flows, all reachable from a normal sign-in. The editor extension armed a cancel callback and never called it: the progress notification was not cancellable and no prompt received a cancellation token, so an OAuth flow waiting on a browser round trip had no way out at all. One login-wide cancellable notification now owns cancellation, and its token reaches every quick pick and input box. A second sign-in request started a rival flow behind a competing set of prompts; it now joins the one already running. A completed sign-in that failed only on the status refresh afterwards was reported as failed, sending the user back to a screen they had just finished. `--provider` matched a platform id or a display name, but a catalog provider's id carries an internal prefix and its label is a product name, so the id printed everywhere else - deepseek - matched neither. A cancelled OpenAI Codex sign-in returned before tearing its wait down, leaving both the callback timeout and the listening callback server to hold the host event loop for the remainder of two minutes.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 30 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThis change adds shared multi-provider login flows, Dynamic Workflow controls and structured output, workflow warning events, TUI and VS Code updates, release notes, OAuth and catalog validation, and the stop-slop skill. ChangesApplication and workflow updates
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Comment |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (8)
packages/agent-core/src/agent/dynamic-workflow/size-guideline.ts (1)
15-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the accepted env names from an existing source of truth.
WORKFLOW_SIZE_GUIDELINE_NAMESis a third copy of the guideline member list, afterWorkflowSizeGuidelineSchemainpackages/agent-core/src/config/schema.tsandGUIDELINE_TARGETSabove.GUIDELINE_TARGETSis exhaustiveness-checked by itsRecord<WorkflowSizeGuideline, ...>type, but the Set is not. If a new member is added to the enum, the env parser silently keeps rejecting it. Deriving the check fromGUIDELINE_TARGETSalso removes theas WorkflowSizeGuidelinecast.♻️ Proposed refactor
-const WORKFLOW_SIZE_GUIDELINE_NAMES = new Set<string>([ - 'small', - 'medium', - 'large', - 'unrestricted', -]); - /** The numeric subagent-count target for a guideline, or `undefined` for `unrestricted`. */ export function workflowSizeGuidelineTarget(guideline: WorkflowSizeGuideline): number | undefined { return GUIDELINE_TARGETS[guideline]; } function parseWorkflowSizeGuidelineEnv(value: string | undefined): WorkflowSizeGuideline | undefined { const normalized = value?.trim().toLowerCase(); - if (normalized === undefined || !WORKFLOW_SIZE_GUIDELINE_NAMES.has(normalized)) return undefined; - return normalized as WorkflowSizeGuideline; + if (normalized === undefined) return undefined; + return Object.keys(GUIDELINE_TARGETS).find( + (name): name is WorkflowSizeGuideline => name === normalized, + ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/agent-core/src/agent/dynamic-workflow/size-guideline.ts` around lines 15 - 31, Remove the manually maintained WORKFLOW_SIZE_GUIDELINE_NAMES set and derive accepted values from the exhaustiveness-checked GUIDELINE_TARGETS keys. Update parseWorkflowSizeGuidelineEnv to validate normalized input against those keys and return the validated key without the WorkflowSizeGuideline cast, preserving trimming, lowercasing, and undefined handling.packages/agent-core/src/session/subagent-batch.ts (1)
165-169: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard against non-finite
concurrencyLimitvalues.
Math.trunc(NaN)producesNaN, so the batch never starts pending tasks.Infinitydisables the concurrency cap. Treat non-finite input asMAX_CONCURRENT_WORKFLOW_SUBAGENTSand add tests for both cases.Proposed guard
- this.concurrencyLimit = Math.max(1, Math.trunc(concurrencyLimit)); + this.concurrencyLimit = Number.isFinite(concurrencyLimit) + ? Math.max(1, Math.trunc(concurrencyLimit)) + : MAX_CONCURRENT_WORKFLOW_SUBAGENTS;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/agent-core/src/session/subagent-batch.ts` around lines 165 - 169, Update the constructor’s concurrencyLimit normalization to detect non-finite values before truncation, defaulting NaN and Infinity to MAX_CONCURRENT_WORKFLOW_SUBAGENTS while preserving the existing minimum of one for finite values. Add tests covering both non-finite inputs and verifying the batch processes pending tasks with the default limit.packages/node-sdk/src/login/flows.ts (1)
56-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
KIMI_CODE_PLATFORM_IDinstead of the string literal.
packages/node-sdk/src/login/platform-options.tsline 29 exportsKIMI_CODE_PLATFORM_IDand uses it to build the option. This dispatch compares against a copy of that literal. If the id changes, the option stays selectable but the dispatch falls through togetOpenPlatformById, which returnsundefined, andrunLoginreturnsfalsewith no message.♻️ Import the shared constant
-import { catalogProviderIdFromPlatformValue } from './platform-values'; +import { KIMI_CODE_PLATFORM_ID } from './platform-options'; +import { catalogProviderIdFromPlatformValue } from './platform-values';- if (platformId === 'kimi-code') { + if (platformId === KIMI_CODE_PLATFORM_ID) { return handlePythinkerCodeOAuthLogin(ui); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/node-sdk/src/login/flows.ts` around lines 56 - 58, Update the platform dispatch condition in the login flow around handlePythinkerCodeOAuthLogin to compare platformId with the exported KIMI_CODE_PLATFORM_ID constant from platform-options.ts instead of a duplicated string literal.apps/pythinker-code/test/cli/login.test.ts (3)
88-94: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winResolve the oxlint
import(first)warnings at lines 88 and 89.oxlint reports that these import statements must come first. The placement after the
vi.mockcalls is a common Vitest pattern, and Vitest hoistsvi.mockregardless of import position, so the runtime behavior is correct. The lint rule still fails CI.Either move the imports to the top of the file, or add a scoped
oxlint-disable-next-line import/firstcomment that records why the order is intentional.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pythinker-code/test/cli/login.test.ts` around lines 88 - 94, Resolve the import/first lint warnings for the imports from `@clack/prompts` and `@pythoughts/pythinker-code-sdk` in the login test by moving them before the vi.mock calls, or add scoped oxlint-disable-next-line import/first comments documenting the intentional Vitest hoisting pattern.Source: Linters/SAST tools
112-124: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReset every prompt and catalog mock in
beforeEach.
beforeEachresetsmockLogin,mockStatus,mockGetConfig,mockSetConfig,mockRemoveProvider,openUrl,createPythinkerHarness, andselect. It does not resetpassword,text,fetchCatalog, ormockFetchOpenPlatformModels.Line 319 and line 396 set
passwordwith a persistentmockResolvedValue, and line 397 setsmockFetchOpenPlatformModelsthe same way. Those implementations leak into later tests and make the suite order-dependent. A future test that expects the API-key prompt to be unanswered would silently receive'sk-test-key'.♻️ Proposed fix to reset the remaining mocks
vi.mocked(select).mockReset(); + vi.mocked(password).mockReset(); + vi.mocked(text).mockReset(); + vi.mocked(fetchCatalog).mockReset(); + vi.mocked(fetchCatalog).mockRejectedValue(new Error('offline')); + mockFetchOpenPlatformModels.mockReset();Add
textto the import at line 89. Alternatively, enableclearMocksandmockResetin the Vitest config so every mock resets between tests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pythinker-code/test/cli/login.test.ts` around lines 112 - 124, Update the beforeEach setup to reset every prompt and catalog mock, including password, text, fetchCatalog, and mockFetchOpenPlatformModels, alongside the existing resets. Import text if it is not currently available, and use mockReset so persistent implementations from tests such as the API-key prompt and model catalog do not leak into later cases.
312-348: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSplit the display-name case and the bare-id case into two tests.
This
itblock runs two independent scenarios and resetsselect,mockSetConfig, andexitSpyin the middle at lines 333-335. If the display-name assertion at line 329 fails, the bare-id path never runs, and the failure message does not identify which form of--providerbroke.Two separate
itblocks remove the mid-test reset and name each failure precisely.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pythinker-code/test/cli/login.test.ts` around lines 312 - 348, Split the combined test into two independent it blocks: one covering the display-name --provider value and another covering the bare id value. Move each scenario’s setup and assertions into its respective test, remove the mid-test mockReset/mockClear calls, and give each test a name that identifies the provider form being exercised..changeset/login-cancel-and-provider-id.md (1)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSplit this changeset body into separate statements.
Line 5 joins five independent fixes into one sentence: bare provider ids for
--provider, the cancelled OpenAI Codex sign-in no longer holding the process for the callback timeout, the single cancellable progress notification, joining a repeated sign-in, and the status-refresh failure no longer reporting a completed sign-in as failed.Changeset bodies become published release notes. A bulleted list makes each fix findable.
♻️ Proposed rewrite
-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. +Login fixes: + +- `--provider` accepts a provider's plain id. A catalog provider no longer has to be named by its full display name. +- A cancelled OpenAI Codex sign-in no longer holds the process open for the rest of its two-minute callback timeout. +- The editor extension shows one cancellable progress notification during sign-in. +- A repeated sign-in joins the one already running instead of opening a second set of prompts. +- A completed sign-in is no longer reported as failed when the status refresh behind it fails.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.changeset/login-cancel-and-provider-id.md at line 5, Split the changeset body into a concise bulleted list, with one release-note statement for each independent fix: accepting bare provider ids for login, promptly ending cancelled OpenAI Codex sign-in, using one cancellable editor progress notification, joining repeated sign-in requests, and preserving completed sign-in success when the follow-up status refresh fails.apps/pythinker-code/src/tui/components/dialogs/platform-selector.ts (1)
1-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the direct SDK call from
PlatformSelectorComponent.Build the options in
promptPlatformSelectionor a utility, then pass them to the component.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pythinker-code/src/tui/components/dialogs/platform-selector.ts` around lines 1 - 2, Remove the direct buildPlatformOptions SDK import and usage from PlatformSelectorComponent; have promptPlatformSelection or a separate utility construct the platform options, then pass the resulting options into the component through its existing input interface.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.agents/skills/stop-slop/README.md:
- Line 5: Replace the image’s non-descriptive alt text in the README with a
concise description of what the image shows, or use an empty alt attribute if it
is purely decorative.
- Around line 13-22: Add the text language identifier to the fenced
directory-tree block in README.md, changing the opening fence to specify text
while preserving the tree contents and closing fence.
In @.agents/skills/stop-slop/references/examples.md:
- Around line 20-23: Update the “After” examples in examples.md, including the
sections around “Teams struggle with alignment,” to comply with the skill rules:
replace lazy extremes such as “Nobody,” make Example 3 a complete sentence with
the required word count, and remove the em dash from Example 4 while preserving
each example’s intended meaning.
In @.changeset/acp-login-auth-method.md:
- Around line 2-5: Confirm whether the ACP authentication method label is a
supported public contract for external clients; if so, change the changeset bump
for `@pythoughts/pythinker-code` from minor to major, otherwise retain minor. Do
not make the major-bump change without explicit confirmation.
In
`@apps/pythinker-code/src/tui/components/messages/dynamic-workflow-mission-control.ts`:
- Around line 1043-1050: The idle color selection near the dynamic workflow
member rendering must only apply warning or error thresholds when member.phase
is 'running'. Keep pending, queued, and suspended members muted or represented
with an idle placeholder, and add a test covering a queued member whose idle
time exceeds stalledIdleMs.
In `@apps/vscode/src/auth/vscode-login-ui.ts`:
- Around line 161-164: Validate the result returned by fetchCatalog before
assigning or using it in the login flow, ensuring it is an array whose provider
entries have the required valid shape. Reject or handle malformed entries such
as a null provider before catalogConnectionWire builds login options, while
preserving the existing abort signal and HTTPS fetch behavior.
- Around line 143-148: Update showLoginAuthorizationPrompt to validate the
selected authorization URL before calling ctx.broadcast or openBrowser. Require
a non-empty URL with an https: protocol, and reject invalid values before either
side effect while preserving the existing flow for valid URLs.
In `@apps/vscode/test/bridge-handler.test.ts`:
- Around line 88-94: Update the mock Uri.parse implementation and Uri.toString
method to retain parsed URI schemes, including https, and serialize them
correctly. Add an OAuth test verifying that openExternal receives the expected
parsed URI.
- Around line 905-919: Update the test case around bridge.handle and the
“Signing in to Pythinker” progress notification to exercise the credential flow
rather than dismissing the provider picker. Assert that the API-key, model, and
effort prompts each receive the shared UI cancellation token, then cancel the
withProgress token and verify the prompt observes cancellation without requiring
token object identity.
In `@apps/vscode/webview-ui/src/components/WorkflowCard.tsx`:
- Around line 131-135: Update the workflowWarning message container in
WorkflowCard to remove the truncate styling and allow long warning text to wrap
fully within narrow webview panels.
In `@packages/node-sdk/src/index.ts`:
- Around line 53-79: Update the node SDK package metadata to set private to
false, and add a changeset targeting the SDK package for the newly public
exports. Use the package’s existing release format and appropriate public-facing
change classification.
In `@packages/node-sdk/src/login/flows.ts`:
- Around line 203-208: Update both setConfig patches in
packages/node-sdk/src/login/flows.ts at lines 203-208 and 299-304 to include
thinking: config.thinking, preserving the selected effort set by both apply
functions when selection.effort is not off.
In `@packages/node-sdk/test/catalog.test.ts`:
- Around line 151-164: Update the test around applyCatalogProvider to seed
config.thinking.effort with a non-off value before applying thinking:false and
effort:'off', then assert it is cleared. Modify applyCatalogProvider in
catalog.ts so the off flow explicitly removes the existing effort instead of
only writing non-off efforts.
In `@skills-lock.json`:
- Around line 5-8: Update the GitHub skill installation and loading flow for the
locked source “hardikpandya/stop-slop” to pin downloads to a commit and validate
the downloaded SKILL.md content against skills-lock.json’s computedHash before
loading it; reject mismatches and do not load the skill when validation fails.
---
Nitpick comments:
In @.changeset/login-cancel-and-provider-id.md:
- Line 5: Split the changeset body into a concise bulleted list, with one
release-note statement for each independent fix: accepting bare provider ids for
login, promptly ending cancelled OpenAI Codex sign-in, using one cancellable
editor progress notification, joining repeated sign-in requests, and preserving
completed sign-in success when the follow-up status refresh fails.
In `@apps/pythinker-code/src/tui/components/dialogs/platform-selector.ts`:
- Around line 1-2: Remove the direct buildPlatformOptions SDK import and usage
from PlatformSelectorComponent; have promptPlatformSelection or a separate
utility construct the platform options, then pass the resulting options into the
component through its existing input interface.
In `@apps/pythinker-code/test/cli/login.test.ts`:
- Around line 88-94: Resolve the import/first lint warnings for the imports from
`@clack/prompts` and `@pythoughts/pythinker-code-sdk` in the login test by moving
them before the vi.mock calls, or add scoped oxlint-disable-next-line
import/first comments documenting the intentional Vitest hoisting pattern.
- Around line 112-124: Update the beforeEach setup to reset every prompt and
catalog mock, including password, text, fetchCatalog, and
mockFetchOpenPlatformModels, alongside the existing resets. Import text if it is
not currently available, and use mockReset so persistent implementations from
tests such as the API-key prompt and model catalog do not leak into later cases.
- Around line 312-348: Split the combined test into two independent it blocks:
one covering the display-name --provider value and another covering the bare id
value. Move each scenario’s setup and assertions into its respective test,
remove the mid-test mockReset/mockClear calls, and give each test a name that
identifies the provider form being exercised.
In `@packages/agent-core/src/agent/dynamic-workflow/size-guideline.ts`:
- Around line 15-31: Remove the manually maintained
WORKFLOW_SIZE_GUIDELINE_NAMES set and derive accepted values from the
exhaustiveness-checked GUIDELINE_TARGETS keys. Update
parseWorkflowSizeGuidelineEnv to validate normalized input against those keys
and return the validated key without the WorkflowSizeGuideline cast, preserving
trimming, lowercasing, and undefined handling.
In `@packages/agent-core/src/session/subagent-batch.ts`:
- Around line 165-169: Update the constructor’s concurrencyLimit normalization
to detect non-finite values before truncation, defaulting NaN and Infinity to
MAX_CONCURRENT_WORKFLOW_SUBAGENTS while preserving the existing minimum of one
for finite values. Add tests covering both non-finite inputs and verifying the
batch processes pending tasks with the default limit.
In `@packages/node-sdk/src/login/flows.ts`:
- Around line 56-58: Update the platform dispatch condition in the login flow
around handlePythinkerCodeOAuthLogin to compare platformId with the exported
KIMI_CODE_PLATFORM_ID constant from platform-options.ts instead of a duplicated
string literal.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 65b89f3b-33ee-48c2-bf61-51ea4b002d8f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/pnpm-lock.yaml
📒 Files selected for processing (104)
.agents/skills/stop-slop/CHANGELOG.md.agents/skills/stop-slop/LICENSE.agents/skills/stop-slop/README.md.agents/skills/stop-slop/SKILL.md.agents/skills/stop-slop/references/examples.md.agents/skills/stop-slop/references/phrases.md.agents/skills/stop-slop/references/structures.md.changeset/acp-login-auth-method.md.changeset/codex-login-declared-efforts.md.changeset/dynamic-workflow-output-schema.md.changeset/login-cancel-and-provider-id.md.changeset/login-effort-persists.md.changeset/login-keeps-provider-on-cancel.md.changeset/multi-provider-login-picker.md.changeset/workflow-blank-items-followups.md.changeset/workflow-ignores-empty-items.md.changeset/workflow-retry-keeps-output-schema.md.changeset/workflow-run-correlation.md.changeset/workflow-running-row-spinner.md.changeset/workflow-size-guideline-and-kill-switch.md.changeset/workflow-subagent-caps.md.changeset/workflow-work-and-idle-column.mdAGENTS.mdapps/pythinker-code/package.jsonapps/pythinker-code/src/auth/terminal-login-ui.tsapps/pythinker-code/src/cli/run-prompt.tsapps/pythinker-code/src/cli/sub/login-flow.tsapps/pythinker-code/src/cli/sub/login.tsapps/pythinker-code/src/tui/commands/auth.tsapps/pythinker-code/src/tui/commands/dynamic-workflow.tsapps/pythinker-code/src/tui/commands/prompts.tsapps/pythinker-code/src/tui/commands/workflow-availability.tsapps/pythinker-code/src/tui/components/dialogs/platform-selector.tsapps/pythinker-code/src/tui/components/messages/dynamic-workflow-mission-control.tsapps/pythinker-code/src/tui/constant/rendering.tsapps/pythinker-code/src/tui/controllers/session-event-handler.tsapps/pythinker-code/src/tui/controllers/subagent-event-handler.tsapps/pythinker-code/src/tui/pythinker-tui.tsapps/pythinker-code/src/tui/utils/event-payload.tsapps/pythinker-code/src/tui/utils/thinking-levels.tsapps/pythinker-code/test/cli/acp.test.tsapps/pythinker-code/test/cli/login.test.tsapps/pythinker-code/test/tui/commands/auth.test.tsapps/pythinker-code/test/tui/commands/dynamic-workflow.test.tsapps/pythinker-code/test/tui/components/dialogs/platform-selector.test.tsapps/pythinker-code/test/tui/components/messages/dynamic-workflow-mission-control.test.tsapps/pythinker-code/test/tui/parity/feature-matrix.tsapps/pythinker-code/test/tui/pythinker-tui-message-flow.test.tsapps/pythinker-code/test/tui/pythinker-tui-startup.test.tsapps/pythinker-web/src/api/daemon/agentEventProjector.tsapps/pythinker-web/test/subagent-goal.test.tsapps/vscode/shared/legacy-sdk.tsapps/vscode/src/auth/vscode-login-ui.tsapps/vscode/src/handlers/auth.handler.tsapps/vscode/src/runtime/event-adapter.tsapps/vscode/test/bridge-handler.test.tsapps/vscode/test/event-adapter.test.tsapps/vscode/test/event-handlers.test.tsapps/vscode/tsdown.config.tsapps/vscode/webview-ui/src/components/ChatMessage.tsxapps/vscode/webview-ui/src/components/LoginScreen.tsxapps/vscode/webview-ui/src/components/ToolRenderers.tsxapps/vscode/webview-ui/src/components/WorkflowCard.tsxapps/vscode/webview-ui/src/components/login-outcome.tsapps/vscode/webview-ui/src/stores/chat.store.tsapps/vscode/webview-ui/src/stores/event-handlers.tsflake.nixpackages/acp-adapter/src/auth-methods.tspackages/acp-adapter/test/server.test.tspackages/agent-core/src/agent/dynamic-workflow/index.tspackages/agent-core/src/agent/dynamic-workflow/run-id.tspackages/agent-core/src/agent/dynamic-workflow/size-guideline.tspackages/agent-core/src/agent/tool/index.tspackages/agent-core/src/agent/turn/index.tspackages/agent-core/src/config/schema.tspackages/agent-core/src/config/toml.tspackages/agent-core/src/session/subagent-batch.tspackages/agent-core/src/session/subagent-host.tspackages/agent-core/src/tools/builtin/collaboration/dynamic-workflow.mdpackages/agent-core/src/tools/builtin/collaboration/dynamic-workflow.tspackages/agent-core/test/agent/tool.test.tspackages/agent-core/test/agent/turn.test.tspackages/agent-core/test/config/configs.test.tspackages/agent-core/test/harness/skill-session.test.tspackages/agent-core/test/session/subagent-batch.test.tspackages/agent-core/test/session/subagent-host.test.tspackages/agent-core/test/tools/builtin-current.test.tspackages/node-sdk/src/catalog.tspackages/node-sdk/src/error-format.tspackages/node-sdk/src/index.tspackages/node-sdk/src/login/flows.tspackages/node-sdk/src/login/model-alias.tspackages/node-sdk/src/login/platform-options.tspackages/node-sdk/src/login/platform-values.tspackages/node-sdk/src/login/types.tspackages/node-sdk/src/thinking-levels.tspackages/node-sdk/test/catalog.test.tspackages/node-sdk/test/session-event-types.test.tspackages/oauth/src/open-platform.tspackages/oauth/src/openai-codex-oauth.tspackages/oauth/test/openai-codex-oauth.test.tspackages/protocol/src/__tests__/events.test.tspackages/protocol/src/events.tsskills-lock.json
…rsing Persist the thinking effort a login picks. Both apply steps wrote config.thinking.effort, but the setConfig patches that saved the result listed everything except it, so an API-key login reopened at the default level. Writing 'off' rather than skipping it also clears a level an earlier login left behind, which a deep-merge patch cannot do by omitting the key. Refuse a device authorization whose verification URL is not HTTPS. Every renderer hands that URL to the host's open-externally API, so a provider answering with file:, javascript:, or an installed app's own scheme would have the agent launch it. Checked where the response is parsed, so the terminal, the TUI, and the editor extension are covered by one guard. Stop a Dynamic Workflow row that has not started from reading as stalled: a queued row measured silence from the launch of the whole run, so a long queue turned every waiting row red. Drop catalog entries that are not objects when the catalog is read. A null entry reached the provider picker and threw past the bundled-catalog fallback that exists to keep the login alive. Fall back to the default subagent concurrency limit when the given one is not finite. NaN passed every clamp and made each free-slot test false, so the batch launched nothing and never finished. Also: wrap the large-workflow warning in the editor panel instead of truncating it, derive the accepted workflow-size names from the target table rather than a second list, and use KIMI_CODE_PLATFORM_ID instead of a repeated literal. Tests: the VS Code URI mock keeps parsed schemes, the login progress token is asserted on every credential prompt rather than only the provider picker, and the effort, URL-scheme, queued-row, catalog, and concurrency fixes each get a regression test.
Contributor
Author
|
All 14 review findings triaged in f2e260f — 8 fixed, 6 dismissed with rationale on their threads. Fixed: the login effort never reaching disk (the Also took three nitpicks: a non-finite subagent concurrency limit now falls back to the default (NaN passed every clamp and stalled the batch), the workflow-size names derive from the target table, and Dismissed: three findings target Gates: typecheck, lint (0 errors), build, and 10,012 tests all green locally. The effort, URL-scheme, queued-row, catalog, and concurrency fixes each have a regression test that fails without its fix. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/pythinker-code/src/tui/components/messages/dynamic-workflow-mission-control.ts (1)
1051-1057: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the negated condition.
Line 1051 triggers
unicorn(no-negated-condition). Invert the outer condition so the running branch selects the threshold color and every other phase returnstextMuted.As per coding guidelines, use oxlint and
pnpm lint:fixfor automatic fixes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/pythinker-code/src/tui/components/messages/dynamic-workflow-mission-control.ts` around lines 1051 - 1057, Update the token selection expression near the phase-color logic to remove the negated condition: check for the running phase first, apply the stalled and quiet idle thresholds there, and return textMuted for every non-running phase. Use oxlint or pnpm lint:fix to apply the lint correction.Sources: Coding guidelines, Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/pythinker-code/test/cli/login.test.ts`:
- Line 89: Move the password, select, and text imports from the mid-file
location into the top-level import block in the login test, alongside the other
imports. Then run pnpm lint:fix to resolve the import/first warnings.
---
Nitpick comments:
In
`@apps/pythinker-code/src/tui/components/messages/dynamic-workflow-mission-control.ts`:
- Around line 1051-1057: Update the token selection expression near the
phase-color logic to remove the negated condition: check for the running phase
first, apply the stalled and quiet idle thresholds there, and return textMuted
for every non-running phase. Use oxlint or pnpm lint:fix to apply the lint
correction.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 194fef57-787e-44b7-9979-414c21744b42
⛔ Files ignored due to path filters (1)
apps/pythinker-code/src/generated/dashboard-web-asset.tsis excluded by!**/generated/**
📒 Files selected for processing (20)
.changeset/catalog-and-batch-robustness.md.changeset/login-effort-reaches-disk.md.changeset/login-url-scheme-guard.md.changeset/workflow-queued-rows-not-stalled.md.changeset/workflow-warning-wraps.mdapps/pythinker-code/src/tui/components/messages/dynamic-workflow-mission-control.tsapps/pythinker-code/test/cli/login.test.tsapps/pythinker-code/test/tui/components/messages/dynamic-workflow-mission-control.test.tsapps/pythinker-code/test/tui/pythinker-tui-message-flow.test.tsapps/vscode/test/bridge-handler.test.tsapps/vscode/webview-ui/src/components/WorkflowCard.tsxpackages/agent-core/src/agent/dynamic-workflow/size-guideline.tspackages/agent-core/src/session/subagent-batch.tspackages/agent-core/test/session/subagent-batch.test.tspackages/node-sdk/src/catalog.tspackages/node-sdk/src/login/flows.tspackages/node-sdk/test/catalog.test.tspackages/oauth/src/oauth.tspackages/oauth/src/open-platform.tspackages/oauth/test/oauth.test.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/node-sdk/test/catalog.test.ts
- apps/vscode/webview-ui/src/components/WorkflowCard.tsx
- packages/oauth/src/open-platform.ts
- packages/node-sdk/src/catalog.ts
- packages/agent-core/test/session/subagent-batch.test.ts
- packages/node-sdk/src/login/flows.ts
- apps/pythinker-code/test/tui/pythinker-tui-message-flow.test.ts
- packages/agent-core/src/agent/dynamic-workflow/size-guideline.ts
- packages/agent-core/src/session/subagent-batch.ts
- apps/pythinker-code/test/tui/components/messages/dynamic-workflow-mission-control.test.ts
The catalog login had a regression test for the effort reaching disk; the open-platform one did not, so both the setConfig patch and the 'off' overwrite in applyOpenPlatformConfig shipped unasserted. Each new assertion fails when its fix is reverted.
Contributor
Author
|
Follow-up on the review-body nitpicks that have no thread to reply on, so nothing is left unaccounted for. Taken (in f2e260f): the non-finite Declined:
Also added in 0b78a4d, prompted by the effort finding rather than by a comment: the open-platform login path had no assertion that the picked effort reaches disk, so its |
The nested ternary tested the negated phase first, which tripped unicorn(no-negated-condition) and made `textMuted` the answer at both ends of the chain. A small `idleColor` helper states the rule the way the panel means it: only a running row can stall. Adds the case that had no coverage — a suspended row keeps its muted count however long it stays silent — so the guard now fails a test when it is removed.
The TUI `thinking-levels` module re-exported the SDK rules only so older import paths kept working. Nothing outside this branch depends on that path, so the callers now name the SDK and the module keeps just what is genuinely TUI-bound: the theme colour token and the short label. The rules also had no test in the package that owns them — only the TUI suite reached them through the re-export. Their suites move to `packages/node-sdk/test/thinking-levels.test.ts`, so a second renderer cannot break the shared rules without a red test in the SDK.
Contributor
Author
|
Two follow-ups on this branch, both green locally.
Left alone on purpose: the Gates: root @coderabbitai review — the last three commits ( |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- commits after f2e260f ---'
git log --oneline --decorate f2e260f..HEAD
echo '--- changed files after f2e260f ---'
git diff --name-status f2e260f..HEAD
echo '--- commit statistics ---'
git diff --stat f2e260f..HEAD
echo '--- relevant file outlines ---'
ast-grep outline apps/pythinker-code/src/tui/components/messages/dynamic-workflow-mission-control.ts --items all || true
ast-grep outline apps/pythinker-code/src/tui/utils/thinking-levels.ts --items all || true
ast-grep outline packages/node-sdk/src/thinking-levels.ts --items all || true
ast-grep outline packages/node-sdk/test/thinking-levels.test.ts --items all || true
echo '--- direct and legacy thinking-level imports ---'
rg -n -C 3 'from ["'\''](?:`@pythoughts/pythinker-code-sdk`|.*thinking-levels)["'\'']' \
apps/pythinker-code/src packages/node-sdk/src packages/node-sdk/test || true
echo '--- commit patches ---'
git diff --find-renames --find-copies --format=fuller f2e260f..HEAD -- \
apps/pythinker-code/src/tui/components/messages/dynamic-workflow-mission-control.ts \
apps/pythinker-code/test/tui/components/messages/dynamic-workflow-mission-control.test.ts \
apps/pythinker-code/src/tui/utils/thinking-levels.ts \
packages/node-sdk/src/thinking-levels.ts \
packages/node-sdk/test/thinking-levels.test.ts \
apps/pythinker-code/src/auth/terminal-login-ui.ts \
apps/pythinker-code/test/cli/login.test.ts \
packages/oauth/src/open-platform.ts \
packages/oauth/test/open-platform.test.tsLength of output: 74330 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- all source patches after f2e260f ---'
git diff --find-renames f2e260f..HEAD -- \
apps/pythinker-code/src/tui/commands/config.ts \
apps/pythinker-code/src/tui/commands/prompts.ts \
apps/pythinker-code/src/tui/components/dialogs/model-selector.ts \
apps/pythinker-code/src/tui/controllers/auth-flow.ts \
apps/pythinker-code/src/tui/controllers/editor-keyboard.ts \
apps/pythinker-code/test/tui/utils/thinking-levels.test.ts \
packages/oauth/src/open-platform.ts
echo '--- current relevant source regions ---'
sed -n '1,90p' apps/pythinker-code/src/tui/commands/config.ts
sed -n '1,75p' apps/pythinker-code/src/tui/commands/prompts.ts
sed -n '1,75p' apps/pythinker-code/src/tui/components/dialogs/model-selector.ts
sed -n '1,65p' apps/pythinker-code/src/tui/controllers/auth-flow.ts
sed -n '1,65p' apps/pythinker-code/src/tui/controllers/editor-keyboard.ts
sed -n '1,130p' apps/pythinker-code/src/tui/utils/thinking-levels.ts
sed -n '1,125p' packages/node-sdk/src/thinking-levels.ts
sed -n '1,120p' packages/oauth/src/open-platform.ts
echo '--- remaining imports from the former TUI rule module ---'
rg -n -C 2 '`#/tui/utils/thinking-levels`' apps/pythinker-code/src apps/pythinker-code/test || true
echo '--- node-sdk test discovery configuration and scripts ---'
fd -HI '^(package\.json|vitest.*\.(ts|mts|cts|js|mjs|cjs))$' packages/node-sdk . | sort | while read -r file; do
echo "--- $file ---"
sed -n '1,220p' "$file"
done
echo '--- migration test patch and current tests ---'
git diff f2e260f..HEAD -- apps/pythinker-code/test/tui/utils/thinking-levels.test.ts
sed -n '1,220p' apps/pythinker-code/test/tui/utils/thinking-levels.test.ts
echo '--- file history membership ---'
git log --oneline f2e260f..HEAD -- \
apps/pythinker-code/src/tui/utils/thinking-levels.ts \
packages/node-sdk/test/thinking-levels.test.ts \
packages/oauth/test/open-platform.test.tsLength of output: 83664
I found no new issues. The The SDK rule migration is complete. The remaining TUI helper imports are presentation-only. The new node-sdk test file matches the package test include pattern, so the moved tests are discovered. The open-platform login test verifies that the selected effort reaches the
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issue
No issue was opened for this work. The problem is described below.
Problem
Two areas needed work.
Dynamic Workflow was unbounded and hard to watch. Nesting had no limit, so a workflow that spawned workflows could grow without end. A blank or empty item in the item list failed argument validation and discarded the whole call before any subagent started. A subagent lost its output schema when a provider rate limit forced a retry, and then answered in prose that the run reported as a success. The progress bar pinned every tool-using agent at 75 percent, so a busy agent and a wedged agent looked the same.
Login went straight to one provider. The user could not pick a provider,
--providerneeded the full display name, a cancelled OpenAI Codex sign-in held the process open for the full two-minute callback timeout, and the picked thinking-effort level was not saved.What changed
Dynamic Workflow
disableWorkflowskill switch and an advisoryworkflowSizeGuideline, both settable in config or by environment variable. Every client shows the large-workflow warning.output_schemamakes each subagent return a validated object. A subagent that cannot satisfy the schema is reported apart from one that failed outright. The schema now survives a rate-limit retry.TUI
Login
pythinker loginopens a provider picker.--provider <id|name>skips it and accepts a plain catalog id.Commit
9ac4620(chore: add stop-slop skill and its lock entry) is unrelated repo tooling that rides along on this branch.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.Summary by CodeRabbit
New Features
Bug Fixes