Skip to content

feat(brain): add memory, hooks, maxTurns; deprecate --initial-prompt#635

Merged
Wirasm merged 3 commits intomainfrom
brain-frontmatter-upgrade
Mar 17, 2026
Merged

feat(brain): add memory, hooks, maxTurns; deprecate --initial-prompt#635
Wirasm merged 3 commits intomainfrom
brain-frontmatter-upgrade

Conversation

@Wirasm
Copy link
Copy Markdown
Owner

@Wirasm Wirasm commented Mar 4, 2026

Summary

  • Add memory: user, maxTurns: 200, and agent-scoped hooks: to kild-brain frontmatter
  • Create bash guard script enforcing "no source code access" at the hook level (PreToolUse)
  • Auto-snapshot fleet state on Stop hook
  • Replace manual memory management (~40 lines) with auto-memory reference
  • Fix router skill to use create-then-inject pattern instead of --initial-prompt
  • Deprecate --initial-prompt in CLI help text + runtime warnings on both create and open
  • Update CLAUDE.md with deprecation notes

Test plan

  • bash -n .claude/hooks/brain-bash-guard.sh passes
  • Guard blocks cat crates/..., cargo test, git diff, git log
  • Guard allows kild list, gh issue list, cat .kild/..., cat ~/.kild/...
  • cargo fmt --check passes
  • cargo clippy --all -- -D warnings passes
  • cargo test --all passes (424 tests)
  • cargo build --all succeeds
  • Manual: start brain session, verify hooks fire, test bash guard enforcement

Wirasm added 2 commits March 4, 2026 22:11
…te --initial-prompt

- Add `memory: user`, `maxTurns: 200`, and agent-scoped `hooks:` to
  kild-brain frontmatter (PreToolUse bash guard + Stop fleet snapshot)
- Create `.claude/hooks/brain-bash-guard.sh` to enforce "no source code
  access" constraint at the hook level
- Replace ~40 lines of manual memory management with auto-memory note
- Fix router skill to use create-then-inject instead of --initial-prompt
- Deprecate --initial-prompt in CLI help text with runtime warnings
- Update CLAUDE.md with deprecation notes and inject-based brain setup
@Wirasm
Copy link
Copy Markdown
Owner Author

Wirasm commented Mar 17, 2026

PR Review Summary

Reviewed by 4 specialized agents: code-reviewer, docs-impact, silent-failure-hunter, code-simplifier.

Critical Issues (3 found)

Agent Issue Location
code-reviewer Fleet sessions get two contradictory "Use instead" hints — general deprecation prints create && sleep 5 && inject, then fleet block prints just inject. Non-fleet sessions are fine; fleet sessions get conflicting guidance. Guard the general block with !(fleet_mode_active && is_claude_fleet_agent) create.rs:140-176, open.rs:72-94
silent-failure-hunter Guard JSON parsing bypassed by escaped quotes[^"]* regex stops at first \", so cat "crates/foo.rs" produces truncated CMD, guard exits 0 (allow). Use jq -r '.command // empty' instead brain-bash-guard.sh:21
silent-failure-hunter Guard bypass via subshellbash -c 'cat crates/...' is not caught by any blocklist pattern. Blocklist approach is fundamentally shallow; consider allowlist or document as advisory brain-bash-guard.sh:28-50

Important Issues (5 found)

Agent Issue Location
silent-failure-hunter Stop hook 2>/dev/null || true silently swallows all errors — brain bootstraps from stale state.json with no warning. Log stderr to a file instead of /dev/null kild-brain.md Stop hook
code-reviewer set -euo pipefail + grep -o interaction — if grep finds no match (exit 1), set -e kills the script with exit 1 (not 0, not 2). Add `
silent-failure-hunter src/ blocklist pattern too broad — blocks any command string containing src/, including kild inject branch "check src/auth.rs". Either anchor the pattern or remove it (project source is under crates/) brain-bash-guard.sh:28
code-reviewer, simplifier open.rs deprecation warning uses bare eprintln! while create.rs uses color::warning()/color::hint(). Inconsistent visual output for the same message open.rs:72-82 vs create.rs:140-157
silent-failure-hunter Inbox fallback errors not structurally loggedErr(e) branch prints to stderr but no error!() event for cli.{create,open}.inbox_fallback_failed create.rs:165-174, open.rs:84-93

Suggestions (3 found)

Agent Suggestion Location
simplifier Remove initial_prompt_for_warning variable — clone at .with_initial_prompt(initial_prompt.clone()) use site instead of pre-emptive clone with renamed binding create.rs:89-90
simplifier Use [[ == *pattern* ]] instead of echo | grep -q for literal substring checks — fewer subprocess forks, more idiomatic bash brain-bash-guard.sh:28-50
silent-failure-hunter sleep 5 in SKILL.md is a race condition — if agent init takes longer, inject is silently lost. Add a kild list check after sleep to confirm session is active before injecting SKILL.md:44-55

Documentation Issues

  • CLAUDE.md: All --initial-prompt references are already updated in this PR. No stale references remain after merge.
  • New features (memory, hooks, maxTurns, bash guard): Agent-internal — no CLAUDE.md additions needed.

Strengths

  • Deprecation uses exit 2 correctly for hook blocks (not exit 1)
  • create.rs error paths properly log with error!() + events::log_app_error()
  • open.rs batch handle_open_all accumulates partial failures correctly
  • Create-then-inject pattern is the right architectural direction

Verdict: NEEDS FIXES

Priority order:

  1. Fix double-warning for fleet sessions (contradictory user guidance)
  2. Fix guard JSON parsing (jq or fail-closed on parse error)
  3. Fix set -e + grep interaction (script can exit 1 unexpectedly)
  4. Fix Stop hook error suppression (log to file, not /dev/null)
  5. Fix open.rs color inconsistency (use color::warning/color::hint)
  6. Consider guard bypass vectors and document limitations or switch to allowlist

- Guard: switch from fragile grep+sed JSON parsing to jq, fail closed
  on parse failure, add ERR trap, block subshell invocations (bash -c,
  sh -c), remove overly broad src/ pattern, document advisory nature
- Deprecation: eliminate contradictory double-warning for fleet sessions
  by branching into fleet-specific vs general paths (not both)
- Deprecation: use color::warning/color::hint consistently in open.rs
  (was bare eprintln, now matches create.rs)
- Deprecation: remove redundant initial_prompt_for_warning clone in
  create.rs, clone at use site instead
- Logging: add structured error!() events for inbox fallback failures
  in both create.rs and open.rs
- Stop hook: log stderr to file instead of /dev/null, ensure dir exists
- SKILL.md: add session-active check after sleep 5 before injecting,
  warn user if session not ready instead of silently losing the message
@Wirasm Wirasm merged commit ad384fe into main Mar 17, 2026
6 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.

1 participant