feat: extract swappable captain-facing persona into persona.md - #29
Conversation
📝 WalkthroughWalkthroughThe change introduces a configurable persona contract. Session startup loads ChangesPersona-driven session behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant SessionStart as fm-session-start.sh
participant PersonaSources as persona.md and config/persona.md
participant DigestStages as context and fleet digests
SessionStart->>PersonaSources: Resolve local override or tracked persona
PersonaSources-->>SessionStart: Return persona contents or diagnostics
SessionStart->>SessionStart: Print PERSONA section
SessionStart->>DigestStages: Print subsequent digests
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.agents/skills/fmx-respond/SKILL.md:
- Around line 107-110: Update the public-facing response guidance in the
firstmate voice section to require all mandatory address rules from the active
persona, including direct captain address at least once per response. Keep
optional address terms discretionary when using replacement personas, and
preserve the existing allowance for natural persona seasoning.
In `@AGENTS.md`:
- Around line 151-156: Align all session-stage numbering with the actual emitted
order: update AGENTS.md lines 151-156, bin/fm-session-start.sh lines 38-51,
lines 411-427, and line 498 so Persona, Context, Fleet-state, Supervision, and
the closing reminder use one consistent sequence; preserve the existing
execution order and labels.
In `@bin/fm-session-start.sh`:
- Around line 151-180: The active persona path must be readable before its
contents are printed. Update print_persona, using the source selected by
resolve_persona_path, to detect an unreadable local override or tracked default
before cat, emit a source-specific repair diagnostic, and return safely without
triggering startup failure. Add regression coverage for unreadable files in both
selected-source cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2c48bdae-2e64-49f9-84ba-65008015f89d
📒 Files selected for processing (8)
.agents/skills/fmx-respond/SKILL.mdAGENTS.mdCONTRIBUTING.mdbin/fm-session-start.shdocs/configuration.mddocs/documentation-audiences.jsonpersona.mdtests/fm-session-start.test.sh
Extract the nautical ship-person voice into a dedicated persona.md that AGENTS.md points to, so the persona can be replaced without touching the always-loaded operational contract. - persona.md: address rule, voice-flavor seasoning menu (and when to drop it), house-vocabulary exemption, and the routine acknowledgment phrase. A local gitignored config/persona.md fully overrides it, mirroring config/crew-harness. - AGENTS.md: preamble now only points to persona.md/config/persona.md; section 9 keeps the functional etiquette (outcomes not mechanics, translation table, escalation triggers, evidence-first reporting) unchanged in strength, referencing persona.md only for voice detail. - bin/fm-session-start.sh: prints the active persona file's full contents unconditionally every session (local override wins, ABSENT when neither file exists), so the voice is always in force with no per-reply trigger to skip. - docs/configuration.md: documents the override precedence and the ABSENT-persona repair signal. - tests/fm-session-start.test.sh: covers default-persona printing, local-override precedence, and the absent-persona repair signal.
… and doc-audience inventory to persona.md
- bin/fm-session-start.sh: an active persona file that exists but is not readable now prints an explicit UNREADABLE repair message instead of silently failing the cat or falling back to the tracked default, since the persona is always-loaded and a broken override must surface. - AGENTS.md: reordered the session-start numbered list so it matches the script's actual runtime order (supervision instructions run before persona and context, not after fleet-state); the prior wording put "Supervision operating instructions" last while its own text said it ran "after the wake queue and before context." - bin/fm-session-start.sh: renumbered the header ordering comment to include supervision instructions as its own step (was missing, leaving persona/context/fleet/closing off by one). - .agents/skills/fmx-respond/SKILL.md: the public-reply voice section now requires the active persona's address term at least once per reply (persona.md's mandatory rule), rather than the softened "when it fits" wording that made a mandatory rule read as optional; seasoning stays explicitly optional. - tests/fm-session-start.test.sh: covers the unreadable-persona-file repair signal.
…oughly; fixed one inaccuracy: ABSENT vs UNREADABLE persona states were conflated in AGENTS.md and docs/configuration.md omitted the UNREADABLE case entirely, so both now describe it as its own distinct repair failure matching bin/fm-session-start.sh's actual behavior.
b6dcd61 to
f7d5524
Compare
Intent
extract firstmate's ship-person persona (captain address, nautical seasoning, house vocab, ack phrase) into a swappable tracked persona.md with a config/persona.md local override, loaded every session via fm-session-start so it stays always-in-force; functional etiquette (outcomes/translation/escalation, AGENTS.md sec 9) kept unchanged in strength; AGENTS.md keeps only a pointer. This round also addresses CodeRabbit review findings on PR #29: an unreadable active persona file must surface as a repair failure not fail silently; the session-start step numbering/order in AGENTS.md and bin/fm-session-start.sh must be internally consistent and match actual runtime order; and the fmx-respond public-reply voice section must require persona.md's mandatory captain-address rule (not phrase it as optional) while keeping seasoning optional.
What Changed
AGENTS.mdinto a new trackedpersona.md, with an optional localconfig/persona.mdoverride;bin/fm-session-start.shnow loads and prints the active persona every session, ahead of the context digest, andAGENTS.mdkeeps only a pointer while its functional captain-etiquette rules (outcomes/translation/escalation) stay unchanged.AGENTS.md/docs/configuration.mdto match the script's actual runtime sequence, and added 4 new tests plus updateddocs/documentation-audiences.jsonfor the new file.CONTRIBUTING.mdand thefmx-respondskill's public-reply voice section to referencepersona.md, making the mandatory captain-address rule required (not optional) while leaving nautical seasoning optional.Risk Assessment
✅ Low: Purely a documentation/config refactor (persona extraction into persona.md + local override) plus targeted CodeRabbit fixups (unreadable-persona repair signal, doc numbering consistency, mandatory-address wording); the new bash logic (resolve_persona_path/print_persona) is small, correctly ordered relative to lock state, has no fallback-on-unreadable bug, and is covered by new tests matching the described behavior.
Testing
Ran the full fm-session-start test suite (34/34 pass, 4 new persona-specific cases) and additionally executed the real bin/fm-session-start.sh end-to-end against this repo, capturing a transcript showing the tracked persona.md printed in full, correctly ordered between the supervision block and the context digest exactly as AGENTS.md's updated step numbering (now internally consistent and matching runtime order) describes; no failures or missing evidence found.
Evidence: Real fm-session-start.sh run against this repo's tracked persona.md, showing the PERSONA section positioned after supervision instructions and before the context digest
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
bash tests/fm-session-start.test.sh(34/34 passed, including the 4 new persona tests: tracked default printed ahead of CONTEXT, local config/persona.md override fully replaces rather than merges, fully-absent persona reported as needing repair, unreadable active persona file reported as needing repair)Manual end-to-end run ofbin/fm-session-start.shagainst this repo's real root and a fresh throwaway FM_HOME, capturing the full digest transcriptgrep -n "^# --- [0-9]" bin/fm-session-start.shcompared against AGENTS.md section 3's numbered list to confirm the step numbering (1 lock, 2 bootstrap, 3 wake-drain, 4 supervision, 5 persona, 6 context, 7 fleet, 8 closing) matches both the doc and the script's actual execution orderReviewed the fmx-respond SKILL.md diff confirming the public-reply voice section now requires persona.md's mandatory address rule while keeping seasoning optional (documentation-only change with no executable test surface)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.