Skip to content

chore: remove audit residue - #3242

Merged
Astro-Han merged 3 commits into
apache:mainfrom
Sun-GLiang:chore/3218-audit-residue-cleanup
Aug 20, 2026
Merged

chore: remove audit residue#3242
Astro-Han merged 3 commits into
apache:mainfrom
Sun-GLiang:chore/3218-audit-residue-cleanup

Conversation

@Sun-GLiang

@Sun-GLiang Sun-GLiang commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Related #3218
Related #3215
Related #3267

Summary

  • remove the orphaned Astryx alignment check while retaining the surface inventory gate
  • remove the unused DeepSeek live cost baseline entry point
  • remove stale active-code references to the obsolete Desktop readiness/rebind authority

The original branch also removed the obsolete Desktop chat-readiness module. That deletion and the associated compatibility-projection cleanup are now supplied by main through #3252. The rebase preserves #3252 unchanged and retains only the stale-comment cleanup that it did not cover.

The original branch also consolidated the duplicate CLI /skill: token authority. That exact three-file change is already owned by #3267, so it has been removed from this PR to avoid overlapping review and a guaranteed merge conflict. This PR therefore no longer independently closes #3218; the remaining CLI item belongs to #3267.

These changes remove unused consumers and stale authority references without changing product behavior, persistence, IPC, or database formats. Runtime V1 compatibility remains follow-up work under #3215.

Verification

  • npm run build
  • npm run typecheck
  • npm run lint
  • npm run format:check
  • npm run astryx:surface-inventory
  • npm --workspace maka-agent test
  • npm --workspace @maka/core test
  • npm --workspace @maka/runtime-host test
  • npm --workspace @maka/desktop test
  • git diff --check
  • repository-wide zero-reference and conflict-marker audit

AI use

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

Tool(s) and scope: OpenAI Codex contributed to implementation of the cleanup changes, repository analysis, conflict resolution, test execution, review follow-up, and PR preparation. The human contributor reviewed the work and submitted the contribution. Each materially AI-authored commit includes Generated-by: OpenAI Codex; the trailer must be retained in the final squash commit.

@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 021999a5-5e33-4be7-8b2a-0ec20e09911a

📥 Commits

Reviewing files that changed from the base of the PR and between b03b85e and 55a64c5.

📒 Files selected for processing (16)
  • apps/desktop/src/main/__tests__/chat-readiness.test.ts
  • apps/desktop/src/main/chat-readiness.ts
  • apps/desktop/src/main/main-window.ts
  • apps/desktop/src/main/onboarding-service.ts
  • apps/desktop/src/renderer/app-shell-session-start-actions.ts
  • apps/desktop/src/renderer/session-health-notice.ts
  • docs/astryx-alignment-inventory.md
  • package.json
  • packages/cli/src/pi-tui-pickers.ts
  • packages/cli/src/skill-highlight-editor.ts
  • packages/cli/src/skill-token.ts
  • packages/core/src/connection-readiness.ts
  • packages/core/src/session-send-projection.ts
  • packages/runtime/src/runtime-runner.ts
  • scripts/check-astryx-alignment.mjs
  • scripts/deepseek-live-cost-baseline.mjs
💤 Files with no reviewable changes (6)
  • scripts/check-astryx-alignment.mjs
  • package.json
  • apps/desktop/src/main/tests/chat-readiness.test.ts
  • docs/astryx-alignment-inventory.md
  • packages/cli/src/skill-token.ts
  • apps/desktop/src/main/chat-readiness.ts

Included review availability: Your plan provides up to 3 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Summary

This PR removes unused audit residue in four areas:

  • Deletes the Astryx alignment-check script and related documentation.
  • Removes the DeepSeek live cost baseline command and obsolete dependency path.
  • Makes Runtime token grammar the single /skill: authority. CLI retains local partial-prefix handling.
  • Removes the obsolete Desktop chat-readiness gate and its tests. Production error formatting remains local.

The changes remove duplicate authorities and unused consumers. They do not change product behavior, persistence, IPC, or database formats. Runtime V1 compatibility remains follow-up work under #3215.

Source of truth

The PR extends the existing Runtime /skill: grammar instead of creating a parallel path. The CLI keeps only the local behavior required for partial-prefix handling and imports the canonical token source from @maka/core/skill-invocation-token.

The Desktop changes remove an obsolete admission gate. Runtime Host projections and validation remain authoritative.

Scope and complexity

The changes form the smallest coherent cleanup described by the objectives. The added CLI helper is necessary to retain existing local TUI behavior after removing the duplicate parser. The local errorMessage helper is necessary for existing smoke-diagnostic logging.

The deleted chat-readiness tests and module remove obsolete behavior and regression coverage for that removed gate. The remaining Runtime and projection documentation reflects the current ownership model.

Complexity delta

The PR removes:

  • One Astryx validation script and its test/documentation residue.
  • One DeepSeek baseline command and its configuration entry.
  • One CLI token parser, token interface, re-export, and duplicate grammar authority.
  • One Desktop readiness module, its public helpers, its state transitions, and 550 lines of related tests.
  • Related comments, imports, and obsolete documentation.

The PR adds:

  • One small local CLI partial-prefix helper.
  • One local Desktop error-formatting helper.

It removes authorities, branches, configuration, public surface, and test-maintenance burden. Total maintenance complexity decreases.

Validation

The PR summary reports targeted package tests, typecheck, lint, formatting checks, diff validation, surface inventory, and a repository-wide zero-reference audit. Existing unrelated root test failures are documented.

The final status of required checks is unverified from the available evidence.

Review-relevant risks

The diff removes Desktop chat-readiness validation and its tests. This can affect session admission and error behavior, although Runtime Host validation remains documented as authoritative. This protected behavior requires independent human review under repository policy.

The diff removes CLI parser exports and changes token-grammar ownership. This can affect public package contracts and /skill: handling. Any material public-contract change requires independent human review under repository policy.

The person performing the merge must review the final diff. A maintainer makes the final determination.

Walkthrough

The change removes unused desktop readiness code and tests, consolidates CLI skill-token handling on shared grammar, deletes obsolete audit tooling, removes a cost-baseline script alias, and updates runtime ownership documentation.

Changes

Desktop readiness cleanup

Layer / File(s) Summary
Desktop readiness wiring and diagnostics
apps/desktop/src/main/chat-readiness.ts, apps/desktop/src/main/__tests__/chat-readiness.test.ts, apps/desktop/src/main/main-window.ts, apps/desktop/src/main/onboarding-service.ts, apps/desktop/src/renderer/...
The unused readiness module and tests are deleted. Diagnostic error formatting remains local to main-window.ts. Documentation uses Runtime Host projections and the current NO_REAL_CONNECTION format.
Shared readiness and execution contracts
packages/core/src/connection-readiness.ts, packages/core/src/session-send-projection.ts, packages/runtime/src/runtime-runner.ts
Documentation identifies shared readiness consumers and states that Runtime Host remains authoritative for execution and revalidation.

CLI skill-token consolidation

Layer / File(s) Summary
Shared skill-token parsing and picker integration
packages/cli/src/pi-tui-pickers.ts, packages/cli/src/skill-highlight-editor.ts, packages/cli/src/skill-token.ts
The local skill-token module is deleted. The picker keeps cursor-prefix detection, and the editor imports the token grammar from @maka/core/skill-invocation-token.

Audit residue removal

Layer / File(s) Summary
Obsolete audit tooling and script cleanup
scripts/check-astryx-alignment.mjs, docs/astryx-alignment-inventory.md, package.json
The alignment checker and its inventory entry are removed. The cost:deepseek-baseline npm script is removed.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 55a64

This cleanup removes unused checks, scripts, and stale readiness code while preserving the documented product behavior and compatibility scope; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR covers Astryx, CLI, and chat-readiness cleanup, but it does not delete the DeepSeek baseline script or obsolete dependency required by #3218. Delete scripts/deepseek-live-cost-baseline.mjs and remove its obsolete legacy compaction dependency, then rerun the zero-reference audit.
Ai Use Disclosure ⚠️ Warning The PR description selects neither AI-use declaration, and all four PR commits have no Generated-by trailer. Select exactly one declaration and provide tool/scope if applicable. See CONTRIBUTING.md, “Human ownership and AI attribution”; add trailers to material AI-authored commits so they survive squash or amend.
✅ Passed checks (3 passed)
Check name Status Explanation
Out of Scope Changes check ✅ Passed All listed changes align with the cleanup objectives in #3218; no unrelated product, persistence, IPC, or database changes are shown.
Title check ✅ Passed The title accurately identifies the pull request as cleanup of obsolete audit-related code and configuration.
Description check ✅ Passed The description covers the cleanup scope, verification commands, known failures, AI use, and linked issues, but omits the checklist section.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Remove orphaned audit tooling and duplicate runtime authorities

✨ Enhancement ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Removes orphaned Astryx checks and the unused DeepSeek cost baseline entry point.
• Centralizes complete skill token parsing in Runtime while retaining TUI prefix autocomplete.
• Retires Desktop readiness duplication and clarifies Runtime Host execution authority.
Diagram

graph TD
  TUI["TUI input"] --> Auto["Local autocomplete"]
  TUI --> Grammar["Runtime grammar"] --> Host["Runtime Host"]
  Connections["Connection facts"] --> Projection["Core projection"] --> Onboarding["Onboarding snapshot"] --> Notice["Health notice"]
Loading
High-Level Assessment

Directly deleting zero-reference tooling and obsolete Desktop gates is preferable to retaining compatibility wrappers or deprecated entry points. The PR preserves the active surface-inventory gate, keeps interactive prefix parsing close to the TUI, and leaves complete token and execution decisions with existing Runtime authorities.

Files changed (11) +48 / -44

Refactor (3) +22 / -3
main-window.tsLocalize smoke diagnostic error formatting +4/-1

Localize smoke diagnostic error formatting

• Removes the dependency on the deleted chat-readiness module by defining the generic error-to-string helper beside its only remaining consumer.

apps/desktop/src/main/main-window.ts

pi-tui-pickers.tsKeep partial skill prefixes local to autocomplete +17/-1

Keep partial skill prefixes local to autocomplete

• Moves incomplete '/skill:' prefix recognition into the TUI picker because partial tokens are an interactive autocomplete concern rather than canonical Runtime grammar.

packages/cli/src/pi-tui-pickers.ts

skill-highlight-editor.tsImport the shared skill token grammar directly +1/-1

Import the shared skill token grammar directly

• Uses the Core token source directly after removal of the duplicate CLI skill-token module.

packages/cli/src/skill-highlight-editor.ts

Documentation (7) +26 / -40
onboarding-service.tsDocument Runtime Host credential projection authority +2/-3

Document Runtime Host credential projection authority

• Replaces stale send-path wiring commentary with the current read-only Runtime Host credential projection behavior.

apps/desktop/src/main/onboarding-service.ts

app-shell-session-start-actions.tsRemove obsolete readiness source reference +1/-1

Remove obsolete readiness source reference

• Retains the setup-required error-code documentation without pointing to the deleted Desktop readiness module.

apps/desktop/src/renderer/app-shell-session-start-actions.ts

session-health-notice.tsClarify advisory session-health semantics +4/-4

Clarify advisory session-health semantics

• Documents the Core projection as an advisory renderer input and Runtime Host backend creation as the execution authority.

apps/desktop/src/renderer/session-health-notice.ts

astryx-alignment-inventory.mdRemove orphaned Astryx gate claim +0/-1

Remove orphaned Astryx gate claim

• Deletes the inventory entry claiming coverage from the removed alignment-check script while retaining the active surface inventory.

docs/astryx-alignment-inventory.md

connection-readiness.tsRefresh connection readiness authority documentation +8/-9

Refresh connection readiness authority documentation

• Updates comments to describe onboarding, task-submission, and session-health consumers without referencing the deleted Desktop send gate.

packages/core/src/connection-readiness.ts

session-send-projection.tsDefine session projection as advisory +8/-18

Define session projection as advisory

• Removes obsolete claims that the projection mirrors a Desktop send gate and clarifies that Runtime Host revalidates connection and model execution.

packages/core/src/session-send-projection.ts

runtime-runner.tsRemove obsolete future-gate commentary +3/-4

Remove obsolete future-gate commentary

• Generalizes RuntimeGate documentation around tests, adapters, and composition boundaries instead of a completed Desktop migration phase.

packages/runtime/src/runtime-runner.ts

Other (1) +0 / -1
package.jsonRemove DeepSeek baseline command +0/-1

Remove DeepSeek baseline command

• Removes the root npm entry point for the unused DeepSeek live cost baseline harness.

package.json

Astro-Han
Astro-Han previously approved these changes Aug 19, 2026

@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.

The current diff removes only orphaned tooling or duplicate authorities: complete skill-token parsing remains owned by Runtime/Core, the TUI keeps only its partial autocomplete prefix seam, and Desktop execution readiness remains owned by Runtime Host while the renderer projection stays advisory. The deleted chat-readiness module has no remaining production consumer, and no removed script/module references remain outside archived documentation.

No P0-P3 findings. git diff --check and the exact-head zero-reference audit are clean. CI and the bot reviews are still running, so this approval is not yet a merge-readiness claim.

AI-assisted review disclosure: Codex reviewed exact head 55a64c5, all four commits against the live base, current references, CI, and thread state; no external model was used.

中文说明

当前 diff 只删除无生产消费者的工具或重复权威:完整 skill token 解析仍由 Runtime/Core 负责,TUI 只保留局部 autocomplete prefix;Desktop 执行 readiness 仍由 Runtime Host 负责,renderer projection 只是提示。没有剩余引用,也没有 P0-P3。CI 和机器人审查仍在运行,因此当前是代码 APPROVE,不代表已经 merge-ready。

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Tip of the day
💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@Sun-GLiang

Copy link
Copy Markdown
Contributor Author

The only failing check is the known timing race tracked in #2916. After steering /compact explain, this run typed the next / before the composer reset/remount completed, leaving /compact explain/; the slash-command menu therefore never opened and the /side locator timed out.

I reproduced the same failure once locally, followed by 5/5 passing focused repetitions; the unchanged test passed 6/6 focused runs on current upstream/main. This PR does not modify the composer, slash-menu implementation, or the E2E. I added this failure variant and its CI evidence to #2916.

Could a maintainer please rerun the failed e2e job?

@Astro-Han

Copy link
Copy Markdown
Contributor

CI reruned!

@Astro-Han

Copy link
Copy Markdown
Contributor

One thing before merging this PR, Could we state if there is any AI tools used in the process? Thanks!

@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.

One provenance item remains before merge: the PR body does not currently state whether generative tooling contributed, and none of the four commit messages carries a Generated-by: trailer.

Please add an explicit AI-use declaration to the PR body. If no generative tool made a substantive contribution, saying so is sufficient and no trailer is needed. If one did, please name the tool and scope in the body and add Generated-by: <tool> to each materially AI-authored commit, preserving it in the final squash commit.

This is a provenance/governance gate rather than a code finding; the earlier technical approval otherwise stands.

中文说明

合并前还缺一项来源说明:当前 PR body 没有明确是否使用生成式工具,4 个 commit 也都没有 Generated-by: trailer。

如果没有实质 AI 辅助,在 PR body 明确写明即可,不需要 trailer;如果有,请在 body 中说明工具和范围,并为每个包含实质 AI 内容的 commit 补上对应 trailer,最终 squash 时继续保留。

这属于 provenance/governance gate,不是新的代码 finding;此前技术审查结论不变。

@Sun-GLiang
Sun-GLiang force-pushed the chore/3218-audit-residue-cleanup branch from 55a64c5 to 07ecab7 Compare August 19, 2026 12:12
@Sun-GLiang

Copy link
Copy Markdown
Contributor Author

Resolved the provenance gate:

  • the PR body now declares OpenAI Codex and its substantive scope;
  • all four materially AI-authored commits now include Generated-by: OpenAI Codex;
  • the final squash commit should retain that trailer.

The commits were rewritten only to add metadata. The aggregate tree hash remains df626e0dbd473c4c8dae2a4e279312fdef5f7471, identical to the reviewed revision.

@Sun-GLiang
Sun-GLiang force-pushed the chore/3218-audit-residue-cleanup branch from 07ecab7 to 5331982 Compare August 20, 2026 03:38
@Sun-GLiang

Copy link
Copy Markdown
Contributor Author

Resolved the merge conflicts by rebasing onto current apache/maka:main at f944ab6b1.

Conflict resolution preserves the Desktop authority cleanup already merged in #3252, including its narrower compatibility-projection exports and current Runtime Host ownership wording. The overlapping Desktop deletion was dropped from this branch; the fourth commit now contains only three stale active-code comment fixes not covered by #3252. The Astryx, DeepSeek, and CLI authority cleanups remain unchanged in intent.

Fresh verification on the rebased tree passed: full build, typecheck, lint, format check, Astryx inventory, CLI/Core/Runtime Host/Desktop tests, diff check, and repository-wide zero-reference/conflict-marker audits. All four commits retain Generated-by: OpenAI Codex.

@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.

Thanks — the deletions check out, and I verified each of them rather than taking the description's word for it. Reviewed exact head 53319827cf828c827c6b488e797c0a2bf7eef788.

packages/cli/src/skill-token.ts really was duplicate authority: parseSkillInvocationTokens and stripSkillInvocationTokens already live in packages/runtime/src/skill-invocation.ts, which is what interactive-turn-coordinator.ts and root-turn-coordinator.ts actually call, and the CLI copies had no consumer anywhere in the repository. The Astryx alignment script is genuinely orphaned — .github/workflows/ci.yml runs astryx:surface-inventory and astryx:theme, never the alignment check — and a search at this head finds no surviving reference to either deleted script or to the removed cost:deepseek-baseline entry. The comment cleanups are real fixes, not cosmetics: hasConnectionSecret and resolveConnectionSecret no longer exist anywhere in apps/desktop, so that comment was pointing at nothing.

Two P2s inline, both about the shape of the PR rather than about any deletion being wrong. Also worth noting factually: no CI checks are reported at this head, so the twelve local commands in the description are currently the only evidence for a change that removes a package.json script and 3,190 lines.

Review disclosure: this review was prepared with Claude Code, which read the diff at this head, ran the zero-reference searches described above against the head tree and against main, and compared this PR's CLI changes against #3267. Nothing here was reproduced by execution. The human contributor reviewed this before posting.

Comment thread packages/cli/src/pi-tui-pickers.ts Outdated
Comment thread packages/cli/src/pi-tui-pickers.ts Outdated
@Sun-GLiang
Sun-GLiang force-pushed the chore/3218-audit-residue-cleanup branch from 5331982 to a108efd Compare August 20, 2026 08:00

@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.

Thanks for narrowing this — both of my earlier P2s are resolved by the change itself. Re-reviewed exact head a108efd45a79ec80a9479102c125b2710e2a7f78.

The three CLI files are gone, so the collision with #3267 is gone with them and that PR can own the skill-token consolidation on its own; packages/cli/src/skill-token.ts is untouched here, which is the right end state. The hardcoded [A-Za-z0-9._-] copy went with it, so the grammar-duplication finding no longer applies to this PR either.

What remains is one coherent removal pass, and I re-verified it at this head rather than assuming it carried over: no reference anywhere in the tree to check-astryx-alignment, the removed cost:deepseek-baseline script, or the deleted DeepSeek baseline; CI runs astryx:surface-inventory and astryx:theme, never the alignment check, so the surface inventory gate is genuinely retained; and the three comment edits are real fixes rather than cosmetics, since hasConnectionSecret and resolveConnectionSecret no longer exist anywhere in apps/desktop.

No findings. Approving — the required test check is still running at this head, and branch protection will hold the merge until it is green.

@Astro-Han
Astro-Han merged commit 677e5ae into apache:main Aug 20, 2026
3 of 4 checks passed
@Sun-GLiang

Copy link
Copy Markdown
Contributor Author

Review follow-up is pushed at a108efd45: the overlapping CLI consolidation was removed from this PR, the branch was rebased onto current main (3db43c238), both inline threads were answered and resolved, and the PR body now leaves #3218 open for #3267.

The refreshed package check reached the end of the automatic-update flow, then failed while cleaning up an already-exited relaunched process:

taskkill /PID 10216 /T /F failed with exit code 128
ERROR: The process "10216" not found.

#3242 has no diff in the Windows packaging/update paths, and the same workflow completed successfully in the adjacent run 32344311384. I attempted a failed-job rerun, but GitHub requires repository admin rights. Could a maintainer please rerun the failed package job?

Fresh local verification at a108efd45 passed build, typecheck, lint, format check, Astryx inventory, CLI/Core/Runtime Host/Desktop tests, diff check, and the active-residue audit.

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.

chore: audit residue — dead scripts, duplicated /skill: grammar, chat-readiness remnant

2 participants