diff --git a/.githooks/post-checkout b/.githooks/post-checkout index 2d4e3533..ebdb0353 100755 --- a/.githooks/post-checkout +++ b/.githooks/post-checkout @@ -14,7 +14,7 @@ else echo "worklog: wl/worklog not found; skipping post-checkout sync" >&2 exit 0 fi -if "$WL" sync >/dev/null 2>&1; then +if "$WL" sync --git-branch refs/worklog/data >/dev/null 2>&1; then : else echo "worklog: sync failed or not initialized; continuing" >&2 diff --git a/.githooks/post-merge b/.githooks/post-merge index 6a6bf745..e1643efc 100755 --- a/.githooks/post-merge +++ b/.githooks/post-merge @@ -1,4 +1,4 @@ #!/bin/sh # worklog:post-pull-hook:v1 # Wrapper that delegates to central Worklog post-pull script (committed hooks). -exec "/tmp/Worklog/.githooks/worklog-post-pull" "$@" +exec "$(dirname "$0")/worklog-post-pull" "$@" diff --git a/.githooks/post-rewrite b/.githooks/post-rewrite index 6a6bf745..e1643efc 100755 --- a/.githooks/post-rewrite +++ b/.githooks/post-rewrite @@ -1,4 +1,4 @@ #!/bin/sh # worklog:post-pull-hook:v1 # Wrapper that delegates to central Worklog post-pull script (committed hooks). -exec "/tmp/Worklog/.githooks/worklog-post-pull" "$@" +exec "$(dirname "$0")/worklog-post-pull" "$@" diff --git a/.githooks/worklog-post-pull b/.githooks/worklog-post-pull index f5bddc72..cadb3cd2 100755 --- a/.githooks/worklog-post-pull +++ b/.githooks/worklog-post-pull @@ -14,7 +14,7 @@ else echo "worklog: wl/worklog not found; skipping post-pull sync" >&2 exit 0 fi -if "$WL" sync >/dev/null 2>&1; then +if "$WL" sync --git-branch refs/worklog/data >/dev/null 2>&1; then : else echo "worklog: sync failed or not initialized; continuing" >&2 diff --git a/CHANGELOG.md b/CHANGELOG.md index ecb771f6..6af3b124 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,26 @@ # Changelog +## v1.0.3 (2026-07-11) +### Features +- Extend doctor upgrade to refresh installed hooks from .githooks (WL-0MRDEM7OO005UB1H) +- Session Health Extension for Pi Footer (WL-0MRDRZ32L00404D0) +- proactively release leases (WL-0MRE6JDT3004OSTF) +- Show time since last streaming chunk in session health footer (WL-0MRERDU1J000W1IS) +- Add model/provider line to TUI status bar footer (WL-0MRF19R71004YJND) +- Show initial prompt preview in footer info line (WL-0MRF6JXMD003KHLQ) +- Restructure session health footer: elapsed time next to state, total session time in center (WL-0MRFJGLI4009GLEE) +- Compact footer status line: shorten skill indicator and truncate work item IDs (WL-0MRFOJFVA003S8IB) +- Show initial prompt preview in footer info line (WL-0MRF6JR7P005TJU9) +### Bug Fixes +- RCA: wl sync pre-push hook destroyed Tableau-Card-Engine repository (WL-0MRCTZZ82000X7TM) +- Fix extractInitialPrompt to handle Pi's array-based content format (WL-0MRFH8W4J007HIY5) +### Other +- Move provider/model display to start of Worklog extension status line (WL-0MRC91VIN0089UXD) +- Fix pre-existing test failures in unrelated test files (WL-0MRFMOU4Q009GUWH) +- Integrate model-display extension into the Worklog extension (WL-0MRC3F30E008XJN3) +- Change audit/review question mark icon from red to grey in selection list (WL-0MRCN2HF5003HSG9) +- Model display extension is not showing the provider (WL-0MRC3NOVZ000P0P5) + ## v1.0.2 (2026-07-08) ### Other - Integrate model-display extension into the Worklog extension (WL-0MRC3F30E008XJN3) diff --git a/docs/branch-protection.md b/docs/branch-protection.md new file mode 100644 index 00000000..dd78d62b --- /dev/null +++ b/docs/branch-protection.md @@ -0,0 +1,53 @@ +# Branch Protection Rules + +> **Operational/GitHub admin task.** These rules must be configured in GitHub repository settings. + +## Background + +On 2026-07-09, an incident (WL-0MRCTZZ82000X7TM) corrupted the `dev` branch of this repository when a pre-push hook executed a destructive `wl sync` that deleted all source files. The commit `b639581` rendered `origin/dev` unusable until a force-push from a feature branch restored it. + +While the hooks have been fixed to use safe patterns, branch protection rules provide an additional safety layer that prevents any single commit (even from a trusted hook) from irreversibly damaging the repository state. + +## Required Rules + +### `main` + +- **Require pull request reviews before merging**: ON +- **Dismiss stale pull request approvals when new commits are pushed**: ON +- **Require status checks to pass before merging**: ON + - All CI checks must pass +- **Require branches to be up to date before merging**: ON +- **Require linear history**: ON +- **Do not allow force pushes to the `main` branch**: ON +- **Do not allow deletions of the `main` branch**: ON + +### `dev` + +- **Require pull request reviews before merging**: ON +- **Dismiss stale pull request approvals when new commits are pushed**: ON +- **Require status checks to pass before merging**: ON + - All CI checks must pass +- **Require branches to be up to date before merging**: ON +- **Require linear history**: ON (if possible) +- **Do not allow force pushes to the `dev` branch**: ON (strongly recommended) +- **Do not allow deletions of the `dev` branch**: ON + +## Rationale + +- **No force pushes**: Prevents accidental or malicious overwriting of branch history. The `dev` branch corruption incident was only recoverable because a clean feature branch existed. Without force-push protection, similar incidents could permanently lose work. +- **PR reviews**: Ensures at least one human (or automated system) approves changes before they reach protected branches. This is especially important for hooks, automation scripts, and configuration changes. +- **Status checks**: Ensures CI passes before merging. This prevents broken code from reaching protected branches. +- **Linear history**: Makes it easier to reason about the commit history and reduces the risk of accidental merges introducing conflicts or regressions. + +## Configuration + +These rules are configured via GitHub's UI: + +1. Go to **Settings > Branches > Branch protection rules** +2. Click **Add rule** or **Edit** for each branch +3. Configure the rules as described above + +## Related + +- Incident: WL-0MRCTZZ82000X7TM (RCA: wl sync pre-push hook destroyed repository) +- Hook fixes: Same work item — `.githooks/pre-push`, `.githooks/post-merge`, `.githooks/post-rewrite`, `.githooks/post-checkout`, `.githooks/worklog-post-pull`, `.git/hooks/pre-push`, `.git/hooks/worklog-post-pull` diff --git a/docs/migrations.md b/docs/migrations.md index 0f6dcc2a..8e11bb43 100644 --- a/docs/migrations.md +++ b/docs/migrations.md @@ -89,3 +89,46 @@ Audit field migration note - Use `wl update --audit-text ` to set audit via the existing CLI interface (writes to `audit_results` table). - Redaction/safety rules for audit text are tracked separately in `Redaction and Safety Rules for Audit Text (WL-0MMNCOIYS15A1YSI)`. - See `docs/AUDIT_STATUS.md` for full documentation on audit status semantics. + +Hook upgrades +------------- +The `wl doctor upgrade` command also checks and upgrades outdated git hooks installed in +`.git/hooks/` from the committed safe versions in `.githooks/`. This ensures existing +installations automatically get the safe hook patterns (using `--git-branch refs/worklog/data`) +without manual re-installation. + +A hook is considered outdated when it: +- Lacks the safe `--git-branch refs/worklog/data` guard, or +- Contains hardcoded paths like `/tmp/Worklog/...` or absolute paths to `.git/hooks/` for + the central post-pull script (should use `$(dirname "$0")`), or +- Differs in any way from the committed `.githooks/` version. + +Only hooks that contain a Worklog marker (`worklog:pre-push-hook:`, `worklog:post-pull-hook:`, +or `worklog:post-checkout-hook:`) are considered for upgrade. Non-Worklog hooks are skipped. + +Examples: + +- Preview outdated hooks alongside migrations (dry-run): + + `wl doctor upgrade --dry-run` + +- Preview outdated hooks with JSON output: + + `wl doctor upgrade --dry-run --json` + +- Apply outdated hooks alongside migrations (non-interactively): + + `wl doctor upgrade --confirm` + +- Apply outdated hooks alongside migrations (interactive prompt): + + `wl doctor upgrade` + +The committed hook scripts live in `.githooks/` and include: +- `pre-push` — auto-syncs Worklog data before pushing, uses `--git-branch refs/worklog/data` +- `post-checkout` — auto-syncs after branch checkout +- `post-merge` — wrapper that delegates to `worklog-post-pull` +- `post-rewrite` — wrapper that delegates to `worklog-post-pull` +- `worklog-post-pull` — central sync script + +See `src/doctor/hook-upgrade.ts` for the implementation. diff --git a/package-lock.json b/package-lock.json index 02727a0a..6eecfa45 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "worklog", - "version": "1.0.0", + "version": "1.0.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "worklog", - "version": "1.0.0", + "version": "1.0.2", "license": "MIT", "dependencies": { "@worklog/shared": "file:./packages/shared", diff --git a/package.json b/package.json index 81db6969..25a48587 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "worklog", - "version": "1.0.2", + "version": "1.0.3", "description": "A simple experimental issue tracker for AI agents", "main": "dist/index.js", "type": "module", diff --git a/packages/shared/src/database.ts b/packages/shared/src/database.ts index ef080e84..628e5f02 100644 --- a/packages/shared/src/database.ts +++ b/packages/shared/src/database.ts @@ -969,6 +969,9 @@ export class WorklogDatabase { this.store.saveWorkItem(item); this.store.upsertFtsEntry(item); this.triggerSemanticIndex(item); + // Clear comment caches that triggerSemanticIndex may have populated + // with stale (empty) data, so subsequent reads see fresh results. + this.store.clearCommentCaches(); this.triggerAutoSync(); return item; } diff --git a/packages/shared/src/persistent-store.ts b/packages/shared/src/persistent-store.ts index 29ba73c5..c1e3b47e 100644 --- a/packages/shared/src/persistent-store.ts +++ b/packages/shared/src/persistent-store.ts @@ -614,6 +614,7 @@ export class SqlitePersistentStore { }); doUpdates(); + this.invalidateWorkItemCaches(); return updated; } @@ -1686,6 +1687,7 @@ export class SqlitePersistentStore { */ private invalidateWorkItemCaches(): void { this.cacheInvalidatePrefix('workitem_'); + this.cacheInvalidatePrefix('commentsForItem_'); this.cacheInvalidate('allWorkItems'); this.cacheInvalidate('countWorkItems'); this.cacheInvalidate('allChildren'); @@ -1710,6 +1712,13 @@ export class SqlitePersistentStore { this.cacheInvalidate('allDependencyEdges'); } + /** + * Public wrapper to clear comment-related caches. + */ + clearCommentCaches(): void { + this.invalidateCommentCaches(); + } + close(): void { this.db.close(); this.cacheClear(); diff --git a/packages/tui/extensions/README.md b/packages/tui/extensions/README.md index 7011ac60..66f85dfb 100644 --- a/packages/tui/extensions/README.md +++ b/packages/tui/extensions/README.md @@ -178,6 +178,109 @@ when used outside the Pi TUI. - Built-in Pi commands and free-form text clear the indicator via the same `input` event handler. +## Session Health Footer + +The extension displays a **real-time session health footer** that replaces +Pi's default footer with a rich health dashboard showing: + +### What It Displays + +The footer uses a **three-section layout**: **left** (status + elapsed time since last response + turn count + last-chunk timer), **center** (total wall-clock session duration), and **right** (token usage + context + model). + +| Element | Description | +|---------|-------------| +| **Status marker** | `○` idle, `●` streaming, `⚡ ` tool execution | +| **Last chunk time** | Elapsed time since the last streaming chunk (e.g., `(Last Chunk: 3s ago)`) — shown only during active streaming | +| **Turn count** | Number of turns in the current session (e.g., `#3`) | +| **Response elapsed** | Colour-coded elapsed time since the last model response (shown after the state marker in the left section) | +| **Total session time** | Total wall-clock session duration (e.g., `Total: 5m 42s`) — shown in the center section | +| **Token usage** | Input/output token counts (e.g., `↑1.2k ↓4.5k`) | +| **Context usage** | Percentage of context window (e.g., `76.8%/128k`) | +| **Model ID** | Currently active model (e.g., `gpt-4`) | + +### Colour Coding + +The response age indicator uses colour coding to provide at-a-glance health: + +| Colour | Threshold | Meaning | +|--------|-----------|--------| +| Green (`success`) | < 5s | Healthy — response received recently | +| Yellow/Orange (`warning`) | 5–30s | Moderate delay — model is processing | +| Red (`error`) | > 30s | Stuck or slow — consider interrupting | + +### Layout + +The footer spans two lines. The first line shows extension status entries +(e.g., resolved provider/model, activity indicator). The second line shows +session health metrics in a **three-section layout**: + +``` +openai/gpt-4 ⏵ /wl ← Extension statuses +● Streaming 45s #5 (3s ago) Total: 5m 42s ↑1.2k ↓4.5k 39.1%/128k ← Session health +│ │ │ │ │ │ │ │ │ │ +│ │ │ │ │ │ │ │ └────────────── Context usage +│ │ │ │ │ │ │ └────────────────────── Output tokens +│ │ │ │ │ │ └─────────────────────────── Input tokens +│ │ │ │ │ └─────────────────────────────────────── Total session time +│ │ │ │ └───────────────────────────────────────────────────── Last chunk timer (streaming only) +│ │ │ └──────────────────────────────────────────────────────── Turn count +│ │ └──────────────────────────────────────────────────────────── Elapsed time since last response +└────────────────────────────────────────────────────────────────────────── Status marker +``` + +During **idle** or **tool execution**, the last-chunk timer is not shown. + +### Event Tracking + +The extension subscribes to the following Pi lifecycle events to update state: + +| Event | Update | +|-------|--------| +| `turn_start` | Increment turn count, set status to streaming, set last-response timer | +| `message_update` | Update last-chunk timer (shown during active streaming) | +| `message_end` (assistant) | Set status to idle, update response time | +| `tool_execution_start` | Set status to tool with tool name | +| `tool_execution_end` | Reset status to idle | +| `model_select` | Refresh footer display | +| `session_start` | Reset counters, initialize footer | +| `session_shutdown` | Clean up ticker interval | + +### Ticker + +A 1-second `setInterval` ticker refreshes the token counts and context usage +from the session manager. The footer re-renders automatically when the branch +changes. + +### Graceful Degradation + +The session health footer gracefully degrades in non-TUI modes (print, JSON, +RPC) where `setFooter` is a no-op. The feature has no effect and does not +produce errors when used outside the Pi TUI. + +### Technical Notes + +- Uses Pi's `ctx.ui.setFooter()` API to replace the entire footer with a + custom render function. +- The footer renderer uses `truncateToWidth` and `visibleWidth` from + `@earendil-works/pi-tui` for safe ANSI-aware truncation. +- Only one `setFooter()` can be active at a time. This module's footer + replaces Pi's default footer (git branch, cwd path, etc.). +- The footer includes extension status entries (set via `ctx.ui.setStatus()`) + from `footerData.getExtensionStatuses()` as the first line. This ensures + that status entries from other modules — such as the resolved + provider/model (`worklog-0model`) and the activity indicator + (`worklog-activity`) — remain visible alongside the session health metrics. +- The footer uses a **three-section layout**: left (status marker + elapsed + time since last response + turn count + last-chunk timer), center (total + session duration), and right (token counts + context usage + model ID). +- A `lastChunkTime` property tracks the timestamp of the last `message_update` + event. The **last-chunk timer** `(Last Chunk: Xs ago)` is displayed in the + left section only when `status === 'streaming'`. +- Token counts are calculated from session entries by summing + `usage.input` and `usage.output` from assistant messages. +- Context usage is obtained from `ctx.getContextUsage()` which returns + `{ tokens, contextWindow, percent }`. + ## Error Recovery Module The extension includes a built-in automatic error recovery module that replaces the @@ -589,4 +692,40 @@ Example: "description": "Update the priority of the selected work item" } ``` + +## Lease Release + +The extension includes a **proactive lease release** module that automatically +releases the previous session's model lease when a new Pi session is created +(via `/new`). This speeds up model reclamation on the Local Proxy provider. + +### How It Works + +1. When Pi fires a `session_start` event with reason `"new"` (indicating a + session replacement), the module reads `~/.pi/agent/models.json` to locate + the `"Local Proxy"` provider's `baseUrl`. +2. It sends a best-effort `POST {baseUrl}/leases/release` with a JSON body + containing the previous session's identifier (`previousSessionFile`). +3. The call is **fire-and-forget**: it does not block session startup. + Failures (network errors, non-2xx responses) are silently logged at + debug level only — no user-visible errors. +4. If the `"Local Proxy"` provider is not configured, no request is sent. + +### When It Fires + +| Session Start Reason | Lease Release Triggered | +|----------------------|------------------------| +| `"startup"` (initial Pi launch) | No | +| `"new"` (session via `/new`) | Yes | +| `"resume"` (session resumed) | No | +| `"fork"` (session forked) | No | +| `"reload"` (extensions reloaded) | No | + +### Technical Notes + +- Implemented in `Worklog/lease-release.ts`. +- The proxy configuration is read at runtime from `~/.pi/agent/models.json`. +- Results are cached per-extension-lifecycle to avoid repeated filesystem reads. +- Registered in `Worklog/index.ts` via `registerLeaseRelease(pi)`. +- Tests are in `Worklog/lease-release.test.ts`. ``` diff --git a/packages/tui/extensions/Worklog/activity-indicator.ts b/packages/tui/extensions/Worklog/activity-indicator.ts index 81746b04..c64e3f35 100644 --- a/packages/tui/extensions/Worklog/activity-indicator.ts +++ b/packages/tui/extensions/Worklog/activity-indicator.ts @@ -37,6 +37,7 @@ import type { ExtensionAPI, ExtensionContext, ExtensionUIContext } from '@earendil-works/pi-coding-agent'; import { runWl } from '../wl-integration.js'; +import { truncateWorkItemId } from './terminal-utils.js'; /** * Status key used for the activity indicator in the footer. @@ -237,10 +238,10 @@ async function showActivityWithTitleLookup(ctx: StatusContext, text: string, sho const title = await resolveWorkItemTitle(id); if (!title) return; - // Replace with command + ID + title format, truncated to fit terminal width. + // Replace with command + truncated ID + title format, truncated to fit terminal width. // The command is formatted via formatCommandContext (e.g., /skill:audit → audit). const commandCtx = formatCommandContext(text); - const display = `${commandCtx} ${id} ${title}`; + const display = `${commandCtx} ${truncateWorkItemId(id)} ${title}`; showActivity(ctx, display, showIndicator); } diff --git a/packages/tui/extensions/Worklog/index.ts b/packages/tui/extensions/Worklog/index.ts index f852a805..d48f678f 100644 --- a/packages/tui/extensions/Worklog/index.ts +++ b/packages/tui/extensions/Worklog/index.ts @@ -20,9 +20,11 @@ import { reloadSettings, currentSettings, STAGE_MAP, VALID_STAGES, updateSetting import { runWl, defaultListWorkItems, defaultListWorkItemsWithStage, createDefaultListWorkItems, createListWorkItemsWithStage, createDefaultListWorkItemsDb, createListWorkItemsWithStageDb, fetchTotalActionableCountDb } from './lib/tools.js'; import { registerAutoInject } from './lib/auto-inject.js'; import { registerModelDisplay } from './model-display.js'; +import { registerSessionHealth } from './session-health.js'; import { INSTALL_GUARDRAILS } from './lib/guardrails.js'; import { registerSkillPathTool } from './lib/skill-path.js'; import { registerRecoveryModule } from './lib/recovery/register-recovery.js'; +import { registerLeaseRelease } from './lease-release.js'; import { type WorklogBrowseItem, type WorklogBrowseDependencies, @@ -90,6 +92,7 @@ export function createWorklogBrowseExtension(deps: WorklogBrowseDependencies = { registerActivityIndicator(pi, () => currentSettings.showActivityIndicator); registerAutoInject(pi); registerModelDisplay(pi); + registerSessionHealth(pi); INSTALL_GUARDRAILS(pi, { enabled: currentSettings.guardrailsEnabled }); // ── Skill path discovery tool ───────────────────────────────── @@ -100,6 +103,9 @@ export function createWorklogBrowseExtension(deps: WorklogBrowseDependencies = { // ── Recovery module (automatic error recovery) ──────────────── registerRecoveryModule(pi); + // ── Lease release (proactive proxy model lease release on /new) ── + registerLeaseRelease(pi); + // Subscribe to config changes for hot-reload notifications // When settings change via /wl settings or file edit, all onChange // subscribers are notified immediately without requiring /reload. diff --git a/packages/tui/extensions/Worklog/lease-release.test.ts b/packages/tui/extensions/Worklog/lease-release.test.ts new file mode 100644 index 00000000..eced3d46 --- /dev/null +++ b/packages/tui/extensions/Worklog/lease-release.test.ts @@ -0,0 +1,290 @@ +/** + * Unit tests for lease-release.ts — Proactive lease release on session close. + * + * Verifies that: + * 1. releaseLease() reads ~/.pi/agent/models.json to find the Local Proxy baseUrl. + * 2. releaseLease() sends POST {baseUrl}/leases/release with correct body. + * 3. Errors (network failures, non-2xx) are handled gracefully (no throw, debug log). + * 4. Missing "Local Proxy" provider: no request sent, debug log. + * 5. Missing models.json file: no request sent, debug log. + * 6. The registerLeaseRelease() function sets up session_start handler. + * 7. The session_start handler calls releaseLease on "new" reason. + * 8. Non-"new" reasons (startup, resume, fork) do not trigger releaseLease. + * + * Run: npx vitest run packages/tui/extensions/Worklog/lease-release.test.ts + */ + +import { describe, it, expect, vi, beforeEach, afterEach, afterAll } from 'vitest'; + +// ── Mock node:fs/promises ──────────────────────────────────────────── + +const mockReadFile = vi.fn(); +vi.mock('node:fs/promises', () => ({ + readFile: mockReadFile, +})); + +// ── Mock global fetch ──────────────────────────────────────────────── + +const mockFetch = vi.fn(); +vi.stubGlobal('fetch', mockFetch); + +// ── Mock console.debug ─────────────────────────────────────────────── + +const mockConsoleDebug = vi.fn(); +const origConsoleDebug = console.debug; + +// ── Module under test (loaded lazily) ──────────────────────────────── + +let mod: typeof import('./lease-release.js'); + +describe('lease-release', () => { + beforeEach(async () => { + vi.clearAllMocks(); + process.env.HOME = '/home/test-user'; + // Patch console.debug for this test + console.debug = mockConsoleDebug; + // Re-import fresh each test so module-level state is reset + mod = await import('./lease-release.js'); + mod._resetLeaseReleaseState(); + }); + + afterEach(() => { + console.debug = origConsoleDebug; + }); + + afterAll(() => { + console.debug = origConsoleDebug; + }); + + // ── releaseLease() function tests ───────────────────────────────── + + describe('releaseLease', () => { + it('reads models.json and sends POST to correct endpoint with session ID', async () => { + mockReadFile.mockResolvedValue(JSON.stringify({ + providers: { + 'Local Proxy': { baseUrl: 'http://192.168.0.199:8000/v1' }, + }, + })); + mockFetch.mockResolvedValue({ ok: true, status: 200 }); + + await mod.releaseLease('session-abc-123'); + + expect(mockReadFile).toHaveBeenCalledWith( + '/home/test-user/.pi/agent/models.json', + 'utf-8', + ); + expect(mockFetch).toHaveBeenCalledWith( + 'http://192.168.0.199:8000/v1/leases/release', + expect.objectContaining({ + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ session_id: 'session-abc-123' }), + }), + ); + }); + + it('does not throw on network failure (fetch rejects)', async () => { + mockReadFile.mockResolvedValue(JSON.stringify({ + providers: { + 'Local Proxy': { baseUrl: 'http://192.168.0.199:8000/v1' }, + }, + })); + mockFetch.mockRejectedValue(new Error('Network error')); + + await expect(mod.releaseLease('session-abc-123')).resolves.toBeUndefined(); + expect(mockConsoleDebug).toHaveBeenCalled(); + }); + + it('does not throw on non-2xx response', async () => { + mockReadFile.mockResolvedValue(JSON.stringify({ + providers: { + 'Local Proxy': { baseUrl: 'http://192.168.0.199:8000/v1' }, + }, + })); + mockFetch.mockResolvedValue({ ok: false, status: 500 }); + + await expect(mod.releaseLease('session-abc-123')).resolves.toBeUndefined(); + }); + + it('does not send request when "Local Proxy" provider is missing', async () => { + mockReadFile.mockResolvedValue(JSON.stringify({ + providers: { + 'Other Provider': { baseUrl: 'http://other:8000/v1' }, + }, + })); + + await mod.releaseLease('session-abc-123'); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it('does not send request when models.json cannot be read', async () => { + mockReadFile.mockRejectedValue(new Error('ENOENT: file not found')); + + await mod.releaseLease('session-abc-123'); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it('does not send request when Local Proxy provider has no baseUrl', async () => { + mockReadFile.mockResolvedValue(JSON.stringify({ + providers: { + 'Local Proxy': {}, + }, + })); + + await mod.releaseLease('session-abc-123'); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it('passes the session ID as-is to the proxy', async () => { + mockReadFile.mockResolvedValue(JSON.stringify({ + providers: { + 'Local Proxy': { baseUrl: 'http://192.168.0.199:8000/v1' }, + }, + })); + mockFetch.mockResolvedValue({ ok: true, status: 200 }); + + await mod.releaseLease('/home/test-user/.pi/sessions/session-xyz.json'); + + expect(mockFetch).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + body: JSON.stringify({ session_id: '/home/test-user/.pi/sessions/session-xyz.json' }), + }), + ); + }); + + it('still sends request when session_id is empty string (proxy handles validation)', async () => { + mockReadFile.mockResolvedValue(JSON.stringify({ + providers: { + 'Local Proxy': { baseUrl: 'http://192.168.0.199:8000/v1' }, + }, + })); + mockFetch.mockResolvedValue({ ok: true, status: 200 }); + + await expect(mod.releaseLease('')).resolves.toBeUndefined(); + expect(mockFetch).toHaveBeenCalled(); + }); + + it('only calls console.debug on error (not console.error or console.warn)', async () => { + mockReadFile.mockResolvedValue(JSON.stringify({ + providers: { + 'Local Proxy': { baseUrl: 'http://192.168.0.199:8000/v1' }, + }, + })); + mockFetch.mockRejectedValue(new Error('Network error')); + + await mod.releaseLease('session-abc-123'); + + expect(mockConsoleDebug).toHaveBeenCalled(); + }); + + it('survives missing Local Proxy in models.json (no crash)', async () => { + mockReadFile.mockResolvedValue(JSON.stringify({ + providers: {}, + })); + + await expect(mod.releaseLease('session-abc-123')).resolves.toBeUndefined(); + expect(mockFetch).not.toHaveBeenCalled(); + }); + }); + + // ── registerLeaseRelease() integration tests ────────────────────── + + describe('registerLeaseRelease', () => { + /** Tracks the session_start handler registered by registerLeaseRelease */ + let registeredHandler: Function | null = null; + let mockPi: any; + + beforeEach(() => { + registeredHandler = null; + mockPi = { + on: vi.fn((event: string, handler: Function) => { + if (event === 'session_start') { + registeredHandler = handler; + } + }), + }; + }); + + it('registers a session_start handler', () => { + mod.registerLeaseRelease(mockPi); + expect(mockPi.on).toHaveBeenCalledWith('session_start', expect.any(Function)); + expect(registeredHandler).not.toBeNull(); + }); + + it('calls releaseLease when reason is "new" and previousSessionFile is set', async () => { + mockReadFile.mockResolvedValue(JSON.stringify({ + providers: { + 'Local Proxy': { baseUrl: 'http://192.168.0.199:8000/v1' }, + }, + })); + mockFetch.mockResolvedValue({ ok: true, status: 200 }); + + mod.registerLeaseRelease(mockPi); + + // Keep a reference to the handler before awaiting + const handler = registeredHandler!; + await handler( + { type: 'session_start', reason: 'new', previousSessionFile: '/sessions/s1.json' }, + {}, + ); + + expect(mockFetch).toHaveBeenCalledWith( + 'http://192.168.0.199:8000/v1/leases/release', + expect.objectContaining({ + body: JSON.stringify({ session_id: '/sessions/s1.json' }), + }), + ); + }); + + it('does NOT call releaseLease when reason is "startup"', async () => { + mod.registerLeaseRelease(mockPi); + + await registeredHandler!( + { type: 'session_start', reason: 'startup' }, + {}, + ); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it('does NOT call releaseLease when reason is "resume"', async () => { + mod.registerLeaseRelease(mockPi); + + await registeredHandler!( + { type: 'session_start', reason: 'resume', previousSessionFile: '/sessions/s1.json' }, + {}, + ); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it('does NOT call releaseLease when reason is "fork"', async () => { + mod.registerLeaseRelease(mockPi); + + await registeredHandler!( + { type: 'session_start', reason: 'fork', previousSessionFile: '/sessions/s1.json' }, + {}, + ); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it('does NOT call releaseLease when reason is "reload"', async () => { + mod.registerLeaseRelease(mockPi); + + await registeredHandler!( + { type: 'session_start', reason: 'reload' }, + {}, + ); + expect(mockFetch).not.toHaveBeenCalled(); + }); + + it('does NOT call releaseLease when previousSessionFile is undefined', async () => { + mod.registerLeaseRelease(mockPi); + + await registeredHandler!( + { type: 'session_start', reason: 'new' }, + {}, + ); + expect(mockFetch).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/packages/tui/extensions/Worklog/lease-release.ts b/packages/tui/extensions/Worklog/lease-release.ts new file mode 100644 index 00000000..1127c0e1 --- /dev/null +++ b/packages/tui/extensions/Worklog/lease-release.ts @@ -0,0 +1,171 @@ +/** + * Lease release module for the Worklog Pi extension. + * + * Proactively releases the previous session's model lease when a new Pi + * session is created (via `/new`). Reads the Local Proxy provider + * configuration from `~/.pi/agent/models.json` and sends a best-effort + * HTTP POST to `{baseUrl}/leases/release`. + * + * The call is fire-and-forget: + * - It does not block or delay session startup. + * - Failures (network errors, non-2xx responses) are silently logged at + * debug level only — no user-visible errors. + * - If the "Local Proxy" provider is not configured, no request is sent. + */ + +import { readFile } from 'node:fs/promises'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; + +// ── Constants ──────────────────────────────────────────────────────────── + +/** Path to Pi's model/provider configuration file. */ +const MODELS_JSON_PATH = join(homedir(), '.pi', 'agent', 'models.json'); + +/** The name of the Local Proxy provider in models.json. */ +const LOCAL_PROVIDER_NAME = 'Local Proxy'; + +/** The lease release API endpoint path (appended to the provider's baseUrl). */ +const LEASE_RELEASE_PATH = '/leases/release'; + +// ── Module-level cache ─────────────────────────────────────────────────── + +/** + * Cached base URL for the Local Proxy provider. + * Set once on first successful read, or cleared to force re-read. + * Module-level cache reduces filesystem reads during repeated calls + * within the same extension lifecycle. + */ +let _cachedBaseUrl: string | null | undefined = undefined; + +/** + * Reset the cached base URL (for testing). + * + * @internal Used by tests to ensure isolation between test cases. + */ +export function _resetLeaseReleaseState(): void { + _cachedBaseUrl = undefined; +} + +// ── Internal helpers ───────────────────────────────────────────────────── + +/** + * Read the Local Proxy base URL from Pi's models.json. + * + * Reads `~/.pi/agent/models.json`, parses the provider entries, and + * extracts the `baseUrl` of the `"Local Proxy"` provider. + * + * Returns `null` if: + * - The file does not exist or cannot be read. + * - The file is not valid JSON. + * - The `"Local Proxy"` provider is not found. + * - The found provider has no `baseUrl` field. + * + * @returns The base URL string, or `null` if unavailable. + */ +async function readLocalProxyBaseUrl(): Promise { + // Return cached value if previously resolved + if (_cachedBaseUrl !== undefined) { + return _cachedBaseUrl; + } + + try { + const content = await readFile(MODELS_JSON_PATH, 'utf-8'); + const config = JSON.parse(content); + + const provider = config?.providers?.[LOCAL_PROVIDER_NAME]; + const baseUrl = provider?.baseUrl; + + if (!baseUrl || typeof baseUrl !== 'string') { + console.debug( + `[lease-release] Local Proxy provider "${LOCAL_PROVIDER_NAME}" has no baseUrl in ${MODELS_JSON_PATH}; skipping lease release`, + ); + _cachedBaseUrl = null; + return null; + } + + _cachedBaseUrl = baseUrl; + return baseUrl; + } catch (err: unknown) { + const message = err instanceof Error ? err.message : String(err); + console.debug( + `[lease-release] Could not read proxy config from ${MODELS_JSON_PATH}: ${message}`, + ); + _cachedBaseUrl = null; + return null; + } +} + +// ── Public API ─────────────────────────────────────────────────────────── + +/** + * Proactively release the model lease for a previous session. + * + * Reads the Local Proxy base URL from `~/.pi/agent/models.json` and sends + * a best-effort `POST {baseUrl}/leases/release` with the session identifier. + * + * The call is fire-and-forget: + * - Failures are logged at debug level only (no user-visible errors). + * - If the Local Proxy is not configured, no request is sent. + * - If the models.json file cannot be read, no request is sent. + * + * @param previousSessionId - The session identifier from the previous + * session (value of `previousSessionFile` from the session_start event). + * Passed as-is to the proxy. + */ +export async function releaseLease(previousSessionId: string): Promise { + const baseUrl = await readLocalProxyBaseUrl(); + + if (!baseUrl) { + return; + } + + const url = `${baseUrl.replace(/\/+$/, '')}${LEASE_RELEASE_PATH}`; + const body = JSON.stringify({ session_id: previousSessionId }); + + try { + const response = await fetch(url, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body, + }); + + if (!response.ok) { + console.debug( + `[lease-release] Lease release request to ${url} returned status ${response.status}`, + ); + } + } catch (err: unknown) { + const message = err instanceof Error ? err.message : String(err); + console.debug( + `[lease-release] Lease release request to ${url} failed: ${message}`, + ); + } +} + +/** + * Register the lease release session_start handler with a Pi extension instance. + * + * Sets up a listener for `session_start` that calls `releaseLease()` when: + * - `event.reason === "new"` (session replaced via `/new`) + * - `event.previousSessionFile` is present + * + * The call is fire-and-forget and does not block session startup. + * + * @param pi - The ExtensionAPI instance + */ +export function registerLeaseRelease(pi: { + on: (event: string, handler: (event: any, ctx: any) => void | Promise) => void; +}): void { + // The handler is async so the event loop drains microtasks between turns. + // From Pi's perspective this is still fire-and-forget because the event + // system does not await the handler's returned promise. + pi.on('session_start', async (event: any, _ctx: any) => { + if (event.reason === 'new' && event.previousSessionFile) { + // Fire-and-forget from Pi's perspective, but we await internally so + // the microtask queue is drained properly (helpful for tests). + // releaseLease already catches all errors internally. + await releaseLease(event.previousSessionFile); + } + }); +} diff --git a/packages/tui/extensions/Worklog/model-display.test.ts b/packages/tui/extensions/Worklog/model-display.test.ts index b76655b4..2bb2c69b 100644 --- a/packages/tui/extensions/Worklog/model-display.test.ts +++ b/packages/tui/extensions/Worklog/model-display.test.ts @@ -1,16 +1,14 @@ /** - * Unit tests for model-display.ts — Model/provider display in Pi's status bar. + * Unit tests for model-display.ts — Model/provider display state management. * * Verifies that: - * 1. registerModelDisplay() sets up event listeners for model_select and - * after_provider_response. - * 2. The status display shows only the resolved provider/model (no model alias). - * 3. after_provider_response updates the status bar with the resolved - * provider/model from X-Resolved-Model header. - * 4. When no proxy response has been received yet, the status bar shows (pending). - * 5. The status bar key is "worklog-0model" (not "model-display"), - * with the `0` prefix ensuring it sorts first in the extension status line. - * 6. The module exports the MODEL_DISPLAY_STATUS_KEY constant. + * 1. registerModelDisplay() sets up event listeners for session_start, + * model_select, and after_provider_response. + * 2. The getters return the correct values after events fire. + * 3. after_provider_response updates the resolved model from X-Resolved-Model header. + * 4. When no proxy response has been received yet, getResolvedModel() returns null. + * 5. The module exports the MODEL_DISPLAY_STATUS_KEY and getter/registration functions. + * 6. onModelChange() notifies consumers when model state changes. * * Run: npx vitest run packages/tui/extensions/Worklog/model-display.test.ts */ @@ -25,29 +23,32 @@ vi.mock('@earendil-works/pi-coding-agent', () => ({ // ── Module under test ───────────────────────────────────────────────── -import { registerModelDisplay, MODEL_DISPLAY_STATUS_KEY } from './model-display.js'; +import { + registerModelDisplay, + MODEL_DISPLAY_STATUS_KEY, + getResolvedModel, + getSelectedModel, + onModelChange, + _resetModelDisplayState, +} from './model-display.js'; describe('model-display', () => { /** Tracks event listeners registered by registerModelDisplay. */ const registeredListeners: Record = {}; - /** Tracks status bar calls: { key: value } */ - const statusCalls: Record = {}; - /** Mock ExtensionAPI with on() and a minimal context with ui. */ + /** Mock ExtensionAPI with on(). */ let mockPi: any; + /** Mock context (minimal, not used after the refactor). */ let mockCtx: any; beforeEach(() => { // Reset tracking Object.keys(registeredListeners).forEach(k => delete registeredListeners[k]); - Object.keys(statusCalls).forEach(k => delete statusCalls[k]); // Mock extension context mockCtx = { ui: { - setStatus: vi.fn((key: string, value: string | undefined) => { - statusCalls[key] = value; - }), + setStatus: vi.fn(), }, }; @@ -57,8 +58,13 @@ describe('model-display', () => { registeredListeners[event] = handler; }), }; + + // Reset module-level state so each test starts clean + _resetModelDisplayState(); }); + + // ── Exports ─────────────────────────────────────────────────────── it('exports MODEL_DISPLAY_STATUS_KEY', () => { @@ -69,8 +75,33 @@ describe('model-display', () => { expect(typeof registerModelDisplay).toBe('function'); }); + it('exports getResolvedModel and getSelectedModel as functions', () => { + expect(typeof getResolvedModel).toBe('function'); + expect(typeof getSelectedModel).toBe('function'); + }); + + it('exports onModelChange as a function', () => { + expect(typeof onModelChange).toBe('function'); + }); + + // ── Initial state ────────────────────────────────────────────────── + + it('getResolvedModel returns null before any events', () => { + expect(getResolvedModel()).toBeNull(); + }); + + it('getSelectedModel returns null before any events', () => { + expect(getSelectedModel()).toBeNull(); + }); + // ── Event registration ─────────────────────────────────────────── + it('registers session_start listener', () => { + registerModelDisplay(mockPi); + expect(mockPi.on).toHaveBeenCalledWith('session_start', expect.any(Function)); + expect(registeredListeners['session_start']).toBeDefined(); + }); + it('registers model_select listener', () => { registerModelDisplay(mockPi); expect(mockPi.on).toHaveBeenCalledWith('model_select', expect.any(Function)); @@ -83,62 +114,94 @@ describe('model-display', () => { expect(registeredListeners['after_provider_response']).toBeDefined(); }); + // ── session_start behavior ────────────────────────────────────── + + it('session_start captures Pi model alias from context', () => { + registerModelDisplay(mockPi); + const handler = registeredListeners['session_start']; + + handler({}, { model: { id: 'code' } }); + + expect(getSelectedModel()).toBe('code'); + expect(getResolvedModel()).toBeNull(); + }); + + it('session_start does not overwrite an already-set selectedModel', () => { + registerModelDisplay(mockPi); + const modelHandler = registeredListeners['model_select']; + const sessionHandler = registeredListeners['session_start']; + + // Model already selected + modelHandler({ model: { id: 'plan' } }, mockCtx); + expect(getSelectedModel()).toBe('plan'); + + // session_start with a different model context + sessionHandler({}, { model: { id: 'code' } }); + + // Should NOT overwrite — model_select is the authoritative source + expect(getSelectedModel()).toBe('plan'); + }); + // ── model_select behavior ──────────────────────────────────────── - it('model_select sets status to (pending) when no response yet', () => { + it('model_select sets selectedModel when model id is provided', () => { registerModelDisplay(mockPi); const handler = registeredListeners['model_select']; handler({ model: { id: 'plan', provider: 'openai' } }, mockCtx); - expect(mockCtx.ui.setStatus).toHaveBeenCalledWith( - 'worklog-0model', - '(pending)', - ); + expect(getSelectedModel()).toBe('plan'); + expect(getResolvedModel()).toBeNull(); }); - it('model_select without model id does not crash and does not set status', () => { + it('model_select without model id does not crash and does not change state', () => { registerModelDisplay(mockPi); const handler = registeredListeners['model_select']; handler({ model: null }, mockCtx); - // No status call expected because there's nothing to display - expect(mockCtx.ui.setStatus).not.toHaveBeenCalled(); + // State should remain unchanged (null) + expect(getSelectedModel()).toBeNull(); + expect(getResolvedModel()).toBeNull(); + }); + + it('model_select clears selectedModel when model is null after being set', () => { + registerModelDisplay(mockPi); + const handler = registeredListeners['model_select']; + + // First set a model + handler({ model: { id: 'plan' } }, mockCtx); + expect(getSelectedModel()).toBe('plan'); + + // Then clear it + handler({ model: null }, mockCtx); + expect(getSelectedModel()).toBeNull(); }); - it('model_select no longer shows model alias when after_provider_response was previously received', () => { + it('after_provider_response overwrites resolvedModel even when model_select was previously received', () => { registerModelDisplay(mockPi); const modelHandler = registeredListeners['model_select']; const responseHandler = registeredListeners['after_provider_response']; - // First, receive a provider response + // First, select a model + modelHandler({ model: { id: 'code' } }, mockCtx); + + // Then, receive a provider response responseHandler({ headers: { 'x-resolved-model': 'openai/gpt-4' } }, mockCtx); - expect(mockCtx.ui.setStatus).toHaveBeenLastCalledWith( - 'worklog-0model', - 'openai/gpt-4', - ); - // Then, select a model — should still show provider/model only (no model alias) - modelHandler({ model: { id: 'plan' } }, mockCtx); - expect(mockCtx.ui.setStatus).toHaveBeenLastCalledWith( - 'worklog-0model', - 'openai/gpt-4', - ); + expect(getResolvedModel()).toBe('openai/gpt-4'); + expect(getSelectedModel()).toBe('code'); }); // ── after_provider_response behavior ────────────────────────────── - it('after_provider_response extracts X-Resolved-Model from headers (no arrow prefix)', () => { + it('after_provider_response extracts X-Resolved-Model from headers', () => { registerModelDisplay(mockPi); const handler = registeredListeners['after_provider_response']; handler({ headers: { 'x-resolved-model': 'openai/gpt-4' } }, mockCtx); - expect(mockCtx.ui.setStatus).toHaveBeenCalledWith( - 'worklog-0model', - 'openai/gpt-4', - ); + expect(getResolvedModel()).toBe('openai/gpt-4'); }); it('after_provider_response handles uppercase X-Resolved-Model header', () => { @@ -147,10 +210,7 @@ describe('model-display', () => { handler({ headers: { 'X-Resolved-Model': 'anthropic/claude-3' } }, mockCtx); - expect(mockCtx.ui.setStatus).toHaveBeenCalledWith( - 'worklog-0model', - 'anthropic/claude-3', - ); + expect(getResolvedModel()).toBe('anthropic/claude-3'); }); it('after_provider_response without resolved model header does not crash', () => { @@ -159,81 +219,114 @@ describe('model-display', () => { handler({ headers: {} }, mockCtx); - // No call expected because nothing to update - expect(mockCtx.ui.setStatus).not.toHaveBeenCalled(); + expect(getResolvedModel()).toBeNull(); }); - it('after_provider_response shows provider/model (no model alias) when model was previously selected', () => { + it('after_provider_response shows provider/model even when no model was previously selected', () => { registerModelDisplay(mockPi); - const modelHandler = registeredListeners['model_select']; - const responseHandler = registeredListeners['after_provider_response']; - - // First, select a model - modelHandler({ model: { id: 'code' } }, mockCtx); + const handler = registeredListeners['after_provider_response']; - // Then, receive a provider response - responseHandler({ headers: { 'x-resolved-model': 'anthropic/claude-sonnet-4' } }, mockCtx); + // Without any prior model_select + handler({ headers: { 'x-resolved-model': 'openai/gpt-4' } }, mockCtx); - expect(mockCtx.ui.setStatus).toHaveBeenLastCalledWith( - 'worklog-0model', - 'anthropic/claude-sonnet-4', - ); + expect(getResolvedModel()).toBe('openai/gpt-4'); + expect(getSelectedModel()).toBeNull(); }); - // ── Status key verification ─────────────────────────────────────── + // ── onModelChange behavior ──────────────────────────────────────── - it('uses status key "worklog-0model" (sorts first) not "model-display"', () => { + it('onModelChange fires on session_start when model is available', () => { registerModelDisplay(mockPi); + const changeCb = vi.fn(); + onModelChange(changeCb); + + const handler = registeredListeners['session_start']; + handler({}, { model: { id: 'code' } }); + + expect(changeCb).toHaveBeenCalledOnce(); + }); + + it('onModelChange fires on model_select', () => { + registerModelDisplay(mockPi); + const changeCb = vi.fn(); + onModelChange(changeCb); + const handler = registeredListeners['model_select']; + handler({ model: { id: 'plan' } }, mockCtx); - handler({ model: { id: 'plan', provider: 'openai' } }, mockCtx); + expect(changeCb).toHaveBeenCalledOnce(); + }); - // Verify the old key is NOT used - expect(statusCalls['model-display']).toBeUndefined(); - // Verify the new key IS used - expect(statusCalls['worklog-0model']).toBeDefined(); + it('onModelChange fires on after_provider_response', () => { + registerModelDisplay(mockPi); + const changeCb = vi.fn(); + onModelChange(changeCb); + + const handler = registeredListeners['after_provider_response']; + handler({ headers: { 'x-resolved-model': 'openai/gpt-4' } }, mockCtx); + + expect(changeCb).toHaveBeenCalledOnce(); + }); + + it('onModelChange disposer stops notifications', () => { + registerModelDisplay(mockPi); + const changeCb = vi.fn(); + const dispose = onModelChange(changeCb); + + dispose(); // Unregister + + const handler = registeredListeners['after_provider_response']; + handler({ headers: { 'x-resolved-model': 'openai/gpt-4' } }, mockCtx); + + expect(changeCb).not.toHaveBeenCalled(); }); // ── Edge cases ─────────────────────────────────────────────────── - it('handles multiple model_select calls, staying in pending state', () => { + it('handles multiple model_select calls, tracking the latest selection', () => { registerModelDisplay(mockPi); const handler = registeredListeners['model_select']; handler({ model: { id: 'plan' } }, mockCtx); - handler({ model: { id: 'code' } }, mockCtx); + expect(getSelectedModel()).toBe('plan'); - expect(mockCtx.ui.setStatus).toHaveBeenLastCalledWith( - 'worklog-0model', - '(pending)', - ); + handler({ model: { id: 'code' } }, mockCtx); + expect(getSelectedModel()).toBe('code'); }); it('handles multiple after_provider_response calls, overwriting previous resolved model', () => { registerModelDisplay(mockPi); - const modelHandler = registeredListeners['model_select']; - const responseHandler = registeredListeners['after_provider_response']; + const handler = registeredListeners['after_provider_response']; - modelHandler({ model: { id: 'code' } }, mockCtx); - responseHandler({ headers: { 'x-resolved-model': 'openai/gpt-4' } }, mockCtx); - responseHandler({ headers: { 'x-resolved-model': 'anthropic/claude-sonnet-4' } }, mockCtx); + handler({ headers: { 'x-resolved-model': 'openai/gpt-4' } }, mockCtx); + expect(getResolvedModel()).toBe('openai/gpt-4'); - expect(mockCtx.ui.setStatus).toHaveBeenLastCalledWith( - 'worklog-0model', - 'anthropic/claude-sonnet-4', - ); + handler({ headers: { 'x-resolved-model': 'anthropic/claude-sonnet-4' } }, mockCtx); + expect(getResolvedModel()).toBe('anthropic/claude-sonnet-4'); }); - it('after_provider_response with no prior model_select shows provider/model only', () => { + it('does not fire onModelChange when resolved model value does not change', () => { registerModelDisplay(mockPi); + const changeCb = vi.fn(); + onModelChange(changeCb); + const handler = registeredListeners['after_provider_response']; + handler({ headers: { 'x-resolved-model': 'openai/gpt-4' } }, mockCtx); + expect(changeCb).toHaveBeenCalledTimes(1); - // Without any prior model_select + // Same value again — should not fire handler({ headers: { 'x-resolved-model': 'openai/gpt-4' } }, mockCtx); + expect(changeCb).toHaveBeenCalledTimes(1); + }); + + it('does not fire onModelChange when header value is undefined', () => { + registerModelDisplay(mockPi); + const changeCb = vi.fn(); + onModelChange(changeCb); + + const handler = registeredListeners['after_provider_response']; + handler({ headers: {} }, mockCtx); - expect(mockCtx.ui.setStatus).toHaveBeenCalledWith( - 'worklog-0model', - 'openai/gpt-4', - ); + expect(changeCb).not.toHaveBeenCalled(); }); }); diff --git a/packages/tui/extensions/Worklog/model-display.ts b/packages/tui/extensions/Worklog/model-display.ts index 31a0e2b0..b3bb2e43 100644 --- a/packages/tui/extensions/Worklog/model-display.ts +++ b/packages/tui/extensions/Worklog/model-display.ts @@ -1,8 +1,12 @@ /** * Model display module for the Worklog Pi extension. * - * Displays the resolved provider/model from llama-proxy responses in Pi's - * status bar. Integrated from the standalone model-display extension + * Tracks the resolved provider/model from llama-proxy responses and the + * user-selected model alias. Exports getters so that other modules (e.g. + * session-health) can read the current provider/model for display in the + * footer, without needing to go through `setStatus`. + * + * Integrated from the standalone model-display extension * (previously at packages/tui/extensions/model-display.ts). * * Shows the format: `/` (e.g., `openai/gpt-4`) @@ -12,8 +16,9 @@ * that the llama-proxy adds to every proxy-routed request. * - Listens to `model_select` to track whether a model is selected (for the * `(pending)` fallback state), but does not display the model alias. - * - Updates the status bar on every request so the user always sees which - * provider and model actually served their request. + * - Exports `getResolvedModel()` and `getSelectedModel()` so the footer renderer + * can display the model on its own dedicated line. + * - Exports `onModelChange()` so consumers can be notified when state changes. * * Requires the llama-proxy to be configured as Pi's provider and to include * the `X-Resolved-Model` header in its responses (see LP-0MR4ZIGDT004A3E1). @@ -21,67 +26,126 @@ import type { ExtensionAPI } from '@earendil-works/pi-coding-agent'; +// ── Module-level state ──────────────────────────────────────────────────── + +/** The most recently resolved provider/model from a proxy response. */ +let _resolvedModel: string | null = null; + +/** The most recently selected Pi model alias. */ +let _selectedModel: string | null = null; + +/** Optional callback invoked whenever the model state changes. */ +let _onModelChange: (() => void) | null = null; + +/** + * Reset all module-level state to initial values. + * + * @internal Used by tests to ensure isolation between test cases. + */ +export function _resetModelDisplayState(): void { + _resolvedModel = null; + _selectedModel = null; + _onModelChange = null; +} + +// ── Public getters ──────────────────────────────────────────────────────── + +/** + * Return the resolved provider/model string (e.g. `"openai/gpt-4"`) + * from the last `after_provider_response` event, or `null` if no + * proxy response has been received yet. + */ +export function getResolvedModel(): string | null { + return _resolvedModel; +} + +/** + * Return the Pi model alias (e.g. `"code"`, `"plan"`) from the last + * `model_select` event, or `null` if no model is selected. + */ +export function getSelectedModel(): string | null { + return _selectedModel; +} + /** * Status key used for the model/provider display in the footer. * The `0` prefix ensures this status sorts first in the extension status * line (localeCompare order), placing the provider/model at the start. * Changed from "model-display" when this module was integrated into the * Worklog extension, to avoid conflicts with other status entries. + * + * @deprecated The model is now displayed on a dedicated footer line + * managed by session-health.ts. This constant is retained for + * backward compatibility with any external consumers. */ export const MODEL_DISPLAY_STATUS_KEY = 'worklog-0model'; +/** + * Register a callback that fires whenever the resolved model or selected + * model changes. Returns a disposer function that unregisters the callback. + * + * @param cb - Callback to invoke on model state change + * @returns Disposer function to unregister the callback + */ +export function onModelChange(cb: () => void): () => void { + _onModelChange = cb; + return () => { + _onModelChange = null; + }; +} + /** * Register model-display event handlers with a Pi extension instance. * * Sets up listeners for: - * - `model_select` — tracks whether a model is selected (for `(pending)` state) + * - `session_start` — captures the current Pi model alias from context + * - `model_select` — tracks model selection changes * - `after_provider_response` — reads the `X-Resolved-Model` header from - * llama-proxy responses and updates the status bar + * llama-proxy responses * - * The status display shows only the resolved provider/model from the - * `X-Resolved-Model` header. When no response has been received yet but a - * model is selected, it shows `(pending)`. The Pi model alias is not shown. + * The resolved provider/model can be read via `getResolvedModel()`. + * When no response has been received yet but a model is selected, + * `getSelectedModel()` returns the model alias (for `(pending)` display). + * The Pi model alias is not exposed in the resolved model value. * * @param pi - The ExtensionAPI instance */ export function registerModelDisplay(pi: ExtensionAPI): void { - // Track whether a model is selected (used for the (pending) fallback state) - let selectedModel: string | null = null; - - // Track the resolved model from the last proxy response - let resolvedModel: string | null = null; - - // Update the status display - function updateStatus(ctx: { ui: { setStatus: (key: string, value: string) => void } }) { - if (resolvedModel) { - ctx.ui.setStatus(MODEL_DISPLAY_STATUS_KEY, resolvedModel); - } else if (selectedModel) { - ctx.ui.setStatus(MODEL_DISPLAY_STATUS_KEY, '(pending)'); + // Capture initial Pi model alias from session context. + // This runs before the footer is set up, ensuring getSelectedModel() + // returns the alias even when no model_select event has fired yet. + // Only populates if model_select hasn't already set the value. + pi.on('session_start', (_event, ctx) => { + if (ctx.model?.id && !_selectedModel) { + _selectedModel = ctx.model.id; + _onModelChange?.(); } - // If neither resolvedModel nor selectedModel, no status call (nothing to display) - } + }); - // Track model selection changes (used only for (pending) fallback) - pi.on('model_select', (_event, ctx) => { + // Track model selection changes + pi.on('model_select', (_event, _ctx) => { // event.model has provider and id fields const model = _event.model; if (model && model.id) { - selectedModel = model.id; + _selectedModel = model.id; + _onModelChange?.(); + } else if (_selectedModel !== null) { + _selectedModel = null; + _onModelChange?.(); } - updateStatus(ctx); }); // Extract resolved model from proxy response headers - pi.on('after_provider_response', (event, ctx) => { + pi.on('after_provider_response', (event, _ctx) => { // event.headers contains the normalized response headers // X-Resolved-Model header is lowercase in normalized form const headers = event.headers as Record; const headerValue: string | undefined = headers['x-resolved-model'] ?? headers['X-Resolved-Model']; - if (headerValue) { - resolvedModel = headerValue; + if (headerValue !== undefined && headerValue !== _resolvedModel) { + _resolvedModel = headerValue; + _onModelChange?.(); } - updateStatus(ctx); }); } diff --git a/packages/tui/extensions/Worklog/session-health.test.ts b/packages/tui/extensions/Worklog/session-health.test.ts new file mode 100644 index 00000000..5d306cc6 --- /dev/null +++ b/packages/tui/extensions/Worklog/session-health.test.ts @@ -0,0 +1,1214 @@ +/** + * Unit tests for session-health.ts — Session health footer for the Worklog + * Pi extension. + * + * Verifies: + * 1. registerSessionHealth() sets up event listeners for relevant lifecycle + * events (session_start, turn_start, message_end, tool_execution_start, + * tool_execution_end, model_select, session_shutdown). + * 2. The footer renders correctly for all status states (idle, streaming, + * tool execution). + * 3. Elapsed time is colour-coded correctly (green <5s, yellow 5-15s, + * orange 15-30s, red >30s). + * 4. Token counts are formatted with k suffixes. + * 5. Context usage is formatted correctly. + * 6. The module exports the SESSION_HEALTH_STATUS_KEY constant. + * 7. Token extraction from session entries works correctly. + * 8. Total session time is displayed in the center section via + * formatTotalSessionTime(). + * 9. Elapsed time since last response is displayed in the left section, + * after the state marker. + * + * Run: npx vitest run packages/tui/extensions/Worklog/session-health.test.ts + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +// ── Mock @earendil-works/pi-coding-agent ────────────────────────────── + +vi.mock('@earendil-works/pi-coding-agent', () => ({ + getAgentDir: () => '/home/test-user/.pi/agent', +})); + +// ── Mock @earendil-works/pi-tui ─────────────────────────────────────── + +vi.mock('@earendil-works/pi-tui', () => ({ + truncateToWidth: (text: string, width: number) => text.slice(0, width), + visibleWidth: (text: string) => text.length, +})); + +// ── Mock ./model-display.js ──────────────────────────────────────────── +// Used by the footer factory to read the current resolved/selected model. +// Tests for the model line control the return values to verify correct +// rendering of the third footer line. + +const mocks = vi.hoisted(() => ({ + mockGetResolvedModel: vi.fn(() => null), + mockGetSelectedModel: vi.fn(() => null), + mockOnModelChange: vi.fn(() => () => {}), +})); + +vi.mock('./model-display.js', () => ({ + getResolvedModel: mocks.mockGetResolvedModel, + getSelectedModel: mocks.mockGetSelectedModel, + onModelChange: mocks.mockOnModelChange, + _resetModelDisplayState: vi.fn(), + MODEL_DISPLAY_STATUS_KEY: 'worklog-0model', +})); + +// ── Module under test ───────────────────────────────────────────────── + +import { + registerSessionHealth, + SESSION_HEALTH_STATUS_KEY, + STATUS_IDLE, + STATUS_STREAMING, + STATUS_TOOL, + getElapsedTime, + formatElapsedTime, + formatShortElapsedTime, + getTimeColor, + formatTokens, + formatContextUsage, + formatTotalSessionTime, + extractTokenUsage, + extractInitialPrompt, + renderFooter, + type SessionHealthState, +} from './session-health.js'; + +describe('session-health', () => { + /** Tracks event listeners registered by registerSessionHealth. */ + const registeredListeners: Record = {}; + /** Tracks footer set calls. */ + const footerCalls: Array<{ factory: Function }> = []; + /** Tracks status bar calls. */ + const statusCalls: Array<{ key: string; value: string | undefined }> = []; + + /** Mock ExtensionAPI with on(). */ + let mockPi: any; + /** Mock context with ui and model. */ + let mockCtx: any; + + /** Helper to get registered handlers for an event. */ + function getHandler(event: string): Function | undefined { + const handlers = registeredListeners[event]; + return handlers?.[handlers.length - 1]; // Return the last registered handler + } + + beforeEach(() => { + // Reset tracking + Object.keys(registeredListeners).forEach(k => delete registeredListeners[k]); + footerCalls.length = 0; + statusCalls.length = 0; + + // Mock extension context + mockCtx = { + ui: { + setStatus: vi.fn((key: string, value: string | undefined) => { + statusCalls.push({ key, value }); + }), + setFooter: vi.fn((factory: Function | undefined) => { + if (factory) { + footerCalls.push({ factory }); + } + }), + theme: { + fg: vi.fn((color: string, text: string) => `[${color}${text}]`), + }, + }, + mode: 'tui' as const, + model: { id: 'gpt-4' }, + sessionManager: { + getBranch: vi.fn(() => []), + }, + getContextUsage: vi.fn(() => ({ + tokens: 50000, + contextWindow: 128000, + percent: 39.1, + })), + }; + + // Mock Pi API + mockPi = { + on: vi.fn((event: string, handler: Function) => { + if (!registeredListeners[event]) { + registeredListeners[event] = []; + } + registeredListeners[event].push(handler); + }), + }; + }); + + // ── Exports ───────────────────────────────────────────────────────── + + it('exports SESSION_HEALTH_STATUS_KEY', () => { + expect(SESSION_HEALTH_STATUS_KEY).toBe('worklog-session-health'); + }); + + it('exports status constants (idle, streaming, tool)', () => { + expect(STATUS_IDLE).toBe('○'); + expect(STATUS_STREAMING).toBe('●'); + expect(STATUS_TOOL).toBe('⚡'); + }); + + it('exports registerSessionHealth as a function', () => { + expect(typeof registerSessionHealth).toBe('function'); + }); + + // ── Event registration ────────────────────────────────────────────── + + it('registers session_start listener', () => { + registerSessionHealth(mockPi); + expect(registeredListeners['session_start']).toBeDefined(); + }); + + it('registers turn_start listener', () => { + registerSessionHealth(mockPi); + expect(registeredListeners['turn_start']).toBeDefined(); + }); + + it('registers message_end listener', () => { + registerSessionHealth(mockPi); + expect(registeredListeners['message_end']).toBeDefined(); + }); + + it('registers tool_execution_start listener', () => { + registerSessionHealth(mockPi); + expect(registeredListeners['tool_execution_start']).toBeDefined(); + }); + + it('registers tool_execution_end listener', () => { + registerSessionHealth(mockPi); + expect(registeredListeners['tool_execution_end']).toBeDefined(); + }); + + it('registers model_select listener', () => { + registerSessionHealth(mockPi); + expect(registeredListeners['model_select']).toBeDefined(); + }); + + it('registers session_shutdown listener', () => { + registerSessionHealth(mockPi); + expect(registeredListeners['session_shutdown']).toBeDefined(); + }); + + // ── Time formatting ───────────────────────────────────────────────── + + describe('getElapsedTime', () => { + it('returns Infinity for null timestamp', () => { + expect(getElapsedTime(null)).toBe(Infinity); + }); + + it('returns 0 for future timestamp', () => { + expect(getElapsedTime(Date.now() + 1000)).toBe(0); + }); + + it('returns elapsed seconds for past timestamp', () => { + const past = Date.now() - 5000; + const elapsed = getElapsedTime(past); + expect(elapsed).toBeGreaterThanOrEqual(4.9); + expect(elapsed).toBeLessThanOrEqual(5.1); + }); + }); + + describe('formatElapsedTime', () => { + it('returns "—" for Infinity', () => { + expect(formatElapsedTime(Infinity)).toBe('—'); + }); + + it('returns "—" for negative', () => { + expect(formatElapsedTime(-1)).toBe('—'); + }); + + it('formats seconds < 60', () => { + expect(formatElapsedTime(3.5)).toBe('4s'); + expect(formatElapsedTime(1.1)).toBe('1s'); + expect(formatElapsedTime(0.5)).toBe('1s'); + }); + + it('formats minutes and seconds', () => { + expect(formatElapsedTime(65)).toBe('1m 5s'); + expect(formatElapsedTime(120)).toBe('2m'); + expect(formatElapsedTime(90)).toBe('1m 30s'); + }); + }); + + describe('formatShortElapsedTime', () => { + it('returns "—" for Infinity', () => { + expect(formatShortElapsedTime(Infinity)).toBe('—'); + }); + + it('returns "—" for negative', () => { + expect(formatShortElapsedTime(-1)).toBe('—'); + }); + + it('formats seconds < 60', () => { + expect(formatShortElapsedTime(3.5)).toBe('4s ago'); + expect(formatShortElapsedTime(1.1)).toBe('1s ago'); + expect(formatShortElapsedTime(0.5)).toBe('1s ago'); + }); + + it('formats minutes and seconds', () => { + expect(formatShortElapsedTime(65)).toBe('1m 5s ago'); + expect(formatShortElapsedTime(120)).toBe('2m ago'); + expect(formatShortElapsedTime(90)).toBe('1m 30s ago'); + }); + }); + + describe('getTimeColor', () => { + it('returns "dim" for Infinity or negative', () => { + expect(getTimeColor(Infinity)).toBe('dim'); + expect(getTimeColor(-1)).toBe('dim'); + }); + + it('returns "success" for <5s', () => { + expect(getTimeColor(0)).toBe('success'); + expect(getTimeColor(4)).toBe('success'); + expect(getTimeColor(4.9)).toBe('success'); + }); + + it('returns "warning" for 5-30s', () => { + expect(getTimeColor(5)).toBe('warning'); + expect(getTimeColor(10)).toBe('warning'); + expect(getTimeColor(15)).toBe('warning'); + expect(getTimeColor(20)).toBe('warning'); + expect(getTimeColor(30)).toBe('warning'); + }); + + it('returns "error" for >30s', () => { + expect(getTimeColor(30.1)).toBe('error'); + expect(getTimeColor(60)).toBe('error'); + }); + }); + + // ── Token formatting ──────────────────────────────────────────────── + + // ── Token formatting ──────────────────────────────────────────────── + + describe('formatTokens', () => { + it('returns raw number for <1000', () => { + expect(formatTokens(42)).toBe('42'); + expect(formatTokens(999)).toBe('999'); + }); + + it('returns k suffix for >=1000', () => { + expect(formatTokens(1000)).toBe('1.0k'); + expect(formatTokens(1500)).toBe('1.5k'); + expect(formatTokens(9999)).toBe('10.0k'); + }); + + it('returns M suffix for >=1M', () => { + expect(formatTokens(1_000_000)).toBe('1.0M'); + expect(formatTokens(2_500_000)).toBe('2.5M'); + }); + }); + + // ── Context usage formatting ──────────────────────────────────────── + + describe('formatContextUsage', () => { + it('returns "—/128k" when tokens is null', () => { + const result = formatContextUsage({ + tokens: null, + contextWindow: 128000, + percent: null, + }); + expect(result).toBe('—/128.0k'); + }); + + it('returns formatted percentage when tokens is set', () => { + const result = formatContextUsage({ + tokens: 50000, + contextWindow: 128000, + percent: 39.1, + }); + expect(result).toBe('39.1%/128.0k'); + }); + + it('handles large context windows', () => { + const result = formatContextUsage({ + tokens: 1000000, + contextWindow: 10000000, + percent: 10.0, + }); + expect(result).toBe('10.0%/10.0M'); + }); + }); + + // ── Initial prompt extraction ─────────────────────────────────────── + + describe('extractInitialPrompt', () => { + it('extracts first user message content', () => { + const entries = [ + { type: 'message', message: { role: 'user', content: 'Fix the bug by adding validation' } }, + ]; + const result = extractInitialPrompt(entries); + expect(result).toBe('Fix the bug by adding validation'); + }); + + it('returns first line of multi-line message', () => { + const entries = [ + { type: 'message', message: { role: 'user', content: 'First line\nSecond line\nThird line' } }, + ]; + const result = extractInitialPrompt(entries); + expect(result).toBe('First line'); + }); + + it('returns null when no user message found', () => { + const entries = [ + { type: 'message', message: { role: 'assistant', content: 'Hello!' } }, + ]; + const result = extractInitialPrompt(entries); + expect(result).toBeNull(); + }); + + it('returns null for empty entries', () => { + const result = extractInitialPrompt([]); + expect(result).toBeNull(); + }); + + it('ignores non-message entries', () => { + const entries = [ + { type: 'something_else', data: {} }, + { type: 'message', message: { role: 'user', content: 'Actual prompt' } }, + ]; + const result = extractInitialPrompt(entries); + expect(result).toBe('Actual prompt'); + }); + + it('returns null for empty content', () => { + const entries = [ + { type: 'message', message: { role: 'user', content: '' } }, + ]; + const result = extractInitialPrompt(entries); + expect(result).toBeNull(); + }); + + it('returns null for whitespace-only content', () => { + const entries = [ + { type: 'message', message: { role: 'user', content: ' ' } }, + ]; + const result = extractInitialPrompt(entries); + expect(result).toBeNull(); + }); + + // ── Array content format tests (Pi's actual format) ────────────── + + it('extracts first user message from array content format (Pi default)', () => { + const entries = [ + { type: 'message', message: { role: 'user', content: [{ type: 'text', text: 'Fix the bug by adding validation' }] } }, + ]; + const result = extractInitialPrompt(entries); + expect(result).toBe('Fix the bug by adding validation'); + }); + + it('returns first line of multi-line message in array format', () => { + const entries = [ + { type: 'message', message: { role: 'user', content: [{ type: 'text', text: 'First line\nSecond line\nThird line' }] } }, + ]; + const result = extractInitialPrompt(entries); + expect(result).toBe('First line'); + }); + + it('combines multiple text parts in array format', () => { + const entries = [ + { type: 'message', message: { role: 'user', content: [{ type: 'text', text: 'First part' }, { type: 'text', text: 'Second part' }] } }, + ]; + const result = extractInitialPrompt(entries); + expect(result).toBe('First part'); + }); + + it('returns null for empty array content', () => { + const entries = [ + { type: 'message', message: { role: 'user', content: [] } }, + ]; + const result = extractInitialPrompt(entries); + expect(result).toBeNull(); + }); + + it('ignores non-text parts in array content', () => { + const entries = [ + { type: 'message', message: { role: 'user', content: [{ type: 'image', data: 'base64...', mimeType: 'image/png' }] } }, + ]; + const result = extractInitialPrompt(entries); + expect(result).toBeNull(); + }); + + it('extracts first user message when content is in array format but earlier messages are assistant', () => { + const entries = [ + { type: 'message', message: { role: 'assistant', content: [{ type: 'text', text: 'How can I help?' }] } }, + { type: 'message', message: { role: 'user', content: [{ type: 'text', text: 'Fix the bug' }] } }, + ]; + const result = extractInitialPrompt(entries); + expect(result).toBe('Fix the bug'); + }); + + // ── Skill block tests ─────────────────────────────────────────── + + it('extracts skill name from expanded skill block', () => { + const entries = [ + { + type: 'message', + message: { + role: 'user', + content: [{ type: 'text', text: '\nReferences are relative to /home/user/.pi/agent/skills/audit/.\n\n# SKILL FILE CONTENT\n\n' }], + }, + }, + ]; + const result = extractInitialPrompt(entries); + expect(result).toBe('audit:'); + }); + + it('extracts skill name and args from expanded skill block with args', () => { + const entries = [ + { + type: 'message', + message: { + role: 'user', + content: [{ type: 'text', text: '\nReferences are relative to /home/user/.pi/agent/skills/audit/.\n\n# SKILL FILE CONTENT\n\n\n\nWL-123' }], + }, + }, + ]; + const result = extractInitialPrompt(entries); + expect(result).toBe('audit: WL-123'); + }); + + it('extracts skill name and multi-word args from expanded skill block', () => { + const entries = [ + { + type: 'message', + message: { + role: 'user', + content: [{ type: 'text', text: '\nReferences are relative to /home/user/.pi/agent/skills/implement/.\n\n[SKILL CONTENT]\n\n\n\nWL-123 implement the feature' }], + }, + }, + ]; + const result = extractInitialPrompt(entries); + expect(result).toBe('implement: WL-123 implement the feature'); + }); + + it('handles skill block in string format content (backward compat)', () => { + const entries = [ + { + type: 'message', + message: { + role: 'user', + content: '\n\n\nWL-456', + }, + }, + ]; + const result = extractInitialPrompt(entries); + expect(result).toBe('audit: WL-456'); + }); + + it('returns regular first line for non-skill content in array format', () => { + const entries = [ + { type: 'message', message: { role: 'user', content: [{ type: 'text', text: 'Fix the bug' }] } }, + ]; + const result = extractInitialPrompt(entries); + expect(result).toBe('Fix the bug'); + }); + }); + + // ── Token extraction ──────────────────────────────────────────────── + + describe('extractTokenUsage', () => { + it('returns zeros for empty entries', () => { + const result = extractTokenUsage([]); + expect(result).toEqual({ inputTokens: 0, outputTokens: 0 }); + }); + + it('extracts tokens from assistant messages', () => { + const entries = [ + { + type: 'message', + message: { role: 'assistant', usage: { input: 100, output: 200 } }, + }, + { + type: 'message', + message: { role: 'assistant', usage: { input: 150, output: 250 } }, + }, + ]; + const result = extractTokenUsage(entries); + expect(result).toEqual({ inputTokens: 250, outputTokens: 450 }); + }); + + it('ignores non-assistant messages', () => { + const entries = [ + { type: 'message', message: { role: 'user' } }, + { + type: 'message', + message: { role: 'assistant', usage: { input: 100, output: 200 } }, + }, + { type: 'message', message: { role: 'toolResult' } }, + ]; + const result = extractTokenUsage(entries); + expect(result).toEqual({ inputTokens: 100, outputTokens: 200 }); + }); + + it('handles missing usage gracefully', () => { + const entries = [ + { type: 'message', message: { role: 'assistant' } }, + { + type: 'message', + message: { role: 'assistant', usage: { input: 100, output: 200 } }, + }, + ]; + const result = extractTokenUsage(entries); + expect(result).toEqual({ inputTokens: 100, outputTokens: 200 }); + }); + }); + + // ── Footer rendering ──────────────────────────────────────────────── + + // ── Total session time formatting ───────────────────────────────── + + describe('formatTotalSessionTime', () => { + it('returns "Total: —" for Infinity', () => { + expect(formatTotalSessionTime(Infinity)).toBe('Total: —'); + }); + + it('returns "Total: —" for negative', () => { + expect(formatTotalSessionTime(-1)).toBe('Total: —'); + }); + + it('returns "Total: —" for 0', () => { + expect(formatTotalSessionTime(0)).toBe('Total: —'); + }); + + it('formats seconds-only duration', () => { + expect(formatTotalSessionTime(5)).toBe('Total: 5s'); + expect(formatTotalSessionTime(42)).toBe('Total: 42s'); + }); + + it('formats minutes and seconds', () => { + expect(formatTotalSessionTime(65)).toBe('Total: 1m 5s'); + expect(formatTotalSessionTime(90)).toBe('Total: 1m 30s'); + expect(formatTotalSessionTime(125)).toBe('Total: 2m 5s'); + }); + + it('formats whole minutes', () => { + expect(formatTotalSessionTime(60)).toBe('Total: 1m'); + expect(formatTotalSessionTime(120)).toBe('Total: 2m'); + expect(formatTotalSessionTime(3600)).toBe('Total: 60m'); + }); + }); + + describe('renderFooter', () => { + const mockTheme = { + fg: vi.fn((color: string, text: string) => `[${color}${text}]`), + }; + const mockCtx = { + model: { id: 'gpt-4' }, + }; + + it('renders idle status with elapsed time in left section and total in center', () => { + const state: SessionHealthState = { + status: 'idle', + toolName: null, + lastResponseTime: Date.now() - 2000, + lastChunkTime: null, + turnCount: 1, + inputTokens: 1000, + outputTokens: 2000, + contextUsage: { tokens: 50000, contextWindow: 128000, percent: 39.1 }, + sessionStartTime: Date.now() - 120000, // 2 minutes ago + }; + + const result = renderFooter(state, mockCtx, mockTheme as any, 500); + + // Left section: marker + elapsed time + turn count + expect(result).toContain('[dim○ Idle]'); + expect(result).toContain('[success2s]'); + expect(result).toContain('[dim#1]'); + + // Center section: total session time + expect(result).toContain('[mutedTotal: 2m]'); + }); + + it('renders streaming status with ● Streaming', () => { + const state: SessionHealthState = { + status: 'streaming', + toolName: null, + lastResponseTime: Date.now() - 3000, + lastChunkTime: Date.now() - 1000, + turnCount: 1, + inputTokens: 1000, + outputTokens: 0, + contextUsage: { tokens: null, contextWindow: 128000, percent: null }, + sessionStartTime: Date.now() - 300000, // 5 minutes ago + }; + + const result = renderFooter(state, mockCtx, mockTheme as any, 200); + + // Left section: marker + elapsed time + turn count + last chunk + expect(result).toContain('[dim● Streaming]'); + expect(result).toContain('[success3s]'); + expect(result).toContain('s ago'); + + // Right section: context (no tokens) + expect(result).toContain('[dim—/128.0k]'); + + // Center section: total session time + expect(result).toContain('[mutedTotal:'); + expect(result).toContain('5m]'); + }); + + it('renders tool status with ⚡ Tool: read', () => { + const state: SessionHealthState = { + status: 'tool', + toolName: 'read', + lastResponseTime: Date.now() - 1000, + lastChunkTime: null, + turnCount: 1, + inputTokens: 1000, + outputTokens: 0, + contextUsage: { tokens: null, contextWindow: 128000, percent: null }, + sessionStartTime: Date.now() - 60000, // 1 minute ago + }; + + const result = renderFooter(state, mockCtx, mockTheme as any, 120); + + expect(result).toContain('[dim⚡ Tool: read]'); + expect(result).toContain('[success1s]'); + }); + + it('handles missing sessionStartTime (no active session)', () => { + const state: SessionHealthState = { + status: 'idle', + toolName: null, + lastResponseTime: Date.now() - 2000, + lastChunkTime: null, + turnCount: 1, + inputTokens: 1000, + outputTokens: 2000, + contextUsage: { tokens: 50000, contextWindow: 128000, percent: 39.1 }, + sessionStartTime: null, + }; + + const result = renderFooter(state, { model: null } as any, mockTheme as any, 500); + + // Left section: marker + elapsed time + turn count + expect(result).toContain('[dim○ Idle]'); + expect(result).toContain('[success2s]'); + expect(result).toContain('[dim#1]'); + + // Center section: Total: — when no session start time + expect(result).toContain('[mutedTotal: —]'); + }); + + it('truncates content for narrow terminals', () => { + const state: SessionHealthState = { + status: 'idle', + toolName: null, + lastResponseTime: Date.now() - 2000, + turnCount: 1, + inputTokens: 1000000, + outputTokens: 2000000, + contextUsage: { tokens: 500000, contextWindow: 10000000, percent: 5.0 }, + sessionStartTime: Date.now() - 600000, // 10 minutes ago + }; + + const result = renderFooter(state, mockCtx, mockTheme as any, 20); + + // Should not exceed width + expect(result.length).toBeLessThanOrEqual(20); + }); + }); + + // ── Integration tests ─────────────────────────────────────────────── + + describe('registerSessionHealth integration', () => { + it('sets up footer on session_start', () => { + registerSessionHealth(mockPi); + + // Get the session_start handler + const handler = getHandler('session_start'); + expect(handler).toBeDefined(); + + // Call the handler with the mock context + handler({}, mockCtx); + + // setFooter should have been called + expect(mockCtx.ui.setFooter).toHaveBeenCalled(); + }); + + it('increments turnCount on turn_start', () => { + // We can't easily test the internal state, but we can verify the + // handler is registered and doesn't crash + registerSessionHealth(mockPi); + + const handler = getHandler('turn_start'); + expect(handler).toBeDefined(); + + expect(() => { + handler({}, mockCtx); + }).not.toThrow(); + }); + + it('handles message_end for assistant messages', () => { + registerSessionHealth(mockPi); + + const handler = getHandler('message_end'); + expect(handler).toBeDefined(); + + // Assistant message + expect(() => { + handler({ message: { role: 'assistant' } }, mockCtx); + }).not.toThrow(); + + // User message (should be ignored) + expect(() => { + handler({ message: { role: 'user' } }, mockCtx); + }).not.toThrow(); + }); + + it('handles tool_execution_start', () => { + registerSessionHealth(mockPi); + + const handler = getHandler('tool_execution_start'); + expect(handler).toBeDefined(); + + expect(() => { + handler({ toolName: 'read' }, mockCtx); + }).not.toThrow(); + }); + + it('handles tool_execution_end', () => { + registerSessionHealth(mockPi); + + const handler = getHandler('tool_execution_end'); + expect(handler).toBeDefined(); + + expect(() => { + handler({ toolName: 'read' }, mockCtx); + }).not.toThrow(); + }); + + it('handles model_select', () => { + registerSessionHealth(mockPi); + + const handler = getHandler('model_select'); + expect(handler).toBeDefined(); + + expect(() => { + handler({ model: { id: 'gpt-4' } }, mockCtx); + }).not.toThrow(); + }); + + it('handles session_shutdown (cleans up ticker)', () => { + registerSessionHealth(mockPi); + + const handler = getHandler('session_shutdown'); + expect(handler).toBeDefined(); + + expect(() => { + handler({}, mockCtx); + }).not.toThrow(); + }); + }); + + // ── message_update / lastChunkTime ──────────────────────────────────── + + it('registers message_update listener', () => { + registerSessionHealth(mockPi); + expect(registeredListeners['message_update']).toBeDefined(); + }); + + it('updates lastChunkTime on message_update', () => { + // We use a closure to capture the internal state + let capturedState: SessionHealthState | undefined; + const patchedMockCtx = { ...mockCtx, ui: { ...mockCtx.ui, setFooter: vi.fn() } }; + const origOn = mockPi.on; + + // Patch the register to capture state changes + registerSessionHealth(mockPi); + + const handler = getHandler('message_update'); + expect(handler).toBeDefined(); + + // Call message_update handler + const beforeCall = Date.now(); + handler({}, patchedMockCtx); + const afterCall = Date.now(); + + // The handler should have updated lastChunkTime + // We verify by checking the handler doesn't throw and that it calls updateState + // Since we can't directly inspect the captured state, we verify the + // handler exists and is callable (the real integration test verifies state) + expect(() => { + handler({ message: { role: 'assistant' } }, patchedMockCtx); + }).not.toThrow(); + }); + + it('does NOT update lastChunkTime on turn_start', () => { + registerSessionHealth(mockPi); + const handler = getHandler('turn_start'); + expect(handler).toBeDefined(); + // turn_start should not call updateState for lastChunkTime + expect(() => { + handler({}, mockCtx); + }).not.toThrow(); + }); + + it('does NOT update lastChunkTime on message_end', () => { + registerSessionHealth(mockPi); + const handler = getHandler('message_end'); + expect(handler).toBeDefined(); + expect(() => { + handler({ message: { role: 'assistant' } }, mockCtx); + }).not.toThrow(); + }); + + // ── Three-section footer layout ─────────────────────────────────────── + + describe('renderFooter — three-section layout (restructured)', () => { + const mockTheme = { + fg: vi.fn((color: string, text: string) => `[${color}${text}]`), + }; + const mockCtx = { + model: { id: 'gpt-4' }, + }; + + it('shows elapsed time since last chunk only during streaming', () => { + // Streaming state — should include last chunk info + const streamingState: SessionHealthState = { + status: 'streaming', + toolName: null, + lastResponseTime: Date.now() - 5000, + lastChunkTime: Date.now() - 2000, + turnCount: 3, + inputTokens: 1000, + outputTokens: 500, + contextUsage: { tokens: 50000, contextWindow: 128000, percent: 39.1 }, + sessionStartTime: Date.now() - 300000, // 5 minutes ago + }; + + const result = renderFooter(streamingState, mockCtx, mockTheme as any, 120); + + expect(result).toContain('s ago'); + }); + + it('does NOT show elapsed time since last chunk when idle', () => { + const idleState: SessionHealthState = { + status: 'idle', + toolName: null, + lastResponseTime: Date.now() - 2000, + lastChunkTime: null, + turnCount: 1, + inputTokens: 1000, + outputTokens: 2000, + contextUsage: { tokens: 50000, contextWindow: 128000, percent: 39.1 }, + sessionStartTime: Date.now() - 120000, // 2 minutes ago + }; + + const result = renderFooter(idleState, mockCtx, mockTheme as any, 120); + + expect(result).not.toContain('s ago'); + }); + + it('does NOT show elapsed time since last chunk when tool executing', () => { + const toolState: SessionHealthState = { + status: 'tool', + toolName: 'read', + lastResponseTime: Date.now() - 1000, + lastChunkTime: null, + turnCount: 1, + inputTokens: 1000, + outputTokens: 0, + contextUsage: { tokens: null, contextWindow: 128000, percent: null }, + sessionStartTime: Date.now() - 60000, // 1 minute ago + }; + + const result = renderFooter(toolState, mockCtx, mockTheme as any, 120); + + expect(result).not.toContain('s ago'); + }); + + it('shows total session time in center section', () => { + const state: SessionHealthState = { + status: 'streaming', + toolName: null, + lastResponseTime: Date.now() - 45000, // 45 seconds ago + lastChunkTime: Date.now() - 1000, + turnCount: 5, + inputTokens: 1000, + outputTokens: 2000, + contextUsage: { tokens: 50000, contextWindow: 128000, percent: 39.1 }, + sessionStartTime: Date.now() - 600000, // 10 minutes ago + }; + + const result = renderFooter(state, mockCtx, mockTheme as any, 200); + + // Center section should show total session time + expect(result).toContain('[mutedTotal: 10m]'); + // Left section should show elapsed time since last response (color-coded) + expect(result).toContain('[error45s]'); + }); + + it('places left, center, and right sections in correct order', () => { + const state: SessionHealthState = { + status: 'streaming', + toolName: null, + lastResponseTime: Date.now() - 10000, + lastChunkTime: Date.now() - 3000, + turnCount: 2, + inputTokens: 500, + outputTokens: 1000, + contextUsage: { tokens: 25000, contextWindow: 128000, percent: 19.5 }, + sessionStartTime: Date.now() - 600000, // 10 minutes ago + }; + + const result = renderFooter(state, mockCtx, mockTheme as any, 500); + + // Left section: marker + elapsed time + turn count + last chunk elapsed time + expect(result).toContain('[dim● Streaming]'); + expect(result).toContain('[warning10s]'); + expect(result).toContain('[dim#2]'); + expect(result).toContain('s ago'); + // Center section: total session time (not elapsed time) + expect(result).toContain('[mutedTotal:'); + expect(result).toContain('[mutedTotal: 10m]'); + // Right section: tokens + expect(result).toContain('[muted↑500 ↓1.0k]'); + // Right section: context + expect(result).toContain('[dim19.5%/128.0k]'); + }); + + it('renders footer with wide terminal (no truncation)', () => { + const state: SessionHealthState = { + status: 'streaming', + toolName: null, + lastResponseTime: Date.now() - 65000, + lastChunkTime: Date.now() - 5000, + turnCount: 10, + inputTokens: 50000, + outputTokens: 100000, + contextUsage: { tokens: 64000, contextWindow: 128000, percent: 50.0 }, + sessionStartTime: Date.now() - 7200000, // 2 hours ago + }; + + const result = renderFooter(state, mockCtx, mockTheme as any, 400); + + expect(result).toContain('s ago'); + // 65s = 1m 5s, and > 30s so error color — now in LEFT section + expect(result).toContain('[error1m 5s]'); + // Center section: total session time + expect(result).toContain('[mutedTotal:'); + expect(result).toContain('Total: 120m'); + expect(result).toContain('[muted↑50.0k ↓100.0k]'); + expect(result).toContain('[dim50.0%/128.0k]'); + }); + }); + + // ── Model line in footer (Line 3) ────────────────────────────────────── + + describe('model line in footer', () => { + /** + * Helper: invoke the footer factory and return its render(…) result. + * Simulates what Pi TUI does: calls setFooter(factory), then calls + * factory(tui, theme, footerData) → { render(width) } → render(lines). + * + * @param ctx - Extension context + * @param initialPrompt - Optional initial user message to inject into + * the session entries so the ticker's refreshState populates it. + */ + function fabricateFooterLines(ctx: any, initialPrompt?: string | null, width: number = 500): string[] { + registerSessionHealth(mockPi); + + // Override getBranch to return entries with the initial prompt + const sessionBranch = initialPrompt + ? [{ type: 'message', message: { role: 'user', content: initialPrompt } }] + : []; + const ctxWithPrompt = { + ...ctx, + sessionManager: { + getBranch: () => sessionBranch, + }, + }; + + // Start a session to trigger footer setup + const sessionStartHandler = getHandler('session_start'); + sessionStartHandler({}, ctxWithPrompt); + + // Capture the factory passed to setFooter + expect(footerCalls.length).toBeGreaterThanOrEqual(1); + const factory = footerCalls[footerCalls.length - 1].factory; + + // Call the factory to get the footer object + const footerObj = factory( + { requestRender: vi.fn() }, + { fg: (color: string, text: string) => `[${color}${text}]` }, + { + getExtensionStatuses: () => [], + onBranchChange: () => () => {}, + requestRender: vi.fn(), + }, + ); + + return footerObj.render(width); + } + + beforeEach(() => { + // Reset mock return values before each test + mocks.mockGetResolvedModel.mockReturnValue(null); + mocks.mockGetSelectedModel.mockReturnValue(null); + mocks.mockOnModelChange.mockReturnValue(() => {}); + + // Clear footer calls before each test + footerCalls.length = 0; + }); + + it('shows selected alias and resolved model in grey on line 3 when both available', () => { + mocks.mockGetResolvedModel.mockReturnValue('openai/gpt-4'); + mocks.mockGetSelectedModel.mockReturnValue('code'); + + const lines = fabricateFooterLines({ ...mockCtx, mode: 'tui', ui: { ...mockCtx.ui, theme: { fg: vi.fn((c: string, t: string) => `[${c}${t}]`) } } }); + + // With no extension statuses: lines[0]=session health, lines[1]=model info + expect(lines.length).toBe(2); + // Format: "" + expect(lines[1]).toContain('code → openai/gpt-4'); + expect(lines[1]).toContain('[dim'); + }); + + it('shows just the selected alias when model selected but not yet resolved', () => { + mocks.mockGetResolvedModel.mockReturnValue(null); + mocks.mockGetSelectedModel.mockReturnValue('code'); + + const lines = fabricateFooterLines({ ...mockCtx, mode: 'tui', ui: { ...mockCtx.ui, theme: { fg: vi.fn((c: string, t: string) => `[${c}${t}]`) } } }); + + // With no extension statuses: lines[0]=session health, lines[1]=alias only + expect(lines.length).toBe(2); + expect(lines[1]).toContain('code'); + expect(lines[1]).toContain('[dim'); + }); + + it('shows a grey dash when no model selected and no resolved model', () => { + mocks.mockGetResolvedModel.mockReturnValue(null); + mocks.mockGetSelectedModel.mockReturnValue(null); + + const lines = fabricateFooterLines({ ...mockCtx, mode: 'tui', ui: { ...mockCtx.ui, theme: { fg: vi.fn((c: string, t: string) => `[${c}${t}]`) } } }); + + // With no extension statuses: lines[0]=session health, lines[1]=dash + expect(lines.length).toBe(2); + expect(lines[1]).toContain('—'); + expect(lines[1]).toContain('[dim'); + }); + + it('shows initial prompt preview alongside model info on line 3', () => { + mocks.mockGetResolvedModel.mockReturnValue('openai/gpt-4'); + mocks.mockGetSelectedModel.mockReturnValue('code'); + + const lines = fabricateFooterLines( + { ...mockCtx, mode: 'tui', ui: { ...mockCtx.ui, theme: { fg: vi.fn((c: string, t: string) => `[${c}${t}]`) } } }, + 'Fix the bug by adding validation', + ); + + expect(lines.length).toBe(2); + // Format: "" (no quotes) + expect(lines[1]).toContain('code → openai/gpt-4'); + expect(lines[1]).toContain('Fix the bug by adding validation'); + expect(lines[1]).not.toContain('"'); + expect(lines[1]).toContain('[dim'); + }); + + it('truncates long initial prompt preview', () => { + mocks.mockGetResolvedModel.mockReturnValue(null); + mocks.mockGetSelectedModel.mockReturnValue('code'); + + const longPrompt = 'Write a comprehensive test suite for the authentication module including all edge cases like token expiry and refresh'; + // Use a narrow terminal width (120) to trigger truncation + // With width=120, reserved=38, available=max(15, 120-38)=82, prompt length=151 > 82 + const lines = fabricateFooterLines( + { ...mockCtx, mode: 'tui', ui: { ...mockCtx.ui, theme: { fg: vi.fn((c: string, t: string) => `[${c}${t}]`) } } }, + longPrompt, + 120, + ); + + expect(lines.length).toBe(2); + // Should show alias + truncated prompt with ... + expect(lines[1]).toContain('code'); + expect(lines[1]).toContain('...'); + expect(lines[1]).toContain('[dim'); + }); + + it('shows just model info when no initial prompt available', () => { + mocks.mockGetResolvedModel.mockReturnValue('openai/gpt-4'); + mocks.mockGetSelectedModel.mockReturnValue('code'); + + const lines = fabricateFooterLines( + { ...mockCtx, mode: 'tui', ui: { ...mockCtx.ui, theme: { fg: vi.fn((c: string, t: string) => `[${c}${t}]`) } } }, + null, // no initial prompt + ); + + expect(lines.length).toBe(2); + expect(lines[1]).toContain('code → openai/gpt-4'); + // Should NOT contain a quoted prompt + expect(lines[1]).not.toContain('"'); + }); + + it('updates when after_provider_response fires (requestRender triggered)', () => { + const requestRender = vi.fn(); + registerSessionHealth(mockPi); + + // Fire all needed events to set up footer + const sessionStartHandler = getHandler('session_start'); + sessionStartHandler({}, { + ...mockCtx, + mode: 'tui', + ui: { + ...mockCtx.ui, + theme: { fg: vi.fn((c: string, t: string) => `[${c}${t}]`) }, + setFooter: vi.fn((factory: Function) => { + // Call factory to get dispose/invalidate/render + const result = factory( + { requestRender }, + { fg: (c: string, t: string) => `[${c}${t}]` }, + { + getExtensionStatuses: () => [], + onBranchChange: () => () => {}, + }, + ); + expect(typeof result.render).toBe('function'); + }), + }, + }); + + // Fire after_provider_response - the listener should call requestRender + const aprHandler = getHandler('after_provider_response'); + expect(aprHandler).toBeDefined(); + aprHandler({ headers: { 'x-resolved-model': 'anthropic/claude-sonnet-4' } }, {}); + + // requestRender should have been called by the after_provider_response handler + expect(requestRender).toHaveBeenCalled(); + }); + }); + + // ── Edge cases ────────────────────────────────────────────────────── + + describe('edge cases', () => { + it('formatElapsedTime handles edge cases', () => { + expect(formatElapsedTime(Infinity)).toBe('—'); + expect(formatElapsedTime(-1)).toBe('—'); + expect(formatElapsedTime(0)).toBe('0s'); + }); + + it('getTimeColor handles boundary values', () => { + expect(getTimeColor(4.99)).toBe('success'); + expect(getTimeColor(5.0)).toBe('warning'); + expect(getTimeColor(14.99)).toBe('warning'); + expect(getTimeColor(15.0)).toBe('warning'); + expect(getTimeColor(29.99)).toBe('warning'); + expect(getTimeColor(30.01)).toBe('error'); + }); + + it('formatTokens handles edge cases', () => { + expect(formatTokens(0)).toBe('0'); + expect(formatTokens(999)).toBe('999'); + expect(formatTokens(1000)).toBe('1.0k'); + expect(formatTokens(999999)).toBe('1000.0k'); + expect(formatTokens(1000000)).toBe('1.0M'); + }); + }); +}); diff --git a/packages/tui/extensions/Worklog/session-health.ts b/packages/tui/extensions/Worklog/session-health.ts new file mode 100644 index 00000000..48db6877 --- /dev/null +++ b/packages/tui/extensions/Worklog/session-health.ts @@ -0,0 +1,646 @@ +/** + * Session health extension for the Worklog Pi extension. + * + * Displays real-time session health metrics in the footer: + * - Status indicator (idle, streaming, tool execution) + * - Elapsed time since last response (colour-coded) + * - Token usage (input/output) + * - Context window usage percentage + * - Provider/model on a dedicated line + * - Turn count + * + * Uses `ctx.ui.setFooter()` to replace the default footer with a custom + * health display. Gracefully degrades in non-TUI modes. + */ + +import type { + ExtensionAPI, + ExtensionContext, +} from '@earendil-works/pi-coding-agent'; +import { detectWorkItemId } from './activity-indicator.js'; +import { truncateToTerminalWidth, truncateWorkItemId, visibleWidth } from './terminal-utils.js'; +import { getResolvedModel, getSelectedModel, onModelChange } from './model-display.js'; +import { runWl } from '../wl-integration.js'; + +// ── Status constants ────────────────────────────────────────────────────── + +/** Status key used for the session health footer. */ +export const SESSION_HEALTH_STATUS_KEY = 'worklog-session-health'; + +/** Status indicator for idle state. */ +export const STATUS_IDLE = '○'; + +/** Status indicator for streaming state. */ +export const STATUS_STREAMING = '●'; + +/** Status indicator for tool execution state. */ +export const STATUS_TOOL = '⚡'; + +/** + * Session health state. + * + * - `status`: Current session status (idle, streaming, or tool execution) + * - `toolName`: Name of the currently executing tool (when status === 'tool') + * - `lastResponseTime`: Timestamp of the last model response (updated at + * `turn_start` and `message_end`; used for the center elapsed timer) + * - `lastChunkTime`: Timestamp of the last `message_update` event (updated + * only during active streaming; used for the "last chunk" timer in the + * left section) + * - `turnCount`: Number of turns in the current session + * - `inputTokens`: Cumulative input tokens + * - `outputTokens`: Cumulative output tokens + * - `contextUsage`: Context usage info from ctx.getContextUsage() + */ +export interface SessionHealthState { + status: 'idle' | 'streaming' | 'tool'; + toolName: string | null; + lastResponseTime: number | null; + lastChunkTime: number | null; + turnCount: number; + inputTokens: number; + outputTokens: number; + contextUsage: { tokens: number | null; contextWindow: number; percent: number | null }; + /** First few characters of the session's initial user message. */ + initialPrompt: string | null; + /** Wall-clock timestamp of when the current session started (Date.now()). */ + sessionStartTime: number | null; +} + +/** Default initial state. */ +const DEFAULT_STATE: SessionHealthState = { + status: 'idle', + toolName: null, + lastResponseTime: null, + lastChunkTime: null, + turnCount: 0, + inputTokens: 0, + outputTokens: 0, + contextUsage: { tokens: null, contextWindow: 128000, percent: null }, + initialPrompt: null, + sessionStartTime: null, +}; + +/** Ticker interval in milliseconds (1 second). */ +const TICK_INTERVAL_MS = 1000; + +/** + * Get elapsed time since a timestamp. + * + * @param timestamp - Timestamp in milliseconds + * @returns Elapsed time in seconds (0 if timestamp is null/old) + */ +export function getElapsedTime(timestamp: number | null): number { + if (!timestamp) return Infinity; + return Math.max(0, (Date.now() - timestamp) / 1000); +} + +/** + * Format elapsed time for display. + * + * @param seconds - Elapsed time in seconds + * @returns Formatted string (e.g., "3s", "1m 30s", "2m") + */ +export function formatElapsedTime(seconds: number): string { + if (seconds === Infinity || seconds < 0) return '—'; + if (seconds < 10) return `${Math.round(seconds)}s`; + if (seconds < 60) return `${Math.round(seconds)}s`; + const mins = Math.floor(seconds / 60); + const secs = Math.round(seconds % 60); + return secs > 0 ? `${mins}m ${secs}s` : `${mins}m`; +} + +/** + * Format total session duration for display in the center section. + * + * Produces "Total: 5m 42s" or "Total: —" when no session has started. + * + * @param seconds - Total session duration in seconds (pass 0 or Infinity for no session) + * @returns Formatted string (e.g., "Total: 5m 42s", "Total: —") + */ +export function formatTotalSessionTime(seconds: number): string { + if (seconds === Infinity || seconds < 0 || seconds === 0) return 'Total: —'; + const mins = Math.floor(seconds / 60); + const secs = Math.round(seconds % 60); + if (mins > 0 && secs > 0) return `Total: ${mins}m ${secs}s`; + if (mins > 0) return `Total: ${mins}m`; + return `Total: ${Math.round(seconds)}s`; +} + +/** + * Format a short "Xs ago" string for the last-chunk indicator. + * + * Used only during active streaming to show how long it has been since the + * last streaming chunk arrived. + * + * @param seconds - Elapsed time in seconds + * @returns Formatted string (e.g., "2s ago", "1m 5s ago") + */ +export function formatShortElapsedTime(seconds: number): string { + if (seconds === Infinity || seconds < 0) return '—'; + if (seconds < 10) return `${Math.round(seconds)}s ago`; + if (seconds < 60) return `${Math.round(seconds)}s ago`; + const mins = Math.floor(seconds / 60); + const secs = Math.round(seconds % 60); + return secs > 0 ? `${mins}m ${secs}s ago` : `${mins}m ago`; +} + +/** + * Get color for elapsed time based on thresholds. + * + * - Green (<5s): good response time + * - Yellow (5-15s): moderate delay + * - Orange (15-30s): significant delay + * - Red (>30s): potentially stuck + * + * @param seconds - Elapsed time in seconds + * @returns Theme color name + */ +export function getTimeColor(seconds: number): string { + if (seconds === Infinity || seconds < 0) return 'dim'; + if (seconds < 5) return 'success'; + if (seconds <= 30) return 'warning'; + return 'error'; +} + +/** + * Format token count with k suffix for large numbers. + * + * @param tokens - Token count + * @returns Formatted string (e.g., "1.2k", "42") + */ +export function formatTokens(tokens: number): string { + if (tokens >= 1_000_000) return `${(tokens / 1_000_000).toFixed(1)}M`; + if (tokens >= 1_000) return `${(tokens / 1_000).toFixed(1)}k`; + return `${tokens}`; +} + +/** + * Format context usage as "X%/Yk" string. + * + * @param usage - Context usage info + * @returns Formatted string (e.g., "76.8%/128k", "—/128k") + */ +export function formatContextUsage(usage: SessionHealthState['contextUsage']): string { + if (usage.tokens === null || usage.percent === null) { + return `—/${formatTokens(usage.contextWindow)}`; + } + const windowStr = formatTokens(usage.contextWindow); + return `${usage.percent.toFixed(1)}%/${windowStr}`; +} + +/** + * Build the footer render string. + * + * Layout (three-section): + * Left: [marker] [elapsed since last response] [#turn] Xs ago + * Center: Total: [total session wall-clock duration] + * Right: ↑input ↓output [context%/window] + * + * The model/provider is displayed on a dedicated line below the session + * health line (see the `render()` function in `registerSessionHealth`). + * The elapsed time since last chunk (e.g. "4s ago") appears only when + * `status === 'streaming'`. + * + * @param state - Session health state + * @param ctx - Extension context with UI and model access + * @param theme - Theme function for styling + * @param width - Available terminal width + * @returns Rendered footer string + */ +export function renderFooter( + state: SessionHealthState, + ctx: ExtensionContext, + theme: { fg: (color: string, text: string) => string }, + width: number, +): string { + // ── Left section ──────────────────────────────────────────────────── + // Status marker with label followed by elapsed time since last response + let marker = ''; + switch (state.status) { + case 'idle': + marker = `${STATUS_IDLE} Idle`; + break; + case 'streaming': + marker = `${STATUS_STREAMING} Streaming`; + break; + case 'tool': + marker = `${STATUS_TOOL}${state.toolName ? ` Tool: ${state.toolName}` : ' Tool'}`; + break; + } + const markerStr = theme.fg('dim', marker); + + // Elapsed time since last response (color-coded, placed after state marker) + const elapsed = getElapsedTime(state.lastResponseTime); + const timeStr = formatElapsedTime(elapsed); + const timeColor = getTimeColor(elapsed); + const timeStrStyled = theme.fg(timeColor, timeStr); + + // Turn count + const turnStr = theme.fg('dim', `#${state.turnCount}`); + + // Last chunk indicator (streaming only) + let lastChunkStr = ''; + if (state.status === 'streaming' && state.lastChunkTime !== null) { + const chunkElapsed = getElapsedTime(state.lastChunkTime); + const lastChunkElapsed = formatShortElapsedTime(chunkElapsed); + lastChunkStr = theme.fg('dim', lastChunkElapsed); + } + + const left = `${markerStr} ${timeStrStyled} ${turnStr} ${lastChunkStr}`; + + // ── Center section ────────────────────────────────────────────────── + // Total session wall-clock duration (computed from sessionStartTime) + const totalSeconds = state.sessionStartTime + ? Math.max(0, (Date.now() - state.sessionStartTime) / 1000) + : Infinity; + const totalStr = formatTotalSessionTime(totalSeconds); + const totalStrStyled = theme.fg('muted', totalStr); + + // ── Right section ─────────────────────────────────────────────────── + // Token counts + const tokensStr = `↑${formatTokens(state.inputTokens)} ↓${formatTokens(state.outputTokens)}`; + const tokensStrStyled = theme.fg('muted', tokensStr); + + // Context usage + const contextStr = formatContextUsage(state.contextUsage); + const contextStrStyled = theme.fg('dim', contextStr); + + // Model ID is now displayed on its own line below the session health line + let right = `${tokensStrStyled} ${contextStrStyled}`; + + // ── Layout ────────────────────────────────────────────────────────── + // Calculate visible widths and pad + const leftWidth = visibleWidth(left); + const centerWidth = visibleWidth(totalStrStyled); + const rightWidth = visibleWidth(right); + // Account for the space between center and right sections + const hasSeparatorSpace = rightWidth > 0 ? 1 : 0; + const totalContentWidth = leftWidth + centerWidth + rightWidth + hasSeparatorSpace; + + if (totalContentWidth >= width) { + // Not enough space: truncate right + const maxRight = Math.max(0, width - leftWidth - centerWidth - 1); + right = truncateToTerminalWidth(right, maxRight, { ellipsis: '…' }); + } + + const padding = ' '.repeat(Math.max(0, width - totalContentWidth)); + return truncateToTerminalWidth(left + padding + totalStrStyled + ' ' + right, width); +} + +/** + * Extract the first user message content from session entries. + * Returns the first line of the initial user message, or null if none found. + * + * Handles: + * - String content (legacy format) + * - Array content (Pi's default format: [{ type: "text", text: "..." }, ...]) + * - Skill expansion blocks (...) — returns a compact + * representation like "[skill:audit] WL-123" instead of the raw XML first line + * + * Text extraction matches the approach used by Pi's own `extractTextContent` + * in session-manager.js and `_getUserMessageText` in agent-session.js. + * + * @param entries - Session entries from ctx.sessionManager.getBranch() + * @returns Initial user prompt text, or null + */ +export function extractInitialPrompt( + entries: Array<{ type: string; message?: any }>, +): string | null { + for (const entry of entries) { + if (entry.type === 'message' && entry.message?.role === 'user') { + const content = entry.message.content; + const text = extractMessageText(content); + if (!text) continue; + + // Check for skill expansion block: ... + // Pi expands /skill:name into a wrapping XML block before storing + const skillBlockMatch = text.match(/^ tag + const afterSkill = text.match(/<\/skill>\s*\n?\s*([\s\S]*)$/); + const args = afterSkill ? afterSkill[1].trim() : ''; + return args ? `${skillName}: ${args}` : `${skillName}:`; + } + + // Regular content: return first line + return text.split('\n')[0].trim(); + } + } + return null; +} + +/** + * Extract text content from a message content field, which may be either + * a string (legacy format) or an array of TextContent/ImageContent parts + * (Pi's default format). + * + * Matches the approach used by Pi's own `extractTextContent` in + * session-manager.js and `_getUserMessageText` in agent-session.js. + * + * @param content - The message.content field (string or content part array) + * @returns The extracted text, or null if the content is empty or non-text + */ +function extractMessageText(content: any): string | null { + if (typeof content === 'string') { + return content.trim() || null; + } + if (Array.isArray(content)) { + const textParts: string[] = []; + for (const part of content) { + if (part.type === 'text' && typeof part.text === 'string') { + textParts.push(part.text); + } + } + const combined = textParts.join('\n').trim(); + return combined || null; + } + return null; +} + +/** + * Extract token usage from session entries. + * + * Iterates through session entries to sum up input/output tokens + * from assistant messages. + * + * @param entries - Session entries from ctx.sessionManager.getBranch() + * @returns { inputTokens, outputTokens } + */ +export function extractTokenUsage( + entries: Array<{ type: string; message?: any }>, +): { inputTokens: number; outputTokens: number } { + let inputTokens = 0; + let outputTokens = 0; + + for (const entry of entries) { + if (entry.type === 'message' && entry.message?.role === 'assistant') { + const usage = entry.message.usage; + if (usage) { + inputTokens += usage.input || 0; + outputTokens += usage.output || 0; + } + } + } + + return { inputTokens, outputTokens }; +} + +/** + * Register session health event handlers with a Pi extension instance. + * + * Subscribes to lifecycle events to track: + * - Session turns and status changes + * - Model selection changes + * - Tool execution lifecycle + * + * The footer renderer updates every second via setInterval and on state + * changes via tui.requestRender(). + * + * @param pi - The ExtensionAPI instance + */ +export function registerSessionHealth(pi: ExtensionAPI): void { + // ── State ─────────────────────────────────────────────────────────── + let state: SessionHealthState = { ...DEFAULT_STATE }; + let tickerInterval: ReturnType | null = null; + let requestRender: (() => void) | null = null; + + /** + * Update state and request a footer re-render. + */ + function updateState( + _event: any, + updates: Partial, + ): void { + state = { ...state, ...updates }; + if (requestRender) { + requestRender(); + } + } + + // ── Event handlers ────────────────────────────────────────────────── + + // Track turns + pi.on('turn_start', (_event) => { + state.turnCount += 1; + state.status = 'streaming'; + state.lastResponseTime = Date.now(); + updateState(_event, {}); + }); + + // Track streaming chunks — updates lastChunkTime only on message_update + pi.on('message_update', (_event) => { + state.lastChunkTime = Date.now(); + updateState(_event, {}); + }); + + pi.on('message_end', (event) => { + if (event.message?.role === 'assistant') { + // Reset to idle after assistant message ends + state.status = 'idle'; + state.lastResponseTime = Date.now(); + updateState(event, {}); + } + }); + + // Tool execution lifecycle + pi.on('tool_execution_start', (event) => { + state.status = 'tool'; + state.toolName = event.toolName ?? null; + updateState(event, {}); + }); + + pi.on('tool_execution_end', (event) => { + // Only reset to idle if we're not currently streaming + if (state.status === 'tool') { + state.status = 'idle'; + state.toolName = null; + updateState(event, {}); + } + }); + + // Model selection + pi.on('model_select', (event) => { + updateState(event, {}); + }); + + // Session start — reset counters and record start time + pi.on('session_start', (_event, ctx) => { + state = { ...DEFAULT_STATE, contextUsage: state.contextUsage, sessionStartTime: Date.now() }; + updateState(_event, {}); + + // Set the footer and start the ticker on first session start + setFooter(ctx); + startTicker(ctx); + }); + + // Listen for model/provider changes and trigger a footer re-render + pi.on('after_provider_response', (_event) => { + if (requestRender) requestRender(); + }); + + // Session shutdown — clean up ticker + pi.on('session_shutdown', () => { + if (tickerInterval) { + clearInterval(tickerInterval); + tickerInterval = null; + } + }); + + // ── Ticker ────────────────────────────────────────────────────────── + function startTicker(ctx: ExtensionContext): void { + if (tickerInterval) return; // Already running + + // Refresh state from session + function refreshState(): void { + try { + const entries = ctx.sessionManager.getBranch(); + const tokens = extractTokenUsage(entries); + state.inputTokens = tokens.inputTokens; + state.outputTokens = tokens.outputTokens; + state.contextUsage = ctx.getContextUsage() ?? { + tokens: null, + contextWindow: 128000, + percent: null, + }; + // Capture the initial prompt on first encounter (it may be set + // on the very first tick after session_start) + if (!state.initialPrompt) { + state.initialPrompt = extractInitialPrompt(entries); + + // Fire-and-forget: resolve any work item ID to its title + // so the footer shows "WL-123 Title" instead of just "WL-123" + if (state.initialPrompt) { + const wlId = detectWorkItemId(state.initialPrompt); + if (wlId) { + runWl('show', [wlId], { timeout: 2000 }) + .then(result => { + if (result?.title && state.initialPrompt) { + const enriched = `${wlId} ${result.title}`; + state.initialPrompt = state.initialPrompt.replace( + wlId, + enriched, + ); + requestRender?.(); + } + }) + .catch(() => { + // Silently ignore — raw ID is already shown + }); + } + } + + requestRender?.(); + } + } catch { + // Best-effort: if session manager unavailable, keep current state + } + } + + refreshState(); // Initial refresh + tickerInterval = setInterval(refreshState, TICK_INTERVAL_MS); + } + + // ── Set footer ────────────────────────────────────────────────────── + function setFooter(ctx: ExtensionContext): void { + // Gracefully degrade in non-TUI modes + if (ctx.mode !== 'tui') return; + if (typeof ctx.ui.setFooter !== 'function') return; + + const theme = ctx.ui.theme; + if (!theme?.fg) return; + + ctx.ui.setFooter((tui, _theme, footerData) => { + // Store requestRender for use in event handlers + requestRender = () => tui.requestRender(); + + const disposeBranchChange = footerData.onBranchChange(() => tui.requestRender()); + // Subscribe to model state changes for reactive footer updates + let disposeModelChange: (() => void) | null = null; + return { + dispose() { + if (tickerInterval) { + clearInterval(tickerInterval); + tickerInterval = null; + } + if (disposeModelChange) { + disposeModelChange(); + disposeModelChange = null; + } + disposeBranchChange(); + }, + invalidate() { + // Theme changed — nothing special to do + }, + render(width: number): string[] { + const lines: string[] = []; + + // Subscribe to model changes on first render + if (!disposeModelChange) { + disposeModelChange = onModelChange(() => tui.requestRender()); + } + + // Line 1: Extension statuses (activity-indicator, etc.) + // These are set via ctx.ui.setStatus() and would be hidden when a + // custom footer is active. We include them here so that status + // entries remain visible. + // Note: The provider/model is no longer shown as a status entry; + // it is displayed on Line 3 below. + const statuses = footerData.getExtensionStatuses(); + if (statuses && statuses.length > 0) { + const statusLine = statuses.join(' '); + lines.push(truncateToTerminalWidth(theme.fg('muted', statusLine), width)); + } + + // Line 2: Session health + lines.push(renderFooter(state, ctx, theme, width)); + + // Line 3: Provider/model + initial prompt preview (grey/dim text) + // Shows the Pi model alias (e.g. "code", "plan") and, when available, + // the provider/model resolved by the router (e.g. "openai/gpt-4"). + // Also shows a preview of the first user message that started the + // session when available. Always visible. + const selectedModel = getSelectedModel(); + const resolvedModel = getResolvedModel(); + const initialPrompt = state.initialPrompt; + + // Build model portion + let modelPart: string; + if (selectedModel && resolvedModel) { + modelPart = `${selectedModel} → ${resolvedModel}`; + } else if (selectedModel) { + modelPart = selectedModel; + } else if (resolvedModel) { + modelPart = resolvedModel; + } else { + modelPart = '—'; + } + + // Build initial prompt portion — unquoted preview with generous + // space allocation. The line is ultimately truncated to `width` by + // truncateToTerminalWidth, so we use a dynamic limit. + let promptPart: string | null = null; + if (initialPrompt) { + // Truncate work item IDs to compact form (e.g., WL-0MQL0T5TR0060AEH + // → WL...68HD) before applying length truncation. + const compacted = truncateWorkItemId(initialPrompt); + // Reserve space for model part (up to ~30 chars), separator (4), + // and a small buffer. Then clip at the end. + const reserved = 38; + const maxLen = Math.max(15, width - reserved); + const preview = + compacted.length > maxLen + ? `${compacted.slice(0, maxLen - 3)}...` + : compacted; + promptPart = preview; + } + + const label = promptPart ? `${modelPart} │ ${promptPart}` : modelPart; + lines.push(truncateToTerminalWidth(theme.fg('dim', label), width)); + + return lines; + }, + }; + }); + } +} diff --git a/packages/tui/extensions/Worklog/terminal-utils.test.ts b/packages/tui/extensions/Worklog/terminal-utils.test.ts index 269fb38b..60c446c9 100644 --- a/packages/tui/extensions/Worklog/terminal-utils.test.ts +++ b/packages/tui/extensions/Worklog/terminal-utils.test.ts @@ -11,6 +11,7 @@ import { getCharWidth, visibleWidth, truncateToTerminalWidth, + truncateWorkItemId, wrapToTerminalWidth, } from './terminal-utils.js'; @@ -331,4 +332,67 @@ describe('terminal-utils', () => { expect(result).toContain('...'); }); }); + + // ── truncateWorkItemId ────────────────────────────────────────────── + + describe('truncateWorkItemId', () => { + it('truncates a standard WL- prefixed work item ID', () => { + const result = truncateWorkItemId('WL-0MQL0T5TR0060AEH'); + expect(result).toBe('WL...0AEH'); + }); + + it('truncates a SA- prefixed work item ID', () => { + const result = truncateWorkItemId('SA-0MPYMFZXO0004ZU4'); + expect(result).toBe('SA...4ZU4'); + }); + + it('truncates a CG- prefixed work item ID', () => { + const result = truncateWorkItemId('CG-0MQK0OM6I00168HD'); + expect(result).toBe('CG...68HD'); + }); + + it('does not truncate short IDs (fewer than 15 chars after dash)', () => { + expect(truncateWorkItemId('WL-123')).toBe('WL-123'); + expect(truncateWorkItemId('WL-abc123')).toBe('WL-abc123'); + expect(truncateWorkItemId('WL-0MQL0T5TR')).toBe('WL-0MQL0T5TR'); + }); + + it('truncates all occurrences in a string with multiple IDs', () => { + const result = truncateWorkItemId( + 'WL-0MQL0T5TR0060AEH and CG-0MQK0OM6I00168HD' + ); + expect(result).toBe('WL...0AEH and CG...68HD'); + }); + + it('does not truncate IDs that appear at the start of a string', () => { + const result = truncateWorkItemId('WL-0MQL0T5TR0060AEH is the ID'); + expect(result).toBe('WL...0AEH is the ID'); + }); + + it('does not truncate IDs that appear at the end of a string', () => { + const result = truncateWorkItemId('Process item WL-0MQL0T5TR0060AEH'); + expect(result).toBe('Process item WL...0AEH'); + }); + + it('returns original text when no work item IDs are present', () => { + expect(truncateWorkItemId('Hello world')).toBe('Hello world'); + expect(truncateWorkItemId('')).toBe(''); + expect(truncateWorkItemId('No IDs here at all')).toBe('No IDs here at all'); + }); + + it('works with IDs embedded in longer text with spaces', () => { + const result = truncateWorkItemId('implement WL-0MQL0T5TR0060AEH the feature'); + expect(result).toBe('implement WL...0AEH the feature'); + }); + + it('handles IDs with ANSI escape sequences (passes through)', () => { + const result = truncateWorkItemId('\x1b[31mWL-0MQL0T5TR0060AEH\x1b[0m'); + expect(result).toBe('\x1b[31mWL...0AEH\x1b[0m'); + }); + + it('handles multiple IDs separated by various delimiters', () => { + const result = truncateWorkItemId('WL-0MQL0T5TR0060AEH,WL-0MQLG8PK80041FM3'); + expect(result).toBe('WL...0AEH,WL...1FM3'); + }); + }); }); \ No newline at end of file diff --git a/packages/tui/extensions/Worklog/terminal-utils.ts b/packages/tui/extensions/Worklog/terminal-utils.ts index c0005ebd..7ada69ce 100644 --- a/packages/tui/extensions/Worklog/terminal-utils.ts +++ b/packages/tui/extensions/Worklog/terminal-utils.ts @@ -199,6 +199,62 @@ export function truncateToTerminalWidth( return result + ellipsis; } +// ───────────────────────────────────────────────────────────────────── +// Work item ID truncation +// ───────────────────────────────────────────────────────────────────── + +/** + * Regex to match work item ID patterns for truncation. + * + * Matches patterns like `WL-0MQL0T5TR0060AEH` (prefix + dash + 15+ alphanumeric chars). + * The prefix must be 2-3 uppercase letters followed by a dash and at least 15 + * alphanumeric characters. Shorter IDs are intentionally excluded. + */ +const WORK_ITEM_ID_TRUNCATION_REGEX = /\b([A-Z]{2,3})-([A-Z0-9]{15,})/g; + +/** + * Regex to strip ANSI escape sequences for clean matching. + */ +const ANSI_ESCAPE_RE = /\x1b\[[;0-9]*m/g; + +/** + * Truncate a work item ID to `PREFIX...LAST4` format. + * + * Replaces full-length work item IDs (2-3 uppercase letter prefix, dash, 15+ + * alphanumeric chars) with a compact form that preserves the prefix and last + * 4 characters, separated by `...` and no dash. + * + * Shorter IDs are left unchanged. + * + * @param text - Input string that may contain work item IDs + * @returns String with work item IDs truncated + * + * @example + * truncateWorkItemId('WL-0MQL0T5TR0060AEH') // => 'WL...0AEH' + * truncateWorkItemId('WL-123') // => 'WL-123' (unchanged) + * truncateWorkItemId('CG-0MQK0OM6I00168HD') // => 'CG...68HD' + */ +export function truncateWorkItemId(text: string): string { + // Use a placeholder approach to handle ANSI codes: replace each ANSI + // escape sequence with a unique non-word-character placeholder, + // apply ID truncation, then restore the codes. + // The placeholder is a null character which is not a word character, + // so \b word boundaries work correctly around it. + const ansiCodes: string[] = []; + const prepared = text.replace(ANSI_ESCAPE_RE, (match) => { + ansiCodes.push(match); + return '\x00'; // null char (not a word char, works with \b) + }); + + const truncated = prepared.replace(WORK_ITEM_ID_TRUNCATION_REGEX, (_match, prefix, code) => { + return `${prefix}...${code.slice(-4)}`; + }); + + // Restore ANSI codes from placeholder positions + let ai = 0; + return truncated.replace(/\x00/g, () => ansiCodes[ai++]); +} + // ───────────────────────────────────────────────────────────────────── // Utility functions for wrapToTerminalWidth // ───────────────────────────────────────────────────────────────────── diff --git a/packages/tui/tests/activity-indicator.test.ts b/packages/tui/tests/activity-indicator.test.ts index c2ad3159..7a86791f 100644 --- a/packages/tui/tests/activity-indicator.test.ts +++ b/packages/tui/tests/activity-indicator.test.ts @@ -513,10 +513,12 @@ describe('registerActivityIndicator - input events', () => { ACTIVITY_STATUS_KEY, expect.stringContaining('WL-0MQL0T5TR0060AEH') ); - // Final display should include the command context alongside ID + title + // Final display should include the command context alongside ID + title. + // Work item IDs are truncated to PREFIX...LAST4 format for display. const lastCallArg = (ctx.ui.setStatus as ReturnType).mock.calls.slice(-1)[0][1] as string; expect(lastCallArg).toContain('/intake'); - expect(lastCallArg).toContain('WL-0MQL0T5TR0060AEH'); + expect(lastCallArg).toContain('WL...0AEH'); + expect(lastCallArg).not.toContain('WL-0MQL0T5TR0060AEH'); expect(lastCallArg).toContain('Fix login bug'); // Verify runWl was called with the correct arguments @@ -557,11 +559,12 @@ describe('registerActivityIndicator - input events', () => { await inputHandlers[0](event, ctx); - // Should show skill name (with /skill: prefix stripped) + ID + title + // Should show skill name (with /skill: prefix stripped) + truncated ID + title const lastCallArg = (ctx.ui.setStatus as ReturnType).mock.calls.slice(-1)[0][1] as string; expect(lastCallArg).not.toContain('/skill:'); expect(lastCallArg).toContain('implement'); - expect(lastCallArg).toContain('WL-0MP15TA8J009NZUU'); + expect(lastCallArg).toContain('WL...NZUU'); + expect(lastCallArg).not.toContain('WL-0MP15TA8J009NZUU'); expect(lastCallArg).toContain('Add user authentication'); expect(mockRunWl).toHaveBeenCalledWith('show', ['WL-0MP15TA8J009NZUU'], { timeout: 2000 }); }); @@ -599,10 +602,11 @@ describe('registerActivityIndicator - input events', () => { await inputHandlers[0](event, ctx); - // Should show command + ID + title + // Should show command + truncated ID + title const lastCallArg = (ctx.ui.setStatus as ReturnType).mock.calls.slice(-1)[0][1] as string; expect(lastCallArg).toContain('/custom-command'); - expect(lastCallArg).toContain('WL-0MQLG8PK80041FM3'); + expect(lastCallArg).toContain('WL...1FM3'); + expect(lastCallArg).not.toContain('WL-0MQLG8PK80041FM3'); expect(lastCallArg).toContain('Resolve work item IDs to titles'); expect(mockRunWl).toHaveBeenCalledWith('show', ['WL-0MQLG8PK80041FM3'], { timeout: 2000 }); }); @@ -626,7 +630,8 @@ describe('registerActivityIndicator - input events', () => { const lastCallArg = (ctx.ui.setStatus as ReturnType).mock.calls.slice(-1)[0][1] as string; expect(lastCallArg).toContain('/implement'); - expect(lastCallArg).toContain('WL-0MQL0T5TR0060AEH'); + expect(lastCallArg).toContain('WL...0AEH'); + expect(lastCallArg).not.toContain('WL-0MQL0T5TR0060AEH'); expect(lastCallArg).toContain('First work item title'); }); }); diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts index 1c5249f8..abae4a40 100644 --- a/src/commands/doctor.ts +++ b/src/commands/doctor.ts @@ -7,6 +7,7 @@ import { loadStatusStageRules } from '../status-stage-rules.js'; import { validateStatusStageItems } from '../doctor/status-stage-check.js'; import { validateDependencyEdges } from '../doctor/dependency-check.js'; import { listPendingMigrations, runMigrations } from '../migrations/index.js'; +import { dryRunHooks, upgradeHooks, type HookDryRunResult, type HookUpgradeResult } from '../doctor/hook-upgrade.js'; import { validateFilePaths, applyFilePathsFix, DEFAULT_INTAKE_STAGES } from '../doctor/file-paths-check.js'; import { importFromJsonl } from '../jsonl.js'; import { mergeWorkItems, mergeComments, mergeAuditResults } from '../sync.js'; @@ -29,55 +30,116 @@ export default function register(ctx: PluginContext): void { doctor .command('upgrade') - .description('Preview or apply pending database schema migrations') - .option('--dry-run', 'Preview pending migrations without applying them') - .option('--confirm', 'Apply pending migrations (non-interactive)') + .description('Preview or apply pending database schema migrations and outdated git hooks') + .option('--dry-run', 'Preview pending migrations and outdated hooks without applying them') + .option('--confirm', 'Apply pending migrations and outdated hooks (non-interactive)') .option('--prefix ', 'Override the default prefix') .action(async (opts: { dryRun?: boolean; confirm?: boolean; prefix?: string }) => { - // Migration upgrade subcommand + // Migration + hook upgrade subcommand utils.requireInitialized(); try { const pending = listPendingMigrations(); - if (!pending || pending.length === 0) { + const hooksResult = dryRunHooks(); + + const hasPendingMigrations = pending && pending.length > 0; + const hasOutdatedHooks = hooksResult.outdatedCount > 0; + + if (!hasPendingMigrations && !hasOutdatedHooks) { if (utils.isJsonMode()) { - output.json({ success: true, pending: [] }); + output.json({ success: true, pending: [], hooks: hooksResult }); return; } console.log('Doctor: no pending migrations. See docs/migrations.md for migration policy and guidance.'); + console.log('Doctor: all installed hooks are up-to-date.'); return; } if (opts.dryRun) { if (utils.isJsonMode()) { - output.json({ success: true, dryRun: true, pending }); + const out: any = { + success: true, + dryRun: true, + pending: hasPendingMigrations ? pending : [], + hooks: hooksResult, + }; + output.json(out); return; } - // Dry-run: list all pending migrations (no prompt, purely informational) - console.log('Pending migrations:'); - pending.forEach(p => console.log(` - ${p.id}: ${p.description} (safe=${p.safe})`)); + // Dry-run: list all pending migrations + if (hasPendingMigrations) { + console.log('Pending migrations:'); + pending.forEach(p => console.log(` - ${p.id}: ${p.description} (safe=${p.safe})`)); + } + // List outdated hooks + if (hasOutdatedHooks) { + console.log(''); + console.log('Outdated hooks:'); + hooksResult.hooks + .filter(h => h.status === 'outdated') + .forEach(h => console.log(` - ${h.name} at ${h.hookPath}`)); + } + if (!hasOutdatedHooks && hasPendingMigrations) { + console.log(''); + console.log('Doctor: all installed hooks are up-to-date.'); + } return; } - // Not a dry-run: list safe migrations, print blank line, and ask to apply - const safeMigs = pending.filter(p => p.safe); + // Not a dry-run: list safe migrations and outdated hooks, ask to apply + const safeMigs = pending ? pending.filter(p => p.safe) : []; if (utils.isJsonMode()) { if (!opts.confirm) { - output.json({ success: true, pending, safeMigrations: safeMigs, requiresConfirm: true }); + const out: any = { + success: true, + pending: hasPendingMigrations ? pending : [], + safeMigrations: safeMigs, + hooks: hooksResult, + requiresConfirm: true, + }; + output.json(out); + return; + } + + // Apply hooks first, then migrations + let hooksApplied: string[] = []; + let hooksSkipped: string[] = []; + let hookError: string | undefined; + + if (hasOutdatedHooks) { + const hookResult = upgradeHooks(); + if (hookResult.success) { + hooksApplied = hookResult.upgraded; + hooksSkipped = hookResult.skipped; + } else { + hookError = hookResult.error; + } + } + + if (hookError) { + process.exitCode = 1; + output.json({ success: false, error: hookError }); return; } try { - const result = runMigrations({ - dryRun: false, - confirm: true, - logger: { info: s => console.error(s), error: s => console.error(s) } - }); + let migrationResult = { applied: [] as any[], backups: [] as string[] }; + if (hasPendingMigrations) { + migrationResult = runMigrations({ + dryRun: false, + confirm: true, + logger: { info: s => console.error(s), error: s => console.error(s) } + }); + } + output.json({ success: true, - pending, + pending: hasPendingMigrations ? pending : [], safeMigrations: safeMigs, - applied: result.applied, - backups: result.backups, + applied: migrationResult.applied, + backups: migrationResult.backups, + hooks: hooksResult, + hooksApplied, + hooksSkipped, }); return; } catch (err) { @@ -93,12 +155,16 @@ export default function register(ctx: PluginContext): void { // Confirm before applying unless --confirm provided let proceed = Boolean(opts.confirm); - if (!proceed) { + const totalItems = (hasPendingMigrations ? pending.length : 0) + hooksResult.outdatedCount; + if (totalItems > 0 && !proceed) { // Prompt interactively const readlineMod = await import('node:readline'); const answer = await new Promise(resolve => { const rl = readlineMod.createInterface({ input: process.stdin, output: process.stdout }); - rl.question(`Apply ${pending.length} pending migration(s)? (y/N): `, (a: string) => { + const parts: string[] = []; + if (hasPendingMigrations) parts.push(`${pending.length} pending migration(s)`); + if (hasOutdatedHooks) parts.push(`${hooksResult.outdatedCount} outdated hook(s)`); + rl.question(`Apply ${parts.join(' and ')}? (y/N): `, (a: string) => { rl.close(); const v = (a || '').trim().toLowerCase(); resolve(v === 'y' || v === 'yes'); @@ -108,20 +174,49 @@ export default function register(ctx: PluginContext): void { } if (!proceed) { - if (utils.isJsonMode()) output.json({ success: false, message: 'User declined to apply migrations' }); - else console.log('Aborted: migrations not applied.'); + if (utils.isJsonMode()) output.json({ success: false, message: 'User declined to apply changes' }); + else console.log('Aborted: changes not applied.'); + return; + } + + // Apply hooks first, then migrations + let hooksApplied: string[] = []; + let hookError: string | undefined; + if (hasOutdatedHooks) { + const hookResult = upgradeHooks(); + if (hookResult.success) { + hooksApplied = hookResult.upgraded; + } else { + hookError = hookResult.error; + } + } + + if (hookError) { + if (utils.isJsonMode()) { + output.json({ success: false, error: hookError }); + } else { + console.error(`Hook upgrade failed: ${hookError}`); + } return; } // Apply migrations try { - const result = runMigrations({ dryRun: false, confirm: true, logger: { info: s => console.error(s), error: s => console.error(s) } }); + let migrationResult = { applied: [] as any[], backups: [] as string[] }; + if (hasPendingMigrations) { + migrationResult = runMigrations({ dryRun: false, confirm: true, logger: { info: s => console.error(s), error: s => console.error(s) } }); + } if (utils.isJsonMode()) { - output.json({ success: true, applied: result.applied, backups: result.backups }); + output.json({ success: true, applied: migrationResult.applied, backups: migrationResult.backups }); return; } - console.log(`Applied migrations: ${result.applied.map(a => a.id).join(', ')}`); - if (result.backups && result.backups.length > 0) console.log(`Backups: ${result.backups.join(', ')}`); + if (migrationResult.applied.length > 0) { + console.log(`Applied migrations: ${migrationResult.applied.map(a => a.id).join(', ')}`); + } + if (migrationResult.backups && migrationResult.backups.length > 0) console.log(`Backups: ${migrationResult.backups.join(', ')}`); + if (hooksApplied.length > 0) { + console.log(`Upgraded hooks: ${hooksApplied.join(', ')}`); + } } catch (err) { const message = err instanceof Error ? err.message : String(err); if (utils.isJsonMode()) output.json({ success: false, error: message }); diff --git a/src/commands/init.ts b/src/commands/init.ts index 34b0651a..fceefb22 100644 --- a/src/commands/init.ts +++ b/src/commands/init.ts @@ -198,7 +198,7 @@ function installPrePushHook(options: { silent: boolean }): { installed: boolean; ` exit 0\n` + `fi\n` + `\n` + - `\"$WL\" sync\n` + + `$WL sync --git-branch refs/worklog/data\n` + `\n` + `exit 0\n`; @@ -270,7 +270,7 @@ function installPostPullHooks(options: { silent: boolean }): { installed: boolea ` exit 0\n` + `fi\n` + `# Run sync but do not fail the checkout/merge if sync is not available or fails\n` + - `if \"$WL\" sync >/dev/null 2>&1; then\n` + + `if \"$WL\" sync --git-branch refs/worklog/data >/dev/null 2>&1; then\n` + ` :\n` + `else\n` + ` # Check if this is a new checkout/worktree (no .worklog directory)\n` + @@ -363,7 +363,7 @@ function installCommittedHooks(options: { silent: boolean }): { installed: boole ` echo \"worklog: wl/worklog not found; skipping post-pull sync\" >&2\n` + ` exit 0\n` + `fi\n` + - `if \"$WL\" sync >/dev/null 2>&1; then\n` + + `if \"$WL\" sync --git-branch refs/worklog/data >/dev/null 2>&1; then\n` + ` :\n` + `else\n` + ` if [ ! -d \".worklog\" ]; then\n` + @@ -407,7 +407,7 @@ function installCommittedHooks(options: { silent: boolean }): { installed: boole ` echo \"worklog: wl/worklog not found; skipping pre-push sync\" >&2\n` + ` exit 0\n` + `fi\n` + - `\"$WL\" sync\n` + + `$WL sync --git-branch refs/worklog/data\n` + `exit 0\n`; const postCheckoutContent = @@ -427,7 +427,7 @@ function installCommittedHooks(options: { silent: boolean }): { installed: boole ` echo \"worklog: wl/worklog not found; skipping post-checkout sync\" >&2\n` + ` exit 0\n` + `fi\n` + - `if \"$WL\" sync >/dev/null 2>&1; then\n` + + `if \"$WL\" sync --git-branch refs/worklog/data >/dev/null 2>&1; then\n` + ` :\n` + `else\n` + ` if [ ! -d \".worklog\" ]; then\n` + diff --git a/src/doctor/hook-upgrade.ts b/src/doctor/hook-upgrade.ts new file mode 100644 index 00000000..c981c583 --- /dev/null +++ b/src/doctor/hook-upgrade.ts @@ -0,0 +1,300 @@ +/** + * Hook upgrade module — detects and upgrades outdated git hooks + * installed in .git/hooks/ from committed versions in .githooks/. + */ + +import * as fs from 'fs'; +import * as path from 'path'; + +// Markers used by Worklog to identify its own hooks. +const WORKLOG_MARKERS = [ + 'worklog:pre-push-hook:', + 'worklog:post-pull-hook:', + 'worklog:post-checkout-hook:', +] as const; + +// Hooks in .githooks/ that should be tracked +const HOOK_NAMES = [ + 'pre-push', + 'post-checkout', + 'post-merge', + 'post-rewrite', + 'worklog-post-pull', +] as const; + +/** + * Whether a file appears to be a Worklog-managed hook. + */ +function hasWorklogMarker(content: string): boolean { + return WORKLOG_MARKERS.some(marker => content.includes(marker)); +} + +/** + * Whether a hook is outdated. + * + * A hook is considered outdated when: + * - It lacks the safe `--git-branch refs/worklog/data` guard, or + * - It contains hardcoded paths like `/tmp/Worklog/...` or absolute paths to + * `.git/hooks/` for the central post-pull script (should use `$(dirname "$0")`), + * - Its content differs from the committed `.githooks/` version. + */ +function isHookOutdated(installed: string, committed: string): boolean { + // If they are identical, not outdated + if (installed.trim() === committed.trim()) return false; + + // Outdated if missing the safe --git-branch guard + if (!installed.includes('--git-branch refs/worklog/data')) return true; + + // Outdated if installed hook contains hardcoded absolute paths to .git/hooks/ + // instead of using the relative $(dirname "$0") approach + const hasHardcodedPaths = /exec\s+["']\/[^"']*["']\s*["']worklog-post-pull["']/.test(installed); + if (hasHardcodedPaths) return true; + + // If content differs at all, it is outdated (the committed .githooks/ version + // is the source of truth) + return true; +} + +export interface HookInfo { + name: string; + hookPath: string; + committedPath: string; + committedContent: string; + currentContent?: string; + status: 'up-to-date' | 'outdated' | 'not-installed'; + reason?: string; +} + +export interface HookDryRunResult { + success: boolean; + dryRun: true; + hooks: Array<{ + name: string; + hookPath: string; + status: 'up-to-date' | 'outdated' | 'not-installed'; + reason?: string; + upgraded: string[]; + }>; + upgraded: string[]; // names of hooks that would be upgraded + outdatedCount: number; + upToDateCount: number; + notInstalledCount: number; +} + +export interface HookUpgradeResult { + success: boolean; + confirmed: true; + hooks: Array<{ + name: string; + hookPath: string; + status: 'up-to-date' | 'outdated' | 'not-installed'; + reason?: string; + }>; + upgraded: string[]; + skipped: string[]; + error?: string; +} + +/** + * Scan the repository for git hooks and classify each as up-to-date, outdated, + * or not-installed. + */ +export function listOutdatedHooks( + githooksDir: string = '.githooks', + hooksDir: string = '.git/hooks' +): HookInfo[] { + const info: HookInfo[] = []; + + // Check if .githooks directory exists + if (!fs.existsSync(githooksDir)) { + return info; + } + + const gitHooksPath = path.isAbsolute(hooksDir) ? hooksDir : path.resolve(hooksDir); + if (!fs.existsSync(gitHooksPath)) { + // No hooks directory at all — all hooks are "not-installed" + for (const name of HOOK_NAMES) { + const committedPath = path.join(githooksDir, name); + if (fs.existsSync(committedPath)) { + info.push({ + name, + hookPath: path.join(gitHooksPath, name), + committedPath, + committedContent: fs.readFileSync(committedPath, 'utf-8'), + status: 'not-installed', + }); + } + } + return info; + } + + for (const name of HOOK_NAMES) { + const committedPath = path.join(githooksDir, name); + if (!fs.existsSync(committedPath)) { + continue; + } + + const committedContent = fs.readFileSync(committedPath, 'utf-8'); + const hookPath = path.join(gitHooksPath, name); + + if (!fs.existsSync(hookPath)) { + info.push({ + name, + hookPath, + committedPath, + committedContent, + status: 'not-installed', + }); + continue; + } + + const currentContent = fs.readFileSync(hookPath, 'utf-8'); + + // Only consider worklog-managed hooks for upgrade + if (!hasWorklogMarker(currentContent)) { + // Not a worklog hook — skip + continue; + } + + if (isHookOutdated(currentContent, committedContent)) { + info.push({ + name, + hookPath, + committedPath, + committedContent, + currentContent, + status: 'outdated', + reason: `Hook '${name}' is outdated and would be upgraded from .githooks/${name}`, + }); + } else { + info.push({ + name, + hookPath, + committedPath, + committedContent, + currentContent, + status: 'up-to-date', + }); + } + } + + return info; +} + +/** + * Perform a dry-run: return what would be upgraded without making changes. + */ +export function dryRunHooks( + githooksDir?: string, + hooksDir?: string +): HookDryRunResult { + const outdatedHooks = listOutdatedHooks(githooksDir, hooksDir); + const upgraded: string[] = []; + let outdatedCount = 0; + let upToDateCount = 0; + let notInstalledCount = 0; + + const hooks = outdatedHooks.map(h => { + const hookResult = { + name: h.name, + hookPath: h.hookPath, + status: h.status, + reason: h.reason, + upgraded: [] as string[], + }; + + if (h.status === 'outdated') { + outdatedCount++; + upgraded.push(h.name); + hookResult.upgraded.push(h.name); + } else if (h.status === 'up-to-date') { + upToDateCount++; + } else { + notInstalledCount++; + } + + return hookResult; + }); + + return { + success: true, + dryRun: true, + hooks, + upgraded, + outdatedCount, + upToDateCount, + notInstalledCount, + }; +} + +/** + * Apply hook upgrades (confirm mode): replace outdated hooks with committed versions. + */ +export function upgradeHooks( + githooksDir?: string, + hooksDir?: string +): HookUpgradeResult { + const outdatedHooks = listOutdatedHooks(githooksDir, hooksDir); + const upgraded: string[] = []; + const skipped: string[] = []; + let error: string | undefined; + + const hooks = outdatedHooks.map(h => { + if (h.status === 'outdated') { + const hookPath = h.hookPath; + try { + // Ensure the hooks directory exists + const hookDir = path.dirname(hookPath); + if (!fs.existsSync(hookDir)) { + fs.mkdirSync(hookDir, { recursive: true }); + } + + fs.writeFileSync(hookPath, h.committedContent, { + encoding: 'utf-8', + mode: 0o755, + }); + + upgraded.push(h.name); + return { + name: h.name, + hookPath, + status: 'up-to-date' as const, + reason: 'Upgraded from .githooks/', + }; + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + error = `Failed to upgrade hook '${h.name}': ${message}`; + skipped.push(h.name); + return { + name: h.name, + hookPath, + status: 'outdated' as const, + reason: `Upgrade failed: ${message}`, + }; + } + } else if (h.status === 'up-to-date') { + skipped.push(h.name); + return { + name: h.name, + hookPath: h.hookPath, + status: 'up-to-date' as const, + }; + } else { + skipped.push(h.name); + return { + name: h.name, + hookPath: h.hookPath, + status: 'not-installed' as const, + }; + } + }); + + return { + success: !error, + confirmed: true, + hooks, + upgraded, + skipped, + error, + }; +} diff --git a/src/version.ts b/src/version.ts index 50fff331..6f0c9a78 100644 --- a/src/version.ts +++ b/src/version.ts @@ -1,2 +1,2 @@ // Auto-generated; do not edit. -export const WORKLOG_VERSION = '1.0.1'; +export const WORKLOG_VERSION = '1.0.2'; diff --git a/tests/cli/doctor-upgrade.test.ts b/tests/cli/doctor-upgrade.test.ts index a6055aa4..81bb0bcf 100644 --- a/tests/cli/doctor-upgrade.test.ts +++ b/tests/cli/doctor-upgrade.test.ts @@ -1,15 +1,35 @@ import { describe, it, expect, beforeEach, afterEach } from 'vitest'; import * as path from 'path'; +import * as fs from 'fs'; import Database from 'better-sqlite3'; import { cliPath, execAsync, + execWithInput, enterTempDir, leaveTempDir, writeConfig, writeInitSemaphore, } from './cli-helpers.js'; +function createGitRepo(dir: string): void { + // Create a minimal git repo + fs.mkdirSync(path.join(dir, '.git', 'hooks'), { recursive: true }); + // Initialize git + const { execSync } = require('child_process'); + try { + execSync('git init', { cwd: dir, stdio: 'pipe' }); + execSync('git config user.email "test@test.com"', { cwd: dir, stdio: 'pipe' }); + execSync('git config user.name "Test"', { cwd: dir, stdio: 'pipe' }); + // Create initial commit + fs.writeFileSync(path.join(dir, '.gitignore'), 'node_modules/\n', 'utf-8'); + execSync('git add .', { cwd: dir, stdio: 'pipe' }); + execSync('git commit -m "initial"', { cwd: dir, stdio: 'pipe' }); + } catch (_e) { + // git may not be available in test environment; skip + } +} + function createLegacyDbWithoutAudit(dbPath: string): void { const db = new Database(dbPath); try { @@ -97,4 +117,275 @@ describe('doctor upgrade command', () => { db.close(); } }); + + describe('hook upgrade via CLI', () => { + it('includes hook info in dry-run JSON output', async () => { + const { stdout } = await execAsync(`tsx ${cliPath} --json doctor upgrade --dry-run`); + const result = JSON.parse(stdout); + + expect(result.success).toBe(true); + expect(result.dryRun).toBe(true); + // Hooks should be present in the output + expect(result.hooks).toBeDefined(); + expect(result.hooks.outdatedCount).toBeGreaterThanOrEqual(0); + }); + + it('reports outdated hooks when they exist', async () => { + // Create a git repo with a .githooks/ directory containing a hook + // and a mismatched hook in .git/hooks/ + const dir = tempState.tempDir; + const gitHooks = path.join(dir, '.git', 'hooks'); + const githooks = path.join(dir, '.githooks'); + + try { + // Ensure .git/hooks exists + fs.mkdirSync(gitHooks, { recursive: true }); + + // Create .githooks with a fixed pre-push hook + fs.mkdirSync(githooks, { recursive: true }); + const fixedHook = `#!/bin/sh +# worklog:pre-push-hook:v1 +# Force the data branch to refs/worklog/data. +set -e +if [ "$WORKLOG_SKIP_PRE_PUSH" = "1" ]; then + exit 0 +fi +skip=0 +while read local_ref local_sha remote_ref remote_sha; do + if [ "$remote_ref" = "refs/worklog/data" ]; then + skip=1 + fi +done +if [ "$skip" = "1" ]; then + exit 0 +fi +if command -v wl >/dev/null 2>&1; then + WL=wl +elif command -v worklog >/dev/null 2>&1; then + WL=worklog +else + echo "worklog: wl/worklog not found; skipping pre-push sync" >&2 + exit 0 +fi +"$WL" sync --git-branch refs/worklog/data +exit 0 +`; + + // Create an outdated pre-push hook in .git/hooks/ + const outdatedHook = `#!/bin/sh +# worklog:pre-push-hook:v1 +# Auto-sync Worklog data before pushing. +set -e +if [ "$WORKLOG_SKIP_PRE_PUSH" = "1" ]; then + exit 0 +fi +skip=0 +while read local_ref local_sha remote_ref remote_sha; do + if [ "$remote_ref" = "refs/worklog/data" ]; then + skip=1 + fi +done +if [ "$skip" = "1" ]; then + exit 0 +fi +if command -v wl >/dev/null 2>&1; then + WL=wl +elif command -v worklog >/dev/null 2>&1; then + WL=worklog +else + echo "worklog: wl/worklog not found; skipping pre-push sync" >&2 + exit 0 +fi +$WL sync --git-branch refs/worklog/data +exit 0 +`; + + fs.writeFileSync(path.join(githooks, 'pre-push'), fixedHook, { mode: 0o755 }); + fs.writeFileSync(path.join(gitHooks, 'pre-push'), outdatedHook, { mode: 0o755 }); + + const { stdout } = await execAsync(`tsx ${cliPath} --json doctor upgrade --dry-run`); + const result = JSON.parse(stdout); + + expect(result.success).toBe(true); + expect(result.hooks.outdatedCount).toBe(1); + expect(result.hooks.upgraded).toContain('pre-push'); + } finally { + // Clean up + try { fs.rmSync(githooks, { recursive: true, force: true }); } catch {} + try { fs.rmSync(path.join(gitHooks, 'pre-push'), { force: true }); } catch {} + } + }); + + it('applies hook upgrades with --confirm and reports them', async () => { + const dir = tempState.tempDir; + const gitHooks = path.join(dir, '.git', 'hooks'); + const githooks = path.join(dir, '.githooks'); + + try { + fs.mkdirSync(gitHooks, { recursive: true }); + fs.mkdirSync(githooks, { recursive: true }); + + const fixedHook = `#!/bin/sh +# worklog:pre-push-hook:v1 +# Force the data branch to refs/worklog/data. +set -e +if [ "$WORKLOG_SKIP_PRE_PUSH" = "1" ]; then + exit 0 +fi +skip=0 +while read local_ref local_sha remote_ref remote_sha; do + if [ "$remote_ref" = "refs/worklog/data" ]; then + skip=1 + fi +done +if [ "$skip" = "1" ]; then + exit 0 +fi +if command -v wl >/dev/null 2>&1; then + WL=wl +elif command -v worklog >/dev/null 2>&1; then + WL=worklog +else + echo "worklog: wl/worklog not found; skipping pre-push sync" >&2 + exit 0 +fi +"$WL" sync --git-branch refs/worklog/data +exit 0 +`; + + const outdatedHook = `#!/bin/sh +# worklog:pre-push-hook:v1 +# Auto-sync Worklog data before pushing. +set -e +if [ "$WORKLOG_SKIP_PRE_PUSH" = "1" ]; then + exit 0 +fi +skip=0 +while read local_ref local_sha remote_ref remote_sha; do + if [ "$remote_ref" = "refs/worklog/data" ]; then + skip=1 + fi +done +if [ "$skip" = "1" ]; then + exit 0 +fi +if command -v wl >/dev/null 2>&1; then + WL=wl +elif command -v worklog >/dev/null 2>&1; then + WL=worklog +else + echo "worklog: wl/worklog not found; skipping pre-push sync" >&2 + exit 0 +fi +$WL sync --git-branch refs/worklog/data +exit 0 +`; + + fs.writeFileSync(path.join(githooks, 'pre-push'), fixedHook, { mode: 0o755 }); + fs.writeFileSync(path.join(gitHooks, 'pre-push'), outdatedHook, { mode: 0o755 }); + + // Read old hook content before upgrade + const oldContent = fs.readFileSync(path.join(gitHooks, 'pre-push'), 'utf-8'); + + const { stdout } = await execAsync(`tsx ${cliPath} --json doctor upgrade --confirm`); + const result = JSON.parse(stdout); + + expect(result.success).toBe(true); + // Hooks should be applied + expect(result.hooksApplied).toBeDefined(); + expect(result.hooksApplied).toContain('pre-push'); + + // Verify the hook was actually upgraded + const newContent = fs.readFileSync(path.join(gitHooks, 'pre-push'), 'utf-8'); + expect(newContent).toBe(fixedHook); + expect(newContent).not.toBe(oldContent); + } finally { + // Clean up + try { fs.rmSync(githooks, { recursive: true, force: true }); } catch {} + try { fs.rmSync(path.join(gitHooks, 'pre-push'), { force: true }); } catch {} + } + }); + + it('skips up-to-date hooks without error', async () => { + const dir = tempState.tempDir; + const gitHooks = path.join(dir, '.git', 'hooks'); + const githooks = path.join(dir, '.githooks'); + + try { + fs.mkdirSync(gitHooks, { recursive: true }); + fs.mkdirSync(githooks, { recursive: true }); + + const fixedHook = `#!/bin/sh +# worklog:pre-push-hook:v1 +# Force the data branch to refs/worklog/data. +set -e +if [ "$WORKLOG_SKIP_PRE_PUSH" = "1" ]; then + exit 0 +fi +skip=0 +while read local_ref local_sha remote_ref remote_sha; do + if [ "$remote_ref" = "refs/worklog/data" ]; then + skip=1 + fi +done +if [ "$skip" = "1" ]; then + exit 0 +fi +if command -v wl >/dev/null 2>&1; then + WL=wl +elif command -v worklog >/dev/null 2>&1; then + WL=worklog +else + echo "worklog: wl/worklog not found; skipping pre-push sync" >&2 + exit 0 +fi +"$WL" sync --git-branch refs/worklog/data +exit 0 +`; + + // Write identical hooks + fs.writeFileSync(path.join(githooks, 'pre-push'), fixedHook, { mode: 0o755 }); + fs.writeFileSync(path.join(gitHooks, 'pre-push'), fixedHook, { mode: 0o755 }); + + const { stdout } = await execAsync(`tsx ${cliPath} --json doctor upgrade --dry-run`); + const result = JSON.parse(stdout); + + expect(result.success).toBe(true); + expect(result.hooks.outdatedCount).toBe(0); + } finally { + try { fs.rmSync(githooks, { recursive: true, force: true }); } catch {} + try { fs.rmSync(path.join(gitHooks, 'pre-push'), { force: true }); } catch {} + } + }); + + it('skips non-worklog hooks', async () => { + const dir = tempState.tempDir; + const gitHooks = path.join(dir, '.git', 'hooks'); + const githooks = path.join(dir, '.githooks'); + + try { + fs.mkdirSync(gitHooks, { recursive: true }); + fs.mkdirSync(githooks, { recursive: true }); + + // .githooks has a worklog hook + fs.writeFileSync(path.join(githooks, 'pre-push'), '#!/bin/sh\n# worklog:pre-push-hook:v1\necho hook\n', { mode: 0o755 }); + // .git/hooks has a non-worklog hook (different content, no marker) + fs.writeFileSync(path.join(gitHooks, 'pre-push'), '#!/bin/sh\necho custom hook\n', { mode: 0o755 }); + + const { stdout } = await execAsync(`tsx ${cliPath} --json doctor upgrade --dry-run`); + const result = JSON.parse(stdout); + + expect(result.success).toBe(true); + // Non-worklog hooks should not be reported + const prePush = result.hooks.hooks.find((h: any) => h.name === 'pre-push'); + // Either not listed or marked as not-installed (since non-worklog is skipped) + if (prePush) { + expect(prePush.status).not.toBe('outdated'); + } + } finally { + try { fs.rmSync(githooks, { recursive: true, force: true }); } catch {} + try { fs.rmSync(path.join(gitHooks, 'pre-push'), { force: true }); } catch {} + } + }); + }); }); diff --git a/tests/cli/status.test.ts b/tests/cli/status.test.ts index 5d0abe00..aa0d04a7 100644 --- a/tests/cli/status.test.ts +++ b/tests/cli/status.test.ts @@ -108,7 +108,7 @@ describe('CLI Status Tests', () => { expect(stdout).toContain('Worklog System Status'); expect(stdout).toContain('Initialized: Yes'); - expect(stdout).toContain('Version: 1.0.1'); + expect(stdout).toContain('Version: 1.0.2'); expect(stdout).toContain('Configuration:'); expect(stdout).toContain('Database Summary:'); expect(stdout).toContain('Work Items:'); diff --git a/tests/doctor-hook-upgrade.test.ts b/tests/doctor-hook-upgrade.test.ts new file mode 100644 index 00000000..91751cee --- /dev/null +++ b/tests/doctor-hook-upgrade.test.ts @@ -0,0 +1,466 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as path from 'path'; +import { + listOutdatedHooks, + dryRunHooks, + upgradeHooks, + type HookInfo, +} from '../src/doctor/hook-upgrade.js'; + +// ---------- helpers ---------- + +function createTestDirs(): { root: string; githooks: string; gitHooks: string } { + const root = fs.mkdtempSync(path.join('/tmp', 'hook-upgrade-test-')); + const githooks = path.join(root, 'githooks'); + const gitHooks = path.join(root, 'gitHooks'); + fs.mkdirSync(githooks, { recursive: true }); + fs.mkdirSync(gitHooks, { recursive: true }); + return { root, githooks, gitHooks }; +} + +function removeDir(dir: string): void { + if (fs.existsSync(dir)) { + fs.rmSync(dir, { recursive: true, force: true }); + } +} + +// ---------- fixtures ---------- + +const FIXED_PRE_PUSH = `#!/bin/sh +# worklog:pre-push-hook:v1 + +# Auto-sync Worklog data before pushing. +# Force the data branch to refs/worklog/data regardless of config. +set -e + +if [ "$WORKLOG_SKIP_PRE_PUSH" = "1" ]; then + exit 0 +fi + +skip=0 +while read local_ref local_sha remote_ref remote_sha; do + if [ "$remote_ref" = "refs/worklog/data" ]; then + skip=1 + fi +done + +if [ "$skip" = "1" ]; then + exit 0 +fi + +if command -v wl >/dev/null 2>&1; then + WL=wl +elif command -v worklog >/dev/null 2>&1; then + WL=worklog +else + echo "worklog: wl/worklog not found; skipping pre-push sync" >&2 + exit 0 +fi + +"$WL" sync --git-branch refs/worklog/data +exit 0 +`; + +const OLD_PRE_PUSH = `#!/bin/sh +# worklog:pre-push-hook:v1 +# Auto-sync Worklog data before pushing. +set -e + +if [ "$WORKLOG_SKIP_PRE_PUSH" = "1" ]; then + exit 0 +fi + +skip=0 +while read local_ref local_sha remote_ref remote_sha; do + if [ "$remote_ref" = "refs/worklog/data" ]; then + skip=1 + fi +done + +if [ "$skip" = "1" ]; then + exit 0 +fi + +if command -v wl >/dev/null 2>&1; then + WL=wl +elif command -v worklog >/dev/null 2>&1; then + WL=worklog +else + echo "worklog: wl/worklog not found; skipping pre-push sync" >&2 + exit 0 +fi + +$WL sync --git-branch refs/worklog/data + +exit 0 +`; + +const FIXED_WRAPPER = `#!/bin/sh +# worklog:post-pull-hook:v1 +# Wrapper that delegates to central Worklog post-pull script (committed hooks). +exec "$(dirname "$0")/worklog-post-pull" "$@" +`; + +const OLD_WRAPPER = `#!/bin/sh +# worklog:post-pull-hook:v1 +# Wrapper that delegates to central Worklog post-pull script. +exec "/home/rgardler/projects/ContextHub/.git/hooks/worklog-post-pull" "$@" +`; + +const FIXED_POST_PULL = `#!/bin/sh +# worklog:post-pull-hook:v1 +# Central Worklog post-pull sync script (committed hooks). +# Set WORKLOG_SKIP_POST_PULL=1 to bypass. +set -e +if [ "$WORKLOG_SKIP_POST_PULL" = "1" ]; then + exit 0 +fi +if command -v wl >/dev/null 2>&1; then + WL=wl +elif command -v worklog >/dev/null 2>&1; then + WL=worklog +else + echo "worklog: wl/worklog not found; skipping post-pull sync" >&2 + exit 0 +fi +if "$WL" sync --git-branch refs/worklog/data >/dev/null 2>&1; then + : +else + echo "worklog: sync failed or not initialized; continuing" >&2 +fi +exit 0 +`; + +const OLD_POST_PULL = `#!/bin/sh +# worklog:post-pull-hook:v1 +# Central Worklog post-pull sync script. +# Set WORKLOG_SKIP_POST_PULL=1 to bypass. +set -e +if [ "$WORKLOG_SKIP_POST_PULL" = "1" ]; then + exit 0 +fi +if command -v wl >/dev/null 2>&1; then + WL=wl +elif command -v worklog >/dev/null 2>&1; then + WL=worklog +else + echo "worklog: wl/worklog not found; skipping post-pull sync" >&2 + exit 0 +fi +if "$WL" sync --git-branch refs/worklog/data >/dev/null 2>&1; then + : +else + # Check if this is a new checkout/worktree (no .worklog directory) + if [ ! -d ".worklog" ]; then + echo "worklog: not initialized in this checkout/worktree. Run \\"wl init\\" to set up this location." >&2 + else + echo "worklog: sync failed; continuing" >&2 + fi +fi +exit 0 +`; + +const FIXED_POST_CHECKOUT = `#!/bin/sh +# worklog:post-checkout-hook:v1 +# Auto-sync Worklog data after branch checkout (committed hooks). +# Set WORKLOG_SKIP_POST_CHECKOUT=1 to bypass. +set -e +if [ "$WORKLOG_SKIP_POST_CHECKOUT" = "1" ]; then + exit 0 +fi +if command -v wl >/dev/null 2>&1; then + WL=wl +elif command -v worklog >/dev/null 2>&1; then + WL=worklog +else + echo "worklog: wl/worklog not found; skipping post-checkout sync" >&2 + exit 0 +fi +if "$WL" sync --git-branch refs/worklog/data >/dev/null 2>&1; then + : +else + echo "worklog: sync failed or not initialized; continuing" >&2 +fi +exit 0 +`; + +const OLD_POST_CHECKOUT = `#!/bin/sh +# worklog:post-checkout-hook:v1 +# Auto-sync Worklog data after branch checkout (committed hooks). +# Set WORKLOG_SKIP_POST_CHECKOUT=1 to bypass. +set -e +if [ "$WORKLOG_SKIP_POST_CHECKOUT" = "1" ]; then + exit 0 +fi +if command -v wl >/dev/null 2>&1; then + WL=wl +elif command -v worklog >/dev/null 2>&1; then + WL=worklog +else + echo "worklog: wl/worklog not found; skipping post-checkout sync" >&2 + exit 0 +fi +if "$WL" sync --git-branch refs/worklog/data >/dev/null 2>&1; then + : +else + echo "worklog: sync failed; continuing" >&2 +fi +exit 0 +`; + +// ---------- tests ---------- + +describe('hook-upgrade module', () => { + let dirs: { root: string; githooks: string; gitHooks: string }; + + beforeEach(() => { + dirs = createTestDirs(); + }); + + afterEach(() => { + removeDir(dirs.root); + }); + + describe('listOutdatedHooks', () => { + it('returns empty array when .githooks dir does not exist', () => { + const result = listOutdatedHooks('non-existent', dirs.gitHooks); + expect(result).toHaveLength(0); + }); + + it('returns not-installed for all hooks when .git/hooks does not exist', () => { + // Write committed hooks but remove gitHooks + fs.writeFileSync(path.join(dirs.githooks, 'pre-push'), FIXED_PRE_PUSH); + fs.writeFileSync(path.join(dirs.githooks, 'post-merge'), FIXED_WRAPPER); + removeDir(dirs.gitHooks); + + const result = listOutdatedHooks(dirs.githooks, dirs.gitHooks); + const names = result.map(r => r.name); + expect(names).toContain('pre-push'); + expect(names).toContain('post-merge'); + expect(result.every(r => r.status === 'not-installed')).toBe(true); + }); + + it('identifies outdated pre-push hook (missing --git-branch guard)', () => { + fs.writeFileSync(path.join(dirs.githooks, 'pre-push'), FIXED_PRE_PUSH); + fs.writeFileSync(path.join(dirs.gitHooks, 'pre-push'), OLD_PRE_PUSH); + + const result = listOutdatedHooks(dirs.githooks, dirs.gitHooks); + const prePush = result.find(r => r.name === 'pre-push'); + expect(prePush).toBeDefined(); + expect(prePush!.status).toBe('outdated'); + }); + + it('identifies outdated wrapper hooks (hardcoded paths)', () => { + fs.writeFileSync(path.join(dirs.githooks, 'post-merge'), FIXED_WRAPPER); + fs.writeFileSync(path.join(dirs.gitHooks, 'post-merge'), OLD_WRAPPER); + + const result = listOutdatedHooks(dirs.githooks, dirs.gitHooks); + const postMerge = result.find(r => r.name === 'post-merge'); + expect(postMerge).toBeDefined(); + expect(postMerge!.status).toBe('outdated'); + }); + + it('identifies outdated post-pull central script', () => { + fs.writeFileSync(path.join(dirs.githooks, 'worklog-post-pull'), FIXED_POST_PULL); + fs.writeFileSync(path.join(dirs.gitHooks, 'worklog-post-pull'), OLD_POST_PULL); + + const result = listOutdatedHooks(dirs.githooks, dirs.gitHooks); + const pp = result.find(r => r.name === 'worklog-post-pull'); + expect(pp).toBeDefined(); + expect(pp!.status).toBe('outdated'); + }); + + it('identifies outdated post-checkout hook', () => { + fs.writeFileSync(path.join(dirs.githooks, 'post-checkout'), FIXED_POST_CHECKOUT); + fs.writeFileSync(path.join(dirs.gitHooks, 'post-checkout'), OLD_POST_CHECKOUT); + + const result = listOutdatedHooks(dirs.githooks, dirs.gitHooks); + const pc = result.find(r => r.name === 'post-checkout'); + expect(pc).toBeDefined(); + expect(pc!.status).toBe('outdated'); + }); + + it('skips non-worklog hooks (no marker)', () => { + // .githooks has a hook, .git/hooks has a different non-worklog hook + fs.writeFileSync(path.join(dirs.githooks, 'pre-push'), FIXED_PRE_PUSH); + fs.writeFileSync(path.join(dirs.gitHooks, 'pre-push'), '#!/bin/sh\necho "custom hook"\n'); + + const result = listOutdatedHooks(dirs.githooks, dirs.gitHooks); + expect(result).toHaveLength(0); + }); + + it('identifies up-to-date hooks', () => { + fs.writeFileSync(path.join(dirs.githooks, 'pre-push'), FIXED_PRE_PUSH); + fs.writeFileSync(path.join(dirs.gitHooks, 'pre-push'), FIXED_PRE_PUSH); + + const result = listOutdatedHooks(dirs.githooks, dirs.gitHooks); + const prePush = result.find(r => r.name === 'pre-push'); + expect(prePush).toBeDefined(); + expect(prePush!.status).toBe('up-to-date'); + }); + + it('includes all tracked hooks in result', () => { + fs.writeFileSync(path.join(dirs.githooks, 'pre-push'), FIXED_PRE_PUSH); + fs.writeFileSync(path.join(dirs.githooks, 'post-checkout'), FIXED_POST_CHECKOUT); + fs.writeFileSync(path.join(dirs.githooks, 'post-merge'), FIXED_WRAPPER); + fs.writeFileSync(path.join(dirs.githooks, 'post-rewrite'), FIXED_WRAPPER); + fs.writeFileSync(path.join(dirs.githooks, 'worklog-post-pull'), FIXED_POST_PULL); + + // Install some hooks, skip others + fs.writeFileSync(path.join(dirs.gitHooks, 'pre-push'), FIXED_PRE_PUSH); + fs.writeFileSync(path.join(dirs.gitHooks, 'post-merge'), FIXED_WRAPPER); + + const result = listOutdatedHooks(dirs.githooks, dirs.gitHooks); + const names = result.map(r => r.name); + expect(names).toContain('pre-push'); + expect(names).toContain('post-checkout'); + expect(names).toContain('post-merge'); + expect(names).toContain('post-rewrite'); + expect(names).toContain('worklog-post-pull'); + expect(result.length).toBe(5); + }); + }); + + describe('dryRunHooks', () => { + it('reports outdated hooks without making changes', () => { + fs.writeFileSync(path.join(dirs.githooks, 'pre-push'), FIXED_PRE_PUSH); + fs.writeFileSync(path.join(dirs.gitHooks, 'pre-push'), OLD_PRE_PUSH); + + const result = dryRunHooks(dirs.githooks, dirs.gitHooks); + + expect(result.success).toBe(true); + expect(result.dryRun).toBe(true); + expect(result.outdatedCount).toBe(1); + expect(result.upgraded).toContain('pre-push'); + expect(result.upToDateCount).toBe(0); + expect(result.notInstalledCount).toBe(0); + }); + + it('reports no outdated hooks when all are up-to-date', () => { + fs.writeFileSync(path.join(dirs.githooks, 'pre-push'), FIXED_PRE_PUSH); + fs.writeFileSync(path.join(dirs.gitHooks, 'pre-push'), FIXED_PRE_PUSH); + + const result = dryRunHooks(dirs.githooks, dirs.gitHooks); + + expect(result.success).toBe(true); + expect(result.outdatedCount).toBe(0); + expect(result.upToDateCount).toBe(1); + }); + + it('reports not-installed hooks', () => { + fs.writeFileSync(path.join(dirs.githooks, 'pre-push'), FIXED_PRE_PUSH); + + const result = dryRunHooks(dirs.githooks, dirs.gitHooks); + + expect(result.success).toBe(true); + expect(result.notInstalledCount).toBe(1); + }); + + it('returns JSON-compatible structure', () => { + fs.writeFileSync(path.join(dirs.githooks, 'pre-push'), FIXED_PRE_PUSH); + fs.writeFileSync(path.join(dirs.gitHooks, 'pre-push'), OLD_PRE_PUSH); + + const result = dryRunHooks(dirs.githooks, dirs.gitHooks); + // Ensure it is serializable (no circular refs, etc.) + expect(() => JSON.stringify(result)).not.toThrow(); + const parsed = JSON.parse(JSON.stringify(result)); + expect(parsed.success).toBe(true); + expect(parsed.dryRun).toBe(true); + expect(Array.isArray(parsed.hooks)).toBe(true); + }); + }); + + describe('upgradeHooks (confirm mode)', () => { + it('replaces outdated hooks with committed versions', () => { + fs.writeFileSync(path.join(dirs.githooks, 'pre-push'), FIXED_PRE_PUSH); + fs.writeFileSync(path.join(dirs.gitHooks, 'pre-push'), OLD_PRE_PUSH); + + const result = upgradeHooks(dirs.githooks, dirs.gitHooks); + + expect(result.success).toBe(true); + expect(result.upgraded).toContain('pre-push'); + expect(result.skipped).not.toContain('pre-push'); + + // Verify the hook was actually replaced + const upgradedContent = fs.readFileSync(path.join(dirs.gitHooks, 'pre-push'), 'utf-8'); + expect(upgradedContent).toBe(FIXED_PRE_PUSH); + }); + + it('skips up-to-date hooks', () => { + fs.writeFileSync(path.join(dirs.githooks, 'pre-push'), FIXED_PRE_PUSH); + fs.writeFileSync(path.join(dirs.gitHooks, 'pre-push'), FIXED_PRE_PUSH); + + const result = upgradeHooks(dirs.githooks, dirs.gitHooks); + + expect(result.success).toBe(true); + expect(result.skipped).toContain('pre-push'); + }); + + it('skips not-installed hooks', () => { + fs.writeFileSync(path.join(dirs.githooks, 'pre-push'), FIXED_PRE_PUSH); + + const result = upgradeHooks(dirs.githooks, dirs.gitHooks); + + expect(result.success).toBe(true); + expect(result.skipped).toContain('pre-push'); + }); + + it('returns JSON-compatible structure', () => { + fs.writeFileSync(path.join(dirs.githooks, 'pre-push'), FIXED_PRE_PUSH); + fs.writeFileSync(path.join(dirs.gitHooks, 'pre-push'), OLD_PRE_PUSH); + + const result = upgradeHooks(dirs.githooks, dirs.gitHooks); + expect(() => JSON.stringify(result)).not.toThrow(); + const parsed = JSON.parse(JSON.stringify(result)); + expect(parsed.success).toBe(true); + expect(parsed.confirmed).toBe(true); + }); + + it('handles multiple hooks', () => { + fs.writeFileSync(path.join(dirs.githooks, 'pre-push'), FIXED_PRE_PUSH); + fs.writeFileSync(path.join(dirs.gitHooks, 'pre-push'), OLD_PRE_PUSH); + fs.writeFileSync(path.join(dirs.githooks, 'post-merge'), FIXED_WRAPPER); + fs.writeFileSync(path.join(dirs.gitHooks, 'post-merge'), OLD_WRAPPER); + + const result = upgradeHooks(dirs.githooks, dirs.gitHooks); + + expect(result.success).toBe(true); + expect(result.upgraded).toContain('pre-push'); + expect(result.upgraded).toContain('post-merge'); + }); + }); + + describe('integration with real repo', () => { + it('detects outdated hooks in the actual .githooks/.git/hooks setup', () => { + // Use the real repo paths + const repoGithooks = path.join(process.cwd(), '.githooks'); + const repoHooks = path.join(process.cwd(), '.git', 'hooks'); + + // We expect at least some hooks in .githooks + const realResult = listOutdatedHooks(repoGithooks, repoHooks); + const names = realResult.map(r => r.name); + + // At minimum, some hooks should be present (might be up-to-date or not-installed) + expect(names.length).toBeGreaterThan(0); + }); + + it('dryRun produces JSON output for the real repo', () => { + const repoGithooks = path.join(process.cwd(), '.githooks'); + const repoHooks = path.join(process.cwd(), '.git', 'hooks'); + + const result = dryRunHooks(repoGithooks, repoHooks); + expect(() => JSON.stringify(result)).not.toThrow(); + expect(result.success).toBe(true); + }); + + it('upgrade is idempotent — running twice does not cause errors', () => { + // First run upgrades + const result1 = upgradeHooks(); + // Second run should find no outdated hooks + const result2 = upgradeHooks(); + expect(result1.success).toBe(true); + expect(result2.success).toBe(true); + }); + }); +}); diff --git a/tests/extensions/worklog-browse-extension.test.ts b/tests/extensions/worklog-browse-extension.test.ts index 43adbea5..869d77c7 100644 --- a/tests/extensions/worklog-browse-extension.test.ts +++ b/tests/extensions/worklog-browse-extension.test.ts @@ -90,7 +90,7 @@ function makeListCustomMock() { describe('Worklog browse pi extension', () => { it('formats browse options with status, stage, and audit icons before the title (no ID)', () => { expect(formatBrowseOption({ id: 'WL-42', title: 'Implement thing', status: 'open' })).toBe( - '🔓 ❓ Implement thing', + '🔓 ❔ Implement thing', ); }); @@ -100,24 +100,24 @@ describe('Worklog browse pi extension', () => { { id: 'WL-123456', title: 'A very long work item title that will not fit', status: 'open' }, 24, ), - ).toBe('🔓 ❓ A very long work …'); + ).toBe('🔓 ❔ A very long work …'); }); it('formats epic items with epic icon and no child count when childCount is 0', () => { expect(formatBrowseOption({ id: 'WL-99', title: 'Epic feature', status: 'open', issueType: 'epic', childCount: 0 })).toBe( - '🔓 ❓ 🏰 Epic feature', + '🔓 ❔ 🏰 Epic feature', ); }); it('formats epic items with epic icon and child count when childCount > 0', () => { expect(formatBrowseOption({ id: 'WL-99', title: 'Epic feature', status: 'open', issueType: 'epic', childCount: 5 })).toBe( - '🔓 ❓ 🏰(5) Epic feature', + '🔓 ❔ 🏰(5) Epic feature', ); }); it('does not add epic icon for non-epic items', () => { expect(formatBrowseOption({ id: 'WL-42', title: 'Regular task', status: 'open', issueType: 'feature' })).toBe( - '🔓 ❓ Regular task', + '🔓 ❔ Regular task', ); }); @@ -143,7 +143,7 @@ describe('Worklog browse pi extension', () => { { id: 'WL-1', title: 'Test', status: 'open' }, false, // noIcons=false = use emoji ); - expect(prefix).toBe('🔓 ❓'); + expect(prefix).toBe('🔓 ❔'); }); it('includes stage icon when stage is defined', () => { @@ -151,7 +151,7 @@ describe('Worklog browse pi extension', () => { { id: 'WL-2', title: 'Test', status: 'open', stage: 'in_progress' }, false, ); - expect(prefix).toBe('🔓 🛠️ ❓'); + expect(prefix).toBe('🔓 🛠️ ❔'); }); it('includes epic icon for epic items without child count', () => { @@ -159,7 +159,7 @@ describe('Worklog browse pi extension', () => { { id: 'WL-3', title: 'Test', status: 'open', issueType: 'epic', childCount: 0 }, false, ); - expect(prefix).toBe('🔓 ❓ 🏰'); + expect(prefix).toBe('🔓 ❔ 🏰'); }); it('includes epic icon with child count for epic items with children', () => { @@ -167,7 +167,7 @@ describe('Worklog browse pi extension', () => { { id: 'WL-4', title: 'Test', status: 'open', issueType: 'epic', childCount: 5 }, false, ); - expect(prefix).toBe('🔓 ❓ 🏰(5)'); + expect(prefix).toBe('🔓 ❔ 🏰(5)'); }); it('returns text-fallback icons when noIcons=true', () => { @@ -184,26 +184,26 @@ describe('Worklog browse pi extension', () => { const item = { id: 'WL-1', title: 'Simple task', status: 'open' }; // Default (no prefixWidth): no padding (backward compatible) const noPad = formatBrowseOption(item); - expect(noPad).toBe('🔓 ❓ Simple task'); + expect(noPad).toBe('🔓 ❔ Simple task'); // With prefixWidth larger than natural icon width: pads with spaces - // natural visibleWidth of '🔓 ❓' = 5, prefixWidth = 6 → pad(6-5)=1 → repeat(1+1)=2 spaces + // natural visibleWidth of '🔓 ❔' = 5, prefixWidth = 6 → pad(6-5)=1 → repeat(1+1)=2 spaces const padded = formatBrowseOption(item, undefined, undefined, undefined, 6); - expect(padded).toBe('🔓 ❓ Simple task'); + expect(padded).toBe('🔓 ❔ Simple task'); }); it('does not add extra padding when prefixWidth equals natural width', () => { const item = { id: 'WL-1', title: 'Task', status: 'open' }; - // natural visibleWidth of '🔓 ❓' = 5 + // natural visibleWidth of '🔓 ❔' = 5 const result = formatBrowseOption(item, undefined, undefined, undefined, 5); - expect(result).toBe('🔓 ❓ Task'); + expect(result).toBe('🔓 ❔ Task'); }); it('does not add extra padding when prefixWidth is less than natural width', () => { const item = { id: 'WL-1', title: 'Task', status: 'open', stage: 'in_progress' }; - // natural visibleWidth of '🔓 🛠️ ❓' = 8 + // natural visibleWidth of '🔓 🛠️ ❔' = 8 const result = formatBrowseOption(item, undefined, undefined, undefined, 3); - expect(result).toBe('🔓 🛠️ ❓ Task'); + expect(result).toBe('🔓 🛠️ ❔ Task'); }); it('aligns titles at the same column for different icon combinations', () => {