Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions .claude/commands/forge-loop.md
Original file line number Diff line number Diff line change
Expand Up @@ -188,15 +188,23 @@ Document in `PIPELINE-ISSUES.md` and continue to next step.
bash scripts/forge-helper.sh phase-commit <phase> --package <pkg> "<title>"
```

### 2h. Archive
### 2h. Verify

Run `/opsx-verify` against the change to confirm implementation matches artifacts (completeness, correctness, coherence). Fix any CRITICAL issues before proceeding.

### 2i. Sync specs

Run `/opsx-sync` against the change to merge delta specs into main specs. If no delta specs exist, this is a no-op.

### 2j. Archive

```bash
openspec archive "<change_name>" -y
openspec archive "<change_name>"
```

Warn but continue if archive fails.
The archive command includes its own sync assessment at Step 4 — review and confirm. Warn but continue if archive fails.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Do not continue loop when archive fails.

Line 205’s “warn but continue” risks marking a phase as completed in handoff/status while the change is still unarchived. This can create workflow drift and duplicate future work.

Suggested fix
-The archive command includes its own sync assessment at Step 4 — review and confirm. Warn but continue if archive fails.
+The archive command includes its own sync assessment at Step 4 — review and confirm.
+If archive fails, stop the loop for this phase, document the blocker in PIPELINE-ISSUES.md,
+and resume only after the archive issue is resolved.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
The archive command includes its own sync assessment at Step 4 — review and confirm. Warn but continue if archive fails.
The archive command includes its own sync assessment at Step 4 — review and confirm.
If archive fails, stop the loop for this phase, document the blocker in PIPELINE-ISSUES.md,
and resume only after the archive issue is resolved.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.claude/commands/forge-loop.md at line 205, The archive step currently logs
a warning and continues on failure; change the flow so the loop aborts when the
archive command fails: locate the Step 4 "archive" invocation in the forge-loop
logic (the "archive command" call in Step 4) and replace the "warn but continue"
behavior with a strict failure path that propagates the archive error (return
non-zero / throw / exit the loop) and prevents marking the phase/handoff/status
as completed; ensure any status updates or handoff/state transitions that occur
after the archive call are gated on a successful archive result so failures stop
the loop and surface the error.


### 2i. Update handoff
### 2k. Update handoff

```bash
bash scripts/forge-helper.sh update-handoff <phase> "<title>" --package <pkg> <done> <total>
Expand All @@ -212,7 +220,7 @@ Append new key decisions to `HANDOFF.md`:
- **Impact**: How it affects future phases
```

### 2j. Show summary and loop
### 2l. Show summary and loop

```bash
bash scripts/forge-helper.sh status --package <pkg>
Expand Down
101 changes: 101 additions & 0 deletions HANDOFF.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
# Session Handoff

Persistent context bridge for autonomous ROADMAP execution across sessions. Each session reads this file at startup and updates it after completing a phase.

---

## Current State

| Field | Value |
| ------------------------ | --------------------------- |
| **Active Wave** | Wave 1: Core Infrastructure |
| **Active Package** | telemetry |
| **Last Completed Phase** | telemetry phase 1 |
| **Last Session** | 2026-04-04 |
| **ROADMAP Status** | Phase 1 complete |

---

## Completed Work

| Wave | Package | Phase | Title |
| ------ | --------- | ------- | ------------------------------- |
| Wave 1 | telemetry | Phase 0 | Foundation (types, scaffolding) |
| Wave 1 | telemetry | Phase 1 | Storage Layer (Memory + File) |

---

## Key Decisions (ADR Summary)

- **Storage backends are pluggable**: `StorageBackend` interface with `append()`, `query()`, `count()`. Consumers choose backend at `createTelemetry()` time (Phase 4).
- **JSONL file format**: Default zero-config backend uses newline-delimited JSON. Append-only, no external dependencies.
- **`matchesFilter` is shared**: Extracted to `src/storage/matchesFilter.ts` to avoid duplication across backends.
- **Lazy directory creation**: `FileStorageBackend` creates directories on first `append()`, not at construction time.

---

## Project Patterns

Established patterns -- follow these in all packages:

- **Module format**: ESM (`"type": "module"` in all packages)
- **Imports**: Use `.js` extension in relative imports
- **Types**: Co-locate in `types.ts` per module, export from `index.ts`
- **Tests**: Place in `tests/` directory at package root, named `*.test.ts`
- **Quality gates**: `npx nx affected -t build,test,lint`
- **Shared utilities**: Extract duplicated logic to standalone files named after the export (`filename-match-export` rule)
- **Empty catch pattern**: Use `flatMap` with `try/catch` returning `[]` for skip-on-error — no logging needed

---

## Known Issues & Gotchas

- `count()` in both backends loads all events into memory before counting. Acceptable for Phase 1; optimize in a future phase if needed.

---

## Lessons Learned

- The `llm-core/filename-match-export` lint rule requires files with a single export to be named after that export.
- The comments hook flags all new comments/docstrings — only add when absolutely necessary.
- `afterEach` must be explicitly imported from vitest (not relied upon as a global).

---

## Next Phase Context

### Target

Wave 1: @open-forge/telemetry Phase 2 (Telemetry Core)

### Goal

Implement event emission and querying — the `Telemetry` interface: `emit()`, `query()`, `getPipelineSummary()`.

### Critical Context

- Storage backends are complete and tested (33 tests passing)
- Types already defined in `packages/telemetry/src/types.ts` (including `Telemetry`, `PipelineSummary`)
- Phase 2 deliverable: `src/telemetry.ts` + `tests/telemetry.test.ts`
- Sequential dependency: 2.1 (`emit`) → 2.2 (`query`) → 2.3 (`getPipelineSummary`) → 2.4 (tests)
- Requirements covered: F-001, F-002, F-003, F-004, F-005, F-006, F-007, F-013

---

## Session Resumption Instructions

1. Read this file: cat HANDOFF.md
2. Check root status: bash scripts/forge-helper.sh status
3. Check package status: bash scripts/forge-helper.sh status --package telemetry
4. Resume work on the next phase
5. After completion: update this file

---

## Changelog

| Date | Wave | Package | Phase | Notes |
| ---------- | ------ | --------- | ----- | ---------------------------------------- |
| 2026-04-03 | -- | -- | -- | Created HANDOFF.md |
| 2026-04-03 | Wave 1 | telemetry | 1 | Storage Layer -- 4/4 tasks |
| 2026-04-04 | Wave 1 | telemetry | 1 | PR review fixes, matchesFilter extracted |
22 changes: 17 additions & 5 deletions docs/LOOP.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ The agent works autonomously through pending phases. Interrupt at any time; rest
│ 7. Implement tasks, mark each done │
│ 8. forge-helper.sh check --package <pkg> │
│ 9. forge-helper.sh phase-commit <N> --package <pkg> │
│ 10. openspec archive "<pkg>-phase-N" -y │
│ 11. Update HANDOFF.md │
│ 10. /opsx-verify (confirm implementation matches specs) │
│ 11. /opsx-sync (merge delta specs to main specs) │
│ 12. openspec archive "<pkg>-phase-N" │
│ 13. Update HANDOFF.md │
│ │
│ Loop to next phase │
└──────────────────────────────────────────────────────────┘
Expand Down Expand Up @@ -140,13 +142,23 @@ If checks fail:
bash scripts/forge-helper.sh phase-commit N --package <pkg> "<title>"
```

### 8. Archive
### 8. Verify

Run `/opsx-verify` against the change to confirm implementation matches artifacts (completeness, correctness, coherence). Fix any CRITICAL issues before proceeding.

### 9. Sync specs

Run `/opsx-sync` against the change to merge delta specs into main specs. If no delta specs exist, this is a no-op.

### 10. Archive

```bash
openspec archive "$CHANGE" -y
openspec archive "$CHANGE"
```

### 9. Update handoff
The archive command includes its own sync assessment — review and confirm.

### 11. Update handoff

```bash
bash scripts/forge-helper.sh update-handoff N "<title>" --package <pkg> <done> <total>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-04-04
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# telemetry-phase-1

telemetry Phase 1: Storage Layer
59 changes: 59 additions & 0 deletions openspec/changes/archive/2026-04-04-telemetry-phase-1/design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
## Context

The `@open-forge/telemetry` package currently exports only TypeScript types (Phase 0). The storage layer is the first real implementation — it provides the persistence mechanism that `emit()` and `query()` (Phase 2) will use. The existing `PipelineEvent` and `EventFilter` types in `src/types.ts` define the data model.

Constraint: Zero required runtime dependencies for core functionality (per package ROADMAP).

## Goals / Non-Goals

**Goals:**

- Define a `StorageBackend` interface that decouples telemetry logic from persistence
- Ship two concrete backends: in-memory (for tests/lightweight) and file-based JSONL (for production default)
- All backends satisfy the same contract and are interchangeable
- File backend works zero-config with only a path

**Non-Goals:**

- SQLite or database backends (deferred per ROADMAP)
- Concurrent write safety across processes (single-writer assumption for v1)
- Streaming/pagination for large result sets (can be added later)
- Hot-swapping backends at runtime

## Decisions

### 1. StorageBackend as a standalone interface (not extending Telemetry)

**Decision**: `StorageBackend` is a separate interface with `append()`, `query()`, `count()` — it does NOT extend or overlap with the `Telemetry` interface.

**Rationale**: `Telemetry.emit()` will do validation and enrichment before calling `StorageBackend.append()`. Keeping them separate enforces single responsibility. The `Telemetry` implementation (Phase 2) composes a `StorageBackend`.

**Alternative considered**: Having `Telemetry` directly implement storage. Rejected — couples business logic to I/O.

### 2. JSONL for file storage format

**Decision**: One JSON object per line, append-only.

**Rationale**: Simple, human-readable, no corruption from partial writes (each line is independent), streamable, and requires no dependencies. Alternatives like SQLite or binary formats add complexity for marginal gains at this scale.

### 3. File storage creates directories on first write, not on construction

**Decision**: The `FileStorageBackend` constructor stores the path. Directory/file creation happens on first `append()`.

**Rationale**: Avoids side effects during construction. `query()` on a non-existent file returns empty — no error.

### 4. Query filtering uses in-process filtering

**Decision**: Both backends load/scan events and filter in-process using `EventFilter` fields.

**Rationale**: Simplicity. At expected telemetry volumes (hundreds to low thousands of events per pipeline), in-process filtering is fast enough. Indexing can be added in a future phase if needed.

## Risks / Trade-offs

- **[Large file performance]** → File backend reads the entire file for queries. Mitigation: acceptable for v1 volumes; SQLite backend is in the deferred items list.
- **[No concurrent writes]** → Single-writer assumption. Mitigation: pipeline is single-threaded; document the limitation.
- **[JSONL corruption on crash]** → Partial line write possible if process crashes mid-append. Mitigation: each line is independent; partial lines are skipped on read.

## Open Questions

None — scope is well-defined by the ROADMAP and requirements doc.
30 changes: 30 additions & 0 deletions openspec/changes/archive/2026-04-04-telemetry-phase-1/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
## Why

The telemetry package currently exports only type definitions (Phase 0). Every downstream consumer — the pipeline orchestrator, constraint evaluator, and evaluations integration — needs real storage backends that can persist and query pipeline events. Without a storage layer, telemetry cannot function. This is the critical path: Wave 1 in the unified ROADMAP, blocking all subsequent waves.

## What Changes

- New `StorageBackend` interface defining the contract for all storage implementations (`append`, `query`, `count`)
- In-memory storage backend for testing and lightweight/ephemeral use cases
- File-based JSONL storage backend as the default zero-config production backend (append-only, no external dependencies)
- All backends are pluggable — consumers choose at `createTelemetry()` time (Phase 4)

## Capabilities

### New Capabilities

- `storage-interface`: Defines the `StorageBackend` contract — `append()`, `query()`, `count()` — that all storage implementations must satisfy
- `memory-storage`: In-memory `StorageBackend` implementation for testing and lightweight use
- `file-storage`: JSONL append-only file `StorageBackend` implementation as the default zero-config backend

### Modified Capabilities

<!-- No existing capabilities are being modified — this is all new implementation -->

## Impact

- **New files**: `src/storage/StorageBackend.ts`, `src/storage/MemoryStorageBackend.ts`, `src/storage/FileStorageBackend.ts`, `src/storage/matchesFilter.ts`, `tests/storage.test.ts`
- **Modified files**: `src/index.ts` (export storage implementations)
- **APIs**: New `StorageBackend` interface and two concrete implementations
- **Dependencies**: Zero runtime dependencies (Node.js `fs` and `path` for file backend only)
- **Downstream**: Unblocks Phase 2 (Telemetry Core) which needs storage to implement `emit()` and `query()`
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
## ADDED Requirements

### Requirement: File storage implements StorageBackend

The `FileStorageBackend` class SHALL implement the `StorageBackend` interface, persisting events as JSONL (one JSON object per line) in an append-only file.

#### Scenario: Create file storage with path

- **WHEN** a `FileStorageBackend` is instantiated with a file path
- **THEN** it SHALL create the file (and parent directories) if they do not exist

### Requirement: File storage append writes JSONL

The `FileStorageBackend.append()` method SHALL serialize the event as JSON and append it as a single line to the storage file.

#### Scenario: Append writes one line per event

- **WHEN** three events are appended
- **THEN** the file SHALL contain exactly three lines, each being valid JSON

#### Scenario: Append is atomic per event

- **WHEN** `append()` is called
- **THEN** each event SHALL be written as a complete line (no partial writes visible to readers)

### Requirement: File storage query reads and filters JSONL

The `FileStorageBackend.query()` method SHALL read the JSONL file, parse each line, and filter by the provided `EventFilter`.

#### Scenario: Query filters persisted events

- **WHEN** events are appended and then queried with a filter
- **THEN** only matching events SHALL be returned

### Requirement: File storage persists across instances

Events stored by `FileStorageBackend` SHALL be readable by a new instance pointing to the same file.

#### Scenario: Data survives instance recreation

- **WHEN** events are appended, the instance is discarded, and a new instance is created with the same path
- **THEN** the new instance SHALL return the previously stored events via `query()`

### Requirement: File storage count avoids full deserialization

The `FileStorageBackend.count()` method SHALL count matching events efficiently without requiring all events to be held in memory simultaneously.
Comment on lines +44 to +46

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check FileStorageBackend.count() implementation

# Show the count() method implementation
ast-grep --pattern $'count($$$) {
  $$$
}'  packages/telemetry/src/storage/FileStorageBackend.ts

# Show if it calls readAll() which loads everything into memory
rg -A 5 'async count\(' packages/telemetry/src/storage/FileStorageBackend.ts

Repository: pertrai1/open-forge

Length of output: 282


Fix count() to match spec requirement: avoid full deserialization.

The spec requires count() to avoid holding all events in memory simultaneously. The current implementation directly violates this by calling readAll(), which deserializes the entire file into a PipelineEvent[] array before filtering. For large telemetry files, this causes excessive memory usage.

Implement a streaming approach instead—process the file line-by-line, deserializing only to check the filter, without materializing the full event array:

async count(filter: EventFilter): Promise<number> {
  const content = await readFile(this.filePath, 'utf-8').catch(err => {
    if (isNodeError(err) && err.code === 'ENOENT') return '';
    throw err;
  });
  
  let count = 0;
  for (const line of content.split('\n')) {
    if (line.trim() === '') continue;
    try {
      const event = JSON.parse(line) as PipelineEvent;
      if (matchesFilter(event, filter)) count++;
    } catch {
      // Skip malformed lines
    }
  }
  return count;
}

For even better memory efficiency with very large files, consider using Node.js streams with readline to avoid loading the entire file as a string.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@openspec/changes/archive/2026-04-04-telemetry-phase-1/specs/file-storage/spec.md`
around lines 44 - 46, The FileStorageBackend.count() currently calls readAll()
which deserializes the entire file into memory; change it to stream/iterate the
file line-by-line instead so only one event is deserialized at a time: replace
the readAll() usage in FileStorageBackend.count with a streaming/readline or
line-wise file read that skips empty/malformed lines, JSON.parse each line to a
PipelineEvent and apply matchesFilter(event, filter) to increment a counter,
handle ENOENT by treating file as empty and rethrow other errors; ensure no
array of events is built and malformed JSON lines are ignored.


#### Scenario: Count returns correct number

- **WHEN** events are appended and `count()` is called with a matching filter
- **THEN** the correct count SHALL be returned

### Requirement: File storage handles empty or missing file

The `FileStorageBackend` SHALL gracefully handle cases where the file does not yet exist or is empty.

#### Scenario: Query on non-existent file

- **WHEN** `query()` is called before any events are appended (file does not exist)
- **THEN** an empty array SHALL be returned

#### Scenario: Count on empty file

- **WHEN** `count()` is called on an empty file
- **THEN** zero SHALL be returned
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
## ADDED Requirements

### Requirement: In-memory storage implements StorageBackend

The `MemoryStorageBackend` class SHALL implement the `StorageBackend` interface, storing all events in an in-memory array.

#### Scenario: Create in-memory storage

- **WHEN** a `MemoryStorageBackend` is instantiated
- **THEN** it SHALL be ready for use with zero configuration

### Requirement: In-memory storage append is synchronous-fast

The `MemoryStorageBackend.append()` method SHALL resolve immediately after pushing the event to the internal array.

#### Scenario: Append resolves without delay

- **WHEN** `append()` is called
- **THEN** the promise SHALL resolve within 1ms under normal conditions

### Requirement: In-memory storage query filters correctly

The `MemoryStorageBackend.query()` method SHALL filter the internal array by all provided `EventFilter` fields.

#### Scenario: Query with combined filters

- **WHEN** multiple events exist for different pipelines and stages
- **THEN** `query()` with combined filters SHALL return only matching events

### Requirement: In-memory storage is ephemeral

Events stored in `MemoryStorageBackend` SHALL not persist beyond the lifetime of the instance.

#### Scenario: Data lost on new instance

- **WHEN** a `MemoryStorageBackend` is created, events are appended, and a new instance is created
- **THEN** the new instance SHALL have zero events
Loading
Loading