Skip to content

fix(cli): omit persona from generated names - #12790

Merged
chrarnoldus merged 3 commits into
mainfrom
button-barberry
Aug 3, 2026
Merged

fix(cli): omit persona from generated names#12790
chrarnoldus merged 3 commits into
mainfrom
button-barberry

Conversation

@chrarnoldus

Copy link
Copy Markdown
Collaborator

Title and branch-name requests currently inherit the Kilo soul through shared LLM request assembly. Those unrelated persona instructions can confuse the small models used for constrained metadata generation.

Exclude the soul for the hidden title and branch-name agents in both standard and OpenAI OAuth request paths, while preserving each generator prompt and request-specific system content. Other agent requests continue to include the Kilo persona.

@chrarnoldus chrarnoldus self-assigned this Aug 3, 2026
Comment thread packages/opencode/src/kilocode/system-prompt.ts
@kilo-code-bot

kilo-code-bot Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Files Reviewed (4 files, incremental since 0cfd301)
  • packages/opencode/src/agent/agent.ts
  • packages/opencode/src/kilocode/branch-name.ts
  • packages/opencode/src/kilocode/system-prompt.ts
  • packages/opencode/test/kilocode/session-llm-request.test.ts

Notes

  • 323f096e11 reverts the shared-identifier indirection from 0cfd301745. The previous SUGGESTION on packages/opencode/src/agent/agent.ts is resolved: agent.ts and kilocode/branch-name.ts are back to their pre-PR content, so the net PR diff against shared upstream files is now limited to session/llm/request.ts.
  • Net behavior at HEAD is unchanged from the earlier reviewed state: shouldIncludePersona() excludes the soul for title and branch-name in both the standard system array and the OpenAI OAuth options.instructions path, while the generator prompt and request-specific system text are preserved.
  • The test matrix reverted to literal ["title", "branch-name"], which now matches the literals in shouldIncludePersona() — the predicate and its tests stay in the same Kilo-owned file, so they can't drift independently.
  • No memory-leak or resource-lifecycle risk: no new subscriptions, handles, timers, or child processes; the reverted imports also remove the agent.tskilocode/system-prompt import chain.
  • Tests were not executed (read-only review); assertions were verified by reading prepare in packages/opencode/src/session/llm/request.ts.
Previous Review Summaries (2 snapshots, latest commit 0cfd301)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 0cfd301)

Status: 1 Issue Found | Recommendation: Optional follow-up, safe to merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
packages/opencode/src/agent/agent.ts 313 The registration site now reads the name from KilocodeSystemPrompt.agents.title, but all other consumers (session/prompt.ts:284, session/llm.ts:274, plugin/openai/codex.ts:657, plugin/github-copilot/copilot.ts:364, kilocode/agent/index.ts:341) still hardcode "title". Editing the constant would register the agent under a new name while lookups keep asking for "title", so the indirection adds shared-file diff without providing the intended rename safety.
Files Reviewed (5 files, incremental since 8be3303)
  • packages/opencode/src/agent/agent.ts - 1 issue
  • packages/opencode/src/kilocode/branch-name.ts - 0 issues
  • packages/opencode/src/kilocode/system-prompt.ts - 0 issues
  • packages/opencode/src/session/llm/request.ts - 0 issues
  • packages/opencode/test/kilocode/session-llm-request.test.ts - 0 issues

Notes

  • The previous review's suggestion about duplicated agent-name literals in kilocode/system-prompt.ts is addressed: names are centralized in KilocodeSystemPrompt.agents and the predicate is now the clearer shouldIncludePersona().
  • Behavior is unchanged for enabled-persona requests; the exclusion still applies to both the standard system array and the OpenAI OAuth options.instructions path.
  • Driving the test cases off Object.values(KilocodeSystemPrompt.agents) is a good call — a future excluded agent automatically gains coverage in both paths.
  • The allowlist deliberately excludes only title and branch-name; compaction and summary still receive the persona. That looks intentional since those outputs feed back into conversation context rather than constrained metadata.
  • No memory-leak or resource-lifecycle risk: no new subscriptions, handles, timers, or child processes. The new agent.ts import chain (kilocode/system-promptkilocode/editor-context, kilocode/memory/marker) introduces no import cycle back into agent/agent.ts.
  • Tests were not executed (read-only review); assertions were verified by reading prepare in session/llm/request.ts.

Fix these issues in Kilo Cloud

Previous review (commit 8be3303)

Status: 1 Issue Found | Recommendation: Optional follow-up, safe to merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
packages/opencode/src/kilocode/system-prompt.ts 17 Agent names are duplicated string literals across the agent definitions, the predicate, and the test; a rename would silently re-enable the persona. Consider shared constants or keying off agent.hidden, and decide explicitly whether summary/compaction should also be excluded.
Files Reviewed (4 files)
  • .changeset/quiet-metadata-generators.md - 0 issues
  • packages/opencode/src/kilocode/system-prompt.ts - 1 issue
  • packages/opencode/src/session/llm/request.ts - 0 issues
  • packages/opencode/test/kilocode/session-llm-request.test.ts - 0 issues

Notes

  • Fork hygiene looks good: the predicate lives in src/kilocode/, and the shared session/llm/request.ts change is two marked lines plus an import.
  • SystemPrompt.soul() is injected in exactly one place, so both the standard and OpenAI OAuth paths are covered with no other leak site.
  • The OAuth rewrite is behavior-preserving when the persona is enabled: [soul, ...system].join("\n") matches the previous soul() + "\n" + system.join("\n").
  • No memory-leak or resource-lifecycle risk introduced; no new subscriptions, handles, or child processes.
  • Tests were not executed (read-only review); correctness of the assertions was verified by reading prepare rather than by running bun test.

Fix these issues in Kilo Cloud


Reviewed by claude-opus-5 · Input: 24 · Output: 3.5K · Cached: 541.7K

Review guidance: REVIEW.md from base branch main

Comment thread packages/opencode/src/agent/agent.ts Outdated
@chrarnoldus
chrarnoldus merged commit 52211a9 into main Aug 3, 2026
31 checks passed
@chrarnoldus
chrarnoldus deleted the button-barberry branch August 3, 2026 11:55
t7tran pushed a commit to t7tran/kilocode that referenced this pull request Aug 14, 2026
fix(cli): omit persona from generated names
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.

2 participants