Skip to content

Commit d227057

Browse files
author
Sorra
authored
Merge pull request #548 from TheWizardsCode/copilot/unblock-items-on-in-review
Add tests and docs for in_review stage unblocking dependency-edge dependents
2 parents 760a6b0 + 96778e1 commit d227057

4 files changed

Lines changed: 151 additions & 4 deletions

File tree

‎CLI.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,8 @@ Behavior:
183183
- `dep list --incoming` shows only inbound dependencies.
184184

185185
**Automatic unblocking:** Dependents are automatically unblocked when all their blockers
186-
become inactive (completed or deleted). This reconciliation happens via `db.update()` and
186+
become inactive (completed, deleted, or moved to a non-blocking stage such as `in_review`
187+
or `done`). This reconciliation happens via `db.update()` and
187188
`db.delete()` — any status or stage change triggers the reconciliation logic. See
188189
[Dependency Reconciliation](docs/dependency-reconciliation.md) for developer details.
189190

‎docs/dependency-reconciliation.md‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ All reconciliation logic lives in `src/database.ts`:
1515
| `reconcileDependentsForTarget(targetId)` | ~1811 | Entry point: finds all dependents of `targetId` and reconciles each one |
1616
| `reconcileDependentStatus(dependentId)` | ~1772 | Determines whether a dependent should be blocked or unblocked |
1717
| `reconcileBlockedStatus(itemId)` | ~1749 | Sets or clears `blocked` status based on active blockers |
18-
| `isDependencyActive(item)` | ~1701 | Returns `true` if an item is an active blocker (not completed, not deleted) |
18+
| `isDependencyActive(item)` | ~1701 | Returns `true` if an item is an active blocker (not completed, not deleted, not in `in_review` or `done` stage) |
1919
| `hasActiveBlockers(itemId)` | ~1738 | Returns `true` if any inbound dependency edges point to active items |
2020
| `getInboundDependents(targetId)` | ~1726 | Returns IDs of items that depend on `targetId` |
2121
| `listDependencyEdgesTo(targetId)` | ~1696 | Returns all dependency edges where `targetId` is the prerequisite |
@@ -39,10 +39,19 @@ All reconciliation logic lives in `src/database.ts`:
3939
| Close a blocker (sole blocker) | Dependent unblocked (status -> `open`) |
4040
| Close a blocker (other blockers remain) | Dependent stays `blocked` |
4141
| Delete a blocker | Dependent unblocked if no other active blockers |
42+
| Move blocker to `in_review` stage (sole blocker) | Dependent unblocked (status -> `open`) |
43+
| Move blocker to `in_review` stage (other active blockers remain) | Dependent stays `blocked` |
44+
| Move blocker to `done` stage | Dependent unblocked if no other active blockers |
4245
| Reopen a closed blocker | Dependent re-blocked (status -> `blocked`) |
46+
| Move blocker back from `in_review` to an active stage | Dependent re-blocked (status -> `blocked`) |
4347
| Close already-closed blocker | No-op (idempotent) |
48+
| Move blocker to `in_review` multiple times | No-op (idempotent) |
4449
| Dependent is completed/deleted | No status change (already terminal) |
4550

51+
> **Note:** The `in_review` stage is treated as non-blocking for **dependency edges only**.
52+
> Parent/child relationships are not affected by this change — a child item moving to
53+
> `in_review` does not unblock its parent.
54+
4655
## CLI and TUI Parity
4756

4857
Both the CLI `close` command (`src/commands/close.ts`) and the TUI close handler (`src/tui/controller.ts`) call `db.update(id, { status: 'completed' })`, which triggers the same reconciliation path. There is no separate unblock logic in either interface — all unblocking is handled by the shared database layer.
@@ -53,5 +62,5 @@ The `wl dep add` command (`src/commands/dep.ts`) adds a dependency edge and then
5362

5463
## Test Coverage
5564

56-
- **Unit tests**: `tests/database.test.ts` — `dependency edges` describe block contains tests for single-blocker unblock, multi-blocker scenarios, chain dependencies, delete unblock, reopen re-block, idempotence, and more.
57-
- **CLI integration tests**: `tests/cli/issue-management.test.ts` — tests for `close` and `dep` commands verifying end-to-end unblock behaviour through the CLI.
65+
- **Unit tests**: `tests/database.test.ts` — `dependency edges` describe block contains tests for single-blocker unblock, multi-blocker scenarios, chain dependencies, delete unblock, reopen re-block, idempotence, `in_review` stage unblocking (single blocker, partial multi-blocker, all blockers, mixed in_review/completed, idempotence, re-block on stage revert, multiple dependents), and more.
66+
- **CLI integration tests**: `tests/cli/issue-management.test.ts` — tests for `close` and `dep` commands verifying end-to-end unblock behaviour through the CLI, including `in_review` stage unblocking (single blocker → in_review, partial multi-blocker, all blockers → in_review).

‎tests/cli/issue-management.test.ts‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -686,5 +686,56 @@ describe('CLI Issue Management Tests', () => {
686686
expect(result.error).toBe('Cannot use --incoming and --outgoing together.');
687687
}
688688
});
689+
690+
it('should unblock dependent when sole blocker moves to in_review stage via update', async () => {
691+
const { stdout: blockedStdout } = await execAsync(`tsx ${cliPath} --json create -t "Blocked"`);
692+
const { stdout: blockerStdout } = await execAsync(`tsx ${cliPath} --json create -t "Blocker"`);
693+
const blockedId = JSON.parse(blockedStdout).workItem.id;
694+
const blockerId = JSON.parse(blockerStdout).workItem.id;
695+
696+
await execAsync(`tsx ${cliPath} --json dep add ${blockedId} ${blockerId}`);
697+
const { stdout: blockedShowStdout } = await execAsync(`tsx ${cliPath} --json show ${blockedId}`);
698+
expect(JSON.parse(blockedShowStdout).workItem.status).toBe('blocked');
699+
700+
await execAsync(`tsx ${cliPath} --json update ${blockerId} --status completed --stage in_review`);
701+
const { stdout: unblockedShowStdout } = await execAsync(`tsx ${cliPath} --json show ${blockedId}`);
702+
expect(JSON.parse(unblockedShowStdout).workItem.status).toBe('open');
703+
});
704+
705+
it('should keep dependent blocked when only one of multiple blockers moves to in_review', async () => {
706+
const { stdout: blockedStdout } = await execAsync(`tsx ${cliPath} --json create -t "Blocked"`);
707+
const { stdout: blockerAStdout } = await execAsync(`tsx ${cliPath} --json create -t "BlockerA"`);
708+
const { stdout: blockerBStdout } = await execAsync(`tsx ${cliPath} --json create -t "BlockerB"`);
709+
const blockedId = JSON.parse(blockedStdout).workItem.id;
710+
const blockerAId = JSON.parse(blockerAStdout).workItem.id;
711+
const blockerBId = JSON.parse(blockerBStdout).workItem.id;
712+
713+
await execAsync(`tsx ${cliPath} --json dep add ${blockedId} ${blockerAId}`);
714+
await execAsync(`tsx ${cliPath} --json dep add ${blockedId} ${blockerBId}`);
715+
716+
await execAsync(`tsx ${cliPath} --json update ${blockerAId} --status completed --stage in_review`);
717+
const { stdout: stillBlockedStdout } = await execAsync(`tsx ${cliPath} --json show ${blockedId}`);
718+
expect(JSON.parse(stillBlockedStdout).workItem.status).toBe('blocked');
719+
});
720+
721+
it('should unblock dependent when all blockers move to in_review', async () => {
722+
const { stdout: blockedStdout } = await execAsync(`tsx ${cliPath} --json create -t "Blocked"`);
723+
const { stdout: blockerAStdout } = await execAsync(`tsx ${cliPath} --json create -t "BlockerA"`);
724+
const { stdout: blockerBStdout } = await execAsync(`tsx ${cliPath} --json create -t "BlockerB"`);
725+
const blockedId = JSON.parse(blockedStdout).workItem.id;
726+
const blockerAId = JSON.parse(blockerAStdout).workItem.id;
727+
const blockerBId = JSON.parse(blockerBStdout).workItem.id;
728+
729+
await execAsync(`tsx ${cliPath} --json dep add ${blockedId} ${blockerAId}`);
730+
await execAsync(`tsx ${cliPath} --json dep add ${blockedId} ${blockerBId}`);
731+
732+
await execAsync(`tsx ${cliPath} --json update ${blockerAId} --status completed --stage in_review`);
733+
const { stdout: stillBlockedStdout } = await execAsync(`tsx ${cliPath} --json show ${blockedId}`);
734+
expect(JSON.parse(stillBlockedStdout).workItem.status).toBe('blocked');
735+
736+
await execAsync(`tsx ${cliPath} --json update ${blockerBId} --status completed --stage in_review`);
737+
const { stdout: unblockedStdout } = await execAsync(`tsx ${cliPath} --json show ${blockedId}`);
738+
expect(JSON.parse(unblockedStdout).workItem.status).toBe('open');
739+
});
689740
});
690741
});

‎tests/database.test.ts‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -657,6 +657,92 @@ describe('WorklogDatabase', () => {
657657
}
658658
}
659659
});
660+
661+
describe('in_review stage unblocking (dependency edges only)', () => {
662+
it('should unblock dependent when sole blocker moves to in_review stage', () => {
663+
const blocker = db.create({ title: 'Blocker', status: 'open', stage: 'in_progress' });
664+
const blocked = db.create({ title: 'Blocked', status: 'blocked' });
665+
db.addDependencyEdge(blocked.id, blocker.id);
666+
667+
db.update(blocker.id, { stage: 'in_review' });
668+
expect(db.get(blocked.id)?.status).toBe('open');
669+
});
670+
671+
it('should keep dependent blocked when one of multiple blockers moves to in_review', () => {
672+
const blockerA = db.create({ title: 'Blocker A', status: 'open', stage: 'in_progress' });
673+
const blockerB = db.create({ title: 'Blocker B', status: 'open', stage: 'in_progress' });
674+
const blocked = db.create({ title: 'Blocked', status: 'blocked' });
675+
db.addDependencyEdge(blocked.id, blockerA.id);
676+
db.addDependencyEdge(blocked.id, blockerB.id);
677+
678+
db.update(blockerA.id, { stage: 'in_review' });
679+
expect(db.get(blocked.id)?.status).toBe('blocked');
680+
});
681+
682+
it('should unblock dependent when all blockers move to in_review', () => {
683+
const blockerA = db.create({ title: 'Blocker A', status: 'open', stage: 'in_progress' });
684+
const blockerB = db.create({ title: 'Blocker B', status: 'open', stage: 'in_progress' });
685+
const blocked = db.create({ title: 'Blocked', status: 'blocked' });
686+
db.addDependencyEdge(blocked.id, blockerA.id);
687+
db.addDependencyEdge(blocked.id, blockerB.id);
688+
689+
db.update(blockerA.id, { stage: 'in_review' });
690+
expect(db.get(blocked.id)?.status).toBe('blocked');
691+
692+
db.update(blockerB.id, { stage: 'in_review' });
693+
expect(db.get(blocked.id)?.status).toBe('open');
694+
});
695+
696+
it('should unblock dependent when mix of in_review and completed blockers are all non-blocking', () => {
697+
const blockerA = db.create({ title: 'Blocker A', status: 'open', stage: 'in_progress' });
698+
const blockerB = db.create({ title: 'Blocker B', status: 'open', stage: 'in_progress' });
699+
const blocked = db.create({ title: 'Blocked', status: 'blocked' });
700+
db.addDependencyEdge(blocked.id, blockerA.id);
701+
db.addDependencyEdge(blocked.id, blockerB.id);
702+
703+
db.update(blockerA.id, { status: 'completed' });
704+
expect(db.get(blocked.id)?.status).toBe('blocked');
705+
706+
db.update(blockerB.id, { stage: 'in_review' });
707+
expect(db.get(blocked.id)?.status).toBe('open');
708+
});
709+
710+
it('should be idempotent: moving blocker to in_review multiple times does not break state', () => {
711+
const blocker = db.create({ title: 'Blocker', status: 'open', stage: 'in_progress' });
712+
const blocked = db.create({ title: 'Blocked', status: 'blocked' });
713+
db.addDependencyEdge(blocked.id, blocker.id);
714+
715+
db.update(blocker.id, { stage: 'in_review' });
716+
expect(db.get(blocked.id)?.status).toBe('open');
717+
718+
db.update(blocker.id, { stage: 'in_review' });
719+
expect(db.get(blocked.id)?.status).toBe('open');
720+
});
721+
722+
it('should re-block dependent when blocker moves back from in_review to in_progress', () => {
723+
const blocker = db.create({ title: 'Blocker', status: 'open', stage: 'in_progress' });
724+
const blocked = db.create({ title: 'Blocked', status: 'blocked' });
725+
db.addDependencyEdge(blocked.id, blocker.id);
726+
727+
db.update(blocker.id, { stage: 'in_review' });
728+
expect(db.get(blocked.id)?.status).toBe('open');
729+
730+
db.update(blocker.id, { stage: 'in_progress' });
731+
expect(db.get(blocked.id)?.status).toBe('blocked');
732+
});
733+
734+
it('should unblock multiple dependents when their shared blocker moves to in_review', () => {
735+
const blocker = db.create({ title: 'Shared Blocker', status: 'open', stage: 'in_progress' });
736+
const dependentA = db.create({ title: 'Dependent A', status: 'blocked' });
737+
const dependentB = db.create({ title: 'Dependent B', status: 'blocked' });
738+
db.addDependencyEdge(dependentA.id, blocker.id);
739+
db.addDependencyEdge(dependentB.id, blocker.id);
740+
741+
db.update(blocker.id, { stage: 'in_review' });
742+
expect(db.get(dependentA.id)?.status).toBe('open');
743+
expect(db.get(dependentB.id)?.status).toBe('open');
744+
});
745+
});
660746
});
661747

662748
describe('import and export', () => {

0 commit comments

Comments
 (0)