fix: enable Windows native auto-update, remove dead datasource plugin - #16
fix: enable Windows native auto-update, remove dead datasource plugin#16elkaix wants to merge 18 commits into
Conversation
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @pythoughts/pythinker-code@0.6.0 ### Minor Changes - [`d7a2554`](d7a2554) - Maintenance release with internal improvements and dependency updates. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…#7) ## Related Issue No linked issue — problem explained below. ## Problem Connecting a provider from the models.dev catalog in `/login` or `/provider` failed with `Environment variable "X" is not set or is empty.` instead of asking for an API key. The same gate also broke the OpenAI (API key) and Anthropic (API key) login entries, which route through the same path, and `pythinker provider catalog add` had no way to pass a key directly. Separately, the update banner and `/update` implied Homebrew installs auto-update when they deliberately do not, and the native install scripts pointed at nonexistent release assets. ## What changed - Catalog provider login now falls back to the API key input dialog when the provider's environment variable is unset or empty, and stores the entered key in `config.toml` via the existing `api_key` provider field. When the env var is set, behavior is unchanged. - `pythinker provider catalog add` gains `--api-key <key>`, taking precedence over `--api-key-env` and the catalog's declared env var; error messages now mention the flag. - `/update` and the startup update notice state that Homebrew installs do not auto-update and show the native installer command for automatic background updates. - The native install scripts download the correct published release assets. - Site: legacy downloads popup and Python download milestone highlight. Tests: new coverage for the prompt fallback (env set / unset / undeclared / cancelled) in the TUI, and for `--api-key` precedence and Commander wiring in the CLI. Full suite green. ## Checklist - [x] I have read the [CONTRIBUTING](https://github.com/Pythoughts-labs/pythinker-code/blob/main/CONTRIBUTING.md) document. - [x] I have linked a related issue, or explained the problem above. - [x] I have added tests that prove my feature works. - [x] Ran `gen-changesets` skill, or this PR needs no changeset. - [x] Ran `gen-docs` skill, or this PR needs no doc update. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Catalog provider setup now accepts API keys interactively or through `--api-key`. - Native installers download verified, architecture-specific release packages on macOS, Linux, and Windows. - Added a dismissible community download milestone popup to the website. - Updated installation guidance, including Homebrew recommendations for automatic updates. - **Bug Fixes** - Provider login no longer fails when the configured API-key environment variable is missing or empty. - **Documentation** - Updated architecture imagery and installation messaging. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Related Issue No linked issue — problem explained below. ## Problem On Windows, `pythinker` installed via npm fails immediately: ``` Failed to start Pythinker Code: The feature process.execve is unavailable on the current platform, which is being used to run Node.js ``` Windows Node defines `process.execve` as a function that throws `ERR_FEATURE_UNAVAILABLE_ON_PLATFORM` when called. The launcher's existence check (`process.execve !== undefined`) therefore routed Windows into the execve path and crashed before the existing spawn fallback could run. ## What changed The launcher checks the platform first: win32 always uses the spawn fallback; the execve path (which preserves pid, process group, and controlling terminal) remains for POSIX. Added a regression test that fakes win32 with a throwing `execve` and asserts the fallback child completes — verified it fails against the previous launcher. ## Checklist - [x] I have read the [CONTRIBUTING](https://github.com/Pythoughts-labs/pythinker-code/blob/main/CONTRIBUTING.md) document. - [x] I have linked a related issue, or explained the problem above. - [x] I have added tests that prove my feature works. - [x] Ran `gen-changesets` skill, or this PR needs no changeset. - [x] Ran `gen-docs` skill, or this PR needs no doc update.
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @pythoughts/pythinker-code@0.6.1 ### Patch Changes - [#7](#7) [`d396320`](d396320) - Prompt for an API key when connecting a catalog provider whose environment variable is not set, instead of failing with "Environment variable is not set or is empty". Applies to `/login`, `/provider`, and `pythinker provider catalog add`, which now also accepts `--api-key <key>`. - [#7](#7) [`d396320`](d396320) - Explain in `/update` and the startup update notice that Homebrew installs do not auto-update, and point to the native installer for automatic background updates. - [#7](#7) [`d396320`](d396320) - Point the native install scripts at the published release assets. - [#7](#7) [`d396320`](d396320) - Show a clear requirement message with the native-installer alternative when the CLI is launched on Node.js older than 26.4, instead of failing with a cryptic flag error. - [#8](#8) [`9b1b195`](9b1b195) - Fix the CLI failing to start on Windows with "process.execve is unavailable" by using the spawn fallback instead of calling execve there. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
## Related Issue No linked issue — problem explained below. ## Problem The install scripts' animated intro draws the logo grid and progress bar at fixed screen rows (5-9 and 17). When the terminal already has output (e.g. after an apt run), the animation interleaves with the existing text and renders garbled. ## What changed Both `install.sh` and `install.ps1` clear the screen and home the cursor when the animated intro starts. The non-animated path (`CI`, `NO_COLOR`, redirected output) is unchanged, so logs and pipes are unaffected. ## Checklist - [x] I have read the [CONTRIBUTING](https://github.com/Pythoughts-labs/pythinker-code/blob/main/CONTRIBUTING.md) document. - [x] I have linked a related issue, or explained the problem above. - [x] I have added tests that prove my feature works. - [x] Ran `gen-changesets` skill, or this PR needs no changeset. - [x] Ran `gen-docs` skill, or this PR needs no doc update. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Redesigned browser OAuth confirmation pages with a lighter, grid-based layout, improved spacing, typography, and responsive animations. - Added animated success and error indicators with clearer status messaging. - **Bug Fixes** - Installation scripts now clear the terminal and reset the cursor before displaying their animated introductions. - Updated installation instructions and commands to use the current code hosting URLs across supported platforms. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @pythoughts/pythinker-code@0.6.2 ### Patch Changes - [#10](#10) [`ad2391b`](ad2391b) - Clear the terminal before the install script's animated intro so earlier shell output no longer interleaves with the logo animation. - [#10](#10) [`ad2391b`](ad2391b) - Restyle the browser OAuth sign-in confirmation pages for all providers to match the website's light design. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
## Related Issue No issue filed — problem explained directly. ## Problem Context compaction fails with a provider 400 whenever the model alias has no `maxOutputSize` configured: ``` Compaction cancelled Error: [compaction.failed] APIStatusError: 400 Error from provider (Console Go): Upstream request failed: [invalid_request_error] Invalid max_tokens value, the valid range of max_tokens is [1, 393216] ``` The compaction request resolved its completion budget without `maxOutputSize`, so the cap fell back to the model's full context window and was sent as `max_tokens`. Providers that enforce `input + max_tokens <= context window` (Moonshot/Console gateways, DeepSeek, Groq, and also Anthropic/OpenAI/Google under the same conditions) reject the request outright. The class of bug is provider-agnostic; compaction just hits it first because its prompt is the largest. ## What changed Port of the two upstream fixes for this (kimi-code `794db5553` and `76c643bcb`), adapted to pythinker: - Compaction now resolves its budget with `maxOutputSize ?? min(contextWindow, 128k)` instead of falling through to the context window (`packages/agent-core/src/agent/compaction/full.ts`). - `withMaxCompletionTokens` gained optional context: OpenAI-legacy and Pythinker chat-completions providers clamp the cap to the remaining context window (`maxContextTokens - usedContextTokens`), and OpenAI-legacy additionally to a 128k ceiling (`packages/kosong`). - Normal turns pass the live context token count through `KosongLLM` so per-request caps shrink as the context fills. Also includes a second commit: restart Homebrew-managed installs after `/update` (helper + activation step, doctor/preflight updates, docs). Both changes carry their own changeset; happy to split into a separate PR if preferred. ## Checklist - [x] I have read the [CONTRIBUTING](https://github.com/Pythoughts-labs/pythinker-code/blob/main/CONTRIBUTING.md) document. - [x] I have linked a related issue, or explained the problem above. - [x] I have added tests that prove my feature works. - [x] Ran `gen-changesets` skill, or this PR needs no changeset. - [x] Ran `gen-docs` skill, or this PR needs no doc update. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Automatic updates can download and verify in the background, then install on the next launch. * Added clearer update status, failure details, pending versions, and installer logs to diagnostics. * **Bug Fixes** * Improved context compaction to prevent excessive completion sizes and provider overflows. * Workflow progress now advances smoothly, displays a Finalizing state, and aligns correctly on narrow screens. * **Documentation** * Updated upgrade, configuration, environment-variable, and data-location guidance for the revised update behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @pythoughts/pythinker-code@0.7.0 ### Minor Changes - [#12](#12) [`02f7f8d`](02f7f8d) - Prepare verified Homebrew updates in the background and install them automatically on the next interactive launch. ### Patch Changes - [#12](#12) [`02f7f8d`](02f7f8d) - Fix context compaction failing with provider "Invalid max_tokens" errors by capping requested completion tokens to the remaining context window and a safe output ceiling instead of the full context window size. - [#12](#12) [`02f7f8d`](02f7f8d) - Fix Dynamic Workflow progress sticking at 90% during long streaming, show a Finalizing state once all delegated agents finish, and fix member row alignment at narrow widths. Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
## Related Issue
No existing issue — the problem is described below. This is a
reproducible bug fix with a focused diff.
## Problem
Every request to a Kimi / Moonshot model fails before the model runs:
```
400 tools.function.parameters is not a valid moonshot flavored json schema,
details: <At path 'root': when using anyOf, type should be defined in anyOf items
instead of the parent schema>
```
The wire validator refuses a tool schema that uses `anyOf` as a
*refinement* of the
node it sits on. Two rules are involved:
- `type` may not appear next to `anyOf` on the same node.
- No validation keyword (`properties`, `items`, `additionalProperties`,
…) may
appear on both a node and one of its `anyOf` branches.
Both are legal standard JSON Schema, so schemas that work on every other
provider
are rejected here. `TaskStop` trips them: it adds a hand-written
`anyOf: [{ required: ['task_id'] }, { required: ['shell_id'] }]` on top
of an
object schema that already declares `type` and `properties`. Because
tool
definitions are sent with every request, a single offending tool takes
down the
whole session.
`normalizePythinkerToolSchema` already existed for exactly this class of
provider-compatibility repair, but it only visited *nested* property
schemas and
deliberately skipped the root, so it never saw the offending node.
## What changed
The repair lives in the provider's schema normalizer rather than in
`TaskStop`, so
MCP- and plugin-contributed schemas are covered by the same pass — this
shape is
common enough that several other clients have hit it independently.
**Nested nodes — distribute.** A node's own constraints are pushed down
into each
`anyOf` branch, and branches that already declare a keyword keep their
own
(narrower) value. This is lossless: `P ∧ (B₁ ∨ B₂)` and `(P ∧ B₁) ∨ (P ∧
B₂)`
accept exactly the same instances. Sibling combinators (`allOf` /
`oneOf` / `not` /
`if`) are left in place — the validator does not read them, so moving
them would
only churn the schema.
**Root — fold away.** The root cannot use the branch form, because a
tool's
parameters must be a plain object: the wire separately requires
`parameters.type == "object"`, which cannot coexist with a root `anyOf`.
The two
constraints are jointly unsatisfiable, so the root's `anyOf` is dropped.
That only
ever widens what is accepted, and a root `anyOf` is in practice a "one
of these
fields is required" hint that the tool re-checks at run time (`TaskStop`
already
returns `Missing required parameter: task_id`). Branch properties are
merged into
the root first, so a schema that kept its arguments inside the branches
does not
silently lose them.
## Verification
Rather than asserting against an assumed reading of the spec, every
schema was
replayed through the provider's own validator:
- Reproduced the original 400 against the live endpoint, then confirmed
it is gone.
- Replayed **all 36 built-in tool schemas and all 70 tool schemas from
five
connected MCP servers** through the normalizer and into the provider's
validator:
106 accepted, 0 rejected. Before the change, `TaskStop` was the single
failure.
- Full `kosong` suite passes (1185 tests); `typecheck` clean.
- Each of the two new code paths was individually disabled to confirm
the new tests
actually fail without it.
## Checklist
- [x] I have read the
[CONTRIBUTING](https://github.com/Pythoughts-labs/pythinker-code/blob/main/CONTRIBUTING.md)
document.
- [x] I have linked a related issue, or explained the problem above.
- [x] I have added tests that prove my feature works.
- [x] Ran `gen-changesets` skill, or this PR needs no changeset.
- [x] Ran `gen-docs` skill, or this PR needs no doc update — no
user-facing behavior or CLI surface changes.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Improved compatibility with Kimi and Moonshot models when tools use
combined schema types.
* Fixed invalid tool schema errors involving `anyOf`, `oneOf`, and
`allOf` combinations.
* Improved normalization of nested and object-based tool schemas while
preserving metadata, constraints, references, and definitions.
* Reduced duplicate schema alternatives and improved handling of
required fields.
* **Tests**
* Added comprehensive coverage for root and nested schema normalization,
cyclic definitions, and combinators.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Windows native installs could rename a running exe but not overwrite it, so the CLI hard-disabled auto-update on that platform and told users to run install.ps1 by hand instead. install.ps1 now stages the new binary inside the install dir and replaces the running exe with a rename-aside-then-move sequence, retrying cleanup of the old file for a few seconds since it may still be locked by the exiting process. The CLI drops the win32 auto-install gate and spawns the installer via PowerShell the same way it spawns curl|bash on macOS/Linux, and sweeps any leftover renamed-aside binary on the next startup.
The plugin proxied every tool call to api.pythinker.com/coding/v1/tools, a gateway backend that was never built — every call returned a plain 404, so the plugin never worked. Rather than run a dead marketplace entry, drop the plugin, its CDN package, and its docs section entirely. build-cdn.mjs now skips a plugin channel directory that doesn't exist instead of throwing, since removing the only official-tier zip makes git drop the now-empty plugins/cdn/official/ directory on checkout. Also move lint-staged's oxlint invocation into .lintstagedrc.mjs so it filters out oxlint-ignored paths before running: oxlint exits non-zero on an all-ignored fileset, which broke committing an isolated change to a script under apps/*/scripts/ (an intentionally oxlint-ignored path).
Correct the advertised minimum Node.js version and add the English goal-mode feature breakdown (core workflow, metrics/budgets, and user interaction contracts for agent-core's goal driver).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe change enables automatic Windows native updates with rollback and stale-backup cleanup, removes the Pythinker Datasource plugin, adds a goal-mode specification, introduces lint-staged oxlint processing, improves CDN plugin copying, and expands repository guidance. ChangesWindows native updates
Datasource plugin removal
Project specifications and tooling
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant UpdatePreflight
participant PowerShell
participant InstallScript
participant RunningExecutable
UpdatePreflight->>PowerShell: Start detached native update
PowerShell->>InstallScript: Execute install.ps1 with selected version
InstallScript->>RunningExecutable: Rename current executable to .old
InstallScript->>RunningExecutable: Move updated executable into place
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
commit: |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
.lintstagedrc.mjs (1)
19-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winResolve the reported regex lint warnings.
Replace the literal
*regular expression with string replacement. Add Unicode mode to the regular expressions. Verify the warnings against the repository's pinnedoxlint1.70.0, because the supplied report uses 1.76.0.Suggested fix
- const escaped = body.replaceAll(/[.+^${}()|[\]\\]/g, '\\$&').replaceAll(/\*/g, '[^/]*'); - return isDir ? new RegExp(`(^|/)${escaped}(/|$)`) : new RegExp(`(^|/)${escaped}$`); + const escaped = body + .replaceAll(/[.+^${}()|[\]\\]/gu, '\\$&') + .replaceAll('*', '[^/]*'); + return isDir + ? new RegExp(`(^|/)${escaped}(/|$)`, 'u') + : new RegExp(`(^|/)${escaped}$`, 'u');🤖 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 @.lintstagedrc.mjs around lines 19 - 20, Update the escaping logic near `escaped` to replace literal asterisks using string replacement instead of a regular-expression literal, and add Unicode mode to both `RegExp` patterns returned by the directory and file branches. Validate the resulting lint output with the repository-pinned oxlint 1.70.0 rather than relying on the 1.76.0 report.Source: 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 @.lintstagedrc.mjs:
- Around line 33-34: Update the command construction in the targets mapping to
insert the option terminator -- immediately before the quoted staged paths in
both oxlint commands, while preserving the existing fix, type-aware, and quiet
options.
- Around line 23-26: Update lintableFiles so the path passed to ignoreRegexes is
normalized from backslashes to forward slashes before matching, while retaining
the original native path for returned command arguments.
In `@apps/pythinker-code/src/cli/update/preflight.ts`:
- Around line 150-154: Update the Windows branch of spawnForSource in
apps/pythinker-code/src/cli/update/preflight.ts (lines 150-154) to pass the
selected version as a PYTHINKER_VERSION environment override to the detached
PowerShell process, without interpolating it into -Command text. Update
apps/pythinker-code/test/cli/update/preflight.test.ts at lines 568-578 and
1508-1517 to assert that background installation and spawnForSource preserve and
forward the selected version on Windows.
In `@apps/pythinker-web/public/install.ps1`:
- Around line 527-543: Update the stale-backup cleanup before the replacement
flow: only remove $stale when $target exists, and preserve $stale when $target
is absent so the rollback in the Move-Item failure path can restore it. Keep the
existing rename, move, and rollback behavior otherwise unchanged.
- Around line 567-569: Initialize $extractDir to $null before entering the try
block, then update the finally cleanup condition to use $null -ne $extractDir.
Remove the directory with Remove-Item -LiteralPath $extractDir while preserving
the existing recursive, forced, and silent-error behavior.
In `@GOAL.md`:
- Around line 26-28: Clarify the complete-state contract in GOAL.md: specify
when the required final wrap-up turn and completion summary run relative to
clearing the goal, whether they use the active goal or a detached snapshot, and
how their work counts against the budget. Define whether the headless CLI emits
this same summary or an additional one, and ensure the persistence behavior is
consistent with the chosen ordering.
In `@README.md`:
- Line 14: Update the Node.js badge in README.md to advertise Node.js 26+ so it
matches the existing >=26.4.0 requirement in package.json; do not change the
engine requirement or related CI and runtime support.
---
Nitpick comments:
In @.lintstagedrc.mjs:
- Around line 19-20: Update the escaping logic near `escaped` to replace literal
asterisks using string replacement instead of a regular-expression literal, and
add Unicode mode to both `RegExp` patterns returned by the directory and file
branches. Validate the resulting lint output with the repository-pinned oxlint
1.70.0 rather than relying on the 1.76.0 report.
🪄 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: 4fa0651c-b60a-41c5-9487-bc00cb1c8bbd
⛔ Files ignored due to path filters (1)
plugins/cdn/official/pythinker-datasource.zipis excluded by!**/*.zip
📒 Files selected for processing (26)
.agents/skills/write-tui/DESIGN.md.changeset/remove-datasource-plugin.md.changeset/windows-native-auto-update.md.lintstagedrc.mjsGOAL.mdREADME.mdapps/pythinker-code/src/cli/update/preflight.tsapps/pythinker-code/src/main.tsapps/pythinker-code/src/native/native-assets.tsapps/pythinker-code/test/cli/update/preflight.test.tsapps/pythinker-code/test/native/native-assets.test.tsapps/pythinker-code/test/utils/pythinker-datasource-plugin.test.tsapps/pythinker-web/public/install.ps1apps/site/scripts/build-cdn.mjsdocs/AGENTS.mddocs/customization/datasource.mddocs/customization/plugins.mdpackage.jsonplugins/cdn/marketplace.jsonplugins/marketplace.jsonplugins/official/pythinker-datasource/.gitignoreplugins/official/pythinker-datasource/CHANGELOG.mdplugins/official/pythinker-datasource/SKILL.mdplugins/official/pythinker-datasource/bin/pythinker-datasource.mjsplugins/official/pythinker-datasource/pythinker.plugin.jsonplugins/official/pythinker-datasource/watchlist.json
💤 Files with no reviewable changes (12)
- plugins/official/pythinker-datasource/watchlist.json
- plugins/marketplace.json
- plugins/official/pythinker-datasource/.gitignore
- docs/customization/datasource.md
- plugins/cdn/marketplace.json
- plugins/official/pythinker-datasource/CHANGELOG.md
- docs/customization/plugins.md
- plugins/official/pythinker-datasource/pythinker.plugin.json
- plugins/official/pythinker-datasource/bin/pythinker-datasource.mjs
- plugins/official/pythinker-datasource/SKILL.md
- package.json
- apps/pythinker-code/test/utils/pythinker-datasource-plugin.test.ts
- .lintstagedrc.mjs: match ignore patterns against a POSIX-normalized relative path so a Windows-style staged path can't bypass an oxlint-ignored directory; add -- before staged paths in both oxlint invocations so a path shaped like an oxlint flag (e.g. --config=...) can't be parsed as one; use string replacement and unicode-mode regexes for the escape step. - preflight.ts: forward the selected update version to the Windows installer via a PYTHINKER_VERSION env override (install.ps1 already reads it) instead of always installing whatever the CDN currently reports as latest. - install.ps1: only remove a previous update's stale backup once the current target is confirmed present, so an interrupted prior update can't lose its only runnable executable; initialize $extractDir before entering the try block so a failure before its first assignment can't resolve a same-named variable left over in the caller's iex-hosted scope during cleanup. - README.md: restore the Node.js badge to 26+ to match package.json's actual >=26.4.0 engine requirement.
|
Addressed in d3a04cd:
All 7 inline threads addressed: 6 fixed and resolved, 1 (GOAL.md completion-contract ambiguity) left open with a reply — it's a genuine product-design question for whoever owns goal mode, not something to resolve unilaterally in this PR. |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/pythinker-code/src/cli/update/preflight.ts (1)
102-102: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRestrict native auto-install to supported platforms.
detectInstallSourcereturns'native'for any SEA process, andcanAutoInstallaccepts it on everyNodeJS.Platform. Unsupported platforms then use the Unixbashinstaller. Restore the platform guard or reject unsupported platforms during native source detection.🤖 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/cli/update/preflight.ts` at line 102, Update canAutoInstall to allow the native InstallSource only on supported platforms, preserving the existing behavior for other install sources. Ensure unsupported NodeJS.Platform values cannot proceed to the Unix bash installer, either by restoring the platform guard in canAutoInstall or by filtering them during detectInstallSource.
🧹 Nitpick comments (1)
apps/pythinker-web/public/install.ps1 (1)
497-504: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSave
apps/pythinker-web/public/install.ps1with a UTF-8 BOM.The file contains non-ASCII characters and has no UTF-8 BOM. Windows PowerShell 5.1 can misread these characters when it uses the system code page.
🤖 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-web/public/install.ps1` around lines 497 - 504, Save install.ps1 with a UTF-8 BOM while preserving its existing contents and behavior, so Windows PowerShell 5.1 correctly interprets the non-ASCII characters.Source: 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 `@AGENTS.md`:
- Line 26: Update the catalog link in AGENTS.md to use the repository-relative
target packages/kosong/src/catalog.ts instead of the developer-specific
filesystem path, and replace any internal identifiers in the surrounding public
text or test data with neutral placeholders.
In `@apps/pythinker-code/src/cli/update/preflight.ts`:
- Around line 151-160: Update the Windows auto-install path in the preflight
command construction around NATIVE_INSTALL_COMMAND_WIN so the install.ps1
bootstrap is authenticated before execution. Add verification using a trusted
signature or an independently authenticated checksum, and only invoke the
installer after that validation succeeds; do not leave the current irm | iex
flow as the first trust decision.
- Around line 813-817: Update the spawn options in the child process creation
flow to include windowsHide set to whether platform equals 'win32'. Keep the
existing detached, stdio, and environment options unchanged.
---
Outside diff comments:
In `@apps/pythinker-code/src/cli/update/preflight.ts`:
- Line 102: Update canAutoInstall to allow the native InstallSource only on
supported platforms, preserving the existing behavior for other install sources.
Ensure unsupported NodeJS.Platform values cannot proceed to the Unix bash
installer, either by restoring the platform guard in canAutoInstall or by
filtering them during detectInstallSource.
---
Nitpick comments:
In `@apps/pythinker-web/public/install.ps1`:
- Around line 497-504: Save install.ps1 with a UTF-8 BOM while preserving its
existing contents and behavior, so Windows PowerShell 5.1 correctly interprets
the non-ASCII characters.
🪄 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: 81b7aeb0-fc40-43ae-b4a1-18204672057c
📒 Files selected for processing (5)
.lintstagedrc.mjsAGENTS.mdapps/pythinker-code/src/cli/update/preflight.tsapps/pythinker-code/test/cli/update/preflight.test.tsapps/pythinker-web/public/install.ps1
🚧 Files skipped from review as they are similar to previous changes (1)
- .lintstagedrc.mjs
|
|
||
| ### Model Selection | ||
|
|
||
| Flows through the **catalog** ([catalog.ts](file:///Users/panda/Projects/active/pythinker-code-tsc/packages/kosong/src/catalog.ts)): |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a repository-relative catalog link.
Line 26 contains a developer-specific /Users/panda/... path. The link fails in other clones and exposes a local filesystem path. Replace it with packages/kosong/src/catalog.ts.
As per coding guidelines, replace internal identifiers with neutral placeholders in public text and test data.
🤖 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 `@AGENTS.md` at line 26, Update the catalog link in AGENTS.md to use the
repository-relative target packages/kosong/src/catalog.ts instead of the
developer-specific filesystem path, and replace any internal identifiers in the
surrounding public text or test data with neutral placeholders.
Source: Coding guidelines
| if (platform === 'win32') { | ||
| // install.ps1 reads $env:PYTHINKER_VERSION when set instead of | ||
| // fetching the CDN's current latest, so the version this preflight | ||
| // decided on is the one actually installed. | ||
| return { | ||
| cmd: 'powershell.exe', | ||
| args: ['-NoProfile', '-ExecutionPolicy', 'Bypass', '-Command', NATIVE_INSTALL_COMMAND_WIN], | ||
| env: { PYTHINKER_VERSION: version }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 \
'PYTHINKER_VERSION|irm|iex|Get-FileHash|Authenticode|signature|SHA256|hash|exit|throw' \
apps/pythinker-web/public/install.ps1Repository: Pythoughts-labs/pythinker-code
Length of output: 4061
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '20,65p;450,535p' apps/pythinker-web/public/install.ps1Repository: Pythoughts-labs/pythinker-code
Length of output: 5205
Supply-chain Integrity (CWE-494): Download of Code Without Integrity Check
Reachability: External · Exploitability: Difficult
Authenticate the Windows installer bootstrap before enabling auto-install.
install.ps1 executes through irm | iex before it verifies the binary. The binary and its SHA-256 file are fetched from the same GitHub release, so this does not provide independent authenticity. Sign or independently authenticate the bootstrap script and checksum.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn } from 'node:child_process';
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 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/cli/update/preflight.ts` around lines 151 - 160,
Update the Windows auto-install path in the preflight command construction
around NATIVE_INSTALL_COMMAND_WIN so the install.ps1 bootstrap is authenticated
before execution. Add verification using a trusted signature or an independently
authenticated checksum, and only invoke the installer after that validation
succeeds; do not leave the current irm | iex flow as the first trust decision.
- AGENTS.md: replace a developer-specific absolute file:// link with a repository-relative one, so it resolves in any clone. - preflight.ts: set windowsHide on the detached Windows update spawn — detached child processes get their own console window on Windows regardless of stdio, so stdio: 'ignore' alone did not suppress it.
Summary
install.ps1now stages the new binary and replaces the running exe with a rename-aside-then-move sequence (retrying cleanup of the old file since it may still be locked); the CLI drops the win32 gate, spawns the installer via PowerShell the same way it spawns curl|bash elsewhere, and sweeps any leftover renamed-aside binary on next startup.api.pythinker.com/coding/v1/tools, a gateway backend that was never built — every call 404'd. Removed the plugin, its CDN package, and its docs section entirely rather than ship a dead marketplace entry.build-cdn.mjsnow tolerates a missing plugin channel directory instead of throwing.apps/*/scripts/) failed the pre-commit hook; lint-staged now filters ignored paths itself via.lintstagedrc.mjs.Test plan
vitest runtargeted suites for update preflight, native-assets cleanup, and plugin marketplace/selector — all passingpwshsyntax check on install.ps1 (matches CI's check)pnpm --filter @pythoughts/pythinker-code typecheckcleanSummary by CodeRabbit
New Features
Removed
Documentation