fix: claim on already asasigned to $self

[fblgit]

This PR:

• fix: when a task is already pre-assigned (worker) it doesn't gets claimed

[claude]

Claude finished @fblgit's task —— View job

---

Review Tasks

• Read repository conventions (CLAUDE.md)
• Examine the modified handler file
• Analyze the specific changes made
• Provide comprehensive review feedback

Code Review for PR #22

I've reviewed the changes to apps/server/src/handlers/task/task.assign.handler.ts and here's my feedback:

📋 Summary

The PR fixes a bug where tasks already assigned to the same instance would incorrectly throw an error when attempting to reassign them to the same worker.

✅ Code Quality

Good fix! The change correctly addresses the issue by allowing re-assignment to the same instance while still preventing cross-instance assignment conflicts. The modification is minimal and targeted.

🔍 Specific Changes

apps/server/src/handlers/task/task.assign.handler.ts:56

- if (previousAssignment) {
+ if (previousAssignment && previousAssignment !== input.instanceId) {

This change ensures that:

• ✅ Tasks can be re-assigned to the same instance (idempotent operation)
• ✅ Tasks cannot be stolen by different instances
• ✅ The error message remains accurate for actual conflicts

🐛 Potential Issues

None identified. The fix correctly handles the edge case.

⚡ Performance Considerations

No performance impact - this is a simple logical check adjustment.

🔒 Security Concerns

No security issues. The change maintains proper task ownership validation.

🧪 Test Coverage Recommendations

Consider adding these test cases to verify the fix:

1. Idempotent assignment test: Verify that assigning a task to the same instance twice succeeds
2. Cross-instance protection test: Confirm that assigning to a different instance still fails
3. Integration test: Check that the Redis keys and PostgreSQL state remain consistent after re-assignment

Example test structure:

it('should allow re-assignment to the same instance', async () => {
  const taskId = 't-123';
  const instanceId = 'worker-1';
  
  // First assignment
  await handler.handle({ taskId, instanceId }, ctx);
  
  // Second assignment to same instance should succeed
  const result = await handler.handle({ taskId, instanceId }, ctx);
  expect(result.instanceId).toBe(instanceId);
});

it('should prevent assignment to different instance', async () => {
  const taskId = 't-456';
  
  // Assign to first instance
  await handler.handle({ taskId, instanceId: 'worker-1' }, ctx);
  
  // Assignment to different instance should fail
  await expect(
    handler.handle({ taskId, instanceId: 'worker-2' }, ctx)
  ).rejects.toThrow('already assigned to worker-1');
});

💡 Additional Observations

• The comment update on line 54 accurately reflects the new behavior
• The fix aligns with the "Localhost Reality" principle from CLAUDE.md
• The backward compatibility approach is maintained correctly

✅ Verdict

This is a clean, focused fix that resolves the described issue without introducing side effects. The change is production-ready.