From 9bbff7b016613d9c6e2a14c29e108b5b3d8aa51b Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Fri, 29 May 2026 15:00:06 +0800 Subject: [PATCH] claude skills --- .claude/rules/openwolf.md | 15 + .claude/settings.json | 72 + .claude/skills/kiro-spec-design/SKILL.md | 8 +- .../templates/specs/design.md | 331 +++++ .../templates/specs/research.md | 61 + .claude/skills/kiro-spec-init/SKILL.md | 6 +- .../kiro-spec-init/templates/specs/init.json | 24 + .../templates/specs/requirements-init.md | 9 + .claude/skills/kiro-spec-quick/SKILL.md | 8 +- .../kiro-spec-quick/templates/specs/init.json | 24 + .../templates/specs/requirements-init.md | 9 + .../skills/kiro-spec-requirements/SKILL.md | 2 +- .../templates/specs/requirements.md | 32 + .claude/skills/kiro-spec-tasks/SKILL.md | 6 +- .../kiro-spec-tasks/templates/specs/tasks.md | 24 + .claude/skills/kiro-steering-custom/SKILL.md | 6 +- .../steering-custom/api-standards.md | 69 + .../steering-custom/authentication.md | 67 + .../templates/steering-custom/database.md | 46 + .../templates/steering-custom/deployment.md | 54 + .../steering-custom/error-handling.md | 59 + .../templates/steering-custom/security.md | 55 + .../templates/steering-custom/testing.md | 47 + .claude/skills/kiro-steering/SKILL.md | 4 +- .../templates/steering/product.md | 18 + .../templates/steering/structure.md | 41 + .../kiro-steering/templates/steering/tech.md | 45 + .claude/ucai-formatter-cache.local.json | 1 + .kiro/specs/ha-config-bootstrap/brief.md | 1208 +++++++++++++++++ .../specs/ha-config-bootstrap/requirements.md | 151 +++ .kiro/specs/ha-config-bootstrap/research.md | 277 ++++ .kiro/steering/error-handling.md | 92 ++ .kiro/steering/product.md | 43 + .kiro/steering/security.md | 61 + .kiro/steering/structure.md | 99 ++ .kiro/steering/tech.md | 100 ++ .kiro/steering/testing.md | 89 ++ CLAUDE.md | 42 +- .../2026-05-20-config-repl-wire-protocol.md | 549 ++++++++ src/docs/config-replication-gap-analysis.md | 252 ++++ 40 files changed, 4072 insertions(+), 34 deletions(-) create mode 100644 .claude/rules/openwolf.md create mode 100644 .claude/settings.json create mode 100644 .claude/skills/kiro-spec-design/templates/specs/design.md create mode 100644 .claude/skills/kiro-spec-design/templates/specs/research.md create mode 100644 .claude/skills/kiro-spec-init/templates/specs/init.json create mode 100644 .claude/skills/kiro-spec-init/templates/specs/requirements-init.md create mode 100644 .claude/skills/kiro-spec-quick/templates/specs/init.json create mode 100644 .claude/skills/kiro-spec-quick/templates/specs/requirements-init.md create mode 100644 .claude/skills/kiro-spec-requirements/templates/specs/requirements.md create mode 100644 .claude/skills/kiro-spec-tasks/templates/specs/tasks.md create mode 100644 .claude/skills/kiro-steering-custom/templates/steering-custom/api-standards.md create mode 100644 .claude/skills/kiro-steering-custom/templates/steering-custom/authentication.md create mode 100644 .claude/skills/kiro-steering-custom/templates/steering-custom/database.md create mode 100644 .claude/skills/kiro-steering-custom/templates/steering-custom/deployment.md create mode 100644 .claude/skills/kiro-steering-custom/templates/steering-custom/error-handling.md create mode 100644 .claude/skills/kiro-steering-custom/templates/steering-custom/security.md create mode 100644 .claude/skills/kiro-steering-custom/templates/steering-custom/testing.md create mode 100644 .claude/skills/kiro-steering/templates/steering/product.md create mode 100644 .claude/skills/kiro-steering/templates/steering/structure.md create mode 100644 .claude/skills/kiro-steering/templates/steering/tech.md create mode 100644 .claude/ucai-formatter-cache.local.json create mode 100644 .kiro/specs/ha-config-bootstrap/brief.md create mode 100644 .kiro/specs/ha-config-bootstrap/requirements.md create mode 100644 .kiro/specs/ha-config-bootstrap/research.md create mode 100644 .kiro/steering/error-handling.md create mode 100644 .kiro/steering/product.md create mode 100644 .kiro/steering/security.md create mode 100644 .kiro/steering/structure.md create mode 100644 .kiro/steering/tech.md create mode 100644 .kiro/steering/testing.md create mode 100644 docs/superpowers/plans/2026-05-20-config-repl-wire-protocol.md create mode 100644 src/docs/config-replication-gap-analysis.md diff --git a/.claude/rules/openwolf.md b/.claude/rules/openwolf.md new file mode 100644 index 00000000..97854106 --- /dev/null +++ b/.claude/rules/openwolf.md @@ -0,0 +1,15 @@ +--- +description: OpenWolf protocol enforcement — active on all files +globs: **/* +--- + +- Check .wolf/anatomy.md before reading any project file +- Check .wolf/cerebrum.md Do-Not-Repeat list before generating code +- After writing or editing files, update .wolf/anatomy.md and append to .wolf/memory.md +- After receiving a user correction, update .wolf/cerebrum.md immediately (Preferences, Learnings, or Do-Not-Repeat) +- LEARN from every interaction: if you discover a convention, user preference, or project pattern, add it to .wolf/cerebrum.md. Low threshold — when in doubt, log it. +- BEFORE fixing any bug or error: read .wolf/buglog.json for known fixes +- AFTER fixing any bug, error, failed test, failed build, or user-reported problem: ALWAYS log to .wolf/buglog.json with error_message, root_cause, fix, and tags +- If you edit a file more than twice in a session, that likely indicates a bug — log it to .wolf/buglog.json +- When the user asks to check/evaluate UI design: run `openwolf designqc` to capture screenshots, then read them from .wolf/designqc-captures/ +- When the user asks to change/pick/migrate UI framework: read .wolf/reframe-frameworks.md, ask decision questions, recommend a framework, then execute with the framework's prompt diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 00000000..0e5a4cd1 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,72 @@ +{ + "hooks": { + "SessionStart": [ + { + "matcher": "", + "hooks": [ + { + "type": "command", + "command": "node \"$CLAUDE_PROJECT_DIR/.wolf/hooks/session-start.js\"", + "timeout": 5 + } + ] + } + ], + "PreToolUse": [ + { + "matcher": "Read", + "hooks": [ + { + "type": "command", + "command": "node \"$CLAUDE_PROJECT_DIR/.wolf/hooks/pre-read.js\"", + "timeout": 5 + } + ] + }, + { + "matcher": "Write|Edit|MultiEdit", + "hooks": [ + { + "type": "command", + "command": "node \"$CLAUDE_PROJECT_DIR/.wolf/hooks/pre-write.js\"", + "timeout": 5 + } + ] + } + ], + "PostToolUse": [ + { + "matcher": "Read", + "hooks": [ + { + "type": "command", + "command": "node \"$CLAUDE_PROJECT_DIR/.wolf/hooks/post-read.js\"", + "timeout": 5 + } + ] + }, + { + "matcher": "Write|Edit|MultiEdit", + "hooks": [ + { + "type": "command", + "command": "node \"$CLAUDE_PROJECT_DIR/.wolf/hooks/post-write.js\"", + "timeout": 10 + } + ] + } + ], + "Stop": [ + { + "matcher": "", + "hooks": [ + { + "type": "command", + "command": "node \"$CLAUDE_PROJECT_DIR/.wolf/hooks/stop.js\"", + "timeout": 10 + } + ] + } + ] + } +} \ No newline at end of file diff --git a/.claude/skills/kiro-spec-design/SKILL.md b/.claude/skills/kiro-spec-design/SKILL.md index 8a611780..5a006d07 100644 --- a/.claude/skills/kiro-spec-design/SKILL.md +++ b/.claude/skills/kiro-spec-design/SKILL.md @@ -27,9 +27,9 @@ Otherwise, load all necessary context: - `.kiro/specs/{feature}/research.md` (if exists, contains gap analysis from `/kiro-validate-gap`) - Core steering context: `product.md`, `tech.md`, `structure.md` - Additional steering files only when directly relevant to requirement coverage, architecture boundaries, integrations, runtime prerequisites, security/performance constraints, or team conventions that affect implementation readiness -- `.kiro/settings/templates/specs/design.md` for document structure +- `templates/specs/design.md` from this skill's directory for document structure - Read `rules/design-principles.md` from this skill's directory for design principles -- `.kiro/settings/templates/specs/research.md` for discovery log structure +- `templates/specs/research.md` from this skill's directory for discovery log structure **Validate requirements approval**: - If auto-approve flag is true: Auto-approve requirements in spec.json @@ -157,7 +157,7 @@ Provide brief summary in the language specified in spec.json: **Format**: Concise Markdown (under 200 words) - this is the command output, NOT the design document itself -**Note**: The actual design document follows `.kiro/settings/templates/specs/design.md` structure. +**Note**: The actual design document follows `templates/specs/design.md` (this skill's directory) structure. ## Safety & Fallback @@ -174,7 +174,7 @@ Provide brief summary in the language specified in spec.json: - **Suggested Action**: "Run `/kiro-spec-requirements {feature}` to generate requirements first" **Template Missing**: -- **User Message**: "Template file missing at `.kiro/settings/templates/specs/design.md`" +- **User Message**: "Template file missing at `templates/specs/design.md` in this skill's directory" - **Suggested Action**: "Check repository setup or restore template file" - **Fallback**: Use inline basic structure with warning diff --git a/.claude/skills/kiro-spec-design/templates/specs/design.md b/.claude/skills/kiro-spec-design/templates/specs/design.md new file mode 100644 index 00000000..1d31e71a --- /dev/null +++ b/.claude/skills/kiro-spec-design/templates/specs/design.md @@ -0,0 +1,331 @@ +# Design Document Template + +--- +**Purpose**: Provide sufficient detail to ensure implementation consistency across different implementers, preventing interpretation drift. + +**Approach**: +- Include essential sections that directly inform implementation decisions +- Omit optional sections unless critical to preventing implementation errors +- Match detail level to feature complexity +- Use diagrams and tables over lengthy prose + +**Warning**: Approaching 1000 lines indicates excessive feature complexity that may require design simplification or splitting into multiple specs. +--- + +> Sections may be reordered (e.g., surfacing Requirements Traceability earlier or moving Data Models nearer Architecture) when it improves clarity. Within each section, keep the flow **Summary → Scope → Decisions → Impacts/Risks** so reviewers can scan consistently. + +## Overview +2-3 paragraphs max +**Purpose**: This feature delivers [specific value] to [target users]. +**Users**: [Target user groups] will utilize this for [specific workflows]. +**Impact** (if applicable): Changes the current [system state] by [specific modifications]. + + +### Goals +- Primary objective 1 +- Primary objective 2 +- Success criteria + +### Non-Goals +- Explicitly excluded functionality +- Future considerations outside current scope +- Integration points deferred + +## Boundary Commitments + +State the responsibility boundary of this spec in concrete terms. Treat this as the anchor for architecture, tasks, and later validation. + +### This Spec Owns +- Capabilities and behaviors this spec is responsible for +- Data it owns or is authoritative for +- Interfaces or contracts it defines or stabilizes + +### Out of Boundary +- Related concerns this spec explicitly does NOT own +- Work deferred to another spec, existing subsystem, or later phase +- Changes this spec must not absorb as "just one more thing" + +### Allowed Dependencies +- Upstream systems/specs/components this design may depend on +- Shared infrastructure this design may use +- Dependency constraints that must not be violated + +### Revalidation Triggers +List the kinds of changes that should force dependent specs or consumers to re-check integration. + +- Contract shape changes +- Data ownership changes +- Dependency direction changes +- Startup/runtime prerequisite changes + +## Architecture + +> Reference detailed discovery notes in `research.md` only for background; keep design.md self-contained for reviewers by capturing all decisions and contracts here. +> Capture key decisions in text and let diagrams carry structural detail—avoid repeating the same information in prose. +> Supporting sections below should remain as light as possible unless they materially clarify the responsibility boundary, dependency rules, or integration seams. + +### Existing Architecture Analysis (if applicable) +When modifying existing systems: +- Current architecture patterns and constraints +- Existing domain boundaries to be respected +- Integration points that must be maintained +- Technical debt addressed or worked around + +### Architecture Pattern & Boundary Map +**RECOMMENDED**: Include Mermaid diagram showing the chosen architecture pattern and system boundaries (required for complex features, optional for simple additions) + +**Architecture Integration**: +- Selected pattern: [name and brief rationale] +- Domain/feature boundaries: [how responsibilities are separated to avoid conflicts] +- Existing patterns preserved: [list key patterns] +- New components rationale: [why each is needed] +- Steering compliance: [principles maintained] + +### Technology Stack + +| Layer | Choice / Version | Role in Feature | Notes | +|-------|------------------|-----------------|-------| +| Frontend / CLI | | | | +| Backend / Services | | | | +| Data / Storage | | | | +| Messaging / Events | | | | +| Infrastructure / Runtime | | | | + +> Keep rationale concise here and, when more depth is required (trade-offs, benchmarks), add a short summary plus pointer to the Supporting References section and `research.md` for raw investigation notes. + +## File Structure Plan + +Map the directory structure and file responsibilities for this feature. This section directly drives task `_Boundary:_` annotations and implementation Task Briefs. Use the appropriate level of detail: + +- **Small features**: List individual files with responsibilities +- **Large features**: Describe directory-level structure + per-domain/module pattern, list only non-obvious files individually + +### Directory Structure +``` +src/ +├── domain-a/ # Domain A responsibility +│ ├── controller.ts # Endpoint handlers +│ ├── service.ts # Business logic +│ └── types.ts # Domain types +├── domain-b/ # Domain B (same pattern as domain-a) +└── shared/ + └── cross-cutting.ts # Non-obvious: why this exists +``` + +> For repeated structures, describe the pattern once (e.g., "domain-b follows same pattern as domain-a"). List individual files only when their responsibility isn't obvious from the path. + +### Modified Files +- `path/to/existing.ts` — What changes and why + +> Each file should have one clear responsibility. Group files that change together. For repeated structures, describe the pattern once rather than listing every file. +> Avoid duplicating what Components and Interfaces already describes — focus on the physical file layout that Components maps to. + +## System Flows + +Provide only the diagrams needed to explain non-trivial flows. Use pure Mermaid syntax. Common patterns: +- Sequence (multi-party interactions) +- Process / state (branching logic or lifecycle) +- Data / event flow (pipelines, async messaging) + +Skip this section entirely for simple CRUD changes. +> Describe flow-level decisions (e.g., gating conditions, retries) briefly after the diagram instead of restating each step. + +## Requirements Traceability + +Use this section for complex or compliance-sensitive features where requirements span multiple domains. Straightforward 1:1 mappings can rely on the Components summary table. + +Map each requirement ID (e.g., `2.1`) to the design elements that realize it. + +| Requirement | Summary | Components | Interfaces | Flows | +|-------------|---------|------------|------------|-------| +| 1.1 | | | | | +| 1.2 | | | | | + +> Omit this section only when a single component satisfies a single requirement without cross-cutting concerns. + +## Components and Interfaces + +Provide a quick reference before diving into per-component details. + +- Summaries can be a table or compact list. Example table: + | Component | Domain/Layer | Intent | Req Coverage | Key Dependencies (P0/P1) | Contracts | + |-----------|--------------|--------|--------------|--------------------------|-----------| + | ExampleComponent | UI | Displays XYZ | 1, 2 | GameProvider (P0), MapPanel (P1) | Service, State | +- Only components introducing new boundaries (e.g., logic hooks, external integrations, persistence) require full detail blocks. Simple presentation components can rely on the summary row plus a short Implementation Note. + +Group detailed blocks by domain or architectural layer. For each detailed component, list requirement IDs as `2.1, 2.3` (omit “Requirement”). When multiple UI components share the same contract, reference a base interface/props definition instead of duplicating code blocks. + +### [Domain / Layer] + +#### [Component Name] + +| Field | Detail | +|-------|--------| +| Intent | 1-line description of the responsibility | +| Requirements | 2.1, 2.3 | +| Owner / Reviewers | (optional) | + +**Responsibilities & Constraints** +- Primary responsibility +- Domain boundary and transaction scope +- Data ownership / invariants + +**Dependencies** +- Inbound: Component/service name — purpose (Criticality) +- Outbound: Component/service name — purpose (Criticality) +- External: Service/library — purpose (Criticality) + +Summarize external dependency findings here; deeper investigation (API signatures, rate limits, migration notes) lives in `research.md`. + +**Contracts**: Service [ ] / API [ ] / Event [ ] / Batch [ ] / State [ ] ← check only the ones that apply. + +##### Service Interface +```typescript +interface [ComponentName]Service { + methodName(input: InputType): Result; +} +``` +- Preconditions: +- Postconditions: +- Invariants: + +##### API Contract +| Method | Endpoint | Request | Response | Errors | +|--------|----------|---------|----------|--------| +| POST | /api/resource | CreateRequest | Resource | 400, 409, 500 | + +##### Event Contract +- Published events: +- Subscribed events: +- Ordering / delivery guarantees: + +##### Batch / Job Contract +- Trigger: +- Input / validation: +- Output / destination: +- Idempotency & recovery: + +##### State Management +- State model: +- Persistence & consistency: +- Concurrency strategy: + +**Implementation Notes** +- Integration: +- Validation: +- Risks: + +## Data Models + +Focus on the portions of the data landscape that change with this feature. + +### Domain Model +- Aggregates and transactional boundaries +- Entities, value objects, domain events +- Business rules & invariants +- Optional Mermaid diagram for complex relationships + +### Logical Data Model + +**Structure Definition**: +- Entity relationships and cardinality +- Attributes and their types +- Natural keys and identifiers +- Referential integrity rules + +**Consistency & Integrity**: +- Transaction boundaries +- Cascading rules +- Temporal aspects (versioning, audit) + +### Physical Data Model +**When to include**: When implementation requires specific storage design decisions + +**For Relational Databases**: +- Table definitions with data types +- Primary/foreign keys and constraints +- Indexes and performance optimizations +- Partitioning strategy for scale + +**For Document Stores**: +- Collection structures +- Embedding vs referencing decisions +- Sharding key design +- Index definitions + +**For Event Stores**: +- Event schema definitions +- Stream aggregation strategies +- Snapshot policies +- Projection definitions + +**For Key-Value/Wide-Column Stores**: +- Key design patterns +- Column families or value structures +- TTL and compaction strategies + +### Data Contracts & Integration + +**API Data Transfer** +- Request/response schemas +- Validation rules +- Serialization format (JSON, Protobuf, etc.) + +**Event Schemas** +- Published event structures +- Schema versioning strategy +- Backward/forward compatibility rules + +**Cross-Service Data Management** +- Distributed transaction patterns (Saga, 2PC) +- Data synchronization strategies +- Eventual consistency handling + +Skip subsections that are not relevant to this feature. + +## Error Handling + +### Error Strategy +Concrete error handling patterns and recovery mechanisms for each error type. + +### Error Categories and Responses +**User Errors** (4xx): Invalid input → field-level validation; Unauthorized → auth guidance; Not found → navigation help +**System Errors** (5xx): Infrastructure failures → graceful degradation; Timeouts → circuit breakers; Exhaustion → rate limiting +**Business Logic Errors** (422): Rule violations → condition explanations; State conflicts → transition guidance + +**Process Flow Visualization** (when complex business logic exists): +Include Mermaid flowchart only for complex error scenarios with business workflows. + +### Monitoring +Error tracking, logging, and health monitoring implementation. + +## Testing Strategy + +### Default sections (adapt names/sections to fit the domain) +- Unit Tests: 3–5 items from core functions/modules (e.g., auth methods, subscription logic) +- Integration Tests: 3–5 cross-component flows (e.g., webhook handling, notifications) +- E2E/UI Tests (if applicable): 3–5 critical user paths (e.g., forms, dashboards) +- Performance/Load (if applicable): 3–4 items (e.g., concurrency, high-volume ops) + +## Optional Sections (include when relevant) + +### Security Considerations +_Use this section for features handling auth, sensitive data, external integrations, or user permissions. Capture only decisions unique to this feature; defer baseline controls to steering docs._ +- Threat modeling, security controls, compliance requirements +- Authentication and authorization patterns +- Data protection and privacy considerations + +### Performance & Scalability +_Use this section when performance targets, high load, or scaling concerns exist. Record only feature-specific targets or trade-offs and rely on steering documents for general practices._ +- Target metrics and measurement strategies +- Scaling approaches (horizontal/vertical) +- Caching strategies and optimization techniques + +### Migration Strategy +Include a Mermaid flowchart showing migration phases when schema/data movement is required. +- Phase breakdown, rollback triggers, validation checkpoints + +## Supporting References (Optional) +- Create this section only when keeping the information in the main body would hurt readability (e.g., very long TypeScript definitions, vendor option matrices, exhaustive schema tables). Keep decision-making context in the main sections so the design stays self-contained. +- Link to the supporting references from the main text instead of inlining large snippets. +- Background research notes and comparisons continue to live in `research.md`, but their conclusions must be summarized in the main design. diff --git a/.claude/skills/kiro-spec-design/templates/specs/research.md b/.claude/skills/kiro-spec-design/templates/specs/research.md new file mode 100644 index 00000000..b7c32906 --- /dev/null +++ b/.claude/skills/kiro-spec-design/templates/specs/research.md @@ -0,0 +1,61 @@ +# Research & Design Decisions Template + +--- +**Purpose**: Capture discovery findings, architectural investigations, and rationale that inform the technical design. + +**Usage**: +- Log research activities and outcomes during the discovery phase. +- Document design decision trade-offs that are too detailed for `design.md`. +- Provide references and evidence for future audits or reuse. +--- + +## Summary +- **Feature**: `` +- **Discovery Scope**: New Feature / Extension / Simple Addition / Complex Integration +- **Key Findings**: + - Finding 1 + - Finding 2 + - Finding 3 + +## Research Log +Document notable investigation steps and their outcomes. Group entries by topic for readability. + +### [Topic or Question] +- **Context**: What triggered this investigation? +- **Sources Consulted**: Links, documentation, API references, benchmarks +- **Findings**: Concise bullet points summarizing the insights +- **Implications**: How this affects architecture, contracts, or implementation + +_Repeat the subsection for each major topic._ + +## Architecture Pattern Evaluation +List candidate patterns or approaches that were considered. Use the table format where helpful. + +| Option | Description | Strengths | Risks / Limitations | Notes | +|--------|-------------|-----------|---------------------|-------| +| Hexagonal | Ports & adapters abstraction around core domain | Clear boundaries, testable core | Requires adapter layer build-out | Aligns with existing steering principle X | + +## Design Decisions +Record major decisions that influence `design.md`. Focus on choices with significant trade-offs. + +### Decision: `` +- **Context**: Problem or requirement driving the decision +- **Alternatives Considered**: + 1. Option A — short description + 2. Option B — short description +- **Selected Approach**: What was chosen and how it works +- **Rationale**: Why this approach fits the current project context +- **Trade-offs**: Benefits vs. compromises +- **Follow-up**: Items to verify during implementation or testing + +_Repeat the subsection for each decision._ + +## Risks & Mitigations +- Risk 1 — Proposed mitigation +- Risk 2 — Proposed mitigation +- Risk 3 — Proposed mitigation + +## References +Provide canonical links and citations (official docs, standards, ADRs, internal guidelines). +- [Title](https://example.com) — brief note on relevance +- ... diff --git a/.claude/skills/kiro-spec-init/SKILL.md b/.claude/skills/kiro-spec-init/SKILL.md index 2bc54fc5..a0917d1c 100644 --- a/.claude/skills/kiro-spec-init/SKILL.md +++ b/.claude/skills/kiro-spec-init/SKILL.md @@ -17,8 +17,8 @@ Generate a unique feature name from the project description ($ARGUMENTS) and ini 3. **Check Uniqueness**: Verify `.kiro/specs/` for naming conflicts. If the directory already exists with only `brief.md` (no `spec.json`), use that directory (discovery created it). 4. **Create Directory**: `.kiro/specs/[feature-name]/` (skip if already exists from discovery) 5. **Initialize Files Using Templates**: - - Read `.kiro/settings/templates/specs/init.json` - - Read `.kiro/settings/templates/specs/requirements-init.md` + - Read `templates/specs/init.json` from this skill's directory + - Read `templates/specs/requirements-init.md` from this skill's directory - Replace placeholders: - `{{FEATURE_NAME}}` → generated feature name - `{{TIMESTAMP}}` → current ISO 8601 timestamp @@ -46,6 +46,6 @@ Provide output in the language specified in `spec.json` with the following struc ## Safety & Fallback - **Ambiguous Feature Name**: If feature name generation is unclear, propose 2-3 options and ask user to select -- **Template Missing**: If template files don't exist in `.kiro/settings/templates/specs/`, report error with specific missing file path and suggest checking repository setup +- **Template Missing**: If template files don't exist in this skill's `templates/specs/`, report error with specific missing file path and suggest checking skill installation - **Directory Conflict**: If feature name already exists, append numeric suffix (e.g., `feature-name-2`) and notify user of automatic conflict resolution - **Write Failure**: Report error with specific path and suggest checking permissions or disk space diff --git a/.claude/skills/kiro-spec-init/templates/specs/init.json b/.claude/skills/kiro-spec-init/templates/specs/init.json new file mode 100644 index 00000000..410b1cd5 --- /dev/null +++ b/.claude/skills/kiro-spec-init/templates/specs/init.json @@ -0,0 +1,24 @@ +{ + "feature_name": "{{FEATURE_NAME}}", + "created_at": "{{TIMESTAMP}}", + "updated_at": "{{TIMESTAMP}}", + "language": "en", + "phase": "initialized", + "approvals": { + "requirements": { + "generated": false, + "approved": false + }, + "design": { + "generated": false, + "approved": false + }, + "tasks": { + "generated": false, + "approved": false + } + }, + "ready_for_implementation": false +} + + diff --git a/.claude/skills/kiro-spec-init/templates/specs/requirements-init.md b/.claude/skills/kiro-spec-init/templates/specs/requirements-init.md new file mode 100644 index 00000000..edde05de --- /dev/null +++ b/.claude/skills/kiro-spec-init/templates/specs/requirements-init.md @@ -0,0 +1,9 @@ +# Requirements Document + +## Project Description (Input) +{{PROJECT_DESCRIPTION}} + +## Requirements +<!-- Will be generated in /kiro-spec-requirements phase --> + + diff --git a/.claude/skills/kiro-spec-quick/SKILL.md b/.claude/skills/kiro-spec-quick/SKILL.md index 8df3fe12..ac5fc449 100644 --- a/.claude/skills/kiro-spec-quick/SKILL.md +++ b/.claude/skills/kiro-spec-quick/SKILL.md @@ -72,10 +72,10 @@ Execute these 4 phases in order: 5. **Initialize Files from Templates**: - a. Read templates: + a. Read templates from this skill's directory: ``` - - .kiro/settings/templates/specs/init.json - - .kiro/settings/templates/specs/requirements-init.md + - templates/specs/init.json + - templates/specs/requirements-init.md ``` b. Replace placeholders: @@ -230,7 +230,7 @@ Next Steps: ### Error Scenarios **Template Missing**: -- Check `.kiro/settings/templates/specs/` exists +- Check `templates/specs/` in this skill's directory exists - Report specific missing file - Exit with error diff --git a/.claude/skills/kiro-spec-quick/templates/specs/init.json b/.claude/skills/kiro-spec-quick/templates/specs/init.json new file mode 100644 index 00000000..410b1cd5 --- /dev/null +++ b/.claude/skills/kiro-spec-quick/templates/specs/init.json @@ -0,0 +1,24 @@ +{ + "feature_name": "{{FEATURE_NAME}}", + "created_at": "{{TIMESTAMP}}", + "updated_at": "{{TIMESTAMP}}", + "language": "en", + "phase": "initialized", + "approvals": { + "requirements": { + "generated": false, + "approved": false + }, + "design": { + "generated": false, + "approved": false + }, + "tasks": { + "generated": false, + "approved": false + } + }, + "ready_for_implementation": false +} + + diff --git a/.claude/skills/kiro-spec-quick/templates/specs/requirements-init.md b/.claude/skills/kiro-spec-quick/templates/specs/requirements-init.md new file mode 100644 index 00000000..edde05de --- /dev/null +++ b/.claude/skills/kiro-spec-quick/templates/specs/requirements-init.md @@ -0,0 +1,9 @@ +# Requirements Document + +## Project Description (Input) +{{PROJECT_DESCRIPTION}} + +## Requirements +<!-- Will be generated in /kiro-spec-requirements phase --> + + diff --git a/.claude/skills/kiro-spec-requirements/SKILL.md b/.claude/skills/kiro-spec-requirements/SKILL.md index 76976a7e..cbf3189a 100644 --- a/.claude/skills/kiro-spec-requirements/SKILL.md +++ b/.claude/skills/kiro-spec-requirements/SKILL.md @@ -32,7 +32,7 @@ Otherwise, load all necessary context: ### Step 2: Read Guidelines - Read `rules/ears-format.md` from this skill's directory for EARS syntax rules - Read `rules/requirements-review-gate.md` from this skill's directory for pre-write review criteria -- Read `.kiro/settings/templates/specs/requirements.md` for document structure +- Read `templates/specs/requirements.md` from this skill's directory for document structure #### Parallel Research (subagent dispatch) diff --git a/.claude/skills/kiro-spec-requirements/templates/specs/requirements.md b/.claude/skills/kiro-spec-requirements/templates/specs/requirements.md new file mode 100644 index 00000000..42424014 --- /dev/null +++ b/.claude/skills/kiro-spec-requirements/templates/specs/requirements.md @@ -0,0 +1,32 @@ +# Requirements Document + +## Introduction +{{INTRODUCTION}} + +<!-- Optional when scope could be misread or the feature touches adjacent systems/specs --> +## Boundary Context (Optional) +- **In scope**: {{IN_SCOPE_BEHAVIORS}} +- **Out of scope**: {{OUT_OF_SCOPE_BEHAVIORS}} +- **Adjacent expectations**: {{ADJACENT_SYSTEM_OR_SPEC_EXPECTATIONS}} + +## Requirements + +### Requirement 1: {{REQUIREMENT_AREA_1}} +<!-- Requirement headings MUST include a leading numeric ID only (for example: "Requirement 1: ...", "1. Overview", "2 Feature: ..."). Alphabetic IDs like "Requirement A" are not allowed. --> +**Objective:** As a {{ROLE}}, I want {{CAPABILITY}}, so that {{BENEFIT}} + +#### Acceptance Criteria +1. When [event], the [system] shall [response/action] +2. If [trigger], then the [system] shall [response/action] +3. While [precondition], the [system] shall [response/action] +4. Where [feature is included], the [system] shall [response/action] +5. The [system] shall [response/action] + +### Requirement 2: {{REQUIREMENT_AREA_2}} +**Objective:** As a {{ROLE}}, I want {{CAPABILITY}}, so that {{BENEFIT}} + +#### Acceptance Criteria +1. When [event], the [system] shall [response/action] +2. When [event] and [condition], the [system] shall [response/action] + +<!-- Additional requirements follow the same pattern --> diff --git a/.claude/skills/kiro-spec-tasks/SKILL.md b/.claude/skills/kiro-spec-tasks/SKILL.md index e8cf9d0a..6f8376f4 100644 --- a/.claude/skills/kiro-spec-tasks/SKILL.md +++ b/.claude/skills/kiro-spec-tasks/SKILL.md @@ -39,7 +39,7 @@ Otherwise, load all necessary context: - Read `rules/tasks-generation.md` from this skill's directory for principles - Read `rules/tasks-parallel-analysis.md` from this skill's directory for parallel judgement criteria -- Read `.kiro/settings/templates/specs/tasks.md` for format (supports `(P)` markers) +- Read `templates/specs/tasks.md` from this skill's directory for format (supports `(P)` markers) #### Parallel Research @@ -176,9 +176,9 @@ Provide brief summary in the language specified in spec.json: - **Suggested Action**: "Refine requirements.md or design.md, then re-run `/kiro-spec-tasks {feature}`" **Template/Rules Missing**: -- **User Message**: "Template or rules files missing in `.kiro/settings/`" +- **User Message**: "Template or rules files missing in this skill's `templates/specs/` or `rules/` directory" - **Fallback**: Use inline basic structure with warning -- **Suggested Action**: "Check repository setup or restore template files" +- **Suggested Action**: "Check skill installation or restore template files" - **Missing Numeric Requirement IDs**: - **Stop Execution**: All requirements in requirements.md MUST have numeric IDs. If any requirement lacks a numeric ID, stop and request that requirements.md be fixed before generating tasks. diff --git a/.claude/skills/kiro-spec-tasks/templates/specs/tasks.md b/.claude/skills/kiro-spec-tasks/templates/specs/tasks.md new file mode 100644 index 00000000..98298741 --- /dev/null +++ b/.claude/skills/kiro-spec-tasks/templates/specs/tasks.md @@ -0,0 +1,24 @@ +# Implementation Plan + +## Task Format Template + +Use whichever pattern fits the work breakdown: + +### Major task only +- [ ] {{NUMBER}}. {{TASK_DESCRIPTION}}{{PARALLEL_MARK}} + - {{DETAIL_ITEM_1}} *(Include details only when needed. If the task stands alone, omit bullet items.)* + - _Requirements: {{REQUIREMENT_IDS}}_ + +### Major + Sub-task structure +- [ ] {{MAJOR_NUMBER}}. {{MAJOR_TASK_SUMMARY}} +- [ ] {{MAJOR_NUMBER}}.{{SUB_NUMBER}} {{SUB_TASK_DESCRIPTION}}{{SUB_PARALLEL_MARK}} + - {{DETAIL_ITEM_1}} + - {{DETAIL_ITEM_2}} + - {{OBSERVABLE_COMPLETION_ITEM}} *(At least one detail item should state the observable completion condition for this task.)* + - _Requirements: {{REQUIREMENT_IDS}}_ *(IDs only; do not add descriptions or parentheses.)* + - _Boundary: {{COMPONENT_NAMES}}_ *(Only for (P) tasks. Omit when scope is obvious.)* + - _Depends: {{TASK_IDS}}_ *(Only for non-obvious cross-boundary dependencies. Most tasks omit this.)* + +> **Parallel marker**: Append ` (P)` only to tasks that can be executed in parallel. Omit the marker when running in `--sequential` mode. +> +> **Optional test coverage**: When a sub-task is deferrable test work tied to acceptance criteria, mark the checkbox as `- [ ]*` and explain the referenced requirements in the detail bullets. diff --git a/.claude/skills/kiro-steering-custom/SKILL.md b/.claude/skills/kiro-steering-custom/SKILL.md index 9a83d57b..2292f5e0 100644 --- a/.claude/skills/kiro-steering-custom/SKILL.md +++ b/.claude/skills/kiro-steering-custom/SKILL.md @@ -27,7 +27,7 @@ You are a specialized skill for creating custom steering documents beyond core f If steering context is already available from conversation, skip redundant file reads. Otherwise: -- Check `.kiro/settings/templates/steering-custom/` for available templates +- Check `templates/steering-custom/` in this skill's directory for available templates - Read `rules/steering-principles.md` from this skill's directory for steering principles ## Workflow @@ -37,7 +37,7 @@ Otherwise: - Specific requirements or patterns to document 2. **Check if template exists**: - - Load from `.kiro/settings/templates/steering-custom/{name}.md` if available + - Load from `templates/steering-custom/{name}.md` in this skill's directory if available - Use as starting point, customize based on project 3. **Analyze codebase** (JIT) for relevant patterns: @@ -60,7 +60,7 @@ After all parallel research completes, synthesize findings for steering document ## Available Templates -Templates available in `.kiro/settings/templates/steering-custom/`: +Templates available in `templates/steering-custom/` (this skill's directory): 1. **api-standards.md** - REST/GraphQL conventions, error handling 2. **testing.md** - Test organization, mocking, coverage diff --git a/.claude/skills/kiro-steering-custom/templates/steering-custom/api-standards.md b/.claude/skills/kiro-steering-custom/templates/steering-custom/api-standards.md new file mode 100644 index 00000000..acf6d4e3 --- /dev/null +++ b/.claude/skills/kiro-steering-custom/templates/steering-custom/api-standards.md @@ -0,0 +1,69 @@ +# API Standards + +[Purpose: consistent API patterns for naming, structure, auth, versioning, and errors] + +## Philosophy +- Prefer predictable, resource-oriented design +- Be explicit in contracts; minimize breaking changes +- Secure by default (auth first, least privilege) + +## Endpoint Pattern +``` +/{version}/{resource}[/{id}][/{sub-resource}] +``` +Examples: +- `/api/v1/users` +- `/api/v1/users/:id` +- `/api/v1/users/:id/posts` + +HTTP verbs: +- GET (read, safe, idempotent) +- POST (create) +- PUT/PATCH (update) +- DELETE (remove, idempotent) + +## Request/Response + +Request (typical): +```json +{ "data": { ... }, "metadata": { "requestId": "..." } } +``` + +Success: +```json +{ "data": { ... }, "meta": { "timestamp": "...", "version": "..." } } +``` + +Error: +```json +{ "error": { "code": "ERROR_CODE", "message": "...", "field": "optional" } } +``` +(See error-handling for rules.) + +## Status Codes (pattern) +- 2xx: Success (200 read, 201 create, 204 delete) +- 4xx: Client issues (400 validation, 401/403 auth, 404 missing) +- 5xx: Server issues (500 generic, 503 unavailable) +Choose the status that best reflects the outcome. + +## Authentication +- Credentials in standard location +``` +Authorization: Bearer {token} +``` +- Reject unauthenticated before business logic + +## Versioning +- Version via URL/header/media-type +- Breaking change → new version +- Non-breaking → same version +- Provide deprecation window and comms + +## Pagination/Filtering (if applicable) +- Pagination: `page`, `pageSize` or cursor-based +- Filtering: explicit query params +- Sorting: `sort=field:asc|desc` +Return pagination metadata in `meta`. + +--- +_Focus on patterns and decisions, not endpoint catalogs._ diff --git a/.claude/skills/kiro-steering-custom/templates/steering-custom/authentication.md b/.claude/skills/kiro-steering-custom/templates/steering-custom/authentication.md new file mode 100644 index 00000000..e6afadfc --- /dev/null +++ b/.claude/skills/kiro-steering-custom/templates/steering-custom/authentication.md @@ -0,0 +1,67 @@ +# Authentication & Authorization Standards + +[Purpose: unify auth model, token/session lifecycle, permission checks, and security] + +## Philosophy +- Clear separation: authentication (who) vs authorization (what) +- Secure by default: least privilege, fail closed, short-lived tokens +- UX-aware: friction where risk is high, smooth otherwise + +## Authentication + +### Method (choose + rationale) +- Options: JWT, Session, OAuth2, hybrid +- Choice: [our method] because [reason] + +### Flow (high-level) +``` +1) User proves identity (credentials or provider) +2) Server verifies and issues token/session +3) Client sends token per request +4) Server verifies token and proceeds +``` + +### Token/Session Lifecycle +- Storage: httpOnly cookie or Authorization header +- Expiration: short-lived access, longer refresh (if used) +- Refresh: rotate tokens; respect revocation +- Revocation: blacklist/rotate on logout/compromise + +### Security Pattern +- Enforce TLS; never expose tokens to JS when avoidable +- Bind token to audience/issuer; include minimal claims +- Consider device binding and IP/risk checks for sensitive actions + +## Authorization + +### Permission Model +- Choose one: RBAC / ABAC / ownership-based / hybrid +- Define roles/attributes centrally; avoid hardcoding across codebase + +### Checks (where to enforce) +- Route/middleware: coarse-grained gate +- Domain/service: fine-grained decisions +- UI: conditional rendering (no security reliance) + +Example pattern: +```typescript +requirePermission('resource:action'); // route +if (!user.can('resource:action')) throw ForbiddenError(); // domain +``` + +### Ownership +- Pattern: owner OR privileged role can act +- Verify on entity boundary before mutation + +## Passwords & MFA +- Passwords: strong policy, hashed (bcrypt/argon2), never plaintext +- Reset: time-limited token, single-use, notify user +- MFA: step-up for risky operations (policy-driven) + +## API-to-API Auth +- Use API keys or OAuth client credentials +- Scope keys minimally; rotate and audit usage +- Rate limit by identity (user/key) + +--- +_Focus on patterns and decisions. No library-specific code._ diff --git a/.claude/skills/kiro-steering-custom/templates/steering-custom/database.md b/.claude/skills/kiro-steering-custom/templates/steering-custom/database.md new file mode 100644 index 00000000..bfd7c592 --- /dev/null +++ b/.claude/skills/kiro-steering-custom/templates/steering-custom/database.md @@ -0,0 +1,46 @@ +# Database Standards + +[Purpose: guide schema design, queries, migrations, and integrity] + +## Philosophy +- Model the domain first; optimize after correctness +- Prefer explicit constraints; let database enforce invariants +- Query only what you need; measure before optimizing + +## Naming & Types +- Tables: `snake_case`, plural (`users`, `order_items`) +- Columns: `snake_case` (`created_at`, `user_id`) +- FKs: `{table}_id` referencing `{table}.id` +- Types: timezone-aware timestamps; strong IDs; precise money types + +## Relationships +- 1:N: FK in child +- N:N: join table with compound key +- 1:1: FK + UNIQUE + +## Migrations +- Immutable migrations; always add rollback +- Small, focused steps; test on non-prod first +- Naming: `{seq}_{action}_{object}` (e.g., `002_add_email_index`) + +## Query Patterns +- ORM for simple CRUD and safety; raw SQL for complex/perf-critical +- Avoid N+1 (eager load/batching); paginate large sets +- Index FKs and frequently filtered/sorted columns + +## Connection & Transactions +- Use pooling (size/timeouts based on workload) +- One connection per unit of work; close/return promptly +- Wrap multi-step changes in transactions + +## Data Integrity +- Use NOT NULL/UNIQUE/CHECK/FK constraints +- Validate at DB when appropriate (defense in depth) +- Prefer generated columns for consistent derivations + +## Backup & Recovery +- Regular backups with retention; test restores +- Document RPO/RTO targets; monitor backup jobs + +--- +_Focus on patterns and decisions. No environment-specific settings._ diff --git a/.claude/skills/kiro-steering-custom/templates/steering-custom/deployment.md b/.claude/skills/kiro-steering-custom/templates/steering-custom/deployment.md new file mode 100644 index 00000000..0d97a0bd --- /dev/null +++ b/.claude/skills/kiro-steering-custom/templates/steering-custom/deployment.md @@ -0,0 +1,54 @@ +# Deployment Standards + +[Purpose: safe, repeatable releases with clear environment and pipeline patterns] + +## Philosophy +- Automate; test before deploy; verify after deploy +- Prefer incremental rollout with fast rollback +- Production changes must be observable and reversible + +## Environments +- Dev: fast iteration; debugging enabled +- Staging: mirrors prod; release validation +- Prod: hardened; monitored; least privilege + +## CI/CD Flow +``` +Code → Test → Build → Scan → Deploy (staged) → Verify +``` +Principles: +- Fail fast on tests/scans; block deploy +- Artifact builds are reproducible (lockfiles, pinned versions) +- Manual approval for prod; auditable trail + +## Deployment Strategies +- Rolling: gradual instance replacement +- Blue-Green: switch traffic between two pools +- Canary: small % users first, expand on health +Choose per risk profile; document default. + +## Zero-Downtime & Migrations +- Health checks gate traffic; graceful shutdown +- Backwards-compatible DB changes during rollout +- Separate migration step; test rollback paths + +## Rollback +- Keep previous version ready; automate revert +- Rollback faster than fix-forward; document triggers + +## Configuration & Secrets +- 12-factor config via env; never commit secrets +- Secret manager; rotate; least privilege; audit access +- Validate required env vars at startup + +## Health & Monitoring +- Endpoints: `/health`, `/health/live`, `/health/ready` +- Monitor latency, error rate, throughput, saturation +- Alerts on SLO breaches/spikes; tune to avoid fatigue + +## Incident Response & DR +- Standard playbook: detect → assess → mitigate → communicate → resolve → post-mortem +- Backups with retention; test restore; defined RPO/RTO + +--- +_Focus on rollout patterns and safeguards. No provider-specific steps._ diff --git a/.claude/skills/kiro-steering-custom/templates/steering-custom/error-handling.md b/.claude/skills/kiro-steering-custom/templates/steering-custom/error-handling.md new file mode 100644 index 00000000..2d534398 --- /dev/null +++ b/.claude/skills/kiro-steering-custom/templates/steering-custom/error-handling.md @@ -0,0 +1,59 @@ +# Error Handling Standards + +[Purpose: unify how errors are classified, shaped, propagated, logged, and monitored] + +## Philosophy +- Fail fast where possible; degrade gracefully at system boundaries +- Consistent error shape across the stack (human + machine readable) +- Handle known errors close to source; surface unknowns to a global handler + +## Classification (decide handling by source) +- Client: Input/validation/user action issues → 4xx +- Server: System failures/unexpected exceptions → 5xx +- Business: Rule/state violations → 4xx (e.g., 409) +- External: 3rd-party/network failures → map to 5xx or 4xx with context + +## Error Shape (single canonical format) +```json +{ + "error": { + "code": "ERROR_CODE", + "message": "Human-readable message", + "requestId": "trace-id", + "timestamp": "ISO-8601" + } +} +``` +Principles: stable code enums, no secrets, include trace info. + +## Propagation (where to convert) +- API layer: Convert domain errors → HTTP status + canonical body +- Service layer: Throw typed business errors, avoid stringly-typed errors +- Data/external layer: Wrap provider errors with safe, actionable codes +- Unknown errors: Bubble to global handler → 500 + generic message + +Example pattern: +```typescript +try { return await useCase(); } +catch (e) { + if (e instanceof BusinessError) return respondMapped(e); + logError(e); return respondInternal(); +} +``` + +## Logging (context over noise) +Log: operation, userId (if available), code, message, stack, requestId, minimal context. +Do not log: passwords, tokens, secrets, full PII, full bodies with sensitive data. +Levels: ERROR (failures), WARN (recoverable/edge), INFO (key events), DEBUG (diagnostics). + +## Retry (only when safe) +Retry when: network/timeouts/transient 5xx AND operation is idempotent. +Do not retry: 4xx, business errors, non-idempotent flows. +Strategy: exponential backoff + jitter, capped attempts; require idempotency keys. + +## Monitoring & Health +Track: error rates by code/category, latency, saturation; alert on spikes/SLI breaches. +Expose health: `/health` (live), `/health/ready` (ready). Link errors to traces. + +--- +_Focus on patterns and decisions. No implementation details or exhaustive lists._ diff --git a/.claude/skills/kiro-steering-custom/templates/steering-custom/security.md b/.claude/skills/kiro-steering-custom/templates/steering-custom/security.md new file mode 100644 index 00000000..aca172a3 --- /dev/null +++ b/.claude/skills/kiro-steering-custom/templates/steering-custom/security.md @@ -0,0 +1,55 @@ +# Security Standards + +[Purpose: define security posture with patterns for validation, authz, secrets, and data] + +## Philosophy +- Defense in depth; least privilege; secure by default; fail closed +- Validate at boundaries; sanitize for context; never trust input +- Separate authentication (who) and authorization (what) + +## Input & Output +- Validate at API boundaries and UI forms; enforce types and constraints +- Sanitize/escape based on destination (HTML, SQL, shell, logs) +- Prefer allow-lists over block-lists; reject early with minimal detail + +## Authentication & Authorization +- Authentication: verify identity; issue short-lived tokens/sessions +- Authorization: check permissions before actions; deny by default +- Centralize policies; avoid duplicating checks across code + +Pattern: +```typescript +if (!user.hasPermission('resource:action')) throw ForbiddenError(); +``` + +## Secrets & Configuration +- Never commit secrets; store in secret manager or env +- Rotate regularly; audit access; scope minimal +- Validate required env vars at startup; fail fast on missing + +## Sensitive Data +- Minimize collection; mask/redact in logs; encrypt at rest and in transit +- Restrict access by role/need-to-know; track access to sensitive records + +## Session/Token Security +- httpOnly + secure cookies where possible; TLS everywhere +- Short expiration; rotate on refresh; revoke on logout/compromise +- Bind tokens to audience/issuer; include minimal claims + +## Logging (security-aware) +- Log auth attempts, permission denials, and sensitive operations +- Never log passwords, tokens, secrets, full PII; avoid full bodies +- Include requestId and context to correlate events + +## Headers & Transport +- Enforce TLS; HSTS +- Set security headers (CSP, X-Frame-Options, X-Content-Type-Options) +- Prefer modern crypto; disable weak protocols/ciphers + +## Vulnerability Posture +- Prefer secure libraries; keep dependencies updated +- Static/dynamic scans in CI; track and remediate +- Educate team on common classes; encode as patterns above + +--- +_Focus on patterns and principles. Link concrete configs to ops docs._ diff --git a/.claude/skills/kiro-steering-custom/templates/steering-custom/testing.md b/.claude/skills/kiro-steering-custom/templates/steering-custom/testing.md new file mode 100644 index 00000000..f4e22fe9 --- /dev/null +++ b/.claude/skills/kiro-steering-custom/templates/steering-custom/testing.md @@ -0,0 +1,47 @@ +# Testing Standards + +[Purpose: guide what to test, where tests live, and how to structure them] + +## Philosophy +- Test behavior, not implementation +- Prefer fast, reliable tests; minimize brittle mocks +- Cover critical paths deeply; breadth over 100% pursuit + +## Organization +Options: +- Co-located: `component.tsx` + `component.test.tsx` +- Separate: `/src/...` and `/tests/...` +Pick one as default; allow exceptions with rationale. + +Naming: +- Files: `*.test.*` or `*.spec.*` +- Suites: what is under test; Cases: expected behavior + +## Test Types +- Unit: single unit, mocked dependencies, very fast +- Integration: multiple units together, mock externals only +- E2E: full flows, minimal mocks, only for critical journeys + +## Structure (AAA) +```typescript +it('does X when Y', () => { + // Arrange + const input = setup(); + // Act + const result = act(input); + // Assert + expect(result).toEqual(expected); +}); +``` + +## Mocking & Data +- Mock externals (API/DB); never mock the system under test +- Use factories/fixtures; reset state between tests +- Keep test data minimal and intention-revealing + +## Coverage +- Target: [% overall]; higher for critical domains +- Enforce thresholds in CI; exceptions require review rationale + +--- +_Focus on patterns and decisions. Tool-specific config lives elsewhere._ diff --git a/.claude/skills/kiro-steering/SKILL.md b/.claude/skills/kiro-steering/SKILL.md index 541d3102..93cff6dd 100644 --- a/.claude/skills/kiro-steering/SKILL.md +++ b/.claude/skills/kiro-steering/SKILL.md @@ -30,7 +30,7 @@ You are a specialized skill for maintaining `.kiro/steering/` as persistent proj If steering context is already available from conversation, skip redundant file reads. -- For Bootstrap mode: Read templates from `.kiro/settings/templates/steering/` +- For Bootstrap mode: Read templates from `templates/steering/` in this skill's directory - For Sync mode: Read all existing `.kiro/steering/*.md` files - Read `rules/steering-principles.md` from this skill's directory for steering principles @@ -45,7 +45,7 @@ Check `.kiro/steering/` status: ## Bootstrap Flow -1. Load templates from `.kiro/settings/templates/steering/` +1. Load templates from `templates/steering/` in this skill's directory 2. Analyze codebase (JIT): #### Parallel Research diff --git a/.claude/skills/kiro-steering/templates/steering/product.md b/.claude/skills/kiro-steering/templates/steering/product.md new file mode 100644 index 00000000..a0f43fa7 --- /dev/null +++ b/.claude/skills/kiro-steering/templates/steering/product.md @@ -0,0 +1,18 @@ +# Product Overview + +[Brief description of what this product does and who it serves] + +## Core Capabilities + +[3-5 key capabilities, not exhaustive features] + +## Target Use Cases + +[Primary scenarios this product addresses] + +## Value Proposition + +[What makes this product unique or valuable] + +--- +_Focus on patterns and purpose, not exhaustive feature lists_ diff --git a/.claude/skills/kiro-steering/templates/steering/structure.md b/.claude/skills/kiro-steering/templates/steering/structure.md new file mode 100644 index 00000000..01162476 --- /dev/null +++ b/.claude/skills/kiro-steering/templates/steering/structure.md @@ -0,0 +1,41 @@ +# Project Structure + +## Organization Philosophy + +[Describe approach: feature-first, layered, domain-driven, etc.] + +## Directory Patterns + +### [Pattern Name] +**Location**: `/path/` +**Purpose**: [What belongs here] +**Example**: [Brief example] + +### [Pattern Name] +**Location**: `/path/` +**Purpose**: [What belongs here] +**Example**: [Brief example] + +## Naming Conventions + +- **Files**: [Pattern, e.g., PascalCase, kebab-case] +- **Components**: [Pattern] +- **Functions**: [Pattern] + +## Import Organization + +```typescript +// Example import patterns +import { Something } from '@/path' // Absolute +import { Local } from './local' // Relative +``` + +**Path Aliases**: +- `@/`: [Maps to] + +## Code Organization Principles + +[Key architectural patterns and dependency rules] + +--- +_Document patterns, not file trees. New files following patterns shouldn't require updates_ diff --git a/.claude/skills/kiro-steering/templates/steering/tech.md b/.claude/skills/kiro-steering/templates/steering/tech.md new file mode 100644 index 00000000..c7b25b40 --- /dev/null +++ b/.claude/skills/kiro-steering/templates/steering/tech.md @@ -0,0 +1,45 @@ +# Technology Stack + +## Architecture + +[High-level system design approach] + +## Core Technologies + +- **Language**: [e.g., TypeScript, Python] +- **Framework**: [e.g., React, Next.js, Django] +- **Runtime**: [e.g., Node.js 20+] + +## Key Libraries + +[Only major libraries that influence development patterns] + +## Development Standards + +### Type Safety +[e.g., TypeScript strict mode, no `any`] + +### Code Quality +[e.g., ESLint, Prettier rules] + +### Testing +[e.g., Jest, coverage requirements] + +## Development Environment + +### Required Tools +[Key tools and version requirements] + +### Common Commands +```bash +# Dev: [command] +# Build: [command] +# Test: [command] +``` + +## Key Technical Decisions + +[Important architectural choices and rationale] + +--- +_Document standards and patterns, not every dependency_ diff --git a/.claude/ucai-formatter-cache.local.json b/.claude/ucai-formatter-cache.local.json new file mode 100644 index 00000000..cf1386ff --- /dev/null +++ b/.claude/ucai-formatter-cache.local.json @@ -0,0 +1 @@ +{"detected":false,"exe":null,"args":null,"extensions":[]} \ No newline at end of file diff --git a/.kiro/specs/ha-config-bootstrap/brief.md b/.kiro/specs/ha-config-bootstrap/brief.md new file mode 100644 index 00000000..fb5c37c0 --- /dev/null +++ b/.kiro/specs/ha-config-bootstrap/brief.md @@ -0,0 +1,1208 @@ +# Brief: ha-election-raft-config + +## Problem + +The current LSF-style priority-based election has three critical safety gaps: + +1. **Config freshness not guaranteed**: A node with missing or stale config can be elected primary and serve incorrect cluster state to workers/schedulers. + +2. **False self-elections during concurrent startup**: When pc13v1 starts while pc13v (the priority-0 primary) is still initializing, pc13v's peer listener port is not open yet. pc13v1's election probe gets ECONNREFUSED, falsely concludes the primary is absent, and self-elects. Both nodes declare themselves primary. + +3. **No bootstrap path for genuine failover**: When a standby is promoted during real failover (original primary down), it has no mechanism to pull config from peer standbys if its local `conf/current` is missing. + +Reproduction from logs: +- pc13v starts at 12:30:03, pc13v1 starts at 12:30:22 (19s later) +- pc13v1 probes pc13v:9990 during election, times out within 3046ms +- pc13v is healthy but still in `load_primary_config()` or early phase2 — listener not open +- pc13v1 wins: "no higher-priority candidate responded" +- pc13v1 calls `load_primary_config()` → "Primary has no config at '/tmp/xce/pc13v1/conf/current' (rc=-2)" → fatal exit + +## Current State + +- **Election** (ha.c:243–306): LSF-style priority-based. Priority-0 always wins unconditionally; priority-N probes [0..N-1] and self-elects on timeout. No quorum. No config-version check. Logically correct for simple cases but unsafe for config freshness. +- **Listener timing** (myapp.c:21–96): `bind()+listen()` happens inside `cluster_init_phase2()` AFTER election and config load. Creates a 2–3 second window where the primary is unreachable to probes. +- **Config versioning** (config_repl.c): SHA-256 tagged, atomic writes, retention policy, staleness detection (`conf_stale` flag). Complete and correct. +- **Config fetch** (config_repl.c:891–965): standby→primary pull framework exists; `do_one_config_fetch()` returns `-ENOSYS` (stub, Task 11 on branch). +- **Bootstrap after failover**: not implemented. Design §14.4 deferred quorum consensus. +- **Simultaneous primaries**: `ha_check_priority_stepdown()` (ha.c:322) has a term=0 exact check that fails to resolve races when both nodes increment term before heartbeat exchange. + +## Availability Trade-off (CP chosen) + +This design chooses **Consistency over Availability** (CP in CAP terms): + +- A cluster of N nodes requires a **majority (≥(N+1)/2)** for both **election** and **write commits**. +- **Election**: A minority partition cannot elect a primary. The cluster is unavailable until the majority partition is restored. +- **Writes (WAL replication)**: Primary waits for majority standby ACKs before confirming a write (job submit, queue change, state update). In-flight writes that cannot reach majority are rejected. This adds **~1 LAN RTT per write** (~0.1–5ms depending on network). +- **Zero data loss on failover**: A promoted standby is guaranteed to have all committed WAL entries because majority replication ensures durability before write ACK. +- **Operational implication**: Plan for N≥3 nodes. A 2-node cluster has no meaningful majority; it is equivalent to single-node with a cold standby. For HPC, zero data loss (no lost job submissions, no stale queue state) outweighs the write-latency cost. + +## Desired Outcome + +1. **Raft-style quorum election (CP)**: A candidate must obtain ACKs from a strict majority of peers (≥(N+1)/2) before becoming primary. A minority partition cannot elect a primary. The cluster is unavailable until the majority partition is restored. + +2. **Config freshness encoded in votes**: Each vote reply includes the peer's config tag and WAL LSN. The candidate with the freshest config (highest tag, then highest LSN) among quorum members wins tiebreaks. + +3. **No-config nodes bootstrap after election (deferred fetch)**: A node with `config_tag=0` + never initiates candidacy. It probes peers to determine whether config exists anywhere, + but **defers the actual fetch until the primary is known** — the election proves who + has the freshest state, so bootstrap fetches from the proven winner rather than + speculatively guessing. Three sub-cases: + - `tag=0`, peers have config → set `defer_config_fetch`, participate as voter only, fetch from winner after election + - `tag=0`, all peers also `tag=0` → first-boot cluster, proceed to election as-is + - `tag=0`, no peer reachable → retry: probe loop with exponential backoff (500ms→10s); + log WARN every attempt; block indefinitely until a peer responds or admin kills + +4. **Listener before election**: Peer listener socket opens before election so concurrent-startup probes reach an open port. False self-elections during overlapping startup are eliminated. + +5. **Config freshness preserved through all paths**: A node always has a valid config + before entering phase 2 — either from local disk (normal case) or fetched from the + proven election winner after the election resolves (no-config case, deferred fetch in + Change C). A quorum winner therefore always has `config_tag > 0` when peers do, and + serves correct cluster state immediately after promotion with no additional bootstrap step. + +6. **Most up-to-date primary**: Among all reachable nodes, only the one with the + highest config_tag (then highest LSN) can become primary. A candidate that wins + the vote count but sees a fresher responding peer yields back to standby — + the fresher peer runs its own election and wins. Non-responding (partitioned or + crashed) peers never block election. + +7. **No simultaneous primaries**: Only one node can obtain a quorum at any given term. Term-based fencing prevents split-brain on partition heal. `voted_for` tracking (persisted to `ha-term.bin`) enforces one vote per term per node across restarts. + +8. **Majority-commit WAL replication (CP writes)**: Primary waits for majority (≥(N+1)/2) standby ACKs before marking a WAL entry committed and confirming the write to the caller. In-flight writes that cannot reach majority within a timeout are rejected. Zero data loss on failover: promoted standby has all committed entries. + +9. **Majority-commit config push (CP config)**: Config upgrades (`OP_upgradeConfig`) use the + same `repl_broadcast()` + `repl_wait_majority_ack()` infrastructure as WAL replication — + the primary waits for ≥(N+1)/2 standby ACKs before confirming a config change to the + operator. Operator changes cannot be silently lost on failover. + +10. **Hard SSL/cert gate**: Controller refuses to boot if TLS cert/key files are missing or unreadable. Operator-friendly error message with file path. + +11. **Observable via logs**: Election results, vote tallies, quorum counts, config tag comparisons, bootstrap operations, WAL commit quorum status, config push quorum status, and cert validation logged at INFO or WARN severity. + +## Approach + +**Replace LSF priority election with Raft-style quorum voting AND add majority-commit WAL replication for full CP durability. Config upgrades use the same majority-commit infrastructure for uniform CP semantics.** + +Eight focused changes: + +### Change A — Move listener, iomgr, and vtaskmgr before election + +Extract `bind()+listen()`, `iomgr_init()`, and `vtaskmgr_init()` from +`cluster_init_phase2()` so the full connection framework is running before election. +`vtaskmgr_init()` uses the new `--vcpus N` CLI parameter (Change D item 1) instead of +`cluster_config->vcpus`, removing the last config dependency on this startup block. + +New startup order in `myapp.c`: +``` +validate_peer_count() ← Change D: odd-N check before any init +validate_certs() ← Change D: cert pre-flight +cluster_init_phase1() ← ha_init() reads ha-term.bin: restores term + voted_for + ← conf_read_tag() reads config_version_tag from disk (0 if absent) + ← Node now has durable identity before network is up +create_ssl_context() +iomgr_init() + iomgr_set_ssl_ctx() +cluster_open_listener(port) ← bind + listen; register fd with iomgr +vtaskmgr_init(vcpus) +iomgr_start() +vtaskmgr_start() +atomic_store(ha.role, CTRL_INIT) ← node now answers OP_requestVote + OP_probeConfigTag +bootstrap_config_if_needed() ← Change C: if config_tag==0, probe peers; defers fetch until primary known +ha_wait_for_heartbeat_or_timeout() ← Raft: sem_timedwait(election_timeout + rand_jitter) + ← heartbeat received → CTRL_STANDBY, skip election + ← timeout fires → CTRL_ELECTING → am_i_primary_quorum() +[primary or standby path] +load_primary_config() / ha_connect_to_primary() +cluster_init_phase2_rest() ← kvmgr, xstring, jobmgr, delaymgr, apiserver, etc. +cluster_run() +``` + +With iomgr and vtaskmgr running before election, incoming `OP_requestVote` messages +are handled by the normal dispatch switch — no special pre-framework path needed. +Outgoing vote requests use the existing `connect_ssl_raw()` + `xce_encode()/xce_decode()` +pattern (same as config fetch), keeping per-peer threads lightweight. + +**Full 4-state controller FSM (`ha_role_t`)**: + +The controller lifecycle is a single FSM stored in `c->ha.role` (atomic): +``` +CTRL_INIT ──── timeout + jitter ──→ CTRL_ELECTING ──── quorum won ──→ CTRL_PRIMARY + └─── heartbeat from │ │ + existing primary ─→ └─── quorum lost ─→ CTRL_STANDBY ────┘ + │ + EPOLLRDHUP / heartbeat timeout + │ + CTRL_ELECTING +``` + +**Dispatch gate** (top of vtaskmgr switch, before any `case`): +```c +switch (atomic_load(&c->ha.role)) { +case CTRL_INIT: +case CTRL_ELECTING: + // Only vote + config-probe pass; all others get NOT_READY and retry (Change G) + if (msg->op != OP_requestVote && msg->op != OP_probeConfigTag) { + xce_send_error(task, XCE_ERR_NOT_READY, "controller not ready"); + return; + } + break; +case CTRL_STANDBY: + // Primary-only ops rejected with XCE_ERR_STANDBY + redirect_host/port (Change G) + break; +case CTRL_PRIMARY: + break; // full dispatch +} +``` + +**State transition triggers**: +- `CTRL_INIT → CTRL_ELECTING`: `ha_wait_for_heartbeat_or_timeout()` times out — no + heartbeat received within `election_timeout + rand_jitter`. Different jitter per node + (xorshift64 from `jitter_seed`) means nodes fire at staggered times, so the first + candidate typically wins unopposed. This is Raft's primary split-vote prevention. +- `CTRL_INIT → CTRL_STANDBY`: heartbeat or `OP_ctrlHello` received from an existing + primary during the wait window — node skips election entirely. +- `CTRL_ELECTING → CTRL_PRIMARY`: `am_i_primary_quorum()` returns 1. +- `CTRL_ELECTING → CTRL_STANDBY`: `am_i_primary_quorum()` returns 0. +- `CTRL_STANDBY → CTRL_ELECTING` (fast): `EPOLLRDHUP` on `primary_ctrl_fd`. +- `CTRL_STANDBY → CTRL_ELECTING` (fallback): heartbeat timeout vtask fires — primary + is alive on TCP but not sending heartbeats. Vtask is reset on each heartbeat received; + fires after `election_timeout + rand_jitter` of silence. + +**`ha_wait_for_heartbeat_or_timeout()`** in `myapp.c`: +```c +struct timespec dl = now_plus_ms(c->ha.config.election_timeout_ms + + ha_rand_jitter(&c->ha)); +int hb = (sem_timedwait(&c->ha.heartbeat_seen, &dl) == 0); +if (hb) { + atomic_store(&c->ha.role, CTRL_STANDBY); +} else { + atomic_store(&c->ha.role, CTRL_ELECTING); + rc = am_i_primary_quorum(c); +} +``` +`ha.heartbeat_seen` is posted by the heartbeat dispatch handler when any heartbeat +or `OP_ctrlHello` is received while `ha.role == CTRL_INIT`. + +### Change B — Raft-style quorum voting in `am_i_primary()` + +Replace the current timeout-based probe with a full request-vote protocol: + +**New wire message: OP_requestVote** (Cap'n Proto) +```c +struct RequestVoteRequest { + term @0 :UInt64; // candidate's term + candidate_id @1 :Text; // candidate's name (from --peers) + config_tag @2 :UInt64; // candidate's config SHA-256 tag (0 if none) + last_log_lsn @3 :UInt64; // candidate's WAL applied_lsn + // NOTE: priority removed — Raft config_tag+LSN freshness fully determines winner; + // jitter breaks split votes. Priority plays no role in quorum election. +} + +struct RequestVoteReply { + term @0 :UInt64; // peer's current term + vote_granted @1 :Bool; // true = ACK + peer_config_tag @2 :UInt64; // peer's config tag (for config comparison) + peer_last_log_lsn @3 :UInt64; // peer's WAL LSN (for log comparison) +} +``` + +**Election uses iomgr + vtaskmgr (FSM model)**: + +Since iomgr and vtaskmgr are running before election (Change A), vote requests are +sent via iomgr's async connect path and replies arrive through the normal vtaskmgr +dispatch switch. No election-specific threads are created. + +**New `ha_vote_entry_t`** struct (ha.h) — per-peer election slice; only meaningful as +`ctx->votes[i]` inside `ha_elect_ctx_t`: +```c +typedef struct { + xce_candidate_t *peer; + uint64_t my_term; // term sent in RequestVote + uint64_t my_tag; // config_tag sent in RequestVote + uint64_t my_lsn; // WAL LSN sent in RequestVote + int vote_granted; // 1 = peer granted vote + int replied; // 1 = peer responded (any reply, including denial) + uint64_t peer_tag; // peer's config_version_tag from reply + uint64_t peer_lsn; // peer's applied_lsn from reply + // NOTE: no pthread_t — FSM uses iomgr async sends + vtaskmgr dispatch; + // ha_elect_ctx_t.done semaphore blocks am_i_primary_quorum() +} ha_vote_entry_t; +``` + +**New `ha_elect_ctx_t`** struct (ha.h) — holds all election state; single allocation via +C99 flexible array member (one `calloc`, one `free`): +```c +typedef struct { + int n_peers; + int votes_granted; // includes self-vote (starts at 1) + int n_replied; // peers that responded (any reply) + uint64_t max_resp_tag; // freshest config_tag seen across all replies + uint64_t max_resp_lsn; // freshest lsn seen across all replies + int result; // set before sem_post: 1=primary, 0=standby, <0=error + sem_t done; // startup thread blocks here; dispatch posts when done + ha_vote_entry_t votes[]; // C99 flexible array — n_peers-1 entries +} ha_elect_ctx_t; +// Allocate: calloc(1, sizeof(*ctx) + (n_peers-1)*sizeof(ctx->votes[0])) +// Free: free(ctx) — single allocation, no separate votes free needed +``` + +**`am_i_primary_quorum()`** (ha.c) — initiates election, blocks until done: +```c +int am_i_primary_quorum(cluster_t *c) { + int n_peers = c->pcfg.n_candidates; + if (n_peers == 1) return 1; + + uint64_t my_term = atomic_fetch_add(&c->ha.term, 1) + 1; + ha_persist_term(c, my_term, c->myconf->name); + + ha_elect_ctx_t *ctx = calloc(1, sizeof(*ctx) + (n_peers-1)*sizeof(ctx->votes[0])); + ctx->n_peers = n_peers; + ctx->votes_granted = 1; // self-vote + ctx->max_resp_tag = c->myconf->config_version_tag; + ctx->max_resp_lsn = atomic_load(&c->persist.applied_lsn); + sem_init(&ctx->done, 0, 0); + atomic_store(&c->ha.elect_ctx, ctx); // dispatch reads this + + // Send RequestVote to all peers via iomgr (async non-blocking connects) + for (int i = 0, j = 0; i < n_peers; i++) { + if (strcmp(c->pcfg.candidates[i].name, c->myconf->name) == 0) continue; + ctx->votes[j].peer = &c->pcfg.candidates[i]; + ctx->votes[j].my_term = my_term; + ctx->votes[j].my_tag = c->myconf->config_version_tag; + ctx->votes[j].my_lsn = atomic_load(&c->persist.applied_lsn); + iomgr_async_vote_request(c, &ctx->votes[j]); // non-blocking + j++; + } + + // Arm election timeout vtask — fires ha_elect_timeout() if peers don't reply + vtaskmgr_schedule(c, c->ha.config.election_timeout_ms, ha_elect_timeout, ctx); + + sem_wait(&ctx->done); // startup blocks here; dispatch or timeout signals it + + int result = ctx->result; + ha_elect_ctx_free(ctx); + atomic_store(&c->ha.elect_ctx, NULL); + return result; +} +``` + +**`handle_request_vote_reply()`** — called from vtaskmgr dispatch switch on +`OP_requestVoteReply`; accumulates votes and signals completion: +```c +void handle_request_vote_reply(cluster_t *c, RequestVoteReply *reply, int peer_idx) { + ha_elect_ctx_t *ctx = atomic_load(&c->ha.elect_ctx); + if (!ctx) return; // no election in progress + + ha_vote_entry_t *v = &ctx->votes[peer_idx]; + v->replied = 1; + v->peer_tag = reply->peer_config_tag; + v->peer_lsn = reply->peer_last_log_lsn; + if (reply->vote_granted) { v->vote_granted = 1; ctx->votes_granted++; } + if (v->peer_tag > ctx->max_resp_tag || + (v->peer_tag == ctx->max_resp_tag && v->peer_lsn > ctx->max_resp_lsn)) { + ctx->max_resp_tag = v->peer_tag; ctx->max_resp_lsn = v->peer_lsn; + } + ctx->n_replied++; + + int quorum = (ctx->n_peers + 1) / 2; + // Signal when quorum achieved or all peers replied (or timed out) + if (ctx->votes_granted >= quorum || ctx->n_replied == ctx->n_peers - 1) + ha_elect_evaluate_and_signal(c, ctx); +} +``` + +**`ha_elect_timeout()`** vtask — fires when `election_timeout_ms` expires: +counts unreplied peers as non-respondents, calls `ha_elect_evaluate_and_signal()`. + +**`ha_elect_evaluate_and_signal()`** — applies gates, sets `ctx->result`, signals: +- Defense-in-depth assertion (config_tag=0 BUG check) +- Hard gate B (yield if responding peer has fresher data → `result=0`) +- Quorum met → `result=1` +- Quorum lost → schedule jitter vtask, retry once with incremented term; + if second round also fails → `result=0`; finally `sem_post(&ctx->done)` + +**Vote handler on peers** (new function in ha.c): +```c +void handle_request_vote(cluster_t *c, RequestVoteRequest *req, RequestVoteReply *reply) { + pthread_mutex_lock(&c->ha.lock); + uint64_t my_term = atomic_load(&c->ha.term); + + if (req->term < my_term) { // stale candidate — reject + reply->term = my_term; + reply->vote_granted = 0; + goto done; + } + if (req->term > my_term) { // update term from candidate + atomic_store(&c->ha.term, req->term); + my_term = req->term; + // Clear voted_for on new term transition + c->ha.voted_for_term = 0; + c->ha.voted_for_name[0] = '\0'; + } + + // Catch-up gate (Clarification 6): behind nodes must not influence elections + if (c->ha.in_catch_up) { + reply->vote_granted = 0; + goto fill_reply; // still send peer_config_tag/lsn so candidate has full picture + } + + uint64_t my_tag = c->myconf->config_version_tag; + uint64_t my_lsn = atomic_load(&c->persist.applied_lsn); + int log_ok = (req->config_tag > my_tag) || + (req->config_tag == my_tag && req->last_log_lsn >= my_lsn); + + // One vote per term: grant only if not yet voted, or voted for same candidate + int already_voted = (c->ha.voted_for_term == my_term && + strcmp(c->ha.voted_for_name, req->candidate_id) != 0); + reply->vote_granted = (log_ok && !already_voted) ? 1 : 0; + + if (reply->vote_granted) { + c->ha.voted_for_term = my_term; + strncpy(c->ha.voted_for_name, req->candidate_id, XCE_MAXNAME_LEN - 1); + ha_persist_term(c, my_term, req->candidate_id); // durable before reply + } +done: + reply->term = my_term; + reply->peer_config_tag = my_tag; + reply->peer_last_log_lsn = my_lsn; + pthread_mutex_unlock(&c->ha.lock); +} +``` + +**Election safety extensions**: + +1. **Term + `voted_for` persistence** (`ha-term.bin` in `{workdir}/`): + New `ha_persist_term(cluster_t *c, uint64_t term, const char *voted_for)` writes + a 4-field struct atomically (write + rename) before any vote is granted or term + incremented. On startup, `ha_init()` reads `ha-term.bin` and restores `ha.term`, + `ha.voted_for_term`, `ha.voted_for_name`. Without this, a restarting node resets + to term=0 and can double-vote in a term it already participated in. + +2. **New fields in `ha_state_t`** (`ha.h`): + ```c + uint64_t voted_for_term; // term of last granted vote + char voted_for_name[XCE_MAXNAME_LEN]; // candidate name of last granted vote + int primary_ctrl_fd; // persistent control channel fd (Change F; -1 = none) + bool defer_config_fetch; // Change C: set when tag=0 and peers have config; + // cleared after post-election fetch completes + ``` + +3. **Retire `ha_check_priority_stepdown()`** (ha.c:322–368): + This function's guard `if (my_term > 0 || sender_term > 0) return` makes it + permanently unreachable after the first quorum election (term is always ≥ 1 + thereafter). Change B explicitly removes the call site from the heartbeat handler + and deletes the function body. Term-based leadership is fully handled by the + `reply.term > my_term` check in `handle_request_vote()` and the existing + `ha_do_demote()` path. + +**Key properties**: +- **Most up-to-date wins**: A candidate that won quorum still yields if any *responding* + peer has higher config_tag or (same tag, higher LSN). Non-responding peers never + block election — only reachable peers with fresher state can cause a yield. +- **Config freshness hard gate**: Yielding due to fresher peer returns 0 (standby, not + fatal). Only `config_tag == 0` with peers having `config_tag > 0` is fatal (-ENODATA), + because that means the node has never been configured at all. +- **Split vote prevention (primary)**: Each node waits `election_timeout + rand_jitter` + before candidacy (Change A `ha_wait_for_heartbeat_or_timeout()`). Different jitter + values stagger candidacy attempts so the first node typically wins unopposed — the + same xorshift64 `jitter_seed` used elsewhere. This is Raft's fundamental split-vote + prevention, not just recovery. +- **Split vote recovery (safety net)**: If a split vote still occurs inside + `ha_elect_evaluate_and_signal()`, a second attempt is scheduled with `rand_jitter` + delay and incremented term. If that also fails, node returns 0 (standby). This + safety net handles the rare case where multiple nodes have identical jitter values. +- **No priority tiebreak**: Priority is not used in Raft quorum election. When + config_tag and LSN are identical across all peers, the jitter-based pre-election + timeout is the tiebreaker. The `ha.my_priority` field is preserved in `ha_state_t` + for backward compatibility but plays no role in election outcome. + +### Change C — Pre-election config bootstrap (deferred fetch) + +A node with `config_tag=0` is a config-seeker, not a candidate. Before calling +`am_i_primary_quorum()`, `bootstrap_config_if_needed()` determines whether config +exists among peers, but **defers the actual fetch until the primary is known** — +avoiding a speculative download from a peer that may not win the election. + +**Rationale**: Fetching from the highest-tag *reachable* peer before election can +waste I/O: a fresher peer may start late, win the election, and push its config +anyway. Since the election already proves who has the freshest state, bootstrap +should defer to that proof rather than guessing. + +New function `bootstrap_config_if_needed()` in config_repl.c (called from myapp.c): +```c +void bootstrap_config_if_needed(cluster_t *c) { + if (c->myconf->config_version_tag != 0) return; // already have config + + int retry_ms = 500; + for (;;) { + uint64_t best_tag = 0; + bool any_reachable = false; + + for (int i = 0; i < c->pcfg.n_candidates; i++) { + if (strcmp(c->pcfg.candidates[i].name, c->myconf->name) == 0) continue; + uint64_t peer_tag = 0; + int rc = xce_probe_config_tag(c->pcfg.candidates[i].host, + c->pcfg.candidates[i].port, &peer_tag); + if (rc != 0) continue; // peer unreachable — skip + any_reachable = true; + if (peer_tag > best_tag) best_tag = peer_tag; + } + + if (!any_reachable) { + WARN("No config and no peer reachable — retrying in %dms (kill to abort)", + retry_ms); + usleep((useconds_t)retry_ms * 1000); + retry_ms = MIN(retry_ms * 2, 10000); + continue; + } + + if (best_tag == 0) { + INFO("All peers also have no config — first-boot cluster, proceeding"); + return; // all-zero cluster: first boot, proceed to election as-is + } + + // Config exists among peers — defer fetch until primary is known. + // The election will prove who has the freshest state. + INFO("Config exists among peers (best tag=%016" PRIx64 ") — " + "deferring fetch until primary is known", best_tag); + c->ha.defer_config_fetch = true; + return; + } +} +``` + +The function never returns an error. If no peer is reachable, it retries with +exponential backoff. If all peers are `tag=0`, it returns for first-boot election. +If config exists among peers, it sets `defer_config_fetch` and returns — the node +will fetch from the proven winner after the election resolves. + +`xce_probe_config_tag()` is a lightweight new RPC (single round-trip) that returns +the peer's current `config_version_tag` without transferring the full config payload. +Reuses the existing SSL connection setup pattern (`connect_ssl_raw()`). + +**Cap'n Proto schema for `OP_probeConfigTag`**: +```c +struct ProbeConfigTagRequest { + name @0 :Text; // requester's node name (for logging on peer side) +} +struct ProbeConfigTagReply { + config_tag @0 :UInt64; // peer's current config_version_tag (0 if no config) +} +``` +Answered by all nodes in any FSM state (including `CTRL_INIT`) — the dispatch gate +explicitly allows `OP_probeConfigTag` through before the controller is ready for +normal requests (Change A). Note: the actual config transfer (`OP_replConfig`) does +NOT need to be role-agnostic — with deferred fetch, `do_one_config_fetch()` is called +post-election when the serving peer is already in `CTRL_STANDBY` or `CTRL_PRIMARY` +and full dispatch is active. Only the probe op needs the `CTRL_INIT` exemption. + +Integrated into `myapp.c` (before election): +```c +bootstrap_config_if_needed(&c); // returns immediately: tag>0, first-boot, or defer + +// The tag=0 node participates in the election as a voter but never wins +// against nodes with tag>0 (log_ok gate ensures this). It votes, discovers +// the winner via voted_for, and fetches from the proven primary. +rc = am_i_primary_quorum(&c); +if (rc < 0) return rc; + +if (c.ha.defer_config_fetch && rc != 1) { + // Fetch from the proven winner (known via voted_for or heartbeat source). + // Guard rc != 1: a winning node has tag>0 so defer_config_fetch is never + // set for it, but the explicit guard prevents double load_primary_config() + // if logic shifts during implementation. + do_one_config_fetch(c->ha.primary_host, c->ha.primary_port, 0); + load_primary_config(&c); + c->ha.defer_config_fetch = false; +} + +if (rc == 1) { + rc = load_primary_config(&c); // load into memory (already on disk) + if (rc != 0) return rc; + atomic_store(&c->ha.role, CTRL_PRIMARY); +} +``` + +### Change D — Startup pre-flight checks + ASAN leak fix + +`main.c`, `src/controller/cluster.c`, `lib/netio.c`, `ha.c` + +1. **`--vcpus N` CLI parameter** (`main.c`, `src/controller/cluster.c`): + Add a `--vcpus N` flag parsed in `main.c` alongside the existing `--peers` and `--name` + flags. Default: `8` when not specified. Values `< 1` are rejected at startup with + `"ERROR: --vcpus must be ≥ 1"` and exit `-EINVAL`. Remove the + `vcpus` field from `cluster_config_t` (currently read from the config file). Pass the + parsed value directly to `vtaskmgr_init()` — this removes `vtaskmgr_init()`'s + dependency on `cluster_config`, enabling it to run before election (Change A). + + **Removed from cluster config schema**: the `vcpus:` key in the config file is no + longer read. Existing configs that include it receive a logged deprecation warning + (`"WARN: 'vcpus' in config file is deprecated — use --vcpus N instead"`) but the + controller does not fail. + + **Rationale**: CPU count is a per-host capacity decision, not a cluster-wide policy. + Different controller nodes in the same cluster may have different hardware. Keeping + vcpus in the shared config forces all nodes to use the same value and couples an + infrastructure tunable to the cluster state config file. + +2. **Peer count validation in `main.c`** (before `cluster_init_phase1()`): + The `--peers` list must contain exactly 1 node (single-node) or an **odd number ≥ 3** + (HA mode). Even numbers cannot form a strict majority and are rejected at startup: + ```c + int n = pcfg.n_candidates; + if (n != 1 && n % 2 == 0) { + fprintf(stderr, + "ERROR: --peers must list an odd number of controllers (1, 3, 5, ...).\n" + " %d controllers cannot form a majority. Use 1 for single-node\n" + " or add/remove one controller to reach an odd count.\n", n); + return -EINVAL; + } + ``` + This removes the need for the `n_peers == 2` special case inside + `am_i_primary_quorum()` — the quorum formula `(N+1)/2` is correct for all + validated (odd) values of N. + +3. **Cert pre-flight in `main.c`** (before `myapp_run()`): explicit `access()` check on + `{workdir}/certs/xce.key` and `{workdir}/certs/xce.crt`. On failure: print + operator-friendly `"ERROR: TLS cert not found: <path>"` and exit `-ENOENT`. + Controller never starts without valid certs — same hard-prerequisite model as config. + +4. **Fix ASAN leak in `ha_fini()` (ha.c:451)**: add `SSL_CTX_free(ha->ssl_ctx)` call. + This closes the ~5320-byte direct+indirect leak shown in the pc13v1 ASAN output. + +### Change E — Majority-commit WAL replication + +`src/controller/wal.c` (or `persist.c`), `src/controller/ha.c`, `src/controller/cluster.c` + +**Principle**: Primary does not ACK a write to its caller until ≥(N+1)/2 standbys have +confirmed they wrote the WAL entry to disk. This guarantees any promoted standby has +all committed entries — zero data loss on failover. + +**New wire message: OP_walAck** (Cap'n Proto): +```c +struct WalAckRequest { + term @0 :UInt64; // primary's current term (for stale-detection) + lsn @1 :UInt64; // WAL entry LSN being ACKed +} +struct WalAckReply { + term @0 :UInt64; // standby's term (primary steps down if reply.term > mine) + lsn @1 :UInt64; // highest LSN standby has durably written + success @2 :Bool; +} +``` + +**Primary-side commit protocol** (`persist.c` / `wal.c`): +1. Write WAL entry to local disk (existing) +2. Fan-out `OP_walAck` to all connected standbys in parallel (async send) +3. Wait for ≥(N+1)/2 − 1 standby ACKs (own write counts as 1) within + `commit_timeout_ms` (configurable, default 5000ms) +4. On quorum ACK: mark entry committed, return success to caller +5. On timeout without quorum: return `-ETIMEDOUT` to caller; entry is + written locally but not committed — next primary will re-evaluate on + WAL replay + +**Standby-side WAL write** (existing replication path extended): +- On receiving replicated WAL entry: write to disk, fsync, send `OP_walAck` reply +- No change to standby application logic + +**Backpressure**: Primary maintains a per-standby `pending_ack_count`. If a standby +falls more than `max_pending_acks` (default 64) entries behind, it is removed from the +active replication set and reconnects via existing catch-up path. + +**Single-node mode**: quorum = 1, commit is immediate (own write = majority). No change. + +**Clarification 1 — Durability boundary (which entry types block on majority-commit)**: + +Not all WAL entry types have equal durability requirements. The rule: + +| Entry type | Commit model | Rationale | +|---|---|---| +| `WAL_JOB_SUBMIT` | Majority-commit (blocking) | Job loss on failover is visible to user | +| `WAL_JOB_START` / `WAL_JOB_FINISH` | Majority-commit (blocking) | State transitions must survive failover | +| `WAL_QUEUE_CHANGE` | Majority-commit (blocking) | Queue config changes must be durable | +| `WAL_HOST_STATE` | Best-effort async | Reconstructed from worker reconnect on failover | +| `WAL_CHECKPOINT_REF` | Best-effort async | Checkpoint is self-contained; ref is advisory | + +The existing `persist_wal_sync_append()` path (used for `WAL_JOB_SUBMIT` today with +ANY-standby wait) is extended to use majority quorum. The existing `persist_wal_async()` +path remains fire-and-forget for best-effort types. No new caller API — entry type +determines which path is taken inside `persist.c`. + +**Clarification 2 — `committed_lsn` data model**: + +Two new fields added to `persist_t` (persist.h): +```c +_Atomic uint64_t committed_lsn; // highest LSN majority-ACKed; safe to serve/trim +uint64_t applied_lsn; // existing: highest LSN written locally (may exceed committed) +``` + +`committed_lsn` is advanced by `repl_wait_majority_ack()` after quorum is reached. +On promotion, the new primary replays WAL from its checkpoint up to `committed_lsn` +only — entries above `committed_lsn` are discarded (never committed to majority, so +no client received success for them). WAL trim guard updated: trim threshold = `committed_lsn`, +not `applied_lsn`. + +Catch-up standbys (in_catch_up == 1) are excluded from quorum counting — they are not +yet in the active replication set. This means a cluster with 2-of-3 nodes catching up +cannot make progress until at least one catch-up completes. This is the correct CP +behavior: prefer unavailability over serving uncommitted state. + +**Clarification 3 — `repl_broadcast()` prerequisite**: + +`repl_broadcast()` (repl.c:183–196) is currently a stub. Majority-commit requires a +real broadcast implementation. This is co-developed with Change E — they are the same +work unit. Change E scope explicitly includes: +- Implement `repl_broadcast()` with per-standby TCP send via existing iomgr pipeline +- Implement `repl_wait_majority_ack()` replacing the existing `repl_wait_ack()` (any→majority) +- Add per-standby send error callback: failed send → decrement active standby count, + trigger catch-up reconnect (existing path) + +If `repl_broadcast()` stub lands separately before Change E, Change E still works — +it replaces the stub. No ordering dependency beyond Task 11 (config fetch). + +**New function `persist_truncate_wal_above(persist_t *p, uint64_t cutoff_lsn)`** (`persist.c`): + +Removes all WAL entries with LSN > `cutoff_lsn` from local storage. Called in two +contexts (Clarifications 4 and 5). Implementation: scan WAL segment files from newest +to oldest; truncate the file containing `cutoff_lsn` at the byte offset immediately +after that entry; delete any segment files that start above `cutoff_lsn`. Updates +`p->applied_lsn = cutoff_lsn` atomically after truncation. Must be idempotent (calling +twice with same cutoff is safe). New function — does not currently exist in `persist.c`. + +**Clarification 4 — `committed_lsn` must be piggybacked on heartbeats**: + +A promoted standby needs to know where to truncate its WAL. Without this it only has +`applied_lsn`, which may include uncommitted orphan entries written by the old primary +but never majority-ACKed. + +- Extend `CtrlHeartbeat` Cap'n Proto struct with `committed_lsn @N :UInt64` +- Standbys store it in `ha.last_known_committed_lsn` (`_Atomic uint64_t`, init 0) on + each heartbeat receipt +- On promotion, immediately after `am_i_primary_quorum()` returns 1: + ```c + persist_truncate_wal_above(&c->persist, c->ha.last_known_committed_lsn); + c->persist.committed_lsn = c->ha.last_known_committed_lsn; + ``` +- First-boot failover (no heartbeat ever received): `last_known_committed_lsn = 0`, + so WAL replay starts from checkpoint only — correct and safe + +**Clarification 5 — Log divergence on standby re-join**: + +The existing catch-up path assumes the standby is simply *behind* the primary (a clean +subset). After a primary crash, a standby may have orphaned WAL entries above the new +primary's `committed_lsn` — entries the old primary replicated but never majority-ACKed. +Replaying those entries would corrupt the standby's state. + +Catch-up handshake extension (Change E scope): +1. Primary sends its `committed_lsn` in the catch-up initiation reply +2. Standby calls `persist_truncate_wal_above(&c->persist, primary_committed_lsn)` before + requesting WAL replay — discards any orphaned entries above that boundary +3. Standby sets `applied_lsn = primary_committed_lsn` as the new replay base +4. Normal WAL catch-up (existing mechanism) proceeds from there + +**Clarification 6 — Catch-up standbys excluded from voting (not just quorum count)**: + +Clarification 2 excludes `in_catch_up==1` standbys from the majority quorum count. +They must also be prevented from *granting* votes — a behind-node's stale LSN could +tip an election to a less-fresh candidate. + +In `handle_request_vote()`, before the `log_ok` freshness check: +```c +if (c->ha.in_catch_up) { + reply->vote_granted = 0; + // still populate peer_config_tag/peer_last_log_lsn — candidate needs the full picture + goto fill_reply; +} +``` + +**Clarification 7 — `applied_lsn` in vote request must be post-fsync**: + +`RequestVoteRequest.last_log_lsn` is sourced from `atomic_load(&c->persist.applied_lsn)`. +This is correct only if `applied_lsn` is advanced *after* `fsync()` completes in +`persist.c`. A node that claims an LSN it has not durably written could win an election +and then fail to replay entries it advertised. Verify in `persist.c` before implementing +Change B — if `applied_lsn` is advanced pre-fsync anywhere, fix that first. + +### Change F — Persistent standby↔primary control channel + +**Files**: `src/controller/ha.c`, `src/controller/myapp.c`, `src/controller/cluster.c`, +`src/controller/repl.c` + +**Background**: Standbys currently detect primary failure through `ha_check_heartbeat_nocluster()` +(ha.c:380–405), which polls a `last_heartbeat` timestamp with a 10s floor. Failover detection +latency is up to 10 seconds. Additionally, each config fetch and WAL replication message opens a +new TCP+SSL connection (`connect_ssl_raw()`, config_repl.c:685–775), adding per-message handshake +latency for WAL streaming (Change E). + +**Changes**: + +1. **Standby discovers and connects to primary** — new `ha_find_and_connect_to_primary(cluster_t *c)` + (called in `myapp.c` standby path after election). Opens a TCP+SSL connection to the primary + peer and registers the fd with epoll `EPOLLIN | EPOLLRDHUP`. The connection is stored in + `ha_state_t` as `ha->primary_ctrl_fd`. On connect, the standby sends an extended `OP_ctrlHello` + (item 5) carrying identity and state. The primary registers the standby, initiates catch-up, + and begins streaming any missing WAL before the standby enters the active replication set. + + **Primary discovery algorithm** (how the standby finds the winner's address): + + Two startup paths lead here with different amounts of prior knowledge: + + - **Heartbeat path** (`CTRL_INIT → CTRL_STANDBY`): `ha_wait_for_heartbeat_or_timeout()` received + a heartbeat/`OP_ctrlHello` from an existing primary. The sender's host/port is in the packet. + Store immediately as `ha.primary_host/port`. No further discovery needed — connect directly. + + - **Election path** (`CTRL_ELECTING → CTRL_STANDBY`): lost `am_i_primary_quorum()`. Discovery + uses a three-tier strategy exploiting Raft's structural property that the majority voted for + the winner: + + ``` + Tier 1 (fast path — O(1) for ≥(N+1)/2 nodes): + If ha.voted_for_name is set: + try ha_connect_to_primary(voted_for peer) + on success: store ha.primary_host/port, done + on XCE_ERR_NOT_READY: peer still electing — backoff, retry tier 1 + + Tier 2 (redirect — O(1) once any majority member connected): + Probe remaining peers (random order) with OP_ctrlHello + A CTRL_STANDBY peer that already knows the primary responds with + XCE_ERR_STANDBY + redirect_host/port (same field as Change G) + Follow redirect immediately — connect to that address + + Tier 3 (blind scan fallback — for split-vote minorities): + If redirect not received: try remaining peers sequentially + A CTRL_PRIMARY peer accepts OP_ctrlHello normally → connected + A CTRL_INIT/CTRL_ELECTING peer returns XCE_ERR_NOT_READY → skip+retry + Repeat with 100ms → 5s exponential backoff until primary found + ``` + + **Why this converges quickly**: In a 3-node cluster, at least 2 nodes voted for the winner. + Those 2 connect via Tier 1 immediately. The 3rd probes either of them (Tier 2), gets a + redirect, and connects. Total: 2 direct + 1 redirect = all standbys connected with at most + one extra probe per minority node. In a 5-node cluster, at least 3 connect via Tier 1; the + remaining 2 get redirected. + + **`ha.primary_host/port` update**: set on successful connect; cleared to empty on + `EPOLLRDHUP` (primary disconnect). During the election window it is empty; standbys receiving + `OP_ctrlHello` while `ha.primary_host` is empty reply with `XCE_ERR_STANDBY` and no redirect + (graceful — probing node falls through to Tier 3). + + **No new message types**: the redirect uses the existing `XCE_ERR_STANDBY` error reply fields + already extended in Change G (`redirect_host`, `redirect_port`). The standby dispatch gate + (already returning `XCE_ERR_STANDBY` for primary-only ops) populates these fields from + `ha.primary_host/port` when they are known. + +2. **Standby join synchronization — full state sync on connect** (cluster.c, ha.c, repl.c): + + A standby with stale WAL cannot safely promote — its in-memory state (jobs, queues, + resources) would be out of date, and promoted writes would replay entries clients already + saw as committed. Every standby must be fully current before it enters the quorum. + + **Extended `OP_ctrlHello` Cap'n Proto schema**: + ```c + struct CtrlHelloRequest { + name @0 :Text; // peer name (from --peers) + key @1 :Data; // cluster auth key: SHA-256(cluster_name + shared_secret) + // shared_secret read from myconf->cluster_key (new config field) + // primary rejects connections where HMAC does not match its own + applied_lsn @2 :UInt64; // standby's current applied_lsn (0 if fresh node) + config_tag @3 :UInt64; // standby's current config_version_tag (0 if none) + } + struct CtrlHelloReply { + committed_lsn @0 :UInt64; // primary's current committed_lsn + needs_snapshot @1 :Bool; // true if standby's WAL predates oldest primary segment + config_tag @2 :UInt64; // primary's current config_version_tag + } + ``` + + **New wire messages for sync**: + ```c + struct SnapshotTransfer { + snapshot_lsn @0 :UInt64; // LSN of this snapshot (= checkpoint_lsn after fresh checkpoint) + payload @1 :Data; // serialized snapshot blob (from persist_checkpoint()) + } + struct WalCatchupEntry { + lsn @0 :UInt64; // WAL entry LSN + type @1 :UInt8; // WAL entry type (WAL_JOB_SUBMIT etc.) + payload @2 :Data; // WAL entry payload + } + ``` + `OP_snapshotTransfer` and `OP_walCatchupEntry` flow on the persistent control channel fd — + no new connections. + + **Primary-side `handle_ctrlhello()` sequence** (new handler in cluster.c): + ``` + 1. Extract name, applied_lsn, config_tag from CtrlHelloRequest + 2. repl_add_standby(&c->repl, ep, host, port) + 3. needs_snap = repl_needs_snapshot(&c->repl, applied_lsn, oldest_wal_lsn()) + 4. Reply: CtrlHelloReply{c->persist.committed_lsn, needs_snap, myconf->config_version_tag} + 5. repl_start_catchup(&c->repl, ep, applied_lsn) // sets standby's in_catch_up=1 + 6. If needs_snapshot: + persist_checkpoint(&c->persist) // flush in-memory state to disk + send OP_snapshotTransfer(snapshot_lsn, blob) + applied_lsn = snapshot_lsn // advance standby's effective base + 7. Streaming loop — runs until standby is fully current: + target_lsn = atomic_load(&c->persist.committed_lsn) // read current live value + stream WAL entries [applied_lsn+1 → target_lsn] via OP_walCatchupEntry + wait for OP_walAck with acked_lsn >= target_lsn + if atomic_load(&c->persist.committed_lsn) > target_lsn: + applied_lsn = target_lsn; continue loop // new entries committed, catch up again + // standby is now at current committed_lsn — exit loop + 8. repl_complete_catchup(&c->repl, ep) // in_catch_up=0; standby enters live replication + 9. If standby config_tag != primary config_tag: + push_config_to_standbys(c, blob, blen, tag) + ``` + + **`repl_broadcast()` skips `in_catch_up==1` standbys**: while the streaming loop (step 7) + is in progress, `repl_broadcast()` does not send to this standby. The loop itself handles + delivery of all committed entries — including entries committed after the handshake — by + re-reading `committed_lsn` on each iteration. The loop exits only when the standby's + `acked_lsn` equals the *current* `committed_lsn`, at which point `repl_complete_catchup()` + fires and live broadcast takes over. There is no window where entries can be missed. + + **Standby-side sequence** (after receiving CtrlHelloReply in ha.c / myapp.c): + ``` + 1. Store primary_committed_lsn = reply.committed_lsn + 2. persist_truncate_wal_above(&c->persist, primary_committed_lsn) // Clarification 5 + 3. ha.last_known_committed_lsn = primary_committed_lsn // Clarification 4 + 4. ha.in_catch_up = 1 // deny votes during sync + 5. If reply.needs_snapshot: + wait for OP_snapshotTransfer + write received blob as local snapshot file + load_snapshot(&c->persist, path, snapshot_lsn) + applied_lsn = snapshot.snapshot_lsn + 6. Receive OP_walCatchupEntry stream; call apply_wal_entry() for each + 7. Send OP_walAck after each entry (or configurable batch): + OP_walAck{acked_lsn = last applied LSN} + 8. When primary sends no more OP_walCatchupEntry (catch-up loop ended on primary side): + ha.in_catch_up = 0 // standby now in live replication; votes granted normally + ``` + + `apply_wal_entry()` is the same WAL replay function used during startup recovery — it + applies a single WAL entry to the in-memory state (jobs, queues, resources). Must exist + or be extracted from the startup replay path before this change lands. + + **`repl_needs_snapshot()` already exists** in repl.c — compares `standby_applied_lsn` + against `oldest_wal_lsn` (the trim floor). If standby is behind the trim floor, + `persist_checkpoint()` is called on the primary to produce a fresh snapshot, which is + then streamed in full. This is the only path where `persist_checkpoint()` is called + on-demand rather than on schedule. + + **`push_config_to_standbys()` is declared** in config_repl.h; config sync at join + handles the case where a standby was offline during a config upgrade. + +2. **Heartbeat runs on control channel** — primary sends periodic heartbeats on the controller↔controller + channel (in addition to existing worker heartbeats). Standby updates `last_heartbeat` on receipt, + but the epoll path is the fast lane. + +3. **Replace timestamp polling with dual-trigger failover detection** — + `ha_check_heartbeat_nocluster()` (ha.c:380–405) is retired. Two complementary paths: + - **Fast path (`EPOLLRDHUP`)**: TCP disconnect detected immediately; `primary_ctrl_fd` + closed and nulled before `am_i_primary_quorum()` is called. Sub-second latency. + - **Fallback path (heartbeat timeout vtask)**: armed after each heartbeat is received + (`vtaskmgr_schedule(election_timeout + rand_jitter, ha_heartbeat_timeout_cb)`); reset + on every subsequent heartbeat. Fires when primary is silent but TCP is alive — covers + network partitions that keep TCP open, primary overload, or missed heartbeat bursts. + On fire: close+null `primary_ctrl_fd`, set `CTRL_ELECTING`, call `am_i_primary_quorum()`. + +4. **WAL streaming over persistent channel (Change E complement)** — `repl_broadcast()` + (repl.c:183–196) sends WAL entries on the persistent control channels established by each + standby rather than opening per-entry connections. No new connection setup per WAL write. + +**Role flip on promotion**: When a standby wins election and becomes primary, it must +close and discard its `primary_ctrl_fd` (the old connection to the now-dead primary) +*before* calling `am_i_primary_quorum()` triggers the role transition. After promotion, +the new primary accepts `OP_ctrlHello` connections from the remaining standbys — the +role of `primary_ctrl_fd` inverts from "my connection out" to "their connections in", +tracked in a per-standby fd table rather than a single field. + +**Reconnect behavior**: On primary_ctrl_fd disconnect, standby closes and nulls +`primary_ctrl_fd` first, then calls `am_i_primary_quorum()`. +If it becomes primary, the old primary_ctrl_fd is discarded. If it remains standby, it reconnects +with exponential backoff (100ms, 200ms, 400ms, max 5s) until the new primary is reachable. + +**Single-node mode**: `ha_connect_to_primary()` is a no-op when `n_peers == 1`. + +**Key properties**: +- Sub-second failover detection (`EPOLLRDHUP` vs 10s timeout) +- No per-entry TCP+SSL handshake overhead for WAL replication +- Single liveness signal: connection alive = peer alive +- Backward-compatible: standby reconnect on disconnect triggers election (same semantics as today, + faster latency) + +### Change G — libxce client-side retry for transient controller errors + +**Files**: `src/lib/` (libxce — client request/connection layer) + +**Background**: With `CTRL_ELECTING` gating requests during the election window +(Change A), and standbys already returning `XCE_ERR_STANDBY` to misdirected +requests, every daemon (worker, scheduler) needs to handle these transient errors +gracefully. Without built-in retry in libxce, each daemon would need its own +ad-hoc loop — fragile and inconsistent. + +**Changes**: + +1. **Retry policy in libxce's client request path** (the function wrapping + `xce_encode()` + send + recv + `xce_decode()`): + - On `XCE_ERR_NOT_READY`: back off and retry against same endpoint + - On `XCE_ERR_STANDBY`: **redirect** — the standby includes the current primary's + `host:port` in the error payload when known (`ha.primary_host/port`). libxce + reconnects directly to the redirected endpoint instead of blind backoff. + When primary is unknown (e.g. during startup), falls back to normal retry. + - On TCP-level `ECONNREFUSED` or `ETIMEDOUT`: reconnect and retry + (handles the window before `cluster_open_listener()` runs on a starting peer) + - Non-retriable errors (`XCE_ERR_AUTH`, `XCE_ERR_INVALID`, `XCE_ERR_NOENT`, + `XCE_ERR_PERM`, etc.): surface immediately to caller + + **`XCE_ERR_STANDBY` wire extension**: add optional `redirect_host` (Text) and + `redirect_port` (UInt16) fields to the error reply Cap'n Proto struct. Standbys + populate these when `ha.primary_host` is non-empty; clients that don't understand + the new fields simply retry with backoff (backward-compatible). + +2. **Retry configuration** (sensible defaults, overridable per client): + ```c + typedef struct { + int max_retries; // default 20 + int initial_retry_ms; // default 50 + int max_retry_ms; // default 5000 + double backoff_multiplier; // default 2.0 (exponential) + } xce_retry_config_t; + ``` + Defaults cover a full election round (up to 20 × 50ms–5000ms) without + requiring callers to configure anything. Daemons that need tighter or looser + bounds pass a custom `xce_retry_config_t`. + +3. **Transparent to callers**: existing daemon code makes one `xce_request()` call; + libxce handles retry internally. No changes needed in worker, scheduler, or + apiserver code. + +**Key properties**: +- Decouples daemon startup order — a worker that starts before the primary is ready + simply waits for the retry window to expire rather than crashing or looping manually +- `XCE_ERR_STANDBY` retry enables transparent primary failover: after failover, the + next request to the old primary (now standby) gets `XCE_ERR_STANDBY`, libxce retries + against the same host/port and the new primary responds +- Backoff prevents thundering-herd when 50 workers restart simultaneously after a failover + +### Change H — Majority-commit config push + +`src/controller/config_repl.c`, `src/controller/repl.c` + +**Principle**: Config upgrades (`OP_upgradeConfig`) use the same majority-commit +infrastructure as WAL replication (Change E). The primary does not ACK a config +change to the operator until ≥(N+1)/2 standbys have confirmed durable write. + +**Why this matters**: Without majority-commit, a primary could accept a config change, +ACK to the operator, then crash. The new primary serves the old config, and the +operator's change is silently lost. For strict CP, config writes must have the same +durability guarantee as WAL writes — even though they are orders of magnitude rarer. + +**Changes**: + +1. **Reuse Change E infrastructure** — `repl_broadcast()` (sends to all connected + standbys on persistent channels) and `repl_wait_majority_ack()` (blocks until + ≥(N+1)/2 ACKs or `commit_timeout_ms`) already built for WAL. Both functions + accept a message-type parameter so they can serve WAL (`OP_walAck`) and config + (`OP_upgradeConfigAck`) ACKs without duplication. Zero new infrastructure. + +2. **`push_config_to_standbys()` extended** (config_repl.c): + ```c + int push_config_to_standbys(cluster_t *c, const char *blob, size_t len, uint64_t tag) { + // 1. Write locally (existing atomic write+rename — unchanged) + int rc = config_repl_write_local(c, blob, len, tag); + if (rc != 0) return rc; + + // 2. Fan-out OP_upgradeConfig via repl_broadcast (same channel as WAL) + rc = repl_broadcast(c, OP_upgradeConfig, blob, len); + if (rc != 0) return rc; + + // 3. Wait for ≥(N+1)/2 standby ACKs (own write = 1 vote) + rc = repl_wait_majority_ack(c, c->ha.config.commit_timeout_ms); + + // 4. On quorum: advance committed_config_tag and return success + if (rc == 0) { + atomic_store(&c->myconf->committed_config_tag, tag); + } + return rc; // -ETIMEDOUT on failure — operator retries + } + ``` + +3. **Standby-side config ACK** — on receiving `OP_upgradeConfig`, standby: + - Writes config blob to disk (existing atomic write path) + - Loads config into memory + - Sends `OP_upgradeConfigAck` with `acked_tag = written_tag` + - ACK semantics identical to `OP_walAck` — durable write complete + +4. **`OP_upgradeConfigAck` Cap'n Proto**: + ```c + struct UpgradeConfigAck { + term @0 :UInt64; // standby's term (primary steps down if ack.term > mine) + acked_tag @1 :UInt64; // config tag standby has durably written + success @2 :Bool; + } + ``` + +5. **`committed_config_tag` field** added to cluster config state (analogous to + `committed_lsn` for WAL). Advanced only on majority ACK. Stored alongside + `config_version_tag` in durable state. On promotion, a new primary serves only + configs up to `committed_config_tag`. **Piggybacked on heartbeats** (same as + `committed_lsn` — Clarification 4): standbys store the latest value in + `ha.last_known_committed_config_tag` so a promoted standby knows the safe + boundary without an extra round-trip. First-boot promotion (no heartbeat + received): value is 0, standby serves whatever config it loaded from disk — + correct for first-boot where no upgrade has been committed. + +**Key properties**: +- Same `commit_timeout_ms` as WAL (configurable, default 5000ms) +- Single-node mode: quorum=1, commit is immediate (own write = majority) +- Config upgrades are operator-initiated and rare — the blocking ACK wait adds + negligible overhead to cluster operations +- Catch-up standbys (`in_catch_up==1`) are excluded from quorum count, + consistent with WAL majority rules +- If commit times out, operator gets `-ETIMEDOUT` and can retry — libxce retry + (Change G) handles this transparently for the API server + +## Scope + +- **In**: + - Change A: Extract `cluster_open_listener()`, `iomgr_init()`, and `vtaskmgr_init()` + from phase2; call all three before election in myapp.c so the full connection + framework is running during election (incoming `OP_requestVote` handled by normal + dispatch switch); add `CTRL_ELECTING` role + pre-dispatch gate returning + `XCE_ERR_NOT_READY` for non-vote requests during election window + - Change B: Full Raft-style quorum voting in `am_i_primary_quorum()` + - New `OP_requestVote` wire protocol (Cap'n Proto request + reply) + - `handle_request_vote()` handler on all peers with `voted_for` enforcement + - Config tag + LSN freshness comparison in vote grant and candidate yield gate + - Hard gate A (fatal): refuse primary if config_tag=0 and any peer has tag>0 + - Hard gate B (yield): winner yields if any *responding* peer has fresher data + - Split vote retry with jitter (one retry after timeout) + - Term + `voted_for` persistence to `ha-term.bin` (survives restart) + - New `ha_state_t` fields: `voted_for_term`, `voted_for_name`, `primary_ctrl_fd` + - Retire `ha_check_priority_stepdown()` (permanently unreachable after first election) + - Change C: `bootstrap_config_if_needed()` pre-election bootstrap — probes peers + for config tags, defers fetch until primary is known (avoids speculative download + from a peer that may not win the election); tag=0 node participates as voter only; + new `xce_probe_config_tag()` lightweight RPC; role-agnostic + `handle_config_request()` so peers can serve config to a bootstrapping node + - Change D: `--vcpus N` CLI parameter + remove `vcpus` from cluster config (deprecation + warning for existing configs); odd-N peer-count validator at startup (`-EINVAL` on + even count); cert pre-flight check in `main.c`; `SSL_CTX_free` ASAN fix in `ha.c` + - Change E: Majority-commit WAL replication — `OP_walAck` protocol, primary + commit gating, standby ACK path, backpressure, `commit_timeout_ms` config + - Change F: Persistent standby↔primary control channel — `ha_connect_to_primary()`, + `EPOLLRDHUP`-based failover detection, heartbeat on control channel, WAL streaming + over persistent connection + - Change G: libxce client-side retry — exponential backoff on `XCE_ERR_NOT_READY`, + `XCE_ERR_STANDBY`, `ECONNREFUSED`, `ETIMEDOUT`; transparent to daemon callers; + configurable via `xce_retry_config_t` with safe defaults + - Change H: Majority-commit config push — `OP_upgradeConfig` uses the same + `repl_broadcast()` + `repl_wait_majority_ack()` infrastructure as WAL replication + (Change E). Primary waits for ≥(N+1)/2 standby ACKs before confirming a config + upgrade to the operator. Reuses `commit_timeout_ms`. Zero new infrastructure cost. + - Unit and integration tests for all eight changes +- **Out**: + - Raft snapshot protocol — existing dual-zarray COW is orthogonal and unchanged + - Joint consensus for cluster membership changes — candidate list is static + - do_one_config_fetch() stub wiring (Task 11) — prerequisite; must land first or co-developed + - mTLS between controllers — trusted-LAN model with server-side TLS is sufficient + +## Boundary Candidates + +1. **Election protocol** — full replacement of `am_i_primary()` with quorum voting +2. **Config wire protocol** — new OP_requestVote, OP_probeConfigTag, OP_upgradeConfigAck; existing OP_replConfig used for post-election deferred fetch +3. **Primary startup path** — bootstrap integration in myapp.c +4. **Listener lifecycle** — extracted from phase2 to phase1 + +## Out of Boundary + +- Standby fetch path (`fetch_config_from_primary()`) — untouched +- Config format, versioning, or storage layout — untouched +- Raft snapshot protocol — existing dual-zarray COW is orthogonal +- Joint consensus for cluster membership changes — candidate list is static +- mTLS — trusted-LAN model with server-side TLS is sufficient + +## Upstream / Downstream + +- **Upstream**: Task 11 (`do_one_config_fetch()` stub) on persistent_ha branch — with deferred fetch (Change C), the probe-only path in `bootstrap_config_if_needed()` does not call `do_one_config_fetch()`. Task 11 is hit only at the post-election deferred fetch step in `myapp.c`. If Task 11 is not yet landed, the deferred fetch returns `-ENOSYS` and the bootstrapping standby fails to load config — a fatal but clearly logged error. Both can be co-developed; Task 11 must land before or with Change C for a functional cluster. +- **Downstream**: Phase 3 hardening (membership changes, leader transfer), ops tooling for election/commit observability, monitoring metrics for WAL replication lag. + +## Existing Spec Touchpoints + +- **Adjacent**: none of the active specs (resval-id-decoupling, scheduler-controller-msg-gap, worker-join-refactor) touch config replication, HA election, or cluster init. + +## Constraints + +- C (gnu17), no C++, no new external dependencies. +- ASAN build must pass (`nix build .#xcecore_debug`). +- **`--peers` must list 1 or an odd number ≥ 3 controllers.** Even counts are rejected + at startup with `-EINVAL`. This simplifies quorum math: `(N+1)/2` is exact for all + valid N; no special-case branches needed inside `am_i_primary_quorum()`. +- **Static membership**: the candidate list is fixed at startup from `--peers` and never + changes while the cluster is running. No dynamic join/leave of controller peers. + Worker and scheduler nodes may still join dynamically — only controller peer membership + is static. This eliminates the need for joint consensus (Raft §6) and locks the quorum + denominator for the lifetime of the cluster. +- Single-node mode unaffected: quorum=1 everywhere (election, WAL commit = own write). +- TDD required: failing test first, then implementation. +- All new C test cases go in `src/controller/testsuite/` before implementation. +- `make test` must pass without a live server. +- `commit_timeout_ms` must be configurable (default 5000ms) to allow tuning per deployment latency. +- **WAL write stall during standby catch-up (operational constraint)**: In a 3-node cluster, + quorum for WAL commit is 2 (primary + 1 standby). Between primary winning election and the + first standby completing `repl_complete_catchup()`, the active replication set has 0 standbys. + Every `persist_wal_sync_append()` call blocks for `commit_timeout_ms` and returns `-ETIMEDOUT`. + This is CP-correct behavior — the cluster is unavailable until quorum is formed. Operators + should expect a stall of up to `commit_timeout_ms` per write during the initial catch-up + window on first boot or after a restart. Jobs submitted during this window receive a retriable + error; libxce retry (Change G) handles this transparently for workers and schedulers. + +## Deployment Reference + +### Controller node topology + +| Cluster size | Failure tolerance | WAL ACKs needed | Recommendation | +|---|---|---|---| +| 1 node | 0 | self only | dev/test only | +| 3 nodes | 1 failure | 1 standby ACK | minimum for production | +| 5 nodes | 2 failures | 2 standby ACKs | recommended for critical HPC | +| 7 nodes | 3 failures | 3 standby ACKs | rarely justified; adds latency | + +### Job submission throughput (Raft overhead only) + +Basis: LAN environment, 5 controller nodes, quorum = 3 (primary + 2 standby ACKs). + +**Critical path per `WAL_JOB_SUBMIT`:** + +``` +client → primary: RTT/2 + primary WAL fsync +primary → 2 standbys: RTT/2 + standby WAL fsync (parallel, overlaps with above) +2nd ACK → primary: RTT/2 +primary confirms: RTT/2 +───────────────────────────────────────────────── +Total: 1 LAN RTT + max(fsync_primary, fsync_standby) +``` + +| Parameter | Optimistic | Conservative | +|---|---|---| +| LAN RTT (controller↔controller) | 0.1 ms | 0.5 ms | +| NVMe fsync latency | 0.05 ms | 0.2 ms | +| Effective commit latency | ~0.2 ms | ~0.7 ms | + +**Throughput estimates:** + +| Submit model | Commit latency | Jobs/sec | +|---|---|---| +| Serial single stream | 0.3–1 ms | 1,000–3,300 | +| 10 concurrent clients | 0.5 ms | ~20,000 | +| 100 concurrent clients | 0.5 ms | ~200,000 (non-Raft bottlenecks dominate before this) | + +**Practical HPC ceiling**: 5,000–20,000 jobs/sec before jobmgr queue locking and +in-memory state mutations become the constraint. + +**5-node vs 3-node write latency**: negligible. Majority-commit waits for the +**2nd-fastest** ACK out of 4 standbys (5-node) vs the **1st** out of 2 (3-node). +Under uniform LAN latency the difference is sub-millisecond. The extra two nodes +cost nothing in write throughput but double the failure tolerance. + +**When Raft becomes a bottleneck**: automated pipelines submitting >10,000 short +jobs/sec. Mitigation: batch multiple job submits into a single WAL entry at the +API layer before this design makes that necessary. diff --git a/.kiro/specs/ha-config-bootstrap/requirements.md b/.kiro/specs/ha-config-bootstrap/requirements.md new file mode 100644 index 00000000..0fb6f23e --- /dev/null +++ b/.kiro/specs/ha-config-bootstrap/requirements.md @@ -0,0 +1,151 @@ +# Requirements: HA Config Bootstrap & Raft-Style Quorum Election + +## Introduction + +The current LSF-style priority-based controller election has three critical safety gaps: (1) no guarantee of config freshness on the elected primary, (2) false self-elections during concurrent startup when the peer listener port is not yet open, and (3) no bootstrap path for a standby promoted during genuine failover whose local config is missing. This feature replaces priority-based election with Raft-style quorum voting, adds majority-commit WAL and config replication for full CP durability, establishes persistent standby-to-primary control channels, and hardens the controller startup sequence. The system shall maintain CP semantics: consistency over availability, with zero data loss on failover. + +## Boundary Context + +- **In scope**: quorum-based controller election, pre-election config bootstrap for unconfigured nodes, listener binding before election, startup pre-flight validation (peer count, certs, vcpus), majority-commit WAL replication with entry-type differentiated durability, majority-commit config push, persistent standby-to-primary control channels with catch-up synchronization, client-side transparent retry for transient controller errors, election term and voted_for durability across restarts, observable election and commit events via structured logs +- **Out of scope**: Raft snapshot protocol (existing COW is orthogonal), joint consensus for cluster membership changes (candidate list is static at startup from `--peers`), mTLS between controllers (trusted-LAN model with server-side TLS is sufficient), config file format or storage layout changes +- **Adjacent expectations**: the existing `do_one_config_fetch()` stub must be completed for config bootstrap to function in the deferred-fetch path; static controller peer membership (no dynamic join/leave) is a hard constraint; single-node mode (quorum=1) must remain fully operational without behavioral changes + +## Requirements + +### Requirement 1: Listener and Connection Framework Before Election + +**Objective:** As a cluster operator, I want peer listener sockets to be open before election begins, so that concurrent-startup nodes can reach each other's vote handlers rather than falsely concluding peers are absent. + +#### Acceptance Criteria +1. When a controller starts, the system shall bind and listen on its peer port before initiating any election probe. +2. When a controller starts, the system shall initialize the I/O manager and virtual task manager before initiating any election probe, so that incoming vote requests are handled by the normal dispatch loop. +3. When the controller is in the startup-initializing or electing state, the system shall reject non-vote, non-config-probe requests with a not-ready error response. +4. When the controller is in standby state, the system shall reject primary-only operations with a standby error response that includes a redirect to the known primary host and port. + +--- + +### Requirement 2: Quorum-Based Controller Election + +**Objective:** As a cluster operator, I want the controller election to require a strict majority of peer votes, so that a minority partition cannot elect a primary and split-brain is prevented. + +#### Acceptance Criteria +1. When a controller enters the electing state, the system shall send vote requests to all peer controllers and require acknowledgments from a strict majority (greater than or equal to half the cluster size rounded up) before becoming primary. +2. When a controller receives a vote request from a candidate with a lower term, the system shall deny the vote. +3. When a controller receives a vote request from a candidate with a higher term, the system shall update its local term and clear any prior vote record for the previous term. +4. When a controller receives a vote request from a candidate whose config tag or WAL log sequence number is less fresh than its own, the system shall deny the vote. +5. When a peer controller is in catch-up synchronization and has not yet caught up to the primary's committed state, the system shall deny its vote in any election. +6. When a controller has already granted a vote to one candidate in a given term, the system shall deny any subsequent vote request from a different candidate in the same term. +7. When a candidate wins the vote count but any responding peer has a higher config tag or equal config tag with a higher WAL log sequence number, the system shall yield to that fresher peer and enter standby state. +8. When a candidate with no local config (config tag zero) determines that any peer has a config with a non-zero tag, the system shall refuse to become primary and exit with a fatal error. +9. When the cluster is a single node, the system shall self-elect as primary immediately with no quorum requirement. +10. When a split vote occurs where no candidate achieves a quorum, the system shall retry the election once with a randomized delay and incremented term. +11. If the second election attempt also fails to achieve a quorum, the system shall enter standby state. +12. When every responding peer has an identical config tag and WAL log sequence number as the candidate, the system shall use randomized pre-election jitter as the tiebreaker without considering priority values. + +--- + +### Requirement 3: Config Bootstrap for Unconfigured Nodes + +**Objective:** As a cluster operator, I want a controller node that starts with no local configuration to automatically discover and fetch configuration from the proven election winner after the election resolves, so that fresh nodes can join an existing cluster without manual intervention. + +#### Acceptance Criteria +1. When a controller starts with no local config (config tag zero), the system shall probe all peer controllers for their config tag before declaring candidacy. +2. When a controller with no local config detects that at least one peer has a non-zero config tag, the system shall participate in the election as a voter only and defer the config fetch until the election winner is known. +3. When a controller with no local config and all reachable peers also have no config, the system shall proceed to election as a normal candidate with config tag zero. +4. When a controller with no local config cannot reach any peer, the system shall retry with exponential backoff (starting at 500 milliseconds, doubling up to 10 seconds) until a peer responds or the process is killed by an administrator. +5. When the election resolves and a deferred config fetch is pending, the system shall fetch the full configuration from the proven election winner. + +--- + +### Requirement 4: Startup Validation and Safety Checks + +**Objective:** As a cluster operator, I want the controller to validate its startup prerequisites before initialization, so that misconfigured deployments are detected immediately with clear, actionable error messages. + +#### Acceptance Criteria +1. When the `--peers` list contains an even number greater than 2, the system shall reject the configuration at startup with an error message explaining that an odd number of controllers is required for majority formation. +2. When the TLS certificate or key file is missing or unreadable at the expected path, the system shall reject startup with an error message that includes the full file path. +3. When the `--vcpus` value is less than 1, the system shall reject startup with an error message. +4. When the cluster config file contains a `vcpus` key, the system shall log a deprecation warning directing operators to use the `--vcpus` CLI parameter instead. + +--- + +### Requirement 5: Majority-Commit WAL Replication + +**Objective:** As a cluster operator, I want WAL entries for critical state changes to be committed only after a majority of controllers have durably written them, so that zero data loss is guaranteed on failover. + +#### Acceptance Criteria +1. When the primary writes a job submission, job state transition, or queue change WAL entry, the system shall not acknowledge the write to the caller until a majority of controllers (including the primary itself) have confirmed durable write of that entry. +2. When the primary writes a host state or checkpoint reference WAL entry, the system shall acknowledge the write without waiting for standby confirmation. +3. When a standby controller receives a WAL entry from the primary, the system shall write it to durable storage and reply with an acknowledgment carrying the written log sequence number. +4. When a standby controller falls more than the configurable maximum pending acknowledgments behind the primary, the system shall remove that standby from the active replication set and trigger catch-up reconnection. +5. If the primary cannot obtain majority acknowledgment within the configured commit timeout, the system shall return a timeout error to the caller. +6. When a standby controller is promoted to primary, the system shall truncate any WAL entries above the primary's last known committed log sequence number before replaying the WAL. +7. When a standby controller reconnects to a primary via catch-up, the system shall truncate any uncommitted WAL entries above the primary's committed log sequence number before requesting WAL replay. +8. When the cluster is a single node, the system shall consider the local write alone as satisfying the majority requirement. + +--- + +### Requirement 6: Persistent Standby-to-Primary Control Channel + +**Objective:** As a cluster operator, I want standby controllers to maintain a persistent connection to the primary, so that failover detection is sub-second and WAL replication avoids per-entry connection overhead. + +#### Acceptance Criteria +1. When a standby controller enters standby state after election, the system shall establish and maintain a persistent TLS connection to the elected primary. +2. When the persistent control channel disconnects, the system shall detect the failure immediately and trigger a new election. +3. When heartbeat messages are received on the persistent control channel, the system shall reset the heartbeat timeout and defer election. +4. When no heartbeat is received within the election timeout plus randomized jitter on the persistent control channel, the system shall trigger a new election. +5. When a standby with an outdated config tag or applied log sequence number connects to the primary, the system shall synchronize all missing state (config snapshot and WAL entries) before admitting the standby to the active replication set. +6. When a standby's applied log sequence number is older than the primary's oldest retained WAL segment, the system shall transfer a full state snapshot before streaming WAL entries. +7. When a standby connects with a config tag that differs from the primary's, the system shall push the primary's current configuration after catch-up synchronization completes. +8. When a standby wins an election and becomes primary, the system shall close its outbound connection to the old primary and begin accepting inbound control channel connections from other standbys. + +--- + +### Requirement 7: Client-Side Transparent Retry for Transient Controller Errors + +**Objective:** As a daemon developer, I want libxce to transparently retry requests when the controller returns transient errors, so that worker, scheduler, and API server code does not need ad-hoc retry loops. + +#### Acceptance Criteria +1. When libxce receives a not-ready error from a controller, the system shall retry the request with exponential backoff against the same endpoint. +2. When libxce receives a standby error that includes a redirect to the current primary, the system shall reconnect to the redirected host and port and retry the request. +3. When libxce receives a TCP-level connection refused or timeout error, the system shall reconnect and retry the request. +4. When libxce receives a non-retriable error (authentication failure, invalid request, permission denied, not found), the system shall surface the error to the caller immediately without retry. +5. The system shall provide configurable retry parameters (maximum retries, initial delay, maximum delay, backoff multiplier) with safe production defaults. +6. When multiple clients retry simultaneously after a failover, the system shall stagger their retry attempts through exponential backoff to prevent thundering herd. + +--- + +### Requirement 8: Majority-Commit Config Replication + +**Objective:** As a cluster operator, I want configuration changes pushed to the cluster to be committed only after a majority of controllers have durably written them, so that operator changes are not silently lost on primary failover. + +#### Acceptance Criteria +1. When the primary pushes a configuration upgrade, the system shall not acknowledge the change as committed until a majority of controllers (including the primary itself) have confirmed durable write. +2. When a standby controller receives a configuration upgrade from the primary, the system shall write the config blob to durable storage and reply with an acknowledgment. +3. If the primary cannot obtain majority acknowledgment for a configuration upgrade within the configured commit timeout, the system shall return a timeout error to the caller. +4. When a standby is promoted to primary, the system shall serve only configurations up to the last known committed config tag received via heartbeat from the previous primary. +5. When the cluster is a single node, the system shall consider the local write alone as satisfying the majority requirement for config upgrades. + +--- + +### Requirement 9: Election State Durability Across Restarts + +**Objective:** As a cluster operator, I want the controller's election term and vote record to survive process restarts, so that a restarting node cannot double-vote in a term it already participated in. + +#### Acceptance Criteria +1. When a controller's term is incremented or its vote is granted to a candidate, the system shall durably persist the current term and voted-for candidate name before responding to the vote request. +2. When a controller starts up, the system shall restore the most recently persisted term and voted-for record from durable storage before participating in any election. +3. When a persisted term file does not exist at startup (first boot), the system shall initialize the term to zero with no prior vote recorded. + +--- + +### Requirement 10: Election and Replication Observability + +**Objective:** As a cluster operator, I want election results, vote tallies, config bootstrap operations, WAL commit quorum status, and config push quorum status to appear in structured logs at INFO or WARN severity, so that cluster health and HA behavior can be monitored without external tooling. + +#### Acceptance Criteria +1. When an election completes, the system shall log at INFO level the outcome (primary won or standby), the term, the number of votes granted, and the quorum size. +2. When a config bootstrap probe determines the config situation (all peers tag zero, config exists on peers, or no peer reachable), the system shall log the finding at INFO or WARN level. +3. When a WAL commit achieves quorum, the system shall log at INFO level; when a WAL commit times out, the system shall log at WARN level. +4. When a configuration upgrade achieves quorum, the system shall log at INFO level; when a configuration upgrade times out, the system shall log at WARN level. +5. When a controller transitions between the init, electing, primary, and standby states, the system shall log the transition at INFO level. diff --git a/.kiro/specs/ha-config-bootstrap/research.md b/.kiro/specs/ha-config-bootstrap/research.md new file mode 100644 index 00000000..538d7801 --- /dev/null +++ b/.kiro/specs/ha-config-bootstrap/research.md @@ -0,0 +1,277 @@ +# Gap Analysis: ha-config-bootstrap + +*Generated: 2026-05-27. Branch: persistent_ha.* + +--- + +## Analysis Summary + +- **Scope**: 10 requirements across 8 distinct changes (A–H). The codebase has strong foundational infrastructure (iomgr, repl slot management, WAL, SSL) but the HA election core, quorum commit, and control channel are mostly absent or stubbed. +- **Largest gaps**: Raft-style quorum election (Req 2) and persistent control channel with full state sync (Req 6) are full rewrites/additions with no current equivalent logic; majority-commit WAL (Req 5) requires implementing the two stub functions at the heart of replication. +- **Strong reusable assets**: `repl_add_standby/remove/start_catchup/complete_catchup/needs_snapshot` are real implementations; the SSL connect pattern (`connect_ssl_raw`), xorshift64 PRNG, `ha_do_promote/demote`, and iomgr epoll machinery are all directly reusable. +- **Risk concentration**: Req 2 and Req 6 carry High risk; Req 5 carries High risk due to `committed_lsn` cross-cutting `persist_t`, `repl.c`, and `ha_state_t`. The remaining requirements are Medium or Low. +- **Recommended approach**: Hybrid (Option A) — implement changes A→D→B→C→E→F→G→H in dependency order, writing C tests first per TDD, one atomic commit per change. + +--- + +## 1. Current State Investigation + +### 1.1 HA Core (ha.h / ha.c) + +**`ctrl_role_t` enum** — only two values: `CTRL_PRIMARY = 0`, `CTRL_STANDBY = 1`. +Missing: `CTRL_INIT`, `CTRL_ELECTING` (required by Req 1). + +**`ha_state_t`** — current fields of interest: + +| Field | Status | Notes | +|---|---|---| +| `term` (_Atomic uint64_t) | Exists | Used for fencing | +| `role` (_Atomic int32_t) | Exists | Cast to ctrl_role_t | +| `my_priority` | Exists | LSF priority index — kept for compat but unused in Raft | +| `applied_lsn` (_Atomic uint64_t) | Exists | Reusable for vote freshness check | +| `primary_host[XCE_MAXNAME_LEN]` | Exists | Reusable | +| `primary_port` | Exists | Reusable | +| `last_heartbeat` (struct timespec) | Exists | Replaces with `EPOLLRDHUP` path in Req 6 | +| `in_catch_up` (_Atomic int) | Exists | Reusable for Clarification 6 vote exclusion | +| `jitter_seed` (uint64_t) | Exists | xorshift64, directly reusable | +| `ssl_ctx` (void*) | Exists | Reusable | +| `standby_endpoints` (zarray_t*) | Exists | Superseded by per-standby fd table in Req 6 | +| `voted_for_term` (uint64_t) | **MISSING** | Req 9 | +| `voted_for_name` (char[]) | **MISSING** | Req 9 | +| `primary_ctrl_fd` (int) | **MISSING** | Req 6 | +| `defer_config_fetch` (bool) | **MISSING** | Req 3 | +| `elect_ctx` (_Atomic ptr) | **MISSING** | Req 2 | +| `last_known_committed_lsn` (_Atomic uint64_t) | **MISSING** | Req 5 | +| `heartbeat_seen` (sem_t) | **MISSING** | Req 1 | + +**`ha_config_t`** — missing: `commit_timeout_ms`. All election fields present (election_timeout_ms, jitter_max_ms, etc.) but values tuned for LSF model. + +**`am_i_primary()`** — LSF priority probe via per-peer pthreads. No quorum, no config_tag/LSN in vote, no term/voted_for persistence. Must be replaced entirely with `am_i_primary_quorum()`. + +**`ha_check_priority_stepdown()`** — permanently unreachable once term > 0; exists only for the term=0 simultaneous-boot race. Must be retired (Req 2). + +**`ha_check_heartbeat_nocluster()`** — timestamp polling with configurable timeout. Replaced by `EPOLLRDHUP` fast path + heartbeat timeout vtask (Req 6). Function itself can be deleted. + +**`ha_watchdog_thread()`** — sets `failover_requested` but cannot call `ha_promote_to_primary()` (no `cluster_t`). Partially functional stub. Replaced by vtask-based heartbeat monitor (Req 6). + +**`start_heartbeat_listener()` / `stop_heartbeat_listener()`** — no-op stubs with comment "full implementation in cluster.c Task 7". **Missing** (Req 1). + +**`ha_fini()`** — missing `SSL_CTX_free(ha->ssl_ctx)`. ASAN-visible leak (~5 KB). + +### 1.2 Startup Sequence (myapp.c / main.c) + +**myapp.c current order**: +``` +cluster_init_phase1() +initialize_openssl() + create_ssl_context() +am_i_primary() ← election here +load_primary_config() / fetch_config_from_primary() +cluster_init_phase2() ← iomgr, vtaskmgr, bind+listen inside +cluster_start() ← more network init +cluster_run() +``` + +**Bind+listen** happens inside `cluster_init_phase2()` or `cluster_start()` — **after** election. Gap: concurrent-startup nodes probe an unbound port (Req 1). + +**No `bootstrap_config_if_needed()`** — standby path goes straight to `fetch_config_from_primary()` post-election without checking whether local config_tag is 0 or whether peers have config. Gap (Req 3). + +**main.c**: +- `--vcpus N` arg: **MISSING**. vcpus comes from cluster config read inside phase2. This creates a dependency that prevents vtaskmgr from running before election (Req 1, 4). +- Peer count validation: **MISSING**. Even-N peer lists accepted silently (Req 4). +- Cert pre-flight `access()` check: **MISSING**. Cert paths computed but SSL context created later in myapp.c, not with early-exit error in main.c (Req 4). + +### 1.3 Replication (repl.c / repl.h) + +| Function | Status | Notes | +|---|---|---| +| `repl_init / fini` | Real | Lifecycle ok | +| `repl_add_standby` | Real | Slot management implemented | +| `repl_remove_standby` | Real | Implemented | +| `repl_start_catchup` | Real | Sets in_catch_up=1 | +| `repl_complete_catchup` | Real | Clears in_catch_up, broadcasts cond | +| `repl_needs_snapshot` | Real | Compares standby_lsn vs oldest_wal_lsn | +| `repl_on_ack` | Real | Updates acked_lsn, signals cond | +| `repl_trim_lsn` | Real | WAL trim guard implemented | +| `repl_broadcast` | **STUB** | Returns 0 without sending anything (Req 5, 8) | +| `repl_wait_ack` | Partial | Waits for ANY non-catch-up standby, not majority (Req 5) | +| `repl_wait_majority_ack` | **MISSING** | Req 5, 8 | + +**`standby_conn_t`** — missing `pending_ack_count` for backpressure (Req 5). + +### 1.4 Persistence (persist.h / persist.c) + +- `current_lsn` and `checkpoint_lsn` exist. **`committed_lsn`** — **MISSING** (Req 5). +- `persist_wal_sync_append()` uses `repl_wait_ack()` (any-standby) — must upgrade to `repl_wait_majority_ack()`. +- `persist_truncate_wal_above()` — **MISSING** (Req 5, 6). +- `persist_config_t.sync_timeout_ms` exists — analogous to `commit_timeout_ms` needed for majority-commit. + +### 1.5 Config Replication (config_repl.c) + +- `do_one_config_fetch()` — **wired** (recent commit `2aae45f` on branch: "wire do_one_config_fetch() — full SSL roundtrip"). Not a stub. +- `bootstrap_config_if_needed()` — **MISSING** (Req 3). +- `xce_probe_config_tag()` — **MISSING**. New lightweight RPC needed (Req 3). +- `push_config_to_standbys()` — declared in config_repl.h; likely fire-and-forget (no majority wait). **Gap**: must upgrade to majority-commit (Req 8). +- `OP_probeConfigTag` Cap'n Proto schema — **MISSING** (Req 3). +- `OP_upgradeConfigAck` Cap'n Proto schema — **MISSING** (Req 8). +- `committed_config_tag` — **MISSING** (Req 8). + +### 1.6 Control Channel (ha.c / cluster.c) + +- No persistent standby-to-primary TCP connection. Each config fetch and WAL entry opens a new SSL connection via `connect_ssl_raw()`. +- `ha_find_and_connect_to_primary()` — **MISSING** (Req 6). +- Extended `OP_ctrlHello` with applied_lsn, config_tag — **MISSING** (Req 6). +- `OP_snapshotTransfer`, `OP_walCatchupEntry` — **MISSING** (Req 6). +- `handle_ctrlhello()` extended handler — **MISSING** (Req 6). +- Three-tier primary discovery — **MISSING** (Req 6). +- EPOLLRDHUP failover detection — **MISSING** (Req 6). +- Heartbeat timeout vtask replacing watchdog thread — **MISSING** (Req 6). + +### 1.7 Election State Durability + +- `ha-term.bin` file — **does not exist**. +- `ha_persist_term()` — **MISSING** (Req 9). +- `ha_init()` zeros term and has no `ha-term.bin` read path. + +### 1.8 libxce Client Retry (src/lib/) + +- `ha_should_redirect()` / `ha_no_primary_known()` exist in ha.c and are used by join.c. +- No transparent retry loop for `XCE_ERR_NOT_READY`, `XCE_ERR_STANDBY`, `ECONNREFUSED`. +- `xce_retry_config_t` — **MISSING** (Req 7). +- `redirect_host` / `redirect_port` fields in `XCE_ERR_STANDBY` error reply — **MISSING** (Req 7). + +--- + +## 2. Requirement-to-Asset Map + +| Req | Key Gap | Key Reusable Asset | Gap Tag | +|---|---|---|---| +| 1 — Listener before election | `CTRL_INIT`/`CTRL_ELECTING` enum values; pre-dispatch gate in vtaskmgr; extract bind+listen from phase2; `ha_wait_for_heartbeat_or_timeout()`; `heartbeat_seen` semaphore | iomgr epoll, vtaskmgr dispatch switch, xorshift64 jitter | **Missing** | +| 2 — Quorum election | Entire `am_i_primary_quorum()` and vote protocol; `ha_vote_entry_t`, `ha_elect_ctx_t`; `handle_request_vote()` / `_reply()`; `ha_elect_evaluate_and_signal()`; OP_requestVote capnp schema | `ha_do_promote/demote`, jitter PRNG, `applied_lsn`, iomgr async connect | **Missing** | +| 3 — Config bootstrap | `bootstrap_config_if_needed()`; `xce_probe_config_tag()`; OP_probeConfigTag capnp; `defer_config_fetch` flag in ha_state_t | `do_one_config_fetch()` (wired), `connect_ssl_raw()` | **Missing** | +| 4 — Startup validation | `--vcpus N` CLI arg; even-N peer count check; cert pre-flight `access()`; `SSL_CTX_free` ASAN fix | argp option parsing pattern in main.c | **Missing** | +| 5 — Majority-commit WAL | `repl_broadcast()` implementation; `repl_wait_majority_ack()`; `committed_lsn` in persist_t; `persist_truncate_wal_above()`; `committed_lsn` in heartbeat; `last_known_committed_lsn` in ha_state_t | `repl_wait_ack()` logic, `standby_conn_t`, `repl_start/complete_catchup`, WAL sync path | **Missing** / **Stub** | +| 6 — Persistent channel | Persistent TCP fd per standby; `ha_find_and_connect_to_primary()`; EPOLLRDHUP failover; extended OP_ctrlHello; OP_snapshotTransfer; OP_walCatchupEntry; WAL streaming loop; heartbeat vtask; three-tier discovery | iomgr EPOLLIN/EPOLLRDHUP, `repl_needs_snapshot()`, `repl_start/complete_catchup()` | **Missing** | +| 7 — Client retry | Retry loop in libxce; `xce_retry_config_t`; redirect fields in error reply | `ha_should_redirect()`, `ha_no_primary_known()` | **Missing** | +| 8 — Majority-commit config | `repl_wait_majority_ack()` (shared with Req 5); OP_upgradeConfigAck; `committed_config_tag`; heartbeat piggybacking | `push_config_to_standbys()` (fire-and-forget → upgrade) | **Missing** | +| 9 — State durability | `ha_persist_term()`; `ha-term.bin` read in `ha_init()`; `voted_for_term`/`voted_for_name` in ha_state_t | Atomic write+rename pattern (existing in config_repl.c) | **Missing** | +| 10 — Observability | Structured log statements throughout all new code paths | Existing INFO/WARN/ERROR macros (xcelog.h) | **Missing** (alongside) | + +--- + +## 3. Implementation Approach Options + +### Option A: Change-at-a-time (recommended) + +Implement changes in dependency order: D → A → B → C → E → H → F → G → (observability woven in). + +**Rationale for ordering**: +1. D first: `--vcpus N` removes vtaskmgr's config dependency, unblocking A. +2. A before B: listener must be open before election dispatch works. +3. B before C: config bootstrap needs the proven election winner from quorum election. +4. E and H together: both use `repl_broadcast()` + `repl_wait_majority_ack()`; implement infrastructure once. +5. F after E: persistent channel carries WAL entries; broadcast must send over that channel. +6. G last: retry makes sense only after the controller states it retries against are implemented. + +**Files touched per change**: +- D: `main.c`, `cluster.c` (remove vcpus from config), `ha.c` (`SSL_CTX_free`) +- A: `ha.h` (enum + semaphore + fields), `myapp.c` (startup reorder), `cluster.c` (extract listener), `ha.c` (wait function, dispatch gate) +- B: `ha.h` (new types), `ha.c` (replace `am_i_primary`), capnp schema, `cluster.c` (dispatch handler) +- C: `config_repl.c` (`bootstrap_config_if_needed`, `xce_probe_config_tag`), capnp schema, `myapp.c` (integration) +- E: `repl.c` (`repl_broadcast`, `repl_wait_majority_ack`), `persist.h`/`persist.c` (`committed_lsn`, `persist_truncate_wal_above`), `ha.h`/`ha.c` (`last_known_committed_lsn`) +- H: `config_repl.c` (upgrade majority-commit), capnp schema, `ha.h` (`committed_config_tag`) +- F: `ha.c`/`cluster.c` (channel connect, extended hello, streaming, EPOLLRDHUP) +- G: `src/lib/` (retry loop, xce_retry_config_t, redirect fields) + +**Trade-offs**: +- ✅ Each atomic commit is testable before the next starts +- ✅ Follows existing TDD convention (failing test first) +- ❌ Long sequential dependency chain; F and G can only start late + +### Option B: Infrastructure-first parallel tracks + +Track 1 (election+bootstrap): D → A → B → C +Track 2 (replication): E → H +Join (channel): F using Track 1 + Track 2 artifacts +Final: G + +**Trade-offs**: +- ✅ Track 1 and Track 2 are mostly independent until F +- ❌ Track 2 cannot be fully validated without Track 1 (no quorum without election) +- ❌ More coordination overhead between developers on shared ha.h + +### Option C: Stub-forward then wire + +Add all new stubs with `-ENOSYS` first, then wire each one. Matches the existing codebase pattern (many stubs already present). + +**Trade-offs**: +- ✅ Compiles throughout; no long broken-build windows +- ❌ Stub-first delays real testability; TDD mandate disfavors this + +**Recommendation**: Option A. The dependency chain is well-defined and single-developer serial implementation is the realistic mode for this C codebase. + +--- + +## 4. Effort and Risk Summary + +| Req | Effort | Risk | Justification | +|---|---|---|---| +| 1 — Listener before election | M (3–5d) | Medium | Refactor startup order; must remove vtaskmgr/config dependency first | +| 2 — Quorum election | XL (2+ wks) | High | Full algorithm replacement; new wire protocol; FSM extension; concurrent dispatch | +| 3 — Config bootstrap | M (3–5d) | Medium | New function + probe RPC; depends on do_one_config_fetch (wired) | +| 4 — Startup validation | S (1d) | Low | Additive CLI + validation; ASAN one-liner | +| 5 — Majority-commit WAL | L (1–2 wks) | High | `committed_lsn` cross-cuts persist+repl+ha; `repl_broadcast` must become real | +| 6 — Persistent channel | XL (2+ wks) | High | New connection lifecycle; full state sync protocol; 3-tier discovery | +| 7 — Client retry | M (3d) | Low | Well-understood retry pattern; additive libxce change | +| 8 — Majority-commit config | M (3d) | Medium | Shares Req 5 infrastructure; mainly wiring push_config_to_standbys | +| 9 — State durability | S (1d) | Low | File write+read; extends ha_init | +| 10 — Observability | S (alongside) | Low | Log statements woven into each change | + +**Total estimated effort**: 6–9 weeks for one engineer implementing with full TDD. + +**High-risk dependencies**: +- Req 2 must complete before Req 3 can be integrated end-to-end. +- Req 5 (`repl_broadcast` implementation) must complete before Req 8 and before Req 6 (WAL streaming over the channel). +- Req 4 (vcpus CLI) must complete before Req 1 (vtaskmgr before election). + +--- + +## 5. Key Reusable Assets + +| Asset | Location | Reuse in | +|---|---|---| +| `repl_add/remove_standby` | repl.c | Req 6 (channel registration) | +| `repl_start/complete_catchup` | repl.c | Req 6 (state sync protocol) | +| `repl_needs_snapshot` | repl.c | Req 6 (snapshot decision) | +| `repl_on_ack` | repl.c | Req 5 (majority-commit ack handler) | +| `repl_trim_lsn` | repl.c | Req 5 (WAL trim with committed_lsn) | +| `connect_ssl_raw()` | netio.c/config_repl.c | Req 3 (probe RPC) | +| `do_one_config_fetch()` | config_repl.c | Req 3 (deferred fetch) | +| `xorshift64_next + ha_seed_jitter` | ha.h / ha.c | Req 1, 2 (jitter) | +| `ha_do_promote / ha_do_demote` | ha.c | Req 2, 6 (role transitions) | +| `ha_should_redirect / ha_no_primary_known` | ha.c | Req 7 (retry redirect) | +| iomgr epoll + EPOLLIN/EPOLLRDHUP | iomgr (cluster.c) | Req 1, 6 | +| Atomic write+rename pattern | config_repl.c | Req 9 (ha-term.bin) | +| INFO/WARN/ERROR macros | xcelog.h | Req 10 | + +--- + +## 6. Constraints for Design Phase + +- **C gnu17, no C++, no new external dependencies**. +- ASAN build (`nix build .#xcecore_debug`) must pass after every change. +- **Static peer membership**: `--peers` list is fixed at startup; no dynamic join/leave of controller peers. Quorum denominator is fixed for cluster lifetime. +- **TDD mandatory**: failing test in `src/controller/testsuite/` before each implementation change. +- `make test` must pass without a live server at every commit. +- `commit_timeout_ms` must be configurable (default 5000ms). +- **Single-node mode** (no `--peers`): quorum=1 everywhere; all new majority-commit paths must short-circuit to self-only. +- **WAL write stall window** (operational constraint, not a bug): during standby catch-up after election, `repl_wait_majority_ack()` will time out on every write until at least one standby completes catch-up. This is correct CP behavior; operators must be warned in docs. + +--- + +## 7. Research Needed (Design Phase) + +- **vtaskmgr API for pre-election dispatch**: confirm that `vtaskmgr_init(vcpus)` can be called before `cluster_config` is loaded (after removing vcpus from config struct). Verify no hidden config dependency. +- **iomgr async connect pattern for vote requests**: confirm `iomgr_async_vote_request()` can be implemented using existing async connect infrastructure without new socket primitives. +- **capnp schema extension**: confirm the capnp code-generation toolchain can add `OP_requestVote`, `OP_probeConfigTag`, `OP_walAck`, `OP_upgradeConfigAck`, `OP_snapshotTransfer`, `OP_walCatchupEntry` without breaking existing decode paths. +- **`persist_truncate_wal_above()` safety**: verify WAL segment file structure supports efficient truncation at arbitrary LSN boundary without full segment rewrite. +- **libxce client API surface**: identify the exact function in src/lib/ that wraps xce_encode()+send+recv+xce_decode() to locate the correct insertion point for retry logic. diff --git a/.kiro/steering/error-handling.md b/.kiro/steering/error-handling.md new file mode 100644 index 00000000..0b6e4ee9 --- /dev/null +++ b/.kiro/steering/error-handling.md @@ -0,0 +1,92 @@ +# Error Handling Standards + +## Philosophy + +Fail fast with a consistent, machine-readable response. Handle errors at the controller boundary; let business logic return typed errors. Never silently swallow errors. + +## Go API Response Pattern + +All HTTP responses go through two helpers in `controllers/response.go`: + +```go +// Success (always HTTP 200) +ReturnSuccess(ctx, common.SUCCESS_CODE, common.SUCCESS_MSG, data) + +// Error (HTTP status == code parameter) +ReturnError(ctx, common.ERROR_AUTH_CODE, common.ERROR_ACCESS_FORBIDDEN_MSG, nil) +ctx.Abort() // always Abort() after ReturnError in middleware +return // always return immediately after ReturnError in handlers +``` + +**The early-return pattern is non-negotiable.** Every error path ends with `ReturnError(...); return`. + +## HTTP Status Code Mapping + +| Code | Constant | When to use | +|------|----------|-------------| +| 200 | `SUCCESS_CODE` | Successful operation | +| 201 | `CREATED` | Resource created | +| 400 | `INVALID_PARAMS_CODE` | Malformed or invalid input | +| 401 | `ERROR_AUTH_CHECK_TOKEN_CODE` | Missing or invalid JWT | +| 403 | `ERROR_AUTH_CODE` | Valid JWT but insufficient permission | +| 404 | `ERROR_NOT_FOUND_CODE` | Resource not found | +| 429 | `ERROR_TOO_MANY_REQUESTS_CODE` | Rate limit exceeded | +| 500 | `ERROR_CODE` | Unexpected server error | + +All code constants are defined in `common/common.go`. Add new codes there — do not inline numeric literals in handlers. + +## Structured Logging with Zap + +Log at the call site before returning an error. Use typed field helpers — never `fmt.Sprintf` in zap calls. + +```go +// Preferred pattern +logger.Error("rate limit exceeded", + zap.String("path", path), + zap.String("client_ip", ctx.ClientIP()), + zap.Int("errorcode", code), +) + +// For error values +logger.Error("validate token error", zap.Error(err)) + +// For complex types +logger.Debug("permissions resolved", zap.Any("permsWithType", permsWithType)) +``` + +**Log levels**: +- `Error` — operation failed, client or server fault +- `Warn` — recoverable edge case (e.g., retried operation succeeded) +- `Info` — key lifecycle events (server start, config load) +- `Debug` — diagnostic detail, disabled in production + +**Never log**: JWT tokens, passwords, secret keys, or full request bodies containing sensitive data. + +## C Error Propagation + +Return negative errno-style codes for C daemon errors. Use the `errnox.h` definitions from `src/include/xce/`. Check and propagate immediately — do not continue on error: + +```c +int rc = xce_send_request(&req); +if (rc < 0) { + log_error("send_request failed: %d", rc); + return rc; +} +``` + +## Input Validation Pattern + +Validate at the controller boundary using a `preCheck{Resource}Request()` helper before touching services: + +```go +func (c JobController) SubmitJob(ctx *gin.Context) { + req, err := preCheckSubmitJobRequest(ctx) + if err != nil { + ReturnError(ctx, common.INVALID_PARAMS_CODE, err.Error(), nil) + return + } + // ... service call +} +``` + +Keep `preCheck*` functions in the same file as the handler. They bind, validate, and return the typed request struct or an error. diff --git a/.kiro/steering/product.md b/.kiro/steering/product.md new file mode 100644 index 00000000..a4266092 --- /dev/null +++ b/.kiro/steering/product.md @@ -0,0 +1,43 @@ +# Product Overview + +XCE is an HPC (High-Performance Computing) cluster management system providing job scheduling, queue management, and cluster resource control. It targets organizations running batch workloads on on-premises or air-gapped compute clusters who need deterministic scheduling, multi-tenant access control, and zero external database dependencies. + +## Core Capabilities + +1. **Job lifecycle management** — submit, start, stop, restart, kill, and monitor jobs including job arrays. Resource usage data is collected and exposed via API. +2. **Queue and scheduling** — plugin-based scheduling engine for job-to-node placement with queue control (enable/disable) and expression-based resource filtering. +3. **Cluster and host management** — node status monitoring (online/offline/unavailable), cluster topology tracking, and host control operations. +4. **Multi-tenant RBAC + pluggable auth** — domain-based `action:scope` permission model (e.g., `job:view:self`). Auth backends: configfile, LDAP, OAuth2, PAM. Built-in IP whitelist and rate limiting. +5. **Controller persistence and HA** — WAL + checkpoint-based single-controller persistence with LSF-style priority election for standby failover and live replication over TCP (Cap'n Proto serialization). + +## Target Use Cases + +- **Local HPC batch computing** — long-running scientific/engineering workloads requiring deterministic scheduling, queue priority, and resource isolation. +- **Air-gapped / secure environments** — standalone deployments with file-based auth + SSL, no external cloud dependencies. +- **Multi-team shared clusters** — multiple teams sharing a cluster through RBAC with JWT token authentication. +- **No-shared-storage HPC** — controller uses local filesystem (ext4/xfs) for storage; job files transferred via API upload/download rather than network shares. + +## Value Proposition + +XCE provides a **fully integrated single-vendor stack** (no database setup, unified build system) with an open, buildable codebase (Nix flakes, AppImage packaging). Compared to Slurm/PBS, it eliminates the external database requirement. Compared to commercial LSF, it is open source and reproducibly buildable. + +**Key architectural trade-off**: single-writer model throughout — no multi-master consensus. HA failover uses LSF-style priority election (simple, predictable) rather than Raft (10× complexity). All controller state is in-memory; persistence is WAL + checkpoint, not a traditional database. + +## Active Development Context + +The project is in a **persistence and HA transformation** (branch `persistent_ha`). Phases: + +| Phase | Goal | Status | +|-------|------|--------| +| P0 | Restart survival (WAL + checkpoint) | Implementing | +| P1 | Standby election and takeover | Designed | +| P2 | Live replication primary → standby | Designed | +| P2.5 | Worker/scheduler auto-reconnect on failover | Designed | +| P3–P4 | Hardening, metrics, docs | Planned | + +Key design decisions already locked: +- No `fork()` for snapshots (mutex inheritance deadlock in multithreaded controller) +- No Raft (single writer makes consensus overhead unjustified) +- No BoltDB (using dual zarray COW for application-level checkpointing) +- No application-layer encryption (trust OS boundary: chmod 0700, LUKS/dm-crypt) +- No WAL logging for job deletion (deferred to checkpoint time to simplify replay) diff --git a/.kiro/steering/security.md b/.kiro/steering/security.md new file mode 100644 index 00000000..c928a46c --- /dev/null +++ b/.kiro/steering/security.md @@ -0,0 +1,61 @@ +# Security Standards + +## Philosophy + +Defense in depth, least privilege, fail closed. Authentication verifies *who*; authorization verifies *what they can do*. Centralize both — never duplicate permission checks across handlers. + +## RBAC Enforcement + +All protected routes use `RequirePermission` middleware attached at registration time: + +```go +// In router/routers.go +hostsGroup.GET("", RequirePermission("host:view"), handler) +hostsGroup.POST("/control", RequirePermission("host:control"), handler) +``` + +`RequirePermission` in `controllers/rbac_check.go` resolves the user from the JWT, looks up their effective permissions (including inherited role permissions), and aborts with 403 if the required permission is absent. + +**cluster_admin** bypasses all permission checks via early return — do not add new bypass paths outside this single mechanism. + +## Permission String Format + +`"action:scope"` — e.g., `"job:submit"`, `"host:view"`, `"queue:manage"`. + +Scopes: `all` (unrestricted), `self` (own resources), `cluster`, `hostgroup`, `self_ug`, `self_hg`. + +Permissions are defined in `config/rbac.yaml`. Roles support inheritance via `includes:` — resolution is recursive. Add new permissions to the YAML first, then wire them onto routes. + +## JWT Authentication + +Token flow: +1. `POST /auth/logon` → validates credentials via configured auth backend → issues JWT with `{username, IP, expiry}` claims +2. Every subsequent request: `Authorization: Bearer <token>` header +3. `middleware/validatehandler.go` strips the prefix, parses with golang-jwt, **validates the IP claim against the request IP** + +IP-bound tokens prevent token theft across network boundaries. Do not remove the IP validation. + +Token secret is loaded from `config/auth.yaml` at startup. It must never appear in source code or logs. + +## Auth Backends + +Backend selection is controlled by `config/auth.yaml`. Supported: `configfile`, `ldap`, `oauth2`, `pam`. All backends implement the same interface — adding a new backend means implementing that interface, not modifying middleware. + +## Input Validation + +Validate at the API boundary inside `preCheck{Resource}Request()` helpers — before any service or store access. Return 400 with a clear message on validation failure. Do not pass unvalidated input to downstream layers. + +## Secrets + +- JWT secret key: `config/auth.yaml` (file-system protected, chmod 0600) +- LDAP bind credentials: `config/auth.yaml` +- **Never log** tokens, passwords, secret keys, or full request bodies containing credentials +- Validate all required config at startup; fail fast with a clear error if missing + +## Rate Limiting and IP Whitelist + +Rate limiting (`middleware/rate_limit.go`) and IP whitelist (`middleware/ip_whitelist.go`) are applied before JWT validation in the middleware chain. They use `ctx.ClientIP()` and abort with 429 / 403 respectively. Both are configured in `config/` YAML — do not hardcode thresholds. + +## Transport + +All daemon-to-daemon communication is over TCP. Production deployments add TLS at the network layer (not handled in application code). The WAL and replication wire format uses Cap'n Proto — schema files in `src/schema/`. Do not add unvalidated deserialization paths for external input. diff --git a/.kiro/steering/structure.md b/.kiro/steering/structure.md new file mode 100644 index 00000000..2c54665b --- /dev/null +++ b/.kiro/steering/structure.md @@ -0,0 +1,99 @@ +# Project Structure + +## Organization Philosophy + +**Layered monorepo, component-per-directory.** Each major subsystem lives under `src/<component>/` with its own Meson build file and CLAUDE.md. The Go frontend is isolated in `src/frontend/` as two independent Go modules. All C daemons share `src/lib/` (libxce) and public headers from `src/include/xce/`. + +``` +src/ +├── lib/ C — shared library (libxce): data structures, IPC, utilities +├── controller/ C — cluster controller daemon +├── worker/ C — per-node job execution daemon (xcecd) +├── scheduler/ C — job scheduling engine (plugin architecture) +├── autoscaler/ C — autoscaling engine (provider plugins) +├── frontend/ Go — apiserver (Gin REST) + xcecli (Cobra CLI) +├── include/xce/ C — public shared headers consumed across all daemons +├── schema/ Cap'n Proto schema files (c.capnp, controller.capnp) +└── docs/ Project-wide documentation (markdown) +``` + +## C Daemon Layout Pattern + +Each C daemon (`controller/`, `worker/`, `scheduler/`, `autoscaler/`) follows: + +``` +src/<component>/ + meson.build — build definition + CLAUDE.md — component-specific AI guide + include/ — component-internal headers + src/ — .c implementation files (flat, one domain per file) + testsuite/ — test programs (test_*.c) +``` + +**Exception**: `controller/` keeps its `.c` files flat in the component root (not in a nested `src/`). All other C daemons use `src/<component>/src/`. + +Plugin-based components add their own subdirectory: +- `scheduler/` — plugins in `src/plugins/` +- `autoscaler/` — providers in `src/autoscaler/providers/` + +## Go Frontend Layout Pattern + +**apiserver** (`src/frontend/apiserver/`): + +| Package | Purpose | +|---------|---------| +| `cmd/` | Entry point, init sequence | +| `router/` | Route registration + middleware chain | +| `controllers/` | HTTP handlers — one file per resource | +| `services/` | Business logic | +| `store/` | Thread-safe in-memory KV stores (generics) | +| `models/` | Request/response DTOs, domain structs | +| `middleware/` | CORS, rate limiting, JWT, IP whitelist | +| `backend/` | CGO bridge to C library | +| `backend/auth/` | Pluggable auth providers | +| `utils/` | Zap logger wrapper, config parsing, crypto | +| `common/` | Global error codes and response helpers | +| `config/` | YAML configs + JSON schemas | + +## Naming Conventions + +### C + +- **Files**: `snake_case.c` / `snake_case.h` — one domain per file +- **Public headers**: `src/include/xce/<name>.h` — the libxce API boundary +- **Component headers**: `src/<component>/include/<name>.h` +- **Test files**: `test_<unit>.c` (e.g., `test_ha_cluster.c`, `test_persist_wal.c`) +- **Functions**: `snake_case` with component prefix — `ha_*`, `cluster_*`, `xcecd_*`, `xce_*` +- **Types**: `snake_case_t` suffix (e.g., `cluster_t`, `ha_state_t`) +- **Struct tags**: `struct <name>_s` (e.g., `struct cluster_s`) +- **Enums/Macros**: `UPPER_SNAKE_CASE` with component prefix (`CTRL_PRIMARY`, `HA_DEFAULT_*`) + +### Go + +- **Files**: one file per resource — `jobs.go`, `queues.go`, `hosts.go` +- **Tests**: `*_test.go` co-located; integration tests as `integration_test.go`, `<resource>_control_test.go` +- **Controller structs**: `{Resource}Controller` +- **Handler methods**: `{Action}{Resource}` (e.g., `SubmitJob`, `ListJobs`) +- **Permission strings**: `"action:scope"` (e.g., `"job:view:self"`) +- **JSON tags**: `snake_case` + +## Test Organization + +**C tests** (`testsuite/` inside each component): +- Standalone `.c` programs, one per unit: `test_<unit>.c` +- `test.c` provides the test runner harness +- Tiered naming: `test_p1_integration.c`, `test_e2e_wal.c`, `test_p3_hardening.c` +- Test-only symbols guarded by `#ifdef XCE_TESTING` + +**Go tests** (co-located `_test.go`): +- Table-driven unit tests, `t.TempDir()` for fixtures +- Global state reset helpers (e.g., `resetRBACState()`) for test isolation +- `make test` = unit only; `make test-all` = unit + integration + +## Code Organization Principles + +- **One domain per file** in C — `ha.c` owns HA logic, `job.c` owns job logic. No megafiles. +- **Single Go module per binary** — apiserver and xcecli share no Go code at the module level. +- **Controllers delegate to services** — HTTP handlers validate input and delegate; business logic lives in `services/`. +- **Dependencies flow inward**: `controllers → services → store/backend`, never the reverse. +- **Cap'n Proto for wire formats** — any new inter-daemon or persistence protocol uses `.capnp` schemas in `src/schema/`. diff --git a/.kiro/steering/tech.md b/.kiro/steering/tech.md new file mode 100644 index 00000000..30b38d2a --- /dev/null +++ b/.kiro/steering/tech.md @@ -0,0 +1,100 @@ +# Technology Stack + +## Architecture + +``` +xcecli (Go/Cobra) ──REST──► apiserver (Go/Gin) ──CGO──► C daemon layer + controller / scheduler / worker / autoscaler + ↑↑↑ all linked against libxce (src/lib/) +``` + +Request path inside apiserver: `Gin Router → Middleware (CORS/RateLimit/IPWhitelist/JWT) → RBAC Check → Controller → Services → In-Memory Store` + +All controller state is in-memory, seeded at startup from YAML config. No persistent database in the API tier. C daemons communicate via libtirpc RPC; WAL + checkpoint provide persistence at the controller level. + +## Core Technologies + +- **Go 1.23.4** — apiserver (REST API) and xcecli (CLI). Two independent Go modules, no `go.work`. +- **C (gnu17)** — controller, worker, scheduler, autoscaler, libxce. No C++. +- **Gin v1.10.0** — HTTP router, middleware, binding +- **Cobra v1.9.1** — CLI framework for xcecli +- **Zap v1.27.0** — structured logging throughout (both Go modules) +- **Viper v1.19.0 / v1.20.1** — YAML config management +- **Cap'n Proto** — wire serialization for WAL and daemon-to-daemon replication +- **libtirpc** — inter-daemon RPC transport + +## Build Systems + +| System | Scope | +|--------|-------| +| **Meson ≥ 1.5** | C daemons + Go modules via `tools/go_build` wrapper | +| **Make** | Go development loop (build/test) from `src/frontend/` | +| **Nix flakes** | Hermetic dev environment, AppImage packaging | + +```bash +# Full build (C + Go, debug + ASAN) +/xce-build + +# Go development loop (from src/frontend/) +make build-server # → /tmp/apiserver +make build-cli # → /tmp/xce_static (CGO_ENABLED=0) +make test # unit tests +make test-integration # regression suite (needs XCE_BIN + SRV_URL) + +# Nix package builds (from src/) +nix build .#xcecore # C daemons + apiserver +nix build .#xcecore_debug # debug + ASAN +nix build .#xcecli # CLI only +``` + +## Development Standards + +### Controller Pattern (Go) + +```go +// Struct: {Resource}Controller +type JobController struct { ... } + +// Handler: {Action}{Resource} +func (c JobController) SubmitJob(ctx *gin.Context) { + // Validate input + req, err := preCheckSubmitJobRequest(ctx) + if err != nil { ReturnError(ctx, code, err, nil); return } + + // Business logic + result, err := c.svc.Submit(req) + if err != nil { ReturnError(ctx, code, err, nil); return } + + ReturnSuccess(ctx, code, "submitted", result) +} +``` + +Early return on every error. Log with zap fields before returning. + +### Testing Requirements + +- **TDD mandatory**: Write failing test first, then implementation. +- **Go unit tests**: Table-driven, no external dependencies, `t.TempDir()` for fixtures. +- **C tests**: Add test case in `testsuite/` before modifying implementation. +- `make test` must pass without a live server. +- `make test-integration` requires `XCE_BIN` + `SRV_URL` env vars. + +### Code Formatting + +- Go: `gofmt` / `goimports` (post-edit hooks configured) +- C: `clang-format` using project config at `xce/.clang-format` + +### Auth and RBAC + +- Auth backend set in `config/auth.yaml`. Supports: configfile, LDAP, OAuth2, PAM. +- Permissions as `"action:scope"` strings: `"job:view:self"`, `"queue:submit:all"`. +- RBAC loaded from `config/rbac.yaml`, enforced by `controllers.RequirePermission()` middleware. + +## Key Technical Decisions + +1. **No Go workspace** — `apiserver` and `xcecli` are independent modules; changes to shared types require updating both. +2. **In-memory API state** — no database in Go tier; state loaded from YAML at startup via `store.LoadBuiltinData()`. +3. **CGO bridge** — `backend/` package wraps C library calls; unit tests run `CGO_ENABLED=0`, backend tests need `CGO_ENABLED=1` inside nix dev shell. +4. **Swagger-first API docs** — `swaggo/swag` annotations on handlers; docs served at runtime by Gin. +5. **Structured logging** — every log entry carries context via `zap.String()`, `zap.Error()`. Never use `fmt.Println` for operational output. +6. **Single-writer HA** — controller is always one active primary; standby election via LSF-style priority, not consensus. diff --git a/.kiro/steering/testing.md b/.kiro/steering/testing.md new file mode 100644 index 00000000..3f588852 --- /dev/null +++ b/.kiro/steering/testing.md @@ -0,0 +1,89 @@ +# Testing Standards + +## Philosophy + +- **TDD mandatory**: Write a failing test first, then the minimal implementation. The cycle is Red → Green → Refactor. +- Test behavior, not implementation details. +- Tests must run without a live server or external services. +- A change is not complete until `make test` (and `make test-integration` where applicable) passes. + +## Go Tests (apiserver / xcecli) + +**Co-located** alongside source: `jobs.go` → `jobs_test.go`. Integration tests use descriptive names: `integration_test.go`, `host_control_test.go`. + +**Table-driven** for validation/filter logic: + +```go +tests := []struct { + name string + input SomeRequest + wantErr bool +}{ + {"valid request", validReq, false}, + {"missing field", missingReq, true}, +} +for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := preCheckRequest(tt.input) + if (err != nil) != tt.wantErr { t.Errorf(...) } + }) +} +``` + +**State isolation** — reset global store state between tests: + +```go +func resetRBACState() { /* clear in-memory stores */ } + +func TestPermissions(t *testing.T) { + t.Cleanup(resetRBACState) + // ... +} +``` + +**Fixtures**: use `t.TempDir()` for file-system state; never rely on absolute paths or external services. + +**Build targets**: +- `make test` — unit tests only (`CGO_ENABLED=0`) +- `make test-backend` — CGO-backed backend tests (requires nix dev shell + build dir) +- `make test-integration` — full regression suite (requires `XCE_BIN` and `SRV_URL` env vars) + +## C Tests (controller / worker / scheduler / lib) + +**Location**: `testsuite/` directory inside each C component. + +**Structure**: one `.c` test program per unit: + +``` +src/controller/testsuite/ + test.c ← test runner harness + test_ha_cluster.c ← tests for ha.c + test_persist_wal.c ← WAL-specific tests + test_e2e_checkpoint.c ← end-to-end checkpoint flow + test_p1_integration.c ← P1 HA integration tier +``` + +File naming convention: `test_<unit>.c`. E2E tests: `test_e2e_<topic>.c`. Integration tiers: `test_p1_*.c`, `test_p2_*.c`, `test_p3_hardening.c`. + +**Test-only symbols** are guarded to prevent production bloat: + +```c +#ifdef XCE_TESTING +extern void reset_ha_state(ha_state_t *s); +#endif +``` + +**Add a test case in `testsuite/` before modifying any C implementation.** + +## Coverage Targets + +- Go: 80% minimum on business logic (controllers, services, store) +- C: every new function in a daemon must have at least one test case in `testsuite/` +- Integration: all public REST endpoints must have at least one integration test case in `regression/` + +## What Not to Mock + +- Mock **external dependencies only** (network calls, file I/O that crosses component boundaries) +- Do **not** mock the system under test +- In-memory stores (`store/`) are real — use them directly in unit tests +- CGO bridge (`backend/`) is covered by `make test-backend` — do not stub it in unit tests diff --git a/CLAUDE.md b/CLAUDE.md index 04ab1d40..cb7f10e6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,3 +1,10 @@ +# OpenWolf + +@.wolf/OPENWOLF.md + +This project uses OpenWolf for context management. Read and follow .wolf/OPENWOLF.md every session. Check .wolf/cerebrum.md before generating code. Check .wolf/anatomy.md before reading files. + + # XCE Cluster Management System ## Overview @@ -133,31 +140,38 @@ Kiro-style Spec-Driven Development on an agentic SDLC ### Active Specifications - Check `.kiro/specs/` for active specifications -- Use `/kiro:spec-status [feature-name]` to check progress +- Use `/kiro-spec-status [feature-name]` to check progress ## Development Guidelines - Think in English, generate responses in English. All Markdown content written to project files (e.g., requirements.md, design.md, tasks.md, research.md, validation reports) MUST be written in the target language configured for this specification (see spec.json.language). ## Minimal Workflow -- Phase 0 (optional): `/kiro:steering`, `/kiro:steering-custom` +- Phase 0 (optional): `/kiro-steering`, `/kiro-steering-custom` - Phase 1 (Specification): - - `/kiro:spec-init "description"` - - `/kiro:spec-requirements {feature}` - - `/kiro:validate-gap {feature}` (optional: for existing codebase) - - `/kiro:spec-design {feature} [-y]` - - `/kiro:validate-design {feature}` (optional: design review) - - `/kiro:spec-tasks {feature} [-y]` -- Phase 2 (Implementation): `/kiro:spec-impl {feature} [tasks]` - - `/kiro:validate-impl {feature}` (optional: after implementation) -- Progress check: `/kiro:spec-status {feature}` (use anytime) + - `/kiro-spec-init "description"` — initialize spec scaffold + - `/kiro-spec-quick "description"` — fast single-pass spec (requirements + design + tasks, no approval gates) + - `/kiro-spec-batch` — generate specs for all features in a roadmap in parallel + - `/kiro-spec-requirements {feature}` + - `/kiro-validate-gap {feature}` (optional: for existing codebase) + - `/kiro-spec-design {feature} [-y]` + - `/kiro-validate-design {feature}` (optional: design review) + - `/kiro-spec-tasks {feature} [-y]` +- Phase 2 (Implementation): `/kiro-spec-impl {feature} [tasks]` + - `/kiro-validate-impl {feature}` (optional: after implementation) + - `/kiro-verify-completion {feature}` — validate all acceptance criteria are met +- Debugging: `/kiro-debug {feature}` — systematic debugging with spec context +- Code review: `/kiro-review` — review implementation against spec requirements +- Progress check: `/kiro-spec-status {feature}` (use anytime) ## Development Rules - 3-phase approval workflow: Requirements → Design → Tasks → Implementation - Human review required each phase; use `-y` only for intentional fast-track -- Keep steering current and verify alignment with `/kiro:spec-status` +- Use `/kiro-spec-quick` for small, well-understood features; full workflow for anything touching HA/persistence/RBAC +- Keep steering current and verify alignment with `/kiro-spec-status` - Follow the user's instructions precisely, and within that scope act autonomously: gather the necessary context and complete the requested work end-to-end in this run, asking questions only when essential information is missing or the instructions are critically ambiguous. ## Steering Configuration - Load entire `.kiro/steering/` as project memory -- Default files: `product.md`, `tech.md`, `structure.md` -- Custom files are supported (managed via `/kiro:steering-custom`) +- Core files: `product.md`, `tech.md`, `structure.md` +- XCE custom files: `testing.md`, `error-handling.md`, `security.md` +- Add new custom files via `/kiro-steering-custom` diff --git a/docs/superpowers/plans/2026-05-20-config-repl-wire-protocol.md b/docs/superpowers/plans/2026-05-20-config-repl-wire-protocol.md new file mode 100644 index 00000000..902de550 --- /dev/null +++ b/docs/superpowers/plans/2026-05-20-config-repl-wire-protocol.md @@ -0,0 +1,549 @@ +# Config Replication — Wire Protocol Completion + Bug Fixes + +> **SDD Plan** — Subagent-Driven Development +> **Date**: 2026-05-20 +> **Goal**: Complete Phase 1 wire protocol (Task 11), fix path bug, implement push timeout. +> **Unblocks**: Standby config fetch, OP_upgradeConfig, Phase 1 design claim becoming true. + +--- + +## Context + +Three independent streams of work emerged from the design review and gap analysis: + +1. **Task 11 (wire protocol)** — `do_one_config_fetch()` and `handle_config_request()` are + stubs returning `-ENOSYS` / sending NULL bytes because `repl_config_reply_t.sha256` and + `.bytes` are `xce_data_t*` (a discriminated cluster-data union), not raw byte carriers. + The pattern to fix this already exists: `xce_replconfigpush_req_raw()` in `request.c:154` + uses `capn_new_list8` + `capn_setv8` to bypass the `xce_data_t` path for raw bytes. + Task 11 applies that same pattern to the reply (primary side) and the request+reply + decode (standby side + upgrade handler). + +2. **Path bug** — `conf_save_config()` uses `make_conf_dir(myconf->workdir, ...)` producing + `workdir/conf`, but `load_primary_config()` reads from `workdir/name/conf`. Files written + by an upgrade would be in the wrong location, causing FATAL on restart. + +3. **Push timeout** — `push_config_to_standbys()` uses blocking `pthread_join()` with no + bound. Design §6.3 specifies a 500ms aggregate timeout. `CONF_PUSH_TIMEOUT_MS` is defined + but unused. + +--- + +## Dependency Map + +``` +Task A ──→ Task B (primary reply shim) +(shims) ──→ Task C (standby fetch) → all wire protocol done + ──→ Task D (upgrade wiring) + +Task E (path bug) — independent, do first +Task F (push timeout) — independent, do in parallel with Task A +``` + +**Wave 1 (parallel):** Task A + Task E + Task F +**Wave 2 (parallel, after Task A done):** Task B + Task C + Task D + +--- + +## Task A — `request.c`: Two new raw shims for OP_replConfig + +**File:** `src/lib/request.c` +**Header:** add declarations to `src/include/request.h` + +**Pattern to copy:** `xce_replconfigpush_req_raw()` at `request.c:154–225` + +### A.1 — Implement `xce_replconfig_reply_raw()` + +Primary uses this to send `ReplConfigReply` with real bytes (up_to_date=false path). + +```c +/* + * xce_replconfig_reply_raw: send OP_replConfig reply with raw byte arrays. + * + * Bypasses xce_data_t for sha256 and bytes fields (same pattern as + * xce_replconfigpush_req_raw). Called by handle_config_request() when + * the standby's tag differs from the primary's. + * + * ep — endpoint to reply on + * from — primary's peerinfo + * seq — request sequence number (for correlation) + * tag — primary's version tag + * sha256 — 32-byte SHA-256 of the config blob + * config — raw capnp config blob + * config_len — byte length of config blob + */ +int xce_replconfig_reply_raw(endpoint_t *ep, xce_peerinfo_t *from, + uint64_t seq, + uint64_t tag, + const uint8_t *sha256, size_t sha256_len, + const uint8_t *config, size_t config_len); +``` + +Implementation mirrors `xce_replconfigpush_req_raw()` but: +- Builds a `RequestReply` (not `Request`) struct +- Sets `opcode = OPCode_replConfig`, `status = 0` +- Sets `data_which = RequestReply_data_replconfigreplymsg` +- Writes `up_to_date = 0`, `version_tag = tag` +- Uses `capn_new_list8` + `capn_setv8` for sha256 and bytes fields +- Calls `write_ReplConfigReply` then `write_RequestReply` +- Enqueues via `endpoint_enq` + +### A.2 — Implement `xce_data_extract_raw()` + +Decoder shim: extracts raw bytes from a received capnp Data field (stored as `capn_list8` +internally, presented as `xce_data_t*` via the typemap). Used by the standby to read the +reply and by the upgrade handler to read input bytes. + +```c +/* + * xce_data_extract_raw: extract raw bytes from a capnp Data field. + * + * The sha256/bytes fields in ReplConfigReply and UpgradeConfigRequest are + * typed as xce_data_t* in the typemap — the underlying capnp pointer is a + * capn_list8 (List(UInt8)). This function reads the bytes out using + * capn_getv8(). + * + * data — the xce_data_t* from the decoded message (may be NULL) + * out — heap-allocated buffer written here (caller must free) + * out_len — byte count written here + * + * Returns 0 on success, -ENODATA if data is NULL/empty, -ENOMEM on alloc failure. + */ +int xce_data_extract_raw(const xce_data_t *data, + uint8_t **out, size_t *out_len); +``` + +Implementation: +- If `data == NULL` → return `-ENODATA` +- The capnp `Data` type stores as `capn_list8`; access via the raw `capn_ptr` embedded in + `xce_data_t` (needs a union-cast or a new accessor — check how `capnp_repl_envelope_t.payload` + is decoded in existing code for the pattern) +- `malloc(len)`, `capn_getv8(list, 0, out, len)`, set `*out_len` + +### A.3 — Implement `xce_replconfig_req_raw()` + +Standby uses this to send a `OP_replConfig` request over the standalone SSL socket. + +```c +/* + * xce_replconfig_req_raw: send OP_replConfig request over an SSL socket. + * + * Used by do_one_config_fetch() on a raw SSL connection (no iomgr). + * Serialises ReplConfigRequest{version_tag}, writes to ssl_fd. + * + * Returns bytes written on success, negative errno on failure. + */ +int xce_replconfig_req_raw(SSL *ssl, uint64_t version_tag); +``` + +Implementation: use `capn_init_malloc` + `write_ReplConfigRequest` + +`capn_write_fd(ssl_fd)` pattern (see how `xce_encode` works in `request.c`). + +### A.4 — Implement `xce_replconfig_reply_decode_raw()` + +Standby uses this to decode the primary's reply from the raw SSL socket. + +```c +/* + * xce_replconfig_reply_decode_raw: read and decode ReplConfigReply from SSL. + * + * Reads exactly one capnp message, decodes into: + * out_up_to_date — set to 1 if tag matched (no bytes follow) + * out_tag — primary's version tag + * out_sha256 — heap-alloc'd 32 bytes (caller frees); NULL if up_to_date + * out_bytes — heap-alloc'd config blob (caller frees); NULL if up_to_date + * out_len — byte length of config blob + * + * Returns 0 on success, negative errno on failure. + */ +int xce_replconfig_reply_decode_raw(SSL *ssl, + int *out_up_to_date, + uint64_t *out_tag, + uint8_t **out_sha256, + uint8_t **out_bytes, + size_t *out_len); +``` + +### A.5 — Tests + +Add to `src/controller/testsuite/test_config_repl.c`: + +- `test_replconfig_reply_raw_roundtrip` — encode reply with 32-byte sha256 and N-byte blob, + write to a `socketpair`, decode back, verify sha256 and bytes match byte-for-byte +- `test_replconfig_req_raw_roundtrip` — encode request with version_tag, decode back, verify +- `test_data_extract_raw_null` — NULL data → `-ENODATA` +- `test_data_extract_raw_bytes` — encode Data field, extract, verify content + +### A.6 — Commit message + +``` +feat(config-repl): add raw capnp shims for OP_replConfig wire protocol + +xce_replconfig_reply_raw(), xce_replconfig_req_raw(), +xce_replconfig_reply_decode_raw(), xce_data_extract_raw() — +same pattern as existing xce_replconfigpush_req_raw(). Unblocks +do_one_config_fetch() and handle_config_request() wiring. +``` + +--- + +## Task B — `config_repl.c`: Wire `handle_config_request()` reply bytes + +**File:** `src/controller/config_repl.c` +**Depends on:** Task A + +Replace the NULL-bytes stub in `handle_config_request()` (lines 650–655). + +**Before (stub):** +```c +rmsg->sha256 = NULL; +rmsg->bytes = NULL; +xce_reply(ep, c->myhost, &reply); +free(bytes); +free(rmsg); +``` + +**After:** +Call `xce_replconfig_reply_raw()` directly instead of `xce_reply()`. This sidesteps the +`xce_data_t` path entirely — the reply is serialised with raw bytes inline. + +**Steps:** +- [ ] Replace the `xce_reply()` path in `handle_config_request()` with + `xce_replconfig_reply_raw(ep, c->myhost, req->seq, my_tag, recomputed, 32, bytes, blen)` +- [ ] Remove the `rmsg` calloc / free (no longer needed) +- [ ] Keep the `up_to_date = true` fast path (tag match) using `xce_reply()` — only + the data-carrying path needs the raw shim +- [ ] Add test: `test_handle_config_request_sends_bytes` — mock a request with mismatched tag, + verify the reply written to a socketpair contains non-NULL sha256 and bytes + +**Commit:** +``` +fix(config-repl): wire handle_config_request() to send real bytes via raw shim +``` + +--- + +## Task C — `config_repl.c`: Wire `do_one_config_fetch()` full SSL path + +**File:** `src/controller/config_repl.c` +**Depends on:** Task A, Task B (primary must reply before standby can fetch) + +Replace the `-ENOSYS` stub in `do_one_config_fetch()` (lines 674–683). + +**Implementation outline:** + +```c +static int do_one_config_fetch(cluster_t *c, const char *host, uint32_t port, + uint64_t our_tag) +{ + // 1. Open raw SSL connection (reuse probe_thread pattern from ha.c) + // socket() + fcntl(O_NONBLOCK) + connect() → poll() → SSL_connect() + // Use c->ha.ssl_ctx (stashed in Phase 1) + + // 2. Send OP_replConfig request + // xce_replconfig_req_raw(ssl, our_tag) + + // 3. Decode reply + // xce_replconfig_reply_decode_raw(ssl, &up_to_date, &tag, &sha256, &bytes, &len) + + // 4. If up_to_date: return 0 (tag matched, nothing to save) + + // 5. Verify SHA-256: SHA256(bytes, len, computed); memcmp(computed, sha256, 32) + // On mismatch: free + return -EBADMSG (retry) + + // 6. Save to disk + // conf_save_config(c->myconf, bytes, len, sha256) + + // 7. Update myconf fields + // config_version_tag, config_sha256, cluster_config (re-decode), config_generation + + // 8. Log success, free buffers, close SSL, return 0 +} +``` + +**SSL connection setup** — copy the non-blocking connect pattern from `probe_thread()` in +`ha.c`. Key constraints: +- Check `atomic_load(&election_done)` between poll iterations (same as probe_thread) +- Use `c->ha.ssl_ctx` — same `SSL_CTX` as election probes +- On SSL errors: treat as fetch failure (return -errno), caller retries + +**Steps:** +- [ ] Extract a `static int connect_ssl_raw(SSL_CTX*, const char*, uint32_t, SSL**)` helper + (reusable by future heartbeat listener implementation) +- [ ] Implement full `do_one_config_fetch()` body per the outline above +- [ ] Wire `load_cluster_config_from_file()` call to re-decode the saved config into + `myconf->cluster_config` after saving (reuse `load_primary_config()` path) +- [ ] Test: `test_do_one_config_fetch_integration` — requires two processes or socketpair mock; + use `XCE_TESTING` guard to inject a mock reply + +**Commit:** +``` +feat(config-repl): wire do_one_config_fetch() — full SSL roundtrip with SHA-256 verify +``` + +--- + +## Task D — `config_repl.c`: Wire `handle_upgrade_config()` to `do_upgrade_config_with_bytes()` + +**File:** `src/controller/config_repl.c` +**Depends on:** Task A (`xce_data_extract_raw()`) + +Replace the `-ENOSYS` stub in `handle_upgrade_config()` (lines 1032–1049). + +**Current (gated stub):** +```c +#if CONF_ENABLE_UPGRADE_CONFIG +int handle_upgrade_config(cluster_t *c, endpoint_t *ep, xce_request_t *req) +{ + send_upgrade_error(ep, req, c, "OP_upgradeConfig byte extraction requires Task 11..."); + return -ENOSYS; +} +``` + +**After:** +```c +#if CONF_ENABLE_UPGRADE_CONFIG +int handle_upgrade_config(cluster_t *c, endpoint_t *ep, xce_request_t *req) +{ + /* Extract raw bytes from xce_data_t fields using Task A shim */ + uint8_t *in_bytes = NULL; size_t in_len = 0; + uint8_t *in_sha = NULL; size_t sha_len = 0; + + int rc = xce_data_extract_raw(req->body.upgradeconfigrequestmsg->bytes, + &in_bytes, &in_len); + if (rc != 0) { send_upgrade_error(..., "bytes field missing"); return rc; } + + rc = xce_data_extract_raw(req->body.upgradeconfigrequestmsg->sha256, + &in_sha, &sha_len); + if (rc != 0 || sha_len != 32) { + free(in_bytes); + send_upgrade_error(..., "sha256 field missing or wrong length"); + return -EINVAL; + } + + rc = do_upgrade_config_with_bytes(c, ep, req, in_bytes, in_len, in_sha); + free(in_bytes); + free(in_sha); + return rc; +} +``` + +**Steps:** +- [ ] Replace `handle_upgrade_config()` body with `xce_data_extract_raw()` calls + delegate + to `do_upgrade_config_with_bytes()` +- [ ] Remove the `Task 11 dependency` comments from `do_upgrade_config_with_bytes()` +- [ ] Test: existing `test_upgrade_config_sha256_mismatch` and `test_upgrade_config_invalid_capnp` + in `test_config_repl.c` should now exercise real code paths instead of the ENOSYS stub + +**Commit:** +``` +feat(config-repl): wire handle_upgrade_config() via xce_data_extract_raw() shim +``` + +--- + +## Task E — `config_repl.c`: Fix `conf_save_config()` path bug + +**File:** `src/controller/config_repl.c` +**Independent** — no dependencies, fix this first. + +**Root cause:** `conf_save_config()` calls `make_conf_dir(myconf->workdir, ...)` → produces +`workdir/conf`. But `load_primary_config()` reads from `workdir/name/conf`. Files would be +written to the wrong location after a config upgrade restart. + +**Fix:** + +```c +int conf_save_config(myconf_t *myconf, const uint8_t *bytes, size_t len, + const uint8_t sha256[32]) +{ + /* effective_dir = workdir/name — must match load_primary_config() path */ + char effective_dir[4096]; + snprintf(effective_dir, sizeof(effective_dir), "%s/%s", + myconf->workdir, myconf->name); + + char conf_dir[4096]; + make_conf_dir(effective_dir, conf_dir, sizeof(conf_dir)); + // ... rest unchanged, replace all make_conf_dir(myconf->workdir, ...) with conf_dir +``` + +Also fix `conf_trim_old_versions()` call at the bottom of `conf_save_config()`: +```c + // Before: conf_trim_old_versions(myconf->workdir, myconf->conf_keep_n); + // After: + conf_trim_old_versions(effective_dir, myconf->conf_keep_n); +``` + +And fix `conffile` asprintf path at the end of `conf_save_config()`: +```c + // Before: asprintf(&myconf->conffile, "%s/cluster.bin.%s", conf_dir, hex_tag) + // (conf_dir was wrong — missing /name) + // After: conf_dir now correctly is effective_dir/conf, so this is already right + // if conf_dir is built from effective_dir above +``` + +**Steps:** +- [ ] Add `effective_dir` construction at top of `conf_save_config()` +- [ ] Replace all `myconf->workdir` path uses in the function with `effective_dir` +- [ ] Fix `conf_trim_old_versions()` call to pass `effective_dir` not `myconf->workdir` +- [ ] Add test: `test_conf_save_config_path_consistency` — call `conf_save_config()` with + a `myconf` where `workdir="/tmp/w"` and `name="ctrl01"`, verify the written file is at + `/tmp/w/ctrl01/conf/cluster.bin.*` not `/tmp/w/conf/cluster.bin.*` +- [ ] Update §3.2 of `config-replication-design.md`: add one sentence defining + `effective_dir = ${workdir}/${name}` + +**Commit:** +``` +fix(config-repl): conf_save_config() must use workdir/name not workdir + +load_primary_config() reads from workdir/name/conf — conf_save_config() +was writing to workdir/conf. Latent bug: would have corrupted upgrade +restart cycle once Task 11 wired the upgrade path. +``` + +--- + +## Task F — `config_repl.c`: Implement `wait_all_with_timeout()` + wire push timeout + +**File:** `src/controller/config_repl.c` +**Independent** — no dependencies. + +**Design spec (§6.3):** single aggregate 500ms timeout for all standby push threads. + +**Implementation** — use a deadline + polling loop with the atomic stop flag pattern from +`probe_thread()`: + +```c +/* wait_all_with_timeout: join all push threads within a CLOCK_MONOTONIC deadline. + * + * Sets atomic_int *done to 1 when the deadline expires, signalling threads + * to exit their push_thread loop. Then joins all threads (they exit quickly + * once *done is set). + * + * Returns the number of threads that had acked before the deadline. + */ +static int wait_all_with_timeout(pthread_t *threads, push_ctx_t *ctxs, + int n, uint32_t timeout_ms, atomic_int *done) +{ + struct timespec deadline; + clock_gettime(CLOCK_MONOTONIC, &deadline); + deadline.tv_sec += timeout_ms / 1000; + deadline.tv_nsec += (timeout_ms % 1000) * 1000000L; + if (deadline.tv_nsec >= 1000000000L) { + deadline.tv_sec++; + deadline.tv_nsec -= 1000000000L; + } + + /* Poll until deadline or all threads done */ + while (1) { + struct timespec now; + clock_gettime(CLOCK_MONOTONIC, &now); + if (now.tv_sec > deadline.tv_sec || + (now.tv_sec == deadline.tv_sec && now.tv_nsec >= deadline.tv_nsec)) + break; + + /* Check if all started threads have finished */ + int pending = 0; + for (int i = 0; i < n; i++) { + if (ctxs[i].started && !ctxs[i].acked && !ctxs[i].done) + pending++; + } + if (pending == 0) break; + + usleep(5000); /* 5ms poll — low overhead, tight enough for 500ms window */ + } + + atomic_store(done, 1); /* signal threads to exit their push_thread loop */ + + int acked = 0; + for (int i = 0; i < n; i++) { + if (ctxs[i].started) { + pthread_join(threads[i], NULL); + if (ctxs[i].acked) acked++; + } + } + return acked; +} +``` + +**Wire into `push_config_to_standbys()`:** +- [ ] Add `atomic_int push_done` initialized to 0 +- [ ] Add `int done` flag to `push_ctx_t` (set by `push_thread` when it completes) +- [ ] Update `push_thread()` to set `ctx->done = 1` on exit (success or failure) +- [ ] Replace the blocking `pthread_join` loop with `wait_all_with_timeout()` +- [ ] Pass `CONF_PUSH_TIMEOUT_MS` as the timeout + +**Test:** +- [ ] `test_push_config_timeout` — launch N push threads that sleep 2s each, call + `wait_all_with_timeout()` with 100ms timeout, verify it returns within ~200ms + (not 2s × N), and `acked` is 0 +- [ ] `test_push_config_all_ack` — all threads ACK immediately, verify timeout is not hit + +**Update design doc §6.3:** replace the `wait_all_with_timeout()` pseudocode stub with the +actual implementation signature and add a note explaining the 5ms poll interval. + +**Commit:** +``` +feat(config-repl): implement wait_all_with_timeout() for push fan-out + +Replaces blocking pthread_join() per thread with a CLOCK_MONOTONIC +deadline + 5ms poll loop. CONF_PUSH_TIMEOUT_MS (500ms) now enforced. +Matches design §6.3 aggregate timeout guarantee. +``` + +--- + +## Execution Order (SDD Waves) + +### Wave 1 — Parallel (no dependencies between tasks) + +Launch three subagents simultaneously: + +| Subagent | Task | ETA | +|---|---|---| +| Subagent-1 | Task A — raw shims in `request.c` | ~2h | +| Subagent-2 | Task E — path bug fix + design update | ~30m | +| Subagent-3 | Task F — push timeout implementation | ~45m | + +### Wave 2 — After Task A completes (parallel) + +| Subagent | Task | Prerequisite | +|---|---|---| +| Subagent-4 | Task B — wire `handle_config_request()` | Task A done | +| Subagent-5 | Task C — wire `do_one_config_fetch()` | Task A done | +| Subagent-6 | Task D — wire `handle_upgrade_config()` | Task A done | + +### Wave 3 — Final integration + +- Run full test suite: `ninja -C build test` +- Verify `impl-status.md` table entries updated +- Update design doc status header: `Status: Phase 1 wire protocol complete; Phase 2 gated` + +--- + +## Files Changed Summary + +| File | Tasks | Change | +|---|---|---| +| `src/lib/request.c` | A | +4 new shim functions (~120 LOC) | +| `src/include/request.h` | A | +4 declarations | +| `src/controller/config_repl.c` | B,C,D,E,F | ~150 LOC changes | +| `src/controller/testsuite/test_config_repl.c` | A,B,C,D,E,F | +~10 tests | +| `src/docs/config-replication-design.md` | E,F | §3.2 effective_dir definition; §6.3 timeout impl | +| `src/docs/config-replication-impl-status.md` | all | Update task status rows | + +**Total estimated net-new LOC:** ~280 (request.c shims ~120, config_repl.c wiring ~100, +tests ~60). + +--- + +## Definition of Done + +- [ ] `ninja -C build test` passes with 0 failures +- [ ] `do_one_config_fetch()` no longer has `-ENOSYS` stub comment +- [ ] `handle_config_request()` sends non-NULL sha256 and bytes in reply +- [ ] `handle_upgrade_config()` calls `do_upgrade_config_with_bytes()` (not ENOSYS) +- [ ] `conf_save_config()` path verified by new test against `workdir/name/conf` +- [ ] `push_config_to_standbys()` returns within `CONF_PUSH_TIMEOUT_MS` even if standbys hang +- [ ] Design doc §3.2 defines `effective_dir`, §6.3 timeout mechanism documented +- [ ] `impl-status.md` header updated to reflect Phase 1 wire protocol complete diff --git a/src/docs/config-replication-gap-analysis.md b/src/docs/config-replication-gap-analysis.md new file mode 100644 index 00000000..66b8997c --- /dev/null +++ b/src/docs/config-replication-gap-analysis.md @@ -0,0 +1,252 @@ +# Config Replication — Gap Analysis: Design vs Implementation + +> **Date**: 2026-05-20 +> **Design doc**: `config-replication-design.md` +> **Implementation status**: `config-replication-impl-status.md` +> **Branch**: `persistent_ha` +> **Scope**: Validates what is implemented vs what the design specifies. + +--- + +## Executive Summary + +The infrastructure is largely in place: schema, struct fields, storage functions, election probing, +and the Phase 1 opcode dispatch all exist. However **the core wire protocol is not functional**: +both the standby fetch (`do_one_config_fetch`) and the primary's reply payload +(`handle_config_request`) are explicitly stubbed pending "Task 11" — Cap'n Proto `Data`-typed +field extraction shims. Additionally, a path inconsistency bug affects `conf_save_config()`, and +the push timeout design is not implemented. + +**Phase 1 (pull-only replication)**: Infrastructure exists; wire protocol is a no-op stub. +**Phase 2 (remote push)**: Gate compiles but returns `-ENOSYS`; functional logic exists in `do_upgrade_config_with_bytes()` but is not wired. + +--- + +## Gap 1 — CRITICAL: `do_one_config_fetch()` is a no-op stub + +**Design (§5.5):** The standby connects to the primary over a standalone SSL socket (before +Phase 2 / iomgr), sends `OP_replConfig` with its saved version tag, receives `ReplConfigReply` +with the full config bytes + SHA-256, verifies integrity, writes to `conf/`, and updates +`conf/current`. + +**Code (`config_repl.c:674–683`):** +```c +static int do_one_config_fetch(cluster_t *c, const char *host, uint32_t port, + uint64_t our_tag) +{ + (void)c; (void)host; (void)port; (void)our_tag; + /* Task 11: wire full capnp encode/decode over standalone SSL socket */ + return -ENOSYS; +} +``` + +**Impact:** All 20 retry attempts in `fetch_config_from_primary()` fail immediately with +`-ENOSYS` (~47 seconds of log spam), then fall back to the locally saved config. A brand-new +standby with no saved config will FATAL with "no saved config" error. + +**Root cause:** Task 11 (Cap'n Proto `Data`-type field extraction shims for the standalone SSL path) is not yet done. + +--- + +## Gap 2 — CRITICAL: `handle_config_request()` sends NULL bytes and SHA-256 + +**Design (§5.4):** When the standby's tag differs from the primary's, the primary sends back +`ReplConfigReply { up_to_date=false, versionTag=..., sha256=<32 bytes>, bytes=<raw blob> }`. + +**Code (`config_repl.c:650–655`):** +```c +/* sha256 and bytes are xce_data_t* — raw-byte encoding is wired in Task 11. + * For now set to NULL so encode_Data_ptr emits capn_null pointers. */ +rmsg->sha256 = NULL; +rmsg->bytes = NULL; +``` + +**Impact:** Even if the standby's SSL connection were wired, the primary would reply with an +incomplete message. The standby would receive a tag but no config payload. + +--- + +## Gap 3 — HIGH: `handle_upgrade_config()` always returns `-ENOSYS` + +**Design (§6.4):** When `CONF_ENABLE_UPGRADE_CONFIG=1`, `handle_upgrade_config()` validates +SHA-256, decodes Cap'n Proto, increments generation, saves to disk, fans out to standbys, then +signals restart. The full logic exists in `do_upgrade_config_with_bytes()`. + +**Code (`config_repl.c:1031–1048`):** +```c +#if CONF_ENABLE_UPGRADE_CONFIG +int handle_upgrade_config(cluster_t *c, endpoint_t *ep, xce_request_t *req) +{ + /* Task 11 dependency: ... until Task 11 lands this handler rejects all + * upgrade requests with -ENOSYS even when CONF_ENABLE_UPGRADE_CONFIG=1. */ + send_upgrade_error(ep, req, c, + "OP_upgradeConfig byte extraction requires Task 11 ..."); + return -ENOSYS; +} +``` + +**Impact:** Phase 2 is completely non-functional even when the compile-time gate is enabled. +The `do_upgrade_config_with_bytes()` helper containing the real logic exists but is not called. + +--- + +## Gap 4 — MEDIUM (latent bug): `conf_save_config()` uses wrong directory path + +**Design (§3.1):** Config files live at `${workdir}/${name}/conf/cluster.bin.<sha>`. + +**Code mismatch:** + +| Function | Effective path | Correct? | +|---|---|---| +| `load_primary_config()` | builds `conf_dir = workdir/name`, reads `workdir/name/conf/current` | ✓ | +| `fetch_config_from_primary()` | builds `conf_dir = workdir/name`, calls helpers correctly | ✓ | +| `conf_save_config()` | `make_conf_dir(myconf->workdir, ...)` → `workdir/conf` (no `name`) | ✗ | +| `conf_trim_old_versions()` (called from save) | same `myconf->workdir` → `workdir/conf` | ✗ | + +**`conf_save_config()` (line 235):** +```c +make_conf_dir(myconf->workdir, conf_dir, sizeof(conf_dir)); +// → conf_dir = "/var/xce/conf" (missing "/ctrl01") +``` + +**`load_primary_config()` (line 75–76):** +```c +snprintf(conf_dir, sizeof(conf_dir), "%s/%s", myconf->workdir, myconf->name); +// → conf_dir = "/var/xce/ctrl01" (correct) +``` + +**Status:** Currently latent because `handle_upgrade_config()` returns `-ENOSYS` before calling +`do_upgrade_config_with_bytes()` → `conf_save_config()`. Will trigger as a real bug when Task 11 +wires the upgrade path. + +**Fix:** Change `conf_save_config()` to accept an `effective_dir` parameter (or construct +`workdir/name` internally from `myconf->workdir` + `myconf->name`), consistent with how +`load_primary_config()` and `fetch_config_from_primary()` build the path. + +--- + +## Gap 5 — MEDIUM: No aggregate timeout in `push_config_to_standbys()` + +**Design (§6.3):** "The primary waits at most `CONF_PUSH_TIMEOUT_MS` (default 500 ms) **total** +for all ACKs. This bounds the pre-restart delay to a single timeout period regardless of standby +count." + +**Code (`config_repl.c:840–858`):** +```c +for (int i = 0; i < n; i++) { + if (!ctxs[i].started) continue; + pthread_join(threads[i], NULL); // ← blocks indefinitely per thread + if (ctxs[i].acked) acked++; +} +``` + +`CONF_PUSH_TIMEOUT_MS` is defined in the header (line 26) but is **never used** in this function. +The design calls for `wait_all_with_timeout(push_threads, n_standbys, CONF_PUSH_TIMEOUT_MS)` which +is not implemented. + +**Impact:** If any standby hangs during push, the primary could block indefinitely before +restarting. The `CONF_PUSH_TIMEOUT_MS` safety bound is missing. + +--- + +## Gap 6 — MEDIUM: `start_heartbeat_listener()` is a stub + +**Design (§9.0):** A dedicated heartbeat listener thread starts immediately after +`am_i_primary()` determines the node is primary. It allows lower-priority candidates to detect +the primary during Phase 2 initialization (before iomgr is up). + +**Code (`ha.c:356–365`):** +```c +void start_heartbeat_listener(cluster_t *c) +{ + (void)c; + /* Heartbeat listener stub — full implementation in cluster.c Task 7 */ +} +void stop_heartbeat_listener(cluster_t *c) { (void)c; } +``` + +**Impact:** `impl-status.md` notes: "simultaneous-boot symmetry relies on priority step only." +This means the §9.0 guarantee — that a lower-priority self-declared primary can receive a +heartbeat from the higher-priority true primary during Phase 2 init — is not enforced. +In practice, the priority-based step delay (500ms gap) makes simultaneous-boot split-brain +unlikely, but not impossible. + +--- + +## Gap 7 — LOW: WAL replay routing helpers (`resolve_queue_or_lost_and_found()` etc.) not implemented + +**Design (§6.5):** Three helpers for routing orphaned WAL records: +- `resolve_queue_or_lost_and_found(cluster_t *c, uint32_t queue_id)` +- `resolve_host_or_lost_and_found()` +- `clear_resource_and_pend()` + +These are declared in §12 (Files Changed → `config_repl.c`) and are supposed to be called from +`apply_wal_entry()` in `persist.c`. + +**Code:** Not found in `config_repl.c` or `persist.c`. `create_lost_and_found()` exists and +creates the synthetic queue/host, but the WAL apply path does not use them — unknown queue/host +IDs during `persist_recover()` are not routed to `lost_and_found`. + +**Impact:** Config upgrade (removing a queue/host) + WAL replay would hit an unknown-ID path +that is not yet gracefully handled. Currently not a production risk since `OP_upgradeConfig` is +gated, but a prerequisite for safe queue/host removal. + +--- + +## Gap 8 — LOW: Certificate expiry warning (§11.1) not implemented + +**Design (§11.1):** At startup (after `create_ssl_context`), check `X509_get0_notAfter()` and +WARN if the certificate expires within `CERT_WARN_DAYS` (30 days). + +**Code:** `CERT_WARN_DAYS 30` is defined in `config_repl.h:30` but no code reads it or calls +`X509_get0_notAfter()`. + +--- + +## Gap 9 — LOW: `sd_notify()` not implemented + +**Design (§11.1):** The systemd unit file specifies `Type=notify`, which requires the controller +to call `sd_notify(0, "READY=1")` after Phase 2 completes. + +**Code:** No `sd_notify()` call found in any controller source file. + +--- + +## Summary Table + +| # | Gap | Severity | Currently Reachable? | Blocks | +|---|---|---|---|---| +| 1 | `do_one_config_fetch()` stub | **CRITICAL** | Yes (standby startup) | Standby config fetch | +| 2 | `handle_config_request()` null payload | **CRITICAL** | Yes (any replConfig request) | Primary config reply | +| 3 | `handle_upgrade_config()` -ENOSYS | **HIGH** | Yes (when gate=1) | Phase 2 entirely | +| 4 | `conf_save_config()` wrong path | **MEDIUM** | No (latent; blocked by Gap 3) | Config upgrade restart cycle | +| 5 | No push timeout | **MEDIUM** | No (blocked by Gap 3) | Upgrade restart latency | +| 6 | Heartbeat listener stub | **MEDIUM** | Yes | Simultaneous-boot robustness | +| 7 | WAL routing helpers missing | **LOW** | No (blocked by Gate=0) | Safe config upgrade | +| 8 | Cert expiry warning missing | **LOW** | No (operational only) | Observability | +| 9 | sd_notify() missing | **LOW** | No (operational only) | Systemd integration | + +--- + +## Root Cause: Task 11 Dependency + +Gaps 1, 2, and 3 share a common root cause: **"Task 11"** — Cap'n Proto `Data`-typed field +extraction shims in the typemap layer. The design's `ReplConfigReply.sha256` and +`ReplConfigReply.bytes` (and the corresponding request fields for `OP_upgradeConfig`) are typed as +`xce_data_t*` (a discriminated cluster-data union) in the current typemap. Extracting raw bytes +from them requires shims that don't yet exist. + +Completing Task 11 would unblock: +- `handle_config_request()` → send actual bytes +- `do_one_config_fetch()` → receive and decode config +- `handle_upgrade_config()` → extract input bytes from request + +--- + +## Recommended Fix Order + +1. **Task 11 (typemap Data shims)** — unblocks Gaps 1, 2, 3 simultaneously +2. **Gap 4 fix** (`conf_save_config()` path) — fix before Task 11 ships to avoid silent bug +3. **Gap 5 fix** (push timeout) — add `wait_all_with_timeout()` helper alongside Task 11 +4. **Gap 6** (heartbeat listener) — implement to complete §9.0 simultaneous-boot guarantee +5. **Gap 7** (WAL routing helpers) — required before enabling `CONF_ENABLE_UPGRADE_CONFIG=1`