Skip to content

refactor(core): remove dead exports and relocate test-only machinery - #3094

Merged
Astro-Han merged 1 commit into
apache:mainfrom
CxHsin:refactor/core-remove-dead-exports
Aug 16, 2026
Merged

refactor(core): remove dead exports and relocate test-only machinery#3094
Astro-Han merged 1 commit into
apache:mainfrom
CxHsin:refactor/core-remove-dead-exports

Conversation

@CxHsin

@CxHsin CxHsin commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Remove dead core contracts and keep test-only machinery out of the contracts package output. Delete the unused bot-platform hints and settings result re-export, move the provider conformance matrix into the runtime test surface, move the core test helper under core/src/__tests__, and update the affected test imports.

Fixes #3089

Verification

  • npm --workspace @maka/core test — 544 tests passed
  • Provider conformance focused test — 120 tests passed
  • npm run lint
  • npm run format:check
  • npm run build
  • npm run typecheck
  • npx knip --workspace apps/desktop
  • npx knip --workspace packages/ui
  • Core dist no longer contains bot-platform-hints.js, settings/result.js, provider-contract-matrix.js, or root test-helpers.js
  • Runtime conformance matrix is emitted under dist/__tests__

The full npm --workspace @maka/runtime test suite was not clean in this Windows environment: unrelated shell/PTY, Seatbelt, and POSIX process-semantics tests failed and the run was stopped after it continued without progress. The changed provider conformance suite passed independently.

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex analyzed issue #3089, relocated and removed the specified modules, updated test imports and exports, ran verification, and reviewed the implementation. The human contributor remains responsible for reviewing and submitting the contribution.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@coderabbitai

coderabbitai Bot commented Aug 15, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary

This PR removes unused core contracts and keeps test-only code out of the published @maka/core package.

It deletes bot-platform-hints.ts and the dead settings/result.ts re-export. It moves the provider contract matrix and test helper into test-only runtime and core paths. It updates affected test imports and removes the provider matrix package export.

The PR extends the existing provider conformance tests. It does not create a parallel production path. The provider matrix remains available to runtime tests through local imports.

This is the smallest coherent solution. It removes unused files and package exports while preserving existing test behavior. No additional runtime logic or production machinery is added.

The deleted bot-platform hints and settings re-export can remain removed because the objectives identify them as unused. The provider matrix and test helper cannot be deleted because tests still use them.

Validation includes core tests, the focused provider conformance suite, lint, formatting, build, typecheck, and targeted Knip checks. The full runtime test suite remains unclean in the reported Windows environment because of unrelated shell, PTY, Seatbelt, and POSIX process-semantics failures.

Review-relevant risks

The package export removal and deletion of exported symbols change the public @maka/core contract. Material public-contract or release-impact changes require independent human review under repository policy.

No other protected-area effect was identified in the current diff. Required-check status remains unverified beyond the reported validation. The person performing the merge reviews the final diff, and a maintainer makes the final determination.

Walkthrough

The PR removes obsolete core exports, deletes platform hint and settings result modules, relocates the test assertion helper, and updates core and runtime test imports.

Changes

Core contract cleanup

Layer / File(s) Summary
Public contract cleanup
packages/core/package.json, packages/core/src/bot-platform-hints.ts, packages/core/src/settings/result.ts
The provider contract matrix export and obsolete platform hint and settings result exports are removed.
Core test helper relocation
packages/core/src/__tests__/test-helpers.ts, packages/core/src/__tests__/*.test.ts, packages/core/src/settings/__tests__/sensitive.test.ts
The expect helper is defined in the test directory. Core tests use the local helper path.
Runtime contract test wiring
packages/runtime/src/__tests__/provider-contract-matrix.ts, packages/runtime/src/__tests__/provider-contract-matrix.test.ts
The runtime contract test uses its local matrix module and @maka/core package imports.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 2057b

The refactor removes dead exports and relocates test-only files; the moved helper may still be emitted under dist/tests, but this creates no published-artifact or runtime impact. No actionable merge-blocking risk remains.

Possibly related PRs

Suggested reviewers: uncertaintydeterminesyou4ndme, jackwener

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The diff summary does not show deletion of settings/result.ts, and it does not evidence removal or relocation of the core provider contract matrix. Delete packages/core/src/settings/result.ts and confirm that the core provider contract matrix is moved under tests or otherwise excluded from the package output.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the removal of dead exports and relocation of test-only machinery.
Description check ✅ Passed The description includes the required summary, issue reference, verification results, AI disclosure, and checklist responses.
Out of Scope Changes check ✅ Passed The listed changes are related to removing dead exports and relocating test-only machinery described in issue #3089.
Ai Use Disclosure ✅ Passed The PR selects generative tooling, names Codex and its scope, and the sole introduced commit has the matching standalone trailer Generated-by: Codex.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
packages/core/src/__tests__/test-helpers.ts (1)

3-3: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Do not treat this move as excluding test code from dist. The core build includes src/**/*.ts, so it emits dist/__tests__/test-helpers.js and its declaration. @maka/core is private, so this is not a published-artifact issue.

Source: Path instructions


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: fe1789e2-dda1-4a33-b86d-b62e1799c939

📥 Commits

Reviewing files that changed from the base of the PR and between 90b78a3 and 2057b7d.

📒 Files selected for processing (18)
  • packages/core/package.json
  • packages/core/src/__tests__/additional-permissions.test.ts
  • packages/core/src/__tests__/capabilities.test.ts
  • packages/core/src/__tests__/computer-use.test.ts
  • packages/core/src/__tests__/events.test.ts
  • packages/core/src/__tests__/health.test.ts
  • packages/core/src/__tests__/permission-profile.test.ts
  • packages/core/src/__tests__/provider-auth.test.ts
  • packages/core/src/__tests__/runtime-event.test.ts
  • packages/core/src/__tests__/sandbox-boundary.test.ts
  • packages/core/src/__tests__/settings.test.ts
  • packages/core/src/__tests__/task-submission-readiness.test.ts
  • packages/core/src/__tests__/test-helpers.ts
  • packages/core/src/bot-platform-hints.ts
  • packages/core/src/settings/__tests__/sensitive.test.ts
  • packages/core/src/settings/result.ts
  • packages/runtime/src/__tests__/provider-contract-matrix.test.ts
  • packages/runtime/src/__tests__/provider-contract-matrix.ts
💤 Files with no reviewable changes (3)
  • packages/core/package.json
  • packages/core/src/settings/result.ts
  • packages/core/src/bot-platform-hints.ts

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed against #3089.

bot-platform-hints.ts and settings/result.ts have no remaining importers. provider-contract-matrix belongs next to the runtime conformance suite, not in the contracts package; the suite still imports it from ./provider-contract-matrix.js. The test import rewrites look complete, including settings/__tests__/sensitive.test.ts.

One acceptance line is not literally true. core tsconfig.json is include: ["src/**/*.ts"] with an empty exclude, and the suite runs dist/**/*.test.js, so src/__tests__/test-helpers.ts still emits dist/__tests__/test-helpers.js. The package is private, so this is not a release problem. Fixing it means changing how core is built and tested; do not fold that in here.

Approve.

AI-assisted review: Grok 4.6 drafted the first pass. A second pass on opencode-go/deepseek-v4-flash:max re-grepped the four named symbols on main and on the PR branch and confirmed the conformance import still resolves. I checked the four named deletions, the core exports map, and the tsconfig include/exclude on the PR branch. Unverified by me: I did not rebuild core or rerun the conformance suite.

@Astro-Han

Copy link
Copy Markdown
Contributor

Fast path: dead-contract deletion plus moving test-only files next to their suites. No public export consumers remain for the deleted modules. The package is private, so the leftover dist/__tests__/test-helpers.js emission is not a release change. Required checks are green. Easy to revert.

I reviewed the final diff and am merging on that basis. Human contributor of record for this review: Astro-Han.

@Astro-Han
Astro-Han merged commit 2b1e647 into apache:main Aug 16, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(core): remove dead exports and relocate test-only machinery

2 participants