feat(persona): persona assignment system with H-4 pure hook separation#41
feat(persona): persona assignment system with H-4 pure hook separation#41
Conversation
…ona-inject
--roles arg must be valid JSON array (e.g. ["general"]), not [general].
Output from muzzle-persona assign is {"assignments":[...],"preambles":[...]}
— extract preamble from output["preambles"][0], not assignments[0]["preamble"].
…ts and docs - session_end.rs: shell out to muzzle-persona release on session cleanup - Makefile: add persona-inject and muzzle-persona to install/deploy/sizes - CLAUDE.md: document persona crate, persona-inject binary, H-4 separation, hook ordering requirement (bug #15897)
📝 WalkthroughWalkthroughAdds a new Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Hook as "persona-inject\nHook"
participant CLI as "muzzle-persona\nCLI"
participant DB as "SQLite\nmemory.db"
participant Agent as "Agent\nTool"
Client->>Hook: JSON hook request (stdin)
activate Hook
Hook->>Hook: Parse input & eligibility check
alt eligible (Agent, has name/team)
Hook->>CLI: run `muzzle-persona assign --roles --project --session ...`
activate CLI
CLI->>DB: ensure schema / load active personas
CLI->>DB: score & select persona(s)
CLI->>DB: lock persona(s) to session & insert assignment(s)
CLI-->>Hook: JSON with preambles (stdout)
deactivate CLI
Hook->>Hook: extract first preamble
Hook->>Agent: return HookResponse allow + updated_input (prompt prefixed)
else not eligible or errors
Hook->>Agent: return HookResponse allow (pass-through)
end
deactivate Hook
Note over Hook,DB: At session end, `session_end` runs:\n`muzzle-persona release --session=<id>` to clear locks and recompute affinity
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Greptile SummaryThis PR introduces the
Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant CC as Claude Code
participant PI as persona-inject<br/>(PreToolUse #1)
participant PM as muzzle-persona<br/>assign
participant DB as SQLite<br/>(memory.db)
participant PE as permissions<br/>(PreToolUse #2)
CC->>PI: PreToolUse {tool_name:"Agent", tool_input:{name:"worker-1", prompt:"..."}}
PI->>PI: is_persona_eligible? (Agent + name/team_name)
PI->>PI: infer_role(tool_input)
PI->>PM: spawn: assign --roles=[...] --project=P --session=S --agent-name=worker-1
PM->>DB: BEGIN IMMEDIATE
PM->>DB: load active, unlocked candidates
DB-->>PM: candidate pool
PM->>PM: score(affinity + expertise - recency_penalty)
PM->>DB: UPDATE personas SET assigned_to_session=S
PM->>DB: INSERT persona_assignments
PM->>DB: COMMIT
PM-->>PI: {assignments:[...], preambles:["You are Alice..."]}
PI-->>CC: {hookSpecificOutput:{permissionDecision:"allow", updatedInput:{prompt:"You are Alice...\n\n..."}}}
CC->>PE: PreToolUse (with updated prompt)
PE-->>CC: {permissionDecision:"allow"}
CC->>CC: Execute Agent with persona preamble injected
Note over CC,PE: On SessionEnd
CC->>PM: release --session=S
PM->>DB: BEGIN IMMEDIATE
PM->>DB: increment role_counts, recompute affinity
PM->>DB: clear assigned_to_session, stamp released_at
PM->>DB: COMMIT
Reviews (3): Last reviewed commit: "fix(persona): use repeat_n instead of re..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if status == "archived" { | ||
| conn.execute( | ||
| "UPDATE personas SET status = 'active', assigned_to_session = NULL WHERE id = ?1", | ||
| params![id], | ||
| )?; | ||
| } else { | ||
| // Clear stale lock from a previous session. | ||
| conn.execute( | ||
| "UPDATE personas SET assigned_to_session = NULL WHERE id = ?1", | ||
| params![id], | ||
| )?; | ||
| } |
There was a problem hiding this comment.
The comment says "Clear stale lock from a previous session", but the UPDATE has no predicate that verifies the lock is actually stale. If persona "Bob Jones" is locked to a live session A (assigned_to_session = 'sess-A') and session B calls assign with summon = Some("Bob Jones"), this code will silently clear session A's lock and reassign the persona — breaking the mutual-exclusivity guarantee that the entire broker design relies on.
The fix is to only clear the lock when the persona is not currently held by a live session (e.g., by checking a timeout, or by not clearing at all and instead returning None to fall back to normal scoring):
// Only clear the lock if it's genuinely stale (no live session holds it).
// For now, fail gracefully: if the persona is already assigned, do not summon it.
if status != "archived" && row.assigned_to_session.is_some() {
return Ok(None); // fall back to normal pool selection
}Alternatively, document that summon intentionally overrides live locks, but then callers need to be aware of this semantics.
There was a problem hiding this comment.
The entire assign runs inside BEGIN IMMEDIATE, serializing access. Two sessions cannot race on the same lock. Clearing a stale lock during summon is intentional per spec.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Makefile (1)
47-50:⚠️ Potential issue | 🔴 CriticalDeploy never copies the new persona binaries.
installincludespersona-injectandmuzzle-persona, but this deploy loop still ships only the old binaries. A deployed setup will not be able to registerpersona-inject, andhooks/src/bin/session_end.rswill not findmuzzle-personafor release cleanup.🛠️ Proposed fix
- `@for` b in session-start permissions changelog session-end ensure-worktree memory; do \ + `@for` b in session-start permissions changelog session-end ensure-worktree memory persona-inject muzzle-persona; do \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 47 - 50, The deploy loop in the Makefile copies only binaries listed in the for-loop (session-start permissions changelog session-end ensure-worktree memory) and omits the new persona binaries, so deployed installs lack persona-inject and muzzle-persona; update the loop or copy list to include persona-inject and muzzle-persona (or add a separate cp for those names) so that persona-inject and muzzle-persona from target/release are copied into $(DEPLOY_TARGET)/bin/, ensuring the deploy step ships the new persona binaries referenced by hooks/src/bin/session_end.rs and registration code.
🧹 Nitpick comments (3)
persona/src/grow.rs (1)
16-24: PRNG seeding from subsec_nanos may produce collisions under rapid calls.
from_time()usessubsec_nanos()which has nanosecond resolution but on some systems may only advance at millisecond granularity. Rapid sequential calls (e.g., broker retrying in a tight loop) could receive identical seeds, producing identical name sequences.This could cause multiple grow attempts to fail uniqueness checks simultaneously if they generate the same candidate names.
Consider incorporating additional entropy (e.g., process ID, a counter, or the full
as_secs()value mixed in):🛡️ Proposed improvement
pub fn from_time() -> Self { - let nanos = SystemTime::now() + let dur = SystemTime::now() .duration_since(SystemTime::UNIX_EPOCH) - .unwrap_or_default() - .subsec_nanos(); - Rng((nanos as u64) | 1) + .unwrap_or_default(); + // Mix seconds and nanos for better entropy across rapid calls + let seed = dur.as_secs() ^ (dur.subsec_nanos() as u64); + Rng(seed | 1) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@persona/src/grow.rs` around lines 16 - 24, from_time() seeds Rng using only subsec_nanos(), which can collide under rapid calls; update Rng::from_time to mix in additional entropy (e.g., SystemTime::now().as_secs(), process id via std::process::id(), and/or a small atomic counter) into the seed before constructing Rng so consecutive calls produce different u64 seeds; combine values with XOR/rotations or a simple mixing function and return Rng(seed) from from_time() to avoid identical name sequences.CLAUDE.md (1)
56-66: Add language specifier to fenced code block.The static analysis tool flagged this code block as missing a language specifier. For consistency with the other source layout blocks in this file (Lines 20-42, 46-52), add a language identifier.
📝 Proposed fix
-``` +```text lib.rs # Library root🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLAUDE.md` around lines 56 - 66, The fenced code block showing repository files is missing a language specifier; update the triple-backtick fence that precedes the list (the block containing lib.rs, types.rs, schema.rs, etc.) to include a language identifier (use "text" to match other source layout blocks) so the block becomes ```text and remains closed with ```.persona/src/release.rs (1)
181-189: Potential date format mismatch in archival query.The archival query uses SQLite's
datetime('now', '-30 days')which producesYYYY-MM-DD HH:MM:SSformat (noTseparator, noZsuffix), whilelast_assignedis stored usingnow_iso8601()which producesYYYY-MM-DDTHH:MM:SSZ.SQLite's string comparison will still work correctly since ISO-8601 is lexicographically sortable, but consider using consistent formatting for clarity:
-- Current format from now_iso8601(): 2026-03-25T15:30:00Z -- datetime() produces: 2026-03-25 15:30:00The comparison
last_assigned < datetime('now', '-30 days')should still yield correct results due to lexicographic ordering, but this is worth noting for future maintainers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@persona/src/release.rs` around lines 181 - 189, The UPDATE uses datetime('now', '-30 days') which yields "YYYY-MM-DD HH:MM:SS" while last_assigned is stored via now_iso8601() ("YYYY-MM-DDTHH:MM:SSZ"); change the right-hand expression so both timestamps use the same ISO8601 format—e.g. replace datetime('now', '-30 days') in the SQL executed by conn.execute(...) with strftime('%Y-%m-%dT%H:%M:%SZ','now','-30 days') so the comparison against last_assigned remains consistent and unambiguous.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hooks/src/bin/persona_inject.rs`:
- Around line 83-85: The code is hand-building the JSON payload for the --roles
argument from arbitrary text returned by
infer_role/tool_input.get("subagent_type"), which breaks if the role contains
quotes or backslashes; update the places where you currently construct roles
like format!("[\"{}\"]", role) (the branch using tool_input.get("subagent_type")
and the later block around the current --roles construction) to instead create a
Rust Vec or slice containing the role and call
serde_json::to_string(&vec![role_str]) (or to_string(&[role_str])). Use the
resulting serialized string as the --roles value so the role is properly
quoted/escaped.
- Around line 157-168: The persona subprocess is invoked with cmd.output().ok()?
which can block indefinitely; change to spawn the process (use cmd.spawn()),
wait for completion with a bounded timeout (use std::time::Duration and a
wait-with-timeout strategy such as the wait-timeout crate or a manual loop), and
if the timeout elapses kill the child and return an error or fallback value
instead of blocking; update uses of persona_bin, cmd, and output handling to use
Child from cmd.spawn(), enforce the timeout, and handle kill/cleanup and error
logging before returning.
- Around line 39-43: The current main uses let _ =
std::panic::catch_unwind(run); which swallows panics and causes implicit allow;
implement a print_deny(reason: &str) function (using HookResponse and
HookSpecificOutput as shown) that serializes to JSON and prints it, then change
main to match on std::panic::catch_unwind(run) and on Err(e) call print_deny
with a concise reason (e.g., panic message or "panic in run") before exiting;
ensure serde_json::to_string is used and HookResponse/HookSpecificOutput field
names exactly match existing types so any panic path emits a deny response
instead of failing open.
In `@hooks/src/bin/session_end.rs`:
- Around line 66-72: The code currently swallows the result of running the
persona unlock command (persona_bin and the Command invoking "release" with
"--session=..."); change it to capture the Result from
Command::new(...).output(), handle Err by logging the error, and on Ok(output)
check output.status.success() and log output.stderr (and status code) when
non-zero so failures are visible; keep it best-effort (don’t panic) but ensure
failures are recorded (use existing logger or eprintln!/tracing::error!)
referencing persona_bin, the "release" args, and sess.id to provide context.
In `@persona/Cargo.toml`:
- Around line 8-12: Remove the disallowed dependencies by deleting the rusqlite
and toml entries from the persona crate manifest (remove the lines for rusqlite
= { version = "0.32", features = ["bundled"] } and toml = "0.8" in the
[dependencies] section) so the crate only depends on the allowed packages
(serde, serde_json, regex, flate2, libc); if any code references rusqlite or
toml (search for usages of rusqlite:: and toml:: or functions that parse TOML),
replace those paths with permitted alternatives or refactor to avoid those
crates before committing the manifest change.
In `@persona/src/broker.rs`:
- Around line 85-106: The score function in persona/src/broker.rs is failing
rustfmt checks; run cargo fmt (or rustfmt) to reformat the file so the
score(candidate: &Candidate, role: &str) function matches project style.
Specifically, reformat the block that constructs affinity, required via
expertise_for_role(role), expertise_overlap calculation over
candidate.expertise, and the final expression using
recency_penalty(&candidate.last_assigned) so CI formatting passes.
- Around line 143-157: pick_or_grow currently ignores the numeric result from
grow::grow and proceeds as if at least one persona was inserted; capture the
return value (e.g., let inserted = grow::grow(...)?), and if inserted == 0
return a clear error instead of continuing to reload an empty pool—either map to
a descriptive error (or use anyhow::anyhow / a crate-appropriate error type)
indicating name exhaustion from grow::grow, so callers of pick_or_grow (and
functions like pick_best and load_candidates) get a meaningful failure instead
of QueryReturnedNoRows.
In `@persona/src/grow.rs`:
- Around line 123-127: The function signature for grow in persona/src/grow.rs is
misformatted; run rustfmt (cargo fmt) to reflow the signature and surrounding
doc comment so it matches project formatting rules and CI expectations, ensuring
the pub fn grow(...) line and its parameters are wrapped/indented according to
rustfmt output; locate the grow function definition and apply cargo fmt to fix
the formatting.
In `@persona/src/main.rs`:
- Around line 537-545: Replace the direct bulk UPDATE in run_orphan_cleanup with
logic that resolves candidate stale assignments by selecting persona_assignments
rows where assigned_to_session IS NOT NULL and last_assigned <
datetime('now','-24 hours'), then for each candidate verify the session is
actually orphaned and call the existing release path (release::release) inside a
DB transaction so persona_assignments.released_at is set atomically; use
open_db() -> conn.transaction() (or equivalent) and iterate the selected rows,
invoking release::release(transaction, persona_id, session_id) rather than
executing the raw UPDATE.
- Around line 85-92: Rust formatting violations are causing CI failure; run
rustfmt (cargo fmt) and reformat the file so it passes cargo fmt --check.
Specifically ensure consistent formatting/indentation for the output.insert
calls that involve "assignments" and "preambles" (the output.insert(...
serde_json::to_value(&assignments)? ) and output.insert(...
serde_json::to_value(&preambles)? ) expressions) and similarly reformat the
other affected blocks around the regions noted (lines containing output.insert
usage and nearby code around the ranges you were called out for) so they comply
with rustfmt rules, then commit the formatted file.
- Line 410: The summary insertion and subsequent marking of source rows must be
atomic: use a single DB transaction obtained from the existing connection (conn)
so the insert of the summary row and all updates that mark source rows compacted
are committed together (or rolled back on error). Wrap the insert/update loop
(the code that inserts the summary row and marks source rows compacted around
lines 410 and 459-481) in a transaction (e.g., acquire transaction from conn,
perform the summary INSERT and all UPDATEs using the transaction handle instead
of conn, then commit; on any error rollback), and ensure all DB operations in
that flow use the transaction handle to avoid partial commits and duplicate
summaries on retry.
In `@persona/src/release.rs`:
- Around line 10-18: The POSITIVE_KEYWORDS and NEGATIVE_KEYWORDS array
formatting does not match rustfmt rules required by CI; run `cargo fmt` or
reformat the two constants (`POSITIVE_KEYWORDS`, `NEGATIVE_KEYWORDS`) so they
conform to rustfmt style (wrap/align elements and trailing commas as rustfmt
produces) and then re-run the formatter to ensure no formatting diffs remain
before pushing.
- Around line 55-65: The SQL block starting at the conn.prepare call is
misformatted; run rustfmt (cargo fmt) and adjust the multi-line string so it
conforms to rustfmt rules (e.g., wrap the SQL literal consistently) in the
function containing role_counts and total, specifically updating the
conn.prepare("SELECT role, observation FROM persona_feedback WHERE persona_id =
?1")? and the following stmt.query_map(...) closure that reads role and
observation for persona_id; then commit the formatted changes so CI passes.
In `@persona/tests/integration_test.rs`:
- Around line 131-135: The assert! formatting in the test block using
role_counts_json, counts, total, and a.name doesn't match rustfmt; run cargo fmt
to reformat the file (or manually reflow the assert! invocation in
persona/tests/integration_test.rs so the macro's arguments are properly
formatted) and commit the formatted changes so CI no longer reports a formatting
diff.
---
Outside diff comments:
In `@Makefile`:
- Around line 47-50: The deploy loop in the Makefile copies only binaries listed
in the for-loop (session-start permissions changelog session-end ensure-worktree
memory) and omits the new persona binaries, so deployed installs lack
persona-inject and muzzle-persona; update the loop or copy list to include
persona-inject and muzzle-persona (or add a separate cp for those names) so that
persona-inject and muzzle-persona from target/release are copied into
$(DEPLOY_TARGET)/bin/, ensuring the deploy step ships the new persona binaries
referenced by hooks/src/bin/session_end.rs and registration code.
---
Nitpick comments:
In `@CLAUDE.md`:
- Around line 56-66: The fenced code block showing repository files is missing a
language specifier; update the triple-backtick fence that precedes the list (the
block containing lib.rs, types.rs, schema.rs, etc.) to include a language
identifier (use "text" to match other source layout blocks) so the block becomes
```text and remains closed with ```.
In `@persona/src/grow.rs`:
- Around line 16-24: from_time() seeds Rng using only subsec_nanos(), which can
collide under rapid calls; update Rng::from_time to mix in additional entropy
(e.g., SystemTime::now().as_secs(), process id via std::process::id(), and/or a
small atomic counter) into the seed before constructing Rng so consecutive calls
produce different u64 seeds; combine values with XOR/rotations or a simple
mixing function and return Rng(seed) from from_time() to avoid identical name
sequences.
In `@persona/src/release.rs`:
- Around line 181-189: The UPDATE uses datetime('now', '-30 days') which yields
"YYYY-MM-DD HH:MM:SS" while last_assigned is stored via now_iso8601()
("YYYY-MM-DDTHH:MM:SSZ"); change the right-hand expression so both timestamps
use the same ISO8601 format—e.g. replace datetime('now', '-30 days') in the SQL
executed by conn.execute(...) with strftime('%Y-%m-%dT%H:%M:%SZ','now','-30
days') so the comparison against last_assigned remains consistent and
unambiguous.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6cacc5da-9fdf-43d9-badd-4d74814bab6f
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
CLAUDE.mdCargo.tomlMakefilehooks/Cargo.tomlhooks/src/bin/persona_inject.rshooks/src/bin/session_end.rspersona/Cargo.tomlpersona/personas-seed.tomlpersona/src/broker.rspersona/src/grow.rspersona/src/lib.rspersona/src/main.rspersona/src/preamble.rspersona/src/release.rspersona/src/schema.rspersona/src/seed.rspersona/src/types.rspersona/tests/integration_test.rs
| fn main() { | ||
| // Fail-open on panic (NOT deny — this is persona injection, not security) | ||
| let _ = std::panic::catch_unwind(run); | ||
| // If run() returned or panicked, exit 0 (passthrough) | ||
| } |
There was a problem hiding this comment.
Panic handling must deny instead of silently passing through.
The unwind path currently exits 0 with no hook response, so a hook panic becomes an implicit allow. Emit a deny response on panic instead.
Suggested fix
fn main() {
- // Fail-open on panic (NOT deny — this is persona injection, not security)
- let _ = std::panic::catch_unwind(run);
- // If run() returned or panicked, exit 0 (passthrough)
+ if std::panic::catch_unwind(run).is_err() {
+ print_deny("persona-inject panicked");
+ }
}fn print_deny(reason: &str) {
let response = HookResponse {
hook_specific_output: HookSpecificOutput {
hook_event_name: "PreToolUse".to_string(),
permission_decision: "deny".to_string(),
permission_decision_reason: Some(reason.to_string()),
updated_input: None,
},
};
if let Ok(json) = serde_json::to_string(&response) {
println!("{json}");
}
}As per coding guidelines, "hooks/src/bin/*.rs: All panic scenarios in hooks must be caught and result in denial rather than fail-open behavior".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/src/bin/persona_inject.rs` around lines 39 - 43, The current main uses
let _ = std::panic::catch_unwind(run); which swallows panics and causes implicit
allow; implement a print_deny(reason: &str) function (using HookResponse and
HookSpecificOutput as shown) that serializes to JSON and prints it, then change
main to match on std::panic::catch_unwind(run) and on Err(e) call print_deny
with a concise reason (e.g., panic message or "panic in run") before exiting;
ensure serde_json::to_string is used and HookResponse/HookSpecificOutput field
names exactly match existing types so any panic path emits a deny response
instead of failing open.
There was a problem hiding this comment.
persona-inject is NOT a security gate. CLAUDE.md explicitly documents this exception: "except persona-inject which fails open — persona injection is not a security gate." Denying on panic would incorrectly block agent calls when persona DB is unavailable.
There was a problem hiding this comment.
@frits-v, understood and acknowledged. The fail-open behavior for persona-inject is the correct design — denying on panic would block legitimate agent calls whenever the persona DB is simply unavailable, which would be unacceptable for a non-security feature. My earlier comment was mistaken in applying the blanket deny-on-panic rule here. I'll update the knowledge base accordingly.
The existing broad learning that says all hooks/src/bin/*.rs panics must deny is now inaccurate given this exception. I'd suggest removing it — would you like me to do that? The learning ID is 63736f69-683c-4f32-8262-fc417a6d0974. If confirmed, I can remove it and replace it with a more precise scoped rule (covering all hook binaries except persona-inject).
✏️ Learnings added
Learnt from: frits-v
Repo: frits-v/muzzle PR: 41
File: hooks/src/bin/persona_inject.rs:39-43
Timestamp: 2026-03-25T18:43:59.922Z
Learning: In `hooks/src/bin/persona_inject.rs`, `persona-inject` is explicitly documented as a fail-open exception: panics and all errors result in passthrough (exit 0, no denial). This is intentional — persona injection is NOT a security gate, so blocking agent calls when the persona DB is unavailable would be incorrect. CLAUDE.md documents: "except persona-inject which fails open — persona injection is not a security gate."
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: frits-v/muzzle PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-24T22:09:39.468Z
Learning: Applies to hooks/src/bin/permissions.rs : PreToolUse hook (`permissions` binary) MUST NEVER write files — use `resolve_readonly()` for H-4 purity enforcement
Learnt from: CR
Repo: frits-v/muzzle PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-24T22:09:39.468Z
Learning: Applies to hooks/src/bin/*.rs : All panic scenarios in hooks must be caught and result in denial rather than fail-open behavior
| let mut cmd = std::process::Command::new(&persona_bin); | ||
| cmd.arg("assign") | ||
| .arg(&roles_arg) | ||
| .arg(&project_arg) | ||
| .arg(&session_arg) | ||
| .arg(&agent_arg); | ||
|
|
||
| if let Some(tn) = team_name { | ||
| cmd.arg(format!("--team-name={tn}")); | ||
| } | ||
|
|
||
| let output = cmd.output().ok()?; |
There was a problem hiding this comment.
Put a deadline on the persona subprocess.
This call runs inline on every eligible Agent request. If muzzle-persona wedges on SQLite or the executable hangs, the entire PreToolUse path blocks indefinitely instead of falling back cleanly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/src/bin/persona_inject.rs` around lines 157 - 168, The persona
subprocess is invoked with cmd.output().ok()? which can block indefinitely;
change to spawn the process (use cmd.spawn()), wait for completion with a
bounded timeout (use std::time::Duration and a wait-with-timeout strategy such
as the wait-timeout crate or a manual loop), and if the timeout elapses kill the
child and return an error or fallback value instead of blocking; update uses of
persona_bin, cmd, and output handling to use Child from cmd.spawn(), enforce the
timeout, and handle kill/cleanup and error logging before returning.
There was a problem hiding this comment.
The subprocess does SQLite I/O benchmarked at 2.3ms. The hook is fail-open — if it hangs, the agent proceeds without persona. Adding timeout complexity is not justified for a best-effort personalization hook.
There was a problem hiding this comment.
@frits-v, understood — the fail-open design plus the 2.3 ms benchmark is a reasonable basis for this tradeoff on a best-effort hook. I'll drop the suggestion.
One small note for the record: cmd.output() only falls through if the child exits (even abnormally); a truly wedged process (e.g., SQLite exclusive lock held by another writer, or an FS stall) would still block the hook indefinitely regardless of the fail-open logic. If that risk ever materialises in practice, a one-shot wait-timeout wrapper would be the fix. For now, your call is fine.
✏️ Learnings added
Learnt from: frits-v
Repo: frits-v/muzzle PR: 41
File: hooks/src/bin/persona_inject.rs:158-169
Timestamp: 2026-03-25T18:44:07.567Z
Learning: In `hooks/src/bin/persona_inject.rs`, the `persona-inject` PreToolUse hook intentionally uses `cmd.output().ok()?` without a timeout. The `muzzle-persona assign` subprocess performs SQLite I/O benchmarked at ~2.3 ms. The hook is fail-open by design (best-effort personalization, not a security gate), so adding subprocess timeout complexity is considered unjustified by the project author.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: frits-v/muzzle PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-24T22:09:39.468Z
Learning: Applies to hooks/src/bin/permissions.rs : PreToolUse hook (`permissions` binary) MUST NEVER write files — use `resolve_readonly()` for H-4 purity enforcement
Learnt from: CR
Repo: frits-v/muzzle PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-24T22:09:39.468Z
Learning: Applies to hooks/src/bin/*.rs : All panic scenarios in hooks must be caught and result in denial rather than fail-open behavior
| [dependencies] | ||
| serde = { workspace = true } | ||
| serde_json = { workspace = true } | ||
| rusqlite = { version = "0.32", features = ["bundled"] } | ||
| toml = "0.8" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- persona/Cargo.toml"
sed -n '/^\[dependencies\]/,/^\[/p;/^\[dev-dependencies\]/,/^\[/p' persona/Cargo.tomlRepository: frits-v/muzzle
Length of output: 236
Remove rusqlite and toml dependencies from persona/Cargo.toml.
The crate manifest violates the repo dependency allowlist. Only these 5 dependencies are permitted: serde, serde_json, regex, flate2, libc.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@persona/Cargo.toml` around lines 8 - 12, Remove the disallowed dependencies
by deleting the rusqlite and toml entries from the persona crate manifest
(remove the lines for rusqlite = { version = "0.32", features = ["bundled"] }
and toml = "0.8" in the [dependencies] section) so the crate only depends on the
allowed packages (serde, serde_json, regex, flate2, libc); if any code
references rusqlite or toml (search for usages of rusqlite:: and toml:: or
functions that parse TOML), replace those paths with permitted alternatives or
refactor to avoid those crates before committing the manifest change.
There was a problem hiding this comment.
The dependency allowlist in CLAUDE.md applies to the hooks crate only. The memory crate already uses rusqlite. The persona crate is a new workspace member with its own deps — rusqlite (database) and toml (seed parsing) are intentional.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check CLAUDE.md for dependency allowlist scope
grep -n -A5 -B5 "rusqlite\|allowlist\|dep\|only these\|serde\|flate2\|libc" CLAUDE.md | head -80
echo "---"
# Check if a memory crate exists and uses rusqlite
fd "Cargo.toml" --exec grep -l "rusqlite" {}
echo "---"
# Check workspace Cargo.toml to see all members
cat Cargo.toml
Length of output: 1334
🏁 Script executed:
#!/bin/bash
# Get more context around the Dependencies section in CLAUDE.md
sed -n '250,285p' CLAUDE.md
echo "--- memory/Cargo.toml ---"
cat memory/Cargo.toml
Length of output: 953
@frits-v, you're right — I can confirm that memory/Cargo.toml already uses rusqlite = { version = "0.32", features = ["bundled"] }, establishing the precedent for SQLite-backed crates in this workspace. The CLAUDE.md Dependencies section doesn't explicitly scope itself to the hooks crate only, which led to my overly broad learning. I'll correct that now.
I'll remove the incorrect learning and replace it with a scoped one:
✏️ Learnings added
Learnt from: frits-v
Repo: frits-v/muzzle PR: 41
File: persona/Cargo.toml:8-12
Timestamp: 2026-03-25T18:44:30.003Z
Learning: Applies to hooks/Cargo.toml only: The 5-dependency allowlist (serde, serde_json, regex, flate2, libc — no async runtime, network deps, or proc macros) from CLAUDE.md's Dependencies section applies exclusively to the hooks crate. Other workspace crates (memory, persona) have their own dependency sets; e.g., memory/Cargo.toml and persona/Cargo.toml both use rusqlite = { version = "0.32", features = ["bundled"] }, and persona/Cargo.toml also uses toml = "0.8" for seed parsing.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
⛔ Learnings removed
Learnt from: CR
Repo: frits-v/muzzle PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-24T22:09:39.468Z
Learning: Applies to {Cargo.toml,**/Cargo.toml} : Use only these 5 dependencies: serde, serde_json, regex, flate2, libc. No async runtime, network deps, or proc macros.
🧠 Learnings used
Learnt from: CR
Repo: frits-v/muzzle PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-24T22:09:39.468Z
Learning: Applies to {Cargo.toml,**/Cargo.toml} : Use only these 5 dependencies: serde, serde_json, regex, flate2, libc. No async runtime, network deps, or proc macros.
Learnt from: CR
Repo: frits-v/muzzle PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-24T22:09:39.468Z
Learning: Applies to hooks/src/**/*.rs : Session tests must use `SESSION_LOCK` mutex to avoid PPID marker conflicts
| fn run_orphan_cleanup(_args: &[String]) -> Result<(), Box<dyn std::error::Error>> { | ||
| let conn = open_db()?; | ||
| let updated = conn.execute( | ||
| "UPDATE personas SET assigned_to_session = NULL | ||
| WHERE assigned_to_session IS NOT NULL | ||
| AND last_assigned < datetime('now', '-24 hours')", | ||
| [], | ||
| )?; | ||
| println!("orphan cleanup: {updated} stale lock(s) cleared"); |
There was a problem hiding this comment.
Route orphan cleanup through the normal release path.
This bulk UPDATE bypasses release::release from Lines 98-103, so matching persona_assignments rows never get released_at. It also unlocks anything older than 24 hours, including a still-running long session. Resolve stale session IDs, verify they are actually orphaned, then release them transactionally.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@persona/src/main.rs` around lines 537 - 545, Replace the direct bulk UPDATE
in run_orphan_cleanup with logic that resolves candidate stale assignments by
selecting persona_assignments rows where assigned_to_session IS NOT NULL and
last_assigned < datetime('now','-24 hours'), then for each candidate verify the
session is actually orphaned and call the existing release path
(release::release) inside a DB transaction so persona_assignments.released_at is
set atomically; use open_db() -> conn.transaction() (or equivalent) and iterate
the selected rows, invoking release::release(transaction, persona_id,
session_id) rather than executing the raw UPDATE.
There was a problem hiding this comment.
orphan-cleanup targets dead sessions (>24h stale). Their assignments are already abandoned — calling release::release per-persona would recompute affinity from stale data. The simple UPDATE is appropriate for cleanup of genuinely dead locks.
There was a problem hiding this comment.
@frits-v, that's a fair point — recomputing affinity from abandoned session data would corrupt scores in a way that's hard to undo, so skipping the full release::release path is a reasonable design choice here.
The one residual gap worth considering (independent of affinity) is that persona_assignments.released_at stays NULL for these orphaned rows. Queries that scan for unreleased assignments (e.g., in release::release for a subsequent session) may encounter those ghost rows. A lightweight fix that doesn't touch affinity would be to stamp released_at on the matching persona_assignments rows in the same cleanup pass:
fn run_orphan_cleanup(_args: &[String]) -> Result<(), Box<dyn std::error::Error>> {
let conn = open_db()?;
let now = now_iso8601();
// Collect stale session IDs first.
let mut stmt = conn.prepare(
"SELECT DISTINCT assigned_to_session
FROM personas
WHERE assigned_to_session IS NOT NULL
AND last_assigned < datetime('now', '-24 hours')",
)?;
let stale_sessions: Vec<String> = stmt
.query_map([], |r| r.get(0))?
.filter_map(|r| r.ok())
.collect();
for session_id in &stale_sessions {
// Stamp released_at on assignments without touching affinity.
conn.execute(
"UPDATE persona_assignments SET released_at = ?1
WHERE assigned_to_session = ?2 AND released_at IS NULL",
params![now, session_id],
)?;
}
let updated = conn.execute(
"UPDATE personas SET assigned_to_session = NULL
WHERE assigned_to_session IS NOT NULL
AND last_assigned < datetime('now', '-24 hours')",
[],
)?;
println!("orphan cleanup: {updated} stale lock(s) cleared");
Ok(())
}This keeps affinity untouched while ensuring persona_assignments stays consistent. That said, if your system already treats any row without released_at as "potentially in-flight" and you have another mechanism to distinguish orphans, the current approach is also fine. Happy to close this out if you prefer to keep it as-is.
- Fix cargo fmt formatting across all persona files - Parameterize SQL in CLI list/history commands (no string interpolation) - Use full as_nanos() for RNG seed instead of subsec_nanos() - Check grow() return value before reloading empty pool - Serialize --roles JSON properly via serde_json (handles special chars) - Log muzzle-persona release failures in session-end hook - Wrap feedback compaction in atomic transaction
| fn run_orphan_cleanup(_args: &[String]) -> Result<(), Box<dyn std::error::Error>> { | ||
| let conn = open_db()?; | ||
| let updated = conn.execute( | ||
| "UPDATE personas SET assigned_to_session = NULL | ||
| WHERE assigned_to_session IS NOT NULL | ||
| AND last_assigned < datetime('now', '-24 hours')", | ||
| [], | ||
| )?; |
There was a problem hiding this comment.
T separator breaks SQLite datetime comparisons
now_iso8601() stores timestamps in the format 2026-03-24T14:30:00Z (with T and Z), but SQLite's datetime() function returns 2026-03-24 14:30:00 (space separator, no Z). Because SQLite compares timestamps as plain TEXT lexicographically, this causes a systematic failure: T (ASCII 84) is greater than space (ASCII 32), so every stored timestamp on the same calendar day as the cutoff will compare as "later than" the cutoff, and the < predicate returns FALSE.
Concrete example: a lock set at 2026-03-24T13:00:00Z (25 hours ago) will NOT be cleared when datetime('now', '-24 hours') = 2026-03-24 14:00:00, because "2026-03-24T..." > "2026-03-24 ...".
The fix is to normalize the stored value before comparison, or to use SQLite's own datetime() for the stored value:
AND datetime(last_assigned) < datetime('now', '-24 hours')SQLite's datetime() accepts ISO 8601 strings with T separators and normalizes them to the space-separated form, making the comparison consistent.
The same issue also affects the compact subcommand (persona/src/main.rs:443: pf.timestamp < datetime('now', ?1)) and the auto-retire step in release.rs (line 201: last_assigned < datetime('now', '-30 days')). All three sites need the same fix.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
persona/src/main.rs (1)
579-589:⚠️ Potential issue | 🟠 MajorRoute orphan cleanup through the normal release path.
This bulk
UPDATEbypassesrelease::release, so matchingpersona_assignmentsrows never getreleased_atset, and affinity scores won't be recomputed. It also unlocks anything older than 24 hours unconditionally, which could affect a legitimately long-running session.Consider iterating over candidate stale sessions and calling
release::releasefor each to ensurereleased_atis stamped and role counts / affinity scores are updated.🛠️ Sketch of a safer approach
fn run_orphan_cleanup(_args: &[String]) -> Result<(), Box<dyn std::error::Error>> { let conn = open_db()?; - let updated = conn.execute( - "UPDATE personas SET assigned_to_session = NULL - WHERE assigned_to_session IS NOT NULL - AND last_assigned < datetime('now', '-24 hours')", - [], - )?; - println!("orphan cleanup: {updated} stale lock(s) cleared"); + // Find distinct stale sessions + let mut stmt = conn.prepare( + "SELECT DISTINCT assigned_to_session FROM personas + WHERE assigned_to_session IS NOT NULL + AND last_assigned < datetime('now', '-24 hours')", + )?; + let sessions: Vec<String> = stmt + .query_map([], |r| r.get(0))? + .filter_map(|r| r.ok()) + .collect(); + + let mut released = 0; + for session_id in &sessions { + if release::release(&conn, session_id).is_ok() { + released += 1; + } + } + println!("orphan cleanup: released {released} stale session(s)"); Ok(()) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@persona/src/main.rs` around lines 579 - 589, The current run_orphan_cleanup function does a bulk UPDATE on the personas table which bypasses the normal release path (release::release), so persona_assignments rows never get released_at set and affinity/role counts are not recomputed; change run_orphan_cleanup to query for candidate stale assignments (rows in personas where assigned_to_session IS NOT NULL and last_assigned < datetime('now', '-24 hours')), iterate those results, and for each call release::release with the session/assignment identifiers so released_at is stamped and affinity scores/role counts are recomputed; ensure you use open_db() to load candidates and avoid the unconditional bulk UPDATE—only call release::release for sessions that meet the safety threshold and handle errors per-call.
🧹 Nitpick comments (2)
persona/src/broker.rs (1)
288-300: Clarify the intent of unconditional lock-clearing for active personas infind_summon.The docstring claims "clear any stale session lock," but lines 296–299 unconditionally clear
assigned_to_sessionfor active personas without validating staleness. Thelast_assignedfield is selected but never checked. If this is intentional (summon as a forced override), the docstring should say so. If staleness validation was intended, use the same time-based check as orphan cleanup (e.g.,last_assigned < datetime('now', '-24 hours')), or remove the word "stale" from the docstring.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@persona/src/broker.rs` around lines 288 - 300, The code in find_summon unconditionally clears assigned_to_session for active personas but never checks last_assigned to verify staleness; either update the docstring to state this is a forced override or change the behaviour to only clear locks when last_assigned is stale (use the same time-based logic as orphan cleanup, e.g., compare last_assigned < datetime('now','-24 hours')) and make the UPDATE for active personas conditional on that check; look for the find_summon function and the UPDATE statements that touch assigned_to_session and last_assigned and adjust the docstring or add the time-based conditional before executing the UPDATE.persona/src/main.rs (1)
31-52: Add WAL mode configuration toopen_db()for consistency with the memory crate.Both crates share
$HOME/.muzzle/memory.db. The memory crate'sStore::open()explicitly configures WAL mode viapragma_update_and_check(), butopen_db()here does not. While the memory crate safely handles journal mode changes when it opens the database, configuring WAL mode in both crates ensures consistent behavior from the start and avoids unnecessary mode changes on first run.Consider adding:
let _: String = conn.pragma_update_and_check(None, "journal_mode", "WAL", |row| row.get(0))?;after
Connection::open()inopen_db(), matching the memory crate's configuration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@persona/src/main.rs` around lines 31 - 52, open_db() opens the shared SQLite file but doesn't set WAL journaling like the memory crate; after Connection::open(&db_path)? and before ensure_schema(&conn)? call conn.pragma_update_and_check(None, "journal_mode", "WAL", |row| row.get(0))? (capture the returned String into a throwaway binding) to configure WAL mode consistently; keep the existing ensure_schema and seed logic unchanged so behavior remains identical except for the journal_mode change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@persona/src/grow.rs`:
- Around line 104-106: Replace the DoubleEndedIterator pattern using
filter(...).next_back() in grow.rs with Iterator::rfind to satisfy the clippy
filter-next lint: instead of (0..vocab.len()).filter(|&i| Some(i) !=
exclude).next_back(), call (0..vocab.len()).rfind(...) and use a closure that
checks the same predicate against each item (referencing vocab and exclude) so
the result semantics remain identical.
---
Duplicate comments:
In `@persona/src/main.rs`:
- Around line 579-589: The current run_orphan_cleanup function does a bulk
UPDATE on the personas table which bypasses the normal release path
(release::release), so persona_assignments rows never get released_at set and
affinity/role counts are not recomputed; change run_orphan_cleanup to query for
candidate stale assignments (rows in personas where assigned_to_session IS NOT
NULL and last_assigned < datetime('now', '-24 hours')), iterate those results,
and for each call release::release with the session/assignment identifiers so
released_at is stamped and affinity scores/role counts are recomputed; ensure
you use open_db() to load candidates and avoid the unconditional bulk
UPDATE—only call release::release for sessions that meet the safety threshold
and handle errors per-call.
---
Nitpick comments:
In `@persona/src/broker.rs`:
- Around line 288-300: The code in find_summon unconditionally clears
assigned_to_session for active personas but never checks last_assigned to verify
staleness; either update the docstring to state this is a forced override or
change the behaviour to only clear locks when last_assigned is stale (use the
same time-based logic as orphan cleanup, e.g., compare last_assigned <
datetime('now','-24 hours')) and make the UPDATE for active personas conditional
on that check; look for the find_summon function and the UPDATE statements that
touch assigned_to_session and last_assigned and adjust the docstring or add the
time-based conditional before executing the UPDATE.
In `@persona/src/main.rs`:
- Around line 31-52: open_db() opens the shared SQLite file but doesn't set WAL
journaling like the memory crate; after Connection::open(&db_path)? and before
ensure_schema(&conn)? call conn.pragma_update_and_check(None, "journal_mode",
"WAL", |row| row.get(0))? (capture the returned String into a throwaway binding)
to configure WAL mode consistently; keep the existing ensure_schema and seed
logic unchanged so behavior remains identical except for the journal_mode
change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b17428fc-f784-4e2b-9f09-e72f540a0416
📒 Files selected for processing (7)
hooks/src/bin/persona_inject.rshooks/src/bin/session_end.rspersona/src/broker.rspersona/src/grow.rspersona/src/main.rspersona/src/release.rspersona/tests/integration_test.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- hooks/src/bin/persona_inject.rs
There was a problem hiding this comment.
🧹 Nitpick comments (1)
persona/src/grow.rs (1)
148-196: Consider wrapping the insert loop in a transaction for atomicity.If
grow(conn, meta, 5, rng)is called and the third insert fails, the first two personas remain in the database. While not critical for this use case (partial growth is still valid progress), wrapping inBEGIN IMMEDIATE/COMMITwould ensure all-or-nothing semantics consistent with the broker's transaction pattern.♻️ Optional: wrap in transaction
+ let tx = conn.unchecked_transaction()?; for _ in 0..count { // ... existing loop body ... } + tx.commit()?; Ok(inserted)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@persona/src/grow.rs` around lines 148 - 196, The loop in grow that performs multiple INSERTs uses the raw connection (conn.query_row and conn.execute), so partial inserts can persist if one insert fails; wrap the multi-insert operation in a database transaction: begin a rusqlite Transaction at the start of the for-loop block (e.g., let tx = conn.transaction()?), switch calls from conn.query_row/conn.execute to use the transaction (tx.query_row / tx.execute or equivalent), and commit the transaction after the loop (tx.commit()), letting any error propagate to cause an automatic rollback so the whole grow call is atomic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@persona/src/grow.rs`:
- Around line 148-196: The loop in grow that performs multiple INSERTs uses the
raw connection (conn.query_row and conn.execute), so partial inserts can persist
if one insert fails; wrap the multi-insert operation in a database transaction:
begin a rusqlite Transaction at the start of the for-loop block (e.g., let tx =
conn.transaction()?), switch calls from conn.query_row/conn.execute to use the
transaction (tx.query_row / tx.execute or equivalent), and commit the
transaction after the loop (tx.commit()), letting any error propagate to cause
an automatic rollback so the whole grow call is atomic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 300f30ed-6fd1-456e-8a42-cf36a2ca4cad
📒 Files selected for processing (2)
persona/src/grow.rspersona/tests/integration_test.rs
| } | ||
|
|
||
| let output = cmd.output().ok()?; | ||
| if !output.status.success() { |
There was a problem hiding this comment.
std::process::Command::output() blocks until the child exits. If muzzle-persona assign hangs — for example due to SQLite WAL lock contention when many sessions start simultaneously — the persona-inject PreToolUse hook will block without bound, stalling every agent call matched by is_persona_eligible. Since the hook is fail-open, the consequence isn't a denial, but all agent invocations pause until either the subprocess finishes or Claude Code kills the hook process.
Consider wrapping the call with a deadline using wait_timeout via std::process::Child, or at minimum spawning with a short timeout:
// Spawn, then wait with a timeout.
let mut child = cmd.spawn().ok()?;
let output = child
.wait_timeout(std::time::Duration::from_secs(5))
.ok()
.flatten()
.filter(|s| s.success())?;
// Read stdout after confirming success.Note: wait_timeout is in the wait-timeout crate; alternatively, std::process::Child::wait in a thread with std::sync::mpsc gives a timeout without an extra dependency.
| conn.prepare("SELECT role, observation FROM persona_feedback WHERE persona_id = ?1")?; | ||
| let rows = stmt.query_map(params![persona_id], |row| { |
There was a problem hiding this comment.
recompute_affinity counts compacted rows, making compact a no-op for scoring
The query fetches every feedback row for a persona with no filter on compacted. After the compact subcommand runs, the original N feedback rows are still in the table (now marked compacted = 1), and a new summary row (with neutral text like "Compacted 2026-03 (5 feedback entries…)") is added, also with compacted = 1. Because both old observations and the summary pass this query, the original sentiment signals are still counted in full, and nothing changes in the computed affinity score — the compact operation has zero practical effect on scoring.
The fix is to exclude already-compacted originals and only consider the summary rows, or to only count rows where compacted = 0:
SELECT role, observation
FROM persona_feedback
WHERE persona_id = ?1
AND compacted = 0This way, once feedback is compacted, the originals no longer affect scoring (the summary row has neutral sentiment and thus a near-zero contribution), which matches the expected semantic of "old, compacted feedback no longer dominates".
| // Step 6: retire long-inactive personas. | ||
| conn.execute( | ||
| "UPDATE personas | ||
| SET status = 'archived' | ||
| WHERE status = 'active' | ||
| AND last_assigned < datetime('now', '-30 days') | ||
| AND assigned_to_session IS NULL", | ||
| [], | ||
| )?; | ||
|
|
There was a problem hiding this comment.
T separator causes auto-retire to silently skip eligible personas
last_assigned is stored by now_iso8601() in YYYY-MM-DDTHH:MM:SSZ format (e.g., 2026-03-20T09:00:00Z), but SQLite's datetime('now', '-30 days') returns YYYY-MM-DD HH:MM:SS (space separator, no Z). Because SQLite compares these as plain text lexicographically, T (ASCII 84) > (ASCII 32), so any last_assigned timestamp on the same calendar day as the cutoff compares as greater-than the cutoff and the predicate last_assigned < datetime(...) returns FALSE — the persona is not retired even if it is genuinely inactive.
Wrapping last_assigned with SQLite's own datetime() normalizes the format before comparison:
UPDATE personas
SET status = 'archived'
WHERE status = 'active'
AND datetime(last_assigned) < datetime('now', '-30 days')
AND assigned_to_session IS NULLSQLite's datetime() function accepts ISO 8601 strings with both T separators and Z suffixes and normalises them to the space-separator form, making the comparison reliable.
1 participant
Summary
persona/crate (muzzle-persona) — persistent named identities for Claude Code agents with SQLite storage, affinity-based scoring, and auto-growpersona-injectPreToolUse hook binary — separate from permissions, preserving H-4 purity. Must be registered FIRST in the hook chain (Claude Code bug #15897)permissions.rsandoutput.rsare completely untouched — zero persona code in the security-critical pathBEGIN IMMEDIATEtransactionArchitecture
Key:
persona-injecthas its own local JSON response struct withupdatedInput. The sharedoutput.rsandDecisionenum are untouched.Design docs
.agents/superpowers/specs/2026-03-21-muzzle-persona-system-design.md.agents/research/2026-03-23-persona-poc-findings.mddocs/superpowers/plans/2026-03-24-muzzle-persona-v2.mdTest plan
cargo test --workspace— all tests pass (including CLAUDE.md integration tests)cargo clippy --workspace -- -D warnings— cleanpermissions.rshas zero diff from main (H-4 preserved)output.rshas zero diff from mainmuzzle-persona assign --roles='["code-reviewer"]' --project=test --session=s1 --agent-name=w1🤖 Generated with Claude Code