feat(skills): add reflect skill -- in-session learning capture and mistake prevention - #621
Conversation
chore: promote dev to main (docs dark mode fix)
…take prevention Battle-tested in production multi-agent deployment. Extracts HIGH/MED/LOW confidence patterns from conversations, surfaces corrections as learnings, and routes them to .squad/decisions/inbox/ or agent history.md. Zero breaking changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
🔍 Squad Team ReviewReviewed by: Procedures (Prompt Architect) — Pass 3 of consensus ceremony Review SummaryThis PR implements reflection capabilities, a feature-class enhancement that enables skills to introspect their own behavior and state. Process NotePer our team's documented proposal-first policy (.squad/decisions.md): "Meaningful changes require a proposal in docs/proposals/ before execution." This is a feature-class change that needs a proposal document before merge. What's Needed
Thank you for the contribution! The implementation looks solid — we just need the process alignment. 🙏 ⏸️ Action: HOLD — awaiting proposal + retarget to dev |
bradygaster
left a comment
There was a problem hiding this comment.
FIDO Quality Review — PR #621: reflect skill
Verdict:
Blocking Issue
❌ Wrong package names in changeset: The .changeset/reflect-skill.md uses squad-cli and squad-sdk instead of @bradygaster/squad-cli and @bradygaster/squad-sdk. Changeset won't resolve to any workspace packages.
Fix: Change the changeset frontmatter to:
\\yaml
"@bradygaster/squad-cli": minor
"@bradygaster/squad-sdk": minor
\\
Non-Blocking Notes
- Target branch: Targets
main— considerdev. - Frontmatter format: Uses a nested
metadata:block instead of flat fields. Different from the SKILL.md template (which uses flatdomain:,confidence:at top level). Not breaking, but inconsistent. - Content quality: Very thorough. The trigger classification (HIGH/MED/LOW), integration with existing Squad architecture (history.md, decisions.md), and concrete examples are excellent. The confidence threshold table is well-designed.
- File paths: Correct placement in
packages/squad-cli/templates/skills/reflect/and SDK equivalent.
Fix the changeset and this is a strong contribution.
…radygaster#621) * docs(ai-team): Screenshot review session 2 — orchestration complete Session: 2026-03-01T02-04-00Z-screenshot-review-2 Requested by: Brady Changes: - Created 5 orchestration logs (keaton, kovash, marquez, cheritto, waingro) - Created session log with consolidated P0/P1/P2 findings - Merged 2 decision inbox files to decisions.md - Appended screenshot review findings to all 5 agent history files - Removed inbox files after merging Key findings: 2 P0 blockers, 4 P1 friction points, 6+ P2 polish items. * fix: banner messaging — conditional assembled msg, clearer init hint, reduce redundant lines - Show 'Your squad is assembled' only when agents exist (P0) - Show 'Run squad init to set up your team' when roster empty + first run (P0) - Remove meaningless 'your lead' fallback from leadAgent (P0) - Change empty-roster hint to mention /init in-session option (P1) - Remove redundant 'Squad automatically routes' and '@lead' dim lines (P1) - ThinkingIndicator already says 'Routing to agent', no Coordinator label found (P1) - No trailing-period formatting issues found (P2) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: update fenster history with banner messaging cleanup Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: add Round 2 REPL UX fix tests (17 new, 47 total) Add comprehensive test coverage for scrollback and UX fixes: - Screen corruption prevention: Static key session prefix, MemoryManager archival - Banner logic: assembled text gating, @lead hint derivation, empty roster - Compaction removal: help text always full, spacing always renders - Coordinator label: Squad mapping in messages and streaming content - Init guidance: empty roster shows actionable squad init/init hints Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: clear terminal on shell start and session restore to prevent screen corruption - Add terminal clear (ESC[2J + ESC[H]) before Ink render() to wipe old scrollback content when the shell starts - Clear terminal and reset messages/archivedMessages during session restore via onRestoreSession() to prevent old content bleeding through - These changes work with the session-scoped Static keys and clearMessages API already on this branch to fully resolve the P0 screen corruption issue Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: update kovash history and file clearMessages decision Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: memoize REPL header and first-run elements (P1) Header box and first-run hint now wrapped in useMemo to prevent unnecessary re-layout on every state change. Dependencies are stable (welcome data, width) so elements are created once and reused across renders. This addresses the P1 header duplication concern — memoized elements produce stable React nodes that Ink can reconcile without redundant layout work. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * docs: update cheritto history with scrollback fix learnings Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * decision: no content suppression based on terminal width Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Fixed per FIDO review: corrected changeset package names from squad-cli/squad-sdk to @bradygaster/squad-cli/@bradygaster/squad-sdk. Also retargeting to dev branch. |
|
Thank you for this work, Tamir! 🚀 To get this PR ready for team review and merge, we need two quick adjustments:
Once those are in place, the team will take another look and we can get this merged! Thanks for your continued contributions to Squad 👍 |
617c18e to
d7b1b0d
Compare
|
Fixed per FIDO review: corrected changeset package names from |
|
Addressed both review items: (1) Fixed changeset package names to @bradygaster/squad-cli and @bradygaster/squad-sdk. (2) Added docs/proposals/reflect.md proposal document. PR already retargeted to dev. |
There was a problem hiding this comment.
Pull request overview
Adds a new reflect skill template intended to capture in-session learnings (corrections/praise/edge cases) and route them into Squad’s longer-term knowledge stores (history/decisions). This fits into the repo’s skills-based workflow by shipping the skill in both CLI and SDK template sets, plus documenting the design and versioning via Changesets.
Changes:
- Add
reflectskill template to bothpackages/squad-cliandpackages/squad-sdktemplate skill catalogs. - Add a design proposal doc describing the motivation/approach.
- Add a changeset bumping CLI and SDK versions (minor) to ship the new templates.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| packages/squad-sdk/templates/skills/reflect/SKILL.md | New reflect skill template (SDK distribution) |
| packages/squad-cli/templates/skills/reflect/SKILL.md | New reflect skill template (CLI distribution) |
| docs/proposals/reflect.md | Proposal/design doc for the reflect skill |
| .changeset/reflect-skill.md | Changeset to release the new skill templates |
| | Trigger | Example | Why Critical | | ||
| |---------|---------|--------------| | ||
| | User correction | "no", "wrong", "not like that", "never do" | Captures mistakes to prevent repetition | | ||
| | Architectural insight | "you removed that without understanding why" | Documents design decisions (Chesterton's Fence) | | ||
| | Immediate fixes | "debug", "root cause", "fix all" | Learns from errors in real-time | |
There was a problem hiding this comment.
The markdown tables are written with a leading || (double pipe), which renders as an empty first column in most markdown renderers. Use a single leading | for standard tables so they render correctly.
There was a problem hiding this comment.
Same as above — tables are correct with single pipes. No change needed.
| │ │ | ||
| │ [HIGH] + Add constraint: "{specific constraint}" │ | ||
| │ Source: "{quoted user correction}" │ | ||
| │ Target: .squad/decisions/inbox/{topic}.md │ |
There was a problem hiding this comment.
The decision inbox target in this example omits the {agent}- prefix (.squad/decisions/inbox/{topic}.md), which conflicts with the earlier guidance in this same doc (.squad/decisions/inbox/{agent}-{topic}.md) and the repo’s common inbox naming pattern. Make the examples consistent (include the agent/name prefix) so tools and humans can reliably attribute decisions.
| │ Target: .squad/decisions/inbox/{topic}.md │ | |
| │ Target: .squad/decisions/inbox/{agent}-{topic}.md │ |
There was a problem hiding this comment.
Fixed in 17fa9a8 — same fix applied to SDK copy.
| metadata: | ||
| version: 1.0.0-squad | ||
| domain: team-memory, learning | ||
| confidence: high |
There was a problem hiding this comment.
The skill frontmatter nests domain and confidence under metadata:. Today this works only because the repository’s skill frontmatter parser is line-based and ignores indentation; switching to a real YAML parser later would treat these as nested fields and domain/confidence would be missing. To keep this skill compatible with the established format used by other skills, put domain: and confidence: at the top level (and drop metadata: unless it’s actually consumed).
| metadata: | |
| version: 1.0.0-squad | |
| domain: team-memory, learning | |
| confidence: high | |
| version: 1.0.0-squad | |
| domain: team-memory, learning | |
| confidence: high |
There was a problem hiding this comment.
Fixed in 17fa9a8 — flattened frontmatter to use top-level domain: and confidence: fields, consistent with other skills.
|
|
||
| **Reflect complements existing Squad knowledge systems:** | ||
|
|
||
| 1. **`.squad/agents/{agent}/history.md`** — Permanent learnings from completed work (READ-ONLY by agents, managed by human) |
There was a problem hiding this comment.
This describes .squad/agents/{agent}/history.md as “READ-ONLY by agents”, but existing guidance in the repo has agents updating their own history.md and Scribe propagating team updates. Consider rewording this to reflect the actual governance (e.g., append-only; agents can update their own; only Scribe updates others) to avoid teaching an incorrect rule.
| 1. **`.squad/agents/{agent}/history.md`** — Permanent learnings from completed work (READ-ONLY by agents, managed by human) | |
| 1. **`.squad/agents/{agent}/history.md`** — Permanent learnings from completed work (append-only; each agent updates their own file; Scribe/human curator propagates cross-agent updates) |
There was a problem hiding this comment.
Fixed in 17fa9a8 — changed to: append-only; each agent updates their own file; Scribe propagates cross-agent updates.
| | Trigger | Example | Why Critical | | ||
| |---------|---------|--------------| | ||
| | User correction | "no", "wrong", "not like that", "never do" | Captures mistakes to prevent repetition | | ||
| | Architectural insight | "you removed that without understanding why" | Documents design decisions (Chesterton's Fence) | | ||
| | Immediate fixes | "debug", "root cause", "fix all" | Learns from errors in real-time | |
There was a problem hiding this comment.
The markdown tables are written with a leading || (double pipe), which renders as an empty first column in most markdown renderers. Use a single leading | for standard tables so they render correctly.
There was a problem hiding this comment.
Verified the raw file content — all tables use single leading | pipes. This appears to be a false positive from the reviewer. The tables render correctly in standard markdown.
| │ │ | ||
| │ [HIGH] + Add constraint: "{specific constraint}" │ | ||
| │ Source: "{quoted user correction}" │ | ||
| │ Target: .squad/decisions/inbox/{topic}.md │ |
There was a problem hiding this comment.
The decision inbox target in this example omits the {agent}- prefix (.squad/decisions/inbox/{topic}.md), which conflicts with the earlier guidance in this same doc (.squad/decisions/inbox/{agent}-{topic}.md) and the repo’s common inbox naming pattern. Make the examples consistent (include the agent/name prefix) so tools and humans can reliably attribute decisions.
| │ Target: .squad/decisions/inbox/{topic}.md │ | |
| │ Target: .squad/decisions/inbox/{agent}-{topic}.md │ |
There was a problem hiding this comment.
Fixed in 17fa9a8 — added {agent}- prefix to match the convention used earlier in the doc and across the repo.
| metadata: | ||
| version: 1.0.0-squad | ||
| domain: team-memory, learning | ||
| confidence: high |
There was a problem hiding this comment.
The skill frontmatter nests domain and confidence under metadata:. Today this works only because the repository’s skill frontmatter parser is line-based and ignores indentation; switching to a real YAML parser later would treat these as nested fields and domain/confidence would be missing. To keep this skill compatible with the established format used by other skills, put domain: and confidence: at the top level (and drop metadata: unless it’s actually consumed).
| metadata: | |
| version: 1.0.0-squad | |
| domain: team-memory, learning | |
| confidence: high | |
| version: 1.0.0-squad | |
| domain: team-memory, learning | |
| confidence: high |
There was a problem hiding this comment.
Fixed in 17fa9a8 — same fix applied to SDK copy.
|
|
||
| **Reflect complements existing Squad knowledge systems:** | ||
|
|
||
| 1. **`.squad/agents/{agent}/history.md`** — Permanent learnings from completed work (READ-ONLY by agents, managed by human) |
There was a problem hiding this comment.
This describes .squad/agents/{agent}/history.md as “READ-ONLY by agents”, but existing guidance in the repo has agents updating their own history.md and Scribe propagating team updates. Consider rewording this to reflect the actual governance (e.g., append-only; agents can update their own; only Scribe updates others) to avoid teaching an incorrect rule.
| 1. **`.squad/agents/{agent}/history.md`** — Permanent learnings from completed work (READ-ONLY by agents, managed by human) | |
| 1. **`.squad/agents/{agent}/history.md`** — Permanent learnings from completed work (append-only; agents update their own; Scribe/human may propagate cross-agent learnings) |
There was a problem hiding this comment.
Fixed in 17fa9a8 — same fix applied to SDK copy.
Flatten frontmatter (remove nested metadata: block for parser compat),
fix history.md governance description (append-only, not read-only),
fix decision inbox path to include {agent}- prefix for consistency.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Adds the
eflect\ skill — a learning capture system that extracts HIGH/MED/LOW confidence patterns from conversations to prevent repeating mistakes.
Battle-tested in a 13-agent production deployment across 6 months of daily use.
What it does
Design
Zero breaking changes.