refactor(storage): drop the barrel and publish narrow entrypoints - #2
Closed
childrentime wants to merge 8 commits into
Closed
childrentime wants to merge 8 commits into
childrentime wants to merge 8 commits into
Conversation
…#3288) Branch protection landed in .asf.yaml (apache#3262) and now requires an approving review and a passing `test` check on every pull request to main, with stale approvals dismissed on each new commit. Every open pull request based on main reports reviewDecision REVIEW_REQUIRED; apache#3282, based on a feature branch, reports nothing. That leaves the fast path with nothing to exempt. CONTRIBUTING defined it as merging without independent human review, which a committer can no longer do. Redefining it as "the baseline alone" does not rescue it: the extra scrutiny in this section applies only to protected areas, and not touching a protected area was already a fast-path precondition, so the two scopes never overlap. A named fast path would exempt nothing while adding a comment obligation, and the maintainer sign-off it claimed to skip was never written down anywhere. State the rule directly instead. The section gives the baseline every pull request clears, names a maintainer as the one who decides whether a change is material and whether the review it received is enough, and closes with "for everything else the baseline is enough" — which answers which changes take the light route without keeping a concept that no longer carries content. Naming the classifier keeps what the old "a maintainer makes the final determination" actually did: settle whether a change needs more than the mechanics. The merge-time comment does not survive; it existed to leave a trace for merges that had no approval, and every merge now has one. Two corrections in the same section. The baseline says branch protection enforces the mechanics and that independent human judgment is policy GitHub cannot verify, because an approval from someone other than the author is not by itself proof of an independent human. The Chinese text is realigned so 重大 distributes across the whole protected list, matching the English, instead of modifying only the first item. The public-decision rule moves from future to present tense. dev@maka.apache.org exists and carries active threads, so project-level decisions belong there now rather than "once an ASF development list is available". .coderabbit.yaml and .github/skills/code-review/SKILL.md told automated reviewers not to make a fast-path determination. With the concept gone, both now say only that automated review is not authorization to merge. Their neighbouring lines about independent human review remain accurate and are left alone. Generated-by: Claude Code
The release environment holds the Apple notarization and code signing credentials that .github/workflows/release-desktop.yml consumes, but it carries no protection rules at all. Its only current guard is the job-level branch condition inside release-desktop.yml, which lives in the same file it is meant to protect and so cannot bind the environment on its own. Declare a deployment branch policy that restricts the environment to main. GitHub enforces it outside the workflow, before any job that requests the environment starts. npm-release already carries the same kind of policy, so this reuses an established pattern rather than inventing one. The directive only visits environments named here, so npm-release and copilot keep their current configuration. npm-release is deliberately left undeclared: the underlying call replaces an environment's settings wholesale, and declaring it without restating its reviewer and branch policy would clear them. Generated-by: Claude Code
…he#3294) Two unrelated test suites have been failing intermittently on green PRs, and since the Apache migration a flaked lane costs a full CI cycle plus a re-review round, because contributors cannot re-run a single lane. computer-use teardown (apache#3290): the suite's after() hook disposed every service and immediately deleted the shared work directory. dispose() is deliberately fire-and-forget — it SIGTERMs the child and schedules an asynchronous image-directory purge on child exit (or on the 3s shutdown-grace SIGKILL) — so the recursive rm raced concurrent purges and a child still flushing its ndjson log, and failed with ENOTEMPTY. The teardown now waits for every per-service image directory to vanish (the 'children exited and purges finished' barrier, tolerant because purge failure is permitted by contract) and retries the final rm as a backstop. slash-command e2e (apache#3289): the 'compacts the active session' spec filled the composer right after the compact completed, in the same remount-vs-fill race that dd4b2d0 removed from the running-turn spec — fill() can land before the contentEditable regains focus, the draft never populates, Enter submits nothing, and the 'Fake backend received: after compact' assertion times out (run 32157698387, attempt 1, line 83). Type through the focused element and require the draft to settle before dispatching, mirroring dd4b2d0. No assertion is weakened and no timeout is raised by either change. Closes apache#3289 Closes apache#3290 Generated-by: Claude Fable 5 Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…pache#3264) Bumps the official-actions group with 1 update: [actions/setup-python](https://github.com/actions/setup-python). Updates `actions/setup-python` from 6.2.0 to 7.0.0 - [Release notes](https://github.com/actions/setup-python/releases) - [Commits](actions/setup-python@a309ff8...5fda3b9) --- updated-dependencies: - dependency-name: actions/setup-python dependency-version: 7.0.0 dependency-type: direct:production update-type: version-update:semver-major dependency-group: official-actions ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* chore(ui): remove orphaned Astryx alignment check Generated-by: OpenAI Codex * chore(runtime): remove orphaned DeepSeek cost baseline Generated-by: OpenAI Codex * docs: remove stale desktop readiness references Generated-by: OpenAI Codex
…#3283) * Use realpath for eval network policy preflight test * Canonicalize eval network policy path in preflight test
`maka --help` printed Node's SQLite ExperimentalWarning to stderr, on a command that never opens a database. The cause was structural, not local: `@maka/storage` published one `export *` barrel, and `cli-core.ts` imported it to reach `resolveMakaDataRoots`. Three modules in that barrel's graph take a static value import of `node:sqlite`, and Node evaluates a builtin the moment it enters a module graph, so every consumer of the barrel loaded SQLite whether or not it wanted a database. `operational-target-schema.ts` was the amplifier: `operational-state-store.ts` imports it, and roughly forty modules import that, which is how three import statements reached 45 of the package's 110 modules and 24 of the barrel's 43 export entries. Remove the barrel instead of working around it. `.` is gone from the exports map, `src/index.ts` is deleted, and the 21 modules that consumers actually reached through it are published as narrow subpaths. This is already the prevailing convention here — `root-authority` has 141 call sites and `execution-stores` 108, against 31 non-test sites on the bare specifier. The three static `node:sqlite` imports stay exactly as they were. They are honest: those modules do need SQLite. What changes is that needing SQLite is now visible in the import path, so `@maka/storage/workspace-root` costs nothing and `@maka/storage/session-store` costs what it should. No lazy-load indirection and no warning suppression are involved. `public-entrypoints.test.ts` pins the boundary: no `.` export, and exactly 29 of the 52 published entrypoints load `node:sqlite`. Widening that set now requires editing the list and saying why. Two tests moved off the barrel's shape rather than its contents: `managed-workspace-baseline` asserted internals were absent from the barrel object and now asserts their modules are absent from the exports map; `provider-request-capture-artifact` reaches its subject directly. Generated-by: Claude Code
childrentime
force-pushed
the
fix/storage-drop-barrel
branch
from
August 20, 2026 09:24
3cfdb17 to
0124d97
Compare
Owner
Author
|
Submitted upstream as apache#3301. Closing this review copy. |
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
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.
Summary
maka --helpprinted Node's SQLiteExperimentalWarningto stderr, on a command that never opens a database. Every other CLI entry did the same. The cause is structural rather than local, so this removes the structure instead of muffling the symptom:@maka/storageno longer publishes a barrel.Fixes the regression first reported in apache#1257.
Why the warning was there
Node evaluates a builtin the moment it enters a module graph. A static value import is therefore not a declaration of intent — it is the load:
Exactly three modules do this:
operational-target-schema.ts,operational-state-backup.tsandsession-bundle-policy.ts.operational-target-schema.tsis the amplifier.operational-state-store.tsimports it — andoperational-state-store.tsis imported by roughly forty modules, which is how three import statements reached 45 of the package's 110 modules and 24 of the barrel's 43 export entries.operational-state-store.tsalready loaded SQLite lazily; it was pulled under anyway by the module it imports.The last link is the CLI.
cli-core.tsimports@maka/storageto reachresolveMakaDataRoots, a pure path helper. The barrel'sexport *surface hands it the whole storage layer, SQLite included, before--helpprints a single line.Change
packages/storage/src/index.ts(159 lines) and drop.from theexportsmap.packages/runtimeare all tests; that package's production code is untouched.managed-workspace-baselineasserted that internals were absent from the barrel object and now asserts that their modules are absent from theexportsmap — a stronger claim, since reachability is now decided there.provider-request-capture-artifactreaches its subject directly.Regression guard
public-entrypoints.test.tsimports all 52 published entrypoints concurrently (0.6s) and asserts:.entrypoint;node:sqliteare exactly the 29 listed in the test, leaving 23 that are free of it.That list is the package's SQLite boundary written down. Widening it later means editing the list and saying why.
This matters because of how apache#1257 came back. Its guard,
package-import.test.ts, asserted an empty stderr on the package entrypoint; apache#2710 removed it in a bulk test cleanup, apache#1994 then added two of the static imports and apache#2445 a third, and the warning shipped. A list of entrypoints is harder to delete without noticing than an assertion that something is empty.Validation
Node v24.11.1, macOS arm64. Before/after measured on the same source tree by checking
mainout, building, running, and switching back.mainmaka --versionmaka --helpmaka run --helpmaka run <prompt>, isolated profileThe last row is the case laziness alone cannot cover, because a real session does open a database. It stays clean because the Runtime Host that owns the database is a child process spawned with
stdio: 'ignore'(packages/runtime-host/src/client/launcher.ts), andpackages/cli/srcopens no database itself. In the isolated profile used for that row, the host created and initialised a 917 KBruntime.sqlitewhile the client's stderr stayed empty.npm run buildnpm run typechecknpm run lintnpm test --workspace maka-agentnpm test --workspace @maka/runtime-hostnpm test --workspace @maka/storagemain)Remaining warning sites
The paths that still emit the warning all genuinely use SQLite, so the warning is expected there:
maka eval(snapshots result databases) andmaka activate(validates session bundles against SQLite files) — including their--help, since the command module loads before argument handling;@maka/storage;Compatibility
Dropping
.is a breaking change for anything importing the bare specifier.@maka/storageis not published independently and every consumer lives in this repository, so the blast radius today is zero — but if the package is ever published on its own, this belongs in the release notes.Generative tooling disclosure
Claude Code (Claude Fable 5) made a substantive contribution: it traced the regression to the barrel, performed the mechanical rewrite, wrote
public-entrypoints.test.ts, ran the before/after measurements, and drafted this description. Commits carry theGenerated-by: Claude Codetrailer.A human contributor of record has reviewed the diff, verified the evidence above and decided to submit it.