Conversation
Folds in the five open github-actions Dependabot PRs: actions/checkout v7.0.0 -> v7.0.1 (#209) actions/attest v4.2.0 -> v4.2.2 (#218) actions/upload-code-coverage v1.3.0 -> v1.4.2 (#237) github/codeql-action v4.37.0 -> v4.38.0 (#238, #241) pnpm/action-setup v6.0.9 -> v6.1.0 (#243) codeql-action/init and codeql-action/analyze move together on purpose. Dependabot raised them as two PRs, and each one alone fails the Analyze jobs with "Loaded a configuration file for version '4.37.0', but running version '4.38.0'" because the init step writes the config that analyze then reads. Bumping both in one commit is what makes them green. Every new SHA was checked against the GitHub API to confirm it is the commit the version tag points at (dereferencing the annotated tags for codeql-action, pnpm/action-setup and upload-code-coverage).
Folds in 40 of the open npm Dependabot PRs. The point of doing them as one change is that several are only correct together: each of these pairs fails CI when Dependabot raises it alone. vitest + @vitest/coverage-v8 4.1.5 -> 5.0.0 (#260, #242) vite + @vitejs/plugin-react 6.4.2 -> 8.2.2 / 4.7.0 -> 6.1.1 (#256, #273) tailwindcss + @tailwindcss/vite -> 4.3.3 (#277, #270) react + react-dom 19.2.5 -> 19.3.0 (#246, #239) Vitest rejects a mixed-major core/provider pair during coverage init; plugin-react 6 declares `vite: ^8`; React DOM refuses to load against a React of a different version; and Tailwind is only reached through its Vite plugin, so bumping the CSS package alone left the old compiler in place. Also in the catalog: @fontsource-variable/geist{,-mono} 5.3.0, @playwright/test 1.63.0, @testing-library/{jest-dom 7.0.1, react 16.3.3, user-event 14.6.7}, @vitest/eslint-plugin 1.6.27, c8 12.0.0, eslint-import-resolver-typescript 4.4.5, eslint-plugin-import-x 4.17.1, eslint-plugin-n 18.3.0, eslint-plugin-regexp 3.3.0, eventsource-parser 4.1.0, globals 17.12.0, oxfmt 0.67.0, oxlint 1.82.0, tsdown 0.23.0, turbo 2.10.12, typescript-eslint 8.70.0. Published runtime deps keep literal semver per the carve-out in pnpm-workspace.yaml: @clack/prompts ^1.8.0, @hono/node-server ^2.1.1 and commander ^15.0.0 on the two published packages; esbuild, hono, open, posthog-node and zod already had ranges covering their new releases, so only the lockfile moves for those. Four bumps are deliberately held back, each with the reason inline in pnpm-workspace.yaml: typescript (#272), jsdom (#264), eslint-plugin-unicorn (#274) and mint (#268). The `overrides` entry for @mintlify/mdx is new. Regenerating the lockfile at all makes pnpm auto-install a second react-dom@18.3.1 to satisfy that package's stale `^18.3.1` peer and pair it with react 19, which is the cross-major mismatch Codex flagged on roughly 25 of these PRs. peerDependencyRules only silences the warning; the override is what moves the resolution back onto one major.
…70 APIs Two type-level breaks came with the bumps in the previous commit. @clack/prompts 1.x types cancellation as "value is typeof CANCEL_SYMBOL", i.e. one *unique* symbol. Excluding that in the negative branch of isCancel does not remove the wider "symbol" a prompt's return type carries, so the existing inline "if (isCancel(x)) process.exit(1)" in create-arkor left x as "T | symbol" afterwards and failed to compile at four call sites. unwrapPrompt funnels that narrowing through one place, mirroring assertValue in the arkor package, which already worked because it casts. 1.x also widened Validate<T> to hand the callback "T | undefined" (an untouched field reads as undefined, not ""). promptText adapts at the boundary so its own signature keeps taking a plain string, which is what every caller is written against; create-arkor's inline validator coalesces the same way. Behaviour is unchanged: an empty field still produces the "cannot be empty" message on the retry pass. typescript-eslint 8.70 adds no-meaningless-void-operator, which flags "void CloudApiClient;" in whoami. That line existed only to keep an otherwise unused import alive, so both it and the import are gone rather than papering over it with a disable. ../../core/client has no top-level side effects, so dropping the import changes nothing at runtime.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (114)
🧰 Additional context used📓 Path-based instructions (8)Do not paper over this by adding an explicit `@oxlint/binding-` devDependency📄 CodeRabbit inference engine (AGENTS.md) Files:
Add vitest cases under `packages/*/src/**/*.test.ts` for SDK/CLI/scaffold logic changes.📄 CodeRabbit inference engine (AGENTS.md) Files:
Do not use the em dash character (U+2014) or its HTML entity in repository files outside the lint targets.📄 CodeRabbit inference engine (CONTRIBUTING.md) Files:
TypeScript/TSX のコード、コメント、文字列、テンプレートリテラルではエムダッシュ (U+2014) またはその HTML エンティティを使用しない。📄 CodeRabbit inference engine (CONTRIBUTING.ja.md) Files:
Do not use the em dash character (U+2014) in code comments, string literals, or template literals, including CLI messages, generated template bodies, and test names.📄 CodeRabbit inference engine (CONTRIBUTING.md) Files:
SDK、CLI、スキャフォルダーのロジックには Vitest のテストを追加し、Studio コンポーネントには jsdom と Testing Library ベースのテストを使用する。ただしテスト追加自体は PR の必須条件ではない。📄 CodeRabbit inference engine (CONTRIBUTING.ja.md) Files:
リポジトリ内の追跡対象ファイルでは、エムダッシュまたはその HTML エンティティを使用しない。Markdown、YAML、JSON、HTML、設定ファイル、生成テンプレートも含む。📄 CodeRabbit inference engine (CONTRIBUTING.ja.md) Files:
Maintain AGENTS.md as a living document with current agent status and architectural decisions📄 CodeRabbit inference engine (CLAUDE.md) Files:
🪛 LanguageToolAGENTS.md[uncategorized] ~114-~114: The official name of this software platform is spelled with a capital “H”. (GITHUB) [style] ~114-~114: Try using a descriptive adverb here. (ON_PURPOSE_DELIBERATELY) [uncategorized] ~115-~115: The official name of this software platform is spelled with a capital “H”. (GITHUB) [uncategorized] ~115-~115: The official name of this software platform is spelled with a capital “H”. (GITHUB) [uncategorized] ~115-~115: The official name of this software platform is spelled with a capital “H”. (GITHUB) [style] ~117-~117: The word ‘caveat’ is a legal term. To make your text as clear as possible to all readers, do not use this foreign term unless it is used with its legal meaning. Possible alternatives are “caution” or “warning”. (CAVEAT) 🔇 Additional comments (7)
WalkthroughThe pull request updates workspace and package dependencies, adapts CLI code for newer APIs, adds prompt boundary tests, and refreshes pinned GitHub Actions across build, CI, security, and release workflows. ChangesDependency and CLI refresh
Workflow action refresh
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Other Merge Risk: ⚪ Minimal · up to The dependency refresh includes compatibility updates and focused cancellation and validation coverage, with no remaining actionable merge risk identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 50.00% which is insufficient. The required threshold is 100.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 5 files. (3 skipped: 3 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Code Coverage OverviewLanguages: TypeScript TypeScript / code-coverage/arkorThe overall line coverage in commit 0b4fbe0 in the TypeScript / code-coverage/create-arkorThe overall line coverage in commit 0b4fbe0 in the Show a line coverage summary of the most impacted files.
TypeScript / code-coverage/cli-internalThe overall line coverage in commit 0b4fbe0 in the TypeScript / code-coverage/studio-appThe overall line coverage in commit 0b4fbe0 in the Updated |
Code Review BotNo reviewable code changes were analyzed. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
There was a problem hiding this comment.
All reported issues were addressed across 13 files
Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.
Re-trigger cubic
…ocstring Review follow-up on #282. greptile (P1) and cubic (P2) both landed on the same real problem: the scoped "@mintlify/mdx>react" override was half a fix. It moved React DOM off the auto-installed 18.3.1, but left React itself on the 19.2.3 the tree already had, so the docs graph still carried two different React versions. React DOM refuses to load against a React whose version differs from its own, so that is a runtime error waiting for the first render, and it is the same class of defect this PR set out to fix for the Studio app. Overriding react and react-dom outright collapses the whole install to one matched pair: the lockfile now holds exactly one react (19.3.0) and one react-dom, resolved as react-dom@19.3.0(react@19.3.0). "pnpm peers check" reports no React complaints; the one remaining entry is the pre-existing eslint-plugin-jsx-a11y-wants-eslint-9 warning that main already carries. cubic also noticed the comment read as stale against the lockfile: the committed "@mintlify/mdx" block shows peers of ^19.3.0, not the ^18.3.1 the comment cites. That is the override rewriting the recorded peer range, not upstream having widened it. The comment now says so, so the next reader does not draw the same inference. Separately (cubic P3): unwrapPrompt had been inserted between decideGitInit's docstring and decideGitInit itself, leaving that JSDoc attached to the wrong function. Moved above it. Re-verified after the change: build 3/3, typecheck 10/10, lint 7/7, format:check clean on 196 files, test 9/9 tasks, Studio E2E 9/9, mint validate passing.
| /** | ||
| * Narrow a clack prompt result to its value, exiting on cancellation. | ||
| * | ||
| * `isCancel` is typed `value is typeof CANCEL_SYMBOL`, i.e. it narrows to one | ||
| * *unique* symbol. Excluding that in the negative branch does not remove the | ||
| * wider `symbol` member a prompt's return type carries, so an inline | ||
| * `if (isCancel(x)) process.exit(1)` leaves `x` as `T | symbol` afterwards. | ||
| * Funnel the narrowing through here instead of casting at each call site. | ||
| * Mirrors `assertValue` in `packages/arkor/src/cli/prompts.ts`. | ||
| */ | ||
| function unwrapPrompt<T>(value: T | symbol): T { | ||
| if (clack.isCancel(value)) { | ||
| clack.cancel("Cancelled."); | ||
| process.exit(1); | ||
| } | ||
| return value as T; | ||
| } | ||
|
|
There was a problem hiding this comment.
This migration changes prompt cancellation, value unwrapping, and pre-input validation behavior without adding the Vitest coverage required by the repository guide for CLI and scaffolding logic changes. The related validation wrapper also converts an undefined prompt value to an empty string. Existing broad end-to-end runs do not add focused regression cases for these boundaries, so this repository requirement must be satisfied before merging by covering cancellation and validation before input.
Context Used: CLAUDE.md (source)
Knowledge Base Used: CLI project scaffolding
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/create-arkor/src/bin.ts
Line: 307-324
Comment:
**Prompt Changes Lack Tests**
This migration changes prompt cancellation, value unwrapping, and pre-input validation behavior without adding the Vitest coverage required by the repository guide for CLI and scaffolding logic changes. The related validation wrapper also converts an undefined prompt value to an empty string. Existing broad end-to-end runs do not add focused regression cases for these boundaries, so this repository requirement must be satisfied before merging by covering cancellation and validation before input.
**Context Used:** CLAUDE.md ([source](https://github.com/arkorlab/arkor/blob/main/CLAUDE.md))
**Knowledge Base Used:** [CLI project scaffolding](https://app.greptile.com/arkor/-/custom-context/knowledge-base/arkorlab/arkor/-/docs/cli-project-scaffolding.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Brings in ENG-1158, which removes the Mintlify toolchain. Two conflicts, both in files this branch rewrites heavily: pnpm-workspace.yaml: kept this branch's catalog bumps and dropped the "mint" entry, which main deleted along with the docs workspace. pnpm-lock.yaml: regenerated from main's side rather than hand-merged. Main removed 709 packages with Mintlify, so a textual resolution would have been guesswork; "pnpm install" against the merged catalog is the only trustworthy way to produce it. The merge also retires this branch's react/react-dom overrides, exactly as their own comment anticipated. They existed solely because @mintlify/mdx declared a stale "react-dom: ^18.3.1" peer, which made pnpm auto-install a second React DOM on a different major from React. With Mintlify gone the studio app is the only React consumer left, and the tree resolves to one matched pair, react-dom@19.3.0(react@19.3.0), with no override at all. Verified by grepping the regenerated lockfile for every react/react-dom entry. A global override is a blunt instrument to leave lying around once the thing it was aimed at is gone. Re-verified on the merge result: build 3/3, typecheck 10/10, lint 7/7, format:check clean on 194 files, test 9/9 tasks, Studio E2E 9/9. "pnpm peers check" reports only the pre-existing eslint-plugin-jsx-a11y-wants-eslint-9 warning that main already carries.
Self-review caught this: esbuild, hono, open, posthog-node and zod were all still sitting at the versions main already had. Their Dependabot PRs (#247, #278, #263, #254, #279) were therefore not folded in at all, even though the PR description claimed they were. The cause is a wrong assumption about pnpm. These five declare caret ranges in the published packages' dependencies, and the earlier reasoning was that a range already covering the new release means "pnpm install regenerates the lockfile onto it". It does not: install keeps any existing lockfile entry that still satisfies the range, so a dep only moves when its range stops matching or when pnpm update is run explicitly. The react family moved earlier only because that one did go through pnpm update. pnpm update rewrites the declared range to match what it installed, so packages/arkor/package.json picks up the new floors alongside the lockfile. zod lands on 4.6.1 and posthog-node on 5.51.8, both at or past what Dependabot asked for. Audited the whole set afterwards rather than spot-checking: every one of the 35 kept Dependabot targets is now met by the resolved version in the lockfile (not merely by the declared range, which is what hid this), and the three held-back packages are still exactly at typescript 5.9.3, jsdom 29.1.1 and eslint-plugin-unicorn 64.0.0. Re-verified with all five moved: build 3/3, typecheck 10/10, lint 7/7, format:check clean on 194 files, test 9/9 tasks, Studio E2E 9/9. The Studio suite matters most here since hono crossed a minor under the node-server 2.x adapter.
Review follow-up on #282. greptile (P2) is right that the clack migration changed CLI behaviour without the vitest coverage AGENTS.md asks for, and the two boundaries it names are the ones worth pinning: packages/arkor/src/cli/prompts.test.ts covers the validate adapter. It asserts the callback clack receives forwards "" for the undefined that 1.x passes before the first keystroke, and that a caller supplying no validator still gets undefined handed to clack rather than a wrapper that always passes. packages/create-arkor/src/bin.test.ts covers unwrapPrompt: cancelling either the name or the template prompt must call clack.cancel, exit non-zero, and leave scaffold() untouched. The validate case there drives a real directory collision first, because the coalescing only carries weight once retryInitial is armed; on the first pass the retryInitial !== null guard short-circuits before the value is read, so a test that skipped the collision would have passed either way. Each new case was mutation-checked against the pre-fix code rather than assumed to be load-bearing. Reverting the arkor adapter fails with "Cannot read properties of undefined (reading 'trim')"; dropping unwrapPrompt fails the cancellation case; reverting only the create-arkor coalescing fails the retry case with the same TypeError. The @arkor/cli-internal mock gains isClaudeCode. Every pre-existing test in that file passes yes: true and so never reaches the interactive branch, which is why the export was not needed until now. Also retires the "Commander v13" phrasing on the four comments that document why opts.agentsMd is undefined when neither --agents-md nor --no-agents-md is passed. Sentry flagged commander 15 as breaking that (MEDIUM), on the theory that v14's negatable-option change now requires an explicit .default(). Probed the installed commander 15 directly: with the positive flag declared first, no flag still yields undefined, and --agents-md / --no-agents-md yield true / false. Declaring --no-x alone is the case that defaults to true, and this code does not do that. So the finding does not reproduce and no behaviour changes here; the comments now name the verified version and the reason, so the next reviewer does not have to re-derive it.
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…e block Self-review of the previous commit. The clack 1.x boundary tests fake a TTY and clear CLAUDECODE so run() takes the interactive branch, but only restored the process.exit spy afterwards. The file's outer hooks cover CI and the cwd, not those two, so every test declared after that block ran against a fake TTY and a missing CLAUDECODE. Nothing is failing today, because what follows is buildCdLine's pure string cases. It is still a trap for the next test added below it, and packages/arkor/src/cli/prompts.test.ts already had the right pattern (ORIG_TTY captured and put back), so this now matches it. Confirmed the leak was real rather than theoretical: a temporary canary appended after the block asserting the original values fails without this restore and passes with it.
Review follow-up on #282. Two bot findings, neither reproducing as described, but both pointing at something worth changing anyway. cubic (P2, confidence 10) said the validate tests read the first lifetime clack.text call, so `passed` is undefined and the test fails. It does not: measured the call count inside the test and it is 1, because vitest 5 ships clearMocks: true in its own shipped defaults (read from the installed package) and none of the five vitest configs in this repo override it. The tests pass, and the earlier mutation checks already confirmed they read the call under test. The recommendation still stands on its own, because that behaviour rests on a framework default this file never asked for. `lastTextOptions` reads the tail instead. Proved the difference rather than assuming it: with clearMocks forced to false and an extra clack.text test inserted above, the new helper passes and the old calls[0] form fails. sentry (MEDIUM) said clack 1.x dropped `defaultValue` from text(), so Enter-on-empty would return "" and sanitise() would collapse the derived project name to the generic arkor-project fallback. It did not drop it: clack 1.8.0's TextOptions still declares `defaultValue?: string`, documented as "A fallback value returned when the user provides nothing (empty input)", which is exactly what this code wants. What that probing did turn up is that nothing here would have caught it if clack ever does drop it. `run()` spreads the option in conditionally, and TypeScript does not excess-property-check spread members: injecting a bogus property inside that spread compiles clean, so the call site is not the guard it looks like. Hence two additions rather than one: - `_DefaultValueStillSupported` indexes clack's own option type, so removal upstream fails the build. Verified by pointing it at a nonexistent key and watching tsc reject it. - A test asserting the first-pass prompt carries the derived name in both `placeholder` and `defaultValue`, with `initialValue` unset. That covers our side: a refactor dropping the option from this call. Mutation-checked by deleting `defaultValue`, i.e. reproducing the exact state sentry believed we were already in.
…ribe Self-review of the commit before this one, which also turned up a defect in 3e8b37b: inserting the clack 1.x describe block landed it between the "Round 40 follow-up" comment and describe("buildCdLine"), so that comment was left documenting whatever happened to follow it. Exactly the defect cubic flagged on this PR for unwrapPrompt and decideGitInit's docstring, reproduced in a different file and missed by me and by the bots the second time around. The new block and its type canary now sit above that comment, which is glued back to its describe. Move only: no test bodies or assertions changed. 51 passing in create-arkor either way.
The header claimed the "only members with an update available" rule as a property of the SECURITY lane. It is not: a grouped version-update PR carries only the members that have a release available too, so a publish that reaches one half of a pair before the other still lands one-sided. The note now covers both lanes and says what a group actually buys, which is the case where both halves are available and would otherwise arrive as two separately-red PRs (every row of arkorlab#282's table). It also names the residual honestly rather than leaving it implied: the window is narrow for these families, since each is published from one repository at one version and the shared cooldown delays both halves equally, but closing it needs an invariant CI can enforce rather than a Dependabot setting. Reported by CodeRabbit. The CI invariant it suggests is deliberately NOT in this PR: a new gate is its own change and CONTRIBUTING asks for an issue before one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Consolidates the open Dependabot backlog into one change: 43 of the 47 PRs, with 3 held back and 1 made moot upstream. Raising the PR limit in #236 released a queue that had been stuck at 5, and merging it one PR at a time would be both 47 full CI runs and, more importantly, wrong: several of these bumps are only correct in pairs.
Main is merged in, which brings ENG-1158 (the Mintlify removal).
Why one PR rather than 47
Each of these pairs is red on
mainas separate Dependabot PRs, and green here purely because they move together:vitest+@vitest/coverage-v8-> 5.0.0vite8 +@vitejs/plugin-react6.1.1vite: ^8; 4.7.0 supports only Vite 4-7 (310 checks red on #273)react+react-dom19.3.0tailwindcss+@tailwindcss/vite-> 4.3.3codeql-action/init+/analyze-> v4.38.0Loaded a configuration file for version '4.37.0', but running version '4.38.0'What is in
GitHub Actions (
3d3c42echeckout v7.0.1,1e69f48attest v4.2.2,d8e3291upload-code-coverage v1.4.2,b96794fcodeql-action v4.38.0,ea17c68pnpm/action-setup v6.1.0). Every new SHA was checked against the GitHub API to confirm it is the commit the version tag points at, dereferencing the annotated tags where applicable.Catalog (pnpm-workspace.yaml): the four pairs above plus
@fontsource-variable/geist{,-mono}5.3.0,@playwright/test1.63.0,@testing-library/{jest-dom 7.0.1, react 16.3.3, user-event 14.6.7},@vitest/eslint-plugin1.6.27,c812.0.0,eslint-import-resolver-typescript4.4.5,eslint-plugin-import-x4.17.1,eslint-plugin-n18.3.0,eslint-plugin-regexp3.3.0,eventsource-parser4.1.0,globals17.12.0,oxfmt0.67.0,oxlint1.82.0,tsdown0.23.0,turbo2.10.12,typescript-eslint8.70.0.Published runtime deps keep literal semver per the carve-out documented in
pnpm-workspace.yaml:@clack/prompts^1.8.0,@hono/node-server^2.1.1,commander^15.0.0,esbuild^0.28.2,hono^4.13.7,open^11.0.2,posthog-node^5.51.8,zod^4.6.1.Source changes are limited to following two API breaks the bumps introduced:
value is typeof CANCEL_SYMBOL, one unique symbol. Excluding that in the negative branch ofisCanceldoes not remove the widersymbola prompt's return type carries, so create-arkor's inlineif (isCancel(x)) process.exit(1)leftxasT | symboland failed to compile at four sites.unwrapPromptcentralises the narrowing, mirroringassertValuein the arkor package (which already compiled, because it casts). 1.x also widenedValidate<T>to passT | undefined;promptTextadapts at the boundary so its own signature keeps taking a plainstring.no-meaningless-void-operator, which flagsvoid CloudApiClient;inwhoami.ts. That line existed only to keep an otherwise-unused import alive, so it and the import are both gone rather than disabled in place.Held back, with the reason inline in
pnpm-workspace.yamltypescript7.0.2 — 368 checks red. Outside the peer range oftsdown/rolldown-plugin-dts(^5.0.0 || ^6.0.0) and oftypescript-eslint; the declaration build stops emittingdist/bin.mjsand the E2E suites fail wholesale.jsdom30.0.1 —enginesis^22.22.2 || ^24.15.0 || >=26.0.0, which excludes the published floor (Node 22.22.0) and most Node 24 entries the matrix deliberately covers. It fails for real onubuntu-latest / node >=24.10.0 <24.12.0. Taking it means raisingengines.nodeand trimming the matrix, i.e. changing the supported range of the published packages.eslint-plugin-unicorn74 — ten majors of new recommended rules. On this codebase they fire 648 times across 100 files:no-global-object-property-assignment(206) on the tests'globalThisstubs,consistent-boolean-name(109) on every boolean not prefixedis/has/can,no-top-level-assignment-in-function(46) on the vitestbeforeEachfixture pattern,prefer-await(39), andimport-style(47) telling us to invert thenode:pathconvention AGENTS.md documents. Deciding which of those the repo adopts is a lint-policy change, and AGENTS.md's standing guidance is to take rules one at a time rather than by category. Worth its own PR.mintis moot rather than held: ENG-1158 deleted the dependency, so there is nothing left to bump.Review findings addressed
There were no human reviews on the 47 source PRs. CodeRabbit skipped every one (
Bot user detected); socket-security, codecov and github-code-quality post informational output. The substantive reviews were 82 threads from Codex and the license bot, plus greptile and cubic on this PR.The four npm pairs Codex flagged are fixed by consolidating, as above. The fifth row of that table, the codeql
init/analyzesplit, surfaced from CI rather than from a review thread."Keep Mint's React and React DOM on the same major" was Codex's most repeated finding, on roughly 25 of the PRs, and greptile (P1) and cubic (P2) raised it again here. Regenerating the lockfile made pnpm auto-install a second
react-dom@18.3.1to satisfy@mintlify/mdx's stale^18.3.1peer and pair it with React 19. This branch first worked around it withoverrides; those are now gone, because ENG-1158 removed Mintlify and took the cause with it. With the studio app as the only React consumer left, the tree resolves to a single matched pair,react-dom@19.3.0(react@19.3.0), with no override at all. Verified by grepping everyreact/react-domentry in the regenerated lockfile.pnpm peers checkreports no React complaints; the one remaining entry is the pre-existingeslint-plugin-jsx-a11y-wants-eslint-9 warningmainalready carries.tsdown0.23 engines vs the Node 24.0-24.10 matrix entries (P1) andeslint-plugin-regexppullingjsdoc-type-pratt-parser@9.2.1(P2): accepted. pnpm does not enforceengines, both PRs were green, andpnpm build/pnpm lintpass here. Flagging rather than hiding: if those Node entries ever start failing on tsdown, this is the cause.TypeScript 7 peer range and jsdom engines: resolved by holding those two back.
cubic P3, docstring placement: valid, fixed.
unwrapPrompthad been inserted betweendecideGitInit's JSDoc and the function itself.greptile P2, "Prompt Changes Lack Tests": valid against AGENTS.md's standing rule, and now covered.
prompts.test.tspins the validate adapter (clack gets a callback that forwards""for the pre-keystrokeundefined, and a caller supplying no validator still hands clackundefinedrather than a pass-everything wrapper).bin.test.tspinsunwrapPrompt: cancelling either the name or the template prompt callsclack.cancel, exits non-zero, and leavesscaffold()untouched. Each case was mutation-checked against the pre-fix code rather than assumed load-bearing; details in the commit message. The validate case there deliberately drives a real directory collision first, because the coalescing only matters onceretryInitialis armed.sentry MEDIUM, commander 15 negatable options: does not reproduce, no change. The claim was that v14+ requires an explicit
.default()for an option declared with both polarities, soopts.agentsMdwould no longer beundefinedwhen neither flag is passed. Probed the installed commander 15 directly: with the positive flag declared first (which is how both CLIs declare it), no flag still yieldsundefined, and--agents-md/--no-agents-mdyieldtrue/false. Declaring--no-xalone is the case that defaults totrue, and neither CLI does that. The 11e2e/clicases that exercise the flag pass against the built binaries on commander 15, covering all three states: no flag,--agents-md,--no-agents-md, plus the both-flags rejection. The four code comments that documented this were phrased as "Commander v13 leaves this undefined"; they now name the verified version and the reason, so the next reviewer does not re-derive it.cubic P2, "reads the first lifetime
clack.textcall": does not reproduce, but changed anyway. Measured the call count inside the test: it is 1, because vitest 5 shipsclearMocks: truein its own defaults (read from the installed package) and none of this repo's five vitest configs override it. The tests were passing and reading the right call. The recommendation still stands, since that rests on a framework default the file never asked for, so alastTextOptions()helper reads the tail instead. Proved the difference: withclearMocksforced to false and an extraclack.texttest inserted above, the helper passes and the oldcalls[0]form fails.sentry MEDIUM, "clack 1.x dropped
defaultValue": does not reproduce. clack 1.8.0'sTextOptionsstill declaresdefaultValue?: string, documented as "A fallback value returned when the user provides nothing (empty input)". Probing it did surface a real gap, though:run()spreads that option in conditionally, and TypeScript does not excess-property-check spread members (verified by injecting a bogus property inside the spread and watchingtscpass), so the call site would not have caught an upstream removal. Added both a compile-time canary indexing clack's own option type and a test pinning that the first-pass prompt carries the derived name inplaceholderanddefaultValuewithinitialValueunset.license-compliance alerts cannot be fixed in the repo, and are worth reading before treating them as new risk. Every flagged package already exists on
mainat an older version, so this PR adds no new license exposure; the alerts re-fire because the versions moved. MPL-2.0 islightningcss(Vite's CSS transformer, onmainat 1.32.0) plusaxe-coreandico-endec; OFL-1.1 is the two Geist fonts, which the repo bundles deliberately (AGENTS.md keeps them independenciesso they reach the release SBOM); CC-BY-4.0 iscaniuse-lite; BlueOak-1.0.0 is theglob/tarfamily under c8 12. Everything except the fonts is build-time tooling. Clearing these needs a decision on the org's license allow-list, not a code change.Self-review found a real miss
esbuild,hono,open,posthog-nodeandzodwere still at the versionsmainhad, so PRs #247, #278, #263, #254 and #279 were not folded in, even though an earlier revision of this description claimed they were.The cause was a wrong assumption about pnpm: these five declare caret ranges, and "the range already covers the new release" is not the same as "install moves onto it".
pnpm installkeeps any lockfile entry that still satisfies the range; onlypnpm updateadvances it. The react family had moved earlier only because that one did go throughpnpm update.Fixed, and then audited rather than spot-checked: every one of the 35 kept Dependabot targets is now met by the resolved version in the lockfile, not merely by the declared range, which is what hid the problem. The three held-back packages are confirmed still at
typescript5.9.3,jsdom29.1.1 andeslint-plugin-unicorn64.0.0.Verification
All run on the merged branch:
pnpm install --frozen-lockfilepnpm builddist/bin.mjsemitted by tsdown 0.23pnpm typecheckpnpm lintFound 0 warnings and 0 errorsper packagepnpm format:checkpnpm testpnpm --filter @arkor/e2e-studio testnode scripts/check-no-em-dash.mtsTwo things the suites cannot reach were checked by hand, both on the
@hono/node-server1.x -> 2.x major:/api/*token check including both 403 cases, and the non-loopback Host rejection, i.e. the CSRF/DNS-rebinding contract AGENTS.md requires be preserved.dev.test.tscannot cover because it mocks@hono/node-serveroutright. Ran twoarkor devinstances on the same explicit port against an isolatedHOME: the second exits 1 withPort 41877 is already in use. Another 'arkor dev' may be running..., the first stays alive, and its~/.arkor/studio-tokenis left intact. That last part is the one that matters, since the code comments call out a failed second launch deleting a healthy instance's token as the hazard.e2e/cliwas run withSKIP_E2E_INSTALL=1(106 passed). The real-<pm>-install cases need to download package trees per fixture, which this connection cannot do in reasonable time; CI's install matrix covers them.Notes
e2e/cliprompt suites (arkor init,create-arkor, and theCLAUDECODE=1strict-mode cases).AGENTS.mdgets one edit and has no JA mirror.AGENTS.mdhonesty fix: the ENG-788 bullet claims oxlint/ESLint rule parity "as of oxlint 1.66". This moves the pin to 1.82 without repeating that audit, so the bullet now says so rather than silently reading as though the claim still holds.pnpm-workspace.yaml(kept this branch's catalog bumps, dropped themintentry main deleted) andpnpm-lock.yaml(regenerated from main's side rather than hand-merged, since main removed 709 packages with Mintlify).