diff --git a/README.md b/README.md index 8019777..fff7fce 100644 --- a/README.md +++ b/README.md @@ -341,7 +341,7 @@ Hooks run with working directory = the **leader session cwd** and receive contex - `PI_TEAMS_MEMBER` - `PI_TEAMS_TASK_ID`, `PI_TEAMS_TASK_SUBJECT`, `PI_TEAMS_TASK_OWNER`, `PI_TEAMS_TASK_STATUS` -See [`docs/hook-contract.md`](docs/hook-contract.md) for the full versioned contract, JSON schema, and compatibility policy. +See [`docs/hook-contract.md`](docs/hook-contract.md) for the full versioned contract, JSON schema, compatibility policy, and runtime version helpers (`checkHookContractVersion`, `parseHookContextSafe`). Hook policy can be controlled by agents at runtime via `teams` tool actions: diff --git a/docs/hook-contract.md b/docs/hook-contract.md index eb18994..e61d0c1 100644 --- a/docs/hook-contract.md +++ b/docs/hook-contract.md @@ -136,6 +136,11 @@ if (ctx.event === "task_failed") { // ✅ Good: check the version for breaking changes const version = parseInt(process.env.PI_TEAMS_HOOK_CONTEXT_VERSION, 10); if (version > 1) { + // Version is newer than what we were written for. + // Additive fields are safe to ignore — only bail on major mismatch. + console.warn(`Hook context version ${version} is newer than expected (1). Proceeding with best-effort parsing.`); +} +if (version < 1) { console.error(`Unsupported hook contract version: ${version}`); process.exit(1); } @@ -148,6 +153,39 @@ assert(Object.keys(ctx).length === 5); // breaks when fields are added const subject = ctx.task.subject; // crashes when task is null ``` +### Runtime version helpers + +For hooks written in TypeScript/JavaScript that import from `pi-agent-teams`: + +```ts +import { + HOOK_CONTRACT_VERSION, + HOOK_CONTRACT_VERSION_MIN, + checkHookContractVersion, + parseHookContextSafe, +} from "@tmustier/pi-agent-teams/extensions/teams/hooks.js"; + +// checkHookContractVersion returns { status, message } +// status: "compatible" | "newer_minor" | "unsupported" +const check = checkHookContractVersion(2); +if (check.status === "unsupported") process.exit(1); +if (check.status === "newer_minor") console.warn(check.message); + +// parseHookContextSafe combines JSON parse + version check +const result = parseHookContextSafe(process.env.PI_TEAMS_HOOK_CONTEXT_JSON); +if (!result.ok) { + console.error(result.error); + process.exit(1); +} +const { payload, versionCheck } = result; +// payload is typed as HookContextPayload; versionCheck.status tells you +// whether additive fields may be present ("newer_minor"). +``` + +These helpers are optional — hooks may be plain shell scripts or standalone +programs that parse the JSON directly. The version check logic is intentionally +simple enough to reimplement in any language. + ## Hook log format Each hook invocation produces a log file in `/hook-logs/` with the structure: diff --git a/extensions/teams/hooks.ts b/extensions/teams/hooks.ts index 45d6337..c5106c9 100644 --- a/extensions/teams/hooks.ts +++ b/extensions/teams/hooks.ts @@ -5,11 +5,35 @@ import { getTeamsHooksDir } from "./paths.js"; import type { TeamTask } from "./task-store.js"; /** - * Hook contract version. Increment on breaking changes only. - * See docs/hook-contract.md for the full compatibility policy. + * Hook contract version. Increment on **breaking** changes only. + * + * Evolution rules (codified from docs/hook-contract.md): + * + * Additive (no bump): + * - New optional fields in context JSON + * - New environment variables + * - New event types + * - New metadata keys + * - Increasing truncation limits + * + * Breaking (bump required): + * - Removing / renaming existing JSON fields + * - Changing a field's type or semantics + * - Reducing truncation limits below current values + * - Changing exit-code semantics + * - Removing environment variables + * + * When bumping: keep the previous version supported for at least one minor + * release, emit a deprecation warning, then remove in the next major. */ export const HOOK_CONTRACT_VERSION = 1; +/** + * Minimum contract version this runtime supports. + * Used by `checkHookContractVersion` to validate version values. + */ +export const HOOK_CONTRACT_VERSION_MIN = 1; + export type TeamsHookEvent = "idle" | "task_completed" | "task_failed"; export type TeamsHookInvocation = { @@ -74,6 +98,102 @@ export type TeamsHookRunResult = { contractVersion: typeof HOOK_CONTRACT_VERSION; }; +/** + * Result of checking a hook contract version against this runtime's range. + * + * - `compatible`: version is within the supported range + * - `newer_minor`: version has the same major but is newer (additive fields OK) + * - `unsupported`: version is outside the supported range (breaking mismatch) + */ +export type HookVersionCheckResult = { + status: "compatible" | "newer_minor" | "unsupported"; + requestedVersion: number; + currentVersion: typeof HOOK_CONTRACT_VERSION; + minVersion: typeof HOOK_CONTRACT_VERSION_MIN; + message: string; +}; + +/** + * Check whether a requested hook contract version is compatible with this + * runtime. Hook authors should call this to fail fast on breaking mismatches + * and tolerate additive changes. + * + * Rules: + * - `version < HOOK_CONTRACT_VERSION_MIN` → unsupported (too old) + * - `version === HOOK_CONTRACT_VERSION` → compatible (exact match) + * - `version > HOOK_CONTRACT_VERSION` → newer_minor (unknown additive + * fields may appear; hooks written defensively will still work) + * - `version` between min and current → compatible (within range) + * + * Hooks should treat `newer_minor` as a warning, not a hard failure. + */ +export function checkHookContractVersion(version: number): HookVersionCheckResult { + if (!Number.isInteger(version) || version < HOOK_CONTRACT_VERSION_MIN) { + return { + status: "unsupported", + requestedVersion: version, + currentVersion: HOOK_CONTRACT_VERSION, + minVersion: HOOK_CONTRACT_VERSION_MIN, + message: `Hook contract version ${version} is below the minimum supported version ${HOOK_CONTRACT_VERSION_MIN}. Update your hook script.`, + }; + } + if (version > HOOK_CONTRACT_VERSION) { + return { + status: "newer_minor", + requestedVersion: version, + currentVersion: HOOK_CONTRACT_VERSION, + minVersion: HOOK_CONTRACT_VERSION_MIN, + message: `Hook contract version ${version} is newer than the current runtime version ${HOOK_CONTRACT_VERSION}. Additive fields may appear; ensure your hook tolerates unknown keys.`, + }; + } + return { + status: "compatible", + requestedVersion: version, + currentVersion: HOOK_CONTRACT_VERSION, + minVersion: HOOK_CONTRACT_VERSION_MIN, + message: `Hook contract version ${version} is compatible with runtime version ${HOOK_CONTRACT_VERSION}.`, + }; +} + +/** + * Safely parse `PI_TEAMS_HOOK_CONTEXT_JSON` from environment, validate the + * version field, and return a typed result. This is the recommended entry + * point for hook scripts that want version-aware parsing. + * + * Returns `{ ok: true, payload, versionCheck }` on success (including + * `newer_minor`), or `{ ok: false, error, versionCheck? }` on failure. + */ +export function parseHookContextSafe(json: string): { + ok: true; + payload: HookContextPayload; + versionCheck: HookVersionCheckResult; +} | { + ok: false; + error: string; + versionCheck?: HookVersionCheckResult; +} { + let parsed: unknown; + try { + parsed = JSON.parse(json); + } catch { + return { ok: false, error: "Failed to parse PI_TEAMS_HOOK_CONTEXT_JSON as JSON." }; + } + if (typeof parsed !== "object" || parsed === null) { + return { ok: false, error: "PI_TEAMS_HOOK_CONTEXT_JSON is not a JSON object." }; + } + const obj = parsed as Record; + if (typeof obj.version !== "number") { + return { ok: false, error: "Missing or non-numeric 'version' field in hook context JSON." }; + } + const versionCheck = checkHookContractVersion(obj.version); + if (versionCheck.status === "unsupported") { + return { ok: false, error: versionCheck.message, versionCheck }; + } + // For compatible and newer_minor: return the payload. + // The caller tolerates unknown keys (per contract policy). + return { ok: true, payload: parsed as HookContextPayload, versionCheck }; +} + function isErrnoException(err: unknown): err is NodeJS.ErrnoException { return typeof err === "object" && err !== null && "code" in err; } diff --git a/scripts/smoke-test.mts b/scripts/smoke-test.mts index 4b2e0a4..fffa08e 100644 --- a/scripts/smoke-test.mts +++ b/scripts/smoke-test.mts @@ -36,7 +36,10 @@ import { getMemberModel, getMemberThinking, shortModelLabel } from "../extension import { getTeamsNamingRules, getTeamsStrings } from "../extensions/teams/teams-style.js"; import { HOOK_CONTRACT_VERSION, + HOOK_CONTRACT_VERSION_MIN, buildHookContextPayload, + checkHookContractVersion, + parseHookContextSafe, getTeamsHookFailureAction, getTeamsHookFollowupOwnerPolicy, getTeamsHookMaxReopensPerTask, @@ -777,6 +780,186 @@ console.log("\n9. teams-hooks (quality gates)"); else process.env.PI_TEAMS_HOOKS_ENABLED = prevEnabled; } +// ── 9b. hook contract version compatibility ───────────────────────── +console.log("\n9b. hook contract version compatibility"); +{ + // --- constants --- + assert(typeof HOOK_CONTRACT_VERSION === "number", "HOOK_CONTRACT_VERSION is a number"); + assert(Number.isInteger(HOOK_CONTRACT_VERSION), "HOOK_CONTRACT_VERSION is an integer"); + assert(HOOK_CONTRACT_VERSION >= 1, "HOOK_CONTRACT_VERSION >= 1"); + assert(typeof HOOK_CONTRACT_VERSION_MIN === "number", "HOOK_CONTRACT_VERSION_MIN is a number"); + assert(Number.isInteger(HOOK_CONTRACT_VERSION_MIN), "HOOK_CONTRACT_VERSION_MIN is an integer"); + assert(HOOK_CONTRACT_VERSION_MIN >= 1, "HOOK_CONTRACT_VERSION_MIN >= 1"); + assert(HOOK_CONTRACT_VERSION_MIN <= HOOK_CONTRACT_VERSION, "HOOK_CONTRACT_VERSION_MIN <= HOOK_CONTRACT_VERSION"); + + // --- checkHookContractVersion --- + + // Exact match → compatible + const exactMatch = checkHookContractVersion(HOOK_CONTRACT_VERSION); + assertEq(exactMatch.status, "compatible", "exact version match is compatible"); + assertEq(exactMatch.requestedVersion, HOOK_CONTRACT_VERSION, "exact match reports requested version"); + assertEq(exactMatch.currentVersion, HOOK_CONTRACT_VERSION, "exact match reports current version"); + assert(exactMatch.message.length > 0, "exact match has a message"); + + // Future version → newer_minor (additive fields OK) + const futureVersion = checkHookContractVersion(HOOK_CONTRACT_VERSION + 1); + assertEq(futureVersion.status, "newer_minor", "future version is newer_minor"); + assertEq(futureVersion.requestedVersion, HOOK_CONTRACT_VERSION + 1, "future version reports requested version"); + assert(futureVersion.message.includes("newer"), "future version message mentions newer"); + + // Far future version → still newer_minor (not unsupported) + const farFuture = checkHookContractVersion(HOOK_CONTRACT_VERSION + 100); + assertEq(farFuture.status, "newer_minor", "far future version is still newer_minor"); + + // Version 0 → unsupported (below minimum) + const versionZero = checkHookContractVersion(0); + assertEq(versionZero.status, "unsupported", "version 0 is unsupported"); + assert(versionZero.message.includes("below"), "version 0 message mentions below minimum"); + + // Negative version → unsupported + const negativeVersion = checkHookContractVersion(-1); + assertEq(negativeVersion.status, "unsupported", "negative version is unsupported"); + + // Non-integer → unsupported + const fractional = checkHookContractVersion(1.5); + assertEq(fractional.status, "unsupported", "fractional version is unsupported"); + + // NaN → unsupported + const nan = checkHookContractVersion(NaN); + assertEq(nan.status, "unsupported", "NaN version is unsupported"); + + // --- parseHookContextSafe --- + + // Valid v1 payload + const validPayload = buildHookContextPayload({ + event: "task_completed", + teamId: "test-team", + teamDir: "/tmp/test", + taskListId: "test-tl", + style: "normal", + memberName: "agent1", + timestamp: "2025-01-01T00:00:00Z", + completedTask: { + id: "42", + subject: "Test subject", + description: "Test desc", + owner: "agent1", + status: "completed", + blocks: [], + blockedBy: [], + metadata: {}, + createdAt: "2025-01-01T00:00:00Z", + updatedAt: "2025-01-01T00:00:00Z", + }, + }); + const validResult = parseHookContextSafe(JSON.stringify(validPayload)); + assert(validResult.ok === true, "parseHookContextSafe accepts valid v1 payload"); + if (validResult.ok) { + assertEq(validResult.payload.version, HOOK_CONTRACT_VERSION, "parsed payload version matches"); + assertEq(validResult.payload.event, "task_completed", "parsed payload event correct"); + assertEq(validResult.payload.task?.id, "42", "parsed payload task.id correct"); + assertEq(validResult.versionCheck.status, "compatible", "parsed payload version check is compatible"); + } + + // Payload with additive fields (simulates future v2 with extra fields) + const additivePayload = { + ...validPayload, + version: HOOK_CONTRACT_VERSION + 1, + newFutureField: "should be tolerated", + team: { ...validPayload.team, newTeamField: true }, + }; + const additiveResult = parseHookContextSafe(JSON.stringify(additivePayload)); + assert(additiveResult.ok === true, "parseHookContextSafe accepts payload with additive fields"); + if (additiveResult.ok) { + assertEq(additiveResult.versionCheck.status, "newer_minor", "additive payload is newer_minor"); + assertEq(additiveResult.payload.event, "task_completed", "additive payload preserves known fields"); + } + + // Payload with version 0 → rejected + const oldPayload = { ...validPayload, version: 0 }; + const oldResult = parseHookContextSafe(JSON.stringify(oldPayload)); + assert(oldResult.ok === false, "parseHookContextSafe rejects version 0"); + if (!oldResult.ok) { + assert(oldResult.versionCheck !== undefined, "rejected payload includes version check"); + assertEq(oldResult.versionCheck?.status, "unsupported", "version 0 is unsupported in parse"); + } + + // Invalid JSON → rejected + const invalidJson = parseHookContextSafe("not json"); + assert(invalidJson.ok === false, "parseHookContextSafe rejects invalid JSON"); + if (!invalidJson.ok) { + assert(invalidJson.error.includes("parse"), "invalid JSON error mentions parsing"); + } + + // Missing version field → rejected + const noVersion = parseHookContextSafe(JSON.stringify({ event: "idle" })); + assert(noVersion.ok === false, "parseHookContextSafe rejects missing version"); + if (!noVersion.ok) { + assert(noVersion.error.includes("version"), "missing version error mentions version"); + } + + // Non-object JSON → rejected + const arrayJson = parseHookContextSafe(JSON.stringify([1, 2, 3])); + assert(arrayJson.ok === false, "parseHookContextSafe rejects non-object JSON"); + + // String version field → rejected + const stringVersion = parseHookContextSafe(JSON.stringify({ ...validPayload, version: "1" })); + assert(stringVersion.ok === false, "parseHookContextSafe rejects string version"); + + // --- version field in buildHookContextPayload --- + const idlePayload = buildHookContextPayload({ + event: "idle", + teamId: "t", + teamDir: "/tmp", + taskListId: "tl", + style: "normal", + }); + assertEq(idlePayload.version, HOOK_CONTRACT_VERSION, "idle payload version matches contract"); + + const failedPayload = buildHookContextPayload({ + event: "task_failed", + teamId: "t", + teamDir: "/tmp", + taskListId: "tl", + style: "normal", + memberName: "w1", + completedTask: { + id: "99", + subject: "Failed task", + description: "", + owner: "w1", + status: "pending", + blocks: [], + blockedBy: [], + metadata: {}, + createdAt: "2025-01-01T00:00:00Z", + updatedAt: "2025-01-01T00:00:00Z", + }, + }); + assertEq(failedPayload.version, HOOK_CONTRACT_VERSION, "failed payload version matches contract"); + assertEq(failedPayload.event, "task_failed", "failed payload event correct"); + // task_failed events typically have status "pending" (reset before hook) + assertEq(failedPayload.task?.status, "pending", "failed payload preserves actual task status"); + + // --- contract payload tolerates unknown keys (additive change simulation) --- + // Demonstrates that JSON.parse + typed access works with extra fields + const futureJson = JSON.stringify({ + version: HOOK_CONTRACT_VERSION, + event: "task_completed", + team: { id: "t", dir: "/tmp", taskListId: "tl", style: "normal", newField: 42 }, + member: "a", + timestamp: null, + task: null, + extraTopLevel: { nested: true }, + }); + const futureParsed = parseHookContextSafe(futureJson); + assert(futureParsed.ok === true, "payload with unknown keys parses successfully"); + if (futureParsed.ok) { + assertEq(futureParsed.payload.team.id, "t", "known fields survive additive changes"); + assertEq(futureParsed.versionCheck.status, "compatible", "current version with extra keys is compatible"); + } +} + // ── 10. team discovery + attach claims ────────────────────────────── console.log("\n10. team discovery + attach claims"); { @@ -1202,6 +1385,8 @@ console.log("\n15. docs/help drift guard"); assert(readme.includes("PI_TEAMS_HOOKS_FAILURE_ACTION"), "README mentions hook failure action policy"); assert(readme.includes("PI_TEAMS_HOOKS_MAX_REOPENS_PER_TASK"), "README mentions hook reopen cap policy"); assert(readme.includes("PI_TEAMS_HOOK_CONTEXT_JSON"), "README mentions hook context json contract"); + assert(readme.includes("checkHookContractVersion"), "README mentions version check helper"); + assert(readme.includes("parseHookContextSafe"), "README mentions safe context parser"); assert(!readme.includes("claude-sonnet-4"), "README avoids deprecated leader model examples"); assert(readme.includes("task-centric view"), "README mentions panel task-centric view"); assert(readme.includes("`t` or `shift+t`"), "README mentions panel task toggle key"); @@ -1214,6 +1399,23 @@ console.log("\n15. docs/help drift guard"); assert(readme.includes("/team gc"), "README mentions /team gc command"); assert(readme.includes("/team cleanup"), "README mentions /team cleanup command"); assert(readme.includes("docs/hook-contract.md"), "README references hook contract doc"); + + // Hook contract doc drift guard + const hookContractPath = path.join(process.cwd(), "docs/hook-contract.md"); + if (fs.existsSync(hookContractPath)) { + const hookDoc = fs.readFileSync(hookContractPath, "utf8"); + assert(hookDoc.includes("Contract version: 1"), "hook-contract.md documents current version"); + assert(hookDoc.includes("Additive changes"), "hook-contract.md documents additive change policy"); + assert(hookDoc.includes("Breaking changes"), "hook-contract.md documents breaking change policy"); + assert(hookDoc.includes("Version lifecycle"), "hook-contract.md documents version lifecycle"); + assert(hookDoc.includes("Hook author guidelines"), "hook-contract.md includes hook author guidelines"); + assert(hookDoc.includes("checkHookContractVersion"), "hook-contract.md documents version check helper"); + assert(hookDoc.includes("parseHookContextSafe"), "hook-contract.md documents safe context parser"); + assert(hookDoc.includes("newer_minor"), "hook-contract.md documents newer_minor version status"); + } else { + console.log(" (skipped) docs/hook-contract.md not found"); + } + assert(readme.includes("member_status"), "README mentions teams tool member_status action"); assert(readme.includes("/team status"), "README mentions /team status command"); assert(readme.includes("PI_TEAMS_STALL_THRESHOLD_MS"), "README mentions stall threshold env var");