Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions backend/__tests__/unit/controllers/podController.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,51 @@ describe('podController', () => {
expect(res.json).toHaveBeenCalledWith(pod);
});

// ── TASK-166: the creator cannot leave, everyone else still can ─────────

it('leavePod refuses the creator with 409 creator_cannot_leave and keeps them listed', async () => {
const pod = {
_id: 'p1',
createdBy: 'creator',
members: ['creator', 'member'],
save: jest.fn(),
populate: jest.fn(),
};
Pod.findById.mockResolvedValue(pod);
const req = { params: { id: 'p1' }, userId: 'creator' };
const res = { status: jest.fn().mockReturnThis(), json: jest.fn() };

await podController.leavePod(req, res);

expect(res.status).toHaveBeenCalledWith(409);
expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ code: 'creator_cannot_leave' }));
// The refusal is a non-event, not a silent success: nothing was unlisted
// and nothing was saved.
expect(pod.members).toEqual(['creator', 'member']);
expect(pod.save).not.toHaveBeenCalled();
});

// Positive control. The arm above is satisfied by a route that refuses
// everyone, which is exactly the shape the guard must not be.
it('leavePod still removes a non-creator member (control)', async () => {
const pod = {
_id: 'p1',
createdBy: 'creator',
members: ['creator', 'member'],
save: jest.fn(),
populate: jest.fn().mockResolvedValue(),
};
Pod.findById.mockResolvedValue(pod);
const req = { params: { id: 'p1' }, userId: 'member' };
const res = { status: jest.fn().mockReturnThis(), json: jest.fn() };

await podController.leavePod(req, res);

expect(pod.members).toEqual(['creator']);
expect(pod.save).toHaveBeenCalled();
expect(res.json).toHaveBeenCalledWith(pod);
});

// ── ADR-001 §3.10: agent-rooms are 1:1 DMs ──────────────────────────────

it('joinPod rejects a third-person join on agent-room with 403', async () => {
Expand Down
24 changes: 22 additions & 2 deletions backend/__tests__/unit/routes/activity.write-membership.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,22 @@ describe('POST /api/activity/create — pod membership', () => {
expect(create).toHaveBeenCalledTimes(1);
});

it('allows the creator, who is not always listed in members', async () => {
const { app, create } = setup({ _id: 'pod-1', createdBy: CALLER, members: [] });
// TASK-166 inverted this arm. It used to assert that the creator was admitted
// with an empty `members` — i.e. that `createdBy` counted as membership. It
// now asserts the opposite: `createdBy` is who made the pod, and a creator
// `leavePod` has unlisted is subject to the same membership rule as everyone
// else (it is the rule `createMessage` and the relay have always applied).
it('refuses a creator who is no longer listed, and writes nothing', async () => {
const { app, create } = setup({ _id: 'pod-1', createdBy: CALLER, members: [OTHER] });
await request(app).post('/api/activity/create').send(body()).expect(403);
expect(create).not.toHaveBeenCalled();
});

// The control for the inversion: a creator who IS listed still writes. Without
// it, the arm above passes for a route that refuses every pod naming a
// creator.
it('still admits a creator who is also listed', async () => {
const { app, create } = setup({ _id: 'pod-1', createdBy: CALLER, members: [CALLER] });
await request(app).post('/api/activity/create').send(body()).expect(200);
expect(create).toHaveBeenCalledTimes(1);
});
Expand Down Expand Up @@ -155,4 +169,10 @@ describe('POST /api/activity/seed/:podId — pod membership', () => {
await request(app).post('/api/activity/seed/pod-1').send({}).expect(404);
expect(seedPodActivities).not.toHaveBeenCalled();
});

it('refuses a creator who is no longer listed, and never reaches the seeder', async () => {
const { app, seedPodActivities } = setup({ _id: 'pod-1', createdBy: CALLER, members: [OTHER] });
await request(app).post('/api/activity/seed/pod-1').send({}).expect(403);
expect(seedPodActivities).not.toHaveBeenCalled();
});
});
47 changes: 47 additions & 0 deletions backend/__tests__/unit/routes/podInvites.management.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,53 @@ describe('pod invite management routes', () => {
expect(invite.save).not.toHaveBeenCalled();
});

// ── TASK-166: `createdBy` is not membership ─────────────────────────────
// The three site arms below each name one route. A departed creator is
// `createdBy: user1` with `members` no longer listing them — the shape
// `leavePod` leaves behind, and the only shape the permissive predicate
// admitted that the strict one refuses.

it('refuses to create an invite for a creator who left', async () => {
Pod.findById.mockResolvedValue(memberPod({ createdBy: 'user1', members: ['someone-else'] }));

await request(app).post(`/api/pods/${POD_ID}/invites`).send({}).expect(403);

const { PodInvite } = require('../../../models/PodInvite');
expect(PodInvite.create).not.toHaveBeenCalled();
});

it('refuses to list invites for a creator who left', async () => {
Pod.findById.mockResolvedValue(memberPod({ createdBy: 'user1', members: ['someone-else'] }));

await request(app).get(`/api/pods/${POD_ID}/invites`).expect(403);

expect(mockFind).not.toHaveBeenCalled();
});

it('refuses to revoke an invite for a creator who left', async () => {
const invite = {
token: TOKEN, podId: POD_ID, revokedAt: null, save: jest.fn(),
};
mockFindOne.mockResolvedValue(invite);
Pod.findById.mockResolvedValue(memberPod({ createdBy: 'user1', members: ['someone-else'] }));

await request(app).delete(`/api/invites/${TOKEN}`).expect(403);

expect(invite.save).not.toHaveBeenCalled();
});

// Positive control: a creator who is ALSO listed still manages invites, so
// the three arms above cannot pass by refusing every pod that names a
// creator.
it('still lets a creator who is also listed manage invites', async () => {
Pod.findById.mockResolvedValue(memberPod({ createdBy: 'user1', members: ['user1'] }));
mockFind.mockReturnValue(listChain([]));

await request(app).get(`/api/pods/${POD_ID}/invites`).expect(200);

expect(mockFind).toHaveBeenCalledWith({ podId: POD_ID, revokedAt: null });
});

it('rejects a revoked invite on redemption', async () => {
mockFindOne.mockResolvedValue({
token: TOKEN,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,36 @@ describe('ActivityService.getDecisionQueue', () => {
});
});

// TASK-166. `createdBy` is who made the pod, not a standing membership, and
// `leavePod` leaves it behind. Two witnesses: the query term, and the
// in-process filter that re-checks the returned rows.
it('reads settled history by membership only', async () => {
mockPodFind.mockReturnValue(chain([{
_id: 'pod-1', name: 'Current', createdBy: 'owner', members: ['member-1'],
}]));

await ActivityService.getDecisionHistory('member-1', { podId: 'pod-1' });

expect(mockPodFind).toHaveBeenCalledWith({
_id: 'pod-1',
$or: [
{ 'members.userId': 'member-1' },
{ members: 'member-1' },
{ 'members._id': 'member-1' },
],
});
});

it('refuses a creator who left the pod, although createdBy still names them', async () => {
mockPodFind.mockReturnValue(chain([{
_id: 'pod-1', name: 'Former', createdBy: 'creator-1', members: ['member-1'],
}]));

await expect(ActivityService.getDecisionHistory('creator-1', { podId: 'pod-1' }))
.rejects.toThrow('Access denied');
expect(mockDecisionFind).not.toHaveBeenCalled();
});

it('rejects settled history for a viewer outside the requested pod', async () => {
mockPodFind.mockReturnValue(chain([]));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,14 @@ describe('ActivityService.getPendingApprovals', () => {
jest.clearAllMocks();
});

it('queries approvals from both creator and membership pods', async () => {
// TASK-166 inverted this arm: it used to assert the query read `createdBy`
// as membership ("both creator and membership pods"). `createdBy` records who
// made the pod and `leavePod` keeps it, so that term put a departed creator's
// queue back in front of them. The read rule and the write gate in
// `requireActivityApprovalMember` are now the same rule.
it('queries approvals from listed membership only', async () => {
const memberUserId = 'queue-member';
Pod.find.mockReturnValue(podFindResult([{ _id: 'owner-pod' }]));
Pod.find.mockReturnValue(podFindResult([{ _id: 'member-pod' }]));
Activity.getPendingApprovals.mockResolvedValue([{ _id: 'approval-1' }]);

await expect(ActivityService.getPendingApprovals(memberUserId)).resolves.toEqual([
Expand All @@ -34,10 +39,9 @@ describe('ActivityService.getPendingApprovals', () => {

expect(Pod.find).toHaveBeenCalledWith({
$or: [
{ createdBy: memberUserId },
{ members: memberUserId },
],
});
expect(Activity.getPendingApprovals).toHaveBeenCalledWith(['owner-pod']);
expect(Activity.getPendingApprovals).toHaveBeenCalledWith(['member-pod']);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
// TASK-166: the pod feed is pod-scoped, and `getPodFeed` checked membership as
// `createdBy === userId || members.includes(userId)`. `leavePod` filters
// `members` and keeps `createdBy`, so a departed creator could still read the
// pod's whole activity feed — including activities written after they left —
// while every chat and connector write into that pod refused them.
//
// Route-level tests mock this method out (`routes/activity.read.test.js`,
// `routes/activity.identity.test.js`), so the arm lives here.
jest.mock('../../../models/Pod', () => ({ findById: jest.fn() }));
jest.mock('../../../models/User', () => ({ findById: jest.fn() }));
jest.mock('../../../models/Activity', () => ({}));
jest.mock('../../../models/Summary', () => ({}));
jest.mock('../../../models/Post', () => ({}));
jest.mock('../../../models/Task', () => ({ find: jest.fn() }));

const Pod = require('../../../models/Pod');
const User = require('../../../models/User');
const ActivityService = require('../../../services/activityService');

const userChain = (value) => ({ select: () => ({ lean: async () => value }) });
const podChain = (value) => ({ lean: async () => value });

describe('ActivityService.getPodFeed — pod membership', () => {
let aggregateSpy;
let readStateSpy;

beforeEach(() => {
jest.clearAllMocks();
User.findById.mockReturnValue(userChain({ _id: 'caller-1', username: 'Caller' }));
aggregateSpy = jest.spyOn(ActivityService, 'aggregateActivities').mockResolvedValue([]);
readStateSpy = jest.spyOn(ActivityService, 'annotateReadState').mockReturnValue([]);
});

afterEach(() => {
aggregateSpy.mockRestore();
readStateSpy.mockRestore();
});

it('refuses a creator who left the pod', async () => {
Pod.findById.mockReturnValue(podChain({
_id: 'pod-1', name: 'Former', createdBy: 'creator-1', members: ['member-1'],
}));

await expect(ActivityService.getPodFeed('pod-1', 'creator-1', {}))
.rejects.toThrow('Access denied');
expect(aggregateSpy).not.toHaveBeenCalled();
});

it('still serves a listed member (control)', async () => {
Pod.findById.mockReturnValue(podChain({
_id: 'pod-1', name: 'Current', createdBy: 'creator-1', members: ['member-1'],
}));

await expect(ActivityService.getPodFeed('pod-1', 'member-1', {}))
.resolves.toMatchObject({ activities: [], hasMore: false });
});
});
35 changes: 35 additions & 0 deletions backend/__tests__/unit/services/activityService.recap.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,22 @@ describe('ActivityService recap and legacy approval authorization', () => {
findByIdSpy?.mockRestore();
});

test('builds the viewer\'s pod list from membership, not from createdBy', async () => {
await ActivityService.getRecap(ownerId, { window: 'today' });

// The membership decision for this reader is in the query, so the arm
// asserts the term the change removes rather than a row — the Pod mock
// returns whichever fixture it is handed. `createdBy` is written once at
// creation and survives `leavePod`, so reading it here handed a departed
// creator the recap of a pod they are no longer in.
expect(Pod.find.mock.calls[0][0]).toEqual({
$or: [
{ 'members.userId': ownerId },
{ members: ownerId },
],
});
});

test('rejects a requested pod that is outside the viewer membership', async () => {
await expect(ActivityService.getRecap(ownerId, { podId: 'not-a-member-pod' }))
.rejects.toThrow('Access denied');
Expand Down Expand Up @@ -112,6 +128,25 @@ describe('ActivityService recap and legacy approval authorization', () => {
expect(mockResolve).toHaveBeenCalledWith('approval', storedApproval._id);
});

// TASK-166: the read rule and this gate move together. A non-member arm
// cannot see the creator clause at all — only a creator can — so this is the
// arm that distinguishes the two predicates.
test('fails closed when the pod\'s creator has left it', async () => {
const storedApproval = new Activity({ type: 'approval_needed', action: 'approval_needed', podId: pod._id });
const approve = jest.fn().mockResolvedValue();
storedApproval.approve = approve;
findByIdSpy = jest.spyOn(Activity, 'findById').mockResolvedValue(storedApproval);
Pod.findById.mockReturnValue({
select: jest.fn(() => ({
lean: jest.fn().mockResolvedValue({ _id: 'pod-1', createdBy: ownerId, members: ['member-1'] }),
})),
});

await expect(ActivityService.approveActivity(String(storedApproval._id), ownerId, 'Approved'))
.resolves.toEqual({ success: false, status: 403, error: 'Only pod members can decide this' });
expect(approve).not.toHaveBeenCalled();
});

test('fails closed when a non-member attempts a legacy Activity approval', async () => {
const storedApproval = new Activity({ type: 'approval_needed', action: 'approval_needed', podId: pod._id });
const approve = jest.fn().mockResolvedValue();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// TASK-166: `getUserFeed` builds the viewer's pod set from membership. It used
// to `$or` in `{ createdBy: userId }`, and `leavePod` keeps that field after
// unlisting the creator — so a departed creator's ambient feed still contained
// the pod they had left, while every chat and connector write into it refused
// them.
//
// Covered here rather than through `getRecap`, which spies this method out: the
// membership decision is in the query, so the arm asserts the query rather than
// a row (the Pod mock returns whichever fixture it is handed).
jest.mock('../../../models/Pod', () => ({ find: jest.fn() }));
jest.mock('../../../models/User', () => ({ findById: jest.fn() }));
jest.mock('../../../models/Activity', () => ({}));
jest.mock('../../../models/Summary', () => ({}));
jest.mock('../../../models/Post', () => ({}));
jest.mock('../../../models/Task', () => ({ find: jest.fn() }));

const Pod = require('../../../models/Pod');
const User = require('../../../models/User');
const ActivityService = require('../../../services/activityService');

const chain = (value) => ({ select: () => ({ lean: async () => value }) });

describe('ActivityService.getUserFeed — pod membership', () => {
let rankSpy;
let aggregateSpy;

beforeEach(() => {
jest.clearAllMocks();
User.findById.mockReturnValue(chain({ _id: 'caller-1', username: 'Caller' }));
Pod.find.mockReturnValue(chain([]));
rankSpy = jest.spyOn(ActivityService, 'rankPodsByRecentActivity').mockResolvedValue([]);
aggregateSpy = jest.spyOn(ActivityService, 'aggregateActivities').mockResolvedValue([]);
});

afterEach(() => {
rankSpy.mockRestore();
aggregateSpy.mockRestore();
});

it('selects the viewer\'s pods by membership, not by createdBy', async () => {
await ActivityService.getUserFeed('caller-1', {});

expect(Pod.find).toHaveBeenCalledWith({
$or: [
{ 'members.userId': 'caller-1' },
{ members: 'caller-1' },
],
});
});

it('still reaches the aggregator for a member (control)', async () => {
await expect(ActivityService.getUserFeed('caller-1', {})).resolves.toMatchObject({
activities: [],
hasMore: false,
});
expect(aggregateSpy).toHaveBeenCalledTimes(1);
});
});
15 changes: 15 additions & 0 deletions backend/__tests__/unit/services/decisionRequestService.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -269,4 +269,19 @@ describe('DecisionRequestService', () => {
.resolves.toEqual({ status: 403, body: { error: 'Only human pod members can rule on this decision' } });
expect(mockDecision.findOneAndUpdate).not.toHaveBeenCalled();
});

// TASK-166: `createdBy` is who made the pod, not a standing membership. A
// creator who has left the room must not rule on its decision cards — the
// gate is reached from `routes/activity.ts` under plain `auth`, which bot
// and human tokens both satisfy, so this is the reachable population and not
// a hypothetical one.
test('refuses a creator who left the pod before claiming the decision', async () => {
mockDecision.findById.mockResolvedValue(pending());
mockUser.findById.mockReturnValue(userChain({ _id: 'creator-1', username: 'Former owner', isBot: false }));
mockPod.findById.mockReturnValue(podChain({ createdBy: 'creator-1', members: ['member'], type: 'team' }));
await expect(chooseDecision({ decisionId: 'decision-1', callerUserId: 'creator-1', value: 'Canary' }))
.resolves.toEqual({ status: 403, body: { error: 'Only human pod members can rule on this decision' } });
expect(mockDecision.findOneAndUpdate).not.toHaveBeenCalled();
expect(mockPGMessage.create).not.toHaveBeenCalled();
});
});
Loading
Loading