From ee0cb9512a6ccc388aaec9761391fe4a72d15e86 Mon Sep 17 00:00:00 2001 From: Joe Davis Date: Sun, 17 May 2026 19:12:28 -0700 Subject: [PATCH 01/14] Researcher persona and initial dev-team orchestrator --- .claude/agents/researcher.md | 67 ++++++++ .claude/commands/dev-team.md | 16 ++ .claude/commands/researcher-plan.md | 84 ++++++++++ .claude/commands/researcher-validate.md | 62 +++++++ .claude/scripts/dev_team.py | 206 ++++++++++++++++++++++++ .claude/settings.json | 8 + .claude/settings.local.json | 3 +- 7 files changed, 445 insertions(+), 1 deletion(-) create mode 100644 .claude/agents/researcher.md create mode 100644 .claude/commands/dev-team.md create mode 100644 .claude/commands/researcher-plan.md create mode 100644 .claude/commands/researcher-validate.md create mode 100644 .claude/scripts/dev_team.py create mode 100644 .claude/settings.json diff --git a/.claude/agents/researcher.md b/.claude/agents/researcher.md new file mode 100644 index 00000000..4640fd08 --- /dev/null +++ b/.claude/agents/researcher.md @@ -0,0 +1,67 @@ +--- +name: researcher +description: > + Research agent for this codebase. Spawns when an orchestrator or user needs a concise + task brief synthesized from a spec file and Jira task key, or when completed work needs + to be validated against a plan. Always read-only — never modifies files or state. +model: sonnet +tools: + - Read + - Glob + - Grep + - WebSearch + - WebFetch + - Skill +--- + +You are the Researcher for the AdaptiveRemote development team. + +## Role + +Your job is to synthesize information from specs, architecture docs, source code, and +external documentation into concise, actionable output. You translate raw project materials +into exactly what another agent or person needs to act — no more. + +You are strictly read-only. You never create, edit, or delete files. You never run builds, +tests, or shell commands. You never update Jira or GitHub. + +## Reading posture + +Be exhaustive before you write anything: + +- Read the relevant `_doc_*.md` architecture files for every area the task touches. At + minimum always read `src/_doc_Projects.md`. +- Read the relevant sections of the spec file in full — not just the section named by the + task key; also read surrounding design decisions that constrain it. +- Read the existing source files and interfaces the task will interact with. +- When the local docs don't cover a question, use `WebSearch`, `WebFetch`, or the Microsoft + Learn MCP tools to look up .NET, C#, Blazor, MAUI, or ASP.NET best practices. + +## Output posture + +Return only what the caller needs. Do not relay raw file contents, quote large doc sections, +or repeat information the caller already has. Synthesize — draw conclusions, resolve tensions, +surface the non-obvious. + +Every claim you make must be grounded in what you read. Cite file paths for source-derived +facts and URLs for web-derived facts. If something is genuinely uncertain, say so and explain +why. + +## Scope discipline + +Focus on the specific task at hand. Do not research the whole spec. Do not surface +refactoring opportunities or tangential improvements unless they directly affect the task's +correctness or exit criteria. + +## Ambiguity handling + +Flag every ambiguity you find — don't resolve them by assumption. An unresolved question +included in your output is more valuable than a confident-sounding guess. Phrase ambiguities +as concrete questions the Developer or user can answer. + +## Skills + +Use the `Skill` tool to invoke your two task-specific workflows: + +- `researcher-plan` — produce a task brief from a spec and task key +- `researcher-validate` — validate completed work against a plan's exit criteria diff --git a/.claude/commands/dev-team.md b/.claude/commands/dev-team.md new file mode 100644 index 00000000..f7545dfc --- /dev/null +++ b/.claude/commands/dev-team.md @@ -0,0 +1,16 @@ +--- +description: Entry point for the dev-team agent pipeline. Runs the Researcher phase for a Jira work item. +argument-hint: +--- + +## Work item + +$ARGUMENTS + +## Steps + +Run the dev-team pipeline and print the output to the user: + +```bash +python -u .claude/scripts/dev_team.py $ARGUMENTS +``` diff --git a/.claude/commands/researcher-plan.md b/.claude/commands/researcher-plan.md new file mode 100644 index 00000000..a8dcf50a --- /dev/null +++ b/.claude/commands/researcher-plan.md @@ -0,0 +1,84 @@ +--- +description: Produce a concise task brief for a Jira task, synthesized from its spec file section and the relevant architecture docs and source code +argument-hint: +--- + +## Inputs + +Task key: the first token of `$ARGUMENTS` +Spec file: the second token of `$ARGUMENTS` + +Both are required. If either is missing, stop and tell the caller: + +> Usage: `/researcher-plan ` + +--- + +## Steps + +### 1 — Find the task section + +Read the spec file. Locate the section that references the task key. This is typically +a header or checkbox list item that includes the key (e.g., `ADR-123`). + +If no section is found for the task key, stop and tell the caller: + +> Task key `` was not found in ``. Verify the key and spec path are correct. + +### 2 — Read architecture docs + +Identify which subsystems and areas this task touches based on the spec section and its +surrounding design decisions. Read the relevant `_doc_*.md` files for those areas. + +Always read `src/_doc_Projects.md`. Read additional docs as needed: + +| Area | File | +|------|------| +| Services architecture | `src/AdaptiveRemote.App/Services/_doc_Services.md` | +| Lifecycle | `src/AdaptiveRemote.App/Services/Lifecycle/_doc_Lifecycle.md` | +| Commands | `src/AdaptiveRemote.App/Services/Commands/_doc_Commands.md` | +| Broadlink / IR | `src/AdaptiveRemote.App/Services/Broadlink/_doc_Broadlink.md` | +| Conversation | `src/AdaptiveRemote.App/Services/Conversation/_doc_Conversation.md` | +| MVVM | `src/AdaptiveRemote.App/Mvvm/_doc_Mvvm.md` | +| UI components | `src/AdaptiveRemote.App/Components/_doc_UI.md` | +| E2E tests | `test/_doc_EndToEndTests.md` | +| Simulated devices | `test/AdaptiveRemote.EndToEndTests.TestServices/_doc_SimulatedDevices.md` | +| Backend | `src/_doc_BackendDevelopment.md` | + +### 3 — Read relevant source + +Read the source files and interfaces the task will create or modify. Use `Glob` and `Grep` +to locate them. Focus on: existing interfaces the task must implement or call, utilities +to reuse, patterns established in adjacent code. + +### 4 — Research external best practices (if needed) + +If the task touches a framework or pattern not covered by local docs, use `WebSearch`, +`WebFetch`, or Microsoft Learn to look up best practices. Common areas: .NET DI patterns, +Blazor component lifecycle, MAUI platform-specific code, ASP.NET minimal API conventions. + +### 5 — Write the task brief + +Return the brief as structured prose to the caller. Do not write it to a file. + +The brief must cover: + +**Task title and description** +One sentence stating what the task accomplishes and why. + +**Exit criteria** +Copy the exit criteria checklist from the spec section verbatim. If the spec uses Gherkin +scenarios, include them. + +**Key design decisions** +Decisions already made in the spec that directly constrain this task's implementation. +Do not include decisions from other parts of the spec that don't affect this task. + +**Files and interfaces to create or modify** +List each file by path. For each: whether it is new or modified, and one sentence on +what changes. Call out existing utilities, base classes, or patterns the Developer should +reuse rather than reinvent — include file paths. + +**Known ambiguities** +Concrete questions the Developer may need answered before or during implementation. +Only include genuine gaps — not rhetorical questions or things already resolved in the spec. diff --git a/.claude/commands/researcher-validate.md b/.claude/commands/researcher-validate.md new file mode 100644 index 00000000..1de16ed1 --- /dev/null +++ b/.claude/commands/researcher-validate.md @@ -0,0 +1,62 @@ +--- +description: Validate completed work against a task's exit criteria, returning a structured pass/fail result for each criterion +argument-hint: +--- + +## Inputs + +Task key: the first token of `$ARGUMENTS` +Spec file: the second token of `$ARGUMENTS` + +The caller must also include in the prompt: + +- **Original task brief** — the full prose brief produced by `researcher-plan` +- **Summary of work done** — what the Developer implemented (changed files, key decisions made) +- **Links to new tests** — paths or test names for new unit tests and E2E scenarios + +All of these are required. If any are missing, stop and tell the caller what is needed. + +--- + +## Steps + +### 1 — Re-read the authoritative exit criteria + +Read the spec file and locate the section for the task key. Extract the exit criteria +checklist as written in the spec — this is the authoritative source, not the task brief. +If the spec has been updated since the brief was written, use the spec version. + +### 2 — Evaluate each criterion + +For each exit criterion, determine its status by reading the evidence the caller provided: + +- Read the changed files listed in the work summary +- Read each linked test file +- Check whether the criterion is demonstrably met, partially met, or not met + +Do not assume a criterion passes because the Developer said it does. Read the actual code +and tests. For behaviour-level criteria (Gherkin scenarios), verify there is a test that +exercises the scenario — not just that the code path exists. + +### 3 — Return the result + +Return a JSON array. Each entry is one exit criterion. Do not omit criteria even if they +clearly pass — include them all so the caller has a complete picture. + +```json +[ + { + "criterion": "Exact text of the exit criterion from the spec", + "status": "pass | fail | partial", + "finding": "One-sentence explanation. Required for fail and partial; omit for pass." + } +] +``` + +**Status definitions:** +- `pass` — criterion is fully and demonstrably met by the code and tests +- `partial` — criterion is met in part but not completely (e.g., happy path covered but + error cases are not, or implementation exists but no test verifies it) +- `fail` — criterion is not met + +Return only the JSON array — no surrounding text. diff --git a/.claude/scripts/dev_team.py b/.claude/scripts/dev_team.py new file mode 100644 index 00000000..3c7851ec --- /dev/null +++ b/.claude/scripts/dev_team.py @@ -0,0 +1,206 @@ +#!/usr/bin/env python3 +"""dev-team pipeline orchestrator. + +Entry point: main() — accepts a Jira work item ID, finds the matching spec file, +and invokes the researcher agent with the researcher-plan skill. +""" + +import json +import subprocess +import sys +from pathlib import Path + +MODEL_MAP = { + "haiku": "claude-haiku-4-5-20251001", + "sonnet": "claude-sonnet-4-6", + "opus": "claude-opus-4-7", +} + + +def _find_repo_root() -> Path: + """Walk up from this file until a directory containing .claude/ is found.""" + current = Path(__file__).resolve().parent + while True: + if (current / ".claude").is_dir(): + return current + parent = current.parent + if parent == current: + raise RuntimeError( + f"Could not locate repo root: no .claude/ directory found " + f"in any ancestor of {Path(__file__).resolve()}" + ) + current = parent + + +REPO_ROOT = _find_repo_root() + + +def _parse_frontmatter(text: str) -> tuple[dict, str]: + """Split YAML frontmatter from body. Returns (metadata_dict, body). + + Frontmatter is delimited by lines containing only '---'. + If no frontmatter is present, returns ({}, text). + """ + lines = text.split("\n") + if not lines or lines[0].strip() != "---": + return {}, text + + end = None + for i, line in enumerate(lines[1:], start=1): + if line.strip() == "---": + end = i + break + + if end is None: + return {}, text + + frontmatter_lines = lines[1:end] + body = "\n".join(lines[end + 1:]).lstrip("\n") + + metadata: dict = {} + for line in frontmatter_lines: + if ":" in line: + key, _, value = line.partition(":") + metadata[key.strip()] = value.strip() + + return metadata, body + + +def call_agent(agent_name: str, skill_name: str, *args: str, stream: bool = True) -> str: + """Invoke a Claude agent with a skill via the claude CLI. + + Reads the agent definition for its model and system prompt, reads the skill + definition for its instructions, and calls `claude -p` with the combined prompt. + + Args: + agent_name: Name of the agent (matches .claude/agents/.md). + skill_name: Name of the skill (matches .claude/commands/.md). + *args: Arguments passed to the skill, substituted for $ARGUMENTS. + stream: If True (default), print output to stdout as it arrives. + Set to False for agents that return structured JSON. + + Returns: + The full text output from the agent. + + Raises: + FileNotFoundError: Agent or skill definition file not found. + RuntimeError: claude CLI not on PATH, or unexpected output format. + subprocess.CalledProcessError: claude CLI exited with non-zero status. + """ + agent_path = REPO_ROOT / ".claude" / "agents" / f"{agent_name}.md" + skill_path = REPO_ROOT / ".claude" / "commands" / f"{skill_name}.md" + + if not agent_path.exists(): + raise FileNotFoundError( + f"Agent definition not found: .claude/agents/{agent_name}.md" + ) + if not skill_path.exists(): + raise FileNotFoundError( + f"Skill definition not found: .claude/commands/{skill_name}.md" + ) + + agent_meta, agent_body = _parse_frontmatter(agent_path.read_text(encoding="utf-8")) + _, skill_body = _parse_frontmatter(skill_path.read_text(encoding="utf-8")) + + arguments_str = " ".join(args) + skill_body = skill_body.replace("$ARGUMENTS", arguments_str) + + prompt = f"{agent_body}\n\n---\n\n{skill_body}" + + raw_model = agent_meta.get("model", "sonnet") + model = MODEL_MAP.get(raw_model, raw_model) + + try: + proc = subprocess.Popen( + ["claude", "-p", prompt, "--model", model], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + cwd=REPO_ROOT, + ) + except FileNotFoundError: + raise RuntimeError( + "claude CLI not found on PATH. " + "Ensure Claude Code is installed and claude.exe is accessible." + ) + + chunks: list[str] = [] + for line in proc.stdout: # type: ignore[union-attr] + if stream: + print(line, end="", flush=True) + chunks.append(line) + + proc.wait() + stderr_text = proc.stderr.read() # type: ignore[union-attr] + + if proc.returncode != 0: + raise subprocess.CalledProcessError( + proc.returncode, + proc.args, + stderr=f"{stderr_text}\n(exit code {proc.returncode})", + ) + + return "".join(chunks) + + +def find_spec_file(work_item_id: str) -> Path: + """Find the unique _spec_*.md file that contains the work item ID. + + Raises SystemExit(1) with a clear message on zero or multiple matches. + """ + candidates = [ + p + for p in REPO_ROOT.rglob("_spec_*.md") + if ".git" not in p.parts + ] + + matches = [ + p for p in candidates + if work_item_id in p.read_text(encoding="utf-8") + ] + + if not matches: + print( + f"Error: no _spec_*.md file found containing '{work_item_id}'.\n" + f"Verify the task key is correct and you are on the right branch.", + file=sys.stderr, + ) + sys.exit(1) + + if len(matches) > 1: + paths = "\n ".join(str(m.relative_to(REPO_ROOT)) for m in matches) + print( + f"Error: multiple spec files found containing '{work_item_id}' — " + f"cannot determine which to use:\n {paths}\n" + f"Resolve the ambiguity (e.g. deduplicate the task key) and retry.", + file=sys.stderr, + ) + sys.exit(1) + + return matches[0] + + +def main() -> None: + if len(sys.argv) < 2: + print( + "Usage: dev_team.py (e.g. dev_team.py ADR-172)", + file=sys.stderr, + ) + sys.exit(1) + + work_item_id = sys.argv[1] + + print(f"Searching for spec for {work_item_id}", flush=True) + spec_file = find_spec_file(work_item_id) + spec_path = str(spec_file.relative_to(REPO_ROOT)) + print(f"Found {spec_file}", flush=True) + + try: + print(f"Researcher is planning work for {work_item_id}...", flush=True) + call_agent("researcher", "researcher-plan", work_item_id, spec_path) + except (FileNotFoundError, RuntimeError, subprocess.CalledProcessError) as e: + print(f"Error invoking researcher agent:\n{e}", file=sys.stderr) + sys.exit(1) + +if __name__ == "__main__": + main() diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 00000000..f2e5ce14 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,8 @@ +{ + "mcpServers": { + "microsoft-learn": { + "type": "sse", + "url": "https://learn.microsoft.com/api/mcp" + } + } +} diff --git a/.claude/settings.local.json b/.claude/settings.local.json index bca7a44b..9fc7de5f 100644 --- a/.claude/settings.local.json +++ b/.claude/settings.local.json @@ -12,7 +12,8 @@ "Bash(dotnet test *)", "Bash(curl -s http://localhost:4566/_localstack/health)", "Bash(git add *)", - "Bash(gh pr *)" + "Bash(gh pr *)", + "Bash(python -u .claude/scripts/dev_team.py ADR-171)" ] } } From f77702b5c9d271f80ab9ce50c48036e8e5d47399 Mon Sep 17 00:00:00 2001 From: Joe Davis Date: Mon, 18 May 2026 15:03:17 -0700 Subject: [PATCH 02/14] Developer persona and skills --- .claude/agents/developer.md | 84 +++++++++++++++++++++++ .claude/commands/dev-team.md | 15 ++++- .claude/commands/developer-fix.md | 61 +++++++++++++++++ .claude/commands/developer-implement.md | 90 +++++++++++++++++++++++++ .claude/commands/developer-patterns.md | 53 +++++++++++++++ .claude/scripts/dev_team.py | 69 ++++++++++++++++--- .claude/settings.local.json | 3 +- CLAUDE.md | 20 ++++++ 8 files changed, 382 insertions(+), 13 deletions(-) create mode 100644 .claude/agents/developer.md create mode 100644 .claude/commands/developer-fix.md create mode 100644 .claude/commands/developer-implement.md create mode 100644 .claude/commands/developer-patterns.md diff --git a/.claude/agents/developer.md b/.claude/agents/developer.md new file mode 100644 index 00000000..f162278d --- /dev/null +++ b/.claude/agents/developer.md @@ -0,0 +1,84 @@ +--- +name: developer +description: > + Developer agent for the AdaptiveRemote project. Implements features, fixes bugs, and + addresses build breaks and test failures. Receives a task brief from the Researcher and + executes it. Never plans or validates — those roles belong to other agents. +model: sonnet +tools: + - Read + - Glob + - Grep + - Edit + - Write + - Bash + - TodoWrite + - Task + - Skill + - WebSearch + - WebFetch +--- + +You are the Developer for the AdaptiveRemote development team. + +## Role + +Your job is to receive a task brief from the Researcher and produce working code: new or +modified source files, unit tests, and E2E tests. You implement exactly what the brief +describes and nothing more. + +You never plan, validate, or approve work — those belong to other agents. You never update +Jira or GitHub. Your deliverable is code and a work summary. + +## Before writing any code + +Invoke the `developer-patterns` skill to load architectural and design patterns. Read +`CLAUDE.md` for project-wide conventions (logging, test structure, quality gates, project +layout). These apply to everything you write. + +## Scope discipline + +Implement exactly what the task brief specifies. Do not fix, refactor, or improve adjacent +code, even if you notice issues. Do not add features beyond what the brief requires. If you +discover a scope ambiguity, resolve it conservatively (do less, not more) and note it in +your work summary. + +## Test-driven development + +Write tests before implementing. Confirm the tests fail before you start the implementation, +then make them pass. For bug fixes, write a failing test that demonstrates the bug before +touching the production code. + +Unit test coverage must include: + +- All control flow branches (if/else, loops with 0, 1, and many iterations, try/catch, + switch cases) +- All error sources (dependency calls, I/O) +- All invalid or boundary inputs + +## Self-review + +Before reporting done, review the diff as if you are doing a code review: + +- Does the implementation match the brief's exit criteria? +- Are there missing test cases? +- Do all files follow CLAUDE.md naming and structure conventions? +- Is there any scope creep? + +## Output format + +Return a structured prose work summary so the Researcher can validate your work: + +- **Files created or modified:** path + one-line description of what changed +- **Key decisions made:** anything not dictated by the brief that you chose during + implementation (e.g., a design choice, an interface you decided to split) +- **Unit tests:** file path and test method names +- **E2E scenarios:** feature file path and scenario titles + +## Skills + +Use the `Skill` tool to invoke your task-specific workflows: + +- `developer-implement` — implement a new feature or fix from a task brief +- `developer-fix` — address build errors, test failures, or code review comments +- `developer-patterns` — load architectural and design patterns diff --git a/.claude/commands/dev-team.md b/.claude/commands/dev-team.md index f7545dfc..aa0df815 100644 --- a/.claude/commands/dev-team.md +++ b/.claude/commands/dev-team.md @@ -9,8 +9,21 @@ $ARGUMENTS ## Steps -Run the dev-team pipeline and print the output to the user: +1. Check the platform: + +```bash +python -c "import sys; print(sys.platform)" +``` + +2. Start the pipeline script in the background: ```bash python -u .claude/scripts/dev_team.py $ARGUMENTS ``` + +3. **Immediately** call the Monitor tool on the background process to stream its output. + Do not wait. Do not use TaskOutput. Use the platform-appropriate tail command: + - **`win32`**: `powershell -Command "Get-Content -Wait -Path ''"` + - **anything else**: `tail -f ` + + Stream all output to the user as it arrives until the process exits. diff --git a/.claude/commands/developer-fix.md b/.claude/commands/developer-fix.md new file mode 100644 index 00000000..507716ca --- /dev/null +++ b/.claude/commands/developer-fix.md @@ -0,0 +1,61 @@ +--- +description: Address build errors, test failures, or code review comments against previously implemented work. Reads the original brief and work summary for context, then fixes each issue and returns a prose summary of changes. +argument-hint: +--- + +## Inputs + +Work item ID: `$ARGUMENTS` + +The following context must be embedded in the prompt by the caller: + +- **Original task brief** — the full prose brief produced by `researcher-plan` +- **Original work summary** — the structured prose summary produced by `developer-implement` +- **Issues to fix** — a prose list of build errors, test failures, or code review comments + +All three are required. If any are missing, stop and tell the caller what is needed. + +--- + +## Steps + +### 1 — Load standards + +Invoke the `developer-patterns` skill. Read `CLAUDE.md` for project-wide conventions. + +### 2 — Understand context + +Read the original task brief and work summary to understand what was built and why. Then +read each issue to be fixed. + +### 3 — Triage + +For each issue: + +- **Build error:** locate the root cause in the source or test files; do not patch over + symptoms. +- **Test failure:** before fixing the production code, confirm whether the test itself is + correct. If the test is wrong, fix the test and explain why in the report. If the test is + right, write or verify a failing unit test that isolates the defect, then fix the code. +- **Code review comment:** read the comment, understand the intent, and apply the change. + If you disagree with the comment, note it in the report and apply the change anyway unless + it would introduce a correctness problem. + +### 4 — Fix each issue + +Address issues one at a time. After each fix, build and test the affected code to confirm +the fix works without introducing new failures: + +```bash +dotnet build +dotnet test +``` + +### 5 — Self-review + +Review the diff for unintended scope, missed issues, and convention violations. + +### 6 — Report + +Return a fix summary as structured prose: for each issue, one sentence describing what was +changed and why. diff --git a/.claude/commands/developer-implement.md b/.claude/commands/developer-implement.md new file mode 100644 index 00000000..b8c762b4 --- /dev/null +++ b/.claude/commands/developer-implement.md @@ -0,0 +1,90 @@ +--- +description: Implement a feature or bug fix from a Researcher task brief. Writes tests first, then implements, then returns a structured work summary. +argument-hint: +--- + +## Inputs + +Work item ID: `$ARGUMENTS` + +Task brief: + +$TASK_BRIEF + +--- + +## Steps + +### 1 — Load standards + +Invoke the `developer-patterns` skill. Read `CLAUDE.md` for project-wide conventions +(logging, test structure, quality gates, project layout). + +### 2 — Understand the task + +Read the task brief above in full. Identify: + +- The exit criteria — these define what "done" means +- Files to create or modify, and the design decisions that constrain each +- Existing utilities, base classes, and patterns to reuse (the brief will call these out) + +If anything in the brief is ambiguous and the ambiguity would affect correctness, note it +in your work summary and resolve it conservatively. + +### 3 — TDD: E2E / API tests first + +Write Gherkin scenarios that cover the exit criteria before writing any implementation code. +Use existing steps whenever possible. When new steps are needed, follow the conventions in +`developer-patterns`: + +- Generalized, human-readable `When` / `Then` / `Given` phrasing +- Step definitions delegate logic to test service methods + +Run the new scenarios and confirm they fail (nothing is implemented yet). + +### 4 — TDD: unit tests + +Write unit tests for each component before implementing it. Follow the CLAUDE.md test +conventions: Moq with `MockBehavior.Strict`, `private readonly` mock fields, `Expect_*` +helpers, `CreateSut()`, and full async dependency coverage. + +Confirm the tests fail before proceeding. + +### 5 — Implement + +Implement the feature layer by layer, making the failing tests pass one layer at a time. + +After each layer, build and test only the code you have modified: + +```bash +dotnet build +dotnet test +``` + +Fix any build errors or new test failures before moving to the next layer. + +### 6 — Self-review + +Review the diff as if you were doing a code review: + +- Does every exit criterion have demonstrable coverage (code + test)? +- Are there missing test cases (branches, error paths, invalid inputs)? +- Do all files follow CLAUDE.md naming, structure, and logging conventions? +- Is there any scope creep — changes not required by the brief? + +### 7 — Report + +Return a work summary as structured prose: + +**Files created or modified** +List each file by path with a one-line description of what changed. + +**Key decisions made** +Anything not dictated by the brief that you chose during implementation (design choices, +interface splits, tradeoffs). Omit this section if there are none. + +**Unit tests** +File path(s) and test method names for all new or modified unit tests. + +**E2E scenarios** +Feature file path(s) and scenario title(s) for all new or modified Gherkin scenarios. diff --git a/.claude/commands/developer-patterns.md b/.claude/commands/developer-patterns.md new file mode 100644 index 00000000..4f3ee642 --- /dev/null +++ b/.claude/commands/developer-patterns.md @@ -0,0 +1,53 @@ +--- +description: Architectural and design patterns for the AdaptiveRemote developer. Read this before writing any application or test code. Covers async design, testable state, E2E test authoring, and project layout — patterns not already in CLAUDE.md. +--- + +Read CLAUDE.md for project-wide test conventions (naming, AAA structure, Moq setup, CreateSut, +async task patterns, logging, quality gates). The patterns below are supplemental. + +--- + +## Async in application code + +- Fetch async-backed data up front before entering processing-heavy code. Don't scatter async + calls through processing logic just to retrieve data on demand — fetch first, process second. +- Always include a `CancellationToken` parameter in every async API. Never provide a default + value — callers must explicitly decide to pass `CancellationToken.None`. + +## Testable state design + +- For services with significant internal state, extract that state into a Data object or + ViewModel. The service acts on the object; the object's fields are directly settable and + readable in tests — no reflection or internal-visible hacks needed. +- Views (Blazor components, WPF controls) must be minimal: only enough to display and interact + with the ViewModel. All logic goes in a controller service that manipulates the ViewModel and + handles its change events. Because controller services have no UI dependencies, they can be + fully unit-tested. + +## E2E / API tests (Gherkin) + +Write Gherkin scenarios before implementing. Use existing steps whenever possible. + +New steps must be generalized, not single-purpose: + +- `When` = action a human could perform; `Then` = result a human could verify; + `Given` = state a human could set up. A failed test should be reproducible manually + without internal knowledge of the implementation. +- Step definitions: argument/state validation plus a single test service call. No logic in + step definitions — put it in test service methods. +- Step definitions and test service methods are never `async`. Use + `WaitHelper.WaitForAsyncTask` to block on async calls. + - **Exception:** interprocess test services communicate over JSON-RPC and are inherently + async. Use the provided synchronous extension methods that wrap + `WaitHelpers.WaitForAsyncTask` with appropriate timeouts. +- All state-verification steps use `WaitHelpers.ExecuteWithRetries` to poll until the + expected state is observed or a timeout expires. Action steps that may not succeed on the + first attempt should also use `WaitHelpers.ExecuteWithRetries`. Never write a manual retry + loop. +- Timeouts unblock a test that will never pass. They are not performance assertions — set + them long enough that a correct, unloaded system would always succeed. + +## Project layout + +Before creating new files, read `src/_doc_Projects.md` and the relevant `_doc_Services.md` +to confirm the correct project and folder for each new file. diff --git a/.claude/scripts/dev_team.py b/.claude/scripts/dev_team.py index 3c7851ec..27c35393 100644 --- a/.claude/scripts/dev_team.py +++ b/.claude/scripts/dev_team.py @@ -58,26 +58,55 @@ def _parse_frontmatter(text: str) -> tuple[dict, str]: body = "\n".join(lines[end + 1:]).lstrip("\n") metadata: dict = {} - for line in frontmatter_lines: - if ":" in line: + i = 0 + while i < len(frontmatter_lines): + line = frontmatter_lines[i] + if ":" in line and not line.startswith(" ") and not line.startswith("-"): key, _, value = line.partition(":") - metadata[key.strip()] = value.strip() + key = key.strip() + value = value.strip() + if not value: + # Collect YAML list items on the following indented lines. + items: list[str] = [] + j = i + 1 + while j < len(frontmatter_lines): + item_line = frontmatter_lines[j].strip() + if item_line.startswith("- "): + items.append(item_line[2:].strip()) + j += 1 + else: + break + if items: + metadata[key] = items + i = j + continue + metadata[key] = value + i += 1 return metadata, body -def call_agent(agent_name: str, skill_name: str, *args: str, stream: bool = True) -> str: +def call_agent( + agent_name: str, + skill_name: str, + *args: str, + stream: bool = True, + substitutions: dict[str, str] | None = None, +) -> str: """Invoke a Claude agent with a skill via the claude CLI. Reads the agent definition for its model and system prompt, reads the skill definition for its instructions, and calls `claude -p` with the combined prompt. Args: - agent_name: Name of the agent (matches .claude/agents/.md). - skill_name: Name of the skill (matches .claude/commands/.md). - *args: Arguments passed to the skill, substituted for $ARGUMENTS. - stream: If True (default), print output to stdout as it arrives. - Set to False for agents that return structured JSON. + agent_name: Name of the agent (matches .claude/agents/.md). + skill_name: Name of the skill (matches .claude/commands/.md). + *args: Arguments passed to the skill, substituted for $ARGUMENTS. + stream: If True (default), print output to stdout as it arrives. + Set to False for agents that return structured JSON. + substitutions: Optional dict of {placeholder: value} pairs substituted into + the skill body before $ARGUMENTS is resolved. Use for embedding + structured content (e.g. {"$TASK_BRIEF": brief_text}). Returns: The full text output from the agent. @@ -102,6 +131,9 @@ def call_agent(agent_name: str, skill_name: str, *args: str, stream: bool = True agent_meta, agent_body = _parse_frontmatter(agent_path.read_text(encoding="utf-8")) _, skill_body = _parse_frontmatter(skill_path.read_text(encoding="utf-8")) + if substitutions: + for placeholder, value in substitutions.items(): + skill_body = skill_body.replace(placeholder, value) arguments_str = " ".join(args) skill_body = skill_body.replace("$ARGUMENTS", arguments_str) @@ -110,9 +142,14 @@ def call_agent(agent_name: str, skill_name: str, *args: str, stream: bool = True raw_model = agent_meta.get("model", "sonnet") model = MODEL_MAP.get(raw_model, raw_model) + cmd = ["claude", "-p", prompt, "--model", model] + tools = agent_meta.get("tools") + if isinstance(tools, list) and tools: + cmd += ["--allowedTools", ",".join(tools)] + try: proc = subprocess.Popen( - ["claude", "-p", prompt, "--model", model], + cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, @@ -197,10 +234,20 @@ def main() -> None: try: print(f"Researcher is planning work for {work_item_id}...", flush=True) - call_agent("researcher", "researcher-plan", work_item_id, spec_path) + brief = call_agent("researcher", "researcher-plan", work_item_id, spec_path) except (FileNotFoundError, RuntimeError, subprocess.CalledProcessError) as e: print(f"Error invoking researcher agent:\n{e}", file=sys.stderr) sys.exit(1) + try: + print(f"Developer is implementing {work_item_id}...", flush=True) + call_agent( + "developer", "developer-implement", work_item_id, + substitutions={"$TASK_BRIEF": brief}, + ) + except (FileNotFoundError, RuntimeError, subprocess.CalledProcessError) as e: + print(f"Error invoking developer agent:\n{e}", file=sys.stderr) + sys.exit(1) + if __name__ == "__main__": main() diff --git a/.claude/settings.local.json b/.claude/settings.local.json index 9fc7de5f..6f7fcd2d 100644 --- a/.claude/settings.local.json +++ b/.claude/settings.local.json @@ -13,7 +13,8 @@ "Bash(curl -s http://localhost:4566/_localstack/health)", "Bash(git add *)", "Bash(gh pr *)", - "Bash(python -u .claude/scripts/dev_team.py ADR-171)" + "Bash(python -u .claude/scripts/dev_team.py ADR-171)", + "Monitor" ] } } diff --git a/CLAUDE.md b/CLAUDE.md index 5889e767..ab61cfc3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -68,6 +68,17 @@ Example: `TiVoService_InitializeAsync_WaitsForTiVoLocator` Use AAA (Arrange-Act-Assert). Use `[TestInitialize]` for mock setup and `[TestCleanup]` for mock verification. Group setup calls into `Expect_*` helper methods. +### Mocks +- Create `Mock` objects as `private readonly` fields on the test class so they are shared + across setup helpers, test methods, and verification. +- Always use `MockBehavior.Strict` to catch unexpected calls. +- Wrap each `Mock.Setup` chain in an `Expect__On(dependency, ...)` helper method for + readability and resilience to dependency shape changes. + +### `CreateSut()` +Always add a `CreateSut()` method that constructs the subject under test. When the constructor +gains a new dependency, only `CreateSut()` needs to change. + ### Async / Task patterns - Use `TaskCompletionSource` to represent a task that remains incomplete (e.g., simulating a hanging async operation). @@ -75,6 +86,15 @@ mock verification. Group setup calls into `Expect_*` helper methods. `.Should().BeCanceled()`, `.Should().BeFaultedWith(ex)`. - Do not `await` tasks in unit tests when you intend to verify synchronous completion; assert on the Task object directly instead. +- For every `async` method on a dependency, cover all of the following scenarios: + - Returns completed task → code under test continues + - Returns incomplete task → code under test awaits (stays incomplete) + - Incomplete task then completes → code under test resumes + - `CancellationToken` fires, throws `TaskCanceledException` → returned task `IsCanceled` + - `CancellationToken` fires, dependency returns complete → returned task `IsCanceled` + - `CancellationToken` fires, dependency stays incomplete → code stays incomplete + - Dependency throws directly (no `Task` returned) → exception propagates + - Dependency returns faulted `Task` → propagates ### E2E tests Prefer the Headless host for new E2E tests — cross-platform, no display required: From 475960c1c772cb22855f916db47b7b2b06cc05fd Mon Sep 17 00:00:00 2001 From: Joe Davis Date: Tue, 19 May 2026 15:27:15 -0700 Subject: [PATCH 03/14] Rentrant state machine for dev_team.py --- .claude/commands/dev-team.md | 17 + .claude/commands/developer-fix.md | 18 +- .claude/scripts/dev_team.py | 495 ++++++++++++++++++++++++++++-- .claude/scripts/workflow.md | 14 + .gitignore | 3 + CLAUDE.md | 1 + 6 files changed, 509 insertions(+), 39 deletions(-) create mode 100644 .claude/scripts/workflow.md diff --git a/.claude/commands/dev-team.md b/.claude/commands/dev-team.md index aa0df815..14713dc5 100644 --- a/.claude/commands/dev-team.md +++ b/.claude/commands/dev-team.md @@ -7,6 +7,21 @@ argument-hint: $ARGUMENTS +## Role + +Your only job is to start the pipeline script and relay its output to the user. The script +is the orchestrator — it drives every phase (research, implementation, build, test, fixes). +You are a passive observer. + +**Never attempt to:** +- Fix build errors or test failures +- Edit source files or test files +- Invoke agent skills directly (researcher-plan, developer-implement, developer-fix, etc.) +- Take any action in response to failures reported in the script output + +If the script exits with an error, report the final output to the user and stop. Do not +attempt recovery. + ## Steps 1. Check the platform: @@ -27,3 +42,5 @@ python -u .claude/scripts/dev_team.py $ARGUMENTS - **anything else**: `tail -f ` Stream all output to the user as it arrives until the process exits. + +4. When the process exits, report its exit status to the user. Take no further action. diff --git a/.claude/commands/developer-fix.md b/.claude/commands/developer-fix.md index 507716ca..c9ec5848 100644 --- a/.claude/commands/developer-fix.md +++ b/.claude/commands/developer-fix.md @@ -7,13 +7,21 @@ argument-hint: Work item ID: `$ARGUMENTS` -The following context must be embedded in the prompt by the caller: +### Original task brief -- **Original task brief** — the full prose brief produced by `researcher-plan` -- **Original work summary** — the structured prose summary produced by `developer-implement` -- **Issues to fix** — a prose list of build errors, test failures, or code review comments +$TASK_BRIEF -All three are required. If any are missing, stop and tell the caller what is needed. +--- + +### Work summary (all prior implementation and fix rounds) + +$WORK_SUMMARIES + +--- + +### Issues to fix + +$ISSUES --- diff --git a/.claude/scripts/dev_team.py b/.claude/scripts/dev_team.py index 27c35393..3cd023c1 100644 --- a/.claude/scripts/dev_team.py +++ b/.claude/scripts/dev_team.py @@ -2,12 +2,21 @@ """dev-team pipeline orchestrator. Entry point: main() — accepts a Jira work item ID, finds the matching spec file, -and invokes the researcher agent with the researcher-plan skill. +and runs the dev-team pipeline. Reentrant: if a context file exists from a prior +interrupted run, execution resumes from the last completed state. + +To start fresh, delete the context file: + .claude/logs/dev-team/-context.md """ +import datetime import json +import shutil import subprocess import sys +import threading +from abc import ABC, abstractmethod +from dataclasses import dataclass, field from pathlib import Path MODEL_MAP = { @@ -16,6 +25,317 @@ "opus": "claude-opus-4-7", } +MAX_FIX_ITERATIONS = 5 + + +# --------------------------------------------------------------------------- +# State machine +# --------------------------------------------------------------------------- + +TRANSITIONS: dict[str, dict[str, str]] = { + "init": {"start": "researching"}, + "researching": {"research_done": "implementing"}, + "implementing": {"impl_done": "validating"}, + "validating": { + "build_failed": "fixing", + "tests_failed": "fixing", + "clean": "done", + }, + "fixing": { + "fix_done": "validating", + "max_retries": "failed", + }, +} + +TERMINAL_STATES = {"done", "failed"} + + +class StateMachine: + """Simple state machine backed by a dict-of-dicts transition table.""" + + def __init__(self, transitions: dict[str, dict[str, str]], initial: str) -> None: + self._transitions = transitions + self.state = initial + + def transition(self, trigger: str) -> str: + """Advance to the next state via trigger. Returns new state.""" + available = self._transitions.get(self.state, {}) + if trigger not in available: + raise ValueError( + f"Invalid trigger '{trigger}' from state '{self.state}'. " + f"Available: {list(available)}" + ) + self.state = available[trigger] + return self.state + + +# --------------------------------------------------------------------------- +# Pipeline context (serializable state) +# --------------------------------------------------------------------------- + +@dataclass +class PipelineContext: + """All mutable state for a dev-team pipeline run, persisted across resumptions.""" + + work_item_id: str + spec_path: str + state: str = "init" + brief: str = "" + work_summaries: list[str] = field(default_factory=list) + fix_iteration: int = 0 + last_failure: str = "" + started: datetime.datetime = field(default_factory=datetime.datetime.now) + last_updated: datetime.datetime = field(default_factory=datetime.datetime.now) + + def save(self, path: Path) -> None: + """Write context to a markdown file with YAML frontmatter and named sections.""" + self.last_updated = datetime.datetime.now() + + lines = [ + "---", + f"state: {self.state}", + f"work_item_id: {self.work_item_id}", + f"spec_path: {self.spec_path}", + f"fix_iteration: {self.fix_iteration}", + f"started: {self.started.isoformat()}", + f"last_updated: {self.last_updated.isoformat()}", + "---", + "", + f"# {self.work_item_id} Dev Team Context", + ] + + if self.brief: + lines += ["", "## Researcher Brief", "", self.brief.strip()] + + if self.work_summaries: + lines += ["", "## Implementation Summary", "", self.work_summaries[0].strip()] + for i, summary in enumerate(self.work_summaries[1:], start=1): + lines += ["", f"## Fix {i}", "", summary.strip()] + + if self.last_failure: + lines += ["", "## Last Failure", "", self.last_failure.strip()] + + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text("\n".join(lines) + "\n", encoding="utf-8") + + @classmethod + def load(cls, path: Path) -> "PipelineContext": + """Load context from a markdown file previously written by save().""" + text = path.read_text(encoding="utf-8") + meta, body = _parse_frontmatter(text) + + ctx = cls( + work_item_id=meta.get("work_item_id", ""), + spec_path=meta.get("spec_path", ""), + state=meta.get("state", "init"), + fix_iteration=int(meta.get("fix_iteration", 0)), + ) + + try: + ctx.started = datetime.datetime.fromisoformat(meta["started"]) + ctx.last_updated = datetime.datetime.fromisoformat(meta["last_updated"]) + except (KeyError, ValueError): + pass + + sections = _parse_sections(body) + ctx.brief = sections.get("Researcher Brief", "") + + work_summaries: list[str] = [] + if "Implementation Summary" in sections: + work_summaries.append(sections["Implementation Summary"]) + i = 1 + while f"Fix {i}" in sections: + work_summaries.append(sections[f"Fix {i}"]) + i += 1 + ctx.work_summaries = work_summaries + ctx.last_failure = sections.get("Last Failure", "") + + return ctx + + +def _parse_sections(body: str) -> dict[str, str]: + """Split a markdown body into {heading: content} by '## ' headings.""" + sections: dict[str, str] = {} + current_heading: str | None = None + current_lines: list[str] = [] + + for line in body.split("\n"): + if line.startswith("## "): + if current_heading is not None: + sections[current_heading] = "\n".join(current_lines).strip() + current_heading = line[3:].strip() + current_lines = [] + elif current_heading is not None: + current_lines.append(line) + + if current_heading is not None: + sections[current_heading] = "\n".join(current_lines).strip() + + return sections + + +# --------------------------------------------------------------------------- +# Steps +# --------------------------------------------------------------------------- + +class Step(ABC): + """A single phase of the dev-team pipeline.""" + + handles: str # state name this step is responsible for + + @abstractmethod + def run(self, ctx: PipelineContext) -> str: + """Execute step logic (or skip if already done). Returns a trigger name.""" + ... + + +class ResearchStep(Step): + handles = "researching" + + def run(self, ctx: PipelineContext) -> str: + if ctx.brief: + print("Research already complete in context — skipping.", flush=True) + return "research_done" + print(f"Researcher is planning work for {ctx.work_item_id}...", flush=True) + try: + ctx.brief = call_agent( + "researcher", "researcher-plan", + ctx.work_item_id, ctx.spec_path, + ) + except (FileNotFoundError, RuntimeError, subprocess.CalledProcessError) as e: + print(f"Error invoking researcher agent:\n{e}", file=sys.stderr) + sys.exit(1) + return "research_done" + + +class ImplementStep(Step): + handles = "implementing" + + def run(self, ctx: PipelineContext) -> str: + if ctx.work_summaries: + print("Implementation already complete in context — skipping.", flush=True) + return "impl_done" + print(f"Developer is implementing {ctx.work_item_id}...", flush=True) + try: + impl_summary = call_agent( + "developer", "developer-implement", ctx.work_item_id, + substitutions={"$TASK_BRIEF": ctx.brief}, + ) + except (FileNotFoundError, RuntimeError, subprocess.CalledProcessError) as e: + print(f"Error invoking developer agent:\n{e}", file=sys.stderr) + sys.exit(1) + ctx.work_summaries.append(impl_summary) + return "impl_done" + + +class ValidateStep(Step): + handles = "validating" + + def run(self, ctx: PipelineContext) -> str: + print("Running scripts/validate-build...", flush=True) + try: + build_ok, build_output = run_validate_script("validate-build") + except (FileNotFoundError, OSError) as e: + print(f"Error running validate-build:\n{e}", file=sys.stderr) + sys.exit(1) + + if not build_ok: + print("Build failed. Invoking developer to fix...", flush=True) + ctx.last_failure = f"Build failed:\n\n{build_output}" + return "build_failed" + + print("Running scripts/validate-tests...", flush=True) + try: + tests_ok, tests_output = run_validate_script("validate-tests") + except (FileNotFoundError, OSError) as e: + print(f"Error running validate-tests:\n{e}", file=sys.stderr) + sys.exit(1) + + if not tests_ok: + print("Tests failed. Invoking developer to fix...", flush=True) + ctx.last_failure = f"Test failures:\n\n{tests_output}" + return "tests_failed" + + print("Build and tests clean.", flush=True) + ctx.last_failure = "" + return "clean" + + +class FixStep(Step): + handles = "fixing" + + def run(self, ctx: PipelineContext) -> str: + if ctx.fix_iteration >= MAX_FIX_ITERATIONS: + print( + f"Error: still failing after {MAX_FIX_ITERATIONS} fix iterations. " + f"Manual intervention needed.", + file=sys.stderr, + ) + return "max_retries" + print( + f"Invoking developer to fix " + f"(iteration {ctx.fix_iteration + 1} of {MAX_FIX_ITERATIONS})...", + flush=True, + ) + try: + fix_summary = call_agent( + "developer", "developer-fix", ctx.work_item_id, + substitutions={ + "$TASK_BRIEF": ctx.brief, + "$WORK_SUMMARIES": _format_work_summaries(ctx.work_summaries), + "$ISSUES": ctx.last_failure, + }, + ) + except (FileNotFoundError, RuntimeError, subprocess.CalledProcessError) as e: + print(f"Error invoking developer-fix agent:\n{e}", file=sys.stderr) + sys.exit(1) + ctx.fix_iteration += 1 + ctx.work_summaries.append(fix_summary) + return "fix_done" + + +# --------------------------------------------------------------------------- +# Pipeline +# --------------------------------------------------------------------------- + +class DevTeamPipeline: + """Drives the dev-team state machine from init (or a resumed state) to done.""" + + def __init__(self, ctx: PipelineContext, context_path: Path) -> None: + self.ctx = ctx + self.context_path = context_path + self.machine = StateMachine(TRANSITIONS, initial=ctx.state) + self.step_handlers: dict[str, Step] = { + "researching": ResearchStep(), + "implementing": ImplementStep(), + "validating": ValidateStep(), + "fixing": FixStep(), + } + + def run(self) -> None: + if self.machine.state == "init": + self.machine.transition("start") + self.ctx.state = self.machine.state + self.ctx.save(self.context_path) + + while self.machine.state not in TERMINAL_STATES: + step = self.step_handlers.get(self.machine.state) + if step is None: + raise RuntimeError(f"No handler for state '{self.machine.state}'") + + trigger = step.run(self.ctx) + + self.machine.transition(trigger) + self.ctx.state = self.machine.state + self.ctx.save(self.context_path) + + if self.machine.state == "failed": + sys.exit(1) + + +# --------------------------------------------------------------------------- +# Utilities (unchanged from original) +# --------------------------------------------------------------------------- def _find_repo_root() -> Path: """Walk up from this file until a directory containing .claude/ is found.""" @@ -35,6 +355,27 @@ def _find_repo_root() -> Path: REPO_ROOT = _find_repo_root() +def _resolve_claude() -> list[str]: + """Return the command prefix needed to invoke the claude CLI. + + On Windows, claude is often a .cmd batch wrapper. CreateProcess can't run + .cmd files directly — they need cmd.exe. shutil.which resolves the full + path (respecting PATHEXT), and we wrap with cmd /c if needed. + """ + path = shutil.which("claude") + if path is None: + raise RuntimeError( + "claude CLI not found on PATH. " + "Ensure Claude Code is installed and claude.exe is accessible." + ) + if sys.platform == "win32" and path.lower().endswith(".cmd"): + return ["cmd", "/c", path] + return [path] + + +CLAUDE_CMD = _resolve_claude() + + def _parse_frontmatter(text: str) -> tuple[dict, str]: """Split YAML frontmatter from body. Returns (metadata_dict, body). @@ -66,7 +407,6 @@ def _parse_frontmatter(text: str) -> tuple[dict, str]: key = key.strip() value = value.strip() if not value: - # Collect YAML list items on the following indented lines. items: list[str] = [] j = i + 1 while j < len(frontmatter_lines): @@ -142,33 +482,67 @@ def call_agent( raw_model = agent_meta.get("model", "sonnet") model = MODEL_MAP.get(raw_model, raw_model) - cmd = ["claude", "-p", prompt, "--model", model] + # Pass -p without an argument so claude runs in print mode reading from stdin. + # Embedding the prompt inline as "-p " hits the Windows 32 767-char + # CreateProcess limit once build/test output fills $ISSUES. + cmd = CLAUDE_CMD + [ + "-p", "--model", model, + "--output-format", "stream-json", "--verbose", + ] tools = agent_meta.get("tools") if isinstance(tools, list) and tools: cmd += ["--allowedTools", ",".join(tools)] + timestamp = datetime.datetime.now().strftime("%Y%m%dT%H%M%S") + log_dir = REPO_ROOT / ".claude" / "logs" / "dev-team" + log_dir.mkdir(parents=True, exist_ok=True) + log_path = log_dir / f"{timestamp}-{agent_name}-{skill_name}.jsonl" + try: proc = subprocess.Popen( cmd, + stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, cwd=REPO_ROOT, ) - except FileNotFoundError: + except (FileNotFoundError, OSError) as exc: raise RuntimeError( - "claude CLI not found on PATH. " - "Ensure Claude Code is installed and claude.exe is accessible." - ) - - chunks: list[str] = [] - for line in proc.stdout: # type: ignore[union-attr] - if stream: - print(line, end="", flush=True) - chunks.append(line) + f"Failed to start claude CLI: {exc}" + ) from exc + + # Write the prompt to stdin in a background thread. Without the thread the + # pipe buffer fills up before we start reading stdout, causing a deadlock. + def _write_prompt() -> None: + try: + proc.stdin.write(prompt) # type: ignore[union-attr] + proc.stdin.close() # type: ignore[union-attr] + except BrokenPipeError: + pass + + stdin_thread = threading.Thread(target=_write_prompt, daemon=True) + stdin_thread.start() + + result_text: str = "" + with log_path.open("w", encoding="utf-8") as log_file: + for line in proc.stdout: # type: ignore[union-attr] + log_file.write(line) + log_file.flush() + try: + event = json.loads(line) + except json.JSONDecodeError: + continue + if event.get("type") == "result" and event.get("subtype") == "success": + result_text = event.get("result", "") + if stream: + print(result_text, flush=True) proc.wait() stderr_text = proc.stderr.read() # type: ignore[union-attr] + if stderr_text: + with log_path.open("a", encoding="utf-8") as log_file: + log_file.write(json.dumps({"type": "stderr", "text": stderr_text}) + "\n") if proc.returncode != 0: raise subprocess.CalledProcessError( @@ -177,7 +551,56 @@ def call_agent( stderr=f"{stderr_text}\n(exit code {proc.returncode})", ) - return "".join(chunks) + return result_text + + +def _format_work_summaries(summaries: list[str]) -> str: + """Format one or more work summaries for embedding in a developer-fix prompt.""" + if len(summaries) == 1: + return summaries[0] + parts = [] + for i, s in enumerate(summaries, start=1): + label = "Implementation summary" if i == 1 else f"Fix summary {i - 1}" + parts.append(f"### {label}\n\n{s.strip()}") + return "\n\n---\n\n".join(parts) + + +def run_validate_script(script_name: str) -> tuple[bool, str]: + """Run a validate script from the scripts/ directory. + + Selects the platform-appropriate extension (.cmd on Windows, .sh elsewhere). + Streams output to stdout and returns (success, combined_output). + """ + scripts_dir = REPO_ROOT / "scripts" + ext = ".cmd" if sys.platform == "win32" else ".sh" + script_path = scripts_dir / f"{script_name}{ext}" + + if not script_path.exists(): + raise FileNotFoundError( + f"Validate script not found: scripts/{script_name}{ext}" + ) + + if sys.platform == "win32": + invoke = ["cmd", "/c", str(script_path)] + else: + invoke = [str(script_path)] + + proc = subprocess.Popen( + invoke, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + text=True, + cwd=REPO_ROOT, + ) + + chunks: list[str] = [] + with proc.stdout: # type: ignore[union-attr] + for line in proc.stdout: + print(line, end="", flush=True) + chunks.append(line) + + proc.wait() + return proc.returncode == 0, "".join(chunks) def find_spec_file(work_item_id: str) -> Path: @@ -217,6 +640,10 @@ def find_spec_file(work_item_id: str) -> Path: return matches[0] +# --------------------------------------------------------------------------- +# Entry point +# --------------------------------------------------------------------------- + def main() -> None: if len(sys.argv) < 2: print( @@ -227,27 +654,27 @@ def main() -> None: work_item_id = sys.argv[1] - print(f"Searching for spec for {work_item_id}", flush=True) - spec_file = find_spec_file(work_item_id) - spec_path = str(spec_file.relative_to(REPO_ROOT)) - print(f"Found {spec_file}", flush=True) - - try: - print(f"Researcher is planning work for {work_item_id}...", flush=True) - brief = call_agent("researcher", "researcher-plan", work_item_id, spec_path) - except (FileNotFoundError, RuntimeError, subprocess.CalledProcessError) as e: - print(f"Error invoking researcher agent:\n{e}", file=sys.stderr) - sys.exit(1) + log_dir = REPO_ROOT / ".claude" / "logs" / "dev-team" + log_dir.mkdir(parents=True, exist_ok=True) + context_path = log_dir / f"{work_item_id}-context.md" + + if context_path.exists(): + ctx = PipelineContext.load(context_path) + if ctx.state in TERMINAL_STATES: + print(f"Previous run ended with state '{ctx.state}'.") + print(f"Delete {context_path} to run again.") + sys.exit(0 if ctx.state == "done" else 1) + print(f"Resuming {work_item_id} from state '{ctx.state}'...", flush=True) + else: + print(f"Searching for spec for {work_item_id}", flush=True) + spec_file = find_spec_file(work_item_id) + spec_path = str(spec_file.relative_to(REPO_ROOT)) + print(f"Found {spec_file}", flush=True) + ctx = PipelineContext(work_item_id=work_item_id, spec_path=spec_path) + ctx.save(context_path) + + DevTeamPipeline(ctx, context_path).run() - try: - print(f"Developer is implementing {work_item_id}...", flush=True) - call_agent( - "developer", "developer-implement", work_item_id, - substitutions={"$TASK_BRIEF": brief}, - ) - except (FileNotFoundError, RuntimeError, subprocess.CalledProcessError) as e: - print(f"Error invoking developer agent:\n{e}", file=sys.stderr) - sys.exit(1) if __name__ == "__main__": main() diff --git a/.claude/scripts/workflow.md b/.claude/scripts/workflow.md new file mode 100644 index 00000000..21096082 --- /dev/null +++ b/.claude/scripts/workflow.md @@ -0,0 +1,14 @@ +```mermaid +stateDiagram-v2 + [*] --> init + init --> researching : start + researching --> implementing : research_done + implementing --> validating : impl_done + validating --> fixing : build_failed + validating --> fixing : tests_failed + validating --> done : clean + fixing --> validating : fix_done + fixing --> failed : max_retries + done --> [*] + failed --> [*] +``` diff --git a/.gitignore b/.gitignore index 878783b5..d25aeb1e 100644 --- a/.gitignore +++ b/.gitignore @@ -365,3 +365,6 @@ MigrationBackup/ # Fody - auto-generated XML schema FodyWeavers.xsd + +# Claude logs +.claude/logs \ No newline at end of file diff --git a/CLAUDE.md b/CLAUDE.md index ab61cfc3..9ca4eb43 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -52,6 +52,7 @@ Event IDs are organized in ranges by subsystem: | 1200–1299 | RawLayoutService (backend) | | 1300–1699 | (reserved — App subsystems: ProgrammaticSettings, ScopedBackgroundProcess, ConversationState, SamplesRecorder, TestEndpointService, CognitoTokenService) | | 1700–1799 | LayoutProcessingService (backend) | +| 1800–1899 | LayoutCompilerService (backend) | Assign new log messages the next unused ID in the appropriate range. When replacing an existing message, use exact text including whitespace, newlines, and punctuation. From 8310b5647f7fbc78dcfe26a0833cbf85b115f5dc Mon Sep 17 00:00:00 2001 From: Joe Davis Date: Wed, 20 May 2026 06:53:55 -0700 Subject: [PATCH 04/14] Add reviewer persona and skills, plus refactor code guidelines into one place that can be updated. --- .claude/agents/developer.md | 8 +- .claude/agents/reviewer.md | 64 +++++ .claude/commands/dev-team.md | 2 +- .claude/commands/developer-fix.md | 28 +- .claude/commands/developer-implement.md | 21 +- .claude/commands/developer-patterns.md | 55 +--- .claude/commands/reviewer-pr-review.md | 50 ++++ .claude/commands/reviewer-review.md | 130 +++++++++ .claude/commands/reviewer-sign-off.md | 78 ++++++ .claude/scripts/dev_team.py | 301 ++++++++++++++++++--- .claude/scripts/implementation-pipeline.md | 23 ++ CLAUDE.md | 78 +----- CONTRIBUTING.md | 141 +++++++++- 13 files changed, 806 insertions(+), 173 deletions(-) create mode 100644 .claude/agents/reviewer.md create mode 100644 .claude/commands/reviewer-pr-review.md create mode 100644 .claude/commands/reviewer-review.md create mode 100644 .claude/commands/reviewer-sign-off.md create mode 100644 .claude/scripts/implementation-pipeline.md diff --git a/.claude/agents/developer.md b/.claude/agents/developer.md index f162278d..910eb735 100644 --- a/.claude/agents/developer.md +++ b/.claude/agents/developer.md @@ -32,9 +32,9 @@ Jira or GitHub. Your deliverable is code and a work summary. ## Before writing any code -Invoke the `developer-patterns` skill to load architectural and design patterns. Read -`CLAUDE.md` for project-wide conventions (logging, test structure, quality gates, project -layout). These apply to everything you write. +Read `CONTRIBUTING.md` for all code guidelines and patterns: logging, test structure, async +design, testable state, E2E conventions, and project layout. Read `CLAUDE.md` for quality +gates and operational conventions. These apply to everything you write. ## Scope discipline @@ -62,7 +62,7 @@ Before reporting done, review the diff as if you are doing a code review: - Does the implementation match the brief's exit criteria? - Are there missing test cases? -- Do all files follow CLAUDE.md naming and structure conventions? +- Do all files follow CONTRIBUTING.md naming and structure conventions? - Is there any scope creep? ## Output format diff --git a/.claude/agents/reviewer.md b/.claude/agents/reviewer.md new file mode 100644 index 00000000..6303301d --- /dev/null +++ b/.claude/agents/reviewer.md @@ -0,0 +1,64 @@ +--- +name: reviewer +description: > + Reviewer agent for the AdaptiveRemote development team. Reviews code changes for + correctness, performance, security, and compliance with the task brief. Creates GitHub + PRs and posts structured review comments. Read-only on source code; only writes to + GitHub review threads. +model: sonnet +tools: + - Read + - Glob + - Grep + - Skill +--- + +You are the Reviewer for the AdaptiveRemote development team. + +## Role + +Your job is to review code changes and ensure they meet quality, correctness, security, +and requirements standards. You create GitHub PRs and post review comments. You never +modify source files — your only output is GitHub review comments and a structured JSON +result. + +## Before reviewing any code + +Read `CONTRIBUTING.md` for all code guidelines and patterns. These are the standards +against which you review. Read the relevant `_doc_*.md` files for any subsystem touched +by the change. + +## Review priorities + +Evaluate code changes in this priority order. Post inline comments for substantive issues. +Note style issues but do not block approval on style alone. + +1. **Requirements** — every exit criterion from the task brief is met +2. **Correctness and fault tolerance** — exception paths handled; no silent failures; + `CancellationToken` passed everywhere async; no blocking calls in async code +3. **Security** — no injection risks; no sensitive data logged or exposed; auth is checked + at system boundaries +4. **Performance** — no N+1 patterns; no synchronous I/O on hot paths; no unnecessary + allocations in loops +5. **Documentation** — new code conforms to the relevant `_doc_*.md` architecture files; + if a design changed, the doc is updated to match +6. **Code style** (low priority) — naming conventions, `[LoggerMessage]` usage, + `MockBehavior.Strict`, test structure from CONTRIBUTING.md + +## Output format + +After posting the GitHub PR review, output a human-readable summary of the issues found +(for use by the developer if changes are requested), followed by a JSON result on its own +line as the final output: + +```json +{"status": "approved|changes_requested", "pr_url": "https://github.com/..."} +``` + +## Skills + +Use the `Skill` tool to invoke your task-specific workflows: + +- `reviewer-review` — first-pass review: create PR if needed, review all changes, post comments +- `reviewer-sign-off` — sign-off pass: check resolved comments, scan modified files for regressions +- `reviewer-pr-review` — respond to human reviewer comments on a PR diff --git a/.claude/commands/dev-team.md b/.claude/commands/dev-team.md index 14713dc5..193ef586 100644 --- a/.claude/commands/dev-team.md +++ b/.claude/commands/dev-team.md @@ -33,7 +33,7 @@ python -c "import sys; print(sys.platform)" 2. Start the pipeline script in the background: ```bash -python -u .claude/scripts/dev_team.py $ARGUMENTS +python -u .claude/scripts/dev_team.py $ARGUMENTS --workflow .claude/scripts/implementation-pipeline.md ``` 3. **Immediately** call the Monitor tool on the background process to stream its output. diff --git a/.claude/commands/developer-fix.md b/.claude/commands/developer-fix.md index c9ec5848..d51e89af 100644 --- a/.claude/commands/developer-fix.md +++ b/.claude/commands/developer-fix.md @@ -29,7 +29,8 @@ $ISSUES ### 1 — Load standards -Invoke the `developer-patterns` skill. Read `CLAUDE.md` for project-wide conventions. +Invoke the `developer-patterns` skill (loads all guidelines from `CONTRIBUTING.md`). +Read `CLAUDE.md` for quality gates and operational conventions. ### 2 — Understand context @@ -51,13 +52,26 @@ For each issue: ### 4 — Fix each issue -Address issues one at a time. After each fix, build and test the affected code to confirm -the fix works without introducing new failures: +Address issues one at a time. After each fix: -```bash -dotnet build -dotnet test -``` +1. Build and test the affected code to confirm the fix works without introducing new failures: + + ```bash + dotnet build + dotnet test + ``` + +2. Commit the fix immediately with a message describing the specific issue resolved: + + ```bash + git add -A + git commit -m "$ARGUMENTS: " + ``` + + One commit per issue keeps the git history readable and makes individual fixes easy to + review. Do not batch multiple fixes into a single commit. + +Do not push — the pipeline pushes after all fixes pass full validation. ### 5 — Self-review diff --git a/.claude/commands/developer-implement.md b/.claude/commands/developer-implement.md index b8c762b4..973587f1 100644 --- a/.claude/commands/developer-implement.md +++ b/.claude/commands/developer-implement.md @@ -17,8 +17,8 @@ $TASK_BRIEF ### 1 — Load standards -Invoke the `developer-patterns` skill. Read `CLAUDE.md` for project-wide conventions -(logging, test structure, quality gates, project layout). +Invoke the `developer-patterns` skill (loads all guidelines from `CONTRIBUTING.md`). +Read `CLAUDE.md` for quality gates and operational conventions. ### 2 — Understand the task @@ -63,7 +63,20 @@ dotnet test Fix any build errors or new test failures before moving to the next layer. -### 6 — Self-review +### 6 — Commit + +Commit all changes with a clear message that describes what was implemented and why: + +```bash +git add -A +git commit -m "$ARGUMENTS: " +``` + +The message body (optional) can list the key decisions if they are non-obvious. + +Do not push — the pipeline pushes after validation passes. + +### 7 — Self-review Review the diff as if you were doing a code review: @@ -72,7 +85,7 @@ Review the diff as if you were doing a code review: - Do all files follow CLAUDE.md naming, structure, and logging conventions? - Is there any scope creep — changes not required by the brief? -### 7 — Report +### 8 — Report Return a work summary as structured prose: diff --git a/.claude/commands/developer-patterns.md b/.claude/commands/developer-patterns.md index 4f3ee642..108d03fc 100644 --- a/.claude/commands/developer-patterns.md +++ b/.claude/commands/developer-patterns.md @@ -1,53 +1,8 @@ --- -description: Architectural and design patterns for the AdaptiveRemote developer. Read this before writing any application or test code. Covers async design, testable state, E2E test authoring, and project layout — patterns not already in CLAUDE.md. +description: Load code guidelines and architectural patterns before writing any code. Redirects to CONTRIBUTING.md, which is the single source of truth for all coding conventions, async design, testable state, E2E test authoring, and project layout. --- -Read CLAUDE.md for project-wide test conventions (naming, AAA structure, Moq setup, CreateSut, -async task patterns, logging, quality gates). The patterns below are supplemental. - ---- - -## Async in application code - -- Fetch async-backed data up front before entering processing-heavy code. Don't scatter async - calls through processing logic just to retrieve data on demand — fetch first, process second. -- Always include a `CancellationToken` parameter in every async API. Never provide a default - value — callers must explicitly decide to pass `CancellationToken.None`. - -## Testable state design - -- For services with significant internal state, extract that state into a Data object or - ViewModel. The service acts on the object; the object's fields are directly settable and - readable in tests — no reflection or internal-visible hacks needed. -- Views (Blazor components, WPF controls) must be minimal: only enough to display and interact - with the ViewModel. All logic goes in a controller service that manipulates the ViewModel and - handles its change events. Because controller services have no UI dependencies, they can be - fully unit-tested. - -## E2E / API tests (Gherkin) - -Write Gherkin scenarios before implementing. Use existing steps whenever possible. - -New steps must be generalized, not single-purpose: - -- `When` = action a human could perform; `Then` = result a human could verify; - `Given` = state a human could set up. A failed test should be reproducible manually - without internal knowledge of the implementation. -- Step definitions: argument/state validation plus a single test service call. No logic in - step definitions — put it in test service methods. -- Step definitions and test service methods are never `async`. Use - `WaitHelper.WaitForAsyncTask` to block on async calls. - - **Exception:** interprocess test services communicate over JSON-RPC and are inherently - async. Use the provided synchronous extension methods that wrap - `WaitHelpers.WaitForAsyncTask` with appropriate timeouts. -- All state-verification steps use `WaitHelpers.ExecuteWithRetries` to poll until the - expected state is observed or a timeout expires. Action steps that may not succeed on the - first attempt should also use `WaitHelpers.ExecuteWithRetries`. Never write a manual retry - loop. -- Timeouts unblock a test that will never pass. They are not performance assertions — set - them long enough that a correct, unloaded system would always succeed. - -## Project layout - -Before creating new files, read `src/_doc_Projects.md` and the relevant `_doc_Services.md` -to confirm the correct project and folder for each new file. +Read `CONTRIBUTING.md` for all code guidelines and architectural patterns: testing +conventions, logging rules, async design (fetch-first, `CancellationToken`), testable +state design (Data objects / ViewModels, controller services), E2E / API test patterns +(Gherkin, `WaitHelpers`, step-definition rules), and project layout guidance. diff --git a/.claude/commands/reviewer-pr-review.md b/.claude/commands/reviewer-pr-review.md new file mode 100644 index 00000000..6919d52e --- /dev/null +++ b/.claude/commands/reviewer-pr-review.md @@ -0,0 +1,50 @@ +--- +description: Respond to human reviewer comments on a GitHub PR. Reads unresolved human-authored review comments, identifies new guidelines, asks clarifying questions via the PR thread, and outputs suggested additions to CONTRIBUTING.md. Used in the human-in-the-loop review pipeline (not dev-team). +--- + +You are responding to human review comments on PR $PR_URL for work item $WORK_ITEM_ID. + +--- + +## Step 1 — Read the PR review comments + +Using the GitHub MCP, fetch all review comments on the PR. Identify which comments were +left by a human reviewer (not the automated reviewer agent). Focus on unresolved threads. + +## Step 2 — Categorise each comment + +For each human comment, determine: + +1. **Is it a project guideline?** Does the comment point out something that should + always be done (or never done) in this codebase? If so, it is a candidate for + `CONTRIBUTING.md`. + +2. **Is it task-specific?** Does the comment only apply to this particular change and + would not generalise to other code? If so, it is not a guideline candidate. + +3. **Is it ambiguous?** Does the comment require clarification before you can determine + whether it is a guideline? If so, post a question. + +## Step 3 — Ask clarifying questions + +For any comment that is ambiguous, post a reply in the PR thread via the GitHub MCP asking +a specific, narrow question. For example: + +- "Is this a project-wide requirement, or specific to this service?" +- "Should this pattern apply to all async methods, or only public APIs?" + +Do not ask questions about comments you can already categorise clearly. + +## Step 4 — Output suggested guidelines + +For each comment that is clearly a project guideline, output a suggested addition to +`CONTRIBUTING.md` in this format: + +--- +**Section:** [which Code Guidelines section this belongs in] +**Suggested addition:** +> [exact text to add, written in the same style as existing CONTRIBUTING.md guidelines] +**Rationale:** [why this should be a permanent guideline] +--- + +Do not modify `CONTRIBUTING.md` directly. Present the suggestions for human approval first. diff --git a/.claude/commands/reviewer-review.md b/.claude/commands/reviewer-review.md new file mode 100644 index 00000000..a897eacb --- /dev/null +++ b/.claude/commands/reviewer-review.md @@ -0,0 +1,130 @@ +--- +description: First-pass code review for the AdaptiveRemote dev-team pipeline. Creates a GitHub PR if one does not exist, retrieves the PR diff, reviews all changes against requirements and quality criteria, and posts a GitHub PR review. Outputs a JSON result indicating approved or changes_requested. +--- + +You are performing the first-pass code review for work item $WORK_ITEM_ID. + +**Task brief:** +$TASK_BRIEF + +**PR URL (empty = not yet created):** $PR_URL + +**Spec file path:** $SPEC_PATH + +--- + +## Step 1 — Load guidelines + +Read `CONTRIBUTING.md` in full. This is the authoritative reference for all code +conventions you will evaluate. + +## Step 2 — Understand the requirements + +Re-read the task brief above and extract the exit criteria — the explicit list of things +the implementation must do or must not do. You will check each one during review. + +Read the relevant `_doc_*.md` architecture files for any subsystem touched by this change. +Use the area→file table in `CLAUDE.md` to find them. + +## Step 3 — Create the PR if needed + +If `$PR_URL` is empty: + +1. Determine the current branch name (`git branch --show-current` or read `.git/HEAD`) +2. Use the GitHub MCP to create a pull request with: + - A clear, descriptive title (summarises what the work item implements) + - A body that includes: the work item ID, a summary of what changed, and any notable + design decisions from the implementation +3. Record the PR URL — you will include it in your output JSON + +If `$PR_URL` is already set, use that PR for the review. + +## Step 4 — Retrieve and read the diff + +Use the GitHub MCP to fetch the PR diff. Read all changed files in full to understand the +complete context of each change. + +## Step 5 — Review the changes + +Evaluate the diff against each dimension below, in priority order. For each issue you +find, note the file, line number, and a clear description of the problem. + +### Priority 1 — Requirements + +Check each exit criterion from the task brief: +- Is it implemented? +- Is it implemented correctly (not just partially)? +- Are there edge cases the brief implied but the implementation misses? + +### Priority 2 — Correctness and fault tolerance + +- Are all exception paths handled? No swallowed exceptions, no empty `catch` blocks. +- Are `CancellationToken` parameters present in every async method signature? No default + values — callers must pass explicitly. +- Are there blocking calls (`.Result`, `.Wait()`, `Thread.Sleep`) on async code paths? +- Does error handling propagate faithfully, or does it silently discard failures? + +### Priority 3 — Security + +- Is user input validated at system boundaries? +- Are there SQL injection, command injection, or path traversal risks? +- Is sensitive data (tokens, passwords, PII) logged or returned in error messages? +- Are authentication/authorization checks present where the architecture requires them? + +### Priority 4 — Performance + +- Are there N+1 query patterns (fetching inside a loop that could be batched)? +- Is there synchronous I/O on async code paths? +- Are there unnecessary allocations in hot loops (string concatenation, LINQ on every + call, etc.)? +- Are async-backed data fetches happening up front (fetch-first pattern) rather than + scattered through processing logic? + +### Priority 5 — Documentation + +- Does new code conform to the design described in the relevant `_doc_*.md` files? +- If the implementation changed the design (new interface, changed responsibility, + new dependency), has the relevant `_doc_*.md` been updated? + +### Priority 6 — Code style (note, do not block) + +- Do naming conventions follow CONTRIBUTING.md (`ClassName_Method_Scenario_ExpectedResult` + for tests, etc.)? +- Do log messages use `[LoggerMessage]` source-generated methods? +- Do tests use `MockBehavior.Strict` and `Expect_*` helpers? +- Is there a `CreateSut()` method? + +## Step 6 — Post the GitHub PR review + +Use the GitHub MCP to create a **pull request review** (not a regular PR comment). A +review consists of: + +1. A collection of **review comments** — each attached to a specific file and line in + the diff. Create one review comment per issue found. Each comment must: + - Reference the exact file path and diff line + - Explain what the problem is and why it matters (not just what to change) + - For Priority 6 style issues, prefix with "nit:" to signal non-blocking + +2. A **review submission** with event type: + - `APPROVE` — if no Priority 1–5 blocking issues were found + - `REQUEST_CHANGES` — if any Priority 1–5 issue was found + +The GitHub API for this is `POST /repos/{owner}/{repo}/pulls/{pull_number}/reviews`. +The MCP tool that wraps this creates a review with inline comments in a single call. +Do NOT use the regular PR comment endpoint (`POST /repos/{owner}/{repo}/issues/{number}/comments`) +— that creates a plain conversation comment, not a structured review thread. + +## Step 7 — Output + +Write a concise plain-text summary of all issues found (one bullet per issue, Priority +1–5 first, style issues last). This summary will be passed to the developer so they can +address each point without re-reading the full PR thread. + +Then output the JSON result as the final line: + +```json +{"status": "approved|changes_requested", "pr_url": "https://github.com/..."} +``` + +Use `"approved"` if no Priority 1–5 issues were found; `"changes_requested"` otherwise. +Always include the PR URL even when approving. diff --git a/.claude/commands/reviewer-sign-off.md b/.claude/commands/reviewer-sign-off.md new file mode 100644 index 00000000..ddf83649 --- /dev/null +++ b/.claude/commands/reviewer-sign-off.md @@ -0,0 +1,78 @@ +--- +description: Sign-off review for the AdaptiveRemote dev-team pipeline. After the developer has addressed review comments, checks whether each previously requested change has been resolved and scans modified files for new issues. Outputs a JSON result indicating approved or changes_requested. +--- + +You are performing a sign-off review for work item $WORK_ITEM_ID. + +**Task brief:** +$TASK_BRIEF + +**PR URL:** $PR_URL + +--- + +## Step 1 — Load guidelines + +Read `CONTRIBUTING.md` in full. These are the standards against which you are reviewing. + +## Step 2 — Push latest changes to remote + +The developer has made fixes since the last review. Ensure the latest commits are visible +on the remote before reviewing. Check `git status` and `git log --oneline -5` to understand +what has changed since the last push. + +## Step 3 — Retrieve prior review threads + +Using the GitHub MCP, fetch all existing review comments on the PR at `$PR_URL`. For each +unresolved thread, note: +- What was the original issue? +- What file and line was it on? +- Has that file/line been modified since the comment was posted? + +## Step 4 — Check each prior thread for resolution + +For each unresolved review comment: + +1. Read the relevant section of the latest code +2. Determine whether the issue has been adequately addressed: + - **Resolved:** the problem no longer exists in the code. Resolve the thread via the + GitHub MCP and note it as resolved in your output summary. + - **Partially addressed:** the developer made a change but the underlying problem + remains or a different instance was missed. Add a follow-up comment explaining what + still needs to be done. + - **Not addressed:** the code is unchanged. Add a follow-up comment restating what is + needed and why, more clearly if the original comment was ambiguous. + +## Step 5 — Scan modified files for new issues + +Identify all files that were modified since the last review push (use the PR diff or git +log). Scan **only those files** for new issues introduced by the developer's fix — do not +re-review unmodified code. + +Apply the same priority order as the first-pass review: +1. Requirements, 2. Correctness/fault tolerance, 3. Security, 4. Performance, +5. Documentation, 6. Code style (note only) + +Post new inline review comments for any new Priority 1–5 issues found in the modified +files. + +## Step 6 — Submit the sign-off review + +Use the GitHub MCP to submit a **pull request review** (not a plain PR comment) via +`POST /repos/{owner}/{repo}/pulls/{pull_number}/reviews`. Any new issues should be +inline review comments attached to the specific file and line. Submit with event type: +- **APPROVE** if all prior threads are resolved (or resolved via this pass) and no new + Priority 1–5 issues were found in the modified files +- **REQUEST_CHANGES** otherwise + +## Step 7 — Output + +Write a concise summary: +- List each prior comment and whether it was resolved or still needs work +- List any new issues found in the modified files + +Then output the JSON result as the final line: + +```json +{"status": "approved|changes_requested"} +``` diff --git a/.claude/scripts/dev_team.py b/.claude/scripts/dev_team.py index 3cd023c1..e76e0dc0 100644 --- a/.claude/scripts/dev_team.py +++ b/.claude/scripts/dev_team.py @@ -9,8 +9,10 @@ .claude/logs/dev-team/-context.md """ +import argparse import datetime import json +import re import shutil import subprocess import sys @@ -26,28 +28,76 @@ } MAX_FIX_ITERATIONS = 5 +MAX_REVIEW_FIX_ITERATIONS = 3 # --------------------------------------------------------------------------- -# State machine +# Workflow definition (parsed from a Mermaid stateDiagram-v2 file) # --------------------------------------------------------------------------- -TRANSITIONS: dict[str, dict[str, str]] = { - "init": {"start": "researching"}, - "researching": {"research_done": "implementing"}, - "implementing": {"impl_done": "validating"}, - "validating": { - "build_failed": "fixing", - "tests_failed": "fixing", - "clean": "done", - }, - "fixing": { - "fix_done": "validating", - "max_retries": "failed", - }, -} +@dataclass +class WorkflowDefinition: + transitions: dict[str, dict[str, str]] + terminal_states: set[str] + initial_state: str + + +def parse_workflow(path: Path) -> WorkflowDefinition: + """Parse a Mermaid stateDiagram-v2 block from a markdown file. + + Recognises three line forms inside the diagram: + [*] --> StateA → initial state + StateA --> [*] → terminal state + StateA --> StateB : t → transition with trigger t + """ + text = path.read_text(encoding="utf-8") + + # Extract the first ```mermaid ... ``` fenced block. + match = re.search(r"```mermaid\s*\n(.*?)```", text, re.DOTALL) + if not match: + raise ValueError(f"No mermaid fenced block found in {path}") + + transitions: dict[str, dict[str, str]] = {} + terminal_states: set[str] = set() + initial_state: str | None = None + + for raw_line in match.group(1).splitlines(): + line = raw_line.strip() + if "-->" not in line: + continue + + # StateA --> [*] (terminal) + m = re.match(r"^([\w-]+)\s+-->\s+\[\*\]$", line) + if m: + terminal_states.add(m.group(1)) + continue + + # [*] --> StateA (initial) + m = re.match(r"^\[\*\]\s+-->\s+([\w-]+)$", line) + if m: + initial_state = m.group(1) + continue + + # StateA --> StateB : trigger + m = re.match(r"^([\w-]+)\s+-->\s+([\w-]+)\s*:\s*([\w-]+)$", line) + if m: + src, dst, trigger = m.group(1), m.group(2), m.group(3) + transitions.setdefault(src, {})[trigger] = dst + continue + + if initial_state is None: + raise ValueError(f"No initial state ([*] --> ...) found in {path}") + + return WorkflowDefinition( + transitions=transitions, + terminal_states=terminal_states, + initial_state=initial_state, + ) + -TERMINAL_STATES = {"done", "failed"} +# --------------------------------------------------------------------------- +# State machine +# --------------------------------------------------------------------------- class StateMachine: @@ -83,6 +133,9 @@ class PipelineContext: brief: str = "" work_summaries: list[str] = field(default_factory=list) fix_iteration: int = 0 + review_fix_iteration: int = 0 + pr_url: str = "" + review_notes: str = "" last_failure: str = "" started: datetime.datetime = field(default_factory=datetime.datetime.now) last_updated: datetime.datetime = field(default_factory=datetime.datetime.now) @@ -97,6 +150,8 @@ def save(self, path: Path) -> None: f"work_item_id: {self.work_item_id}", f"spec_path: {self.spec_path}", f"fix_iteration: {self.fix_iteration}", + f"review_fix_iteration: {self.review_fix_iteration}", + f"pr_url: {self.pr_url}", f"started: {self.started.isoformat()}", f"last_updated: {self.last_updated.isoformat()}", "---", @@ -112,6 +167,9 @@ def save(self, path: Path) -> None: for i, summary in enumerate(self.work_summaries[1:], start=1): lines += ["", f"## Fix {i}", "", summary.strip()] + if self.review_notes: + lines += ["", "## Review Notes", "", self.review_notes.strip()] + if self.last_failure: lines += ["", "## Last Failure", "", self.last_failure.strip()] @@ -129,6 +187,8 @@ def load(cls, path: Path) -> "PipelineContext": spec_path=meta.get("spec_path", ""), state=meta.get("state", "init"), fix_iteration=int(meta.get("fix_iteration", 0)), + review_fix_iteration=int(meta.get("review_fix_iteration", 0)), + pr_url=meta.get("pr_url", ""), ) try: @@ -148,6 +208,7 @@ def load(cls, path: Path) -> "PipelineContext": work_summaries.append(sections[f"Fix {i}"]) i += 1 ctx.work_summaries = work_summaries + ctx.review_notes = sections.get("Review Notes", "") ctx.last_failure = sections.get("Last Failure", "") return ctx @@ -174,6 +235,60 @@ def _parse_sections(body: str) -> dict[str, str]: return sections +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def _commit_and_push(work_item_id: str) -> None: + """Push the current branch. The developer is expected to have already committed. + + As a safety net, stages and commits any uncommitted changes left by the developer + before pushing. The pipeline never pushes until validation is clean, so partial + or broken work is never sent to the remote. + """ + try: + subprocess.run(["git", "add", "-A"], check=True, cwd=REPO_ROOT, capture_output=True) + diff = subprocess.run( + ["git", "diff", "--cached", "--quiet"], cwd=REPO_ROOT, capture_output=True, + ) + if diff.returncode != 0: + subprocess.run( + ["git", "commit", "-m", f"{work_item_id}: uncommitted changes at validation"], + check=True, cwd=REPO_ROOT, capture_output=True, + ) + subprocess.run( + ["git", "push", "origin", "HEAD"], + check=True, cwd=REPO_ROOT, capture_output=True, + ) + except subprocess.CalledProcessError as e: + print(f"Warning: git commit/push failed (continuing): {e.stderr}", flush=True) + + +def parse_json_output(text: str) -> dict: + """Extract the last parseable JSON object from agent output text. + + Tries single-line JSON objects from the end of the text first, then + fenced code blocks. Returns an empty dict if nothing parses. + """ + for line in reversed(text.strip().splitlines()): + line = line.strip() + if line.startswith("{") and line.endswith("}"): + try: + return json.loads(line) + except json.JSONDecodeError: + continue + + for block in reversed(re.findall(r"```(?:json)?\s*\n([\s\S]*?)\n```", text)): + try: + result = json.loads(block.strip()) + if isinstance(result, dict): + return result + except json.JSONDecodeError: + continue + + return {} + + # --------------------------------------------------------------------------- # Steps # --------------------------------------------------------------------------- @@ -256,11 +371,72 @@ def run(self, ctx: PipelineContext) -> str: ctx.last_failure = f"Test failures:\n\n{tests_output}" return "tests_failed" - print("Build and tests clean.", flush=True) + print("Build and tests clean. Committing and pushing...", flush=True) ctx.last_failure = "" + _commit_and_push(ctx.work_item_id) return "clean" +class ReviewStep(Step): + handles = "reviewing" + + def run(self, ctx: PipelineContext) -> str: + print(f"Reviewer is reviewing {ctx.work_item_id}...", flush=True) + try: + output = call_agent( + "reviewer", "reviewer-review", + substitutions={ + "$WORK_ITEM_ID": ctx.work_item_id, + "$SPEC_PATH": ctx.spec_path, + "$TASK_BRIEF": ctx.brief, + "$PR_URL": ctx.pr_url, + }, + ) + except (FileNotFoundError, RuntimeError, subprocess.CalledProcessError) as e: + print(f"Error invoking reviewer agent:\n{e}", file=sys.stderr) + sys.exit(1) + + result = parse_json_output(output) + if result.get("pr_url"): + ctx.pr_url = result["pr_url"] + ctx.review_notes = output + + status = result.get("status", "changes_requested") + if status == "approved": + print("Review approved.", flush=True) + return "approved" + print("Reviewer requested changes.", flush=True) + return "changes_requested" + + +class SignOffStep(Step): + handles = "reviewing-signoff" + + def run(self, ctx: PipelineContext) -> str: + print(f"Reviewer is signing off on {ctx.work_item_id}...", flush=True) + try: + output = call_agent( + "reviewer", "reviewer-sign-off", + substitutions={ + "$WORK_ITEM_ID": ctx.work_item_id, + "$TASK_BRIEF": ctx.brief, + "$PR_URL": ctx.pr_url, + }, + ) + except (FileNotFoundError, RuntimeError, subprocess.CalledProcessError) as e: + print(f"Error invoking reviewer agent:\n{e}", file=sys.stderr) + sys.exit(1) + + ctx.review_notes = output + result = parse_json_output(output) + status = result.get("status", "changes_requested") + if status == "approved": + print("Sign-off approved.", flush=True) + return "approved" + print("Sign-off requested further changes.", flush=True) + return "changes_requested" + + class FixStep(Step): handles = "fixing" @@ -294,6 +470,44 @@ def run(self, ctx: PipelineContext) -> str: return "fix_done" +class FixPrStep(Step): + handles = "fixing-pr" + + def run(self, ctx: PipelineContext) -> str: + if ctx.review_fix_iteration >= MAX_REVIEW_FIX_ITERATIONS: + print( + f"Error: still failing review after {MAX_REVIEW_FIX_ITERATIONS} " + f"review fix iterations. Manual intervention needed.", + file=sys.stderr, + ) + return "max_retries" + print( + f"Invoking developer to address review comments " + f"(iteration {ctx.review_fix_iteration + 1} of {MAX_REVIEW_FIX_ITERATIONS})...", + flush=True, + ) + issues = ( + f"Code review changes requested on PR {ctx.pr_url}.\n\n" + f"Read the open review threads on the PR and address each one.\n\n" + f"Reviewer summary:\n{ctx.review_notes}" + ) + try: + fix_summary = call_agent( + "developer", "developer-fix", ctx.work_item_id, + substitutions={ + "$TASK_BRIEF": ctx.brief, + "$WORK_SUMMARIES": _format_work_summaries(ctx.work_summaries), + "$ISSUES": issues, + }, + ) + except (FileNotFoundError, RuntimeError, subprocess.CalledProcessError) as e: + print(f"Error invoking developer-fix agent:\n{e}", file=sys.stderr) + sys.exit(1) + ctx.review_fix_iteration += 1 + ctx.work_summaries.append(fix_summary) + return "fix_done" + + # --------------------------------------------------------------------------- # Pipeline # --------------------------------------------------------------------------- @@ -301,24 +515,31 @@ def run(self, ctx: PipelineContext) -> str: class DevTeamPipeline: """Drives the dev-team state machine from init (or a resumed state) to done.""" - def __init__(self, ctx: PipelineContext, context_path: Path) -> None: + def __init__(self, ctx: PipelineContext, context_path: Path, workflow: WorkflowDefinition) -> None: self.ctx = ctx self.context_path = context_path - self.machine = StateMachine(TRANSITIONS, initial=ctx.state) + self.workflow = workflow + self.machine = StateMachine(workflow.transitions, initial=ctx.state) + validate_step = ValidateStep() self.step_handlers: dict[str, Step] = { "researching": ResearchStep(), "implementing": ImplementStep(), - "validating": ValidateStep(), + "validating": validate_step, + "validating-pr": validate_step, "fixing": FixStep(), + "reviewing": ReviewStep(), + "reviewing-signoff": SignOffStep(), + "fixing-pr": FixPrStep(), } def run(self) -> None: - if self.machine.state == "init": - self.machine.transition("start") + if self.machine.state == self.workflow.initial_state: + boot_trigger = next(iter(self.workflow.transitions[self.workflow.initial_state])) + self.machine.transition(boot_trigger) self.ctx.state = self.machine.state self.ctx.save(self.context_path) - while self.machine.state not in TERMINAL_STATES: + while self.machine.state not in self.workflow.terminal_states: step = self.step_handlers.get(self.machine.state) if step is None: raise RuntimeError(f"No handler for state '{self.machine.state}'") @@ -505,6 +726,7 @@ def call_agent( stdout=subprocess.PIPE, stderr=subprocess.PIPE, text=True, + encoding="utf-8", cwd=REPO_ROOT, ) except (FileNotFoundError, OSError) as exc: @@ -645,14 +867,26 @@ def find_spec_file(work_item_id: str) -> Path: # --------------------------------------------------------------------------- def main() -> None: - if len(sys.argv) < 2: - print( - "Usage: dev_team.py (e.g. dev_team.py ADR-172)", - file=sys.stderr, - ) - sys.exit(1) + parser = argparse.ArgumentParser( + prog="dev_team.py", + description="dev-team pipeline orchestrator", + ) + parser.add_argument("work_item_id", metavar="work-item-id", + help="Jira work item ID (e.g. ADR-172)") + parser.add_argument("--workflow", required=True, metavar="path", + help="Path to a Mermaid stateDiagram-v2 workflow file") + args = parser.parse_args() + + work_item_id = args.work_item_id + workflow_path = Path(args.workflow) + if not workflow_path.is_absolute(): + workflow_path = REPO_ROOT / workflow_path - work_item_id = sys.argv[1] + try: + workflow = parse_workflow(workflow_path) + except (FileNotFoundError, ValueError) as e: + print(f"Error loading workflow: {e}", file=sys.stderr) + sys.exit(1) log_dir = REPO_ROOT / ".claude" / "logs" / "dev-team" log_dir.mkdir(parents=True, exist_ok=True) @@ -660,7 +894,7 @@ def main() -> None: if context_path.exists(): ctx = PipelineContext.load(context_path) - if ctx.state in TERMINAL_STATES: + if ctx.state in workflow.terminal_states: print(f"Previous run ended with state '{ctx.state}'.") print(f"Delete {context_path} to run again.") sys.exit(0 if ctx.state == "done" else 1) @@ -670,10 +904,11 @@ def main() -> None: spec_file = find_spec_file(work_item_id) spec_path = str(spec_file.relative_to(REPO_ROOT)) print(f"Found {spec_file}", flush=True) - ctx = PipelineContext(work_item_id=work_item_id, spec_path=spec_path) + ctx = PipelineContext(work_item_id=work_item_id, spec_path=spec_path, + state=workflow.initial_state) ctx.save(context_path) - DevTeamPipeline(ctx, context_path).run() + DevTeamPipeline(ctx, context_path, workflow).run() if __name__ == "__main__": diff --git a/.claude/scripts/implementation-pipeline.md b/.claude/scripts/implementation-pipeline.md new file mode 100644 index 00000000..03c1be19 --- /dev/null +++ b/.claude/scripts/implementation-pipeline.md @@ -0,0 +1,23 @@ +```mermaid +stateDiagram-v2 + [*] --> init + init --> researching : start + researching --> implementing : research_done + implementing --> validating : impl_done + validating --> fixing : build_failed + validating --> fixing : tests_failed + validating --> reviewing : clean + reviewing --> done : approved + reviewing --> fixing-pr : changes_requested + fixing-pr --> validating-pr : fix_done + validating-pr --> fixing-pr : build_failed + validating-pr --> fixing-pr : tests_failed + validating-pr --> reviewing-signoff : clean + reviewing-signoff --> done : approved + reviewing-signoff --> fixing-pr : changes_requested + fixing --> validating : fix_done + fixing --> failed : max_retries + fixing-pr --> failed : max_retries + done --> [*] + failed --> [*] +``` diff --git a/CLAUDE.md b/CLAUDE.md index 9ca4eb43..9e60cf80 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -2,6 +2,8 @@ AdaptiveRemote is an accessible remote control for TV/AV equipment built for users with limited or total loss of mobility. Accessibility is the primary design constraint: vision accessibility first, speech recognition second, eye-gaze input third. +> If you are planning, writing, or reviewing code, read the code guidelines in `CONTRIBUTING.md`. + ## Read Before Making Changes Read the `_doc_*.md` file for any area you plan to modify: @@ -30,81 +32,13 @@ Read the `_doc_*.md` file for any area you plan to modify: ## Logging -All log messages are defined as `[LoggerMessage]` source-generated methods in -`src/AdaptiveRemote.App/Logging/MessageLogger.cs`. Never call `logger.LogXxx()` directly — -add new methods to `MessageLogger` instead. - -Event IDs are organized in ranges by subsystem: - -| Range | Subsystem | -|-------|-----------| -| 100–199 | SpeechRecognitionEngine | -| 200–299 | ConversationController | -| 300–399 | SpeechRecognition | -| 400–499 | SpeechSynthesis | -| 500–599 | ListeningController | -| 600–699 | CommandService | -| 700–799 | ApplicationLifecycle | -| 800–899 | TiVoConnection | -| 900–999 | UdpService | -| 1000–1099 | BroadlinkCommandService | -| 1100–1199 | CompiledLayoutService (backend) | -| 1200–1299 | RawLayoutService (backend) | -| 1300–1699 | (reserved — App subsystems: ProgrammaticSettings, ScopedBackgroundProcess, ConversationState, SamplesRecorder, TestEndpointService, CognitoTokenService) | -| 1700–1799 | LayoutProcessingService (backend) | -| 1800–1899 | LayoutCompilerService (backend) | - -Assign new log messages the next unused ID in the appropriate range. When replacing an existing -message, use exact text including whitespace, newlines, and punctuation. - -In tests, verify log output using `MockLogger.VerifyMessages(log => { log.MethodName(...); })`. +See `CONTRIBUTING.md` for logging conventions (`[LoggerMessage]` source-gen, event ID +ranges, test verification with `MockLogger.VerifyMessages`). ## Testing -### Naming -`ClassName_Method_Scenario_ExpectedResult` -Example: `TiVoService_InitializeAsync_WaitsForTiVoLocator` - -### Structure -Use AAA (Arrange-Act-Assert). Use `[TestInitialize]` for mock setup and `[TestCleanup]` for -mock verification. Group setup calls into `Expect_*` helper methods. - -### Mocks -- Create `Mock` objects as `private readonly` fields on the test class so they are shared - across setup helpers, test methods, and verification. -- Always use `MockBehavior.Strict` to catch unexpected calls. -- Wrap each `Mock.Setup` chain in an `Expect__On(dependency, ...)` helper method for - readability and resilience to dependency shape changes. - -### `CreateSut()` -Always add a `CreateSut()` method that constructs the subject under test. When the constructor -gains a new dependency, only `CreateSut()` needs to change. - -### Async / Task patterns -- Use `TaskCompletionSource` to represent a task that remains incomplete (e.g., simulating a - hanging async operation). -- Assert task state without `await`: `.Should().BeComplete()`, `.Should().NotBeComplete()`, - `.Should().BeCanceled()`, `.Should().BeFaultedWith(ex)`. -- Do not `await` tasks in unit tests when you intend to verify synchronous completion; assert - on the Task object directly instead. -- For every `async` method on a dependency, cover all of the following scenarios: - - Returns completed task → code under test continues - - Returns incomplete task → code under test awaits (stays incomplete) - - Incomplete task then completes → code under test resumes - - `CancellationToken` fires, throws `TaskCanceledException` → returned task `IsCanceled` - - `CancellationToken` fires, dependency returns complete → returned task `IsCanceled` - - `CancellationToken` fires, dependency stays incomplete → code stays incomplete - - Dependency throws directly (no `Task` returned) → exception propagates - - Dependency returns faulted `Task` → propagates - -### E2E tests -Prefer the Headless host for new E2E tests — cross-platform, no display required: - -```bash -dotnet build src/AdaptiveRemote.Headless/AdaptiveRemote.Headless.csproj -pwsh src/AdaptiveRemote.Headless/bin/Debug/net10.0/playwright.ps1 install chromium # if tests crash at startup with a JSON-RPC disconnect -dotnet test --filter "FullyQualifiedName~Host.Headless" -``` +See `CONTRIBUTING.md` for test naming, structure, mock setup, `CreateSut()`, async +scenario matrix, and E2E (Gherkin/Headless) conventions. ## Documentation diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 02bfa19a..116fafa4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,7 +1,9 @@ # Contributing to AdaptiveRemote +> If you are planning, writing, or reviewing code, read the [Code Guidelines](#code-guidelines) section below. + Thank you for your interest in contributing to AdaptiveRemote! This project aims to provide -accessible remote control solutions for users with limited or no mobility, with a focus on +accessible remote control solutions for users with limited or no mobility, with a focus on vision accessibility, speech recognition, and eye-gaze input. ## How to Contribute @@ -36,11 +38,146 @@ templates for [bug reports](../.github/ISSUE_TEMPLATE/bug_report.md) and - Use clear, descriptive commit messages that explain what and why you changed. - No formal convention is required, but clarity is appreciated. - **Supported Platforms:** - - The application targets Windows OS and .NET8. + - The application targets Windows OS and .NET 10. - Required NuGet packages are restored during build. - **Contact:** - For questions or support, open an issue and @jodavis. +--- + +## Code Guidelines + +### Testing + +#### Naming + +`ClassName_Method_Scenario_ExpectedResult` + +Example: `TiVoService_InitializeAsync_WaitsForTiVoLocator` + +#### Structure + +Use AAA (Arrange-Act-Assert). Use `[TestInitialize]` for mock setup and `[TestCleanup]` for +mock verification. Group setup calls into `Expect_*` helper methods. + +#### Mocks + +- Create `Mock` objects as `private readonly` fields on the test class so they are shared + across setup helpers, test methods, and verification. +- Always use `MockBehavior.Strict` to catch unexpected calls. +- Wrap each `Mock.Setup` chain in an `Expect__On(dependency, ...)` helper method for + readability and resilience to dependency shape changes. + +#### `CreateSut()` + +Always add a `CreateSut()` method that constructs the subject under test. When the constructor +gains a new dependency, only `CreateSut()` needs to change. + +#### Async / Task patterns + +- Use `TaskCompletionSource` to represent a task that remains incomplete (e.g., simulating a + hanging async operation). +- Assert task state without `await`: `.Should().BeComplete()`, `.Should().NotBeComplete()`, + `.Should().BeCanceled()`, `.Should().BeFaultedWith(ex)`. +- Do not `await` tasks in unit tests when you intend to verify synchronous completion; assert + on the Task object directly instead. +- For every `async` method on a dependency, cover all of the following scenarios: + - Returns completed task → code under test continues + - Returns incomplete task → code under test awaits (stays incomplete) + - Incomplete task then completes → code under test resumes + - `CancellationToken` fires, throws `TaskCanceledException` → returned task `IsCanceled` + - `CancellationToken` fires, dependency returns complete → returned task `IsCanceled` + - `CancellationToken` fires, dependency stays incomplete → code stays incomplete + - Dependency throws directly (no `Task` returned) → exception propagates + - Dependency returns faulted `Task` → propagates + +#### E2E tests (Gherkin) + +Prefer the Headless host for new E2E tests — cross-platform, no display required: + +```bash +dotnet build src/AdaptiveRemote.Headless/AdaptiveRemote.Headless.csproj +pwsh src/AdaptiveRemote.Headless/bin/Debug/net10.0/playwright.ps1 install chromium +dotnet test --filter "FullyQualifiedName~Host.Headless" +``` + +Write Gherkin scenarios before implementing. Use existing steps whenever possible. + +New steps must be generalized, not single-purpose: + +- `When` = action a human could perform; `Then` = result a human could verify; + `Given` = state a human could set up. A failed test should be reproducible manually + without internal knowledge of the implementation. +- Step definitions: argument/state validation plus a single test service call. No logic in + step definitions — put it in test service methods. +- Step definitions and test service methods are never `async`. Use + `WaitHelper.WaitForAsyncTask` to block on async calls. + - **Exception:** interprocess test services communicate over JSON-RPC and are inherently + async. Use the provided synchronous extension methods that wrap + `WaitHelpers.WaitForAsyncTask` with appropriate timeouts. +- All state-verification steps use `WaitHelpers.ExecuteWithRetries` to poll until the + expected state is observed or a timeout expires. Action steps that may not succeed on the + first attempt should also use `WaitHelpers.ExecuteWithRetries`. Never write a manual retry + loop. +- Timeouts unblock a test that will never pass. They are not performance assertions — set + them long enough that a correct, unloaded system would always succeed. + +### Logging + +All log messages are defined as `[LoggerMessage]` source-generated methods in +`src/AdaptiveRemote.App/Logging/MessageLogger.cs`. Never call `logger.LogXxx()` directly — +add new methods to `MessageLogger` instead. + +Event IDs are organized in ranges by subsystem: + +| Range | Subsystem | +|-------|-----------| +| 100–199 | SpeechRecognitionEngine | +| 200–299 | ConversationController | +| 300–399 | SpeechRecognition | +| 400–499 | SpeechSynthesis | +| 500–599 | ListeningController | +| 600–699 | CommandService | +| 700–799 | ApplicationLifecycle | +| 800–899 | TiVoConnection | +| 900–999 | UdpService | +| 1000–1099 | BroadlinkCommandService | +| 1100–1199 | CompiledLayoutService (backend) | +| 1200–1299 | RawLayoutService (backend) | +| 1300–1699 | (reserved — App subsystems: ProgrammaticSettings, ScopedBackgroundProcess, ConversationState, SamplesRecorder, TestEndpointService, CognitoTokenService) | +| 1700–1799 | LayoutProcessingService (backend) | +| 1800–1899 | LayoutCompilerService (backend) | + +Assign new log messages the next unused ID in the appropriate range. When replacing an +existing message, use exact text including whitespace, newlines, and punctuation. + +In tests, verify log output using `MockLogger.VerifyMessages(log => { log.MethodName(...); })`. + +### Async in Application Code + +- Fetch async-backed data up front before entering processing-heavy code. Don't scatter + async calls through processing logic just to retrieve data on demand — fetch first, + process second. +- Always include a `CancellationToken` parameter in every async API. Never provide a + default value — callers must explicitly decide to pass `CancellationToken.None`. + +### Testable State Design + +- For services with significant internal state, extract that state into a Data object or + ViewModel. The service acts on the object; the object's fields are directly settable and + readable in tests — no reflection or internal-visible hacks needed. +- Views (Blazor components, WPF controls) must be minimal: only enough to display and + interact with the ViewModel. All logic goes in a controller service that manipulates the + ViewModel and handles its change events. Because controller services have no UI + dependencies, they can be fully unit-tested. + +### Project Layout + +Before creating new files, read `src/_doc_Projects.md` and the relevant `_doc_Services.md` +to confirm the correct project and folder for each new file. + +--- + ## Code of Conduct This project follows the From 605d402e65cc5ca57318de250e1906997c01ffae Mon Sep 17 00:00:00 2001 From: Joe Davis Date: Thu, 21 May 2026 06:15:46 -0700 Subject: [PATCH 05/14] Write build and test output to logs instead of forwarding directly to stdout, to minimize log agent noise --- .claude/scripts/dev_team.py | 74 +++++++++++++++++++++++++++++-------- 1 file changed, 58 insertions(+), 16 deletions(-) diff --git a/.claude/scripts/dev_team.py b/.claude/scripts/dev_team.py index e76e0dc0..70efcda2 100644 --- a/.claude/scripts/dev_team.py +++ b/.claude/scripts/dev_team.py @@ -21,6 +21,13 @@ from dataclasses import dataclass, field from pathlib import Path +# Reconfigure stdout/stderr to UTF-8 early so that Unicode characters in agent +# output (e.g. arrows, bullets) don't crash on Windows cp1252 terminals. +if hasattr(sys.stdout, "reconfigure"): + sys.stdout.reconfigure(encoding="utf-8", errors="replace") +if hasattr(sys.stderr, "reconfigure"): + sys.stderr.reconfigure(encoding="utf-8", errors="replace") + MODEL_MAP = { "haiku": "claude-haiku-4-5-20251001", "sonnet": "claude-sonnet-4-6", @@ -137,6 +144,8 @@ class PipelineContext: pr_url: str = "" review_notes: str = "" last_failure: str = "" + build_log: str = "" + test_log: str = "" started: datetime.datetime = field(default_factory=datetime.datetime.now) last_updated: datetime.datetime = field(default_factory=datetime.datetime.now) @@ -152,6 +161,8 @@ def save(self, path: Path) -> None: f"fix_iteration: {self.fix_iteration}", f"review_fix_iteration: {self.review_fix_iteration}", f"pr_url: {self.pr_url}", + f"build_log: {self.build_log}", + f"test_log: {self.test_log}", f"started: {self.started.isoformat()}", f"last_updated: {self.last_updated.isoformat()}", "---", @@ -173,6 +184,14 @@ def save(self, path: Path) -> None: if self.last_failure: lines += ["", "## Last Failure", "", self.last_failure.strip()] + log_links: list[str] = [] + if self.build_log: + log_links.append(f"- Build: {self.build_log}") + if self.test_log: + log_links.append(f"- Tests: {self.test_log}") + if log_links: + lines += ["", "## Logs", ""] + log_links + path.parent.mkdir(parents=True, exist_ok=True) path.write_text("\n".join(lines) + "\n", encoding="utf-8") @@ -189,6 +208,8 @@ def load(cls, path: Path) -> "PipelineContext": fix_iteration=int(meta.get("fix_iteration", 0)), review_fix_iteration=int(meta.get("review_fix_iteration", 0)), pr_url=meta.get("pr_url", ""), + build_log=meta.get("build_log", ""), + test_log=meta.get("test_log", ""), ) try: @@ -349,29 +370,41 @@ class ValidateStep(Step): def run(self, ctx: PipelineContext) -> str: print("Running scripts/validate-build...", flush=True) try: - build_ok, build_output = run_validate_script("validate-build") + build_ok, build_log, build_tail = run_validate_script("validate-build") except (FileNotFoundError, OSError) as e: print(f"Error running validate-build:\n{e}", file=sys.stderr) sys.exit(1) + ctx.build_log = str(build_log) if not build_ok: - print("Build failed. Invoking developer to fix...", flush=True) - ctx.last_failure = f"Build failed:\n\n{build_output}" + print(f"Build FAILED. Log: {build_log}", flush=True) + ctx.last_failure = ( + f"Build failed.\n\n" + f"Full log (read this for details): {build_log}\n\n" + f"Last 30 lines:\n```\n{build_tail}\n```" + ) return "build_failed" + print(f"Build clean. Log: {build_log}", flush=True) + print("Running scripts/validate-tests...", flush=True) try: - tests_ok, tests_output = run_validate_script("validate-tests") + tests_ok, tests_log, tests_tail = run_validate_script("validate-tests") except (FileNotFoundError, OSError) as e: print(f"Error running validate-tests:\n{e}", file=sys.stderr) sys.exit(1) + ctx.test_log = str(tests_log) if not tests_ok: - print("Tests failed. Invoking developer to fix...", flush=True) - ctx.last_failure = f"Test failures:\n\n{tests_output}" + print(f"Tests FAILED. Log: {tests_log}", flush=True) + ctx.last_failure = ( + f"Test failures.\n\n" + f"Full log (read this for details): {tests_log}\n\n" + f"Last 30 lines:\n```\n{tests_tail}\n```" + ) return "tests_failed" - print("Build and tests clean. Committing and pushing...", flush=True) + print(f"Tests clean. Log: {tests_log}", flush=True) ctx.last_failure = "" _commit_and_push(ctx.work_item_id) return "clean" @@ -787,11 +820,12 @@ def _format_work_summaries(summaries: list[str]) -> str: return "\n\n---\n\n".join(parts) -def run_validate_script(script_name: str) -> tuple[bool, str]: +def run_validate_script(script_name: str) -> tuple[bool, Path, str]: """Run a validate script from the scripts/ directory. - Selects the platform-appropriate extension (.cmd on Windows, .sh elsewhere). - Streams output to stdout and returns (success, combined_output). + Logs full output to a timestamped file under .claude/logs/dev-team/. + Nothing is streamed to the console — callers print their own status line. + Returns (success, log_path, tail) where tail is the last 30 lines. """ scripts_dir = REPO_ROOT / "scripts" ext = ".cmd" if sys.platform == "win32" else ".sh" @@ -802,6 +836,11 @@ def run_validate_script(script_name: str) -> tuple[bool, str]: f"Validate script not found: scripts/{script_name}{ext}" ) + timestamp = datetime.datetime.now().strftime("%Y%m%dT%H%M%S") + log_dir = REPO_ROOT / ".claude" / "logs" / "dev-team" + log_dir.mkdir(parents=True, exist_ok=True) + log_path = log_dir / f"{timestamp}-{script_name}.txt" + if sys.platform == "win32": invoke = ["cmd", "/c", str(script_path)] else: @@ -812,17 +851,20 @@ def run_validate_script(script_name: str) -> tuple[bool, str]: stdout=subprocess.PIPE, stderr=subprocess.STDOUT, text=True, + encoding="utf-8", + errors="replace", cwd=REPO_ROOT, ) - chunks: list[str] = [] - with proc.stdout: # type: ignore[union-attr] - for line in proc.stdout: - print(line, end="", flush=True) - chunks.append(line) + lines: list[str] = [] + with log_path.open("w", encoding="utf-8") as log_file: + for line in proc.stdout: # type: ignore[union-attr] + log_file.write(line) + lines.append(line) proc.wait() - return proc.returncode == 0, "".join(chunks) + tail = "".join(lines[-30:]) + return proc.returncode == 0, log_path, tail def find_spec_file(work_item_id: str) -> Path: From 117519bddb89f48bca5f6fb99da7dde64bcd76f1 Mon Sep 17 00:00:00 2001 From: Joe Davis Date: Thu, 21 May 2026 05:22:47 -0700 Subject: [PATCH 06/14] Add todo list use to the agents --- .claude/agents/developer.md | 4 ++++ .claude/agents/researcher.md | 5 +++++ .claude/agents/reviewer.md | 5 +++++ 3 files changed, 14 insertions(+) diff --git a/.claude/agents/developer.md b/.claude/agents/developer.md index 910eb735..1404d1c5 100644 --- a/.claude/agents/developer.md +++ b/.claude/agents/developer.md @@ -75,6 +75,10 @@ Return a structured prose work summary so the Researcher can validate your work: - **Unit tests:** file path and test method names - **E2E scenarios:** feature file path and scenario titles +## Task tracking + +At the start of each task, call `TodoWrite` to create a task list reflecting the steps you plan to take. Mark each item `in_progress` when you start it and `completed` when you finish it. Keep the list accurate throughout. + ## Skills Use the `Skill` tool to invoke your task-specific workflows: diff --git a/.claude/agents/researcher.md b/.claude/agents/researcher.md index 4640fd08..64598298 100644 --- a/.claude/agents/researcher.md +++ b/.claude/agents/researcher.md @@ -12,6 +12,7 @@ tools: - WebSearch - WebFetch - Skill + - TodoWrite --- You are the Researcher for the AdaptiveRemote development team. @@ -59,6 +60,10 @@ Flag every ambiguity you find — don't resolve them by assumption. An unresolve included in your output is more valuable than a confident-sounding guess. Phrase ambiguities as concrete questions the Developer or user can answer. +## Task tracking + +At the start of each task, call `TodoWrite` to create a task list reflecting the steps you plan to take. Mark each item `in_progress` when you start it and `completed` when you finish it. Keep the list accurate throughout. + ## Skills Use the `Skill` tool to invoke your two task-specific workflows: diff --git a/.claude/agents/reviewer.md b/.claude/agents/reviewer.md index 6303301d..2956ecf7 100644 --- a/.claude/agents/reviewer.md +++ b/.claude/agents/reviewer.md @@ -11,6 +11,7 @@ tools: - Glob - Grep - Skill + - TodoWrite --- You are the Reviewer for the AdaptiveRemote development team. @@ -55,6 +56,10 @@ line as the final output: {"status": "approved|changes_requested", "pr_url": "https://github.com/..."} ``` +## Task tracking + +At the start of each task, call `TodoWrite` to create a task list reflecting the steps you plan to take. Mark each item `in_progress` when you start it and `completed` when you finish it. Keep the list accurate throughout. + ## Skills Use the `Skill` tool to invoke your task-specific workflows: From 8f7d23b81e4e56f33aec29396ce0030fccaa3ff3 Mon Sep 17 00:00:00 2001 From: Joe Davis Date: Thu, 21 May 2026 06:07:54 -0700 Subject: [PATCH 07/14] Add Jira and GitHub state management to the dev team --- .claude/agents/developer.md | 12 ++++++ .claude/agents/reviewer.md | 13 +++++++ .claude/commands/developer-implement.md | 7 ++++ .claude/commands/jira-update-failed.md | 31 +++++++++++++++ .claude/commands/reviewer-review.md | 7 ++++ .claude/commands/reviewer-sign-off.md | 9 +++++ .claude/scripts/dev_team.py | 52 ++++++++++++++++++++++++- 7 files changed, 129 insertions(+), 2 deletions(-) create mode 100644 .claude/commands/jira-update-failed.md diff --git a/.claude/agents/developer.md b/.claude/agents/developer.md index 1404d1c5..150062b5 100644 --- a/.claude/agents/developer.md +++ b/.claude/agents/developer.md @@ -17,6 +17,11 @@ tools: - Skill - WebSearch - WebFetch + - mcp__jira__atlassianUserInfo + - mcp__jira__getTransitionsForJiraIssue + - mcp__jira__transitionJiraIssue + - mcp__jira__editJiraIssue + - mcp__jira__addCommentToJiraIssue --- You are the Developer for the AdaptiveRemote development team. @@ -75,6 +80,13 @@ Return a structured prose work summary so the Researcher can validate your work: - **Unit tests:** file path and test method names - **E2E scenarios:** feature file path and scenario titles +## Work item management + +As you work, add brief comments to the Jira issue (`mcp__jira__addCommentToJiraIssue`) at +meaningful progress points — e.g. when tests are written, when implementation is complete, +when a fix resolves a specific failure. Keep comments concise (one sentence). The work item +ID is always provided in your task inputs. + ## Task tracking At the start of each task, call `TodoWrite` to create a task list reflecting the steps you plan to take. Mark each item `in_progress` when you start it and `completed` when you finish it. Keep the list accurate throughout. diff --git a/.claude/agents/reviewer.md b/.claude/agents/reviewer.md index 2956ecf7..f8afd4c6 100644 --- a/.claude/agents/reviewer.md +++ b/.claude/agents/reviewer.md @@ -12,6 +12,13 @@ tools: - Grep - Skill - TodoWrite + - mcp__jira__atlassianUserInfo + - mcp__jira__lookupJiraAccountId + - mcp__jira__getTransitionsForJiraIssue + - mcp__jira__transitionJiraIssue + - mcp__jira__editJiraIssue + - mcp__jira__addCommentToJiraIssue + - mcp__github__add_pull_request_review_request --- You are the Reviewer for the AdaptiveRemote development team. @@ -56,6 +63,12 @@ line as the final output: {"status": "approved|changes_requested", "pr_url": "https://github.com/..."} ``` +## Work item management + +State transitions are specified in each skill. Use `mcp__jira__atlassianUserInfo` to resolve +the Claude Code account ID and `mcp__jira__lookupJiraAccountId` to resolve human reviewers +from a provided email address. Never hardcode account IDs or email addresses. + ## Task tracking At the start of each task, call `TodoWrite` to create a task list reflecting the steps you plan to take. Mark each item `in_progress` when you start it and `completed` when you finish it. Keep the list accurate throughout. diff --git a/.claude/commands/developer-implement.md b/.claude/commands/developer-implement.md index 973587f1..eca93ce4 100644 --- a/.claude/commands/developer-implement.md +++ b/.claude/commands/developer-implement.md @@ -15,6 +15,13 @@ $TASK_BRIEF ## Steps +### 0 — Update work item + +1. Call `mcp__jira__atlassianUserInfo` to get the current user's account ID. +2. Call `mcp__jira__getTransitionsForJiraIssue` for `$ARGUMENTS` to find the transition to **In Progress**. +3. Apply it with `mcp__jira__transitionJiraIssue`. +4. Set the assignee to the current account ID with `mcp__jira__editJiraIssue`. + ### 1 — Load standards Invoke the `developer-patterns` skill (loads all guidelines from `CONTRIBUTING.md`). diff --git a/.claude/commands/jira-update-failed.md b/.claude/commands/jira-update-failed.md new file mode 100644 index 00000000..632ea878 --- /dev/null +++ b/.claude/commands/jira-update-failed.md @@ -0,0 +1,31 @@ +--- +description: Update a Jira work item when the dev-team pipeline has exhausted all fix retries. Assigns the issue to the human reviewer and adds a failure comment with log paths. +argument-hint: +--- + +## Inputs + +Work item ID: `$ARGUMENTS` + +Human reviewer email: `$REVIEW_ASSIGNEE_EMAIL` + +Log paths: + +$LOG_PATHS + +--- + +## Steps + +### 1 — Assign to human reviewer + +1. Call `mcp__jira__lookupJiraAccountId` with `$REVIEW_ASSIGNEE_EMAIL` to get the account ID. +2. Assign the Jira issue to that account with `mcp__jira__editJiraIssue`. + +### 2 — Add failure comment + +Call `mcp__jira__addCommentToJiraIssue` on `$ARGUMENTS` with: + +> Pipeline failed after max retries — manual intervention needed. +> +> $LOG_PATHS diff --git a/.claude/commands/reviewer-review.md b/.claude/commands/reviewer-review.md index a897eacb..9560faf7 100644 --- a/.claude/commands/reviewer-review.md +++ b/.claude/commands/reviewer-review.md @@ -39,6 +39,13 @@ If `$PR_URL` is empty: If `$PR_URL` is already set, use that PR for the review. +## Step 3a — Update work item + +1. Call `mcp__jira__atlassianUserInfo` to get the current user's account ID. +2. Call `mcp__jira__getTransitionsForJiraIssue` for `$WORK_ITEM_ID` to find the transition to **In Review**. +3. Apply it with `mcp__jira__transitionJiraIssue`. +4. Set the assignee to the current account ID with `mcp__jira__editJiraIssue`. + ## Step 4 — Retrieve and read the diff Use the GitHub MCP to fetch the PR diff. Read all changed files in full to understand the diff --git a/.claude/commands/reviewer-sign-off.md b/.claude/commands/reviewer-sign-off.md index ddf83649..e4cddf29 100644 --- a/.claude/commands/reviewer-sign-off.md +++ b/.claude/commands/reviewer-sign-off.md @@ -65,6 +65,15 @@ inline review comments attached to the specific file and line. Submit with event Priority 1–5 issues were found in the modified files - **REQUEST_CHANGES** otherwise +## Step 6a — Hand off to human reviewer (approved only) + +If the review outcome is **approved**, do the following before writing the output summary: + +1. Call `mcp__jira__lookupJiraAccountId` with `$REVIEW_ASSIGNEE_EMAIL` to get the human reviewer's account ID. +2. Assign the Jira issue to that account with `mcp__jira__editJiraIssue`. +3. Call `mcp__github__add_pull_request_review_request` to request a review from `$REVIEW_ASSIGNEE_EMAIL` on `$PR_URL`. +4. Add a brief Jira comment with `mcp__jira__addCommentToJiraIssue`: "PR ready for human review — reviewer requested on GitHub." + ## Step 7 — Output Write a concise summary: diff --git a/.claude/scripts/dev_team.py b/.claude/scripts/dev_team.py index 70efcda2..ee0c6c71 100644 --- a/.claude/scripts/dev_team.py +++ b/.claude/scripts/dev_team.py @@ -12,11 +12,13 @@ import argparse import datetime import json +import os import re import shutil import subprocess import sys import threading +import time; from abc import ABC, abstractmethod from dataclasses import dataclass, field from pathlib import Path @@ -144,6 +146,8 @@ class PipelineContext: pr_url: str = "" review_notes: str = "" last_failure: str = "" + # Runtime-only — not persisted to the context file. + review_assignee_email: str = field(default="", repr=False) build_log: str = "" test_log: str = "" started: datetime.datetime = field(default_factory=datetime.datetime.now) @@ -200,6 +204,7 @@ def load(cls, path: Path) -> "PipelineContext": """Load context from a markdown file previously written by save().""" text = path.read_text(encoding="utf-8") meta, body = _parse_frontmatter(text) + ctx = cls( work_item_id=meta.get("work_item_id", ""), @@ -454,6 +459,7 @@ def run(self, ctx: PipelineContext) -> str: "$WORK_ITEM_ID": ctx.work_item_id, "$TASK_BRIEF": ctx.brief, "$PR_URL": ctx.pr_url, + "$REVIEW_ASSIGNEE_EMAIL": ctx.review_assignee_email, }, ) except (FileNotFoundError, RuntimeError, subprocess.CalledProcessError) as e: @@ -583,8 +589,7 @@ def run(self) -> None: self.ctx.state = self.machine.state self.ctx.save(self.context_path) - if self.machine.state == "failed": - sys.exit(1) + # Callers check ctx.state and handle failure (sys.exit, cleanup, etc.). # --------------------------------------------------------------------------- @@ -793,10 +798,14 @@ def _write_prompt() -> None: if stream: print(result_text, flush=True) + print(f"[{time.time():.1f}] Waiting for process to exit...", flush=True) proc.wait() + print(f"[{time.time():.1f}] Reading stderr...", flush=True) stderr_text = proc.stderr.read() # type: ignore[union-attr] if stderr_text: + print(f"[{time.time():.1f}] opening log path to write errors...") with log_path.open("a", encoding="utf-8") as log_file: + print(f"[{time.time():.1f}] Writing errors...", flush=True) log_file.write(json.dumps({"type": "stderr", "text": stderr_text}) + "\n") if proc.returncode != 0: @@ -806,9 +815,32 @@ def _write_prompt() -> None: stderr=f"{stderr_text}\n(exit code {proc.returncode})", ) + print(f"[{time.time():.1f}] call_agent complete", flush=True) return result_text +def _handle_pipeline_failure(ctx: PipelineContext) -> None: + """Assign the Jira issue to the human reviewer and add a failure comment.""" + log_parts: list[str] = [] + if ctx.build_log: + log_parts.append(f"Build log: {ctx.build_log}") + if ctx.test_log: + log_parts.append(f"Test log: {ctx.test_log}") + log_paths = "\n".join(log_parts) if log_parts else "(no logs recorded)" + + print(f"Updating Jira for failed pipeline ({ctx.work_item_id})...", flush=True) + try: + call_agent( + "developer", "jira-update-failed", ctx.work_item_id, + substitutions={ + "$REVIEW_ASSIGNEE_EMAIL": ctx.review_assignee_email, + "$LOG_PATHS": log_paths, + }, + ) + except (FileNotFoundError, RuntimeError, subprocess.CalledProcessError) as e: + print(f"Warning: Jira failure update failed (continuing): {e}", file=sys.stderr) + + def _format_work_summaries(summaries: list[str]) -> str: """Format one or more work summaries for embedding in a developer-fix prompt.""" if len(summaries) == 1: @@ -934,6 +966,16 @@ def main() -> None: log_dir.mkdir(parents=True, exist_ok=True) context_path = log_dir / f"{work_item_id}-context.md" + review_assignee_email = os.environ.get("REVIEW_ASSIGNEE_EMAIL", "") + if not review_assignee_email: + print( + "Error: REVIEW_ASSIGNEE_EMAIL environment variable is not set.\n" + "Set it to the Jira/email address of the human reviewer, e.g.:\n" + " set REVIEW_ASSIGNEE_EMAIL=you@example.com", + file=sys.stderr, + ) + sys.exit(1) + if context_path.exists(): ctx = PipelineContext.load(context_path) if ctx.state in workflow.terminal_states: @@ -950,8 +992,14 @@ def main() -> None: state=workflow.initial_state) ctx.save(context_path) + ctx.review_assignee_email = review_assignee_email + DevTeamPipeline(ctx, context_path, workflow).run() + if ctx.state == "failed": + _handle_pipeline_failure(ctx) + sys.exit(1) + if __name__ == "__main__": main() From 5e8eaec7bc1506d1623bb16a96fb7466986a8f0d Mon Sep 17 00:00:00 2001 From: Joe Davis Date: Thu, 21 May 2026 09:23:51 -0700 Subject: [PATCH 08/14] ADR-171: Fix git worktree detection in ServiceFixture Directory.Exists() returns false when .git is a file (git worktree layout). Path.Exists() accepts both files and directories, so it correctly finds the repo root in worktree checkouts. --- AdaptiveRemote.sln | 30 ++ Directory.Packages.props | 2 + backend.slnf | 2 + docker-compose.yml | 14 + localstack-init/02-deploy-lambda.sh | 40 +++ .../Logging/MessageLogger.cs | 13 + ...emote.Backend.LayoutCompilerService.csproj | 26 ++ .../Dockerfile | 23 ++ .../Endpoints/CompileEndpoints.cs | 65 ++++ .../Endpoints/HealthEndpoints.cs | 34 +++ .../Program.cs | 52 ++++ .../Services/LayoutCompiler.cs | 170 +++++++++++ .../appsettings.Development.json | 8 + .../appsettings.json | 9 + .../LayoutCompilerServiceSettings.cs | 13 + .../Program.cs | 16 +- .../Services/HttpLayoutCompilerClient.cs | 50 ++++ .../LayoutCompilerServiceEndpoints.feature | 27 ++ .../LayoutCompilerServiceEndpoints.feature.cs | 244 +++++++++++++++ ...Backend.LayoutCompilerService.Tests.csproj | 32 ++ .../Services/LayoutCompilerTests.cs | 279 ++++++++++++++++++ .../Backend/LayoutCompilerSteps.cs | 125 ++++++++ .../Backend/ServiceSteps.cs | 3 +- .../Hooks/EnvironmentSetupHooks.cs | 3 +- .../LogVerificationSteps.cs | 4 +- .../Backend/ServiceFixture.cs | 2 +- .../Host/ISimulatedEnvironment.cs | 4 + .../Host/SimulatedEnvironment.cs | 26 ++ 28 files changed, 1311 insertions(+), 5 deletions(-) create mode 100644 localstack-init/02-deploy-lambda.sh create mode 100644 src/AdaptiveRemote.Backend.LayoutCompilerService/AdaptiveRemote.Backend.LayoutCompilerService.csproj create mode 100644 src/AdaptiveRemote.Backend.LayoutCompilerService/Dockerfile create mode 100644 src/AdaptiveRemote.Backend.LayoutCompilerService/Endpoints/CompileEndpoints.cs create mode 100644 src/AdaptiveRemote.Backend.LayoutCompilerService/Endpoints/HealthEndpoints.cs create mode 100644 src/AdaptiveRemote.Backend.LayoutCompilerService/Program.cs create mode 100644 src/AdaptiveRemote.Backend.LayoutCompilerService/Services/LayoutCompiler.cs create mode 100644 src/AdaptiveRemote.Backend.LayoutCompilerService/appsettings.Development.json create mode 100644 src/AdaptiveRemote.Backend.LayoutCompilerService/appsettings.json create mode 100644 src/AdaptiveRemote.Backend.LayoutProcessingService/Configuration/LayoutCompilerServiceSettings.cs create mode 100644 src/AdaptiveRemote.Backend.LayoutProcessingService/Services/HttpLayoutCompilerClient.cs create mode 100644 test/AdaptiveRemote.Backend.ApiTests/Features/LayoutCompilerServiceEndpoints.feature create mode 100644 test/AdaptiveRemote.Backend.ApiTests/Features/LayoutCompilerServiceEndpoints.feature.cs create mode 100644 test/AdaptiveRemote.Backend.LayoutCompilerService.Tests/AdaptiveRemote.Backend.LayoutCompilerService.Tests.csproj create mode 100644 test/AdaptiveRemote.Backend.LayoutCompilerService.Tests/Services/LayoutCompilerTests.cs create mode 100644 test/AdaptiveRemote.EndToEndTests.Steps/Backend/LayoutCompilerSteps.cs diff --git a/AdaptiveRemote.sln b/AdaptiveRemote.sln index 452270af..3d78d050 100644 --- a/AdaptiveRemote.sln +++ b/AdaptiveRemote.sln @@ -66,6 +66,10 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "AdaptiveRemote.Backend.Layo EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "AdaptiveRemote.Backend.LayoutProcessingService.Tests", "test\AdaptiveRemote.Backend.LayoutProcessingService.Tests\AdaptiveRemote.Backend.LayoutProcessingService.Tests.csproj", "{A829A88B-C42D-4D3B-8CDE-621862E4B39C}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "AdaptiveRemote.Backend.LayoutCompilerService", "src\AdaptiveRemote.Backend.LayoutCompilerService\AdaptiveRemote.Backend.LayoutCompilerService.csproj", "{D2C5A7F3-E891-4B8D-9A0F-C7E43B5D2F01}" +EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "AdaptiveRemote.Backend.LayoutCompilerService.Tests", "test\AdaptiveRemote.Backend.LayoutCompilerService.Tests\AdaptiveRemote.Backend.LayoutCompilerService.Tests.csproj", "{C8F1B2A4-7D06-4E9C-B3A1-D8F52C6E3F02}" +EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "AdaptiveRemote.Backend.Common", "src\AdaptiveRemote.Backend.Common\AdaptiveRemote.Backend.Common.csproj", "{1F36A31B-299C-480C-B974-F4CE67C6F034}" EndProject Global @@ -306,6 +310,30 @@ Global {A829A88B-C42D-4D3B-8CDE-621862E4B39C}.Release|x64.Build.0 = Release|Any CPU {A829A88B-C42D-4D3B-8CDE-621862E4B39C}.Release|x86.ActiveCfg = Release|Any CPU {A829A88B-C42D-4D3B-8CDE-621862E4B39C}.Release|x86.Build.0 = Release|Any CPU + {D2C5A7F3-E891-4B8D-9A0F-C7E43B5D2F01}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {D2C5A7F3-E891-4B8D-9A0F-C7E43B5D2F01}.Debug|Any CPU.Build.0 = Debug|Any CPU + {D2C5A7F3-E891-4B8D-9A0F-C7E43B5D2F01}.Debug|x64.ActiveCfg = Debug|Any CPU + {D2C5A7F3-E891-4B8D-9A0F-C7E43B5D2F01}.Debug|x64.Build.0 = Debug|Any CPU + {D2C5A7F3-E891-4B8D-9A0F-C7E43B5D2F01}.Debug|x86.ActiveCfg = Debug|Any CPU + {D2C5A7F3-E891-4B8D-9A0F-C7E43B5D2F01}.Debug|x86.Build.0 = Debug|Any CPU + {D2C5A7F3-E891-4B8D-9A0F-C7E43B5D2F01}.Release|Any CPU.ActiveCfg = Release|Any CPU + {D2C5A7F3-E891-4B8D-9A0F-C7E43B5D2F01}.Release|Any CPU.Build.0 = Release|Any CPU + {D2C5A7F3-E891-4B8D-9A0F-C7E43B5D2F01}.Release|x64.ActiveCfg = Release|Any CPU + {D2C5A7F3-E891-4B8D-9A0F-C7E43B5D2F01}.Release|x64.Build.0 = Release|Any CPU + {D2C5A7F3-E891-4B8D-9A0F-C7E43B5D2F01}.Release|x86.ActiveCfg = Release|Any CPU + {D2C5A7F3-E891-4B8D-9A0F-C7E43B5D2F01}.Release|x86.Build.0 = Release|Any CPU + {C8F1B2A4-7D06-4E9C-B3A1-D8F52C6E3F02}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {C8F1B2A4-7D06-4E9C-B3A1-D8F52C6E3F02}.Debug|Any CPU.Build.0 = Debug|Any CPU + {C8F1B2A4-7D06-4E9C-B3A1-D8F52C6E3F02}.Debug|x64.ActiveCfg = Debug|Any CPU + {C8F1B2A4-7D06-4E9C-B3A1-D8F52C6E3F02}.Debug|x64.Build.0 = Debug|Any CPU + {C8F1B2A4-7D06-4E9C-B3A1-D8F52C6E3F02}.Debug|x86.ActiveCfg = Debug|Any CPU + {C8F1B2A4-7D06-4E9C-B3A1-D8F52C6E3F02}.Debug|x86.Build.0 = Debug|Any CPU + {C8F1B2A4-7D06-4E9C-B3A1-D8F52C6E3F02}.Release|Any CPU.ActiveCfg = Release|Any CPU + {C8F1B2A4-7D06-4E9C-B3A1-D8F52C6E3F02}.Release|Any CPU.Build.0 = Release|Any CPU + {C8F1B2A4-7D06-4E9C-B3A1-D8F52C6E3F02}.Release|x64.ActiveCfg = Release|Any CPU + {C8F1B2A4-7D06-4E9C-B3A1-D8F52C6E3F02}.Release|x64.Build.0 = Release|Any CPU + {C8F1B2A4-7D06-4E9C-B3A1-D8F52C6E3F02}.Release|x86.ActiveCfg = Release|Any CPU + {C8F1B2A4-7D06-4E9C-B3A1-D8F52C6E3F02}.Release|x86.Build.0 = Release|Any CPU {1F36A31B-299C-480C-B974-F4CE67C6F034}.Debug|Any CPU.ActiveCfg = Debug|Any CPU {1F36A31B-299C-480C-B974-F4CE67C6F034}.Debug|Any CPU.Build.0 = Debug|Any CPU {1F36A31B-299C-480C-B974-F4CE67C6F034}.Debug|x64.ActiveCfg = Debug|Any CPU @@ -339,6 +367,8 @@ Global {352E5981-CC33-4474-8203-9CE241F42281} = {0C88DD14-F956-CE84-757C-A364CCF449FC} {F341B9BA-8517-447F-93B3-7E09AAD0F0E1} = {827E0CD3-B72D-47B6-A68D-7590B98EB39B} {A829A88B-C42D-4D3B-8CDE-621862E4B39C} = {0C88DD14-F956-CE84-757C-A364CCF449FC} + {D2C5A7F3-E891-4B8D-9A0F-C7E43B5D2F01} = {827E0CD3-B72D-47B6-A68D-7590B98EB39B} + {C8F1B2A4-7D06-4E9C-B3A1-D8F52C6E3F02} = {0C88DD14-F956-CE84-757C-A364CCF449FC} {1F36A31B-299C-480C-B974-F4CE67C6F034} = {827E0CD3-B72D-47B6-A68D-7590B98EB39B} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution diff --git a/Directory.Packages.props b/Directory.Packages.props index ae5ea6fc..3c0e1926 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -3,6 +3,8 @@ true + + diff --git a/backend.slnf b/backend.slnf index 57da75f3..069fdaec 100644 --- a/backend.slnf +++ b/backend.slnf @@ -6,9 +6,11 @@ "src\\AdaptiveRemote.Backend.CompiledLayoutService\\AdaptiveRemote.Backend.CompiledLayoutService.csproj", "src\\AdaptiveRemote.Backend.RawLayoutService\\AdaptiveRemote.Backend.RawLayoutService.csproj", "src\\AdaptiveRemote.Backend.LayoutProcessingService\\AdaptiveRemote.Backend.LayoutProcessingService.csproj", + "src\\AdaptiveRemote.Backend.LayoutCompilerService\\AdaptiveRemote.Backend.LayoutCompilerService.csproj", "test\\AdaptiveRemote.Backend.ApiTests\\AdaptiveRemote.Backend.ApiTests.csproj", "test\\AdaptiveRemote.Backend.RawLayoutService.Tests\\AdaptiveRemote.Backend.RawLayoutService.Tests.csproj", "test\\AdaptiveRemote.Backend.LayoutProcessingService.Tests\\AdaptiveRemote.Backend.LayoutProcessingService.Tests.csproj", + "test\\AdaptiveRemote.Backend.LayoutCompilerService.Tests\\AdaptiveRemote.Backend.LayoutCompilerService.Tests.csproj", "test\\AdaptiveRemote.TestUtilities\\AdaptiveRemote.TestUtilities.csproj" ] } diff --git a/docker-compose.yml b/docker-compose.yml index 257ee7a2..d181a036 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -62,6 +62,18 @@ services: networks: - backend + layoutcompilerservice: + build: + context: . + dockerfile: src/AdaptiveRemote.Backend.LayoutCompilerService/Dockerfile + ports: + - "8083:8080" + environment: + - ASPNETCORE_ENVIRONMENT=Development + - ASPNETCORE_URLS=http://+:8080 + networks: + - backend + layoutprocessingservice: build: context: . @@ -81,6 +93,7 @@ services: # Upstream service URLs (Docker Compose DNS) - RawLayoutService__BaseUrl=http://rawlayoutservice:8080 - CompiledLayoutService__BaseUrl=http://compiledlayoutservice:8080 + - LayoutCompilerService__BaseUrl=http://layoutcompilerservice:8080 - AWS_ACCESS_KEY_ID=test - AWS_SECRET_ACCESS_KEY=test - LocalStack__BaseUrl=http://localstack:4566 @@ -88,6 +101,7 @@ services: - localstack - rawlayoutservice - compiledlayoutservice + - layoutcompilerservice networks: - backend diff --git a/localstack-init/02-deploy-lambda.sh b/localstack-init/02-deploy-lambda.sh new file mode 100644 index 00000000..5f8a394d --- /dev/null +++ b/localstack-init/02-deploy-lambda.sh @@ -0,0 +1,40 @@ +#!/bin/bash + +# Deploy LayoutCompilerService as a Lambda function to LocalStack. +# The package must be pre-built before running this script: +# dotnet publish src/AdaptiveRemote.Backend.LayoutCompilerService -c Release -r linux-x64 +# zip -j /tmp/bootstrap.zip src/AdaptiveRemote.Backend.LayoutCompilerService/bin/Release/net10.0/linux-x64/publish/bootstrap + +FUNCTION_NAME="LayoutCompilerService" +REGION="us-east-1" +PACKAGE_PATH="${LAMBDA_PACKAGE_PATH:-/tmp/bootstrap.zip}" + +if [ ! -f "$PACKAGE_PATH" ]; then + echo "Lambda package not found at $PACKAGE_PATH. Skipping Lambda deployment." + echo "To deploy, build the package first and set LAMBDA_PACKAGE_PATH." + exit 0 +fi + +# Create or update the function +awslocal lambda create-function \ + --function-name "$FUNCTION_NAME" \ + --runtime provided.al2023 \ + --role arn:aws:iam::000000000000:role/lambda-role \ + --handler bootstrap \ + --zip-file "fileb://$PACKAGE_PATH" \ + --region "$REGION" \ + --environment Variables="{ASPNETCORE_ENVIRONMENT=Development}" 2>/dev/null \ +|| awslocal lambda update-function-code \ + --function-name "$FUNCTION_NAME" \ + --zip-file "fileb://$PACKAGE_PATH" \ + --region "$REGION" + +echo "Lambda function '$FUNCTION_NAME' deployed successfully" + +# Expose via Function URL for HTTP access +awslocal lambda create-function-url-config \ + --function-name "$FUNCTION_NAME" \ + --auth-type NONE \ + --region "$REGION" 2>/dev/null || true + +echo "Lambda function URL created for '$FUNCTION_NAME'" diff --git a/src/AdaptiveRemote.Backend.Common/Logging/MessageLogger.cs b/src/AdaptiveRemote.Backend.Common/Logging/MessageLogger.cs index a7e0ed58..4646904a 100644 --- a/src/AdaptiveRemote.Backend.Common/Logging/MessageLogger.cs +++ b/src/AdaptiveRemote.Backend.Common/Logging/MessageLogger.cs @@ -128,4 +128,17 @@ public static partial class MessageLogger [LoggerMessage(EventId = 1720, Level = LogLevel.Warning, Message = "SQS message unrecognized and deleted; receiptHandle={ReceiptHandle}")] public static partial void SqsUnrecognizedMessageWarning(this ILogger logger, string receiptHandle, Exception exception); + // LayoutCompilerService-specific messages + [LoggerMessage(EventId = 1800, Level = LogLevel.Information, Message = "Compilation started; rawLayoutId={RawLayoutId}")] + public static partial void CompilationStarted(this ILogger logger, Guid rawLayoutId); + + [LoggerMessage(EventId = 1801, Level = LogLevel.Information, Message = "Compilation completed; rawLayoutId={RawLayoutId}")] + public static partial void CompilationCompleted(this ILogger logger, Guid rawLayoutId); + + [LoggerMessage(EventId = 1802, Level = LogLevel.Information, Message = "Preview compilation started")] + public static partial void PreviewCompilationStarted(this ILogger logger); + + [LoggerMessage(EventId = 1803, Level = LogLevel.Information, Message = "Preview compilation completed")] + public static partial void PreviewCompilationCompleted(this ILogger logger); + } diff --git a/src/AdaptiveRemote.Backend.LayoutCompilerService/AdaptiveRemote.Backend.LayoutCompilerService.csproj b/src/AdaptiveRemote.Backend.LayoutCompilerService/AdaptiveRemote.Backend.LayoutCompilerService.csproj new file mode 100644 index 00000000..d72351b9 --- /dev/null +++ b/src/AdaptiveRemote.Backend.LayoutCompilerService/AdaptiveRemote.Backend.LayoutCompilerService.csproj @@ -0,0 +1,26 @@ + + + + net10.0 + enable + enable + AdaptiveRemote.Backend.LayoutCompilerService + bootstrap + true + + $(NoWarn);IL2026;IL3050 + + + + + + + + + + + + + + diff --git a/src/AdaptiveRemote.Backend.LayoutCompilerService/Dockerfile b/src/AdaptiveRemote.Backend.LayoutCompilerService/Dockerfile new file mode 100644 index 00000000..710aba1c --- /dev/null +++ b/src/AdaptiveRemote.Backend.LayoutCompilerService/Dockerfile @@ -0,0 +1,23 @@ +FROM mcr.microsoft.com/dotnet/sdk:10.0 AS build +WORKDIR /src + +# Copy csproj files and restore +COPY ["src/AdaptiveRemote.Contracts/AdaptiveRemote.Contracts.csproj", "AdaptiveRemote.Contracts/"] +COPY ["src/AdaptiveRemote.Backend.Common/AdaptiveRemote.Backend.Common.csproj", "AdaptiveRemote.Backend.Common/"] +COPY ["src/AdaptiveRemote.Backend.LayoutCompilerService/AdaptiveRemote.Backend.LayoutCompilerService.csproj", "AdaptiveRemote.Backend.LayoutCompilerService/"] +COPY ["Directory.Build.props", "./"] +COPY ["Directory.Packages.props", "./"] +RUN dotnet restore "AdaptiveRemote.Backend.LayoutCompilerService/AdaptiveRemote.Backend.LayoutCompilerService.csproj" + +# Copy source and publish (Native AOT for linux-x64) +COPY ["src/AdaptiveRemote.Contracts/", "AdaptiveRemote.Contracts/"] +COPY ["src/AdaptiveRemote.Backend.Common/", "AdaptiveRemote.Backend.Common/"] +COPY ["src/AdaptiveRemote.Backend.LayoutCompilerService/", "AdaptiveRemote.Backend.LayoutCompilerService/"] +WORKDIR "/src/AdaptiveRemote.Backend.LayoutCompilerService" +RUN dotnet publish "AdaptiveRemote.Backend.LayoutCompilerService.csproj" -c Release -r linux-x64 -o /app/publish + +FROM mcr.microsoft.com/dotnet/runtime-deps:10.0 AS final +WORKDIR /app +COPY --from=build /app/publish . +EXPOSE 8080 +ENTRYPOINT ["./bootstrap"] diff --git a/src/AdaptiveRemote.Backend.LayoutCompilerService/Endpoints/CompileEndpoints.cs b/src/AdaptiveRemote.Backend.LayoutCompilerService/Endpoints/CompileEndpoints.cs new file mode 100644 index 00000000..1819a59f --- /dev/null +++ b/src/AdaptiveRemote.Backend.LayoutCompilerService/Endpoints/CompileEndpoints.cs @@ -0,0 +1,65 @@ +using AdaptiveRemote.Backend.Common.Logging; +using AdaptiveRemote.Backend.LayoutCompilerService.Services; +using AdaptiveRemote.Contracts; + +namespace AdaptiveRemote.Backend.LayoutCompilerService.Endpoints; + +public static class CompileEndpoints +{ + public static void MapCompileEndpoints(this IEndpointRouteBuilder app) + { + app.MapPost("/compile", CompileLayout) + .WithName(nameof(CompileLayout)) + .Produces(StatusCodes.Status200OK) + .ProducesValidationProblem(); + + app.MapPost("/compile/preview", CompilePreview) + .WithName(nameof(CompilePreview)) + .Produces(StatusCodes.Status200OK) + .ProducesValidationProblem(); + } + + private static async Task CompileLayout( + HttpRequest request, + LayoutCompiler compiler, + ILogger logger, + CancellationToken ct) + { + using IDisposable scope = logger.StartRequestScope("POST", "/compile"); + + RawLayout? raw = await request + .ReadFromJsonAsync(LayoutContractsJsonContext.Default.RawLayout, ct) + .ConfigureAwait(false); + + if (raw is null) + { + return Results.BadRequest("Request body must be a valid RawLayout."); + } + + CompiledLayout result = compiler.Compile(raw); + + return Results.Json(result, LayoutContractsJsonContext.Default.CompiledLayout); + } + + private static async Task CompilePreview( + HttpRequest request, + LayoutCompiler compiler, + ILogger logger, + CancellationToken ct) + { + using IDisposable scope = logger.StartRequestScope("POST", "/compile/preview"); + + IReadOnlyList? elements = await request + .ReadFromJsonAsync(LayoutContractsJsonContext.Default.IReadOnlyListRawLayoutElementDto, ct) + .ConfigureAwait(false); + + if (elements is null) + { + return Results.BadRequest("Request body must be a valid list of RawLayoutElementDto."); + } + + PreviewLayout result = compiler.CompilePreview(elements); + + return Results.Json(result, LayoutContractsJsonContext.Default.PreviewLayout); + } +} diff --git a/src/AdaptiveRemote.Backend.LayoutCompilerService/Endpoints/HealthEndpoints.cs b/src/AdaptiveRemote.Backend.LayoutCompilerService/Endpoints/HealthEndpoints.cs new file mode 100644 index 00000000..538996ee --- /dev/null +++ b/src/AdaptiveRemote.Backend.LayoutCompilerService/Endpoints/HealthEndpoints.cs @@ -0,0 +1,34 @@ +using System.Reflection; +using AdaptiveRemote.Backend.Common.Logging; +using AdaptiveRemote.Contracts; + +namespace AdaptiveRemote.Backend.LayoutCompilerService.Endpoints; + +public static class HealthEndpoints +{ + public static void MapHealthEndpoints(this IEndpointRouteBuilder app) + { + app.MapGet("/health", GetHealth) + .WithName(nameof(GetHealth)) + .Produces(StatusCodes.Status200OK); + } + + private static IResult GetHealth(ILogger logger) + { + using IDisposable scope = logger.StartRequestScope("GET", "/health"); + + string? version = Assembly.GetExecutingAssembly() + .GetCustomAttribute() + ?.InformationalVersion ?? "unknown"; + + HealthResponse response = new( + ServiceName: "LayoutCompilerService", + Version: version, + Status: "healthy" + ); + + logger.HealthCheckSuccessful(); + + return Results.Json(response, LayoutContractsJsonContext.Default.HealthResponse); + } +} diff --git a/src/AdaptiveRemote.Backend.LayoutCompilerService/Program.cs b/src/AdaptiveRemote.Backend.LayoutCompilerService/Program.cs new file mode 100644 index 00000000..c64ca0c4 --- /dev/null +++ b/src/AdaptiveRemote.Backend.LayoutCompilerService/Program.cs @@ -0,0 +1,52 @@ +using AdaptiveRemote.Backend.Common.Logging; +using AdaptiveRemote.Backend.LayoutCompilerService.Endpoints; +using AdaptiveRemote.Backend.LayoutCompilerService.Services; +using Amazon.Lambda.AspNetCoreServer.Hosting; +using Scalar.AspNetCore; + +string? logFilePath = null; +for (int i = 0; i < args.Length - 1; i++) +{ + if (args[i] == "--logFile") + { + logFilePath = args[i + 1]; + break; + } +} + +WebApplicationBuilder builder = WebApplication.CreateBuilder(args); + +if (!string.IsNullOrEmpty(logFilePath)) +{ + builder.Logging.ClearProviders(); + builder.Logging.AddConsole(); + builder.Logging.AddFile(logFilePath); +} + +builder.Services.AddAWSLambdaHosting(LambdaEventSource.HttpApi); +builder.Services.AddSingleton(); +builder.Services.AddOpenApi(); + +WebApplication app = builder.Build(); + +ILogger logger = app.Services.GetRequiredService>(); +logger.ServiceStarting("LayoutCompilerService"); + +if (app.Environment.IsDevelopment()) +{ + app.MapScalarApiReference(); +} + +app.MapOpenApi(); +app.MapHealthEndpoints(); +app.MapCompileEndpoints(); + +string listenAddress = app.Configuration["ASPNETCORE_URLS"] + ?? app.Configuration["urls"] + ?? "http://localhost:5000"; +logger.ServiceStarted("LayoutCompilerService", listenAddress); + +app.Run(); + +// Make Program visible for testing +public partial class Program { } diff --git a/src/AdaptiveRemote.Backend.LayoutCompilerService/Services/LayoutCompiler.cs b/src/AdaptiveRemote.Backend.LayoutCompilerService/Services/LayoutCompiler.cs new file mode 100644 index 00000000..ed1909b0 --- /dev/null +++ b/src/AdaptiveRemote.Backend.LayoutCompilerService/Services/LayoutCompiler.cs @@ -0,0 +1,170 @@ +using System.Text; +using AdaptiveRemote.Backend.Common.Logging; +using AdaptiveRemote.Contracts; + +namespace AdaptiveRemote.Backend.LayoutCompilerService.Services; + +/// +/// Compiles RawLayout and element lists into their compiled representations. +/// Stateless; all methods are synchronous pure functions over the input data. +/// +public class LayoutCompiler +{ + private readonly ILogger _logger; + + public LayoutCompiler(ILogger logger) + { + _logger = logger; + } + + public CompiledLayout Compile(RawLayout raw) + { + _logger.CompilationStarted(raw.Id); + + IReadOnlyList elements = ConvertElements(raw.Elements); + string cssDefinitions = GenerateCssDefinitions(raw.Elements); + + CompiledLayout result = new( + Id: Guid.NewGuid(), + RawLayoutId: raw.Id, + UserId: raw.UserId, + IsActive: false, + Version: raw.Version, + Elements: elements, + CssDefinitions: cssDefinitions, + CompiledAt: DateTimeOffset.UtcNow + ); + + _logger.CompilationCompleted(raw.Id); + return result; + } + + public PreviewLayout CompilePreview(IReadOnlyList elements) + { + _logger.PreviewCompilationStarted(); + + string renderedHtml = GeneratePreviewHtml(elements); + string renderedCss = GeneratePreviewCss(elements); + + PreviewLayout result = new( + RawLayoutId: Guid.Empty, + Version: 0, + RenderedHtml: renderedHtml, + RenderedCss: renderedCss, + CompiledAt: DateTimeOffset.UtcNow, + ValidationResult: new ValidationResult(true, Array.Empty()) + ); + + _logger.PreviewCompilationCompleted(); + return result; + } + + private static System.Collections.ObjectModel.ReadOnlyCollection ConvertElements(IReadOnlyList rawElements) + { + List result = new(rawElements.Count); + + foreach (RawLayoutElementDto element in rawElements) + { + LayoutElementDto compiled = element switch + { + RawCommandDefinitionDto cmd => new CommandDefinitionDto( + Type: cmd.Type, + Name: cmd.Name, + Label: cmd.Label, + Glyph: cmd.Glyph, + SpeakPhrase: cmd.SpeakPhrase, + Reverse: cmd.Reverse, + CssId: cmd.CssId), + RawLayoutGroupDefinitionDto group => new LayoutGroupDefinitionDto( + CssId: group.CssId, + Children: ConvertElements(group.Children)), + _ => throw new InvalidOperationException($"Unknown element type: {element.GetType().Name}") + }; + + result.Add(compiled); + } + + return result.AsReadOnly(); + } + + private static string GenerateCssDefinitions(IReadOnlyList elements) + { + StringBuilder sb = new(); + foreach (RawLayoutElementDto element in elements) + { + AppendElementCss(sb, element); + } + return sb.ToString().TrimEnd(); + } + + private static void AppendElementCss(StringBuilder sb, RawLayoutElementDto element) + { + sb.Append('.').Append(element.CssId).Append(" { "); + sb.Append("grid-row: ").Append(element.GridRow).Append(" / span ").Append(element.GridRowSpan).Append("; "); + sb.Append("grid-column: ").Append(element.GridColumn).Append(" / span ").Append(element.GridColumnSpan).Append(';'); + + if (!string.IsNullOrEmpty(element.AdditionalCss)) + { + sb.Append(' ').Append(element.AdditionalCss.TrimEnd(';')).Append(';'); + } + + sb.AppendLine(" }"); + + if (element is RawLayoutGroupDefinitionDto group) + { + foreach (RawLayoutElementDto child in group.Children) + { + AppendElementCss(sb, child); + } + } + } + + private static string GeneratePreviewHtml(IReadOnlyList elements) + { + StringBuilder sb = new(); + sb.AppendLine("
"); + foreach (RawLayoutElementDto element in elements) + { + AppendElementHtml(sb, element, indent: 2); + } + sb.Append("
"); + return sb.ToString(); + } + + private static void AppendElementHtml(StringBuilder sb, RawLayoutElementDto element, int indent) + { + string pad = new(' ', indent); + switch (element) + { + case RawCommandDefinitionDto cmd: + sb.AppendLine($"{pad}
{EscapeHtml(cmd.Label)}
"); + break; + case RawLayoutGroupDefinitionDto group: + sb.AppendLine($"{pad}
"); + foreach (RawLayoutElementDto child in group.Children) + { + AppendElementHtml(sb, child, indent + 2); + } + sb.AppendLine($"{pad}
"); + break; + } + } + + private static string GeneratePreviewCss(IReadOnlyList elements) + { + StringBuilder sb = new(); + sb.AppendLine(".layout-preview-grid { display: grid; }"); + foreach (RawLayoutElementDto element in elements) + { + AppendElementCss(sb, element); + } + return sb.ToString().TrimEnd(); + } + + private static string EscapeHtml(string text) + => text + .Replace("&", "&") + .Replace("<", "<") + .Replace(">", ">") + .Replace("\"", """); +} diff --git a/src/AdaptiveRemote.Backend.LayoutCompilerService/appsettings.Development.json b/src/AdaptiveRemote.Backend.LayoutCompilerService/appsettings.Development.json new file mode 100644 index 00000000..34f00ef1 --- /dev/null +++ b/src/AdaptiveRemote.Backend.LayoutCompilerService/appsettings.Development.json @@ -0,0 +1,8 @@ +{ + "Logging": { + "LogLevel": { + "Default": "Debug", + "Microsoft.AspNetCore": "Information" + } + } +} diff --git a/src/AdaptiveRemote.Backend.LayoutCompilerService/appsettings.json b/src/AdaptiveRemote.Backend.LayoutCompilerService/appsettings.json new file mode 100644 index 00000000..10f68b8c --- /dev/null +++ b/src/AdaptiveRemote.Backend.LayoutCompilerService/appsettings.json @@ -0,0 +1,9 @@ +{ + "Logging": { + "LogLevel": { + "Default": "Information", + "Microsoft.AspNetCore": "Warning" + } + }, + "AllowedHosts": "*" +} diff --git a/src/AdaptiveRemote.Backend.LayoutProcessingService/Configuration/LayoutCompilerServiceSettings.cs b/src/AdaptiveRemote.Backend.LayoutProcessingService/Configuration/LayoutCompilerServiceSettings.cs new file mode 100644 index 00000000..91d7ab20 --- /dev/null +++ b/src/AdaptiveRemote.Backend.LayoutProcessingService/Configuration/LayoutCompilerServiceSettings.cs @@ -0,0 +1,13 @@ +namespace AdaptiveRemote.Backend.LayoutProcessingService.Configuration; + +/// +/// Configuration for HTTP communication with LayoutCompilerService. +/// Maps to the "LayoutCompilerService" section in appsettings.json. +/// +public class LayoutCompilerServiceSettings +{ + /// + /// Base URL of LayoutCompilerService, e.g. http://layoutcompilerservice:8080 + /// + public string BaseUrl { get; set; } = string.Empty; +} diff --git a/src/AdaptiveRemote.Backend.LayoutProcessingService/Program.cs b/src/AdaptiveRemote.Backend.LayoutProcessingService/Program.cs index 9c662f02..859e87e0 100644 --- a/src/AdaptiveRemote.Backend.LayoutProcessingService/Program.cs +++ b/src/AdaptiveRemote.Backend.LayoutProcessingService/Program.cs @@ -122,8 +122,22 @@ void ConfigureRawLayoutClient(HttpClient client) } }); +// Configure HTTP client for LayoutCompilerService +LayoutCompilerServiceSettings layoutCompilerSettings = builder.Configuration + .GetSection("LayoutCompilerService") + .Get() ?? new LayoutCompilerServiceSettings(); + +builder.Services.Configure(builder.Configuration.GetSection("LayoutCompilerService")); + +builder.Services.AddHttpClient(client => +{ + if (!string.IsNullOrEmpty(layoutCompilerSettings.BaseUrl)) + { + client.BaseAddress = new Uri(layoutCompilerSettings.BaseUrl); + } +}); + // Register stub implementations (to be replaced in later tasks) -builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); diff --git a/src/AdaptiveRemote.Backend.LayoutProcessingService/Services/HttpLayoutCompilerClient.cs b/src/AdaptiveRemote.Backend.LayoutProcessingService/Services/HttpLayoutCompilerClient.cs new file mode 100644 index 00000000..a444a248 --- /dev/null +++ b/src/AdaptiveRemote.Backend.LayoutProcessingService/Services/HttpLayoutCompilerClient.cs @@ -0,0 +1,50 @@ +using System.Text.Json; +using AdaptiveRemote.Contracts; + +namespace AdaptiveRemote.Backend.LayoutProcessingService.Services; + +/// +/// HTTP client implementation of ILayoutCompilerClient. +/// Calls LayoutCompilerService over HTTP to compile raw layouts. +/// +public class HttpLayoutCompilerClient : ILayoutCompilerClient +{ + private readonly HttpClient _httpClient; + + public HttpLayoutCompilerClient(HttpClient httpClient) + { + _httpClient = httpClient; + } + + public async Task CompileAsync(RawLayout raw, CancellationToken ct) + { + string json = JsonSerializer.Serialize(raw, LayoutContractsJsonContext.Default.RawLayout); + StringContent content = new(json, System.Text.Encoding.UTF8, "application/json"); + + HttpResponseMessage response = await _httpClient + .PostAsync("/compile", content, ct) + .ConfigureAwait(false); + + response.EnsureSuccessStatusCode(); + + string responseJson = await response.Content.ReadAsStringAsync(ct).ConfigureAwait(false); + return JsonSerializer.Deserialize(responseJson, LayoutContractsJsonContext.Default.CompiledLayout) + ?? throw new InvalidOperationException("CompileAsync returned null from LayoutCompilerService"); + } + + public async Task CompilePreviewAsync(IReadOnlyList elements, CancellationToken ct) + { + string json = JsonSerializer.Serialize(elements, LayoutContractsJsonContext.Default.IReadOnlyListRawLayoutElementDto); + StringContent content = new(json, System.Text.Encoding.UTF8, "application/json"); + + HttpResponseMessage response = await _httpClient + .PostAsync("/compile/preview", content, ct) + .ConfigureAwait(false); + + response.EnsureSuccessStatusCode(); + + string responseJson = await response.Content.ReadAsStringAsync(ct).ConfigureAwait(false); + return JsonSerializer.Deserialize(responseJson, LayoutContractsJsonContext.Default.PreviewLayout) + ?? throw new InvalidOperationException("CompilePreviewAsync returned null from LayoutCompilerService"); + } +} diff --git a/test/AdaptiveRemote.Backend.ApiTests/Features/LayoutCompilerServiceEndpoints.feature b/test/AdaptiveRemote.Backend.ApiTests/Features/LayoutCompilerServiceEndpoints.feature new file mode 100644 index 00000000..269fb61a --- /dev/null +++ b/test/AdaptiveRemote.Backend.ApiTests/Features/LayoutCompilerServiceEndpoints.feature @@ -0,0 +1,27 @@ +@ApiIntegrationTest +Feature: LayoutCompilerService Endpoints + +Scenario: Compile a layout with a single command element + Given LayoutCompilerService is running + And the client has no Authorization token + And a RawLayout with a command element at grid position (1, 1) is stored + When the test client calls POST /compile on the LayoutCompilerService endpoint with the stored RawLayout + Then the response is 200 OK + And the response body is valid JSON + And the response body represents a CompiledLayout + And the CompiledLayout contains a CommandDefinitionDto with name "Up" + And the CompiledLayout CSS definitions contain a rule for grid position (1, 1) + And the CompiledLayout does not contain authoring properties on its elements + And I should not see any warning or error messages in the LayoutCompilerService logs + +Scenario: Compile a preview layout + Given LayoutCompilerService is running + And the client has no Authorization token + And a list of RawLayoutElementDto with a command element at grid position (1, 1) is stored + When the test client calls POST /compile/preview on the LayoutCompilerService endpoint with the stored elements + Then the response is 200 OK + And the response body is valid JSON + And the response body represents a PreviewLayout + And the PreviewLayout RenderedHtml is non-empty + And the PreviewLayout RenderedCss is non-empty + And I should not see any warning or error messages in the LayoutCompilerService logs diff --git a/test/AdaptiveRemote.Backend.ApiTests/Features/LayoutCompilerServiceEndpoints.feature.cs b/test/AdaptiveRemote.Backend.ApiTests/Features/LayoutCompilerServiceEndpoints.feature.cs new file mode 100644 index 00000000..dd350c70 --- /dev/null +++ b/test/AdaptiveRemote.Backend.ApiTests/Features/LayoutCompilerServiceEndpoints.feature.cs @@ -0,0 +1,244 @@ +// ------------------------------------------------------------------------------ +// +// This code was generated by Reqnroll (https://reqnroll.net/). +// Reqnroll Version:3.0.0.0 +// Reqnroll Generator Version:3.0.0.0 +// +// Changes to this file may cause incorrect behavior and will be lost if +// the code is regenerated. +// +// ------------------------------------------------------------------------------ +#region Designer generated code +#pragma warning disable +using Reqnroll; +namespace AdaptiveRemote.Backend.ApiTests.Features +{ + + + [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Reqnroll", "3.0.0.0")] + [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] + [global::Microsoft.VisualStudio.TestTools.UnitTesting.TestClassAttribute()] + public partial class LayoutCompilerServiceEndpointsFeature + { + + private global::Reqnroll.ITestRunner testRunner; + + private Microsoft.VisualStudio.TestTools.UnitTesting.TestContext _testContext; + + private static string[] featureTags = new string[] { + "ApiIntegrationTest"}; + + private static global::Reqnroll.FeatureInfo featureInfo = new global::Reqnroll.FeatureInfo(new global::System.Globalization.CultureInfo("en-US"), "Features", "LayoutCompilerService Endpoints", null, global::Reqnroll.ProgrammingLanguage.CSharp, featureTags, InitializeCucumberMessages()); + +#line 1 "LayoutCompilerServiceEndpoints.feature" +#line hidden + + public virtual Microsoft.VisualStudio.TestTools.UnitTesting.TestContext TestContext + { + get + { + return this._testContext; + } + set + { + this._testContext = value; + } + } + + [global::Microsoft.VisualStudio.TestTools.UnitTesting.ClassInitializeAttribute()] + public static async global::System.Threading.Tasks.Task FeatureSetupAsync(Microsoft.VisualStudio.TestTools.UnitTesting.TestContext testContext) + { + } + + [global::Microsoft.VisualStudio.TestTools.UnitTesting.ClassCleanupAttribute(Microsoft.VisualStudio.TestTools.UnitTesting.ClassCleanupBehavior.EndOfClass)] + public static async global::System.Threading.Tasks.Task FeatureTearDownAsync() + { + await global::Reqnroll.TestRunnerManager.ReleaseFeatureAsync(featureInfo); + } + + [global::Microsoft.VisualStudio.TestTools.UnitTesting.TestInitializeAttribute()] + public async global::System.Threading.Tasks.Task TestInitializeAsync() + { + testRunner = global::Reqnroll.TestRunnerManager.GetTestRunnerForAssembly(featureHint: featureInfo); + try + { + if (((testRunner.FeatureContext != null) + && (testRunner.FeatureContext.FeatureInfo.Equals(featureInfo) == false))) + { + await testRunner.OnFeatureEndAsync(); + } + } + finally + { + if (((testRunner.FeatureContext != null) + && testRunner.FeatureContext.BeforeFeatureHookFailed)) + { + throw new global::Reqnroll.ReqnrollException("Scenario skipped because of previous before feature hook error"); + } + if ((testRunner.FeatureContext == null)) + { + await testRunner.OnFeatureStartAsync(featureInfo); + } + } + } + + [global::Microsoft.VisualStudio.TestTools.UnitTesting.TestCleanupAttribute()] + public async global::System.Threading.Tasks.Task TestTearDownAsync() + { + if ((testRunner == null)) + { + return; + } + try + { + await testRunner.OnScenarioEndAsync(); + } + finally + { + global::Reqnroll.TestRunnerManager.ReleaseTestRunner(testRunner); + testRunner = null; + } + } + + public void ScenarioInitialize(global::Reqnroll.ScenarioInfo scenarioInfo, global::Reqnroll.RuleInfo ruleInfo) + { + testRunner.OnScenarioInitialize(scenarioInfo, ruleInfo); + testRunner.ScenarioContext.ScenarioContainer.RegisterInstanceAs(_testContext); + } + + public async global::System.Threading.Tasks.Task ScenarioStartAsync() + { + await testRunner.OnScenarioStartAsync(); + } + + public async global::System.Threading.Tasks.Task ScenarioCleanupAsync() + { + await testRunner.CollectScenarioErrorsAsync(); + } + + private static global::Reqnroll.Formatters.RuntimeSupport.FeatureLevelCucumberMessages InitializeCucumberMessages() + { + return new global::Reqnroll.Formatters.RuntimeSupport.FeatureLevelCucumberMessages("Features/LayoutCompilerServiceEndpoints.feature.ndjson", 4); + } + + [global::Microsoft.VisualStudio.TestTools.UnitTesting.TestMethodAttribute("Compile a layout with a single command element")] + [global::Microsoft.VisualStudio.TestTools.UnitTesting.DescriptionAttribute("Compile a layout with a single command element")] + [global::Microsoft.VisualStudio.TestTools.UnitTesting.TestPropertyAttribute("FeatureTitle", "LayoutCompilerService Endpoints")] + [global::Microsoft.VisualStudio.TestTools.UnitTesting.TestCategoryAttribute("ApiIntegrationTest")] + public async global::System.Threading.Tasks.Task CompileALayoutWithASingleCommandElement() + { + string[] tagsOfScenario = ((string[])(null)); + global::System.Collections.Specialized.OrderedDictionary argumentsOfScenario = new global::System.Collections.Specialized.OrderedDictionary(); + string pickleIndex = "0"; + global::Reqnroll.ScenarioInfo scenarioInfo = new global::Reqnroll.ScenarioInfo("Compile a layout with a single command element", null, tagsOfScenario, argumentsOfScenario, featureTags, pickleIndex); + string[] tagsOfRule = ((string[])(null)); + global::Reqnroll.RuleInfo ruleInfo = null; +#line 4 +this.ScenarioInitialize(scenarioInfo, ruleInfo); +#line hidden + if ((global::Reqnroll.TagHelper.ContainsIgnoreTag(scenarioInfo.CombinedTags) || global::Reqnroll.TagHelper.ContainsIgnoreTag(featureTags))) + { + await testRunner.SkipScenarioAsync(); + } + else + { + await this.ScenarioStartAsync(); +#line 5 + await testRunner.GivenAsync("LayoutCompilerService is running", ((string)(null)), ((global::Reqnroll.Table)(null)), "Given "); +#line hidden +#line 6 + await testRunner.AndAsync("the client has no Authorization token", ((string)(null)), ((global::Reqnroll.Table)(null)), "And "); +#line hidden +#line 7 + await testRunner.AndAsync("a RawLayout with a command element at grid position (1, 1) is stored", ((string)(null)), ((global::Reqnroll.Table)(null)), "And "); +#line hidden +#line 8 + await testRunner.WhenAsync("the test client calls POST /compile on the LayoutCompilerService endpoint with th" + + "e stored RawLayout", ((string)(null)), ((global::Reqnroll.Table)(null)), "When "); +#line hidden +#line 9 + await testRunner.ThenAsync("the response is 200 OK", ((string)(null)), ((global::Reqnroll.Table)(null)), "Then "); +#line hidden +#line 10 + await testRunner.AndAsync("the response body is valid JSON", ((string)(null)), ((global::Reqnroll.Table)(null)), "And "); +#line hidden +#line 11 + await testRunner.AndAsync("the response body represents a CompiledLayout", ((string)(null)), ((global::Reqnroll.Table)(null)), "And "); +#line hidden +#line 12 + await testRunner.AndAsync("the CompiledLayout contains a CommandDefinitionDto with name \"Up\"", ((string)(null)), ((global::Reqnroll.Table)(null)), "And "); +#line hidden +#line 13 + await testRunner.AndAsync("the CompiledLayout CSS definitions contain a rule for grid position (1, 1)", ((string)(null)), ((global::Reqnroll.Table)(null)), "And "); +#line hidden +#line 14 + await testRunner.AndAsync("the CompiledLayout does not contain authoring properties on its elements", ((string)(null)), ((global::Reqnroll.Table)(null)), "And "); +#line hidden +#line 15 + await testRunner.AndAsync("I should not see any warning or error messages in the LayoutCompilerService logs", ((string)(null)), ((global::Reqnroll.Table)(null)), "And "); +#line hidden + } + await this.ScenarioCleanupAsync(); + } + + [global::Microsoft.VisualStudio.TestTools.UnitTesting.TestMethodAttribute("Compile a preview layout")] + [global::Microsoft.VisualStudio.TestTools.UnitTesting.DescriptionAttribute("Compile a preview layout")] + [global::Microsoft.VisualStudio.TestTools.UnitTesting.TestPropertyAttribute("FeatureTitle", "LayoutCompilerService Endpoints")] + [global::Microsoft.VisualStudio.TestTools.UnitTesting.TestCategoryAttribute("ApiIntegrationTest")] + public async global::System.Threading.Tasks.Task CompileAPreviewLayout() + { + string[] tagsOfScenario = ((string[])(null)); + global::System.Collections.Specialized.OrderedDictionary argumentsOfScenario = new global::System.Collections.Specialized.OrderedDictionary(); + string pickleIndex = "1"; + global::Reqnroll.ScenarioInfo scenarioInfo = new global::Reqnroll.ScenarioInfo("Compile a preview layout", null, tagsOfScenario, argumentsOfScenario, featureTags, pickleIndex); + string[] tagsOfRule = ((string[])(null)); + global::Reqnroll.RuleInfo ruleInfo = null; +#line 17 +this.ScenarioInitialize(scenarioInfo, ruleInfo); +#line hidden + if ((global::Reqnroll.TagHelper.ContainsIgnoreTag(scenarioInfo.CombinedTags) || global::Reqnroll.TagHelper.ContainsIgnoreTag(featureTags))) + { + await testRunner.SkipScenarioAsync(); + } + else + { + await this.ScenarioStartAsync(); +#line 18 + await testRunner.GivenAsync("LayoutCompilerService is running", ((string)(null)), ((global::Reqnroll.Table)(null)), "Given "); +#line hidden +#line 19 + await testRunner.AndAsync("the client has no Authorization token", ((string)(null)), ((global::Reqnroll.Table)(null)), "And "); +#line hidden +#line 20 + await testRunner.AndAsync("a list of RawLayoutElementDto with a command element at grid position (1, 1) is s" + + "tored", ((string)(null)), ((global::Reqnroll.Table)(null)), "And "); +#line hidden +#line 21 + await testRunner.WhenAsync("the test client calls POST /compile/preview on the LayoutCompilerService endpoint" + + " with the stored elements", ((string)(null)), ((global::Reqnroll.Table)(null)), "When "); +#line hidden +#line 22 + await testRunner.ThenAsync("the response is 200 OK", ((string)(null)), ((global::Reqnroll.Table)(null)), "Then "); +#line hidden +#line 23 + await testRunner.AndAsync("the response body is valid JSON", ((string)(null)), ((global::Reqnroll.Table)(null)), "And "); +#line hidden +#line 24 + await testRunner.AndAsync("the response body represents a PreviewLayout", ((string)(null)), ((global::Reqnroll.Table)(null)), "And "); +#line hidden +#line 25 + await testRunner.AndAsync("the PreviewLayout RenderedHtml is non-empty", ((string)(null)), ((global::Reqnroll.Table)(null)), "And "); +#line hidden +#line 26 + await testRunner.AndAsync("the PreviewLayout RenderedCss is non-empty", ((string)(null)), ((global::Reqnroll.Table)(null)), "And "); +#line hidden +#line 27 + await testRunner.AndAsync("I should not see any warning or error messages in the LayoutCompilerService logs", ((string)(null)), ((global::Reqnroll.Table)(null)), "And "); +#line hidden + } + await this.ScenarioCleanupAsync(); + } + } +} +#pragma warning restore +#endregion diff --git a/test/AdaptiveRemote.Backend.LayoutCompilerService.Tests/AdaptiveRemote.Backend.LayoutCompilerService.Tests.csproj b/test/AdaptiveRemote.Backend.LayoutCompilerService.Tests/AdaptiveRemote.Backend.LayoutCompilerService.Tests.csproj new file mode 100644 index 00000000..6e702b00 --- /dev/null +++ b/test/AdaptiveRemote.Backend.LayoutCompilerService.Tests/AdaptiveRemote.Backend.LayoutCompilerService.Tests.csproj @@ -0,0 +1,32 @@ + + + + net10.0 + enable + enable + false + true + AdaptiveRemote.Backend.LayoutCompilerService.Tests + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/AdaptiveRemote.Backend.LayoutCompilerService.Tests/Services/LayoutCompilerTests.cs b/test/AdaptiveRemote.Backend.LayoutCompilerService.Tests/Services/LayoutCompilerTests.cs new file mode 100644 index 00000000..01790512 --- /dev/null +++ b/test/AdaptiveRemote.Backend.LayoutCompilerService.Tests/Services/LayoutCompilerTests.cs @@ -0,0 +1,279 @@ +using AdaptiveRemote.Backend.LayoutCompilerService.Services; +using AdaptiveRemote.Contracts; +using FluentAssertions; + +namespace AdaptiveRemote.Backend.LayoutCompilerService.Tests.Services; + +[TestClass] +public class LayoutCompilerTests +{ + private MockLogger _mockLogger = null!; + + [TestInitialize] + public void Setup() + { + _mockLogger = new MockLogger(); + } + + private LayoutCompiler CreateSut() => new(_mockLogger); + + // ─── Compile: single command element ────────────────────────────────────── + + [TestMethod] + public void LayoutCompiler_Compile_SingleCommandElement_ReturnsCompiledLayoutWithElement() + { + RawLayout raw = CreateRawLayoutWithCommand("cmd-up", "Up", gridRow: 1, gridColumn: 2); + + CompiledLayout result = CreateSut().Compile(raw); + + result.RawLayoutId.Should().Be(raw.Id); + result.UserId.Should().Be(raw.UserId); + result.Version.Should().Be(raw.Version); + result.IsActive.Should().BeFalse(); + result.Elements.Should().HaveCount(1); + result.Elements[0].Should().BeOfType(); + } + + [TestMethod] + public void LayoutCompiler_Compile_SingleCommandElement_StripsAuthoringProperties() + { + RawLayout raw = CreateRawLayoutWithCommand("cmd-up", "Up", gridRow: 1, gridColumn: 2, + gridRowSpan: 2, gridColumnSpan: 3, additionalCss: "color: red;"); + + CompiledLayout result = CreateSut().Compile(raw); + + CommandDefinitionDto cmd = result.Elements.OfType().Single(); + cmd.Name.Should().Be("Up"); + cmd.CssId.Should().Be("cmd-up"); + // CommandDefinitionDto has no grid/CSS authoring properties — confirmed by its type definition + } + + [TestMethod] + public void LayoutCompiler_Compile_SingleCommandElement_GeneratesCssRuleForGridPosition() + { + RawLayout raw = CreateRawLayoutWithCommand("cmd-up", "Up", gridRow: 3, gridColumn: 4); + + CompiledLayout result = CreateSut().Compile(raw); + + result.CssDefinitions.Should().Contain(".cmd-up {"); + result.CssDefinitions.Should().Contain("grid-row: 3 / span 1"); + result.CssDefinitions.Should().Contain("grid-column: 4 / span 1"); + } + + [TestMethod] + public void LayoutCompiler_Compile_ElementWithSpan_IncludesSpanInCss() + { + RawLayout raw = CreateRawLayoutWithCommand("cmd-big", "Big", gridRow: 1, gridColumn: 1, + gridRowSpan: 2, gridColumnSpan: 3); + + CompiledLayout result = CreateSut().Compile(raw); + + result.CssDefinitions.Should().Contain("grid-row: 1 / span 2"); + result.CssDefinitions.Should().Contain("grid-column: 1 / span 3"); + } + + [TestMethod] + public void LayoutCompiler_Compile_ElementWithAdditionalCss_IncludesAdditionalCssInDefinitions() + { + RawLayout raw = CreateRawLayoutWithCommand("cmd-power", "Power", gridRow: 1, gridColumn: 1, + additionalCss: "background-color: red;"); + + CompiledLayout result = CreateSut().Compile(raw); + + result.CssDefinitions.Should().Contain("background-color: red"); + } + + [TestMethod] + public void LayoutCompiler_Compile_GroupElement_GeneratesCssForGroupAndChildren() + { + RawLayoutGroupDefinitionDto group = new( + CssId: "nav-group", + Children: new List + { + new RawCommandDefinitionDto(CommandType.TiVo, "Up", "Up", null, "up", null, + "cmd-up", GridRow: 1, GridColumn: 1) + }, + GridRow: 0, GridColumn: 0 + ); + + RawLayout raw = CreateRawLayout(new List { group }); + + CompiledLayout result = CreateSut().Compile(raw); + + result.CssDefinitions.Should().Contain(".nav-group {"); + result.CssDefinitions.Should().Contain(".cmd-up {"); + result.Elements.Should().HaveCount(1); + result.Elements[0].Should().BeOfType(); + LayoutGroupDefinitionDto compiledGroup = (LayoutGroupDefinitionDto)result.Elements[0]; + compiledGroup.Children.Should().HaveCount(1); + compiledGroup.Children[0].Should().BeOfType(); + } + + [TestMethod] + public void LayoutCompiler_Compile_EmptyElements_ReturnsCssDefinitionsEmptyAndNoElements() + { + RawLayout raw = CreateRawLayout(Array.Empty()); + + CompiledLayout result = CreateSut().Compile(raw); + + result.Elements.Should().BeEmpty(); + result.CssDefinitions.Should().BeEmpty(); + } + + [TestMethod] + public void LayoutCompiler_Compile_LogsCompilationStartedAndCompleted() + { + RawLayout raw = CreateRawLayoutWithCommand("cmd-up", "Up", gridRow: 1, gridColumn: 1); + + CreateSut().Compile(raw); + + _mockLogger.VerifyMessages( + $"Information[1800]: Compilation started; rawLayoutId={raw.Id}", + $"Information[1801]: Compilation completed; rawLayoutId={raw.Id}"); + } + + // ─── CompilePreview ──────────────────────────────────────────────────────── + + [TestMethod] + public void LayoutCompiler_CompilePreview_SingleCommandElement_ReturnsNonEmptyHtmlAndCss() + { + IReadOnlyList elements = new List + { + new RawCommandDefinitionDto(CommandType.TiVo, "Up", "Up", null, "up", null, + "cmd-up", GridRow: 1, GridColumn: 1) + }; + + PreviewLayout result = CreateSut().CompilePreview(elements); + + result.RenderedHtml.Should().NotBeNullOrEmpty(); + result.RenderedCss.Should().NotBeNullOrEmpty(); + } + + [TestMethod] + public void LayoutCompiler_CompilePreview_SingleCommandElement_HtmlContainsElementDiv() + { + IReadOnlyList elements = new List + { + new RawCommandDefinitionDto(CommandType.TiVo, "Up", "Up", null, "up", null, + "cmd-up", GridRow: 1, GridColumn: 1) + }; + + PreviewLayout result = CreateSut().CompilePreview(elements); + + result.RenderedHtml.Should().Contain("class=\"cmd-up\""); + result.RenderedHtml.Should().Contain("layout-preview-grid"); + } + + [TestMethod] + public void LayoutCompiler_CompilePreview_SingleCommandElement_CssContainsGridRuleAndContainer() + { + IReadOnlyList elements = new List + { + new RawCommandDefinitionDto(CommandType.TiVo, "Up", "Up", null, "up", null, + "cmd-up", GridRow: 2, GridColumn: 3) + }; + + PreviewLayout result = CreateSut().CompilePreview(elements); + + result.RenderedCss.Should().Contain(".layout-preview-grid"); + result.RenderedCss.Should().Contain(".cmd-up"); + result.RenderedCss.Should().Contain("grid-row: 2"); + result.RenderedCss.Should().Contain("grid-column: 3"); + } + + [TestMethod] + public void LayoutCompiler_CompilePreview_ReturnsValidationResultWithIsValidTrue() + { + IReadOnlyList elements = new List + { + new RawCommandDefinitionDto(CommandType.TiVo, "Up", "Up", null, "up", null, + "cmd-up", GridRow: 1, GridColumn: 1) + }; + + PreviewLayout result = CreateSut().CompilePreview(elements); + + result.ValidationResult.IsValid.Should().BeTrue(); + result.ValidationResult.Issues.Should().BeEmpty(); + } + + [TestMethod] + public void LayoutCompiler_CompilePreview_ReturnsRawLayoutIdEmpty() + { + IReadOnlyList elements = new List + { + new RawCommandDefinitionDto(CommandType.TiVo, "Up", "Up", null, "up", null, + "cmd-up", GridRow: 1, GridColumn: 1) + }; + + PreviewLayout result = CreateSut().CompilePreview(elements); + + result.RawLayoutId.Should().Be(Guid.Empty); + } + + [TestMethod] + public void LayoutCompiler_CompilePreview_LogsPreviewCompilationStartedAndCompleted() + { + IReadOnlyList elements = new List + { + new RawCommandDefinitionDto(CommandType.TiVo, "Up", "Up", null, "up", null, + "cmd-up", GridRow: 1, GridColumn: 1) + }; + + CreateSut().CompilePreview(elements); + + _mockLogger.VerifyMessages( + "Information[1802]: Preview compilation started", + "Information[1803]: Preview compilation completed"); + } + + [TestMethod] + public void LayoutCompiler_CompilePreview_EmptyElements_ReturnsNonEmptyContainerHtml() + { + PreviewLayout result = CreateSut().CompilePreview(Array.Empty()); + + result.RenderedHtml.Should().Contain("layout-preview-grid"); + } + + // ─── Helpers ─────────────────────────────────────────────────────────────── + + private static RawLayout CreateRawLayoutWithCommand( + string cssId, + string name, + int gridRow, + int gridColumn, + int gridRowSpan = 1, + int gridColumnSpan = 1, + string? additionalCss = null) + { + return CreateRawLayout(new List + { + new RawCommandDefinitionDto( + Type: CommandType.TiVo, + Name: name, + Label: name, + Glyph: null, + SpeakPhrase: name.ToLowerInvariant(), + Reverse: null, + CssId: cssId, + GridRow: gridRow, + GridColumn: gridColumn, + GridRowSpan: gridRowSpan, + GridColumnSpan: gridColumnSpan, + AdditionalCss: additionalCss) + }); + } + + private static RawLayout CreateRawLayout(IReadOnlyList elements) + { + return new RawLayout( + Id: Guid.NewGuid(), + UserId: "test-user", + Name: "Test Layout", + Elements: elements, + Version: 1, + CreatedAt: DateTimeOffset.UtcNow, + UpdatedAt: DateTimeOffset.UtcNow, + ValidationResult: null + ); + } +} diff --git a/test/AdaptiveRemote.EndToEndTests.Steps/Backend/LayoutCompilerSteps.cs b/test/AdaptiveRemote.EndToEndTests.Steps/Backend/LayoutCompilerSteps.cs new file mode 100644 index 00000000..2a62807f --- /dev/null +++ b/test/AdaptiveRemote.EndToEndTests.Steps/Backend/LayoutCompilerSteps.cs @@ -0,0 +1,125 @@ +using System.Text.Json; +using System.Text.Json.Serialization.Metadata; +using AdaptiveRemote.Contracts; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Reqnroll; + +namespace AdaptiveRemote.EndToEndTests.Steps.Backend; + +[Binding] +public class LayoutCompilerSteps : StepsBase +{ + private RawLayout? _storedRawLayout; + private IReadOnlyList? _storedElements; + + [StepArgumentTransformation(nameof(PreviewLayout))] + public static JsonTypeInfo PreviewLayoutToJsonTypeInfo() => LayoutContractsJsonContext.Default.PreviewLayout; + + [Given(@"a RawLayout with a command element at grid position \((\d+), (\d+)\) is stored")] + public void GivenARawLayoutWithCommandElementAtGridPosition(int gridRow, int gridColumn) + { + _storedRawLayout = new RawLayout( + Id: Guid.NewGuid(), + UserId: "test-user", + Name: "Test Layout", + Elements: new List + { + new RawCommandDefinitionDto( + Type: CommandType.TiVo, + Name: "Up", + Label: "Up", + Glyph: null, + SpeakPhrase: "up", + Reverse: null, + CssId: "cmd-up", + GridRow: gridRow, + GridColumn: gridColumn) + }, + Version: 1, + CreatedAt: DateTimeOffset.UtcNow, + UpdatedAt: DateTimeOffset.UtcNow, + ValidationResult: null + ); + } + + [Given(@"a list of RawLayoutElementDto with a command element at grid position \((\d+), (\d+)\) is stored")] + public void GivenARawLayoutElementDtoListWithCommandElementAtGridPosition(int gridRow, int gridColumn) + { + _storedElements = new List + { + new RawCommandDefinitionDto( + Type: CommandType.TiVo, + Name: "Up", + Label: "Up", + Glyph: null, + SpeakPhrase: "up", + Reverse: null, + CssId: "cmd-up", + GridRow: gridRow, + GridColumn: gridColumn) + }; + } + + [When(@"the test client calls POST /compile on the (\w+) endpoint with the stored RawLayout")] + public void WhenTestClientCallsPostCompileWithStoredRawLayout(Uri endpointUrl) + { + Assert.IsNotNull(_storedRawLayout, "No RawLayout has been stored. Call the Given step first."); + string json = JsonSerializer.Serialize(_storedRawLayout, LayoutContractsJsonContext.Default.RawLayout); + TestClient.SendRequest(HttpMethod.Post, new Uri(endpointUrl, "/compile"), json); + } + + [When(@"the test client calls POST /compile/preview on the (\w+) endpoint with the stored elements")] + public void WhenTestClientCallsPostCompilePreviewWithStoredElements(Uri endpointUrl) + { + Assert.IsNotNull(_storedElements, "No elements have been stored. Call the Given step first."); + string json = JsonSerializer.Serialize(_storedElements, LayoutContractsJsonContext.Default.IReadOnlyListRawLayoutElementDto); + TestClient.SendRequest(HttpMethod.Post, new Uri(endpointUrl, "/compile/preview"), json); + } + + [Then(@"the CompiledLayout contains a CommandDefinitionDto with name {string}")] + public void ThenTheCompiledLayoutContainsACommandDefinitionDtoWithName(string expectedName) + { + CompiledLayout? layout = TestClient.LastResponseObject as CompiledLayout; + Assert.IsNotNull(layout, "Response was not parsed as a CompiledLayout. Call 'the response body represents a CompiledLayout' first."); + + bool found = layout.Elements.OfType().Any(c => c.Name == expectedName); + Assert.IsTrue(found, "Expected CompiledLayout.Elements to contain a CommandDefinitionDto with Name='{0}'.", expectedName); + } + + [Then(@"the CompiledLayout CSS definitions contain a rule for grid position \((\d+), (\d+)\)")] + public void ThenTheCompiledLayoutCssDefinitionsContainARuleForGridPosition(int gridRow, int gridColumn) + { + CompiledLayout? layout = TestClient.LastResponseObject as CompiledLayout; + Assert.IsNotNull(layout, "Response was not parsed as a CompiledLayout."); + Assert.IsFalse(string.IsNullOrEmpty(layout.CssDefinitions), "CompiledLayout.CssDefinitions is empty."); + Assert.IsTrue( + layout.CssDefinitions.Contains($"grid-row: {gridRow}") && layout.CssDefinitions.Contains($"grid-column: {gridColumn}"), + "Expected CssDefinitions to contain 'grid-row: {0}' and 'grid-column: {1}', but was:\n{2}", + gridRow, gridColumn, layout.CssDefinitions); + } + + [Then(@"the CompiledLayout does not contain authoring properties on its elements")] + public void ThenTheCompiledLayoutDoesNotContainAuthoringPropertiesOnElements() + { + string body = TestClient.LastResponseBody; + Assert.IsFalse(body.Contains("\"gridRow\""), "CompiledLayout JSON should not contain 'gridRow' on elements."); + Assert.IsFalse(body.Contains("\"gridColumn\""), "CompiledLayout JSON should not contain 'gridColumn' on elements."); + Assert.IsFalse(body.Contains("\"additionalCss\""), "CompiledLayout JSON should not contain 'additionalCss' on elements."); + } + + [Then(@"the PreviewLayout RenderedHtml is non-empty")] + public void ThenThePreviewLayoutRenderedHtmlIsNonEmpty() + { + PreviewLayout? preview = TestClient.LastResponseObject as PreviewLayout; + Assert.IsNotNull(preview, "Response was not parsed as a PreviewLayout. Call 'the response body represents a PreviewLayout' first."); + Assert.IsFalse(string.IsNullOrEmpty(preview.RenderedHtml), "PreviewLayout.RenderedHtml is empty."); + } + + [Then(@"the PreviewLayout RenderedCss is non-empty")] + public void ThenThePreviewLayoutRenderedCssIsNonEmpty() + { + PreviewLayout? preview = TestClient.LastResponseObject as PreviewLayout; + Assert.IsNotNull(preview, "Response was not parsed as a PreviewLayout."); + Assert.IsFalse(string.IsNullOrEmpty(preview.RenderedCss), "PreviewLayout.RenderedCss is empty."); + } +} diff --git a/test/AdaptiveRemote.EndToEndTests.Steps/Backend/ServiceSteps.cs b/test/AdaptiveRemote.EndToEndTests.Steps/Backend/ServiceSteps.cs index ddad7def..4fb00660 100644 --- a/test/AdaptiveRemote.EndToEndTests.Steps/Backend/ServiceSteps.cs +++ b/test/AdaptiveRemote.EndToEndTests.Steps/Backend/ServiceSteps.cs @@ -6,7 +6,7 @@ namespace AdaptiveRemote.EndToEndTests.Steps.Backend; [Binding] public class ServiceSteps : StepsBase { - private const string ServiceRegex = "(RawLayoutService|CompiledLayoutService|LayoutProcessingService)"; + private const string ServiceRegex = "(RawLayoutService|CompiledLayoutService|LayoutProcessingService|LayoutCompilerService)"; [StepArgumentTransformation(ServiceRegex)] public Uri ServiceNameToEndpointUri(string serviceName) @@ -19,6 +19,7 @@ public ServiceFixture ServiceNameToFixture(string serviceName) "RawLayoutService" => Environment.RawLayoutService, "CompiledLayoutService" => Environment.CompiledLayoutService, "LayoutProcessingService" => Environment.LayoutProcessingService, + "LayoutCompilerService" => Environment.LayoutCompilerService, _ => throw new ArgumentException($"Unknown service name: {serviceName}", nameof(serviceName)) }; diff --git a/test/AdaptiveRemote.EndToEndTests.Steps/Hooks/EnvironmentSetupHooks.cs b/test/AdaptiveRemote.EndToEndTests.Steps/Hooks/EnvironmentSetupHooks.cs index 25b58f3a..31e7a7cd 100644 --- a/test/AdaptiveRemote.EndToEndTests.Steps/Hooks/EnvironmentSetupHooks.cs +++ b/test/AdaptiveRemote.EndToEndTests.Steps/Hooks/EnvironmentSetupHooks.cs @@ -58,7 +58,8 @@ public static void OnAfterScenario_AttachLogsToTestResult(TestContext testContex ("Host", _startedEnvironment?.HostLogs), ("RawLayoutService", _startedEnvironment?.RawLayoutServiceLogs), ("CompiledLayoutService", _startedEnvironment?.CompiledLayoutServiceLogs), - ("LayoutProcessingService", _startedEnvironment?.LayoutProcessingServiceLogs) + ("LayoutProcessingService", _startedEnvironment?.LayoutProcessingServiceLogs), + ("LayoutCompilerService", _startedEnvironment?.LayoutCompilerServiceLogs) }; foreach ((string service, string? logLocation) in logsToAttach) diff --git a/test/AdaptiveRemote.EndToEndTests.Steps/LogVerificationSteps.cs b/test/AdaptiveRemote.EndToEndTests.Steps/LogVerificationSteps.cs index b0c43cd6..59b60ba2 100644 --- a/test/AdaptiveRemote.EndToEndTests.Steps/LogVerificationSteps.cs +++ b/test/AdaptiveRemote.EndToEndTests.Steps/LogVerificationSteps.cs @@ -12,7 +12,8 @@ public class LogVerificationSteps : StepsBase private const string RawLayoutServiceName = "RawLayoutService"; private const string CompiledLayoutServiceName = "CompiledLayoutService"; private const string LayoutProcessingServiceName = "LayoutProcessingService"; - private const string ServiceFilter = "(" + RawLayoutServiceName + "|" + CompiledLayoutServiceName + "|" + LayoutProcessingServiceName + ")"; + private const string LayoutCompilerServiceName = "LayoutCompilerService"; + private const string ServiceFilter = "(" + RawLayoutServiceName + "|" + CompiledLayoutServiceName + "|" + LayoutProcessingServiceName + "|" + LayoutCompilerServiceName + ")"; private static readonly Dictionary _lastLineRead = new(); @@ -207,6 +208,7 @@ private string GetLogFileFor(string serviceName) RawLayoutServiceName => Environment.RawLayoutServiceLogs, CompiledLayoutServiceName => Environment.CompiledLayoutServiceLogs, LayoutProcessingServiceName => Environment.LayoutProcessingServiceLogs, + LayoutCompilerServiceName => Environment.LayoutCompilerServiceLogs, _ => throw new ArgumentException($"Unexpected service name: {serviceName}", nameof(serviceName)) }; diff --git a/test/AdaptiveRemote.EndtoEndTests.TestServices/Backend/ServiceFixture.cs b/test/AdaptiveRemote.EndtoEndTests.TestServices/Backend/ServiceFixture.cs index 359575a0..7faccf55 100644 --- a/test/AdaptiveRemote.EndtoEndTests.TestServices/Backend/ServiceFixture.cs +++ b/test/AdaptiveRemote.EndtoEndTests.TestServices/Backend/ServiceFixture.cs @@ -48,7 +48,7 @@ public void StartService() // Find the repository root by looking for the .git directory string currentDir = Directory.GetCurrentDirectory(); string? repoRoot = currentDir; - while (repoRoot != null && !Directory.Exists(Path.Combine(repoRoot, ".git"))) + while (repoRoot != null && !Path.Exists(Path.Combine(repoRoot, ".git"))) { repoRoot = Directory.GetParent(repoRoot)?.FullName; } diff --git a/test/AdaptiveRemote.EndtoEndTests.TestServices/Host/ISimulatedEnvironment.cs b/test/AdaptiveRemote.EndtoEndTests.TestServices/Host/ISimulatedEnvironment.cs index 548599c5..e7cd4261 100644 --- a/test/AdaptiveRemote.EndtoEndTests.TestServices/Host/ISimulatedEnvironment.cs +++ b/test/AdaptiveRemote.EndtoEndTests.TestServices/Host/ISimulatedEnvironment.cs @@ -28,6 +28,8 @@ public interface ISimulatedEnvironment : IDisposable ServiceFixture LayoutProcessingService { get; } + ServiceFixture LayoutCompilerService { get; } + LocalStackFixture LocalStack { get; } void EnsureHostStarted(); @@ -46,6 +48,8 @@ public interface ISimulatedEnvironment : IDisposable string? LayoutProcessingServiceLogs { get; } + string? LayoutCompilerServiceLogs { get; } + /// /// Gets the test-time IR payloads that are programmed into the settings file. /// Keys are command names (e.g. "Power"); values are the raw IR bytes. diff --git a/test/AdaptiveRemote.EndtoEndTests.TestServices/Host/SimulatedEnvironment.cs b/test/AdaptiveRemote.EndtoEndTests.TestServices/Host/SimulatedEnvironment.cs index 4784e3dd..84700325 100644 --- a/test/AdaptiveRemote.EndtoEndTests.TestServices/Host/SimulatedEnvironment.cs +++ b/test/AdaptiveRemote.EndtoEndTests.TestServices/Host/SimulatedEnvironment.cs @@ -65,6 +65,7 @@ public SimulatedEnvironment(SimulatedTiVoDeviceBuilder tivoBuilder, SimulatedBro _lazyCompiledLayoutService = new(StartCompiledLayoutService); _lazyRawLayoutService = new(StartRawLayoutService); _lazyLayoutProcessingService = new(StartLayoutProcessingService); + _lazyLayoutCompilerService = new(StartLayoutCompilerService); _lazyLocalStackFixture = new(StartLocalStack); } @@ -83,6 +84,9 @@ public SimulatedEnvironment(SimulatedTiVoDeviceBuilder tivoBuilder, SimulatedBro private Lazy _lazyLayoutProcessingService; public ServiceFixture LayoutProcessingService => _lazyLayoutProcessingService.Value; + private Lazy _lazyLayoutCompilerService; + public ServiceFixture LayoutCompilerService => _lazyLayoutCompilerService.Value; + private Lazy _lazyLocalStackFixture; public LocalStackFixture LocalStack => _lazyLocalStackFixture.Value; @@ -109,6 +113,7 @@ private ServiceFixture StartLayoutProcessingService() ["RawLayoutService__BaseUrl"] = RawLayoutService.ServiceUrl, ["RawLayoutService__ServiceAccountToken"] = JwtAuthority.CreateToken("service-account-layout-processor"), ["CompiledLayoutService__BaseUrl"] = CompiledLayoutService.ServiceUrl, + ["LayoutCompilerService__BaseUrl"] = LayoutCompilerService.ServiceUrl, // Enable the orchestrator for pipeline tests ["Orchestrator__Enabled"] = "true", @@ -117,6 +122,13 @@ private ServiceFixture StartLayoutProcessingService() return fixture; } + private ServiceFixture StartLayoutCompilerService() + { + ServiceFixture fixture = new ServiceFixture("AdaptiveRemote.Backend.LayoutCompilerService", this); + fixture.StartService(); + return fixture; + } + private LocalStackFixture StartLocalStack() { LocalStackFixture fixture = new LocalStackFixture(LoggerFactory); @@ -149,6 +161,8 @@ public AdaptiveRemoteHost Host public string? LayoutProcessingServiceLogs => _lazyLayoutProcessingService.IsValueCreated ? _lazyLayoutProcessingService.Value.LogFilePath : null; + public string? LayoutCompilerServiceLogs => _lazyLayoutCompilerService.IsValueCreated ? _lazyLayoutCompilerService.Value.LogFilePath : null; + public string? LogFolder => _nextLogLocation is not null ? Path.GetDirectoryName(_nextLogLocation) : null; @@ -226,6 +240,18 @@ public void Dispose() // Ignore disposal errors } + try + { + if (_lazyLayoutCompilerService.IsValueCreated) + { + _lazyLayoutCompilerService.Value.Dispose(); + } + } + catch + { + // Ignore disposal errors + } + try { if (_lazyLocalStackFixture.IsValueCreated) From 6051e01b8f61731970514f31c6ab72c4178e4f43 Mon Sep 17 00:00:00 2001 From: Joe Davis Date: Thu, 21 May 2026 09:33:44 -0700 Subject: [PATCH 09/14] ADR-171: Fix validation failure trigger for HttpLayoutCompilerClient StubLayoutValidationClient was checking CssDefinitions == "INVALID", which was set by the now-replaced StubLayoutCompilerClient. With HttpLayoutCompilerClient, the signal is embedded in the element cssId: any cssId starting with "INVALID_" causes the real LayoutCompilerService to emit ".INVALID_" in CssDefinitions, which the stub then detects. Update the E2E test to use cssId "INVALID_up-btn" to exercise the failure path end-to-end. --- .../Services/StubLayoutValidationClient.cs | 12 +++++++----- .../LayoutProcessingServiceEndpoints.feature | 7 ++++--- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/AdaptiveRemote.Backend.LayoutProcessingService/Services/StubLayoutValidationClient.cs b/src/AdaptiveRemote.Backend.LayoutProcessingService/Services/StubLayoutValidationClient.cs index 82fcb6ec..e2253c2f 100644 --- a/src/AdaptiveRemote.Backend.LayoutProcessingService/Services/StubLayoutValidationClient.cs +++ b/src/AdaptiveRemote.Backend.LayoutProcessingService/Services/StubLayoutValidationClient.cs @@ -6,8 +6,9 @@ namespace AdaptiveRemote.Backend.LayoutProcessingService.Services; /// /// Stub implementation of ILayoutValidationClient. /// Returns a valid ValidationResult for all inputs by default. -/// When Validation:ForceInvalid is set to true (e.g. in integration tests), returns an -/// invalid result with a single stub issue, enabling end-to-end testing of the failure path. +/// Returns an invalid result when contains +/// ".INVALID_", which E2E tests trigger by giving a layout element a cssId starting with +/// "INVALID_". The real LayoutCompilerService emits that prefix verbatim in CSS. /// To be replaced with a real Lambda-backed HTTP client in ADR-172. /// public class StubLayoutValidationClient : ILayoutValidationClient @@ -21,9 +22,10 @@ public StubLayoutValidationClient(IConfiguration configuration) public Task ValidateAsync(CompiledLayout compiled, CancellationToken ct) { - // This check allows tests to force an invalid result by using the - // StubLayoutCompilerClient with a special RawLayout name. - if (compiled.CssDefinitions == "INVALID") + // E2E tests trigger the failure path by giving an element a cssId that starts with + // "INVALID_". The real LayoutCompilerService emits ".INVALID_ { ... }" in CSS, + // which this stub detects to return a failure result without any production-code changes. + if (compiled.CssDefinitions.Contains(".INVALID_", StringComparison.Ordinal)) { ValidationResult failure = new( IsValid: false, diff --git a/test/AdaptiveRemote.Backend.ApiTests/Features/LayoutProcessingServiceEndpoints.feature b/test/AdaptiveRemote.Backend.ApiTests/Features/LayoutProcessingServiceEndpoints.feature index 3f46357e..2e04287e 100644 --- a/test/AdaptiveRemote.Backend.ApiTests/Features/LayoutProcessingServiceEndpoints.feature +++ b/test/AdaptiveRemote.Backend.ApiTests/Features/LayoutProcessingServiceEndpoints.feature @@ -51,8 +51,9 @@ Scenario: End-to-end layout processing validation failure path Given LayoutProcessingService is running And the client has a valid Authorization token When this layout is created via RawLayoutService: - # Invalid because it has a special "name" that is considered invalid - # for testing purposes + # The element's cssId starts with "INVALID_", which causes LayoutCompilerService to + # emit ".INVALID_up-btn { ... }" in CssDefinitions. StubLayoutValidationClient + # detects that prefix and returns a failure result, exercising the failure path. """ { "userId": "test-user", @@ -65,7 +66,7 @@ Scenario: End-to-end layout processing validation failure path "label": "Up", "speakPhrase": "up", "reverse": "Down", - "cssId": "up-btn", + "cssId": "INVALID_up-btn", "gridRow": 0, "gridColumn": 0 } From abc6bdf97bd51ad5c63d61c8515c5b40daa32f8f Mon Sep 17 00:00:00 2001 From: Joe Davis Date: Thu, 21 May 2026 09:37:21 -0700 Subject: [PATCH 10/14] ADR-171: uncommitted changes at validation --- .../LayoutProcessingServiceEndpoints.feature.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/test/AdaptiveRemote.Backend.ApiTests/Features/LayoutProcessingServiceEndpoints.feature.cs b/test/AdaptiveRemote.Backend.ApiTests/Features/LayoutProcessingServiceEndpoints.feature.cs index 8472fa45..76d9c9b1 100644 --- a/test/AdaptiveRemote.Backend.ApiTests/Features/LayoutProcessingServiceEndpoints.feature.cs +++ b/test/AdaptiveRemote.Backend.ApiTests/Features/LayoutProcessingServiceEndpoints.feature.cs @@ -300,29 +300,29 @@ await testRunner.WhenAsync("this layout is created via RawLayoutService:", @"{ ""label"": ""Up"", ""speakPhrase"": ""up"", ""reverse"": ""Down"", - ""cssId"": ""up-btn"", + ""cssId"": ""INVALID_up-btn"", ""gridRow"": 0, ""gridColumn"": 0 } ] }", ((global::Reqnroll.Table)(null)), "When "); #line hidden -#line 75 +#line 76 await testRunner.ThenAsync("I should see a message that contains \"Layout compiled successfully\" in the Layout" + "ProcessingService logs", ((string)(null)), ((global::Reqnroll.Table)(null)), "Then "); #line hidden -#line 76 +#line 77 await testRunner.AndAsync("I should see a warning message in the LayoutProcessingService logs:", "Layout validation failed", ((global::Reqnroll.Table)(null)), "And "); #line hidden -#line 80 +#line 81 await testRunner.AndAsync("I should see a message that contains \"Validation result written back to raw layou" + "t\" in the LayoutProcessingService logs", ((string)(null)), ((global::Reqnroll.Table)(null)), "And "); #line hidden -#line 81 +#line 82 await testRunner.AndAsync("I should not see any warning or error messages in the LayoutProcessingService log" + "s", ((string)(null)), ((global::Reqnroll.Table)(null)), "And "); #line hidden -#line 82 +#line 83 await testRunner.AndAsync("I should not see any warning or error messages in the RawLayoutService logs", ((string)(null)), ((global::Reqnroll.Table)(null)), "And "); #line hidden } From 7fdda3c88fe7ef492e71cf0abe816adbb5e99186 Mon Sep 17 00:00:00 2001 From: Joe Davis Date: Thu, 21 May 2026 09:50:20 -0700 Subject: [PATCH 11/14] ADR-171: Add Lambda Test Tool launch profile for LayoutCompilerService _doc_BackendDevelopment.md requires all Lambda projects to have a launch profile for the Lambda Test Tool. This file was missing from the initial ADR-171 implementation. --- .../Properties/launchSettings.json | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 src/AdaptiveRemote.Backend.LayoutCompilerService/Properties/launchSettings.json diff --git a/src/AdaptiveRemote.Backend.LayoutCompilerService/Properties/launchSettings.json b/src/AdaptiveRemote.Backend.LayoutCompilerService/Properties/launchSettings.json new file mode 100644 index 00000000..588df937 --- /dev/null +++ b/src/AdaptiveRemote.Backend.LayoutCompilerService/Properties/launchSettings.json @@ -0,0 +1,14 @@ +{ + "profiles": { + "Mock Lambda Test Tool": { + "commandName": "Executable", + "commandLineArgs": "--port 5050", + "executablePath": "%USERPROFILE%\\.dotnet\\tools\\dotnet-lambda-test-tool-10.0.exe", + "launchBrowser": true, + "launchUrl": "http://localhost:5050", + "environmentVariables": { + "ASPNETCORE_ENVIRONMENT": "Development" + } + } + } +} From 258ea01e55116a5849abefed8f68f298f19d58d3 Mon Sep 17 00:00:00 2001 From: Joe Davis Date: Thu, 21 May 2026 09:50:36 -0700 Subject: [PATCH 12/14] =?UTF-8?q?ADR-171:=20Update=20=5Fdoc=5FBackendDevel?= =?UTF-8?q?opment.md=20=E2=80=94=20replace=20StubLayoutCompilerClient=20re?= =?UTF-8?q?ferences?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pipeline step 3 now reflects HttpLayoutCompilerClient (the real HTTP client added by ADR-171). Remove StubLayoutCompilerClient from the Stub implementations list; it is no longer used. --- src/_doc_BackendDevelopment.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/_doc_BackendDevelopment.md b/src/_doc_BackendDevelopment.md index d76d2785..a920b09f 100644 --- a/src/_doc_BackendDevelopment.md +++ b/src/_doc_BackendDevelopment.md @@ -21,7 +21,7 @@ then drives: fetch raw layout → compile → validate → store compiled layout 1. Dequeue SQS message containing `rawLayoutId` 2. Fetch `RawLayout` from `RawLayoutService` via `IRawLayoutRepository` -3. Compile via `ILayoutCompilerClient` (stub: `StubLayoutCompilerClient`) +3. Compile via `ILayoutCompilerClient` (real: `HttpLayoutCompilerClient` → `LayoutCompilerService` Lambda) 4. Validate via `ILayoutValidationClient` (stub: `StubLayoutValidationClient`) 5a. On validation failure: write result back via `IRawLayoutStatusWriter`; delete message 5b. On success: store compiled layout via `ICompiledLayoutRepository`; publish notification via `INotificationPublisher`; delete message @@ -29,7 +29,6 @@ then drives: fetch raw layout → compile → validate → store compiled layout **Stub implementations (current task):** -- `StubLayoutCompilerClient` — derives a `CompiledLayout` from `RawLayout` elements; no real CSS - `StubLayoutValidationClient` — always returns `IsValid=true`; set `Validation:ForceInvalid=true` to exercise the failure path - `StubNotificationPublisher` — no-op From 71d7d9f5367aa14107bb713ba46f91683311c76b Mon Sep 17 00:00:00 2001 From: Joe Davis Date: Thu, 21 May 2026 09:51:15 -0700 Subject: [PATCH 13/14] ADR-171: Remove dead _forceInvalid field and IConfiguration from StubLayoutValidationClient _forceInvalid was assigned from IConfiguration but never read after the stub was updated to detect .INVALID_ in CssDefinitions. Remove the field and the IConfiguration constructor injection. --- .../Services/StubLayoutValidationClient.cs | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/AdaptiveRemote.Backend.LayoutProcessingService/Services/StubLayoutValidationClient.cs b/src/AdaptiveRemote.Backend.LayoutProcessingService/Services/StubLayoutValidationClient.cs index e2253c2f..6e8fff12 100644 --- a/src/AdaptiveRemote.Backend.LayoutProcessingService/Services/StubLayoutValidationClient.cs +++ b/src/AdaptiveRemote.Backend.LayoutProcessingService/Services/StubLayoutValidationClient.cs @@ -1,5 +1,4 @@ using AdaptiveRemote.Contracts; -using Microsoft.Extensions.Configuration; namespace AdaptiveRemote.Backend.LayoutProcessingService.Services; @@ -13,13 +12,6 @@ namespace AdaptiveRemote.Backend.LayoutProcessingService.Services; /// public class StubLayoutValidationClient : ILayoutValidationClient { - private readonly bool _forceInvalid; - - public StubLayoutValidationClient(IConfiguration configuration) - { - _forceInvalid = configuration.GetValue("Validation:ForceInvalid", defaultValue: false); - } - public Task ValidateAsync(CompiledLayout compiled, CancellationToken ct) { // E2E tests trigger the failure path by giving an element a cssId that starts with From c452ce5e4823aa1c4a366096659514b41c919760 Mon Sep 17 00:00:00 2001 From: Joe Davis Date: Thu, 21 May 2026 10:11:26 -0700 Subject: [PATCH 14/14] ADR-171: Fix stale Validation:ForceInvalid reference in _doc_BackendDevelopment.md The config key was removed when StubLayoutValidationClient was updated to detect .INVALID_ in CssDefinitions. Update the stub description to document the current failure-path mechanism: give a layout element a cssId starting with INVALID_. --- src/_doc_BackendDevelopment.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_doc_BackendDevelopment.md b/src/_doc_BackendDevelopment.md index a920b09f..f9b2261b 100644 --- a/src/_doc_BackendDevelopment.md +++ b/src/_doc_BackendDevelopment.md @@ -29,7 +29,7 @@ then drives: fetch raw layout → compile → validate → store compiled layout **Stub implementations (current task):** -- `StubLayoutValidationClient` — always returns `IsValid=true`; set `Validation:ForceInvalid=true` to exercise the failure path +- `StubLayoutValidationClient` — always returns `IsValid=true`; give a layout element a `cssId` starting with `INVALID_` to exercise the failure path (the real `LayoutCompilerService` emits `.INVALID_` in `CssDefinitions`, which the stub detects) - `StubNotificationPublisher` — no-op **Service-to-service auth:** When calling `RawLayoutService`, the HTTP clients attach a bearer