Skip to content

fix(agent): let native CLIs own their instruction files - #6655

Merged
M4n5ter merged 1 commit into
rust-previewfrom
fix/external-cli-instruction-ownership
Nov 6, 2026
Merged

M4n5ter merged 1 commit into
rust-previewfrom
fix/external-cli-instruction-ownership

Conversation

@M4n5ter

@M4n5ter M4n5ter commented Nov 6, 2026

Copy link
Copy Markdown
Member

Summary

Maka and native CLIs could both load rule files, duplicating instructions with different discovery scopes and precedence.

  • Let external Codex, Claude and agy own their native global and project rule-file discovery. Maka no longer copies the other global or project instruction files into their injected instruction block.
  • Keep Maka identity, selected global preferences, configured Maka roles and tool/skill guidance. Global preferences continue to use the same selected file as the editor: .maka/AGENTS.md, falling back to .agents/AGENTS.md only when the preferred file is absent.
  • Preserve built-in model instruction loading, admission-time capture and recovery of the accepted prompt. Do not add CLI flags, disable native discovery or change permission boundaries.
  • Document the split in English and Chinese, including that Maka's workspace-instruction preference does not disable CLI-native loading.

Verification

  • cargo fmt --all -- --check and final cargo fmt -p maka-agent-native -- --check: passed.
  • git diff --check: passed.
  • Targeted cargo nextest run --locked -p maka-agent-native --lib regressions:
    • native_external_all_protocols_persist_before_prompt_and_resume_original_identity: passed; covers all three local protocol fixtures, global preference replacement, unchanged-instruction suppression despite project-rule edits, tool receipts and native identity reuse.
    • global_instruction_api_and_admission_share_one_selected_file: passed; covers built-in model loading, external Maka-only capture, selected-file fallback and disabled capture. An initial new assertion incorrectly expected a trailing separator after the final section; it was corrected to assert the exact complete prompt, then rerun successfully.
    • external_skills_candidates_steering_and_queue_edits_use_the_same_bridge_ceiling: passed after adding assertions that skill metadata and loading guidance remain in the delivered external instructions.

These are local fixtures, not paid live-CLI tests. The full workspace suite and full native-crate suite were not run. No running CLI, shared managed daemon or active deployment was changed.

AI use

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

Tool(s) and scope: Maka implemented the change, regression assertions and documentation. The commit includes Generated-by: Maka; preserve it in the squash commit.

Checklist

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

Formatting and compilation of the native test target passed, as did the selected behavior regressions above; broader suites were not run.

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

Keep Maka identity, selected global preferences and tool/skill guidance in external CLI instructions without duplicating native global or project rule files. Preserve built-in model instruction loading and document the ownership boundary.

Generated-by: Maka
@github-actions github-actions Bot added the effort/XXL Over 2500 readable lines label Nov 6, 2026
@M4n5ter
M4n5ter changed the base branch from main to rust-preview November 6, 2026 01:49
@M4n5ter
M4n5ter merged commit a61a85d into rust-preview Nov 6, 2026
1 check passed
@M4n5ter
M4n5ter deleted the fix/external-cli-instruction-ownership branch November 6, 2026 01:51

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

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

I reviewed all nine changed files at 1ccb47d2e7a42177f180a904301ad5548bca11a4 (+55/−27), including instruction capture, admission, notification/scheduler callers, the three regression-test changes and both README versions. I found no confirmed P0–P2 issue in this diff.

In owner.rs, the existing workspace::validate still runs before prompt capture. Only the native external executor (host_bridge) now passes None to capture_behavior; built-in model admission passes the workspace. The notification and scheduler paths also pass a workspace and construct model-backed, non-executor message bases. In owner/assistant.rs, the new early return happens after the selected Maka global preferences are read, and before other global/project rule files are opened. Prompt composition and configured-role handling remain after that branch. I also read the unchanged global-file selector: only absence of .maka/AGENTS.md permits the .agents/AGENTS.md fallback.

The regression changes assert that the native protocol fixtures receive Maka identity, selected global preferences and tool/skill guidance without the separately discovered CLI rule text; they also check unchanged-instruction suppression, updates and native identity reuse. The settings test distinguishes built-in and external capture and the disabled preference. These are inspected assertions and the author's reported local results: I did not rerun those Rust tests, a complete Host, a restart/recovery scenario or paid/live Codex, Claude or agy CLIs. In particular, this review does not independently validate each real CLI's current discovery rules or configuration.

I reproduced the clean merge of this head with 445767cc23447e97411457d7affd10e2f01b5abf; its resulting tree exactly equals the already merged successor a61a85d145e7eae0f381aabb08ed69a7c2084c46. At this check that successor's CI is still running. There is no new protocol/storage schema change in this diff. The documented preference controls Maka's capture and does not promise to disable the CLI's own rule discovery.


Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

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

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

A P2 follow-up to my earlier review: the merged CI now confirms an incomplete test migration caused by the new instruction-capture behavior.

native_external_cancelled_instruction_ack_does_not_advance_confirmed_digest still changes the project AGENTS.md at owner/tests/executor/external.rs:622 and :639, then waits for before_instructions_ack at :650. The new native-CLI capture deliberately ignores that file, so this change no longer creates an instruction update. The fixture instead receives opened, and the test fails before it can stop the invocation and verify the unconfirmed digest.

I read the original failed log from a61a85d1 CI 42146620228, native shard 7 job 127667617505: 40 of 41 tests pass; this test fails in 5.930 seconds at external_support.rs:128, with opened where before_instructions_ack was expected. In the successful preceding-source CI 42141615067, job 127649114294, the same test passes in 11.694 seconds and all 41 pass. That baseline head's tree equals the parent 445767cc; the complete cancellation test body is unchanged, and the relevant final PR files match the merged a61a85d files. The inspected capture change explains the differing fixture phase.

Please move this test's confirmed/changed guidance to the selected Maka global-preference file, as the first protocol test in this PR already does, then rerun this cancellation/digest case and the affected native shard. The confirmed finding is the outdated test stimulus and resulting CI failure; cancellation behavior under a real CLI remains unverified here. I have not run this native test locally or changed the author's branch.


Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

}
}
let Some(workspace) = workspace else {
return Ok((global_instruction, String::new()));

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.

P2: Migrate the remaining instruction-ack cancellation fixture to a file this branch still captures.

The test at owner/tests/executor/external.rs:600 still writes workspace/AGENTS.md at lines 622 and 639 to force an instruction change, then waits for before_instructions_ack. With this early return, that project file is omitted from native-CLI capture; the prompt digest stays unchanged and the fixture emits opened instead.

Merged a61a85d CI job127667617505 fails this assertion (40/41 pass), while the same unchanged test passes on the preceding source tree in job127649114294 (41/41 pass). I read both original logs and compared the test body and final/merged source blobs. Update the fixture to change the selected .maka/AGENTS.md preferences, then rerun the cancellation and retry assertions so they exercise an actual instruction update.


Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XXL Over 2500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants