diff --git a/drizzle/0020_review_performance.sql b/drizzle/0020_review_performance.sql new file mode 100644 index 0000000..57e2111 --- /dev/null +++ b/drizzle/0020_review_performance.sql @@ -0,0 +1,4 @@ +CREATE INDEX `dependencies_blocked_id_idx` ON `dependencies` (`blocked_id`);--> statement-breakpoint +CREATE INDEX `events_type_issue_id_idx` ON `events` (`type`,`issue_id`);--> statement-breakpoint +CREATE INDEX `issues_parent_id_idx` ON `issues` (`parent_id`);--> statement-breakpoint +CREATE INDEX `issues_project_status_idx` ON `issues` (`project_id`,`status`); \ No newline at end of file diff --git a/drizzle/meta/0020_snapshot.json b/drizzle/meta/0020_snapshot.json new file mode 100644 index 0000000..d14ca72 --- /dev/null +++ b/drizzle/meta/0020_snapshot.json @@ -0,0 +1,1721 @@ +{ + "version": "6", + "dialect": "sqlite", + "id": "6a145efa-7e1b-48e7-b3c0-f0b02a8c9f5c", + "prevId": "1de348bf-c65f-4f20-8d49-7b9ba9c34f00", + "tables": { + "actors": { + "name": "actors", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "type": { + "name": "type", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "attended": { + "name": "attended", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": false + }, + "token_hash": { + "name": "token_hash", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch())" + } + }, + "indexes": { + "actors_name_unique": { + "name": "actors_name_unique", + "columns": ["name"], + "isUnique": true + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "affirmation_keys": { + "name": "affirmation_keys", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "actor_id": { + "name": "actor_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "public_key": { + "name": "public_key", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "comment": { + "name": "comment", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch())" + }, + "revoked_at": { + "name": "revoked_at", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "affirmation_keys_active_uniq": { + "name": "affirmation_keys_active_uniq", + "columns": ["actor_id", "public_key"], + "isUnique": true, + "where": "revoked_at is null" + } + }, + "foreignKeys": { + "affirmation_keys_actor_id_actors_id_fk": { + "name": "affirmation_keys_actor_id_actors_id_fk", + "tableFrom": "affirmation_keys", + "tableTo": "actors", + "columnsFrom": ["actor_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "agent_sessions": { + "name": "agent_sessions", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "issue_id": { + "name": "issue_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "actor_id": { + "name": "actor_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "mode": { + "name": "mode", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "pid": { + "name": "pid", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "status": { + "name": "status", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'running'" + }, + "exit_code": { + "name": "exit_code", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "started_at": { + "name": "started_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch())" + }, + "ended_at": { + "name": "ended_at", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "agent_sessions_issue_id_idx": { + "name": "agent_sessions_issue_id_idx", + "columns": ["issue_id"], + "isUnique": false + } + }, + "foreignKeys": { + "agent_sessions_issue_id_issues_id_fk": { + "name": "agent_sessions_issue_id_issues_id_fk", + "tableFrom": "agent_sessions", + "tableTo": "issues", + "columnsFrom": ["issue_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + }, + "agent_sessions_actor_id_actors_id_fk": { + "name": "agent_sessions_actor_id_actors_id_fk", + "tableFrom": "agent_sessions", + "tableTo": "actors", + "columnsFrom": ["actor_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "attachments": { + "name": "attachments", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "issue_id": { + "name": "issue_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "actor_id": { + "name": "actor_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "filename": { + "name": "filename", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "content_type": { + "name": "content_type", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "size": { + "name": "size", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch())" + } + }, + "indexes": {}, + "foreignKeys": { + "attachments_issue_id_issues_id_fk": { + "name": "attachments_issue_id_issues_id_fk", + "tableFrom": "attachments", + "tableTo": "issues", + "columnsFrom": ["issue_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + }, + "attachments_actor_id_actors_id_fk": { + "name": "attachments_actor_id_actors_id_fk", + "tableFrom": "attachments", + "tableTo": "actors", + "columnsFrom": ["actor_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "claim_lease_cutover": { + "name": "claim_lease_cutover", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "completed_at": { + "name": "completed_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch())" + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "claim_leases": { + "name": "claim_leases", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "issue_id": { + "name": "issue_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "actor_id": { + "name": "actor_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "token_hash": { + "name": "token_hash", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "expires_at": { + "name": "expires_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "last_beat_at": { + "name": "last_beat_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "invalidated_at": { + "name": "invalidated_at", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch())" + } + }, + "indexes": { + "claim_leases_token_hash_unique": { + "name": "claim_leases_token_hash_unique", + "columns": ["token_hash"], + "isUnique": true + }, + "claim_leases_issue_id_idx": { + "name": "claim_leases_issue_id_idx", + "columns": ["issue_id"], + "isUnique": false + } + }, + "foreignKeys": { + "claim_leases_issue_id_issues_id_fk": { + "name": "claim_leases_issue_id_issues_id_fk", + "tableFrom": "claim_leases", + "tableTo": "issues", + "columnsFrom": ["issue_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + }, + "claim_leases_actor_id_actors_id_fk": { + "name": "claim_leases_actor_id_actors_id_fk", + "tableFrom": "claim_leases", + "tableTo": "actors", + "columnsFrom": ["actor_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "delivery_attempts": { + "name": "delivery_attempts", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "issue_ref": { + "name": "issue_ref", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "pr_number": { + "name": "pr_number", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "head_sha": { + "name": "head_sha", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "derived_head_sha": { + "name": "derived_head_sha", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "authorization_id": { + "name": "authorization_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "started_at": { + "name": "started_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch())" + }, + "finished_at": { + "name": "finished_at", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "outcome": { + "name": "outcome", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "delivery_attempts_authorization_id_idx": { + "name": "delivery_attempts_authorization_id_idx", + "columns": ["authorization_id"], + "isUnique": false + }, + "delivery_attempts_issue_ref_idx": { + "name": "delivery_attempts_issue_ref_idx", + "columns": ["issue_ref"], + "isUnique": false + } + }, + "foreignKeys": { + "delivery_attempts_authorization_id_events_id_fk": { + "name": "delivery_attempts_authorization_id_events_id_fk", + "tableFrom": "delivery_attempts", + "tableTo": "events", + "columnsFrom": ["authorization_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "delivery_rollout": { + "name": "delivery_rollout", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "completed_at": { + "name": "completed_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch())" + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "dependencies": { + "name": "dependencies", + "columns": { + "blocker_id": { + "name": "blocker_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "blocked_id": { + "name": "blocked_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "dependencies_blocked_id_idx": { + "name": "dependencies_blocked_id_idx", + "columns": ["blocked_id"], + "isUnique": false + } + }, + "foreignKeys": { + "dependencies_blocker_id_issues_id_fk": { + "name": "dependencies_blocker_id_issues_id_fk", + "tableFrom": "dependencies", + "tableTo": "issues", + "columnsFrom": ["blocker_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + }, + "dependencies_blocked_id_issues_id_fk": { + "name": "dependencies_blocked_id_issues_id_fk", + "tableFrom": "dependencies", + "tableTo": "issues", + "columnsFrom": ["blocked_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": { + "dependencies_blocker_id_blocked_id_pk": { + "columns": ["blocker_id", "blocked_id"], + "name": "dependencies_blocker_id_blocked_id_pk" + } + }, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "events": { + "name": "events", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "issue_id": { + "name": "issue_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "actor_id": { + "name": "actor_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "type": { + "name": "type", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "payload": { + "name": "payload", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'{}'" + }, + "via_agent_id": { + "name": "via_agent_id", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "session_id": { + "name": "session_id", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch())" + } + }, + "indexes": { + "events_issue_id_idx": { + "name": "events_issue_id_idx", + "columns": ["issue_id"], + "isUnique": false + }, + "events_type_issue_id_idx": { + "name": "events_type_issue_id_idx", + "columns": ["type", "issue_id"], + "isUnique": false + } + }, + "foreignKeys": { + "events_issue_id_issues_id_fk": { + "name": "events_issue_id_issues_id_fk", + "tableFrom": "events", + "tableTo": "issues", + "columnsFrom": ["issue_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + }, + "events_actor_id_actors_id_fk": { + "name": "events_actor_id_actors_id_fk", + "tableFrom": "events", + "tableTo": "actors", + "columnsFrom": ["actor_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + }, + "events_via_agent_id_actors_id_fk": { + "name": "events_via_agent_id_actors_id_fk", + "tableFrom": "events", + "tableTo": "actors", + "columnsFrom": ["via_agent_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + }, + "events_session_id_sessions_id_fk": { + "name": "events_session_id_sessions_id_fk", + "tableFrom": "events", + "tableTo": "sessions", + "columnsFrom": ["session_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "github_repos": { + "name": "github_repos", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "full_name": { + "name": "full_name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "project_id": { + "name": "project_id", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "secret": { + "name": "secret", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch())" + } + }, + "indexes": { + "github_repos_full_name_unique": { + "name": "github_repos_full_name_unique", + "columns": ["full_name"], + "isUnique": true + } + }, + "foreignKeys": { + "github_repos_project_id_projects_id_fk": { + "name": "github_repos_project_id_projects_id_fk", + "tableFrom": "github_repos", + "tableTo": "projects", + "columnsFrom": ["project_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "issues": { + "name": "issues", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "project_id": { + "name": "project_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "number": { + "name": "number", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "title": { + "name": "title", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "description": { + "name": "description", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "''" + }, + "summary": { + "name": "summary", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "status": { + "name": "status", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "priority": { + "name": "priority", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'none'" + }, + "assignee_id": { + "name": "assignee_id", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "creator_id": { + "name": "creator_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "parent_id": { + "name": "parent_id", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "labels": { + "name": "labels", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'[]'" + }, + "source_type": { + "name": "source_type", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "source_detail": { + "name": "source_detail", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "source_url": { + "name": "source_url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "needs_input": { + "name": "needs_input", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": false + }, + "snoozed_until": { + "name": "snoozed_until", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "worker_preference": { + "name": "worker_preference", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "queue_rank": { + "name": "queue_rank", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch())" + }, + "updated_at": { + "name": "updated_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch())" + } + }, + "indexes": { + "issues_project_id_idx": { + "name": "issues_project_id_idx", + "columns": ["project_id"], + "isUnique": false + }, + "issues_status_idx": { + "name": "issues_status_idx", + "columns": ["status"], + "isUnique": false + }, + "issues_assignee_id_idx": { + "name": "issues_assignee_id_idx", + "columns": ["assignee_id"], + "isUnique": false + }, + "issues_queue_rank_idx": { + "name": "issues_queue_rank_idx", + "columns": ["queue_rank"], + "isUnique": false + }, + "issues_parent_id_idx": { + "name": "issues_parent_id_idx", + "columns": ["parent_id"], + "isUnique": false + }, + "issues_project_status_idx": { + "name": "issues_project_status_idx", + "columns": ["project_id", "status"], + "isUnique": false + } + }, + "foreignKeys": { + "issues_project_id_projects_id_fk": { + "name": "issues_project_id_projects_id_fk", + "tableFrom": "issues", + "tableTo": "projects", + "columnsFrom": ["project_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + }, + "issues_assignee_id_actors_id_fk": { + "name": "issues_assignee_id_actors_id_fk", + "tableFrom": "issues", + "tableTo": "actors", + "columnsFrom": ["assignee_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + }, + "issues_creator_id_actors_id_fk": { + "name": "issues_creator_id_actors_id_fk", + "tableFrom": "issues", + "tableTo": "actors", + "columnsFrom": ["creator_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + }, + "issues_parent_id_issues_id_fk": { + "name": "issues_parent_id_issues_id_fk", + "tableFrom": "issues", + "tableTo": "issues", + "columnsFrom": ["parent_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "login_links": { + "name": "login_links", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "token_hash": { + "name": "token_hash", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "actor_id": { + "name": "actor_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "expires_at": { + "name": "expires_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "used_at": { + "name": "used_at", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch())" + } + }, + "indexes": { + "login_links_token_hash_unique": { + "name": "login_links_token_hash_unique", + "columns": ["token_hash"], + "isUnique": true + } + }, + "foreignKeys": { + "login_links_actor_id_actors_id_fk": { + "name": "login_links_actor_id_actors_id_fk", + "tableFrom": "login_links", + "tableTo": "actors", + "columnsFrom": ["actor_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "pending_actions": { + "name": "pending_actions", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "session_id": { + "name": "session_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "issue_id": { + "name": "issue_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "action_type": { + "name": "action_type", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "payload": { + "name": "payload", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'{}'" + }, + "status": { + "name": "status", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'pending'" + }, + "affirmed_by_id": { + "name": "affirmed_by_id", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "affirmed_at": { + "name": "affirmed_at", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch())" + }, + "expires_at": { + "name": "expires_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + } + }, + "indexes": { + "pending_actions_active_uniq": { + "name": "pending_actions_active_uniq", + "columns": ["session_id", "issue_id", "action_type"], + "isUnique": true, + "where": "status = 'pending'" + } + }, + "foreignKeys": { + "pending_actions_session_id_sessions_id_fk": { + "name": "pending_actions_session_id_sessions_id_fk", + "tableFrom": "pending_actions", + "tableTo": "sessions", + "columnsFrom": ["session_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + }, + "pending_actions_issue_id_issues_id_fk": { + "name": "pending_actions_issue_id_issues_id_fk", + "tableFrom": "pending_actions", + "tableTo": "issues", + "columnsFrom": ["issue_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + }, + "pending_actions_affirmed_by_id_actors_id_fk": { + "name": "pending_actions_affirmed_by_id_actors_id_fk", + "tableFrom": "pending_actions", + "tableTo": "actors", + "columnsFrom": ["affirmed_by_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "pr_links": { + "name": "pr_links", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "issue_id": { + "name": "issue_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "repo": { + "name": "repo", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "pr_number": { + "name": "pr_number", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "role": { + "name": "role", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "declared_by": { + "name": "declared_by", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "declared_at": { + "name": "declared_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch())" + }, + "confirmed_by": { + "name": "confirmed_by", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "confirmed_at": { + "name": "confirmed_at", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "revoked_at": { + "name": "revoked_at", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": { + "pr_links_live_idx": { + "name": "pr_links_live_idx", + "columns": ["issue_id", "repo", "pr_number"], + "isUnique": true, + "where": "\"pr_links\".\"revoked_at\" IS NULL" + }, + "pr_links_pr_idx": { + "name": "pr_links_pr_idx", + "columns": ["repo", "pr_number"], + "isUnique": false + }, + "pr_links_issue_idx": { + "name": "pr_links_issue_idx", + "columns": ["issue_id"], + "isUnique": false + } + }, + "foreignKeys": { + "pr_links_issue_id_issues_id_fk": { + "name": "pr_links_issue_id_issues_id_fk", + "tableFrom": "pr_links", + "tableTo": "issues", + "columnsFrom": ["issue_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + }, + "pr_links_declared_by_actors_id_fk": { + "name": "pr_links_declared_by_actors_id_fk", + "tableFrom": "pr_links", + "tableTo": "actors", + "columnsFrom": ["declared_by"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + }, + "pr_links_confirmed_by_actors_id_fk": { + "name": "pr_links_confirmed_by_actors_id_fk", + "tableFrom": "pr_links", + "tableTo": "actors", + "columnsFrom": ["confirmed_by"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "pr_state": { + "name": "pr_state", + "columns": { + "repo": { + "name": "repo", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "pr_number": { + "name": "pr_number", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "branch": { + "name": "branch", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "issue_ref": { + "name": "issue_ref", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "status": { + "name": "status", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "head_sha": { + "name": "head_sha", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "gh_updated_at": { + "name": "gh_updated_at", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "url": { + "name": "url", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "last_transition_event_id": { + "name": "last_transition_event_id", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "updated_at": { + "name": "updated_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch())" + } + }, + "indexes": { + "pr_state_issue_ref_idx": { + "name": "pr_state_issue_ref_idx", + "columns": ["issue_ref"], + "isUnique": false + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": { + "pr_state_repo_pr_number_pk": { + "columns": ["repo", "pr_number"], + "name": "pr_state_repo_pr_number_pk" + } + }, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "projects": { + "name": "projects", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "key": { + "name": "key", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "name": { + "name": "name", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "next_issue_number": { + "name": "next_issue_number", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 1 + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch())" + } + }, + "indexes": { + "projects_key_unique": { + "name": "projects_key_unique", + "columns": ["key"], + "isUnique": true + } + }, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "sessions": { + "name": "sessions", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "token_hash": { + "name": "token_hash", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "actor_id": { + "name": "actor_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "kind": { + "name": "kind", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "'plain'" + }, + "via_agent_id": { + "name": "via_agent_id", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "closed_at": { + "name": "closed_at", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "expires_at": { + "name": "expires_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch())" + } + }, + "indexes": { + "sessions_token_hash_unique": { + "name": "sessions_token_hash_unique", + "columns": ["token_hash"], + "isUnique": true + } + }, + "foreignKeys": { + "sessions_actor_id_actors_id_fk": { + "name": "sessions_actor_id_actors_id_fk", + "tableFrom": "sessions", + "tableTo": "actors", + "columnsFrom": ["actor_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + }, + "sessions_via_agent_id_actors_id_fk": { + "name": "sessions_via_agent_id_actors_id_fk", + "tableFrom": "sessions", + "tableTo": "actors", + "columnsFrom": ["via_agent_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "settings": { + "name": "settings", + "columns": { + "key": { + "name": "key", + "type": "text", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "value": { + "name": "value", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "updated_at": { + "name": "updated_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch())" + }, + "updated_by_actor_id": { + "name": "updated_by_actor_id", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + } + }, + "indexes": {}, + "foreignKeys": { + "settings_updated_by_actor_id_actors_id_fk": { + "name": "settings_updated_by_actor_id_actors_id_fk", + "tableFrom": "settings", + "tableTo": "actors", + "columnsFrom": ["updated_by_actor_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "webhook_cursor": { + "name": "webhook_cursor", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": false + }, + "last_event_id": { + "name": "last_event_id", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": 0 + } + }, + "indexes": {}, + "foreignKeys": {}, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + }, + "webhooks": { + "name": "webhooks", + "columns": { + "id": { + "name": "id", + "type": "integer", + "primaryKey": true, + "notNull": true, + "autoincrement": true + }, + "url": { + "name": "url", + "type": "text", + "primaryKey": false, + "notNull": true, + "autoincrement": false + }, + "project_id": { + "name": "project_id", + "type": "integer", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "secret": { + "name": "secret", + "type": "text", + "primaryKey": false, + "notNull": false, + "autoincrement": false + }, + "active": { + "name": "active", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": true + }, + "created_at": { + "name": "created_at", + "type": "integer", + "primaryKey": false, + "notNull": true, + "autoincrement": false, + "default": "(unixepoch())" + } + }, + "indexes": {}, + "foreignKeys": { + "webhooks_project_id_projects_id_fk": { + "name": "webhooks_project_id_projects_id_fk", + "tableFrom": "webhooks", + "tableTo": "projects", + "columnsFrom": ["project_id"], + "columnsTo": ["id"], + "onDelete": "no action", + "onUpdate": "no action" + } + }, + "compositePrimaryKeys": {}, + "uniqueConstraints": {}, + "checkConstraints": {} + } + }, + "views": {}, + "enums": {}, + "_meta": { + "schemas": {}, + "tables": {}, + "columns": {} + }, + "internal": { + "indexes": {} + } +} diff --git a/drizzle/meta/_journal.json b/drizzle/meta/_journal.json index bdc4501..d5be6bd 100644 --- a/drizzle/meta/_journal.json +++ b/drizzle/meta/_journal.json @@ -141,6 +141,13 @@ "when": 1785273879961, "tag": "0019_boring_pet_avengers", "breakpoints": true + }, + { + "idx": 20, + "version": "6", + "when": 1790559366825, + "tag": "0020_review_performance", + "breakpoints": true } ] -} \ No newline at end of file +} diff --git a/scripts/agent-worker.ts b/scripts/agent-worker.ts index 6ca9f19..227e8d2 100644 --- a/scripts/agent-worker.ts +++ b/scripts/agent-worker.ts @@ -111,6 +111,12 @@ import { type RunningContainerSessionRow, } from "./worker-select.js"; import { acquirePidLock, isLocked } from "./pidfile.js"; +import { + savePublication, + readPublications, + clearPublication, + type PendingPublication, +} from "./worker-publications.js"; import { publishAgentBranch, prFreshness, originOwnerRepo } from "./delivery-exec.js"; import { agentBranch, @@ -690,7 +696,81 @@ function killSession(child: ChildProcess, containerName: string | null): void { * that survives a worker restart still needs its exit reported and its * branch published, not just silently forgotten. */ -function finishSessionExit( +function publishesContainers(config: WorkerConfig): boolean { + return !!(config.containerized && config.delivery && config.delivery.openPrs !== false); +} + +// Deduplicate the normal exit handler and the polling recovery pass. +const publishing = new Set(); + +async function publishPending( + publication: PendingPublication, + project: WorkerProject, + config: WorkerConfig, + token: string, + logLine: (text: string) => void, + knownLiveNames?: Set, +): Promise { + const { ref, issueTitle, sessionId, exitCode } = publication; + const key = `${project.repo}:${ref}`; + if (publishing.has(key)) return; + publishing.add(key); + try { + // A local docker run/wait client can exit while its daemon-side container + // survives. Only confirmed container disappearance permits completion; + // an inspection error leaves the durable obligation intact for retry. + const liveNames = knownLiveNames ?? (await listLiveContainerNames()); + if (liveNames.has(containerNameFor(ref))) return; + await reportSessionEnd(config, token, Promise.resolve(sessionId), exitCode); + const outcome = await publishAgentBranch(project.repo, ref, issueTitle, config.url, exitCode); + const line = formatPublishOutcome(agentBranch(ref), outcome); + console.log(`${ref}: ${line}`); + logLine(`[worker] ${line}\n`); + if ( + (outcome.status === "opened" || outcome.status === "already-open") && + outcome.prNumber !== null + ) { + const event: Extract = { + type: "pr_opened", + prNumber: outcome.prNumber, + url: outcome.url, + }; + try { + event.repo = await originOwnerRepo(project.repo); + } catch (err) { + console.error(`could not resolve origin repo for ${ref}: ${(err as Error).message}`); + } + try { + const fresh = await prFreshness(project.repo, outcome.prNumber); + event.headSha = fresh.headSha; + event.ghUpdatedAt = fresh.ghUpdatedAt; + } catch (err) { + console.error(`could not fetch PR freshness for ${ref}: ${(err as Error).message}`); + } + // The outbox survives until both the PR and its tracker observation + // exist. A crash/failed POST retries the idempotent already-open path. + await postDeliveryEvent(config, token, ref, event); + } + clearPublication(project.repo, config, ref); + } catch (err) { + const message = (err as Error).message; + console.error(`publish failed for ${ref}: ${message}`); + logLine(`[worker] publish failed: ${message}\n`); + await postComment(config, token, ref, publishFailureComment(ref, message)).catch((e: Error) => + console.error(`could not comment publish failure for ${ref}: ${e.message}`), + ); + await postDeliveryEvent(config, token, ref, { + type: "delivery_failed", + message: `publish failed: ${message}`, + }).catch((e: Error) => + console.error(`could not record delivery_failed for ${ref}: ${e.message}`), + ); + } finally { + publishing.delete(key); + } +} + +async function finishSessionExit( ref: string, issueTitle: string, project: WorkerProject, @@ -699,81 +779,43 @@ function finishSessionExit( sessionId: Promise, exitCode: number | null, logLine: (text: string) => void, -): void { - void reportSessionEnd(config, token, sessionId, exitCode, (message) => - logLine(`[worker] ${message}\n`), - ); + knownLiveNames?: Set, +): Promise { console.log(`${ref} exited with code ${exitCode}`); logLine(`\n[worker] exited with code ${exitCode}\n`); - // Delivery gate (SYD-49): a containerized session that pushed agent/ - // gets its branch published to GitHub as a PR, host-side (gh + git auth - // live here, never in the container). Merging still waits for a human - // done-stamp via scripts/deliver.ts. Publish fires on commit count alone, - // independent of a clean exit (SYD-118) — an errored/killed session that - // still committed partial work opens a PR too, so `exitCode` is passed - // through to mark the PR body when it isn't a clean exit. - if (config.containerized && config.delivery && config.delivery.openPrs !== false) { - publishAgentBranch(project.repo, ref, issueTitle, config.url, exitCode) - .then(async (outcome) => { - const line = formatPublishOutcome(agentBranch(ref), outcome); - console.log(`${ref}: ${line}`); - logLine(`[worker] ${line}\n`); - if ( - (outcome.status === "opened" || outcome.status === "already-open") && - outcome.prNumber !== null - ) { - // SYD-205: name the repo and GitHub's own head SHA/updated_at on - // the publish so pr_state has a producer at open time. Best-effort: - // a failed lookup must never drop the pr_opened publish itself — - // this event is what closes the claim gate. - const event: Extract = { - type: "pr_opened", - prNumber: outcome.prNumber, - url: outcome.url, - }; - try { - event.repo = await originOwnerRepo(project.repo); - } catch (err) { - console.error(`could not resolve origin repo for ${ref}: ${(err as Error).message}`); - } - try { - const fresh = await prFreshness(project.repo, outcome.prNumber); - event.headSha = fresh.headSha; - event.ghUpdatedAt = fresh.ghUpdatedAt; - } catch (err) { - console.error( - `could not fetch PR freshness for ${ref} #${outcome.prNumber}: ${(err as Error).message}`, - ); - } - postDeliveryEvent(config, token, ref, event).catch((err: Error) => { - console.error(`could not record pr_opened event for ${ref}: ${err.message}`); - logLine(`[worker] could not record pr_opened event: ${err.message}\n`); - }); - } - }) - .catch((err: Error) => { - console.error(`publish failed for ${ref}: ${err.message}`); - logLine(`[worker] publish failed: ${err.message}\n`); - // SYD-257: previously this failure lived only in the local worker - // log — the issue itself, already moved to in_review by the session, - // showed no sign anything was wrong. Post actor-visible provenance - // (the git/gh stderr) and a delivery_failed event so the attention - // banner lights up instead of deliver.ts (and any human) waiting on - // a PR that was never opened. - postComment(config, token, ref, publishFailureComment(ref, err.message)).catch( - (e: Error) => { - console.error(`could not comment publish failure for ${ref}: ${e.message}`); - logLine(`[worker] could not comment publish failure: ${e.message}\n`); - }, - ); - postDeliveryEvent(config, token, ref, { - type: "delivery_failed", - message: `publish failed: ${err.message}`, - }).catch((e: Error) => { - console.error(`could not record delivery_failed event for ${ref}: ${e.message}`); - logLine(`[worker] could not record delivery_failed event: ${e.message}\n`); - }); - }); + if (!publishesContainers(config)) { + await reportSessionEnd(config, token, sessionId, exitCode); + return; + } + try { + const publication = { ref, issueTitle, sessionId: (await sessionId) ?? null, exitCode }; + // Save before closing the tracker session: if the process stops after + // the PATCH, the server no longer lists it as active but this survives. + savePublication(project.repo, config, publication); + await publishPending(publication, project, config, token, logLine, knownLiveNames); + } catch (err) { + // Leave the server session active when persistence fails, so restart + // reconciliation can still discover it rather than forgetting the work. + console.error(`could not persist publication for ${ref}: ${(err as Error).message}`); + } +} + +export async function recoverPublications( + config: WorkerConfig, + token: string, + knownLiveNames?: Set, +): Promise { + if (!publishesContainers(config)) return; + const pending = Object.values(config.projects).flatMap((project) => + readPublications(project.repo, config).map((publication) => ({ project, publication })), + ); + if (pending.length === 0) return; + // A marker is written before spawning too. Do not publish an in-flight + // container, including one whose server session was already swept away. + const liveNames = knownLiveNames ?? (await listLiveContainerNames()); + for (const { project, publication } of pending) { + if (active.has(publication.ref) || liveNames.has(containerNameFor(publication.ref))) continue; + await publishPending(publication, project, config, token, () => {}, liveNames); } } @@ -831,8 +873,28 @@ export function dispatch( let cliMcpTmpDir: string | null = null; let child: ChildProcess; + let publicationSaved = false; + const clearNeverStartedPublication = () => { + if (!publicationSaved) return; + try { + clearPublication(project.repo, config, issue.ref); + } catch (err) { + console.error( + `could not clear unstarted publication for ${issue.ref}: ${(err as Error).message}`, + ); + } + }; try { if (config.containerized) { + if (publishesContainers(config)) { + savePublication(project.repo, config, { + ref: issue.ref, + issueTitle: issue.title, + sessionId: null, + exitCode: null, + }); + publicationSaved = true; + } // The container is the sandbox: it clones the repo internally, works on // a branch, and pushes it back out — it never touches this host // filesystem beyond the /origin mount. See scripts/container-entry.sh. @@ -900,6 +962,7 @@ export function dispatch( } } catch (err) { console.error(`failed to dispatch ${issue.ref}: ${(err as Error).message}`); + clearNeverStartedPublication(); // The host already claimed this issue; setup failed before any heartbeat // could start, so release the lease-held claim rather than strand it. if (cliMcpTmpDir) rmSync(cliMcpTmpDir, { recursive: true, force: true }); @@ -951,7 +1014,9 @@ export function dispatch( // 'spawn' only fires once the OS actually launched the process (see the // SYD-74 note in dispatchAnswer) — a failed spawn never creates a session. let sessionId: Promise = Promise.resolve(null); + let spawned = false; child.on("spawn", () => { + spawned = true; sessionId = reportSessionStart( config, token, @@ -971,10 +1036,20 @@ export function dispatch( active.delete(issue.ref); activeMode.delete(issue.ref); if (roleRunsAnswer(role)) triggerUnansweredDrain(config, token); - finishSessionExit(issue.ref, issue.title, project, config, token, sessionId, code, logLine); + void finishSessionExit( + issue.ref, + issue.title, + project, + config, + token, + sessionId, + code, + logLine, + ); }); child.on("error", (err) => { + if (!spawned) clearNeverStartedPublication(); clearTimeout(watchdog); stopHeartbeat(); cleanupLeaseArtifacts(); @@ -1301,8 +1376,16 @@ export async function runTick( await runGated(tickGate, async () => { if (roleRunsCode(role)) { try { + if (!opts.dryRun) await recoverPublications(config, token); + const pendingRefs = new Set( + Object.values(config.projects).flatMap((project) => + readPublications(project.repo, config).map((publication) => publication.ref), + ), + ); const issues = await fetchReadyIssues(config, token); - const eligible = filterRetryCapped(issues, retryState); + const eligible = filterRetryCapped(issues, retryState).filter( + (issue) => !pendingRefs.has(issue.ref), + ); const selected = selectDispatchable(eligible, config, active.keys()); for (const issue of selected) { @@ -1545,7 +1628,7 @@ export function adoptContainerSession( activeMode.delete(session.ref); const parsed = Number.parseInt(output.trim(), 10); const exitCode = Number.isFinite(parsed) ? parsed : null; - finishSessionExit( + void finishSessionExit( session.ref, session.issueTitle, project, @@ -1617,11 +1700,24 @@ export async function reconcileContainerSessions( console.log( `reconcile: ${session.ref} has no running container — closing stuck session ${session.id}`, ); - await reportSessionEnd(config, token, Promise.resolve(session.id), null); + const project = config.projects[projectKeyOf(session.ref)]; + if (!project) continue; + await finishSessionExit( + session.ref, + session.issueTitle, + project, + config, + token, + Promise.resolve(session.id), + null, + () => {}, + liveNames, + ); } for (const session of live) { adopt(session, config, token); } + await recoverPublications(config, token, liveNames); } /** diff --git a/scripts/deliver.ts b/scripts/deliver.ts index 94b6753..903212b 100644 --- a/scripts/deliver.ts +++ b/scripts/deliver.ts @@ -961,10 +961,10 @@ async function tick( token: string, gate: ReturnType, dryRun: boolean, - // SYD-284: the projects branch protection allows us to deliver for. Defaults - // to every configured project, so a caller that doesn't opt into the gate - // behaves exactly as before. - deliverableKeys: string[] = Object.keys(config.projects), + // Omitting the verified scope must fail closed when protection is required. + deliverableKeys: string[] = config.delivery?.requireBranchProtection + ? [] + : Object.keys(config.projects), ): Promise { await runGated(gate, async () => { const url = `${apiBase(config)}/api/delivery-work`; @@ -1039,6 +1039,24 @@ export async function warnOnRelaxedBranchProtection(config: WorkerConfig): Promi return failing; } +/** Keep the verified project scope attached to every scheduled tick. */ +export function startDeliveryPolling( + config: WorkerConfig, + token: string, + gate: ReturnType, + dryRun: boolean, + deliverableKeys: string[], +): ReturnType { + return setInterval( + () => { + tick(config, token, gate, dryRun, deliverableKeys).catch((err) => + console.error(`delivery tick failed: ${(err as Error).message}`), + ); + }, + (config.delivery?.pollSeconds ?? DEFAULT_POLL_SECONDS) * 1000, + ); +} + async function main(): Promise { const args = process.argv.slice(2); const once = args.includes("--once"); @@ -1108,11 +1126,7 @@ async function main(): Promise { console.log( `delivery worker polling every ${pollSeconds}s (projects: ${Object.keys(config.projects).join(", ")})`, ); - const timer = setInterval(() => { - tick(config, token, gate, dryRun).catch((err) => - console.error(`delivery tick failed: ${(err as Error).message}`), - ); - }, pollSeconds * 1000); + const timer = startDeliveryPolling(config, token, gate, dryRun, deliverableKeys); const stop = () => { clearInterval(timer); diff --git a/scripts/delivery-exec.ts b/scripts/delivery-exec.ts index 26337b5..1b39af1 100644 --- a/scripts/delivery-exec.ts +++ b/scripts/delivery-exec.ts @@ -158,9 +158,12 @@ export async function publishAgentBranch( ): Promise { const branch = agentBranch(ref); try { - await runGit(["-C", repo, "rev-parse", "--verify", `refs/heads/${branch}`]); - } catch { - return { status: "no-branch" }; + await runGit(["-C", repo, "show-ref", "--verify", "--quiet", `refs/heads/${branch}`]); + } catch (err) { + // Exit 1 means the ref is absent. Invalid repositories, unreadable files + // and other Git errors must remain retryable publication obligations. + if ((err as { code?: number | string }).code === 1) return { status: "no-branch" }; + throw err; } const ahead = await runGit(["-C", repo, "rev-list", `${MAIN_BRANCH}..${branch}`, "--count"]); if (ahead === "0") return { status: "no-commits" }; @@ -409,8 +412,26 @@ export async function attemptAutoRebase( await ensureCleanClone(repo, cloneDir); try { await runGit(["-C", cloneDir, ...buildFetchAgentBranchArgs(ref)]); - } catch { - return { status: "no-branch" }; + } catch (fetchError) { + // A failed fetch can be a transport or local filesystem problem. Only + // ls-remote's documented "no matching refs" exit code proves absence; + // callers close the PR for no-branch, so uncertainty must not reach it. + try { + await runGit([ + "-C", + cloneDir, + "ls-remote", + "--exit-code", + "--heads", + "origin", + `refs/heads/${agentBranch(ref)}`, + ]); + } catch (lookupError) { + if ((lookupError as { code?: number | string }).code === 2) { + return { status: "no-branch" }; + } + } + throw fetchError; } // The live anchor: compare the head GitHub actually has for the branch // against the heads the worker authorized. FETCH_HEAD is the just-fetched @@ -422,13 +443,16 @@ export async function attemptAutoRebase( await runGit(["-C", cloneDir, ...buildCheckoutRebaseBranchArgs(ref)]); try { await runGit(["-C", cloneDir, ...buildRebaseOntoMainArgs()]); - } catch { + } catch (rebaseError) { const filesOut = await runGit(["-C", cloneDir, ...buildConflictFilesArgs()]).catch(() => ""); const files = filesOut .split("\n") .map((f) => f.trim()) .filter(Boolean); await runGit(["-C", cloneDir, ...buildRebaseAbortArgs()]).catch(() => {}); + // Signing failures, full disks and interrupted Git state are not code + // conflicts. Preserve the error and the remote PR/branch in those cases. + if (files.length === 0) throw rebaseError; return { status: "conflict", files }; } await runGit(["-C", cloneDir, ...buildForcePushWithLeaseArgs(ref)]); diff --git a/scripts/github-poll-lib.ts b/scripts/github-poll-lib.ts index 8a4eef7..8d7a90b 100644 --- a/scripts/github-poll-lib.ts +++ b/scripts/github-poll-lib.ts @@ -56,7 +56,7 @@ export type PollEvent = | { event: "pull_request"; payload: Record } | { event: "check_suite"; payload: Record }; -function prPayload(pr: GhPr, action: "opened" | "closed"): Record { +function prPayload(pr: GhPr, action: "opened" | "closed" | "reopened"): Record { return { action, pull_request: { @@ -89,7 +89,8 @@ function checkSuitePayload(pr: GhPr, run: GhRun): Record { /** * Upsert-observed-state (SYD-206, replacing the old emit-transitions diff): * EVERY PR in the poll result yields a pull_request observation on every - * tick — "opened" for OPEN, "closed" (with the `merged` flag) for + * tick — "opened" for OPEN ("reopened" after a confirmed close→open), + * "closed" (with the `merged` flag) for * CLOSED/MERGED, including PRs first observed already terminal. The server's * upsertPrState/dedupe absorbs repeats, so a lost POST heals within one tick * and a repo linked after its PRs settled still gets correct terminal state @@ -106,6 +107,7 @@ export function observeRepoState( prs: GhPr[], runs: Map, prior: RepoPollState, + confirmedReopened: ReadonlySet = new Set(), ): { events: PollEvent[]; next: RepoPollState } { const events: PollEvent[] = []; const next: RepoPollState = { ...prior }; @@ -114,7 +116,14 @@ export function observeRepoState( const known = prior[pr.number]; events.push({ event: "pull_request", - payload: prPayload(pr, pr.state === "OPEN" ? "opened" : "closed"), + payload: prPayload( + pr, + pr.state === "OPEN" + ? known?.state === "CLOSED" || confirmedReopened.has(pr.number) + ? "reopened" + : "opened" + : "closed", + ), }); let lastRunConclusion = known?.lastRunConclusion ?? null; diff --git a/scripts/github-poll.ts b/scripts/github-poll.ts index f82d39b..a05dcc9 100644 --- a/scripts/github-poll.ts +++ b/scripts/github-poll.ts @@ -41,6 +41,7 @@ import { type PollStateFile, type PollEvent, type RepoPollState, + type GhPr, } from "./github-poll-lib.js"; import { listPullRequests, @@ -122,7 +123,7 @@ export async function fetchProjects( return (await res.json()) as { id: number; key: string }[]; } -type GithubEventOutcome = { duplicate?: boolean }; +type GithubEventOutcome = { duplicate?: boolean; reason?: string }; export async function postGithubEvent( url: string, @@ -141,22 +142,29 @@ export async function postGithubEvent( return (await res.json().catch(() => ({}))) as GithubEventOutcome; } -/** Open pr_state rows for a repo, from GET /api/pr-state — the refresh - * work-list. Tolerant of an older server without the endpoint (deploy skew) +type StoredPrState = { prNumber: number; status: "open" | "closed" | "merged" }; + +/** Server PR state is the reconciliation baseline even when the poller's + * local state was deleted or the PR closed and reopened between polls. + * Tolerant of an older server without the endpoint (deploy skew) * and of transport errors: no list just means no targeted refresh this * tick. */ -async function fetchOpenPrNumbers(url: string, token: string, repo: string): Promise { +async function fetchPrState(url: string, token: string, repo: string): Promise { try { const res = await fetch( - `${url.replace(/\/$/, "")}/api/pr-state?repo=${encodeURIComponent(repo)}&status=open`, + `${url.replace(/\/$/, "")}/api/pr-state?repo=${encodeURIComponent(repo)}`, { headers: { authorization: `Bearer ${token}` } }, ); if (!res.ok) return []; const rows: unknown = await res.json(); if (!Array.isArray(rows)) return []; - return rows - .map((r) => Number((r as { prNumber?: unknown }).prNumber)) - .filter((n) => Number.isInteger(n)); + return rows.filter( + (r): r is StoredPrState => + r !== null && + typeof r === "object" && + Number.isInteger(r.prNumber) && + ["open", "closed", "merged"].includes(r.status), + ); } catch { return []; } @@ -171,17 +179,47 @@ export async function pollRepo( state: PollStateFile, dryRun: boolean, ): Promise { - const prs = await listPullRequests(fullName); + const windowPrs = await listPullRequests(fullName); const repoState: RepoPollState = state[fullName] ?? {}; + const storedRows = await fetchPrState(config.url, token, fullName); + const storedByNumber = new Map(storedRows.map((row) => [row.prNumber, row.status])); + const prs: GhPr[] = []; + const confirmedReopened = new Set(); + + // Search results can lag the PR itself. Never turn a stale OPEN search hit + // into an explicit reopened action without a targeted live read. The + // server baseline also repairs a reset local state file or a close/reopen + // episode that happened entirely between polls. + for (const pr of windowPrs) { + if ( + pr.state === "OPEN" && + (repoState[pr.number]?.state === "CLOSED" || storedByNumber.get(pr.number) === "closed") + ) { + try { + const live = await viewPullRequest(fullName, pr.number); + if (live.number !== pr.number) throw new Error("targeted refresh returned a different PR"); + prs.push(live); + if (live.state === "OPEN") confirmedReopened.add(pr.number); + } catch (err) { + // Preserve the previous state and retry next tick; an unavailable + // confirmation is not evidence of a reopen. + console.error( + `github-poll: reopen confirmation of ${fullName}#${pr.number} failed: ${(err as Error).message}`, + ); + } + } else { + prs.push(pr); + } + } // Targeted refresh (SYD-206): open pr_state rows outside the poll window // still get observed, on a slower cadence, so "absence is not evidence" // has a live producer behind it. Failures never transition anything — they // count toward the staleness alarm and get retried next interval. - const openRows = await fetchOpenPrNumbers(config.url, token, fullName); + const openRows = storedRows.filter((row) => row.status === "open").map((row) => row.prNumber); const candidates = selectRefreshCandidates( openRows, - new Set(prs.map((pr) => pr.number)), + new Set(windowPrs.map((pr) => pr.number)), repoState, Date.now(), REFRESH_INTERVAL_MS, @@ -210,7 +248,7 @@ export async function pollRepo( .map(async (pr) => [pr.number, await latestRun(fullName, pr.headRefName)] as const), ), ); - const { events, next } = observeRepoState(prs, runs, repoState); + const { events, next } = observeRepoState(prs, runs, repoState, confirmedReopened); for (const ev of events) { if (dryRun) { @@ -218,6 +256,9 @@ export async function pollRepo( continue; } const outcome = await postGithubEvent(config.url, token, ev, fullName); + if (outcome.reason && ev.event === "pull_request" && ev.payload.action === "closed") { + console.error(`github-poll: ${fullName}: close observation rejected: ${outcome.reason}`); + } // Steady-state re-observation (SYD-177/206) is absorbed server-side every // tick; only log events the server actually recorded. if (!outcome.duplicate) console.log(`${fullName}: posted ${ev.event}`); diff --git a/scripts/worker-publications.ts b/scripts/worker-publications.ts new file mode 100644 index 0000000..b2e967b --- /dev/null +++ b/scripts/worker-publications.ts @@ -0,0 +1,58 @@ +// Durable host-side publication obligations. No credentials are stored here. +// A worker crash or the server's orphan-session sweep must not lose commits +// already pushed by a container into its host repository. +import { createHash } from "node:crypto"; +import { mkdirSync, readFileSync, readdirSync, renameSync, rmSync, writeFileSync } from "node:fs"; +import path from "node:path"; +import { z } from "zod"; +import type { WorkerConfig } from "./worker-select.js"; + +const publicationSchema = z.object({ + ref: z.string().regex(/^[A-Z]{2,10}-\d+$/), + issueTitle: z.string(), + sessionId: z.number().int().positive().nullable(), + exitCode: z.number().int().nullable(), +}); +export type PendingPublication = z.infer; + +function directory(repo: string, config: WorkerConfig): string { + // Engine workers can share a checkout. Scope by the server and credential + // NAME, never its value, so one worker does not consume another's outbox. + const scope = createHash("sha256") + .update(JSON.stringify([config.url, config.label, config.token ?? "SWITCHYARD_TOKEN"])) + .digest("hex"); + return path.join(repo, ".superpowers", "worker-publications", scope); +} + +export function savePublication( + repo: string, + config: WorkerConfig, + publication: PendingPublication, +): void { + const checked = publicationSchema.parse(publication); + const dir = directory(repo, config); + mkdirSync(dir, { recursive: true }); + const dest = path.join(dir, `${checked.ref}.json`); + const temp = `${dest}.${process.pid}.tmp`; + writeFileSync(temp, JSON.stringify(checked) + "\n", { mode: 0o600, flush: true }); + renameSync(temp, dest); +} + +export function readPublications(repo: string, config: WorkerConfig): PendingPublication[] { + const dir = directory(repo, config); + let files: string[]; + try { + files = readdirSync(dir); + } catch (err) { + if ((err as NodeJS.ErrnoException).code === "ENOENT") return []; + throw err; + } + return files + .filter((file) => file.endsWith(".json")) + .map((file) => publicationSchema.parse(JSON.parse(readFileSync(path.join(dir, file), "utf8")))); +} + +export function clearPublication(repo: string, config: WorkerConfig, ref: string): void { + publicationSchema.shape.ref.parse(ref); + rmSync(path.join(directory(repo, config), `${ref}.json`), { force: true }); +} diff --git a/src/db/schema.ts b/src/db/schema.ts index 61e985f..c902372 100644 --- a/src/db/schema.ts +++ b/src/db/schema.ts @@ -92,6 +92,8 @@ export const issues = sqliteTable( index("issues_status_idx").on(t.status), index("issues_assignee_id_idx").on(t.assigneeId), index("issues_queue_rank_idx").on(t.queueRank), + index("issues_parent_id_idx").on(t.parentId), + index("issues_project_status_idx").on(t.projectId, t.status), ], ); @@ -105,7 +107,10 @@ export const dependencies = sqliteTable( .notNull() .references(() => issues.id), }, - (t) => [primaryKey({ columns: [t.blockerId, t.blockedId] })], + (t) => [ + primaryKey({ columns: [t.blockerId, t.blockedId] }), + index("dependencies_blocked_id_idx").on(t.blockedId), + ], ); /** @@ -199,7 +204,10 @@ export const events = sqliteTable( sessionId: integer("session_id").references(() => sessions.id), createdAt: integer("created_at").notNull().default(now()), }, - (t) => [index("events_issue_id_idx").on(t.issueId)], + (t) => [ + index("events_issue_id_idx").on(t.issueId), + index("events_type_issue_id_idx").on(t.type, t.issueId), + ], ); export const sessions = sqliteTable("sessions", { diff --git a/src/rest/api-routes.ts b/src/rest/api-routes.ts index 7e51adf..16d9496 100644 --- a/src/rest/api-routes.ts +++ b/src/rest/api-routes.ts @@ -50,6 +50,7 @@ import { recordProgressNote, } from "../services/agent-sessions.js"; import { getAttention, listAttentionByIssueId } from "../services/attention.js"; +import { BOARD_STATUSES, getBoard, issueCounts, type DoneFilter } from "../services/board.js"; import { getOpenPr, listOpenPrByIssueId, deliveryPinFor } from "../services/pr-status.js"; import { listQueue, setQueuePosition } from "../services/queue.js"; import { @@ -60,7 +61,7 @@ import { } from "../services/delivery-attempts.js"; import { listRecentEventsPage, listUnansweredQuestions } from "../services/events.js"; import { getDeliveryHealth } from "../services/delivery-health.js"; -import { searchIssues, type SearchFilters } from "../services/search.js"; +import { searchIssues, type SearchSignals, type SearchFilters } from "../services/search.js"; import { requestHumanInput } from "../services/needs-input.js"; import { snoozeIssue, @@ -240,27 +241,53 @@ export function buildApiRoutes(db: Db, attachmentsDir: string = defaultAttachmen app.get("/me", (c) => c.json(c.var.actor)); + app.get("/issue-counts", (c) => c.json(issueCounts(db, c.req.query("project") || undefined))); + app.get("/board", (c) => { + const done = c.req.query("done"); + return c.json( + getBoard(db, { + projectKey: c.req.query("project") ?? "", + doneFilters: + done === undefined ? undefined : done === "all" ? [] : (done.split(",") as DoneFilter[]), + before: Object.fromEntries( + BOARD_STATUSES.flatMap((status) => { + const cursor = c.req.query(`before_${status}`); + return cursor === undefined ? [] : [[status, Number(cursor)]]; + }), + ), + limit: c.req.query("limit") === undefined ? undefined : Number(c.req.query("limit")), + }), + ); + }); + app.get("/issues", (c) => { - const results = searchIssues(db, { - projectKey: c.req.query("project") || undefined, - status: (c.req.query("status") as Status | undefined) || undefined, - assigneeName: c.req.query("assignee") || undefined, - label: c.req.query("label") || undefined, - text: c.req.query("text") || undefined, - needsInput: c.req.query("needs_input") === "true" ? true : undefined, - excludeSnoozed: c.req.query("exclude_snoozed") === "true" ? true : undefined, - attention: (c.req.query("attention") as SearchFilters["attention"]) || undefined, - openPr: - c.req.query("open_pr") === "true" - ? true - : c.req.query("open_pr") === "false" - ? false - : undefined, - }); - const attention = listAttentionByIssueId(db); - const openPrs = listOpenPrByIssueId(db); - const blocked = listBlockedIssueIds(db); - const childCounts = childCountsByParent(db); + const signals: SearchSignals = {}; + const results = searchIssues( + db, + { + projectKey: c.req.query("project") || undefined, + status: (c.req.query("status") as Status | undefined) || undefined, + assigneeName: c.req.query("assignee") || undefined, + label: c.req.query("label") || undefined, + text: c.req.query("text") || undefined, + needsInput: c.req.query("needs_input") === "true" ? true : undefined, + excludeSnoozed: c.req.query("exclude_snoozed") === "true" ? true : undefined, + attention: (c.req.query("attention") as SearchFilters["attention"]) || undefined, + openPr: + c.req.query("open_pr") === "true" + ? true + : c.req.query("open_pr") === "false" + ? false + : undefined, + }, + signals, + ); + if (results.length === 0) return c.json([]); + const ids = results.map((issue) => issue.id); + const openPrs = signals.openPrs ?? listOpenPrByIssueId(db, ids); + const attention = signals.attention ?? listAttentionByIssueId(db, ids, openPrs); + const blocked = listBlockedIssueIds(db, ids); + const childCounts = childCountsByParent(db, ids); return c.json( results.map((r) => ({ ...r, diff --git a/src/services/attention.ts b/src/services/attention.ts index 3a554db..1fbf00e 100644 --- a/src/services/attention.ts +++ b/src/services/attention.ts @@ -35,19 +35,21 @@ import { sql } from "drizzle-orm"; import type { Db } from "../db/index.js"; import { getDeviation, listDeviationByIssueId, type DeviationFlag } from "./deviation.js"; +import { issueIdsCondition } from "./issue-scope.js"; +import type { OpenPr } from "./pr-status.js"; export type AttentionFlag = { reason: "delivery_failed"; message: string } | DeviationFlag; type Row = { issueId: number; message: string | null }; -function unresolvedDeliveryFailures(db: Db, issueId?: number): Row[] { +function unresolvedDeliveryFailures(db: Db, issueIds?: readonly number[]): Row[] { return db.all(sql` SELECT latest.issue_id AS issueId, json_extract(f.payload, '$.message') AS message FROM ( SELECT issue_id, MAX(id) AS eventId FROM events WHERE type = 'delivery_failed' - ${issueId !== undefined ? sql`AND issue_id = ${issueId}` : sql``} + AND ${issueIdsCondition(sql`issue_id`, issueIds)} GROUP BY issue_id ) latest JOIN events f ON f.id = latest.eventId @@ -70,7 +72,7 @@ function unresolvedDeliveryFailures(db: Db, issueId?: number): Row[] { `); } -function unresolvedDoneWithoutMerge(db: Db, issueId?: number): Row[] { +function unresolvedDoneWithoutMerge(db: Db, issueIds?: readonly number[]): Row[] { return db.all(sql` SELECT latest.issue_id AS issueId, json_extract(f.payload, '$.message') AS message FROM ( @@ -78,7 +80,7 @@ function unresolvedDoneWithoutMerge(db: Db, issueId?: number): Row[] { FROM events WHERE type = 'process_deviation' AND json_extract(payload, '$.reason') = 'done_without_merged_pr' - ${issueId !== undefined ? sql`AND issue_id = ${issueId}` : sql``} + AND ${issueIdsCondition(sql`issue_id`, issueIds)} GROUP BY issue_id ) latest JOIN events f ON f.id = latest.eventId @@ -135,11 +137,11 @@ function unresolvedDoneWithoutMerge(db: Db, issueId?: number): Row[] { } export function getAttention(db: Db, issueId: number): AttentionFlag | null { - const [row] = unresolvedDeliveryFailures(db, issueId); + const [row] = unresolvedDeliveryFailures(db, [issueId]); if (row) return { reason: "delivery_failed", message: row.message ?? "delivery failed" }; const deviation = getDeviation(db, issueId); if (deviation) return deviation; - const [doneRow] = unresolvedDoneWithoutMerge(db, issueId); + const [doneRow] = unresolvedDoneWithoutMerge(db, [issueId]); if (doneRow) { return { reason: "done_without_merged_pr", @@ -149,18 +151,23 @@ export function getAttention(db: Db, issueId: number): AttentionFlag | null { return null; } -export function listAttentionByIssueId(db: Db): Map { +export function listAttentionByIssueId( + db: Db, + issueIds?: readonly number[], + openPrs?: Map, +): Map { + if (issueIds?.length === 0) return new Map(); // Start from deviations, then let done_without_merged_pr and finally // unresolved delivery failures overwrite — delivery_failed (a hard error) // outranks everything else on collision. - const map = new Map(listDeviationByIssueId(db)); - for (const r of unresolvedDoneWithoutMerge(db)) { + const map = new Map(listDeviationByIssueId(db, issueIds, openPrs)); + for (const r of unresolvedDoneWithoutMerge(db, issueIds)) { map.set(r.issueId, { reason: "done_without_merged_pr", message: r.message ?? "done without a merged PR", }); } - for (const r of unresolvedDeliveryFailures(db)) { + for (const r of unresolvedDeliveryFailures(db, issueIds)) { map.set(r.issueId, { reason: "delivery_failed", message: r.message ?? "delivery failed" }); } return map; diff --git a/src/services/board.ts b/src/services/board.ts new file mode 100644 index 0000000..c965015 --- /dev/null +++ b/src/services/board.ts @@ -0,0 +1,131 @@ +import { and, count, desc, eq, lt } from "drizzle-orm"; +import type { Db } from "../db/index.js"; +import { issues, type Status } from "../db/schema.js"; +import { listAttentionByIssueId } from "./attention.js"; +import { SwitchyardError } from "./errors.js"; +import { childCountsByParent } from "./issues.js"; +import { issueIdsCondition } from "./issue-scope.js"; +import { listOpenPrByIssueId } from "./pr-status.js"; +import { getProjectByKey } from "./projects.js"; + +export const BOARD_STATUSES = ["backlog", "todo", "in_progress", "in_review", "done"] as const; +export type BoardStatus = (typeof BOARD_STATUSES)[number]; +export const DONE_FILTERS = ["errors", "not_merged", "unlinked"] as const; +export type DoneFilter = (typeof DONE_FILTERS)[number]; +export const BOARD_PAGE_SIZE = 50; + +/** Counts never need full issue bodies or derived attention/PR data. */ +export function issueCounts(db: Db, projectKey?: string): Partial> { + const projectId = projectKey ? getProjectByKey(db, projectKey).id : undefined; + return Object.fromEntries( + db + .select({ status: issues.status, count: count() }) + .from(issues) + .where(projectId === undefined ? undefined : eq(issues.projectId, projectId)) + .groupBy(issues.status) + .all() + .map((r) => [r.status, r.count]), + ); +} + +const cardColumns = { + id: issues.id, + number: issues.number, + title: issues.title, + status: issues.status, + priority: issues.priority, + labels: issues.labels, + needsInput: issues.needsInput, + workerPreference: issues.workerPreference, +}; + +/** Separate, bounded read model for board cards. Legacy /issues and MCP search + * retain their full issue shape and ordering. Cursors are per column, newest + * first, and filter membership is computed before applying the page limit. */ +export function getBoard( + db: Db, + input: { + projectKey: string; + doneFilters?: readonly DoneFilter[]; + before?: Partial>; + limit?: number; + }, +) { + const project = getProjectByKey(db, input.projectKey); + const filters = input.doneFilters ?? ["errors", "not_merged"]; + if (filters.some((filter) => !DONE_FILTERS.includes(filter))) { + throw new SwitchyardError("Unknown done filter — use errors, not_merged, unlinked, or all."); + } + const limit = input.limit ?? BOARD_PAGE_SIZE; + if (!Number.isSafeInteger(limit) || limit < 1 || limit > 100) { + throw new SwitchyardError("Board page size must be a whole number from 1 to 100."); + } + for (const cursor of Object.values(input.before ?? {})) { + if (!Number.isSafeInteger(cursor) || cursor <= 0) { + throw new SwitchyardError("Board cursors must be positive whole issue IDs."); + } + } + const totals = issueCounts(db, project.key); + let doneIds: number[] | undefined; + if (filters.length > 0 && totals.done) { + const ids = db + .select({ id: issues.id }) + .from(issues) + .where(and(eq(issues.projectId, project.id), eq(issues.status, "done"))) + .all() + .map((r) => r.id); + const openPrs = listOpenPrByIssueId(db, ids); + const attention = filters.some((f) => f !== "not_merged") + ? listAttentionByIssueId(db, ids, openPrs) + : new Map(); + doneIds = ids.filter( + (id) => + (filters.includes("errors") && attention.get(id)?.reason === "delivery_failed") || + (filters.includes("not_merged") && openPrs.has(id)) || + (filters.includes("unlinked") && attention.get(id)?.reason === "done_without_merged_pr"), + ); + } + const pages = BOARD_STATUSES.map((status) => { + const conditions = [eq(issues.projectId, project.id), eq(issues.status, status)]; + const before = input.before?.[status]; + if (before !== undefined) conditions.push(lt(issues.id, before)); + if (status === "done" && doneIds !== undefined) + conditions.push(issueIdsCondition(issues.id, doneIds)); + const rows = db + .select(cardColumns) + .from(issues) + .where(and(...conditions)) + .orderBy(desc(issues.id)) + .limit(limit + 1) + .all(); + const items = rows.slice(0, limit); + return { + status, + items, + total: totals[status] ?? 0, + matching: status === "done" && doneIds !== undefined ? doneIds.length : (totals[status] ?? 0), + nextBeforeId: rows.length > limit ? items[items.length - 1].id : null, + }; + }); + const ids = pages.flatMap((page) => page.items.map((issue) => issue.id)); + const openPrs = listOpenPrByIssueId(db, ids); + const attention = listAttentionByIssueId(db, ids, openPrs); + const children = childCountsByParent(db, ids); + return { + columns: Object.fromEntries( + pages.map(({ status, items, ...page }) => [ + status, + { + ...page, + items: items.map(({ number, ...issue }) => ({ + ...issue, + ref: `${project.key}-${number}`, + attention: attention.get(issue.id) ?? null, + openPr: openPrs.get(issue.id) ?? null, + childCount: children.get(issue.id) ?? 0, + })), + }, + ]), + ), + }; +} diff --git a/src/services/delivery-events.ts b/src/services/delivery-events.ts index 63ff036..aa860be 100644 --- a/src/services/delivery-events.ts +++ b/src/services/delivery-events.ts @@ -11,7 +11,8 @@ import { getIssue } from "./issues.js"; import { recordEvent } from "./events.js"; import { boundRepoFullNames, normalizeRepoFullName } from "./github-repos.js"; import { parseGhTimestamp } from "./github-webhook.js"; -import { upsertPrState, attributedRef } from "./pr-state.js"; +import { upsertPrState } from "./pr-state.js"; +import { recordIngestedPrLink } from "./pr-links.js"; export type DeployResult = { ran: false } | { ran: true; ok: boolean; tail: string }; @@ -62,7 +63,7 @@ export function recordDeliveryEvent( if (repo === null) { // SYD-205 deploy-skew rule: infer only when it's unambiguous. const bound = boundRepoFullNames(db, issue.projectId); - if (bound.length === 1) repo = bound[0]; + if (bound.length === 1) repo = normalizeRepoFullName(bound[0]); else if (bound.length > 1) { throw new SwitchyardError( "repo is ambiguous — the issue's project has multiple bound repos, so this delivery event must name its repo.", @@ -86,7 +87,22 @@ export function recordDeliveryEvent( // state, exactly as in webhook ingestion. if (repo !== null && (input.type === "pr_opened" || input.type === "delivered")) { const branch = `agent/${issue.ref}`; - if (attributedRef(db, repo, branch) === issue.ref) { + if ( + boundRepoFullNames(db, issue.projectId).some((bound) => normalizeRepoFullName(bound) === repo) + ) { + // This route is authenticated as human/service infrastructure, and its + // explicit issue ref is the worker host's declaration. GitHub webhooks + // and poller observations have no such authority. Publication alone may + // establish the link; a later delivery must not invent missing attribution. + if (input.type === "pr_opened") { + recordIngestedPrLink(db, { + issueId: issue.id, + repo, + prNumber: input.prNumber, + role: "delivers", + actorId: actor.id, + }); + } upsertPrState(db, actor, { repo, prNumber: input.prNumber, diff --git a/src/services/dependencies.ts b/src/services/dependencies.ts index 426df40..77647ca 100644 --- a/src/services/dependencies.ts +++ b/src/services/dependencies.ts @@ -12,6 +12,7 @@ import { listOpenPrByIssueId } from "./pr-status.js"; import { EXECUTABLE_GATE_ACTIONS, findOrCreatePendingAction, isHardGated } from "./hard-gate.js"; import { getSetting } from "./settings.js"; import { affinityRank, QUEUE_RANK_ORDER } from "./queue.js"; +import { issueIdsCondition } from "./issue-scope.js"; import { callerClassification, isAttendedCaller, @@ -29,6 +30,11 @@ export function addDependency( blockedRef: string, attr: Attribution = {}, ): void { + if (actor.type === "service") { + throw new SwitchyardError( + "Service actors post events, read, and comment — they cannot add dependencies.", + ); + } db.transaction((tx) => { const blocker = getIssue(tx, blockerRef); const blocked = getIssue(tx, blockedRef); @@ -215,12 +221,18 @@ function isReachable(db: DbOrTx, fromId: number, toId: number): boolean { * worker reads can flag blocked issues in one query instead of N (SYD-160). The * Set collapses issues with several open blockers to a single id. */ -export function listBlockedIssueIds(db: Db): Set { +export function listBlockedIssueIds(db: Db, issueIds?: readonly number[]): Set { + if (issueIds?.length === 0) return new Set(); const rows = db .select({ blockedId: dependencies.blockedId }) .from(dependencies) .innerJoin(issues, eq(dependencies.blockerId, issues.id)) - .where(notInArray(issues.status, [...CLOSED])) + .where( + and( + notInArray(issues.status, [...CLOSED]), + issueIdsCondition(dependencies.blockedId, issueIds), + ), + ) .all(); return new Set(rows.map((r) => r.blockedId)); } diff --git a/src/services/deviation.ts b/src/services/deviation.ts index 24f838f..205097f 100644 --- a/src/services/deviation.ts +++ b/src/services/deviation.ts @@ -19,6 +19,7 @@ import { } from "./pr-status.js"; import { getSetting } from "./settings.js"; import { recordEvent } from "./events.js"; +import { issueIdsCondition } from "./issue-scope.js"; export type DeviationReason = | "open_pr_not_in_review" @@ -36,7 +37,7 @@ export type DeviationComputation = DeviationFlag & { prNumber: number | null; }; -type IssueRow = typeof issues.$inferSelect; +type IssueRow = Pick; // `done` earns its place here for done_pr_not_delivered only (SYD-261) — the // three older rules all require todo/in_progress/in_review and stay inert for @@ -201,14 +202,25 @@ export function getDeviation(db: Db, issueId: number): DeviationFlag | null { return c ? { reason: c.reason, message: c.message } : null; } -export function listDeviationByIssueId(db: Db): Map { +export function listDeviationByIssueId( + db: Db, + issueIds?: readonly number[], + openPrs = listOpenPrByIssueId(db, issueIds), +): Map { + if (issueIds?.length === 0) return new Map(); const now = Math.floor(Date.now() / 1000); const threshold = getSetting(db, "claims.deviation_seconds"); - const openPrs = listOpenPrByIssueId(db); const rows = db - .select() + .select({ + id: issues.id, + status: issues.status, + needsInput: issues.needsInput, + createdAt: issues.createdAt, + }) .from(issues) - .where(inArray(issues.status, [...CANDIDATE_STATUSES])) + .where( + and(inArray(issues.status, [...CANDIDATE_STATUSES]), issueIdsCondition(issues.id, issueIds)), + ) .all(); const out = new Map(); for (const issue of rows) { @@ -244,7 +256,14 @@ export function emitProcessDeviations(db: Db): number { const now = Math.floor(Date.now() / 1000); const threshold = getSetting(db, "claims.deviation_seconds"); const rows = db - .select() + .select({ + id: issues.id, + status: issues.status, + needsInput: issues.needsInput, + createdAt: issues.createdAt, + assigneeId: issues.assigneeId, + creatorId: issues.creatorId, + }) .from(issues) .where(inArray(issues.status, [...CANDIDATE_STATUSES])) .all(); diff --git a/src/services/github-webhook.ts b/src/services/github-webhook.ts index be040d2..c1d1f84 100644 --- a/src/services/github-webhook.ts +++ b/src/services/github-webhook.ts @@ -10,9 +10,8 @@ // first, then a bare ref in free text (PR title/body, or commit messages for // push). This is display, and a string may decide it. // - IS THE PR ATTRIBUTED to an issue's work? DECLARED, never parsed: a live -// `delivers` row in pr_links (SYD-280). An `agent/` branch auto-declares -// its own link inside upsertPrState, which is why dispatched work needs no -// extra step; everyone else calls declare_pr_link. +// `delivers` row in pr_links (SYD-280). The authenticated worker host +// declares at publish; a branch name from GitHub only suggests a reference. // // pull_request ingestion answers NEITHER question before writing `pr_state`: // since SYD-287 every PR in a bound repo is observed, because pr_state records @@ -35,7 +34,7 @@ import { getOrCreateActor } from "./actors.js"; import { getIssue, issueRefById } from "./issues.js"; import { recordEvent } from "./events.js"; import { boundRepoFullNames, findGithubRepo, normalizeRepoFullName } from "./github-repos.js"; -import { upsertPrState, attributedRef, type PrObservation } from "./pr-state.js"; +import { upsertPrState, type PrObservation } from "./pr-state.js"; import { recordIngestedPrLink, deliversLinkIssueIds } from "./pr-links.js"; const GITHUB_ACTOR_NAME = "github"; @@ -146,7 +145,14 @@ export type GithubWebhookOutcome = // `ref` is null when nothing attributes the PR to an issue — since SYD-287 a // PR in a bound repo is observed regardless, so "handled" no longer implies // "belongs to someone". Naming an issue anyway would be a guess. - | { handled: true; ref: string | null; type: string; duplicate?: true; recorded?: false } + | { + handled: true; + ref: string | null; + type: string; + duplicate?: true; + recorded?: false; + reason?: string; + } | { handled: false; reason: string }; /** @@ -307,8 +313,6 @@ function handlePullRequest(db: Db, rawPayload: unknown, repo: string | null): Gi // sole-bound-repo inference above. const linkedIssueIds = resolvedRepo === null ? [] : deliversLinkIssueIds(db, resolvedRepo, prNumber); - const branchAttributed = - resolvedRepo !== null && attributedRef(db, resolvedRepo, branch) !== null; // OBSERVATION (SYD-206, widened by SYD-287). Every PR in a bound repo, full // stop — no branch test, no link test. @@ -366,7 +370,13 @@ function handlePullRequest(db: Db, rawPayload: unknown, repo: string | null): Gi observedOutcome = applied.transition !== null ? { handled: true, ref, type } - : { handled: true, ref, type, duplicate: true }; + : { + handled: true, + ref, + type, + duplicate: true, + ...(applied.reason ? { reason: applied.reason } : {}), + }; } // DISPLAY. Unchanged, but no longer the observation's else-branch: a PR @@ -376,7 +386,7 @@ function handlePullRequest(db: Db, rawPayload: unknown, repo: string | null): Gi // text ref's issue already took upsertPrState's canonical co-write, which is // what keeps one transition from appearing twice. let displayOutcome: GithubWebhookOutcome | null = null; - if (issue !== undefined && !branchAttributed && !linkedIssueIds.includes(issue.id)) { + if (issue !== undefined && !linkedIssueIds.includes(issue.id)) { const textOnlyRef = issue.ref; const byPrNumber = { jsonPath: "$.prNumber", value: prNumber }; if (action === "opened") { diff --git a/src/services/issue-scope.ts b/src/services/issue-scope.ts new file mode 100644 index 0000000..bbb93a2 --- /dev/null +++ b/src/services/issue-scope.ts @@ -0,0 +1,8 @@ +import { sql, type SQLWrapper } from "drizzle-orm"; + +/** Bound as one JSON value so large legacy list calls cannot exceed SQLite's + * parameter limit. Undefined means all issues; an empty selection means none. */ +export function issueIdsCondition(column: SQLWrapper, ids?: readonly number[]) { + if (ids === undefined) return sql`1`; + return sql`${column} IN (SELECT value FROM json_each(${JSON.stringify(ids)}))`; +} diff --git a/src/services/issues.ts b/src/services/issues.ts index 91e3b55..2866cea 100644 --- a/src/services/issues.ts +++ b/src/services/issues.ts @@ -29,6 +29,7 @@ import { heartbeatLease, } from "./leases.js"; import { isHardGated, findOrCreatePendingAction, EXECUTABLE_GATE_ACTIONS } from "./hard-gate.js"; +import { issueIdsCondition } from "./issue-scope.js"; export type Provenance = { sourceType: "session" | "todo" | "ci" | "manual"; @@ -151,11 +152,15 @@ export function listChildren(db: DbOrTx, ref: string): IssueView[] { } /** Map of parent issue id → number of children, for at-a-glance epic badges. */ -export function childCountsByParent(db: DbOrTx): Map { +export function childCountsByParent( + db: DbOrTx, + parentIds?: readonly number[], +): Map { + if (parentIds?.length === 0) return new Map(); const rows = db .select({ parentId: issues.parentId, count: sql`count(*)` }) .from(issues) - .where(sql`${issues.parentId} is not null`) + .where(and(sql`${issues.parentId} is not null`, issueIdsCondition(issues.parentId, parentIds))) .groupBy(issues.parentId) .all(); const map = new Map(); @@ -521,9 +526,6 @@ export function updateIssue( ) { changes.assigneeId = null; toRecord.push({ type: "claim_released", payload: { reason: "moved_to_todo" } }); - // SYD-210: self-release ends the claim, so its lease is invalidated - // (the holder already validated above). - invalidateLease(tx, current.id); } } @@ -666,6 +668,16 @@ export function updateIssue( toRecord.push({ type: "needs_input_cleared", payload: {} }); } + // Assignment and lease ownership change together, including explicit + // assignee:null patches and human reassignment. Returning to todo also + // ends the claim even when a human deliberately preserves the assignee. + if ( + (changes.assigneeId !== undefined && changes.assigneeId !== current.assigneeId) || + changes.status === "todo" + ) { + invalidateLease(tx, current.id); + } + // Mint a lease when this update establishes a fresh claim: an unassigned // issue becoming assigned to the actor and in_progress (the claimIssue // self-assign path AND the SYD-111 bare-PATCH auto-claim path both land @@ -729,34 +741,43 @@ export function claimIssue( opts: { takeover?: boolean } = {}, attr: Attribution = {}, ): ClaimResult { - const current = getIssue(db, ref); - const blockers = getOpenBlockers(db, current.id); - if (blockers.length > 0) { - throw new SwitchyardError( - `${ref} is blocked by ${blockers.map((b) => b.ref).join(", ")} — resolve the blocker first, or call next_task for another issue.`, - ); - } - - // Same-actor re-claim of an issue that already has an active lease: fail - // loudly unless takeover is opted in (this project's workflow tells every - // session to claim before touching code, and interactive + dispatched - // sessions share the worker actor — a default takeover would silently kill a - // healthy running container). Takeover only reaches here for the same actor; - // a different actor's claim is refused by assertClaimable below. - if (current.assigneeId === actor.id) { - const active = getActiveLease(db, current.id); - if (active && !opts.takeover) { + return db.transaction((tx) => { + const current = getIssue(tx, ref); + const blockers = getOpenBlockers(tx, current.id); + if (blockers.length > 0) { throw new SwitchyardError( - `${ref} already has an active lease held by this actor — another session may be working it. ` + - `Pass takeover: true to seize the claim (invalidating that session's lease), or call next_task for another issue.`, + `${ref} is blocked by ${blockers.map((b) => b.ref).join(", ")} — resolve the blocker first, or call next_task for another issue.`, ); } - // Re-claim (takeover, or a lease-less holder e.g. after expiry): swap the - // lease in one transaction. The issue is already in_progress + assigned, so - // no status change is needed. - const leaseToken = db.transaction((tx) => { + assertClaimable(tx, actor, current); + + // A preassignment is not necessarily an in_progress claim. Rotate the + // lease and drive the ordinary transition in one transaction so todo and + // in_review work starts correctly, and forbidden status changes roll back + // the provisional lease along with every other change. + if (current.assigneeId === actor.id) { + const active = getActiveLease(tx, current.id); + if (active && !opts.takeover) { + throw new SwitchyardError( + `${ref} already has an active lease held by this actor — another session may be working it. ` + + `Pass takeover: true to seize the claim (invalidating that session's lease), or call next_task for another issue.`, + ); + } + const leaseToken = mintLease( + tx, + current.id, + actor.id, + getSetting(tx, "claims.lease_ttl_seconds"), + ); + const issue = updateIssue( + tx, + actor, + ref, + { status: "in_progress" }, + { presented: leaseToken }, + attr, + ); if (active) { - invalidateLease(tx, current.id); recordEvent(tx, { issueId: current.id, actorId: actor.id, @@ -766,29 +787,22 @@ export function claimIssue( sessionId: attr.sessionId, }); } - return mintLease(tx, current.id, actor.id, getSetting(db, "claims.lease_ttl_seconds")); - }); - return { issue: getIssue(db, ref), leaseToken }; - } + return { issue, leaseToken }; + } - // Fresh claim of an unassigned (or blocked/PR-guarded) issue: assertClaimable - // is re-checked inside updateIssue's in_progress gate; the mint happens there - // via the out-channel. - assertClaimable(db, actor, current); - const minted: { token: string | null } = { token: null }; - const issue = updateIssue( - db, - actor, - ref, - { status: "in_progress", assigneeName: actor.name }, - { minted }, - attr, - ); - if (minted.token === null) { - // Defensive: the auto-claim mint condition should always fire on a fresh - // claim. If it didn't, the claim state is inconsistent — fail rather than - // hand back an empty token. - throw new SwitchyardError(`Failed to mint a lease while claiming ${ref} — retry the claim.`); - } - return { issue, leaseToken: minted.token }; + const minted: { token: string | null } = { token: null }; + const issue = updateIssue( + tx, + actor, + ref, + { status: "in_progress", assigneeName: actor.name }, + { minted }, + attr, + ); + if (minted.token === null) { + // Keep the defensive failure inside the claim transaction as well. + throw new SwitchyardError(`Failed to mint a lease while claiming ${ref} — retry the claim.`); + } + return { issue, leaseToken: minted.token }; + }); } diff --git a/src/services/leases.ts b/src/services/leases.ts index c73b36e..b414212 100644 --- a/src/services/leases.ts +++ b/src/services/leases.ts @@ -14,8 +14,10 @@ export const LEASE_TTL_SETTING = "claims.lease_ttl_seconds" as const; const nowSeconds = () => Math.floor(Date.now() / 1000); /** - * The single active lease of an issue: not invalidated and not past - * expires_at. At most one by construction (a claim is 1:1 with an issue). + * The current assignee's latest lease, while still valid. Older generations + * never become current again, even if a legacy duplicate has a longer TTL + * than its replacement. This also tolerates rows written before reassignment + * invalidated leases: an outgoing assignee's token cannot remain active. */ export function getActiveLease( db: DbOrTx, @@ -24,16 +26,24 @@ export function getActiveLease( ): ClaimLease | null { return ( db - .select() + .select({ lease: claimLeases }) .from(claimLeases) + .innerJoin( + issues, + and(eq(issues.id, claimLeases.issueId), eq(issues.assigneeId, claimLeases.actorId)), + ) .where( and( eq(claimLeases.issueId, issueId), isNull(claimLeases.invalidatedAt), gt(claimLeases.expiresAt, now), + sql`NOT EXISTS ( + SELECT 1 FROM claim_leases newer + WHERE newer.issue_id = ${claimLeases.issueId} AND newer.id > ${claimLeases.id} + )`, ), ) - .get() ?? null + .get()?.lease ?? null ); } @@ -43,23 +53,28 @@ export function getActiveLease( * Runs inside the caller's transaction so a lease and its claim are atomic. */ export function mintLease( - tx: DbOrTx, + db: DbOrTx, issueId: number, actorId: number, ttlSeconds: number, ): string { const token = mintToken("lease"); const now = nowSeconds(); - tx.insert(claimLeases) - .values({ - issueId, - actorId, - tokenHash: hashToken(token), - expiresAt: now + ttlSeconds, - lastBeatAt: now, - }) - .run(); - return token; + return db.transaction((tx) => { + // Retire expired predecessors too: otherwise the next expiry sweep could + // mistake their old claim for this new generation. + invalidateLease(tx, issueId); + tx.insert(claimLeases) + .values({ + issueId, + actorId, + tokenHash: hashToken(token), + expiresAt: now + ttlSeconds, + lastBeatAt: now, + }) + .run(); + return token; + }); } /** @@ -115,17 +130,15 @@ export function heartbeatLease( } /** - * Marks the active lease of an issue invalidated (takeover / self-release / - * human-answer release). No-op if there is no active lease. The REASON is + * Retires all outstanding lease generations, including expired or historical + * duplicate rows (takeover / reassignment / release). The REASON is * carried by the event the caller co-records (claim_released{reason} / * lease_taken_over), not stored on the lease row. */ export function invalidateLease(tx: DbOrTx, issueId: number): void { - const active = getActiveLease(tx, issueId); - if (!active) return; tx.update(claimLeases) .set({ invalidatedAt: sql`(unixepoch())` }) - .where(eq(claimLeases.id, active.id)) + .where(and(eq(claimLeases.issueId, issueId), isNull(claimLeases.invalidatedAt))) .run(); } @@ -170,30 +183,43 @@ export function expireLeases(db: Db, now: number = nowSeconds(), serverStartedAt .all(); let released = 0; for (const lease of expired) { - const issue = db.select().from(issues).where(eq(issues.id, lease.issueId)).get(); - const actorId = issue?.assigneeId ?? issue?.creatorId ?? lease.actorId; const wasReleased = db.transaction((tx) => { // Always invalidate the expired lease, even if the issue moved on, so the // next sweep does not re-scan it. - tx.update(claimLeases) + const retired = tx + .update(claimLeases) .set({ invalidatedAt: sql`(unixepoch())` }) - .where(eq(claimLeases.id, lease.id)) + .where( + and( + eq(claimLeases.id, lease.id), + isNull(claimLeases.invalidatedAt), + lte(claimLeases.expiresAt, now), + ), + ) .run(); + if (retired.changes === 0) return false; const result = tx .update(issues) .set({ status: "todo", assigneeId: null, updatedAt: sql`(unixepoch())` }) .where( and( eq(issues.id, lease.issueId), + eq(issues.assigneeId, lease.actorId), eq(issues.status, "in_progress"), eq(issues.needsInput, false), + // A historical predecessor must never release a newer claim, + // whether the replacement is live, expired, or already retired. + sql`NOT EXISTS ( + SELECT 1 FROM claim_leases newer + WHERE newer.issue_id = ${lease.issueId} AND newer.id > ${lease.id} + )`, ), ) .run(); if (result.changes === 0) return false; recordEvent(tx, { issueId: lease.issueId, - actorId, + actorId: lease.actorId, type: "claim_released", payload: { reason: "lease_expired" }, }); diff --git a/src/services/pr-links.ts b/src/services/pr-links.ts index 72f16d6..3bfcb7a 100644 --- a/src/services/pr-links.ts +++ b/src/services/pr-links.ts @@ -2,8 +2,8 @@ // 2026-07-27-declared-pr-attribution-design.md). // // This module is the ONLY writer of pr_links. It replaces three string -// inference sites — the strict agent/ branch match (pr-state.ts's -// attributedRef), the first free-text ref in a PR title/body +// inference sites — the former strict agent/ branch match, +// the first free-text ref in a PR title/body // (github-webhook.ts's resolveRef), and a branch name reconstructed from the // issue ref (delivery-events.ts) — with a statement made by someone the system // can hold accountable. @@ -198,8 +198,7 @@ function findLiveLink( } /** - * The repo must be bound to the issue's project — the same rule attributedRef - * enforces today (src/services/pr-state.ts). Without it any repo could claim + * The repo must be bound to the issue's project. Without it any repo could claim * any issue, which is the cross-project attribution hole SYD-206 closed. */ function assertRepoBound(db: DbOrTx, projectId: number, repo: string): void { @@ -315,42 +314,16 @@ export function declarePrLink( } /** - * Ingestion's declaration path — webhook/poller and the worker's publish. + * Records an inert reference suggestion from GitHub, or the explicit issue + * declaration made by authenticated worker publication. Only the delivery + * service may pass `delivers`; external observations always pass `references`. + * A signed webhook authenticates GitHub's observation, never a fork author's + * relationship to an issue. Runs inside the caller's transaction. * - * Separate from declarePrLink because ingestion is not an actor staking a - * claim: it is the system recording an attribution it observed, attributed to - * the synthetic `github` actor (which is type `agent`, - * src/services/github-webhook.ts:176, and so could never satisfy the - * claim+lease rules). Runs inside the caller's transaction. - * - * **Scope discipline — this is parity, not a widening.** Only the - * branch-attributed path (a strict agent/ match in a repo bound to that - * ref's project) may pass `role: "delivers"`, because that is precisely the - * signal that gates claims and proves landing *today*. Free-text ref matches - * must pass `role: "references"`, which gates nothing and proves nothing — a - * narrowing of today's behaviour, and the fix for the false-clear hole where - * an unrelated PR that merely mentions an issue silences its warning. - * - * Note the untrusted-ingress caveat: POST /api/github-events accepts any - * human/service token and is indistinguishable from an HMAC-verified delivery - * (design "Scope", analysis §3.7). That is true of today's attribution too, so - * this is not a regression — it is SYD-282's to fix, and this function must - * not be read as making ingested merges trustworthy. - * - * Idempotent: a redelivery finds the live link and returns it unchanged. The - * one exception is the upgrade below — a branch-attributed observation - * supersedes a `references` suggestion an earlier free-text match minted for - * the same (issue, repo, PR), because otherwise ingestion order would decide - * whether a PR gates claims. - * - * **Records no event, deliberately.** The row itself carries the full audit - * (declared_by, declared_at, role), and the observation that prompted it is - * already on the timeline as gh_pr_opened/gh_pr_merged. Emitting an event too - * would put a "pr_link_declared" line above every single PR in every activity - * feed — a signal that fires on ordinary success, which is the noise class the - * intent document's principle 5 warns about. Actor-initiated declarations - * (declarePrLink/confirmPrLink/revokePrLink) DO record events, because those - * are decisions someone made rather than bookkeeping the system did. + * Replays preserve a live link and never undo a revocation. A host publication + * may promote an existing references suggestion to delivers; an explicit + * declarePrLink call is required to override a revoked relationship. + * The host's publication event supplies the audit; suggestions add no event. */ export function recordIngestedPrLink( tx: DbOrTx, @@ -361,13 +334,13 @@ export function recordIngestedPrLink( role: PrLinkRole; actorId: number; }, -): PrLink { +): PrLink | null { const repo = normalizeRepoFullName(input.repo); const now = nowSeconds(); const existing = findLiveLink(tx, input.issueId, repo, input.prNumber); if (existing) { - // Upgrade only, never downgrade (SYD-287). The branch-attributed path is - // strictly more authoritative than the free-text one, so it supersedes a + // Upgrade only, never downgrade. Authenticated host publication + // is an explicit declaration, so it supersedes a // `references` suggestion the same way an actor's declaration does — // otherwise a PR that happened to be ingested by text first would keep a // suggestion where a claim-gating link belongs, and lose its co-written @@ -376,10 +349,28 @@ export function recordIngestedPrLink( tx.update(prLinks).set({ revokedAt: now }).where(eq(prLinks.id, existing.id)).run(); } - // A `delivers` link from the branch-attributed path is confirmed, matching - // the authority pr_state.issue_ref carries today — its confirmer is not a - // human, so §5a's recency binding still applies to it. A `references` link - // is never confirmed: it is a suggestion for a human to promote. + // An observation cannot undo an explicit revocation. Also suppress an + // inert references suggestion after a revoke, so the panel does not offer + // the same rejected relationship on every poll. A deliberate declarePrLink + // call may create a new live row and supersede this tombstone. + if (!existing) { + const revoked = tx + .select({ id: prLinks.id }) + .from(prLinks) + .where( + and( + eq(prLinks.issueId, input.issueId), + sql`lower(${prLinks.repo}) = lower(${repo})`, + eq(prLinks.prNumber, input.prNumber), + sql`${prLinks.revokedAt} IS NOT NULL`, + ), + ) + .get(); + if (revoked) return null; + } + + // Trusted host publication retains its existing confirmation semantics. + // External references suggestions are never confirmed. const confirmed = input.role === "delivers"; const row = tx .insert(prLinks) diff --git a/src/services/pr-state.ts b/src/services/pr-state.ts index f13359e..5f47483 100644 --- a/src/services/pr-state.ts +++ b/src/services/pr-state.ts @@ -10,10 +10,13 @@ // consistent windowed search, so: // // - Terminal states never regress. merged is final; closed may still become -// merged (a merge is more final than a close); open-after-terminal happens +// merged (a merge is more final than a close); open-after-close happens // ONLY via an explicit `reopened` observation whose GitHub timestamp is // strictly newer than the stored terminal row's (fail-closed on a missing // or unparseable timestamp on either side). +// - Closing is reversible: a close cannot overwrite a newer open observation +// (in particular an explicit reopen). Merge evidence remains final even +// when delayed, and a merged row can never reopen. // - Same-status refreshes are monotonic on ghUpdatedAt: an observation with // no timestamp, or one older than the stored row, is a no-op. Equal // timestamps apply (same-second last-write-wins — GitHub timestamps have @@ -23,13 +26,9 @@ // an observation; a PR missing from a poll window simply produces no call. // - This table holds no attribution. It is a pure observation of a PR, keyed // (repo, prNumber); which issue a PR belongs to lives in pr_links (SYD-280). -// `issueRef` survives only as the SYD-280 §10 step-3 dual-write, written -// from the branch and read by nothing — never from a link, which would -// re-couple the two facts this split exists to separate. Step 4 drops it. -// - A strict agent/ match on a repo bound to that ref's project is the -// AUTO-DECLARATION trigger, not a gate: it records the `delivers` link the -// worker would otherwise have to declare by hand. An agent/SYD-1 PR in some -// other project's repo declares nothing. +// `issueRef` retains legacy cutover data only. New observations never infer +// attribution from a branch, including agent/ in a linked repository. +// Worker publication declares through the authenticated delivery service. // - On a real transition it co-writes ONE canonical audit event per issue // holding a live `delivers` link (gh_pr_opened/gh_pr_merged/gh_pr_closed/ // gh_pr_reopened), deduped against history via findEventIdByPayload so a @@ -40,15 +39,13 @@ import { and, eq, sql } from "drizzle-orm"; import type { Db, DbOrTx } from "../db/index.js"; -import { prState, githubRepos, type EventKind } from "../db/schema.js"; +import { prState, type EventKind } from "../db/schema.js"; import type { Actor } from "./actors.js"; import { SwitchyardError } from "./errors.js"; -import { getIssue } from "./issues.js"; -import { getProjectByKey } from "./projects.js"; import { normalizeRepoFullName } from "./github-repos.js"; import { recordEvent, findEventIdByPayload } from "./events.js"; -import { parseGhTimestamp, refFromBranch } from "./github-webhook.js"; -import { recordIngestedPrLink, deliversLinkIssueIds } from "./pr-links.js"; +import { parseGhTimestamp } from "./github-webhook.js"; +import { deliversLinkIssueIds } from "./pr-links.js"; export type PrStateRow = typeof prState.$inferSelect; export type PrStatus = "open" | "merged" | "closed"; @@ -107,40 +104,6 @@ export function listPrState(db: Db, filter: { repo?: string; status?: string } = return (conditions.length > 0 ? query.where(and(...conditions)) : query).all(); } -/** issueRef for a PR, or null: strict agent/ branch match, repo bound to - * that ref's project, and the issue actually exists. */ -export function attributedRef( - db: DbOrTx, - repo: string, - branch: string | null | undefined, -): string | null { - const ref = refFromBranch(branch); - if (!ref) return null; - let projectId: number; - try { - projectId = getProjectByKey(db, ref.split("-")[0]).id; - } catch { - return null; - } - const bound = db - .select({ id: githubRepos.id }) - .from(githubRepos) - .where( - and( - sql`lower(${githubRepos.fullName}) = lower(${repo})`, - eq(githubRepos.projectId, projectId), - ), - ) - .get(); - if (!bound) return null; - try { - getIssue(db, ref); - } catch { - return null; - } - return ref; -} - const EVENT_KIND: Record = { opened: "gh_pr_opened", merged: "gh_pr_merged", @@ -165,11 +128,19 @@ function decide(existing: PrStateRow | undefined, o: PrObservation, ts: number | return { apply: true, transition: null }; } if (existing.status === "open") { - // Terminal transitions are governed by the terminal rules alone, not the - // monotonic guard — a close/merge must land even if its timestamp lost a - // race with a same-status refresh. + // A delayed close from a previous episode must not undo a newer reopen. + // Unlike merge evidence, an unmerged close is reversible and needs to + // be at least as fresh as the open observation it would replace. + if ( + o.status === "closed" && + existing.ghUpdatedAt !== null && + (ts === null || ts < existing.ghUpdatedAt) + ) { + return { apply: false, reason: "close is missing or older than the stored open observation" }; + } return { apply: true, transition: o.status === "merged" ? "merged" : "closed" }; } + if (existing.status === "merged") return { apply: false, reason: "merged is final" }; if (existing.status === "closed" && o.status === "merged") { return { apply: true, transition: "merged" }; } @@ -200,31 +171,19 @@ export function upsertPrState(db: Db, actor: Actor, input: PrObservation): Upser const decision = decide(existing, o, ts); if (!decision.apply) return { applied: false, transition: null, reason: decision.reason }; - const issueRef = attributedRef(tx, o.repo, o.branch) ?? existing?.issueRef ?? null; - - // SYD-280: the branch-attributed path co-declares the issue<->PR link. - // Done here rather than at each caller because upsertPrState is the ONLY - // pr_state writer, so this one site covers webhook, poller, the worker's - // publish and the delivery merge. Idempotent, so refreshes and - // redeliveries are no-ops. - if (issueRef !== null) { - recordIngestedPrLink(tx, { - issueId: getIssue(tx, issueRef).id, - repo: o.repo, - prNumber: o.prNumber, - role: "delivers", - actorId: actor.id, - }); - } + // Observations never establish attribution: even a signed GitHub event + // can describe a fork whose author chose agent/. The authenticated + // worker host declares at publish; every other author declares explicitly. + // Retain old cutover data only. New observations must not seed a later + // backfill with a branch-derived assertion. + const issueRef = existing?.issueRef ?? null; let lastTransitionEventId = existing?.lastTransitionEventId ?? null; if (decision.transition !== null) { // SYD-287: the issues this transition is canonical for are the ones // holding a live `delivers` link — read from pr_links, not from the - // branch-derived issueRef above. For agent/ work that is the same - // single issue it always was (the auto-declaration a few lines up wrote - // its link), so the SYD-280 regression fence holds; for an interactive - // feat/ PR it is the only reason the merge reaches a timeline at all. + // legacy issueRef column. Authenticated publication and explicit + // declarations share this path for worker and interactive PRs. // A PR declared by more than one issue (design §3, SYD-274) gets the // event on each; the row's single lastTransitionEventId takes the // earliest declarer's, which deliversLinkIssueIds orders first. diff --git a/src/services/pr-status.ts b/src/services/pr-status.ts index b724569..40ea881 100644 --- a/src/services/pr-status.ts +++ b/src/services/pr-status.ts @@ -28,6 +28,7 @@ // that let SYD-93 get fixed twice in parallel. import { sql } from "drizzle-orm"; import type { Db, DbOrTx } from "../db/index.js"; +import { issueIdsCondition } from "./issue-scope.js"; // SYD-280: attribution is a declared pr_links row, not a parsed branch name. // These two fragments are the whole trust model at the read sites, kept as @@ -65,7 +66,8 @@ type Row = { issueId: number; prNumber: number; url: string; repo: string; headS // project key + number. Ordered by prNumber so listOpenPrByIssueId's Map // construction (later entries overwrite earlier ones for the same issueId) // keeps the newest PR when an issue somehow has more than one open at once. -function openRows(db: Db, issueId?: number): Row[] { +function openRows(db: Db, issueIds?: readonly number[]): Row[] { + if (issueIds?.length === 0) return []; return db.all(sql` SELECT pl.issue_id AS issueId, ps.pr_number AS prNumber, @@ -77,22 +79,22 @@ function openRows(db: Db, issueId?: number): Row[] { ON lower(ps.repo) = lower(pl.repo) AND ps.pr_number = pl.pr_number WHERE ${LIVE_DELIVERS} AND ps.status = 'open' - ${issueId !== undefined ? sql`AND pl.issue_id = ${issueId}` : sql``} + AND ${issueIdsCondition(sql`pl.issue_id`, issueIds)} ORDER BY ps.pr_number ASC `); } export function getOpenPr(db: Db, issueId: number): OpenPr | null { - const rows = openRows(db, issueId); + const rows = openRows(db, [issueId]); const row = rows[rows.length - 1]; return row ? { prNumber: row.prNumber, url: row.url, repo: row.repo, headSha: row.headSha } : null; } -export function listOpenPrByIssueId(db: Db): Map { +export function listOpenPrByIssueId(db: Db, issueIds?: readonly number[]): Map { return new Map( - openRows(db).map((r) => [ + openRows(db, issueIds).map((r) => [ r.issueId, { prNumber: r.prNumber, url: r.url, repo: r.repo, headSha: r.headSha }, ]), diff --git a/src/services/search.ts b/src/services/search.ts index 38bb87c..97f5deb 100644 --- a/src/services/search.ts +++ b/src/services/search.ts @@ -1,11 +1,12 @@ -import { and, desc, eq, inArray, isNull, or, sql, type SQL } from "drizzle-orm"; +import { and, desc, eq, isNull, or, sql, type SQL } from "drizzle-orm"; import type { Db } from "../db/index.js"; import { actors, issues, projects, type Status } from "../db/schema.js"; import { type IssueView } from "./issues.js"; import { getProjectByKey } from "./projects.js"; import { SwitchyardError } from "./errors.js"; +import { issueIdsCondition } from "./issue-scope.js"; import { listAttentionByIssueId, type AttentionFlag } from "./attention.js"; -import { listOpenPrByIssueId } from "./pr-status.js"; +import { listOpenPrByIssueId, type OpenPr } from "./pr-status.js"; export type SearchFilters = { projectKey?: string; @@ -25,7 +26,16 @@ export type SearchFilters = { openPr?: boolean; }; -export function searchIssues(db: Db, filters: SearchFilters): IssueView[] { +export type SearchSignals = { + attention?: Map; + openPrs?: Map; +}; + +export function searchIssues( + db: Db, + filters: SearchFilters, + signals: SearchSignals = {}, +): IssueView[] { const conditions: SQL[] = []; if (filters.projectKey) conditions.push(eq(issues.projectId, getProjectByKey(db, filters.projectKey).id)); @@ -47,27 +57,6 @@ export function searchIssues(db: Db, filters: SearchFilters): IssueView[] { const now = Math.floor(Date.now() / 1000); conditions.push(or(isNull(issues.snoozedUntil), sql`${issues.snoozedUntil} <= ${now}`)!); } - if (filters.attention) { - const flagged = [...listAttentionByIssueId(db).entries()] - .filter(([, flag]) => flag.reason === filters.attention) - .map(([issueId]) => issueId); - if (flagged.length === 0) return []; - conditions.push(inArray(issues.id, flagged)); - } - if (filters.openPr !== undefined) { - const withOpenPr = [...listOpenPrByIssueId(db).keys()]; - if (filters.openPr) { - if (withOpenPr.length === 0) return []; - conditions.push(inArray(issues.id, withOpenPr)); - } else if (withOpenPr.length > 0) { - conditions.push( - sql`${issues.id} NOT IN (${sql.join( - withOpenPr.map((id) => sql`${id}`), - sql`, `, - )})`, - ); - } - } if (filters.text) { // Escape SQL wildcard characters (%, _, and ~) so they're treated as literals const escaped = filters.text.toLowerCase().replace(/[~%_]/g, "~$&"); @@ -79,6 +68,29 @@ export function searchIssues(db: Db, filters: SearchFilters): IssueView[] { )!, ); } + // Derive expensive signals only for candidates matching ordinary filters. + // The caller can reuse these maps when enriching the resulting list. + if (filters.attention || filters.openPr !== undefined) { + const candidates = db + .select({ id: issues.id }) + .from(issues) + .where(conditions.length ? and(...conditions) : undefined) + .all() + .map((r) => r.id); + if (candidates.length === 0) return []; + const openPrs = listOpenPrByIssueId(db, candidates); + signals.openPrs = openPrs; + if (filters.attention) { + signals.attention = listAttentionByIssueId(db, candidates, signals.openPrs); + } + const matching = candidates.filter( + (id) => + (!filters.attention || signals.attention?.get(id)?.reason === filters.attention) && + (filters.openPr === undefined || openPrs.has(id) === filters.openPr), + ); + if (matching.length === 0) return []; + conditions.push(issueIdsCondition(issues.id, matching)); + } // Triage is worked oldest-first (SYD-159) — humans clear the inbox in // filing order rather than always seeing whatever landed most recently. // Every other view keeps the newest-first default. diff --git a/src/services/webhook-dispatcher.ts b/src/services/webhook-dispatcher.ts index 569c45c..c24650d 100644 --- a/src/services/webhook-dispatcher.ts +++ b/src/services/webhook-dispatcher.ts @@ -1,5 +1,5 @@ import { createHmac } from "node:crypto"; -import { asc, eq, gt } from "drizzle-orm"; +import { and, asc, eq, gt, lt } from "drizzle-orm"; import type { Db } from "../db/index.js"; import { actors, events, issues, projects, webhooks, webhookCursor } from "../db/schema.js"; import { releaseStaleClaims } from "./stale-claims.js"; @@ -9,7 +9,27 @@ import { expirePendingActions } from "./hard-gate.js"; import { getSetting } from "./settings.js"; import { emitProcessDeviations } from "./deviation.js"; -export async function dispatchPending(db: Db, fetchFn: typeof fetch = fetch): Promise { +// Switchyard runs one server process with one database connection. Coalesce +// overlapping callers, including timer ticks, until the current batch settles. +// Housekeeping stays outside this guard and keeps running on its own cadence. +const activeDispatches = new WeakMap>(); + +export function dispatchPending(db: Db, fetchFn: typeof fetch = fetch): Promise { + const active = activeDispatches.get(db); + if (active) return active; + const pending = dispatchBatch(db, fetchFn).finally(() => activeDispatches.delete(db)); + activeDispatches.set(db, pending); + return pending; +} + +function advanceCursor(db: Db, eventId: number): void { + db.update(webhookCursor) + .set({ lastEventId: eventId }) + .where(and(eq(webhookCursor.id, 1), lt(webhookCursor.lastEventId, eventId))) + .run(); +} + +async function dispatchBatch(db: Db, fetchFn: typeof fetch): Promise { let cursor = db.select().from(webhookCursor).where(eq(webhookCursor.id, 1)).get(); if (!cursor) { db.insert(webhookCursor).values({ id: 1, lastEventId: 0 }).run(); @@ -38,7 +58,7 @@ export async function dispatchPending(db: Db, fetchFn: typeof fetch = fetch): Pr let delivered = 0; for (const r of rows) { if (suppressed.has(r.e.type)) { - db.update(webhookCursor).set({ lastEventId: r.e.id }).where(eq(webhookCursor.id, 1)).run(); + advanceCursor(db, r.e.id); continue; } const body = JSON.stringify({ @@ -74,7 +94,7 @@ export async function dispatchPending(db: Db, fetchFn: typeof fetch = fetch): Pr console.error(`webhook ${h.id} -> ${h.url} failed: ${(err as Error).message}`); } } - db.update(webhookCursor).set({ lastEventId: r.e.id }).where(eq(webhookCursor.id, 1)).run(); + advanceCursor(db, r.e.id); } return delivered; } diff --git a/tests/db/indexes.test.ts b/tests/db/indexes.test.ts index 3226d23..f700974 100644 --- a/tests/db/indexes.test.ts +++ b/tests/db/indexes.test.ts @@ -15,4 +15,39 @@ describe("hot column indexes (SYD-142)", () => { expect(names).toContain("issues_assignee_id_idx"); expect(names).toContain("agent_sessions_issue_id_idx"); }); + + it("uses indexed reverse lookups inside task-selection subqueries", () => { + const db = openDb(":memory:"); + const plan = db + .all<{ detail: string }>( + sql`EXPLAIN QUERY PLAN + SELECT i.id FROM issues i WHERE i.status = 'todo' + AND NOT EXISTS (SELECT 1 FROM issues c WHERE c.parent_id = i.id AND c.status NOT IN ('done', 'canceled')) + AND NOT EXISTS (SELECT 1 FROM dependencies d JOIN issues b ON b.id = d.blocker_id + WHERE d.blocked_id = i.id AND b.status NOT IN ('done', 'canceled'))`, + ) + .map((row) => row.detail); + expect( + plan.some((detail) => detail.includes("SEARCH c USING INDEX issues_parent_id_idx")), + ).toBe(true); + expect( + plan.some((detail) => detail.includes("SEARCH d USING INDEX dependencies_blocked_id_idx")), + ).toBe(true); + expect(plan.some((detail) => /^SCAN [cd]$/.test(detail))).toBe(false); + }); + + it("seeks event types and project/status board pages without scanning history", () => { + const db = openDb(":memory:"); + const eventsPlan = db.all<{ detail: string }>(sql`EXPLAIN QUERY PLAN + SELECT issue_id, MAX(id) FROM events WHERE type = 'delivery_failed' GROUP BY issue_id`); + expect( + eventsPlan.some(({ detail }) => + detail.includes("SEARCH events USING COVERING INDEX events_type_issue_id_idx"), + ), + ).toBe(true); + const boardPlan = db.all<{ detail: string }>(sql`EXPLAIN QUERY PLAN + SELECT id, title FROM issues WHERE project_id = 1 AND status = 'done' AND id < 100 ORDER BY id DESC LIMIT 51`); + expect(boardPlan.some(({ detail }) => detail.includes("issues_project_status_idx"))).toBe(true); + expect(boardPlan.some(({ detail }) => detail.includes("TEMP B-TREE"))).toBe(false); + }); }); diff --git a/tests/mcp/write-tools.test.ts b/tests/mcp/write-tools.test.ts index f1a6788..f976a10 100644 --- a/tests/mcp/write-tools.test.ts +++ b/tests/mcp/write-tools.test.ts @@ -7,7 +7,7 @@ import path from "node:path"; import { openDb, type Db } from "../../src/db/index.js"; import { createActor, type Actor } from "../../src/services/actors.js"; import { createProject } from "../../src/services/projects.js"; -import { getIssue, SUMMARY_MAX_LENGTH } from "../../src/services/issues.js"; +import { createIssue, getIssue, SUMMARY_MAX_LENGTH } from "../../src/services/issues.js"; import { snoozeIssue } from "../../src/services/triage-actions.js"; import { buildMcpServer } from "../../src/mcp/server.js"; import { getActivity } from "../../src/services/comments.js"; @@ -36,6 +36,24 @@ beforeEach(async () => { }); describe("MCP write tools", () => { + it("refuses service actors adding a dependency through MCP", async () => { + createIssue(db, human, { projectKey: "AIPI", title: "Blocker" }); + createIssue(db, human, { projectKey: "AIPI", title: "Target" }); + const service = createActor(db, { name: "poller", type: "service" }).actor; + const serviceClient = await connect(service); + try { + const result = await serviceClient.callTool({ + name: "add_dependency", + arguments: { blocker_ref: "AIPI-1", blocked_ref: "AIPI-2" }, + }); + expect(result.isError).toBe(true); + expect(text(result)).toMatch(/cannot add dependencies/); + expect(getActivity(db, "AIPI-2").map((e) => e.type)).toEqual(["created"]); + } finally { + await serviceClient.close(); + } + }); + it("file_issue's description tells agents to set a suggested priority (SYD-65)", async () => { const { tools } = await client.listTools(); const fileIssue = tools.find((t) => t.name === "file_issue")!; diff --git a/tests/rest/api-pr-state.test.ts b/tests/rest/api-pr-state.test.ts index 0dfc477..ee43b0e 100644 --- a/tests/rest/api-pr-state.test.ts +++ b/tests/rest/api-pr-state.test.ts @@ -51,7 +51,7 @@ describe("GET /pr-state", () => { expect(res.status).toBe(200); const rows = (await res.json()) as { prNumber: number; status: string; issueRef: string }[]; expect(rows).toHaveLength(1); - expect(rows[0]).toMatchObject({ prNumber: 7, status: "open", issueRef: "SYD-1" }); + expect(rows[0]).toMatchObject({ prNumber: 7, status: "open", issueRef: null }); }); it("returns all rows for a repo without a status filter", async () => { diff --git a/tests/rest/api-service-actor.test.ts b/tests/rest/api-service-actor.test.ts index 8ec3346..a00a386 100644 --- a/tests/rest/api-service-actor.test.ts +++ b/tests/rest/api-service-actor.test.ts @@ -3,6 +3,7 @@ import { openDb, type Db } from "../../src/db/index.js"; import { createActor, type Actor } from "../../src/services/actors.js"; import { createProject } from "../../src/services/projects.js"; import { createIssue } from "../../src/services/issues.js"; +import { listDependencies } from "../../src/services/dependencies.js"; import { buildApiRoutes } from "../../src/rest/api-routes.js"; // SYD-213: REST-layer authorization for a `service` token. The service-layer @@ -23,6 +24,20 @@ beforeEach(() => { const svc = () => ({ authorization: `Bearer ${serviceToken}`, "content-type": "application/json" }); describe("service token — REST-layer guards", () => { + it("CANNOT add a dependency", async () => { + createIssue(db, human, { projectKey: "SYD", title: "Blocked target" }); + const res = await app.request("/dependencies", { + method: "POST", + headers: svc(), + body: JSON.stringify({ blockerRef: "SYD-1", blockedRef: "SYD-2" }), + }); + expect(res.status).toBe(400); + expect(await res.json()).toMatchObject({ + error: expect.stringMatching(/cannot add dependencies/), + }); + expect(listDependencies(db, "SYD-2").blockedBy).toEqual([]); + }); + it("CANNOT create an actor (requireHumanCaller)", async () => { const res = await app.request("/actors", { method: "POST", diff --git a/tests/rest/board-performance.test.ts b/tests/rest/board-performance.test.ts new file mode 100644 index 0000000..2daa0c6 --- /dev/null +++ b/tests/rest/board-performance.test.ts @@ -0,0 +1,200 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type Database from "better-sqlite3"; +import { openDb, type Db } from "../../src/db/index.js"; +import { buildApiRoutes } from "../../src/rest/api-routes.js"; +import { createActor, type Actor } from "../../src/services/actors.js"; +import { createProject } from "../../src/services/projects.js"; +import { createIssue, updateIssue } from "../../src/services/issues.js"; +import { recordEvent } from "../../src/services/events.js"; +import { addGithubRepo } from "../../src/services/github-repos.js"; +import { recordDeliveryEvent } from "../../src/services/delivery-events.js"; +import type { BoardPage, Issue } from "../../ui/src/types.js"; + +let db: Db, human: Actor, headers: Record; +let app: ReturnType; +beforeEach(() => { + db = openDb(":memory:"); + const actor = createActor(db, { name: "human", type: "human" }); + human = actor.actor; + headers = { authorization: `Bearer ${actor.token}` }; + createProject(db, human, { key: "SYD", name: "Switchyard" }); + createProject(db, human, { key: "OTHER", name: "Other project" }); + app = buildApiRoutes(db); +}); + +async function read(path: string): Promise { + const response = await app.request(path, { headers }); + expect(response.status).toBe(200); + return response.json() as Promise; +} + +function traceReads() { + const client = (db as unknown as { $client: Database.Database }).$client; + const prepare = client.prepare.bind(client); + const reads: { query: string; rows: number }[] = []; + const spy = vi.spyOn(client, "prepare").mockImplementation((query: string) => { + const statement = prepare(query); + const all = statement.all.bind(statement); + statement.all = ((...params: unknown[]) => { + const rows = all(...params); + reads.push({ query, rows: rows.length }); + return rows; + }) as typeof statement.all; + return statement; + }); + return { reads, restore: () => spy.mockRestore() }; +} + +describe("scoped issue enrichment", () => { + it("does no signal queries for an empty project/status selection", async () => { + const other = createIssue(db, human, { + projectKey: "OTHER", + title: "Unrelated", + description: "large".repeat(2000), + }); + recordEvent(db, { issueId: other.id, actorId: human.id, type: "delivery_failed" }); + const trace = traceReads(); + expect(await read("/issues?project=SYD&status=todo")).toEqual([]); + expect(trace.reads.some(({ query }) => /\b(events|pr_state|dependencies)\b/i.test(query))).toBe( + false, + ); + trace.restore(); + }); + + it("hydrates signals only for selected issues and preserves full legacy bodies", async () => { + const selected = createIssue(db, human, { + projectKey: "SYD", + title: "Selected", + description: "Keep this body", + }); + updateIssue(db, human, selected.ref, { status: "todo" }); + recordEvent(db, { + issueId: selected.id, + actorId: human.id, + type: "delivery_failed", + payload: { message: "local failure" }, + }); + for (let n = 0; n < 100; n++) { + const issue = createIssue(db, human, { + projectKey: "OTHER", + title: `Other ${n}`, + description: "unrelated".repeat(1000), + }); + updateIssue(db, human, issue.ref, { status: "done" }); + recordEvent(db, { + issueId: issue.id, + actorId: human.id, + type: "delivery_failed", + payload: { message: "unrelated failure" }, + }); + } + const trace = traceReads(); + const result = await read("/issues?project=SYD&status=todo&attention=delivery_failed"); + expect(result).toHaveLength(1); + expect(result[0]).toMatchObject({ + description: "Keep this body", + attention: { reason: "delivery_failed", message: "local failure" }, + }); + // The old implementation hydrated 100 unrelated issues/failures even + // when the selected project contained just one row. + expect(Math.max(...trace.reads.map((read) => read.rows))).toBeLessThanOrEqual(1); + // The attention filter's result is reused by the REST enrichment pass. + expect( + trace.reads.filter(({ query }) => query.includes("WHERE type = 'delivery_failed'")), + ).toHaveLength(1); + trace.restore(); + }); +}); + +describe("compact board pages", () => { + it("bounds each column, paginates all history, and leaves /issues compatible", async () => { + for (let n = 0; n < 120; n++) { + const issue = createIssue(db, human, { + projectKey: "SYD", + title: `Done ${n}`, + description: "body".repeat(2000), + }); + updateIssue(db, human, issue.ref, { status: "done" }); + } + createIssue(db, human, { projectKey: "OTHER", title: "Not on this board" }); + const first = await read("/board?project=SYD&done=all"); + expect(first.columns.done).toMatchObject({ total: 120, matching: 120 }); + expect(first.columns.done.items).toHaveLength(50); + expect(first.columns.done.items[0]).not.toHaveProperty("description"); + expect(JSON.stringify(first).length).toBeLessThan(30000); + const trace = traceReads(); + const second = await read( + `/board?project=SYD&done=all&before_done=${first.columns.done.nextBeforeId}`, + ); + expect(second.columns.done.items).toHaveLength(50); + expect( + second.columns.done.items.every( + (item) => !first.columns.done.items.some((prior) => prior.id === item.id), + ), + ).toBe(true); + expect(trace.reads.every(({ query }) => !query.includes('"description"'))).toBe(true); + trace.restore(); + const last = await read( + `/board?project=SYD&done=all&before_done=${second.columns.done.nextBeforeId}`, + ); + expect(last.columns.done.items).toHaveLength(20); + expect(last.columns.done.nextBeforeId).toBeNull(); + const legacy = await read("/issues?project=SYD"); + expect(legacy).toHaveLength(120); + expect(legacy[0].description).toBe("body".repeat(2000)); + }); + + it("applies done filters before paging, preserving OR semantics and total badges", async () => { + addGithubRepo(db, human, { fullName: "acme/widgets", projectKey: "SYD" }); + const failed = createIssue(db, human, { projectKey: "SYD", title: "Old failure" }); + const open = createIssue(db, human, { projectKey: "SYD", title: "Old open PR" }); + const unlinked = createIssue(db, human, { projectKey: "SYD", title: "Missing link" }); + for (const issue of [failed, open, unlinked]) + updateIssue(db, human, issue.ref, { status: "done" }); + recordEvent(db, { issueId: failed.id, actorId: human.id, type: "delivery_failed" }); + recordEvent(db, { + issueId: unlinked.id, + actorId: human.id, + type: "process_deviation", + payload: { reason: "done_without_merged_pr" }, + }); + recordDeliveryEvent(db, human, open.ref, { + type: "pr_opened", + prNumber: 7, + url: "https://github.com/acme/widgets/pull/7", + headSha: "reviewed-head", + }); + // Push both actionable cards beyond the first unfiltered history page. + for (let n = 0; n < 60; n++) { + const issue = createIssue(db, human, { projectKey: "SYD", title: `Clean ${n}` }); + updateIssue(db, human, issue.ref, { status: "done" }); + } + const defaults = await read("/board?project=SYD"); + expect(defaults.columns.done.total).toBe(63); + expect(defaults.columns.done.matching).toBe(2); + expect(defaults.columns.done.items.map((i) => i.id).sort()).toEqual( + [failed.id, open.id].sort(), + ); + expect(defaults.columns.done.items.find((i) => i.id === open.id)?.openPr?.headSha).toBe( + "reviewed-head", + ); + const missing = await read("/board?project=SYD&done=unlinked"); + expect(missing.columns.done.items.map((i) => i.id)).toEqual([unlinked.id]); + const small = await read("/board?project=SYD&limit=1"); + expect(small.columns.done.items).toHaveLength(1); + expect(small.columns.done.matching).toBe(2); + expect(await read("/issue-counts?project=SYD")).toEqual({ done: 63 }); + }); + + it("validates filters, page sizes, and cursors", async () => { + for (const query of [ + "done=invalid", + "limit=0", + "limit=101", + "before_done=-1", + "before_todo=not-a-number", + ]) { + expect((await app.request(`/board?project=SYD&${query}`, { headers })).status).toBe(400); + } + }); +}); diff --git a/tests/scripts/agent-worker-publish.test.ts b/tests/scripts/agent-worker-publish.test.ts index f270cf9..e083d46 100644 --- a/tests/scripts/agent-worker-publish.test.ts +++ b/tests/scripts/agent-worker-publish.test.ts @@ -8,12 +8,38 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { EventEmitter } from "node:events"; import type { WorkerConfig } from "../../scripts/worker-select.js"; +import { + savePublication, + clearPublication, + readPublications, +} from "../../scripts/worker-publications.js"; const spawnMock = vi.fn(); +const dockerPs = vi.fn(() => ""); + +vi.mock("../../scripts/worker-publications.js", () => ({ + savePublication: vi.fn(), + readPublications: vi.fn(() => []), + clearPublication: vi.fn(), +})); vi.mock("node:child_process", async (importOriginal) => { const actual = await importOriginal(); - return { ...actual, spawn: (...args: unknown[]) => spawnMock(...args) }; + return { + ...actual, + spawn: (...args: unknown[]) => spawnMock(...args), + execFile: ( + _command: string, + _args: string[], + callback: (error: Error | null, result?: { stdout: string; stderr: string }) => void, + ) => { + try { + callback(null, { stdout: dockerPs(), stderr: "" }); + } catch (error) { + callback(error as Error); + } + }, + }; }); vi.mock("node:fs", async (importOriginal) => { @@ -37,7 +63,8 @@ vi.mock("../../scripts/delivery-exec.js", () => ({ originOwnerRepo: (...args: unknown[]) => originOwnerRepo(...args), })); -const { dispatch, active, activeMode } = await import("../../scripts/agent-worker.js"); +const { dispatch, active, activeMode, recoverPublications } = + await import("../../scripts/agent-worker.js"); class FakeChildProcess extends EventEmitter { pid: number | undefined; @@ -88,6 +115,10 @@ beforeEach(() => { active.clear(); activeMode.clear(); spawnMock.mockReset(); + dockerPs.mockReset().mockReturnValue(""); + vi.mocked(readPublications).mockReturnValue([]); + vi.mocked(savePublication).mockClear(); + vi.mocked(clearPublication).mockClear(); publishAgentBranch.mockReset(); prFreshness.mockReset(); originOwnerRepo.mockReset(); @@ -187,3 +218,66 @@ describe("publish failure surfaces on the board (SYD-257)", () => { errorSpy.mockRestore(); }); }); + +describe("publication intent cleanup", () => { + it("clears the intent when spawn throws before starting a container", () => { + spawnMock.mockImplementation(() => { + throw new Error("spawn setup failed"); + }); + dispatch(issue, config, "tok", "code"); + expect(savePublication).toHaveBeenCalledOnce(); + expect(clearPublication).toHaveBeenCalledWith("/repo/syd", config, "SYD-9"); + expect(publishAgentBranch).not.toHaveBeenCalled(); + }); + + it("clears the intent on an asynchronous failed spawn", () => { + const child = new FakeChildProcess(); + spawnMock.mockReturnValue(child); + dispatch(issue, config, "tok", "code"); + child.emit("error", new Error("ENOENT: docker")); + expect(clearPublication).toHaveBeenCalledWith("/repo/syd", config, "SYD-9"); + expect(publishAgentBranch).not.toHaveBeenCalled(); + }); +}); + +describe("container completion verification", () => { + it("keeps the obligation when the local docker client exits before its container", async () => { + const child = new FakeChildProcess(); + spawnMock.mockReturnValue(child); + dockerPs.mockReturnValue("syd-SYD-9\n"); + publishAgentBranch.mockResolvedValue({ status: "no-commits" }); + dispatch(issue, config, "tok", "code"); + child.pid = 111; + child.emit("spawn"); + child.emit("exit", 1); + await vi.waitFor(() => expect(dockerPs).toHaveBeenCalledOnce()); + expect(publishAgentBranch).not.toHaveBeenCalled(); + expect(clearPublication).not.toHaveBeenCalled(); + expect( + fetchMock.mock.calls.some( + ([url, init]) => String(url).includes("/agent-sessions/") && init?.method === "PATCH", + ), + ).toBe(false); + vi.mocked(readPublications).mockReturnValue([ + { ref: issue.ref, issueTitle: issue.title, sessionId: null, exitCode: 1 }, + ]); + await recoverPublications(config, "tok", new Set()); + expect(publishAgentBranch).toHaveBeenCalledOnce(); + expect(clearPublication).toHaveBeenCalledWith("/repo/syd", config, issue.ref); + }); + + it("preserves the publication marker if Docker inspection fails", async () => { + const child = new FakeChildProcess(); + spawnMock.mockReturnValue(child); + dockerPs.mockImplementation(() => { + throw new Error("Docker daemon unavailable"); + }); + dispatch(issue, config, "tok", "code"); + child.pid = 111; + child.emit("spawn"); + child.emit("exit", 1); + await vi.waitFor(() => expect(dockerPs).toHaveBeenCalledOnce()); + expect(publishAgentBranch).not.toHaveBeenCalled(); + expect(clearPublication).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/scripts/agent-worker.test.ts b/tests/scripts/agent-worker.test.ts index f7b7a20..ff6538d 100644 --- a/tests/scripts/agent-worker.test.ts +++ b/tests/scripts/agent-worker.test.ts @@ -5,6 +5,12 @@ import { answerKey, type WorkerConfig } from "../../scripts/worker-select.js"; const spawnMock = vi.fn(); +vi.mock("../../scripts/worker-publications.js", () => ({ + savePublication: vi.fn(), + readPublications: vi.fn(() => []), + clearPublication: vi.fn(), +})); + vi.mock("node:child_process", async (importOriginal) => { const actual = await importOriginal(); return { ...actual, spawn: (...args: unknown[]) => spawnMock(...args) }; diff --git a/tests/scripts/deliver.test.ts b/tests/scripts/deliver.test.ts index 2a89510..7e4af00 100644 --- a/tests/scripts/deliver.test.ts +++ b/tests/scripts/deliver.test.ts @@ -33,7 +33,7 @@ vi.mock("../../scripts/delivery-exec.js", () => ({ closeDeadAgentPr: (...args: unknown[]) => closeDeadAgentPr(...args), })); -const { deliverQueue, tick, warnOnRelaxedBranchProtection } = +const { deliverQueue, tick, warnOnRelaxedBranchProtection, startDeliveryPolling } = await import("../../scripts/deliver.js"); const token = "test-token"; @@ -850,3 +850,48 @@ describe("warnOnRelaxedBranchProtection (SYD-209/SYD-222)", () => { expect(failing).toEqual(["SYD", "NOC"]); }); }); + +describe("scheduled delivery protection scope (review finding 1)", () => { + it("keeps a blocked project out of both the initial and later ticks", async () => { + resetExecMocks(); + vi.useFakeTimers(); + const config: WorkerConfig = { + url: "http://localhost:3300", + label: "auto", + intervalSeconds: 300, + maxConcurrent: 1, + projects: { SYD: { repo: "/repo/syd" } }, + delivery: { requireBranchProtection: true, pollSeconds: 1 }, + }; + installFetch({ + pending: [ + { + authorizationId: 5, + ref: "SYD-9", + kind: "done_stamp", + pin: { repo: "acme/widgets", prNumber: 42, headSha: "s0" }, + }, + ], + unfinished: [], + deployRetries: [], + }); + const gate = newTickGate(); + const timer = startDeliveryPolling(config, token, gate, false, []); + try { + await tick(config, token, gate, false, []); + // Even a future caller that forgets the verified set fails closed. + await tick(config, token, gate, false); + await vi.advanceTimersByTimeAsync(2_000); + expect( + fetchMock().mock.calls.filter(([u]) => String(u).endsWith("/api/delivery-work")), + ).toHaveLength(4); + expect(startCalls("SYD-9")).toHaveLength(0); + expect(prLiveState).not.toHaveBeenCalled(); + expect(mergeAgentPr).not.toHaveBeenCalled(); + } finally { + clearInterval(timer); + vi.useRealTimers(); + vi.unstubAllGlobals(); + } + }); +}); diff --git a/tests/scripts/delivery-exec.test.ts b/tests/scripts/delivery-exec.test.ts index 4cecda3..48d7981 100644 --- a/tests/scripts/delivery-exec.test.ts +++ b/tests/scripts/delivery-exec.test.ts @@ -20,6 +20,7 @@ import { attemptAutoRebase, checkBranchProtection, mergeAgentPr, + publishAgentBranch, closeDeadAgentPr, } from "../../scripts/delivery-exec.js"; @@ -394,6 +395,95 @@ describe("attemptAutoRebase S0 anchor (SYD-209)", () => { expect(res).toEqual({ status: "head-moved", observed: head }); }); + it("retains publication errors instead of treating an unreadable repository as an absent branch", async () => { + const { repo } = await setupRemoteAndAgentBranch(); + writeFileSync(path.join(repo, ".git", "config"), "[invalid git configuration"); + await expect( + publishAgentBranch(repo, "TEST-1", "test", "http://tracker.invalid"), + ).rejects.toThrow(/config/); + }); + + it("still recognizes a positively absent local publication branch", async () => { + const { repo } = await setupRemoteAndAgentBranch(); + await expect( + publishAgentBranch(repo, "TEST-999", "test", "http://tracker.invalid"), + ).resolves.toEqual({ status: "no-branch" }); + }); + + it("does not classify a failed transport as a missing branch (review finding 2)", async () => { + const { repo, head } = await setupRemoteAndAgentBranch(); + const cloneDir = mkdtempSync(path.join(tmpdir(), "fetch-error-clone-")); + const binDir = mkdtempSync(path.join(tmpdir(), "fetch-error-bin-")); + const realGit = (await execFileP("which", ["git"])).stdout.trim(); + writeFileSync( + path.join(binDir, "git"), + `#!/bin/sh +case "$*" in *"fetch origin agent/TEST-1"*) echo "simulated transport reset" >&2; exit 128;; esac +exec "${realGit}" "$@" +`, + { mode: 0o755 }, + ); + const oldPath = process.env.PATH; + process.env.PATH = `${binDir}${path.delimiter}${oldPath}`; + try { + await expect(attemptAutoRebase(repo, cloneDir, "TEST-1", [head])).rejects.toThrow( + "simulated transport reset", + ); + } finally { + process.env.PATH = oldPath; + } + expect( + (await execFileP("git", ["-C", repo, "ls-remote", "origin", "refs/heads/agent/TEST-1"])) + .stdout, + ).toContain(head); + }); + + it("returns no-branch only when a successful remote query confirms absence", async () => { + const { repo, head } = await setupRemoteAndAgentBranch(); + await g(repo, "push", "origin", "--delete", "agent/TEST-1"); + const cloneDir = mkdtempSync(path.join(tmpdir(), "missing-branch-clone-")); + await expect(attemptAutoRebase(repo, cloneDir, "TEST-1", [head])).resolves.toEqual({ + status: "no-branch", + }); + }); + + it("preserves a valid branch when nonconflicting rebase fails to sign", async () => { + const { repo, head } = await setupRemoteAndAgentBranch(); + await g(repo, "checkout", "main"); + writeFileSync(path.join(repo, "unrelated.txt"), "main advanced"); + await g(repo, "add", "."); + await g(repo, "commit", "-qm", "advance main"); + await g(repo, "push", "origin", "main"); + const cloneDir = mkdtempSync(path.join(tmpdir(), "signing-error-clone-")); + const remote = ( + await execFileP("git", ["-C", repo, "remote", "get-url", "origin"]) + ).stdout.trim(); + await execFileP("git", ["clone", remote, cloneDir]); + await g(cloneDir, "config", "user.name", "test"); + await g(cloneDir, "config", "user.email", "test@example.com"); + await g(cloneDir, "config", "commit.gpgsign", "true"); + await g(cloneDir, "config", "gpg.program", "/nonexistent-review-gpg"); + await expect(attemptAutoRebase(repo, cloneDir, "TEST-1", [head])).rejects.toThrow(/gpg|sign/); + expect( + (await execFileP("git", ["-C", repo, "ls-remote", "origin", "refs/heads/agent/TEST-1"])) + .stdout, + ).toContain(head); + }); + + it("still identifies an actual conflicting index", async () => { + const { repo, head } = await setupRemoteAndAgentBranch(); + await g(repo, "checkout", "main"); + writeFileSync(path.join(repo, "feat.txt"), "different feature"); + await g(repo, "add", "."); + await g(repo, "commit", "-qm", "conflicting main change"); + await g(repo, "push", "origin", "main"); + const cloneDir = mkdtempSync(path.join(tmpdir(), "real-conflict-clone-")); + await expect(attemptAutoRebase(repo, cloneDir, "TEST-1", [head])).resolves.toEqual({ + status: "conflict", + files: ["feat.txt"], + }); + }); + it("rebases and force-pushes when the fetched head is the authorized S0", async () => { const { repo, head } = await setupRemoteAndAgentBranch(); const cloneDir = mkdtempSync(path.join(tmpdir(), "anchor-clone-")); diff --git a/tests/scripts/github-poll-lib.test.ts b/tests/scripts/github-poll-lib.test.ts index d35bf23..c2961cb 100644 --- a/tests/scripts/github-poll-lib.test.ts +++ b/tests/scripts/github-poll-lib.test.ts @@ -67,6 +67,14 @@ describe("diffRepoState / pull requests", () => { ]); }); + it("emits reopened for a closed PR confirmed open, including after local state resets", () => { + const prior: RepoPollState = { 1: { state: "CLOSED", lastRunConclusion: null } }; + expect(diffRepoState([pr({})], new Map(), prior).events[0].payload.action).toBe("reopened"); + expect(diffRepoState([pr({})], new Map(), {}, new Set([1])).events[0].payload.action).toBe( + "reopened", + ); + }); + it("emits closed (merged:false) when a tracked-open PR is later closed unmerged", () => { const prior: RepoPollState = { 1: { state: "OPEN", lastRunConclusion: null } }; const { events, next } = diffRepoState([pr({ state: "CLOSED" })], new Map(), prior); diff --git a/tests/scripts/github-poll.test.ts b/tests/scripts/github-poll.test.ts index e5df97b..4dd616b 100644 --- a/tests/scripts/github-poll.test.ts +++ b/tests/scripts/github-poll.test.ts @@ -8,6 +8,15 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import type { WorkerConfig } from "../../scripts/worker-select.js"; import type { GhPr, PollStateFile } from "../../scripts/github-poll-lib.js"; +import { openDb } from "../../src/db/index.js"; +import { createActor } from "../../src/services/actors.js"; +import { createProject } from "../../src/services/projects.js"; +import { createIssue } from "../../src/services/issues.js"; +import { addGithubRepo } from "../../src/services/github-repos.js"; +import { declarePrLink } from "../../src/services/pr-links.js"; +import { listPrState, upsertPrState } from "../../src/services/pr-state.js"; +import { getOpenPr } from "../../src/services/pr-status.js"; +import { handleGithubWebhook } from "../../src/services/github-webhook.js"; vi.mock("../../scripts/github-poll-exec.js", () => ({ listPullRequests: vi.fn(), @@ -38,10 +47,10 @@ const fetchMock = vi.fn(); /** Routes GET /api/pr-state to `openRows` and every other call (the * /api/github-events POSTs) to a generic ok outcome. */ -function routeFetch(openRows: { prNumber: number }[]) { +function routeFetch(rows: { prNumber: number; status?: "open" | "closed" | "merged" }[]) { fetchMock.mockImplementation(async (url: unknown) => { if (String(url).includes("/api/pr-state")) { - return { ok: true, json: async () => openRows }; + return { ok: true, json: async () => rows.map((r) => ({ status: "open", ...r })) }; } return { ok: true, json: async () => ({ ok: true, handled: true, duplicate: true }) }; }); @@ -58,6 +67,90 @@ afterEach(() => { }); describe("pollRepo", () => { + it.each([undefined, "OPEN", "CLOSED"] as const)( + "repairs a server-side close via live-confirmed reopen with local state %s", + async (priorState) => { + const db = openDb(":memory:"); + const human = createActor(db, { name: "sean", type: "human" }).actor; + createProject(db, human, { key: "SYD", name: "Switchyard" }); + const issue = createIssue(db, human, { projectKey: "SYD", title: "Ship it" }); + addGithubRepo(db, human, { fullName: "acme/widgets", projectKey: "SYD" }); + declarePrLink(db, human, "SYD-1", { repo: "acme/widgets", prNumber: 7 }); + upsertPrState(db, human, { + repo: "acme/widgets", + prNumber: 7, + status: "closed", + ghUpdatedAt: "2026-07-12T09:00:00Z", + }); + vi.mocked(listPullRequests).mockResolvedValue([openPr(7)]); + const live = { ...openPr(7), updatedAt: "2026-07-12T11:00:00Z" }; + vi.mocked(viewPullRequest).mockResolvedValue(live); + fetchMock.mockImplementation(async (url: string, init?: RequestInit) => { + if (url.includes("/api/pr-state")) { + return { ok: true, json: async () => listPrState(db, { repo: "acme/widgets" }) }; + } + const { event, payload, repo } = JSON.parse(String(init?.body)); + const outcome = handleGithubWebhook(db, event, payload, repo); + return { ok: true, json: async () => outcome }; + }); + const state: PollStateFile = priorState + ? { "acme/widgets": { 7: { state: priorState, lastRunConclusion: null } } } + : {}; + + await pollRepo("acme/widgets", config, "tok", state, false); + expect(viewPullRequest).toHaveBeenCalledWith("acme/widgets", 7); + expect(getOpenPr(db, issue.id)?.prNumber).toBe(7); + const posts = fetchMock.mock.calls.filter(([url]) => url.endsWith("/api/github-events")); + expect(JSON.parse(posts[0][1].body)).toMatchObject({ + payload: { action: "reopened", pull_request: { updated_at: live.updatedAt } }, + }); + + // The repaired baseline stops spending targeted calls on ordinary open refreshes. + vi.mocked(viewPullRequest).mockClear(); + await pollRepo("acme/widgets", config, "tok", state, false); + expect(viewPullRequest).not.toHaveBeenCalled(); + expect(getOpenPr(db, issue.id)?.prNumber).toBe(7); + }, + ); + + it("does not reopen a stale OPEN search hit when the targeted PR is still closed", async () => { + vi.mocked(listPullRequests).mockResolvedValue([openPr(7)]); + vi.mocked(viewPullRequest).mockResolvedValue({ ...openPr(7), state: "CLOSED" }); + routeFetch([{ prNumber: 7, status: "closed" }]); + const state: PollStateFile = {}; + + await pollRepo("acme/widgets", config, "tok", state, false); + const posts = fetchMock.mock.calls.filter(([url]) => + String(url).endsWith("/api/github-events"), + ); + expect(JSON.parse(posts[0][1].body)).toMatchObject({ payload: { action: "closed" } }); + expect(state["acme/widgets"][7].state).toBe("CLOSED"); + }); + + it("retries failed reopen confirmation without advancing or emitting the unverified state", async () => { + vi.mocked(listPullRequests).mockResolvedValue([openPr(7)]); + vi.mocked(viewPullRequest).mockRejectedValueOnce(new Error("GitHub unavailable")); + routeFetch([{ prNumber: 7, status: "closed" }]); + const state: PollStateFile = { + "acme/widgets": { 7: { state: "CLOSED", lastRunConclusion: null } }, + }; + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + try { + await pollRepo("acme/widgets", config, "tok", state, false); + expect(state["acme/widgets"][7].state).toBe("CLOSED"); + expect( + fetchMock.mock.calls.filter(([url]) => String(url).endsWith("/api/github-events")), + ).toHaveLength(0); + expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining("reopen confirmation")); + vi.mocked(viewPullRequest).mockResolvedValue(openPr(7)); + await pollRepo("acme/widgets", config, "tok", state, false); + expect(state["acme/widgets"][7].state).toBe("OPEN"); + expect(viewPullRequest).toHaveBeenCalledTimes(2); + } finally { + errorSpy.mockRestore(); + } + }); + it("does not persist repo state when posting an event fails", async () => { vi.mocked(listPullRequests).mockResolvedValue([openPr(7)]); fetchMock.mockRejectedValue(new Error("tracker down")); diff --git a/tests/scripts/worker-publications.test.ts b/tests/scripts/worker-publications.test.ts new file mode 100644 index 0000000..333cd0c --- /dev/null +++ b/tests/scripts/worker-publications.test.ts @@ -0,0 +1,126 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { mkdtempSync, rmSync } from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import type { WorkerConfig } from "../../scripts/worker-select.js"; +import { readPublications, savePublication } from "../../scripts/worker-publications.js"; + +const publishAgentBranch = vi.fn(); +vi.mock("../../scripts/delivery-exec.js", () => ({ + publishAgentBranch: (...args: unknown[]) => publishAgentBranch(...args), + prFreshness: async () => ({ headSha: "head", ghUpdatedAt: "2026-09-27T12:00:00Z" }), + originOwnerRepo: async () => "acme/widgets", +})); +const { reconcileContainerSessions, recoverPublications } = + await import("../../scripts/agent-worker.js"); + +let repo: string; +let config: WorkerConfig; +const publication = { ref: "SYD-9", issueTitle: "Ship it", sessionId: 12, exitCode: 0 }; +const opened = { status: "opened", prNumber: 42, url: "https://github.com/acme/widgets/pull/42" }; + +beforeEach(() => { + repo = mkdtempSync(path.join(os.tmpdir(), "publication-recovery-")); + config = { + url: "http://localhost:3300", + label: "auto", + intervalSeconds: 300, + maxConcurrent: 1, + containerized: true, + projects: { SYD: { repo } }, + delivery: { openPrs: true }, + }; + publishAgentBranch.mockReset(); + publishAgentBranch.mockResolvedValue(opened); +}); +afterEach(() => { + vi.unstubAllGlobals(); + rmSync(repo, { recursive: true, force: true }); +}); + +function installFetch(sessions: unknown[] = [], failObservation = false) { + const fetch = vi.fn(async (url: unknown, init?: RequestInit) => { + const u = String(url); + if (u.includes("?active=true")) return new Response(JSON.stringify(sessions)); + if (u.endsWith("/api/agent-sessions/12")) { + // A crash immediately after this response must still leave durable work. + expect(readPublications(repo, config)).toHaveLength(1); + } + if ( + failObservation && + u.endsWith("/delivery-events") && + JSON.parse(init?.body as string).type === "pr_opened" + ) { + return new Response("test refusal", { status: 400 }); + } + return new Response("{}"); + }); + vi.stubGlobal("fetch", fetch); + return fetch; +} + +describe("durable publication recovery (review finding 11)", () => { + it("publishes a container that completed while the host worker was down", async () => { + const fetch = installFetch([ + { id: 12, ref: "SYD-9", issueTitle: "Ship it", mode: "container" }, + ]); + await reconcileContainerSessions(config, "test-token", { + listLiveContainerNames: async () => new Set(), + }); + expect(publishAgentBranch).toHaveBeenCalledWith(repo, "SYD-9", "Ship it", config.url, null); + expect( + fetch.mock.calls.some( + ([url, init]) => + String(url).endsWith("/delivery-events") && + JSON.parse(init?.body as string).type === "pr_opened", + ), + ).toBe(true); + expect(readPublications(repo, config)).toEqual([]); + }); + + it("retries after session completion even when no active session remains on the server", async () => { + savePublication(repo, config, publication); + installFetch(); + publishAgentBranch.mockRejectedValueOnce( + new Error("fatal: unable to read local branch: Input/output error"), + ); + await recoverPublications(config, "test-token", new Set()); + expect(readPublications(repo, config)).toEqual([publication]); + await reconcileContainerSessions(config, "test-token", { + listLiveContainerNames: async () => new Set(), + }); + expect(publishAgentBranch).toHaveBeenCalledTimes(2); + expect(readPublications(repo, config)).toEqual([]); + }); + + it("retains the outbox until the PR observation is acknowledged", async () => { + savePublication(repo, config, publication); + installFetch([], true); + await recoverPublications(config, "test-token", new Set()); + expect(readPublications(repo, config)).toEqual([publication]); + installFetch(); + publishAgentBranch.mockResolvedValue({ ...opened, status: "already-open" }); + await recoverPublications(config, "test-token", new Set()); + expect(publishAgentBranch).toHaveBeenCalledTimes(2); + expect(readPublications(repo, config)).toEqual([]); + }); + + it("recovers the pre-spawn marker after the server has swept the session", async () => { + savePublication(repo, config, { ...publication, sessionId: null, exitCode: null }); + installFetch(); + await reconcileContainerSessions(config, "test-token", { + listLiveContainerNames: async () => new Set(), + }); + expect(publishAgentBranch).toHaveBeenCalledOnce(); + expect(readPublications(repo, config)).toEqual([]); + }); + + it("leaves still-running containers and other engine outboxes untouched", async () => { + savePublication(repo, config, publication); + installFetch(); + await recoverPublications(config, "test-token", new Set(["syd-SYD-9"])); + expect(publishAgentBranch).not.toHaveBeenCalled(); + expect(readPublications(repo, { ...config, token: "SWITCHYARD_CODEX_TOKEN" })).toEqual([]); + expect(readPublications(repo, config)).toEqual([publication]); + }); +}); diff --git a/tests/services/attention.test.ts b/tests/services/attention.test.ts index 6864d1f..c19a17b 100644 --- a/tests/services/attention.test.ts +++ b/tests/services/attention.test.ts @@ -64,6 +64,7 @@ describe("getAttention", () => { // lands in pr_state with a co-written transition event newer than the // failure — that is what clears the flag now (the deleted SYD-94 // reconcile pass used to do this with per-ref gh lookups). + declarePrLink(db, human, "SYD-1", { repo: REPO, prNumber: 7 }); upsertPrState(db, human, { repo: REPO, prNumber: 7, @@ -197,6 +198,7 @@ describe("getAttention — done_without_merged_pr (SYD-204)", () => { updateIssue(db, human, "SYD-1", { status: "in_review" }); updateIssue(db, human, "SYD-1", { status: "done" }); expect(getAttention(db, getIssue(db, "SYD-1").id)?.reason).toBe("done_without_merged_pr"); + declarePrLink(db, human, "SYD-1", { repo: REPO, prNumber: 9 }); upsertPrState(db, human, { repo: REPO, prNumber: 9, @@ -466,6 +468,7 @@ describe("getAttention — done_without_merged_pr (SYD-204)", () => { const { db, human, agent } = setup(); updateIssue(db, human, "SYD-1", { status: "todo" }); claimIssue(db, agent, "SYD-1"); + declarePrLink(db, human, "SYD-1", { repo: REPO, prNumber: 41 }); recordDeliveryEvent(db, human, "SYD-1", { type: "delivered", prNumber: 41, diff --git a/tests/services/delivery-attempts.test.ts b/tests/services/delivery-attempts.test.ts index 2f95db8..60bf8aa 100644 --- a/tests/services/delivery-attempts.test.ts +++ b/tests/services/delivery-attempts.test.ts @@ -492,6 +492,7 @@ describe("poller-down done-stamp then recovery (SYD-228)", () => { expect(listPendingDeliveryAuthorizations(db)).toEqual([]); // The poller recovers and observes the still-open PR. + declarePrLink(db, human, issue.ref, { repo: REPO, prNumber: 41 }); upsertPrState(db, human, { repo: REPO, prNumber: 41, diff --git a/tests/services/delivery-events.test.ts b/tests/services/delivery-events.test.ts index ac8ae7e..9410c2a 100644 --- a/tests/services/delivery-events.test.ts +++ b/tests/services/delivery-events.test.ts @@ -169,7 +169,7 @@ describe("recordDeliveryEvent / ingestion groundwork (SYD-205)", () => { const row = findPrState(db, "acme/bound", 12)!; expect(row).toMatchObject({ status: "open", - issueRef: "SYD-1", + issueRef: null, branch: "agent/SYD-1", headSha: "a".repeat(40), }); @@ -203,7 +203,7 @@ describe("recordDeliveryEvent / ingestion groundwork (SYD-205)", () => { repo: "Acme/Bound", }); expect(getActivity(db, "SYD-1")[1].payload).toMatchObject({ repo: "acme/bound" }); - expect(findPrState(db, "acme/bound", 12)!.issueRef).toBe("SYD-1"); + expect(findPrState(db, "acme/bound", 12)!.issueRef).toBeNull(); }); it("never writes pr_state when the event's repo is not bound to the issue's project", () => { diff --git a/tests/services/deviation.test.ts b/tests/services/deviation.test.ts index 5dc279f..b6a5fea 100644 --- a/tests/services/deviation.test.ts +++ b/tests/services/deviation.test.ts @@ -1,3 +1,4 @@ +import { declarePrLink } from "../../src/services/pr-links.js"; import { describe, it, expect } from "vitest"; import { eq } from "drizzle-orm"; import { openDb, type Db } from "../../src/db/index.js"; @@ -263,6 +264,7 @@ describe("emitProcessDeviations", () => { branch: "agent/SYD-1", url: `https://github.com/${REPO}/pull/41`, }; + declarePrLink(db, human, "SYD-1", { repo: REPO, prNumber: 41 }); upsertPrState(db, human, { ...observation, status: "open", @@ -343,6 +345,7 @@ describe("getDeviation — done_pr_not_delivered (SYD-261)", () => { updateIssue(db, human, "SYD-1", { status: "in_review" }); updateIssue(db, human, "SYD-1", { status: "done" }); // The PR registers AFTER the stamp — the ordering this issue is about. + declarePrLink(db, human, "SYD-1", { repo: REPO, prNumber: 304 }); upsertPrState(db, human, { repo: REPO, prNumber: 304, @@ -538,6 +541,7 @@ describe("updateIssue done transition — done_without_merged_pr (SYD-204)", () createIssue(db, human, { projectKey: "SYD", title: "Ship it" }); updateIssue(db, human, "SYD-1", { status: "todo" }); claimIssue(db, agent, "SYD-1"); + declarePrLink(db, human, "SYD-1", { repo: REPO, prNumber: 41 }); recordDeliveryEvent(db, human, "SYD-1", { type: "delivered", prNumber: 41, diff --git a/tests/services/github-webhook.test.ts b/tests/services/github-webhook.test.ts index 0c73dad..753d241 100644 --- a/tests/services/github-webhook.test.ts +++ b/tests/services/github-webhook.test.ts @@ -14,7 +14,7 @@ import { refsFromText, repositoryFullName, } from "../../src/services/github-webhook.js"; -import { listLiveLinks } from "../../src/services/pr-links.js"; +import { declarePrLink, listLiveLinks } from "../../src/services/pr-links.js"; function setup(boundRepos: string[] = []) { const db = openDb(":memory:"); @@ -567,10 +567,12 @@ describe("handleGithubWebhook / pr_state integration (SYD-206)", () => { it("writes an attributed pr_state row on opened, with exactly one gh_pr_opened event (co-write, no double record)", () => { const db = setup(["acme/bound"]); + const declarer = createActor(db, { name: "declarer", type: "human" }).actor; + declarePrLink(db, declarer, "SYD-1", { repo: "acme/bound", prNumber: 12 }); const outcome = handleGithubWebhook(db, "pull_request", opened("opened")); expect(outcome).toEqual({ handled: true, ref: "SYD-1", type: "gh_pr_opened" }); const row = findPrState(db, "acme/bound", 12)!; - expect(row).toMatchObject({ status: "open", issueRef: "SYD-1", headSha: "a".repeat(40) }); + expect(row).toMatchObject({ status: "open", issueRef: null, headSha: "a".repeat(40) }); expect(getActivity(db, "SYD-1").filter((a) => a.type === "gh_pr_opened")).toHaveLength(1); }); @@ -684,6 +686,8 @@ describe("handleGithubWebhook / pr_state integration (SYD-206)", () => { it("attributes and writes pr_state despite a casing mismatch between the linked repo and the payload's repository.full_name (SYD-212)", () => { // Repo linked with a hand-typed lowercase full name... const db = setup(["acme/bound"]); + const declarer = createActor(db, { name: "declarer", type: "human" }).actor; + declarePrLink(db, declarer, "SYD-1", { repo: "acme/bound", prNumber: 12 }); // ...but the real webhook delivery carries GitHub's canonical case. const outcome = handleGithubWebhook(db, "pull_request", { ...opened("opened"), @@ -691,7 +695,7 @@ describe("handleGithubWebhook / pr_state integration (SYD-206)", () => { }); expect(outcome).toEqual({ handled: true, ref: "SYD-1", type: "gh_pr_opened" }); const row = findPrState(db, "acme/bound", 12)!; - expect(row).toMatchObject({ status: "open", issueRef: "SYD-1", repo: "acme/bound" }); + expect(row).toMatchObject({ status: "open", issueRef: null, repo: "acme/bound" }); // The stored row itself is normalized, not left as the canonical-case // string the payload happened to carry. expect(findPrState(db, "Acme/Bound", 12)?.repo).toBe("acme/bound"); diff --git a/tests/services/ingestion-authority.test.ts b/tests/services/ingestion-authority.test.ts new file mode 100644 index 0000000..b96b764 --- /dev/null +++ b/tests/services/ingestion-authority.test.ts @@ -0,0 +1,166 @@ +import { describe, expect, it } from "vitest"; +import { createHmac } from "node:crypto"; +import { githubRepos } from "../../src/db/schema.js"; +import { openDb } from "../../src/db/index.js"; +import { createActor } from "../../src/services/actors.js"; +import { createProject } from "../../src/services/projects.js"; +import { createIssue, claimIssue, updateIssue } from "../../src/services/issues.js"; +import { addGithubRepo } from "../../src/services/github-repos.js"; +import { handleGithubWebhook } from "../../src/services/github-webhook.js"; +import { recordDeliveryEvent } from "../../src/services/delivery-events.js"; +import { declarePrLink, listLiveLinkViews, revokePrLink } from "../../src/services/pr-links.js"; +import { getMergedPr, getOpenPr } from "../../src/services/pr-status.js"; +import { findPrState } from "../../src/services/pr-state.js"; +import { buildGithubWebhookRoutes } from "../../src/rest/github-routes.js"; +import { buildApiRoutes } from "../../src/rest/api-routes.js"; + +const repo = "acme/widgets"; +const head = "a".repeat(40); +function setup() { + const db = openDb(":memory:"); + const human = createActor(db, { name: "reviewer", type: "human" }).actor; + const agent = createActor(db, { name: "worker", type: "agent" }).actor; + const service = createActor(db, { name: "host", type: "service" }); + createProject(db, human, { key: "SYD", name: "Switchyard" }); + const issue = createIssue(db, human, { projectKey: "SYD", title: "Real work" }); + updateIssue(db, human, issue.ref, { status: "todo" }); + addGithubRepo(db, human, { fullName: repo, projectKey: "SYD" }); + return { db, human, agent, service, issue }; +} +function payload(action = "opened", timestamp = "2030-01-01T00:00:00Z") { + return { + action, + repository: { full_name: repo }, + pull_request: { + number: 7, + title: "Unrelated change", + head: { ref: "agent/SYD-1", sha: head, repo: { full_name: "attacker/fork" } }, + updated_at: timestamp, + merged: action === "closed", + merge_commit_sha: action === "closed" ? "b".repeat(40) : null, + html_url: `https://github.com/${repo}/pull/7`, + }, + }; +} +const published = { + type: "pr_opened" as const, + repo, + prNumber: 7, + headSha: head, + ghUpdatedAt: "2030-01-01T00:00:00Z", + url: `https://github.com/${repo}/pull/7`, +}; + +describe("GitHub observations cannot assert delivery attribution", () => { + it.each(["webhook", "poller"])( + "observes a fork through %s without trusting its branch", + async (channel) => { + const { db, issue, agent, service } = setup(); + for (const action of ["opened", "closed"]) { + const body = payload(action); + let response: Response; + if (channel === "webhook") { + const raw = JSON.stringify(body); + response = await buildGithubWebhookRoutes(db, "test-secret").request("/webhooks/github", { + method: "POST", + headers: { + "content-type": "application/json", + "x-github-event": "pull_request", + "x-hub-signature-256": + "sha256=" + createHmac("sha256", "test-secret").update(raw).digest("hex"), + }, + body: raw, + }); + } else { + // The current poller does not include head.repo at all. Its credential + // authenticates the observation, not the PR author's issue declaration. + const pollerHead = { ref: body.pull_request.head.ref, sha: body.pull_request.head.sha }; + response = await buildApiRoutes(db).request("/github-events", { + method: "POST", + headers: { + authorization: `Bearer ${service.token}`, + "content-type": "application/json", + }, + body: JSON.stringify({ + event: "pull_request", + repo, + payload: { ...body, pull_request: { ...body.pull_request, head: pollerHead } }, + }), + }); + } + expect(response.status).toBe(200); + expect(findPrState(db, repo, 7)?.status).toBe(action === "opened" ? "open" : "merged"); + expect(findPrState(db, repo, 7)?.issueRef).toBeNull(); + expect(getOpenPr(db, issue.id)).toBeNull(); + expect(getMergedPr(db, issue.id)).toBeNull(); + expect( + listLiveLinkViews(db, issue.id).every( + (link) => link.role === "references" && !link.provesLanded, + ), + ).toBe(true); + } + expect(claimIssue(db, agent, issue.ref).issue.assigneeId).toBe(agent.id); + }, + ); + + it.each([true, false])( + "publishes against a legacy mixed-case repository binding (explicit repo: %s)", + (explicit) => { + const { db, service, issue } = setup(); + // Legacy rows predate normalized writes; both binding and observation + // identity must still converge without requiring an operator migration. + db.update(githubRepos).set({ fullName: "Acme/Widgets" }).run(); + recordDeliveryEvent(db, service.actor, issue.ref, { + ...published, + repo: explicit ? repo : undefined, + }); + expect(getOpenPr(db, issue.id)?.repo).toBe(repo); + expect(listLiveLinkViews(db, issue.id)[0].role).toBe("delivers"); + }, + ); + + it("preserves authenticated host publication, including a webhook that arrived first", () => { + const { db, issue, agent, service } = setup(); + handleGithubWebhook(db, "pull_request", payload()); + recordDeliveryEvent(db, service.actor, issue.ref, published); + expect(getOpenPr(db, issue.id)?.prNumber).toBe(7); + expect(() => claimIssue(db, agent, issue.ref)).toThrow(/open PR/); + handleGithubWebhook(db, "pull_request", payload("closed", "2030-01-01T00:01:00Z")); + expect(getMergedPr(db, issue.id)?.prNumber).toBe(7); + expect(listLiveLinkViews(db, issue.id)[0].provesLanded).toBe(true); + }); +}); + +describe("revocation survives observations and host publication retries", () => { + it.each(["delivers", "references"] as const)( + "preserves a revoked %s link until an explicit re-declaration", + (role) => { + const { db, human, issue, service } = setup(); + if (role === "delivers") recordDeliveryEvent(db, service.actor, issue.ref, published); + else handleGithubWebhook(db, "pull_request", payload()); + revokePrLink(db, human, issue.ref, { repo, prNumber: 7, reason: "Unrelated work" }); + for (const observation of [ + payload(), + payload("synchronize", "2030-01-01T00:01:00Z"), + payload("closed", "2030-01-01T00:02:00Z"), + ]) { + handleGithubWebhook(db, "pull_request", observation); + expect(listLiveLinkViews(db, issue.id)).toEqual([]); + expect(getOpenPr(db, issue.id)).toBeNull(); + expect(getMergedPr(db, issue.id)).toBeNull(); + } + recordDeliveryEvent(db, service.actor, issue.ref, published); + recordDeliveryEvent(db, service.actor, issue.ref, { + type: "delivered", + repo, + prNumber: 7, + mergeSha: "b".repeat(40), + deploy: { ran: false }, + }); + expect(listLiveLinkViews(db, issue.id)).toEqual([]); + declarePrLink(db, human, issue.ref, { repo, prNumber: 7 }); + expect(getMergedPr(db, issue.id)?.prNumber).toBe(7); + expect(listLiveLinkViews(db, issue.id)[0].provesLanded).toBe(true); + }, + ); +}); diff --git a/tests/services/lease-claim-takeover.test.ts b/tests/services/lease-claim-takeover.test.ts index e7c5076..3a2867e 100644 --- a/tests/services/lease-claim-takeover.test.ts +++ b/tests/services/lease-claim-takeover.test.ts @@ -1,10 +1,12 @@ import { describe, it, expect, beforeEach } from "vitest"; import { openDb, type Db } from "../../src/db/index.js"; +import { claimLeases } from "../../src/db/schema.js"; import { createActor, type Actor } from "../../src/services/actors.js"; import { createProject } from "../../src/services/projects.js"; import { createIssue, updateIssue, claimIssue, getIssue } from "../../src/services/issues.js"; import { validateLease, getActiveLease } from "../../src/services/leases.js"; import { listIssueEvents } from "../../src/services/events.js"; +import { nextTask } from "../../src/services/dependencies.js"; let db: Db, human: Actor, agent: Actor; beforeEach(() => { @@ -45,4 +47,72 @@ describe("claimIssue leases + takeover", () => { // exactly one active lease expect(getActiveLease(db, id)?.tokenHash).toBeTruthy(); }); + + it("starts a preassigned todo issue and records its transition", () => { + updateIssue(db, human, "AIPI-1", { assigneeName: agent.name }); + const claimed = claimIssue(db, agent, "AIPI-1"); + expect(claimed.issue).toMatchObject({ status: "in_progress", assigneeId: agent.id }); + expect(nextTask(db, agent)).toBeNull(); + expect(() => validateLease(db, claimed.issue.id, agent.id, claimed.leaseToken)).not.toThrow(); + expect(listIssueEvents(db, claimed.issue.id).at(-1)).toMatchObject({ + type: "status_changed", + payload: { from: "todo", to: "in_progress" }, + }); + expect(() => + updateIssue(db, agent, "AIPI-1", { status: "in_review" }, { presented: claimed.leaseToken }), + ).not.toThrow(); + }); + + it("takes over review work by applying the permitted transition back to in_progress", () => { + const first = claimIssue(db, agent, "AIPI-1"); + updateIssue(db, agent, "AIPI-1", { status: "in_review" }, { presented: first.leaseToken }); + const next = claimIssue(db, agent, "AIPI-1", { takeover: true }); + expect(next.issue.status).toBe("in_progress"); + expect(() => validateLease(db, first.issue.id, agent.id, first.leaseToken)).toThrow(); + expect(() => validateLease(db, next.issue.id, agent.id, next.leaseToken)).not.toThrow(); + expect(listIssueEvents(db, next.issue.id)).toContainEqual( + expect.objectContaining({ + type: "status_changed", + payload: { from: "in_review", to: "in_progress" }, + }), + ); + }); + + it.each(["backlog", "triage", "done", "canceled"] as const)( + "refuses an agent claim on preassigned %s without minting or recording anything", + (status) => { + updateIssue(db, human, "AIPI-1", { status, assigneeName: agent.name }); + const before = getIssue(db, "AIPI-1"); + const eventsBefore = listIssueEvents(db, before.id); + expect(() => claimIssue(db, agent, "AIPI-1", { takeover: true })).toThrow(); + expect(getIssue(db, "AIPI-1")).toEqual(before); + expect(listIssueEvents(db, before.id)).toEqual(eventsBefore); + expect(db.select().from(claimLeases).all()).toEqual([]); + }, + ); + + it("keeps ordinary human transition permissions on a preassigned issue", () => { + updateIssue(db, human, "AIPI-1", { status: "done", assigneeName: human.name }); + expect(claimIssue(db, human, "AIPI-1").issue.status).toBe("in_progress"); + }); + + it("rolls back lease rotation when a takeover would reopen a done issue", () => { + const first = claimIssue(db, agent, "AIPI-1"); + updateIssue(db, human, "AIPI-1", { status: "done" }); + const leasesBefore = db.select().from(claimLeases).all(); + const eventsBefore = listIssueEvents(db, first.issue.id); + expect(() => claimIssue(db, agent, "AIPI-1", { takeover: true })).toThrow(/only humans reopen/); + expect(getIssue(db, "AIPI-1").status).toBe("done"); + expect(db.select().from(claimLeases).all()).toEqual(leasesBefore); + expect(listIssueEvents(db, first.issue.id)).toEqual(eventsBefore); + expect(() => validateLease(db, first.issue.id, agent.id, first.leaseToken)).not.toThrow(); + }); + + it("does not let a preassigned service actor bypass the issue mutation guard", () => { + const service = createActor(db, { name: "poller", type: "service" }).actor; + updateIssue(db, human, "AIPI-1", { assigneeName: service.name }); + expect(() => claimIssue(db, service, "AIPI-1")).toThrow(/cannot modify issues/); + expect(getIssue(db, "AIPI-1").status).toBe("todo"); + expect(db.select().from(claimLeases).all()).toEqual([]); + }); }); diff --git a/tests/services/lease-expiry.test.ts b/tests/services/lease-expiry.test.ts index 99d5088..a3912e4 100644 --- a/tests/services/lease-expiry.test.ts +++ b/tests/services/lease-expiry.test.ts @@ -1,11 +1,16 @@ -import { describe, it, expect, beforeEach } from "vitest"; +import { assert, describe, it, expect, beforeEach } from "vitest"; import { eq } from "drizzle-orm"; import { openDb, type Db } from "../../src/db/index.js"; import { claimLeases, issues, events } from "../../src/db/schema.js"; import { createActor, type Actor } from "../../src/services/actors.js"; import { createProject } from "../../src/services/projects.js"; import { createIssue, updateIssue, claimIssue, getIssue } from "../../src/services/issues.js"; -import { expireLeases, invalidateLease, getActiveLease } from "../../src/services/leases.js"; +import { + expireLeases, + invalidateLease, + getActiveLease, + validateLease, +} from "../../src/services/leases.js"; import { releaseStaleClaims } from "../../src/services/stale-claims.js"; import { listIssueEvents } from "../../src/services/events.js"; @@ -62,6 +67,56 @@ describe("expireLeases", () => { expect(getIssue(db, "AIPI-1").status).toBe("in_progress"); }); + it("preserves a fresh same-actor claim when the old lease expired before its sweep", () => { + const id = claimThenExpireLease(); + const { leaseToken } = claimIssue(db, agent, "AIPI-1"); + expect(expireLeases(db)).toBe(0); + expect(getIssue(db, "AIPI-1")).toMatchObject({ status: "in_progress", assigneeId: agent.id }); + expect(() => validateLease(db, id, agent.id, leaseToken)).not.toThrow(); + expect(listIssueEvents(db, id).some((e) => e.type === "claim_released")).toBe(false); + }); + + it.each(["same", "different"])( + "retires a legacy expired predecessor without releasing its %s-actor replacement", + (holder) => { + const id = claimThenExpireLease(); + const predecessor = db.select().from(claimLeases).get(); + assert(predecessor); + const replacementActor = + holder === "same" ? agent : createActor(db, { name: "codex/worker", type: "agent" }).actor; + if (holder === "different") { + updateIssue(db, human, "AIPI-1", { assigneeName: replacementActor.name }); + } + const { leaseToken } = claimIssue(db, replacementActor, "AIPI-1"); + // Preserve the pre-fix duplicate-row shape while exercising the sweep's + // own generation check independently of corrected minting. + db.update(claimLeases) + .set({ invalidatedAt: null }) + .where(eq(claimLeases.id, predecessor.id)) + .run(); + + expect(expireLeases(db)).toBe(0); + expect(getIssue(db, "AIPI-1")).toMatchObject({ + status: "in_progress", + assigneeId: replacementActor.id, + }); + expect(() => validateLease(db, id, replacementActor.id, leaseToken)).not.toThrow(); + expect( + db.select().from(claimLeases).where(eq(claimLeases.id, predecessor.id)).get() + ?.invalidatedAt, + ).toEqual(expect.any(Number)); + expect(listIssueEvents(db, id).some((e) => e.type === "claim_released")).toBe(false); + }, + ); + + it("does not release a historically reassigned issue whose new holder has no lease yet", () => { + const id = claimThenExpireLease(); + db.update(issues).set({ assigneeId: human.id }).where(eq(issues.id, id)).run(); + expect(expireLeases(db)).toBe(0); + expect(getIssue(db, "AIPI-1")).toMatchObject({ status: "in_progress", assigneeId: human.id }); + expect(listIssueEvents(db, id).some((e) => e.type === "claim_released")).toBe(false); + }); + it("does not release when the issue moved on before the sweep landed (race)", () => { const id = claimThenExpireLease(); db.update(issues).set({ status: "in_review" }).where(eq(issues.id, id)).run(); diff --git a/tests/services/lease-update-issue.test.ts b/tests/services/lease-update-issue.test.ts index bfa0d6a..39bf621 100644 --- a/tests/services/lease-update-issue.test.ts +++ b/tests/services/lease-update-issue.test.ts @@ -1,9 +1,11 @@ import { describe, it, expect, beforeEach } from "vitest"; +import { and, eq, isNull } from "drizzle-orm"; import { openDb, type Db } from "../../src/db/index.js"; +import { claimLeases } from "../../src/db/schema.js"; import { createActor, type Actor } from "../../src/services/actors.js"; import { createProject } from "../../src/services/projects.js"; -import { createIssue, updateIssue, getIssue } from "../../src/services/issues.js"; -import { getActiveLease } from "../../src/services/leases.js"; +import { createIssue, updateIssue, claimIssue, getIssue } from "../../src/services/issues.js"; +import { getActiveLease, heartbeatLease, validateLease } from "../../src/services/leases.js"; import { listIssueEvents } from "../../src/services/events.js"; let db: Db, human: Actor, agent: Actor; @@ -57,4 +59,51 @@ describe("updateIssue lease enforcement", () => { // human reassigns / edits without any lease token — must not throw expect(() => updateIssue(db, human, "AIPI-1", { priority: "high" })).not.toThrow(); }); + + it.each([ + { toTodo: false, sameActor: false }, + { toTodo: false, sameActor: true }, + { toTodo: true, sameActor: false }, + { toTodo: true, sameActor: true }, + ])("retires an explicitly cleared claim before replacement ($toTodo, $sameActor)", (scenario) => { + const first = claimIssue(db, agent, "AIPI-1"); + updateIssue(db, human, "AIPI-1", { + assigneeName: null, + ...(scenario.toTodo ? { status: "todo" as const } : {}), + }); + expect(getActiveLease(db, first.issue.id)).toBeNull(); + expect(() => heartbeatLease(db, first.issue.id, agent.id, first.leaseToken)).toThrow(); + + const nextActor = scenario.sameActor + ? agent + : createActor(db, { name: "codex/worker", type: "agent" }).actor; + const next = claimIssue(db, nextActor, "AIPI-1"); + expect(() => + updateIssue(db, nextActor, "AIPI-1", { title: "Progress" }, { presented: next.leaseToken }), + ).not.toThrow(); + expect(() => validateLease(db, first.issue.id, agent.id, first.leaseToken)).toThrow(); + expect( + db + .select() + .from(claimLeases) + .where(and(eq(claimLeases.issueId, first.issue.id), isNull(claimLeases.invalidatedAt))) + .all(), + ).toHaveLength(1); + }); + + it("retires the outgoing lease on direct human reassignment", () => { + const first = claimIssue(db, agent, "AIPI-1"); + const nextActor = createActor(db, { name: "codex/worker", type: "agent" }).actor; + updateIssue(db, human, "AIPI-1", { assigneeName: nextActor.name }); + expect(() => validateLease(db, first.issue.id, agent.id, first.leaseToken)).toThrow(); + const next = claimIssue(db, nextActor, "AIPI-1"); + expect(() => heartbeatLease(db, next.issue.id, nextActor.id, next.leaseToken)).not.toThrow(); + }); + + it("retires a claim moved to todo even when the human preserves its assignee", () => { + const first = claimIssue(db, agent, "AIPI-1"); + updateIssue(db, human, "AIPI-1", { status: "todo", assigneeName: agent.name }); + expect(getActiveLease(db, first.issue.id)).toBeNull(); + expect(() => heartbeatLease(db, first.issue.id, agent.id, first.leaseToken)).toThrow(); + }); }); diff --git a/tests/services/leases.test.ts b/tests/services/leases.test.ts index 9b2c382..f1e4985 100644 --- a/tests/services/leases.test.ts +++ b/tests/services/leases.test.ts @@ -1,8 +1,10 @@ -import { describe, it, expect, beforeEach } from "vitest"; +import { assert, describe, it, expect, beforeEach } from "vitest"; +import { eq } from "drizzle-orm"; import { openDb, type Db } from "../../src/db/index.js"; +import { claimLeases, issues } from "../../src/db/schema.js"; import { createActor, type Actor } from "../../src/services/actors.js"; import { createProject } from "../../src/services/projects.js"; -import { createIssue } from "../../src/services/issues.js"; +import { createIssue, updateIssue } from "../../src/services/issues.js"; import { getActiveLease, mintLease, validateLease } from "../../src/services/leases.js"; import { getSetting } from "../../src/services/settings.js"; @@ -13,6 +15,7 @@ beforeEach(() => { agent = createActor(db, { name: "claude/worker", type: "agent" }).actor; createProject(db, human, { key: "AIPI", name: "aipi" }); issueId = createIssue(db, human, { projectKey: "AIPI", title: "t" }).id; + updateIssue(db, human, "AIPI-1", { status: "in_progress", assigneeName: agent.name }); }); describe("leases", () => { @@ -40,4 +43,30 @@ describe("leases", () => { expect(getActiveLease(db, other)).toBeNull(); // past expires_at ⇒ not active expect(() => validateLease(db, other, agent.id, stale)).toThrow(); }); + + it("does not resurrect a legacy predecessor when its replacement expires", () => { + const first = mintLease(db, issueId, agent.id, 3600); + const firstId = getActiveLease(db, issueId)?.id; + assert(firstId !== undefined); + mintLease(db, issueId, agent.id, -10); + // Simulate duplicate outstanding rows written before replacement retired + // predecessors. The older, longer-lived token must not become current. + db.update(claimLeases).set({ invalidatedAt: null }).where(eq(claimLeases.id, firstId)).run(); + expect(getActiveLease(db, issueId)).toBeNull(); + expect(() => validateLease(db, issueId, agent.id, first)).toThrow(); + }); + + it("rejects a legacy token whose actor no longer owns the issue", () => { + const token = mintLease(db, issueId, agent.id, 3600); + // Historical human reassignment did not retire the outgoing lease. + db.update(issues).set({ assigneeId: human.id }).where(eq(issues.id, issueId)).run(); + expect(getActiveLease(db, issueId)).toBeNull(); + expect(() => validateLease(db, issueId, agent.id, token)).toThrow(); + }); + + it("rolls back predecessor retirement if replacement minting fails", () => { + const token = mintLease(db, issueId, agent.id, 3600); + expect(() => mintLease(db, issueId, -1, 3600)).toThrow(); + expect(() => validateLease(db, issueId, agent.id, token)).not.toThrow(); + }); }); diff --git a/tests/services/pr-links.test.ts b/tests/services/pr-links.test.ts index b531882..6f68c10 100644 --- a/tests/services/pr-links.test.ts +++ b/tests/services/pr-links.test.ts @@ -411,7 +411,7 @@ describe("the DoS the previous design died on", () => { expect(links[0].confirmedBy).toBeNull(); }); - it("the agent/ branch path still declares delivers — parity with today", () => { + it("the agent/ branch convention alone only suggests a reference", () => { const { db } = setup(); handleGithubWebhook(db, "pull_request", { action: "opened", @@ -427,10 +427,9 @@ describe("the DoS the previous design died on", () => { }); const links = listLiveLinks(db, 1); expect(links).toHaveLength(1); - expect(links[0].role).toBe("delivers"); - // Confirmed, matching the authority pr_state.issue_ref carries today — but - // by a non-human, so §5a recency binding still applies at the read sites. - expect(links[0].confirmedBy).not.toBeNull(); + expect(links[0].role).toBe("references"); + expect(links[0].confirmedBy).toBeNull(); + expect(getOpenPr(db, 1)).toBeNull(); }); it("ingestion records no timeline event — the link row is the audit", () => { diff --git a/tests/services/pr-observation.test.ts b/tests/services/pr-observation.test.ts index e0d9478..a054470 100644 --- a/tests/services/pr-observation.test.ts +++ b/tests/services/pr-observation.test.ts @@ -295,11 +295,12 @@ describe("the SYD-280 regression fence still holds (SYD-287)", () => { }, }); - it("an agent/ PR behaves exactly as before — one delivers link, one event, one row", () => { - const { db } = setup(); + it("a declared agent/ PR retains its attribution — one delivers link, one event, one row", () => { + const { db, human } = setup(); + declarePrLink(db, human, "SYD-1", { repo: REPO, prNumber: 12 }); const outcome = handleGithubWebhook(db, "pull_request", agentPr("opened")); expect(outcome).toEqual({ handled: true, ref: "SYD-1", type: "gh_pr_opened" }); - expect(findPrState(db, REPO, 12)).toMatchObject({ status: "open", issueRef: "SYD-1" }); + expect(findPrState(db, REPO, 12)).toMatchObject({ status: "open", issueRef: null }); const links = listLiveLinks(db, 1); expect(links).toHaveLength(1); expect(links[0].role).toBe("delivers"); @@ -349,6 +350,7 @@ describe("the SYD-280 regression fence still holds (SYD-287)", () => { it("still queues an agent/ PR — the guard bounds the queue, it does not empty it", () => { const { db, human } = setup(); + declarePrLink(db, human, "SYD-1", { repo: REPO, prNumber: 12 }); handleGithubWebhook(db, "pull_request", { action: "opened", repository: { full_name: REPO }, @@ -365,7 +367,7 @@ describe("the SYD-280 regression fence still holds (SYD-287)", () => { expect(pending[0].pin).toMatchObject({ prNumber: 12, repo: REPO }); }); - it("never writes issueRef from a link — that column stays branch-derived until it is dropped", () => { + it("never writes issueRef from a link — that column retains legacy data only", () => { const { db, human } = setup(); declare(db, human); handleGithubWebhook(db, "pull_request", featPr("opened")); diff --git a/tests/services/pr-state-cutover.test.ts b/tests/services/pr-state-cutover.test.ts index 5234c9e..da579b0 100644 --- a/tests/services/pr-state-cutover.test.ts +++ b/tests/services/pr-state-cutover.test.ts @@ -1,3 +1,4 @@ +import { declarePrLink } from "../../src/services/pr-links.js"; // SYD-207 cutover invariants (spec: docs/2026-07-12-sync-simplification- // assessment.md Step 6): the backfill rides POST /api/github-events → // handleGithubWebhook, so these tests drive that exact ingestion and assert @@ -56,7 +57,8 @@ function prPayload( describe("search-vs-claim-gate agreement (SYD-207)", () => { it("the ?openPr= filter and the claim gate answer from the same oracle", () => { - const { db, agent } = setup(); + const { db, human, agent } = setup(); + declarePrLink(db, human, "SYD-1", { repo: REPO, prNumber: 41 }); handleGithubWebhook( db, "pull_request", diff --git a/tests/services/pr-state-ordering.test.ts b/tests/services/pr-state-ordering.test.ts new file mode 100644 index 0000000..2c7718d --- /dev/null +++ b/tests/services/pr-state-ordering.test.ts @@ -0,0 +1,114 @@ +import { describe, expect, it } from "vitest"; +import { openDb } from "../../src/db/index.js"; +import { createActor } from "../../src/services/actors.js"; +import { createProject } from "../../src/services/projects.js"; +import { createIssue } from "../../src/services/issues.js"; +import { addGithubRepo } from "../../src/services/github-repos.js"; +import { declarePrLink } from "../../src/services/pr-links.js"; +import { listIssueEvents } from "../../src/services/events.js"; +import { findPrState, upsertPrState, type PrObservation } from "../../src/services/pr-state.js"; +import { getOpenPr } from "../../src/services/pr-status.js"; +import { handleGithubWebhook } from "../../src/services/github-webhook.js"; + +const REPO = "acme/widgets"; +const T1 = "2026-07-12T10:00:00Z"; +const T2 = "2026-07-12T11:00:00Z"; +const T3 = "2026-07-12T12:00:00Z"; + +function setup() { + const db = openDb(":memory:"); + const human = createActor(db, { name: "sean", type: "human" }).actor; + createProject(db, human, { key: "SYD", name: "Switchyard" }); + const issue = createIssue(db, human, { projectKey: "SYD", title: "Ship it" }); + addGithubRepo(db, human, { fullName: REPO, projectKey: "SYD" }); + declarePrLink(db, human, "SYD-1", { repo: REPO, prNumber: 12 }); + const observe = (o: Partial) => + upsertPrState(db, human, { + repo: REPO, + prNumber: 12, + status: "open", + branch: "feat/widget", + ghUpdatedAt: T1, + ...o, + }); + return { db, issue, observe }; +} + +describe("PR close/reopen ordering", () => { + it("returns a diagnostic to ingestion clients when a close lacks freshness evidence", () => { + const { db, observe } = setup(); + observe({ ghUpdatedAt: T2 }); + const outcome = handleGithubWebhook( + db, + "pull_request", + { + action: "closed", + pull_request: { number: 12, merged: false }, + }, + REPO, + ); + expect(outcome).toMatchObject({ + handled: true, + duplicate: true, + reason: expect.stringContaining("close is missing"), + }); + expect(findPrState(db, REPO, 12)?.status).toBe("open"); + }); + + it("preserves the open claim gate after an old close is replayed", () => { + const { db, issue, observe } = setup(); + observe({ status: "closed" }); + expect(observe({ reopened: true, ghUpdatedAt: T2 }).transition).toBe("reopened"); + expect(observe({ status: "closed" })).toMatchObject({ + applied: false, + reason: expect.stringContaining("older"), + }); + expect(findPrState(db, REPO, 12)?.status).toBe("open"); + expect(getOpenPr(db, issue.id)?.prNumber).toBe(12); + + expect(observe({ reopened: true, ghUpdatedAt: T2 }).transition).toBeNull(); + expect(listIssueEvents(db, issue.id).filter((e) => e.type === "gh_pr_reopened")).toHaveLength( + 1, + ); + expect(observe({ status: "closed", ghUpdatedAt: T3 }).transition).toBe("closed"); + expect(getOpenPr(db, issue.id)).toBeNull(); + }); + + it.each([null, "not-a-time"])( + "rejects a close without valid freshness evidence (%s)", + (ghUpdatedAt) => { + const { db, observe } = setup(); + observe({ status: "closed" }); + observe({ reopened: true, ghUpdatedAt: T2 }); + expect(observe({ status: "closed", ghUpdatedAt })).toMatchObject({ + applied: false, + reason: expect.stringContaining("missing"), + }); + expect(findPrState(db, REPO, 12)?.status).toBe("open"); + }, + ); + + it("accepts final merge evidence even if it arrives behind a fresher open observation", () => { + const { db, observe } = setup(); + observe({ status: "closed" }); + observe({ reopened: true, ghUpdatedAt: T2 }); + observe({ ghUpdatedAt: T3 }); + expect(observe({ status: "merged", ghUpdatedAt: T1 }).transition).toBe("merged"); + expect(findPrState(db, REPO, 12)).toMatchObject({ + status: "merged", + ghUpdatedAt: Date.parse(T3) / 1000, + }); + expect(observe({ reopened: true, ghUpdatedAt: "2026-07-13T00:00:00Z" })).toMatchObject({ + applied: false, + reason: "merged is final", + }); + }); + + it("accepts a close at the current timestamp and a close on an untimestamped open row", () => { + const { observe } = setup(); + observe({ ghUpdatedAt: null }); + expect(observe({ status: "closed" }).transition).toBe("closed"); + observe({ reopened: true, ghUpdatedAt: T2 }); + expect(observe({ status: "closed", ghUpdatedAt: T2 }).transition).toBe("closed"); + }); +}); diff --git a/tests/services/pr-state.test.ts b/tests/services/pr-state.test.ts index 36bd853..e4a263a 100644 --- a/tests/services/pr-state.test.ts +++ b/tests/services/pr-state.test.ts @@ -8,6 +8,8 @@ import { describe, it, expect } from "vitest"; import { openDb, type Db } from "../../src/db/index.js"; import { createActor } from "../../src/services/actors.js"; import { createProject } from "../../src/services/projects.js"; +import { declarePrLink } from "../../src/services/pr-links.js"; +import { getOpenPr } from "../../src/services/pr-status.js"; import { createIssue } from "../../src/services/issues.js"; import { getActivity } from "../../src/services/comments.js"; import { addGithubRepo } from "../../src/services/github-repos.js"; @@ -24,13 +26,16 @@ const T1 = "2026-07-12T10:00:00Z"; const T2 = "2026-07-12T11:00:00Z"; const T3 = "2026-07-12T12:00:00Z"; -function setup(opts: { bindRepo?: boolean } = {}) { +function setup(opts: { bindRepo?: boolean; declare?: boolean } = {}) { const db = openDb(":memory:"); const human = createActor(db, { name: "sean", type: "human" }).actor; const github = createActor(db, { name: "github", type: "agent" }).actor; createProject(db, human, { key: "SYD", name: "Switchyard" }); createIssue(db, human, { projectKey: "SYD", title: "Ship v1" }); if (opts.bindRepo !== false) addGithubRepo(db, human, { fullName: REPO, projectKey: "SYD" }); + if (opts.bindRepo !== false && opts.declare !== false) { + declarePrLink(db, human, "SYD-1", { repo: REPO, prNumber: 12 }); + } return { db, human, github }; } @@ -48,7 +53,7 @@ function obs(o: Partial = {}): PrObservation { } describe("upsertPrState / insert + attribution", () => { - it("inserts an open row from a first observation, attributed via branch + repo binding, and co-writes gh_pr_opened", () => { + it("inserts an open row from a first observation, attributed via an explicit human declaration, and co-writes gh_pr_opened", () => { const { db, github } = setup(); const outcome = upsertPrState(db, github, obs()); expect(outcome).toMatchObject({ applied: true, transition: "opened" }); @@ -59,7 +64,7 @@ describe("upsertPrState / insert + attribution", () => { prNumber: 12, status: "open", branch: "agent/SYD-1", - issueRef: "SYD-1", + issueRef: null, headSha: "a".repeat(40), url: "https://github.com/acme/widgets/pull/12", }); @@ -79,6 +84,7 @@ describe("upsertPrState / insert + attribution", () => { it("normalizes repo casing on write and read: link lowercase, observe canonical case, binding + pr_state both resolve (SYD-212)", () => { const { db, human, github } = setup({ bindRepo: false }); addGithubRepo(db, human, { fullName: REPO.toLowerCase(), projectKey: "SYD" }); + declarePrLink(db, human, "SYD-1", { repo: REPO, prNumber: 12 }); const canonicalRepo = "Acme/Widgets"; const outcome = upsertPrState(db, github, obs({ repo: canonicalRepo })); @@ -86,7 +92,8 @@ describe("upsertPrState / insert + attribution", () => { // Attribution succeeded (bound repo matched despite differing casing)... const rowByLower = findPrState(db, REPO.toLowerCase(), 12)!; - expect(rowByLower.issueRef).toBe("SYD-1"); + expect(getOpenPr(db, 1)?.prNumber).toBe(12); + expect(rowByLower.issueRef).toBeNull(); // ...and the stored row itself converged on one (lowercase) casing rather // than splitting into a second row under the canonical-case key. expect(rowByLower.repo).toBe("acme/widgets"); @@ -96,7 +103,7 @@ describe("upsertPrState / insert + attribution", () => { }); it("refuses attribution for an agent/ PR in a repo not bound to that ref's project (cross-repo), writing a display-only row and no event", () => { - const { db, human, github } = setup(); + const { db, human, github } = setup({ declare: false }); createProject(db, human, { key: "OTH", name: "Other" }); addGithubRepo(db, human, { fullName: "acme/other", projectKey: "OTH" }); @@ -109,7 +116,7 @@ describe("upsertPrState / insert + attribution", () => { }); it("never attributes from a non-agent branch (free-text scanning stays display-only)", () => { - const { db, github } = setup(); + const { db, github } = setup({ declare: false }); upsertPrState(db, github, obs({ branch: "feat/manual-work" })); expect(findPrState(db, REPO, 12)!.issueRef).toBeNull(); }); @@ -118,7 +125,8 @@ describe("upsertPrState / insert + attribution", () => { const { db, github } = setup(); upsertPrState(db, github, obs()); upsertPrState(db, github, obs({ branch: null, ghUpdatedAt: T2 })); - expect(findPrState(db, REPO, 12)!.issueRef).toBe("SYD-1"); + expect(getOpenPr(db, 1)?.prNumber).toBe(12); + expect(findPrState(db, REPO, 12)!.issueRef).toBeNull(); }); it("heals a PR first observed already merged (never-saw-open), co-writing gh_pr_merged", () => { diff --git a/tests/services/pr-status.test.ts b/tests/services/pr-status.test.ts index 297ae3e..d2ae41e 100644 --- a/tests/services/pr-status.test.ts +++ b/tests/services/pr-status.test.ts @@ -6,6 +6,8 @@ import { createIssue, getIssue } from "../../src/services/issues.js"; import { addGithubRepo } from "../../src/services/github-repos.js"; import { recordDeliveryEvent } from "../../src/services/delivery-events.js"; import { recordEvent } from "../../src/services/events.js"; +import { declarePrLink, listLiveLinks } from "../../src/services/pr-links.js"; +import type { Actor } from "../../src/services/actors.js"; import { upsertPrState } from "../../src/services/pr-state.js"; import { getOpenPr, @@ -28,13 +30,19 @@ function setup() { /** An attributed observation for agent/ — the shape every pr_state * writer (webhook, poller, publish, delivery, backfill) converges on. */ -function observe( +function declaredObservation( + db: ReturnType, + human: Actor, ref: string, prNumber: number, status: "open" | "merged" | "closed", ghUpdatedAt: string, extra: { reopened?: boolean } = {}, ) { + const issue = getIssue(db, ref); + if (!listLiveLinks(db, issue.id).some((l) => l.repo === REPO && l.prNumber === prNumber)) { + declarePrLink(db, human, ref, { repo: REPO, prNumber }); + } return { repo: REPO, prNumber, @@ -54,7 +62,11 @@ describe("getOpenPr (pr_state-derived, SYD-207)", () => { it("flags an issue with an open attributed row", () => { const { db, human } = setup(); - upsertPrState(db, human, observe("SYD-1", 41, "open", "2026-07-13T10:00:00Z")); + upsertPrState( + db, + human, + declaredObservation(db, human, "SYD-1", 41, "open", "2026-07-13T10:00:00Z"), + ); expect(getOpenPr(db, getIssue(db, "SYD-1").id)).toEqual({ prNumber: 41, url: `https://github.com/${REPO}/pull/41`, @@ -66,7 +78,7 @@ describe("getOpenPr (pr_state-derived, SYD-207)", () => { it("carries repo and headSha (SYD-208)", () => { const { db, human } = setup(); upsertPrState(db, human, { - ...observe("SYD-1", 41, "open", "2026-07-13T10:00:00Z"), + ...declaredObservation(db, human, "SYD-1", 41, "open", "2026-07-13T10:00:00Z"), headSha: "abc123", }); expect(getOpenPr(db, getIssue(db, "SYD-1").id)).toEqual({ @@ -80,21 +92,39 @@ describe("getOpenPr (pr_state-derived, SYD-207)", () => { it("clears when the row goes merged or closed", () => { const { db, human } = setup(); const issueId = getIssue(db, "SYD-1").id; - upsertPrState(db, human, observe("SYD-1", 41, "open", "2026-07-13T10:00:00Z")); - upsertPrState(db, human, observe("SYD-1", 41, "merged", "2026-07-13T11:00:00Z")); + upsertPrState( + db, + human, + declaredObservation(db, human, "SYD-1", 41, "open", "2026-07-13T10:00:00Z"), + ); + upsertPrState( + db, + human, + declaredObservation(db, human, "SYD-1", 41, "merged", "2026-07-13T11:00:00Z"), + ); expect(getOpenPr(db, issueId)).toBeNull(); }); it("flags again after a genuine reopen", () => { const { db, human } = setup(); const issueId = getIssue(db, "SYD-1").id; - upsertPrState(db, human, observe("SYD-1", 41, "open", "2026-07-13T10:00:00Z")); - upsertPrState(db, human, observe("SYD-1", 41, "closed", "2026-07-13T11:00:00Z")); + upsertPrState( + db, + human, + declaredObservation(db, human, "SYD-1", 41, "open", "2026-07-13T10:00:00Z"), + ); + upsertPrState( + db, + human, + declaredObservation(db, human, "SYD-1", 41, "closed", "2026-07-13T11:00:00Z"), + ); expect(getOpenPr(db, issueId)).toBeNull(); upsertPrState( db, human, - observe("SYD-1", 41, "open", "2026-07-13T12:00:00Z", { reopened: true }), + declaredObservation(db, human, "SYD-1", 41, "open", "2026-07-13T12:00:00Z", { + reopened: true, + }), ); expect(getOpenPr(db, issueId)?.prNumber).toBe(41); }); @@ -131,9 +161,21 @@ describe("getOpenPr (pr_state-derived, SYD-207)", () => { it("a belated close for an old PR can't hide a newer still-open PR (SYD-125 shape)", () => { const { db, human } = setup(); const issueId = getIssue(db, "SYD-1").id; - upsertPrState(db, human, observe("SYD-1", 1, "open", "2026-07-13T09:00:00Z")); - upsertPrState(db, human, observe("SYD-1", 2, "open", "2026-07-13T10:00:00Z")); - upsertPrState(db, human, observe("SYD-1", 1, "closed", "2026-07-13T11:00:00Z")); + upsertPrState( + db, + human, + declaredObservation(db, human, "SYD-1", 1, "open", "2026-07-13T09:00:00Z"), + ); + upsertPrState( + db, + human, + declaredObservation(db, human, "SYD-1", 2, "open", "2026-07-13T10:00:00Z"), + ); + upsertPrState( + db, + human, + declaredObservation(db, human, "SYD-1", 1, "closed", "2026-07-13T11:00:00Z"), + ); expect(getOpenPr(db, issueId)).toEqual({ prNumber: 2, url: `https://github.com/${REPO}/pull/2`, @@ -168,7 +210,11 @@ describe("listOpenPrByIssueId (pr_state-derived, SYD-207)", () => { it("only includes issues with an open attributed row", () => { const { db, human } = setup(); createIssue(db, human, { projectKey: "SYD", title: "Also shipping" }); // SYD-2 - upsertPrState(db, human, observe("SYD-1", 41, "open", "2026-07-13T10:00:00Z")); + upsertPrState( + db, + human, + declaredObservation(db, human, "SYD-1", 41, "open", "2026-07-13T10:00:00Z"), + ); const open = getIssue(db, "SYD-1"); const clean = getIssue(db, "SYD-2"); @@ -184,8 +230,16 @@ describe("listOpenPrByIssueId (pr_state-derived, SYD-207)", () => { it("keeps the newest PR when an issue somehow has two open rows", () => { const { db, human } = setup(); - upsertPrState(db, human, observe("SYD-1", 41, "open", "2026-07-13T10:00:00Z")); - upsertPrState(db, human, observe("SYD-1", 55, "open", "2026-07-13T09:00:00Z")); + upsertPrState( + db, + human, + declaredObservation(db, human, "SYD-1", 41, "open", "2026-07-13T10:00:00Z"), + ); + upsertPrState( + db, + human, + declaredObservation(db, human, "SYD-1", 55, "open", "2026-07-13T09:00:00Z"), + ); expect(listOpenPrByIssueId(db).get(getIssue(db, "SYD-1").id)?.prNumber).toBe(55); }); }); @@ -199,9 +253,13 @@ describe("getMergedPr (pr_state-derived, SYD-207)", () => { it("returns prNumber + the co-written transition event id", () => { const { db, human } = setup(); const issueId = getIssue(db, "SYD-1").id; - upsertPrState(db, human, observe("SYD-1", 41, "open", "2026-07-13T10:00:00Z")); + upsertPrState( + db, + human, + declaredObservation(db, human, "SYD-1", 41, "open", "2026-07-13T10:00:00Z"), + ); upsertPrState(db, human, { - ...observe("SYD-1", 41, "merged", "2026-07-13T11:00:00Z"), + ...declaredObservation(db, human, "SYD-1", 41, "merged", "2026-07-13T11:00:00Z"), mergeSha: "abc123", }); const merged = getMergedPr(db, issueId); @@ -212,13 +270,22 @@ describe("getMergedPr (pr_state-derived, SYD-207)", () => { it("returns the most recently merged PR when several exist", () => { const { db, human } = setup(); const issueId = getIssue(db, "SYD-1").id; - upsertPrState(db, human, observe("SYD-1", 41, "merged", "2026-07-13T10:00:00Z")); - upsertPrState(db, human, observe("SYD-1", 42, "merged", "2026-07-13T12:00:00Z")); + upsertPrState( + db, + human, + declaredObservation(db, human, "SYD-1", 41, "merged", "2026-07-13T10:00:00Z"), + ); + upsertPrState( + db, + human, + declaredObservation(db, human, "SYD-1", 42, "merged", "2026-07-13T12:00:00Z"), + ); expect(getMergedPr(db, issueId)?.prNumber).toBe(42); }); it("the delivery worker's merge (recordDeliveryEvent delivered) is visible", () => { const { db, human } = setup(); + declarePrLink(db, human, "SYD-1", { repo: REPO, prNumber: 41 }); const issueId = getIssue(db, "SYD-1").id; recordDeliveryEvent(db, human, "SYD-1", { type: "delivered", @@ -241,15 +308,15 @@ describe("deliveryPinFor (SYD-208)", () => { const { db, human } = setup(); const issueId = getIssue(db, "SYD-1").id; upsertPrState(db, human, { - ...observe("SYD-1", 40, "closed", "2026-07-13T09:00:00Z"), + ...declaredObservation(db, human, "SYD-1", 40, "closed", "2026-07-13T09:00:00Z"), headSha: "sha-closed", }); upsertPrState(db, human, { - ...observe("SYD-1", 41, "merged", "2026-07-13T10:00:00Z"), + ...declaredObservation(db, human, "SYD-1", 41, "merged", "2026-07-13T10:00:00Z"), headSha: "sha-merged", }); upsertPrState(db, human, { - ...observe("SYD-1", 42, "open", "2026-07-13T08:00:00Z"), + ...declaredObservation(db, human, "SYD-1", 42, "open", "2026-07-13T08:00:00Z"), headSha: "sha-open", }); expect(deliveryPinFor(db, issueId)).toEqual({ @@ -264,11 +331,11 @@ describe("deliveryPinFor (SYD-208)", () => { const { db, human } = setup(); const issueId = getIssue(db, "SYD-1").id; upsertPrState(db, human, { - ...observe("SYD-1", 40, "closed", "2026-07-13T09:00:00Z"), + ...declaredObservation(db, human, "SYD-1", 40, "closed", "2026-07-13T09:00:00Z"), headSha: "sha-closed", }); upsertPrState(db, human, { - ...observe("SYD-1", 41, "merged", "2026-07-13T10:00:00Z"), + ...declaredObservation(db, human, "SYD-1", 41, "merged", "2026-07-13T10:00:00Z"), headSha: "sha-merged", }); expect(deliveryPinFor(db, issueId)).toEqual({ @@ -283,7 +350,7 @@ describe("deliveryPinFor (SYD-208)", () => { const { db, human } = setup(); const issueId = getIssue(db, "SYD-1").id; upsertPrState(db, human, { - ...observe("SYD-1", 40, "closed", "2026-07-13T09:00:00Z"), + ...declaredObservation(db, human, "SYD-1", 40, "closed", "2026-07-13T09:00:00Z"), headSha: "sha-closed", }); expect(deliveryPinFor(db, issueId)).toEqual({ diff --git a/tests/services/service-actor.test.ts b/tests/services/service-actor.test.ts index f9dfebf..3088e06 100644 --- a/tests/services/service-actor.test.ts +++ b/tests/services/service-actor.test.ts @@ -8,7 +8,11 @@ import { } from "../../src/services/actors.js"; import { createProject } from "../../src/services/projects.js"; import { createIssue, updateIssue } from "../../src/services/issues.js"; -import { addDependency, removeDependency } from "../../src/services/dependencies.js"; +import { + addDependency, + removeDependency, + listDependencies, +} from "../../src/services/dependencies.js"; import { addComment } from "../../src/services/comments.js"; import { recordDeliveryEvent } from "../../src/services/delivery-events.js"; import { @@ -146,6 +150,13 @@ describe("service actor — DENIED all issue create/modify (fail-closed)", () => }); describe("service actor — DENIED (config / dependencies / tokens)", () => { + it("cannot add a dependency or emit a blocking event", () => { + const before = listIssueEvents(db, getIssue(db, "AIPI-2").id).length; + expect(() => addDependency(db, service, "AIPI-1", "AIPI-2")).toThrow(/cannot add dependencies/); + expect(listDependencies(db, "AIPI-2").blockedBy).toEqual([]); + expect(listIssueEvents(db, getIssue(db, "AIPI-2").id)).toHaveLength(before); + }); + it("cannot remove a dependency", () => { addDependency(db, human, "AIPI-1", "AIPI-2"); expect(() => removeDependency(db, service, "AIPI-1", "AIPI-2")).toThrowError(/only humans/i); diff --git a/tests/services/webhook-dispatcher-concurrency.test.ts b/tests/services/webhook-dispatcher-concurrency.test.ts new file mode 100644 index 0000000..a0b3efd --- /dev/null +++ b/tests/services/webhook-dispatcher-concurrency.test.ts @@ -0,0 +1,107 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { eq } from "drizzle-orm"; +import { openDb } from "../../src/db/index.js"; +import { webhookCursor } from "../../src/db/schema.js"; +import { createActor } from "../../src/services/actors.js"; +import { createProject } from "../../src/services/projects.js"; +import { createIssue, updateIssue } from "../../src/services/issues.js"; +import { addWebhook } from "../../src/services/webhooks.js"; +import { recordProgressNote } from "../../src/services/agent-sessions.js"; +import * as leases from "../../src/services/leases.js"; +import { dispatchPending, startWebhookDispatcher } from "../../src/services/webhook-dispatcher.js"; + +function setup() { + const db = openDb(":memory:"); + const human = createActor(db, { name: "sean", type: "human" }).actor; + createProject(db, human, { key: "SYD", name: "Switchyard" }); + addWebhook(db, human, { url: "https://receiver.test/hook" }); + createIssue(db, human, { projectKey: "SYD", title: "Ship it" }); + return { db, human }; +} + +function deferredResponse() { + let resolve!: (response: Response) => void; + const promise = new Promise((r) => { + resolve = r; + }); + return { promise, resolve }; +} + +afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + vi.unstubAllGlobals(); +}); + +describe("webhook dispatcher concurrency", () => { + it("coalesces overlapping batches so each event fans out once", async () => { + const { db, human } = setup(); + updateIssue(db, human, "SYD-1", { status: "todo" }); + const firstResponse = deferredResponse(); + const fetchFn = vi + .fn() + .mockImplementationOnce(() => firstResponse.promise) + .mockResolvedValue(new Response(null, { status: 200 })); + + const first = dispatchPending(db, fetchFn); + const second = dispatchPending(db, fetchFn); + expect(second).toBe(first); + expect(fetchFn).toHaveBeenCalledTimes(1); + firstResponse.resolve(new Response(null, { status: 200 })); + expect(await first).toBe(2); + expect(await second).toBe(2); + expect(fetchFn.mock.calls.map(([, init]) => JSON.parse(String(init?.body)).event)).toEqual([ + "created", + "status_changed", + ]); + expect(await dispatchPending(db, fetchFn)).toBe(0); + expect(fetchFn).toHaveBeenCalledTimes(2); + }); + + it("never regresses the cursor, including while skipping suppressed events", async () => { + const { db } = setup(); + const agent = createActor(db, { name: "worker", type: "agent" }).actor; + recordProgressNote(db, agent, "SYD-1", "compiling"); + const response = deferredResponse(); + const fetchFn = vi.fn().mockImplementation(() => response.promise); + const pending = dispatchPending(db, fetchFn); + db.update(webhookCursor).set({ lastEventId: 100 }).where(eq(webhookCursor.id, 1)).run(); + response.resolve(new Response(null, { status: 200 })); + await pending; + + expect(db.select().from(webhookCursor).get()?.lastEventId).toBe(100); + expect(fetchFn).toHaveBeenCalledTimes(1); + }); + + it("releases the active batch after a database failure so a later tick retries", async () => { + const { db } = setup(); + const fetchFn = vi.fn().mockResolvedValue(new Response(null, { status: 200 })); + vi.spyOn(db, "select").mockImplementationOnce(() => { + throw new Error("database unavailable"); + }); + await expect(dispatchPending(db, fetchFn)).rejects.toThrow("database unavailable"); + expect(await dispatchPending(db, fetchFn)).toBe(1); + }); + + it("keeps lease housekeeping running on every tick while a receiver is slow", async () => { + const { db } = setup(); + vi.useFakeTimers(); + const sweep = vi.spyOn(leases, "expireLeases").mockReturnValue(0); + const response = deferredResponse(); + const fetchFn = vi.fn().mockImplementation(() => response.promise); + vi.stubGlobal("fetch", fetchFn); + const stop = startWebhookDispatcher(db, 2000); + try { + await vi.advanceTimersByTimeAsync(6000); + expect(sweep).toHaveBeenCalledTimes(3); + expect(fetchFn).toHaveBeenCalledTimes(1); + const pending = dispatchPending(db, fetchFn); + stop(); + response.resolve(new Response(null, { status: 200 })); + await pending; + } finally { + stop(); + response.resolve(new Response(null, { status: 200 })); + } + }); +}); diff --git a/ui/src/Composer.tsx b/ui/src/Composer.tsx index 30a9460..f9aa7f8 100644 --- a/ui/src/Composer.tsx +++ b/ui/src/Composer.tsx @@ -11,12 +11,14 @@ export function Composer({ placeholder, paste, children, + disabled = false, }: { value: string; onChange: (value: string) => void; placeholder: string; paste: ReturnType | ReturnType; children?: ReactNode; + disabled?: boolean; }) { const { onPaste, uploading, uploadError, setUploadError, textareaRef } = paste; return ( @@ -28,6 +30,7 @@ export function Composer({ )}