diff --git a/docs/architecture/progress-sync-contract.md b/docs/architecture/progress-sync-contract.md index d852d705d..a71420dda 100644 --- a/docs/architecture/progress-sync-contract.md +++ b/docs/architecture/progress-sync-contract.md @@ -59,6 +59,14 @@ The local realtime path is file-backed by default: - Linear remains the external status surface; the repo does not require hosted telemetry to be release-ready. +GitHub sync refreshes source metadata without taking over locally managed work. +Changing an imported item's status, owner, session, priority, or repository +association establishes local ownership; subsequent refreshes and source +disappearance preserve it. An explicit sync `--session` also establishes local +ownership. Untouched imports continue following source readiness and closure. +Legacy imports without a recorded source-state snapshot are preserved +conservatively. Use the work-items CLI to reconcile their status explicitly. + Hosted telemetry such as PostHog can be added later, but it must consume the same event model rather than becoming a second source of truth. diff --git a/scripts/work-items.js b/scripts/work-items.js index 8a130651c..ac6aa5d6e 100644 --- a/scripts/work-items.js +++ b/scripts/work-items.js @@ -210,6 +210,7 @@ function buildGithubPrWorkItem(repo, pr, options = {}) { mergeStateStatus: pr.mergeStateStatus || null, isDraft: Boolean(pr.isDraft), headRefName: pr.headRefName || null, + sourceAuthor: githubAuthorLogin(pr), sourceUpdatedAt: pr.updatedAt || null, syncedBy: 'ecc-work-items-sync-github' } @@ -232,12 +233,45 @@ function buildGithubIssueWorkItem(repo, issue, options = {}) { repo, type: 'issue', labels: Array.isArray(issue.labels) ? issue.labels.map(label => label.name || label).filter(Boolean) : [], + sourceAuthor: githubAuthorLogin(issue), sourceUpdatedAt: issue.updatedAt || null, syncedBy: 'ecc-work-items-sync-github' } }; } +function githubWorkflowState(item) { + return Object.fromEntries(['status', 'owner', 'sessionId', 'priority', 'repoRoot'] + .map(key => [key, item[key] ?? null])); +} + +function isLocallyManagedGithubItem(item) { + const metadata = item.metadata || {}; + const sync = metadata.githubSync; + // Legacy imports have no reliable record of their original owner/status. + if (!sync || !sync.state || sync.managedLocally || metadata.assigneeKind) return true; + return Object.entries(githubWorkflowState(item)).some(([key, value]) => value !== sync.state[key]); +} + +function refreshGithubWorkItem(store, payload, syncedAt) { + const existing = store.getWorkItemById(payload.id); + const preserveExisting = existing && isLocallyManagedGithubItem(existing); + const managedLocally = Boolean(preserveExisting || payload.sessionId); + const state = githubWorkflowState(preserveExisting ? existing : payload); + const { sourceClosedAt: _sourceClosedAt, ...metadata } = (existing && existing.metadata) || {}; + return store.upsertWorkItem({ + ...payload, + ...state, + metadata: { + ...metadata, + ...payload.metadata, + githubSync: { state, managedLocally } + }, + createdAt: existing ? existing.createdAt : undefined, + updatedAt: syncedAt + }); +} + function closeStaleGithubItems(store, repo, activeIds, options = {}) { const payload = store.listWorkItems({ limit: options.limit || 10000 }); const closed = []; @@ -248,6 +282,15 @@ function closeStaleGithubItems(store, repo, activeIds, options = {}) { if (item.metadata.repo !== repo || activeIds.has(item.id)) { continue; } + if (isLocallyManagedGithubItem(item)) { + if (!item.metadata.githubSync || !item.metadata.githubSync.managedLocally) { + store.upsertWorkItem({ + ...item, + metadata: { ...item.metadata, githubSync: { state: githubWorkflowState(item), managedLocally: true } } + }); + } + continue; + } if (item.status === 'closed' || item.status === 'done') { continue; } @@ -258,6 +301,7 @@ function closeStaleGithubItems(store, repo, activeIds, options = {}) { updatedAt: new Date().toISOString(), metadata: { ...item.metadata, + githubSync: { state: { ...githubWorkflowState(item), status: 'closed' }, managedLocally: false }, sourceClosedAt: new Date().toISOString() } }) @@ -276,42 +320,33 @@ function syncGithubWorkItems(store, options) { const prs = runGhJson(['pr', 'list', '--repo', repo, '--state', 'open', '--limit', String(limit), '--json', 'number,title,author,url,updatedAt,mergeStateStatus,isDraft,headRefName']); const issues = runGhJson(['issue', 'list', '--repo', repo, '--state', 'open', '--limit', String(limit), '--json', 'number,title,author,url,updatedAt,labels']); - const syncedAt = new Date().toISOString(); - const activeIds = new Set(); - const items = []; - for (const pr of prs) { - const payload = buildGithubPrWorkItem(repo, pr, options); - activeIds.add(payload.id); - items.push( - store.upsertWorkItem({ - ...payload, - createdAt: undefined, - updatedAt: syncedAt - }) - ); - } - for (const issue of issues) { - const payload = buildGithubIssueWorkItem(repo, issue, options); - activeIds.add(payload.id); - items.push( - store.upsertWorkItem({ - ...payload, - createdAt: undefined, - updatedAt: syncedAt - }) - ); - } + // Fetch before taking the store lock, then reconcile against one current snapshot. + return store._database.transaction(() => { + const syncedAt = new Date().toISOString(); + const activeIds = new Set(); + const items = []; + for (const pr of prs) { + const payload = buildGithubPrWorkItem(repo, pr, options); + activeIds.add(payload.id); + items.push(refreshGithubWorkItem(store, payload, syncedAt)); + } + for (const issue of issues) { + const payload = buildGithubIssueWorkItem(repo, issue, options); + activeIds.add(payload.id); + items.push(refreshGithubWorkItem(store, payload, syncedAt)); + } - const closedItems = closeStaleGithubItems(store, repo, activeIds, { limit: Math.max(limit * 4, 1000) }); - return { - repo, - syncedAt, - prCount: prs.length, - issueCount: issues.length, - closedCount: closedItems.length, - items, - closedItems - }; + const closedItems = closeStaleGithubItems(store, repo, activeIds, { limit: Math.max(limit * 4, 1000) }); + return { + repo, + syncedAt, + prCount: prs.length, + issueCount: issues.length, + closedCount: closedItems.length, + items, + closedItems + }; + })(); } function buildUpsertPayload(options, existing = null) { diff --git a/tests/scripts/work-items-github.test.js b/tests/scripts/work-items-github.test.js new file mode 100644 index 000000000..b8ac7f04b --- /dev/null +++ b/tests/scripts/work-items-github.test.js @@ -0,0 +1,183 @@ +'use strict'; + +const assert = require('assert'); +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { createStateStore } = require('../../scripts/lib/state-store'); +const { syncGithubWorkItems } = require('../../scripts/work-items'); + +let passed = 0; +let failed = 0; + +async function test(name, fn) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'ecc-github-sync-')); + const previousShim = process.env.ECC_GH_SHIM; + let store; + try { + const fixture = path.join(dir, 'source.json'); + const shim = path.join(dir, 'gh.js'); + fs.writeFileSync(shim, `const data = require(${JSON.stringify(fixture)}); process.stdout.write(JSON.stringify(data[process.argv[2]]));`); + process.env.ECC_GH_SHIM = shim; + store = await createStateStore({ dbPath: path.join(dir, 'state.db') }); + store.upsertSession({ id: 'session-1', adapterId: 'test', harness: 'test', state: 'active', startedAt: '2026-10-28T00:00:00Z' }); + const source = { + pr: [{ number: 3, title: 'Original PR', author: { login: 'author' }, isDraft: false, mergeStateStatus: 'CLEAN' }], + issue: [{ number: 8, title: 'Original issue', author: { login: 'author' }, labels: [] }] + }; + const sync = (options = {}) => { + fs.writeFileSync(fixture, JSON.stringify(source)); + return syncGithubWorkItems(store, { githubRepo: 'example/repo', limit: 10, repoRoot: dir, ...options }); + }; + await fn({ store, source, sync, dir }); + console.log(` PASS ${name}`); + passed++; + } catch (error) { + console.log(` FAIL ${name}: ${error.stack}`); + failed++; + } finally { + if (store) store.close(); + if (previousShim === undefined) delete process.env.ECC_GH_SHIM; + else process.env.ECC_GH_SHIM = previousShim; + fs.rmSync(dir, { recursive: true, force: true }); + } +} + +async function run() { + for (const type of ['pr', 'issue']) { + for (const status of ['running', 'claimed', 'blocked', 'hold', 'done', 'closed']) { + await test(`${type}: refresh preserves locally managed ${status} work`, ({ store, source, sync, dir }) => { + const id = `github-example-repo-${type}-${type === 'pr' ? 3 : 8}`; + sync(); + const initial = store.getWorkItemById(id); + const owned = store.upsertWorkItem({ + ...initial, status, owner: 'worker', sessionId: 'session-1', priority: 'urgent', + repoRoot: path.join(dir, 'worktree'), metadata: { ...initial.metadata, assigneeKind: 'agent', note: 'keep me' } + }); + source[type][0] = { ...source[type][0], title: 'Updated title', author: { login: 'renamed-author' }, updatedAt: '2026-10-28T00:00:00Z', labels: [{ name: 'bug' }] }; + for (let attempt = 0; attempt < 2; attempt++) { + sync(); + const item = store.getWorkItemById(id); + for (const field of ['status', 'owner', 'sessionId', 'priority', 'repoRoot', 'createdAt']) { + assert.strictEqual(item[field], owned[field], field); + } + assert.match(item.title, /Updated title/); + assert.strictEqual(item.metadata.sourceAuthor, 'renamed-author'); + assert.strictEqual(item.metadata.sourceUpdatedAt, '2026-10-28T00:00:00Z'); + assert.strictEqual(item.metadata.assigneeKind, 'agent'); + assert.strictEqual(item.metadata.note, 'keep me'); + if (type === 'issue') assert.deepStrictEqual(item.metadata.labels, ['bug']); + assert.strictEqual(store.listWorkItems().totalCount, 2); + } + source.pr = []; source.issue = []; + const result = sync(); + assert.ok(!result.closedItems.some(item => item.id === id)); + assert.strictEqual(store.getWorkItemById(id).status, status); + }); + } + } + + await test('untouched imports follow PR readiness, close, and reopen', ({ store, source, sync }) => { + const id = 'github-example-repo-pr-3'; + sync(); + source.pr[0].isDraft = true; + sync(); + assert.strictEqual(store.getWorkItemById(id).status, 'blocked'); + const pr = source.pr[0]; + source.pr = []; source.issue = []; + assert.strictEqual(sync().closedCount, 2); + source.pr = [{ ...pr, isDraft: false }]; + sync(); + const reopened = store.getWorkItemById(id); + assert.strictEqual(reopened.status, 'needs-review'); + assert.strictEqual(reopened.metadata.sourceClosedAt, undefined); + }); + + await test('local hold alone survives refresh and disappearance', ({ store, source, sync }) => { + sync(); + const id = 'github-example-repo-issue-8'; + store.upsertWorkItem({ ...store.getWorkItemById(id), status: 'blocked' }); + sync(); + assert.strictEqual(store.getWorkItemById(id).status, 'blocked'); + source.issue = []; + sync(); + assert.strictEqual(store.getWorkItemById(id).status, 'blocked'); + }); + + await test('legacy imports without ownership provenance are preserved', ({ store, source, sync }) => { + const id = 'github-example-repo-issue-8'; + store.upsertWorkItem({ id, source: 'github-issue', title: 'Legacy', status: 'blocked', owner: 'worker', metadata: { repo: 'example/repo', syncedBy: 'ecc-work-items-sync-github' } }); + sync(); + source.issue = []; + sync(); + assert.strictEqual(store.getWorkItemById(id).status, 'blocked'); + assert.strictEqual(store.getWorkItemById(id).owner, 'worker'); + }); + + for (const [field, value] of Object.entries({ owner: null, sessionId: 'session-1', priority: 'high', repoRoot: null })) { + await test(`a local ${field} change alone survives disappearance`, ({ store, source, sync }) => { + sync(); + const id = 'github-example-repo-issue-8'; + store.upsertWorkItem({ ...store.getWorkItemById(id), [field]: value }); + source.issue = []; + sync(); + const item = store.getWorkItemById(id); + assert.strictEqual(item.status, 'needs-review'); + assert.strictEqual(item[field], value); + }); + } + + await test('local management persists when fields later match source defaults', ({ store, source, sync }) => { + sync(); + const id = 'github-example-repo-issue-8'; + const initial = store.getWorkItemById(id); + store.upsertWorkItem({ ...initial, status: 'running' }); + sync(); + store.upsertWorkItem({ ...store.getWorkItemById(id), status: initial.status }); + source.issue = []; + assert.ok(!sync().closedItems.some(item => item.id === id)); + assert.strictEqual(store.getWorkItemById(id).status, initial.status); + }); + + await test('a failed reconciliation rolls back earlier writes', ({ store, source, sync }) => { + sync(); + const initial = store.getWorkItemById('github-example-repo-pr-3'); + source.pr[0].title = 'Should roll back'; + const upsert = store.upsertWorkItem; + store.upsertWorkItem = item => { + if (item.source === 'github-issue') throw new Error('write failed'); + return upsert(item); + }; + assert.throws(sync, /write failed/); + assert.deepStrictEqual(store.getWorkItemById(initial.id), initial); + }); + + for (const existing of [false, true]) { + await test(`explicit session on ${existing ? 'existing' : 'new'} imports survives later sync`, ({ store, source, sync }) => { + if (existing) sync(); + sync({ sessionId: 'session-1' }); + sync(); + const id = 'github-example-repo-issue-8'; + assert.strictEqual(store.getWorkItemById(id).sessionId, 'session-1'); + source.issue = []; + sync(); + assert.strictEqual(store.getWorkItemById(id).status, 'needs-review'); + }); + } + + await test('local management first observed on disappearance remains sticky', ({ store, source, sync }) => { + sync(); + const id = 'github-example-repo-issue-8'; + store.upsertWorkItem({ ...store.getWorkItemById(id), status: 'blocked' }); + source.issue = []; + sync(); + store.upsertWorkItem({ ...store.getWorkItemById(id), status: 'needs-review' }); + sync(); + assert.strictEqual(store.getWorkItemById(id).status, 'needs-review'); + }); + + console.log(`Results: ${passed} passed, ${failed} failed`); + if (failed) process.exitCode = 1; +} + +run();