From f87f6c042627ae0ddf245c1ffb3918a1389bcd14 Mon Sep 17 00:00:00 2001 From: Carlos Miguel Date: Wed, 22 Apr 2026 10:59:31 +0200 Subject: [PATCH 1/2] feat: improve reviewer skills after using telamon to review https://github.com/GET-E/hotels-api/pull/1875 it improved itself based on review comments that it analysed and fixed. On some areas it needed help, so I pushed it into the right direction. --- src/skills/workflow/review_changeset/SKILL.md | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/skills/workflow/review_changeset/SKILL.md b/src/skills/workflow/review_changeset/SKILL.md index bea8bd1..8fe50a5 100644 --- a/src/skills/workflow/review_changeset/SKILL.md +++ b/src/skills/workflow/review_changeset/SKILL.md @@ -37,6 +37,7 @@ When a class, method, or interface is deleted or renamed: When constructor or factory signatures change: - Verify all container bindings, registrations, and factories supply the new parameters. - Verify variadic/collection parameters are wired, not silently defaulting to empty. +- Verify new bindings are registered in the component's own ServiceProvider, not in the root AppServiceProvider (unless the binding is cross-cutting). Component encapsulation requires each bounded context to own its wiring. ### 4. Import Hygiene @@ -61,6 +62,7 @@ For every added or modified test method: - If assertions were removed or narrowed (e.g. full-row content comparison reduced to header-only), flag it as a WARNING unless the test's name was also updated to reflect the reduced scope. - A test named `amounts_are_converted_using_billing_rate` that no longer asserts anything about amounts or conversion is a BLOCKER — the test name makes a promise the body must keep. - Fixture files (`*.csv`, `*.json`, `*.xml`, snapshot files) that are updated in the changeset but are no longer loaded by any test are dead fixtures — flag as WARNING and require deletion or re-use. +- Verify test classes don't import traits already provided by the base TestCase (e.g. `RefreshDatabase` when base uses `LazilyRefreshDatabase`). Redundant traits can cause subtle behavior changes. ### 7. Static Analysis Baseline Hygiene @@ -100,6 +102,28 @@ When a `@phpstan-ignore` or `@phpstan-ignore-next-line` comment is added or alre - Under `strict_types=1`, passing a `string` where `int` is declared raises a `TypeError` at runtime. Eloquent commonly returns numeric strings for integer columns when the attribute lacks a cast. A callback typed as `fn (int $id)` applied to a plucked integer column is unsafe without a cast — flag as WARNING and suggest `fn (string|int $id): T => new T((int) $id)`. - An `@phpstan-ignore` that hides a real type mismatch rather than a PHPStan false positive is a WARNING, not an acceptable suppression. +### 13. Primitive Obsession in Commands & Boundaries + +When a command, query, or controller is added or modified: +- Verify constructor parameters use Value Objects when one exists in the domain (e.g. `UserId` not `int`, `HotelId` not `int`, `Mailbox[]` not `string[]`). +- Verify controller `authorize()` calls pass VOs to policies, not raw primitives, when the VO is already constructed in the controller. +- Verify policy methods accept the corresponding VO type, not the primitive. +- A command accepting a raw primitive when a domain VO exists for that concept is a WARNING. + +### 14. Hardcoded Configuration Values + +When application or domain layer handlers are added or modified: +- Flag hardcoded URLs, hostnames, ports, API keys, or environment-dependent strings. These should be injected via constructor + config binding (`giveConfig` or similar). +- Hardcoded values that vary per environment (dev/staging/prod) are a WARNING — they break deployment flexibility and testability. +- Pure domain constants (e.g. mathematical formulas, enum values) are not configuration and should remain inline or as class constants. + +### 15. Magic Values — Use Class Constants + +When a handler, service, or domain class uses literal numbers or strings with domain meaning: +- Flag inline magic numbers (e.g. TTL durations, retry counts, thresholds) and magic strings (e.g. email subjects, status labels) that should be class constants. +- Class constants make intent explicit, enable reuse, and simplify testing. +- A magic value used in more than one place is a WARNING. A single-use magic value with non-obvious meaning is an INFO. + ## Review Report Save to `/REVIEW-YYYY-MM-DD-NNN.md`. @@ -128,6 +152,9 @@ Save to `/REVIEW-YYYY-MM-DD-NNN.md`. > - **Utility Method Abstraction Bypass** — Sibling method state machine not driven into inconsistent state? > - **N+1 Query Regression** — Bulk-loads not undermined by remaining per-row lazy loads in the same pipeline (including in called helper methods)? > - **`@phpstan-ignore` and Type-Masking** — Suppressions masking real type mismatches (e.g. `string` vs `int` under `strict_types=1`) rather than PHPStan false positives? +> - **Primitive Obsession** — Commands, queries, and controller authorize calls use VOs when domain VO exists? Policies accept VOs not primitives? +> - **Hardcoded Configuration** — No hardcoded URLs, hostnames, or environment-dependent values in application/domain handlers? +> - **Magic Values** — Domain-meaningful literals extracted to class constants? > - **Code Style** — Symbols imported? No FQCNs inline? Explicit guards? Sealed/final convention? > - **Role Compliance** — All code changes made by the Developer? > - **Documentation** — Manual config steps documented? Obsolete steps removed? From 3b39a9ccf91ccee3ae4ed3377c609432834060c8 Mon Sep 17 00:00:00 2001 From: Carlos Miguel Date: Fri, 24 Apr 2026 17:01:07 +0200 Subject: [PATCH 2/2] feat: add telamon.roundtable skill for parallel multi-agent discussions --- src/agents/telamon.md | 1 + src/skills/workflow/roundtable/SKILL.md | 142 ++++++++++++++++++++++++ 2 files changed, 143 insertions(+) create mode 100644 src/skills/workflow/roundtable/SKILL.md diff --git a/src/agents/telamon.md b/src/agents/telamon.md index b34d5d7..e9b5bde 100644 --- a/src/agents/telamon.md +++ b/src/agents/telamon.md @@ -35,6 +35,7 @@ When you need to implement, you follow the `telamon.implement_story` skill, invo - When optimizing agent context setup, rules files, or MCP integration, use the skill `context-engineering` - When preparing to deploy to production or coordinating a launch, use the skill `shipping-and-launch` - When writing or organizing documentation files, use the skill `telamon.documentation_rules` +- When multiple specialist perspectives are needed for a cross-cutting decision, use the skill `telamon.roundtable` ## Request Classification diff --git a/src/skills/workflow/roundtable/SKILL.md b/src/skills/workflow/roundtable/SKILL.md new file mode 100644 index 0000000..5a149e8 --- /dev/null +++ b/src/skills/workflow/roundtable/SKILL.md @@ -0,0 +1,142 @@ +--- +name: telamon.roundtable +description: "Orchestrates parallel multi-agent roundtable discussions for cross-cutting decisions. Use when multiple specialist perspectives are needed simultaneously — architecture debates, trade-off analysis, or any decision that benefits from diverse expert viewpoints." +--- + +# Skill: Roundtable + +Facilitate roundtable discussions where telamon subagents participate as **independent parallel agents** — each spawned via the Task tool so they think for themselves. The orchestrator picks voices, builds context, spawns agents, and presents their unabridged responses. + +## Why This Matters + +When one LLM roleplays multiple characters, opinions converge and feel performative. By spawning each agent as its own subagent, you get real diversity of thought — agents that actually disagree, catch things others miss, and bring authentic expertise. + +## When to Use + +- Cross-cutting architectural decisions ("Should we use event sourcing here?") +- Trade-off analysis where multiple domains intersect +- Planning decisions that affect product, architecture, and implementation +- Any moment where sequential delegation would lose the "debate" quality +- When the human asks for "multiple perspectives" or "roundtable" + +## Activation + +When invoked (by orchestrator or human request): + +1. **Identify the question** — what decision or topic needs multi-agent input +2. **Pick 2-4 agents** whose expertise is most relevant (see Agent Selection below) +3. **Build context** for each agent +4. **Spawn all agents in parallel** via Task tool +5. **Present responses** unabridged, one per agent +6. **Facilitate follow-ups** as the human directs + +## Agent Selection + +Choose from the telamon agent roster based on relevance: + +| Agent | Expertise | Bring in when... | +|-------|-----------|-------------------| +| @po | Product domain, requirements, business value | Decision affects user value or business goals | +| @architect | System design, patterns, trade-offs | Technical architecture is at stake | +| @critic | Consistency, pattern drift, quality | Need someone to challenge assumptions | +| @security | Vulnerabilities, threat models, hardening | Security implications exist | +| @ux-designer | User flows, interaction design | UX impact needs consideration | +| @ui-designer | Visual design, design tokens | Visual/UI implications | +| @tester | Test strategy, coverage, quality gates | Testability concerns | +| @reviewer | Code quality, conventions | Implementation quality matters | + +### Selection Guidelines + +- **Simple question**: 2 agents with most relevant expertise +- **Complex or cross-cutting**: 3-4 agents from different domains +- **Human names specific agents**: Always include those, plus 1-2 complementary voices +- **Rotate over time** — avoid same 2 agents dominating every round + +## Spawning Agents + +For each selected agent, spawn a Task with `subagent_type` matching the agent role. Each gets: + +### Prompt Template + +``` +You are participating in a roundtable discussion with other telamon agents. + +## Your Role +You are the {agent_role} agent. Respond authentically from your expertise. + +## Discussion Topic +{the question or decision being discussed} + +## Project Context +{relevant project context — keep under 400 words} + +## What Other Agents Said (if follow-up round) +{previous agent responses, if this is a reaction round} + +## Guidelines +- Respond from your domain expertise. Be specific and actionable. +- Disagree with other agents when your perspective warrants it. Don't hedge. +- If you have nothing substantive to add beyond what others said, say so briefly. +- Keep response focused — scale length to substance, don't pad. +- You may flag risks, trade-offs, or concerns others might miss. +``` + +**Spawn all agents in parallel** — put all Task calls in a single response. + +## Presenting Responses + +Present each agent's full response — distinct, complete, in their own voice. Never blend, paraphrase, or condense. Format: + +``` +### 🏗️ Architect +{full response} + +### 📋 Product Owner +{full response} + +### 🔍 Critic +{full response} +``` + +After all responses, optionally add a brief **Orchestrator Note** — flag disagreements worth exploring, suggest an agent to bring in next round. Keep short and clearly labeled. + +## Follow-up Rounds + +The human drives what happens next: + +| Human says... | Action | +|---|---| +| Continues general discussion | Pick fresh agents, repeat | +| "What does @architect think about @critic's point?" | Spawn just architect with critic's response as context | +| "Bring in @security on this" | Spawn security with discussion summary | +| "I agree with @po, let's go deeper" | Spawn @po + 1-2 others to expand | +| Asks everyone | Back to full round | + +## Context Management + +As conversation grows, summarize prior rounds rather than passing full transcript. Keep "Discussion Context" under 400 words — tight summary of positions taken and what human is driving toward. Update every 2-3 rounds. + +## Edge Cases + +- **Agents all agree**: Bring in @critic or explicitly ask one agent to steelman the opposing view +- **Going in circles**: Summarize the impasse clearly ("@architect and @critic disagree on X because Y"), ask human which angle to pursue +- **Human disengaged**: Ask directly — continue, change topic, or wrap up? +- **Subagents unavailable (`--solo` fallback)**: If Task tool is unavailable, roleplay each agent sequentially in clearly labeled sections. Explicitly note "Solo mode — responses are simulated." Maintain distinct voices; do not blend perspectives within a section. + +## Exit + +When human is done (any natural phrasing), give brief summary of key positions and any consensus reached. Return to normal orchestration. + +## MUST + +- Spawn agents as independent parallel Task calls — never roleplay multiple agents yourself +- Present full unabridged responses — never synthesize or blend +- Pick agents based on relevance to the specific question +- Keep context summaries under 400 words for follow-up rounds + +## MUST NOT + +- Generate agent responses yourself (that defeats the purpose) +- Summarize or paraphrase agent responses before showing them +- Spawn more than 4 agents per round (diminishing returns) +- Force consensus — disagreement is valuable