updates on the ui and updates agents

tests/api/endpoints.test.js

@@ -288,9 +288,13 @@ describe('doormile endpoints', () => {
       expect(mockClient.post).toHaveBeenCalledWith('/admin/bookings/4021/assign-vehicle', { vehicleid: 5 });
     });
 
+    // ADMIN, not hub. /hub/bookings/batch-assign sits behind HubStaffAuth and
+    // 403s for every token this console issues, so this function could never
+    // succeed while it pointed there. /admin/bookings/batch-assign runs the
+    // same solver (controllers/batchAssignService.go) under admin auth.
     it('should batch-assign with a default cap of five stops per rider', async () => {
       await batchAssignBookings([1, 2, 3]);
-      expect(mockClient.post).toHaveBeenCalledWith('/hub/bookings/batch-assign', {
+      expect(mockClient.post).toHaveBeenCalledWith('/admin/bookings/batch-assign', {
         bookingids: [1, 2, 3],
         max_per_rider: 5
       });
@@ -298,12 +302,19 @@ describe('doormile endpoints', () => {
 
     it('should honour an explicit per-rider cap', async () => {
       await batchAssignBookings([1, 2], 2);
-      expect(mockClient.post).toHaveBeenCalledWith('/hub/bookings/batch-assign', {
+      expect(mockClient.post).toHaveBeenCalledWith('/admin/bookings/batch-assign', {
         bookingids: [1, 2],
         max_per_rider: 2
       });
     });
 
+    it('should not call the hub-only route, which 403s from this console', async () => {
+      await batchAssignBookings([1]);
+      expect(mockClient.post).not.toHaveBeenCalledWith(
+        '/hub/bookings/batch-assign', expect.anything()
+      );
+    });
+
     it('should bulk-cancel through the dedicated route', async () => {
       await bulkCancelBookings([1, 2, 3]);
       expect(mockClient.post).toHaveBeenCalledWith('/admin/bookings/bulk-cancel', { bookingIds: [1, 2, 3] });

tests/integration/clientOnboarding.test.jsx

@@ -97,6 +97,7 @@ const fillValidForm = async () => {
   fill('Mobile number', '98765 43210');
   fill('Login email', ' Ops@Acme.Example ');
   fill('Operating city', '1');
+  fill('What do they deliver', 'Clothing');
   fireEvent.change(document.getElementById('onboard-password'), { target: { value: 's3cure-pass' } });
   fill('Confirm password', 's3cure-pass');
   fireEvent.click(screen.getAllByRole('button', { name: 'Pick Client address' })[0]);
@@ -221,6 +222,7 @@ describe('Client onboarding page', () => {
       phone: '9876543210',
       password: 's3cure-pass',
       applocationid: 1,
+      deliverycategory: 'Clothing',
       requiredeliveryotp: false,
       address: '14 DB Road, RS Puram, Coimbatore',
       city: 'Coimbatore',
@@ -263,6 +265,7 @@ describe('Client onboarding page', () => {
     fill('Mobile number', '9876543210');
     fill('Login email', 'ops@acme.example');
     fill('Operating city', '1');
+    fill('What do they deliver', 'Clothing');
     fireEvent.change(document.getElementById('onboard-password'), { target: { value: 's3cure-pass' } });
     fill('Confirm password', 's3cure-pass');
     // Typed, never picked: no map location.
@@ -281,6 +284,41 @@ describe('Client onboarding page', () => {
     expect(await screen.findByText('5 Avinashi Road · 641004')).toBeInTheDocument();
   });
 
+  // The feature itself: what they ship decides whether returns apply.
+  it('offers reverse logistics for a clothing client', async () => {
+    renderPage();
+    await fillValidForm();
+    fill('What do they deliver', 'Clothing');
+    expect(await screen.findByText(/Reverse logistics available/i)).toBeInTheDocument();
+    expect(screen.getByText(/can be returned to this client/i)).toBeInTheDocument();
+  });
+
+  // A returned meal is waste, not inventory — nothing to restock or resell.
+  it('withdraws reverse logistics for a food client, and says why', async () => {
+    renderPage();
+    await fillValidForm();
+    fill('What do they deliver', 'Food');
+    expect(await screen.findByText(/No reverse logistics/i)).toBeInTheDocument();
+    // Stated, not silently hidden: an absent row reads as an oversight, and the
+    // operator needs to know before promising a client a return path.
+    expect(screen.getByText(/cannot be restocked or resold/i)).toBeInTheDocument();
+  });
+
+  it('says nothing about returns until a category is chosen', () => {
+    renderPage();
+    expect(screen.queryByText(/Reverse logistics available/i)).not.toBeInTheDocument();
+    expect(screen.queryByText(/No reverse logistics/i)).not.toBeInTheDocument();
+  });
+
+  it('refuses to submit without a delivery category', async () => {
+    renderPage();
+    await fillValidForm();
+    fill('What do they deliver', '');
+    fireEvent.click(screen.getByRole('button', { name: /Onboard client/ }));
+    expect(await screen.findByText(/choose what this client delivers/i)).toBeInTheDocument();
+    expect(api.onboardClient).not.toHaveBeenCalled();
+  });
+
   it('puts a server conflict under the field it is about', async () => {
     api.onboardClient.mockRejectedValue({ response: { status: 409, data: { success: false, message: 'this email already has a console login' } } });
     renderPage();
@@ -359,7 +397,7 @@ describe('client onboarding helpers', () => {
       expect(pw).toMatch(/\d/);
       expect(pw).toMatch(/[a-z]/i);
       const errors = validateOnboarding({
-        companyname: 'Acme', contactname: 'Priya', email: 'a@b.co', phone: '9876543210', applocationid: 1, password: pw, confirm: pw,
+        companyname: 'Acme', contactname: 'Priya', email: 'a@b.co', phone: '9876543210', applocationid: 1, deliverycategory: 'Clothing', password: pw, confirm: pw,
         address: '14 DB Road, RS Puram', pincode: '641002', latitude: 11.009, longitude: 76.95,
       });
       expect(errors).toEqual({});
@@ -374,7 +412,7 @@ describe('client onboarding helpers', () => {
   });
 
   it('mirrors the server rules', () => {
-    const base = { companyname: 'Acme', contactname: 'Priya', email: 'a@b.co', phone: '9876543210', applocationid: 1, password: 'longenough1', confirm: 'longenough1' };
+    const base = { companyname: 'Acme', contactname: 'Priya', email: 'a@b.co', phone: '9876543210', applocationid: 1, deliverycategory: 'Clothing', password: 'longenough1', confirm: 'longenough1' };
     expect(validateOnboarding({ ...base, phone: '5876543210' }).phone).toBeTruthy();
     expect(validateOnboarding({ ...base, email: 'Ops <a@b.co>' }).email).toBeTruthy();
     expect(validateOnboarding({ ...base, password: 'a@b.co', confirm: 'a@b.co' }).password).toBeTruthy();

tests/lib/agentActions.test.js

@@ -38,16 +38,31 @@ beforeEach(() => {
 
 describe('which proposals can be executed', () => {
   it('only verbs with a real endpoint behind them', () => {
-    expect(executableTools().sort()).toEqual(['notifyRider']);
+    // alert_low_battery_rider joined notifyRider: it targets the same
+    // POST /admin/milers/:id/notify, so it was never a missing capability —
+    // just a verb nothing had been wired to.
+    expect(executableTools().sort()).toEqual([
+      'alert_low_battery_rider', 'assignMiler', 'notifyRider', 'trigger_auto_dispatch'
+    ]);
   });
 
   it('canExecuteProposal is false for the skills tools with no endpoint', () => {
-    ['trigger_auto_dispatch', 'enforce_cash_handoff', 'enforce_otp_verification', 'alert_low_battery_rider']
+    // Still review-only. Each needs an endpoint that does not exist yet —
+    // seed.go marks all three Target: "none yet".
+    ['enforce_cash_handoff', 'enforce_otp_verification', 'dispatch_hub_idle_parcels']
       .forEach((tool) => {
         expect(canExecuteProposal(finding(tool, [row()]))).toBe(false);
       });
   });
 
+  it('alert_low_battery_rider notifies the riders on the finding', async () => {
+    const result = await executeProposal(finding('alert_low_battery_rider', [row()]));
+    expect(result.ok).toBe(true);
+    // Keyed on milerprofileid, not userid — the two are different small
+    // integers on the same record and the wrong one fails silently.
+    expect(api.notifyRider).toHaveBeenCalledWith(99, expect.any(String), expect.stringMatching(/charge/i));
+  });
+
   // Resolving quietly would let a caller mistake "nothing happened" for success.
   it('throws rather than silently doing nothing', async () => {
     await expect(executeProposal(finding('enforce_cash_handoff', [row()]))).rejects.toThrow(/review-only/i);
@@ -130,18 +145,92 @@ describe('notifyRider', () => {
 });
 
 describe('assignMiler', () => {
-  // Deliberately review-only in the console. The branch executed it through
-  // POST /hub/bookings/batch-assign, which is behind HubStaffAuth (roleid 6
-  // only) — every admin/manager/executive click would 403. Until an admin
-  // batch-assign route exists, the card must say "Review only", not fail.
-  it('is not executable, so its card renders "Review only"', () => {
-    expect(canExecuteProposal(finding('assignMiler', [row({ bookingid: 1 })]))).toBe(false);
+  // This was review-only because POST /hub/bookings/batch-assign is behind
+  // HubStaffAuth (roleid 6 only) and 403s for every token this console issues.
+  // It is executable now that POST /admin/bookings/batch-assign runs the same
+  // solver under admin auth.
+  it('is executable', () => {
+    expect(canExecuteProposal(finding('assignMiler', [row({ bookingid: 1 })]))).toBe(true);
   });
 
-  it('throws instead of pretending, and never calls the hub-only endpoint', async () => {
-    await expect(executeProposal(finding('assignMiler', [row()]))).rejects.toThrow(/review-only/);
+  it('sends the findings distinct booking ids, deduplicated', async () => {
+    api.batchAssignBookings.mockResolvedValue({ assigned: 2, skipped: 0, riderssequenced: 1, results: [] });
+    await executeProposal(finding('assignMiler', [
+      row({ bookingid: 11 }), row({ bookingid: 12 }), row({ bookingid: 11 })
+    ]));
+    expect(api.batchAssignBookings).toHaveBeenCalledWith([11, 12]);
+  });
+
+  // The rule the whole module exists for: never report an action that did not
+  // happen. Four of nine landing is four of nine.
+  it('reports a partial assignment as partial, naming the reasons', async () => {
+    api.batchAssignBookings.mockResolvedValue({
+      assigned: 1, skipped: 1, riderssequenced: 1,
+      results: [
+        { bookingid: 11, bookingno: 'DM-11', assigned: true, mileruserid: 7 },
+        { bookingid: 12, bookingno: 'DM-12', assigned: false, reason: 'no available rider under capacity' }
+      ]
+    });
+    const res = await executeProposal(finding('assignMiler', [row({ bookingid: 11 }), row({ bookingid: 12 })]));
+    expect(res.ok).toBe(true);
+    // Declared so reportActed records "partial" rather than "ok". Caught by
+    // driving the real page: the message said partial, the telemetry said ok.
+    expect(res.partial).toBe(true);
+    expect(res.message).toMatch(/Assigned 1 order/);
+    expect(res.message).toMatch(/DM-12 \(no available rider under capacity\)/);
+  });
+
+  // The mocks above return a FLAT object, but batchAssignBookings returns
+  // `response.data` — and utils.OK wraps everything as {success, data}. So
+  // production hands the executor {data: {assigned, skipped, ...}}. Testing
+  // only the flat shape would leave the real one unproven.
+  it('reads the real utils.OK envelope, not just a flat object', async () => {
+    api.batchAssignBookings.mockResolvedValue({
+      success: true,
+      data: {
+        assigned: 1, skipped: 1, riderssequenced: 1,
+        results: [
+          { bookingid: 11, bookingno: 'DM-11', assigned: true, mileruserid: 7 },
+          { bookingid: 12, bookingno: 'DM-12', assigned: false, reason: 'no available rider under capacity' }
+        ]
+      }
+    });
+    const res = await executeProposal(finding('assignMiler', [row({ bookingid: 11 }), row({ bookingid: 12 })]));
+    expect(res.ok).toBe(true);
+    expect(res.message).toMatch(/Assigned 1 order/);
+    expect(res.message).toMatch(/DM-12/);
+  });
+
+  it('is not ok when nothing was assigned', async () => {
+    api.batchAssignBookings.mockResolvedValue({
+      assigned: 0, skipped: 1, riderssequenced: 0,
+      results: [{ bookingid: 12, bookingno: 'DM-12', assigned: false, reason: 'no available rider under capacity' }]
+    });
+    const res = await executeProposal(finding('assignMiler', [row({ bookingid: 12 })]));
+    expect(res.ok).toBe(false);
+    expect(res.sourceCalls[0].status).toBe('error');
+  });
+
+  // Stop sequencing needs ROUTE_OPTIMIZER_URL set in the cluster. Saying
+  // nothing when it is zero leaves the operator assuming routes were ordered.
+  it('says so when stops were not sequenced', async () => {
+    api.batchAssignBookings.mockResolvedValue({ assigned: 2, skipped: 0, riderssequenced: 0, results: [] });
+    const res = await executeProposal(finding('assignMiler', [row({ bookingid: 11 }), row({ bookingid: 12 })]));
+    expect(res.message).toMatch(/not sequenced/i);
+  });
+
+  it('refuses findings with no booking rather than calling the endpoint', async () => {
+    const res = await executeProposal(finding('assignMiler', [row({ bookingid: undefined })]));
+    expect(res.ok).toBe(false);
     expect(api.batchAssignBookings).not.toHaveBeenCalled();
   });
+
+  it('reports a thrown error instead of claiming success', async () => {
+    api.batchAssignBookings.mockRejectedValue(new Error('403 forbidden'));
+    const res = await executeProposal(finding('assignMiler', [row({ bookingid: 11 })]));
+    expect(res.ok).toBe(false);
+    expect(res.message).toMatch(/403/);
+  });
 });
 
 // The gap the tests above have: they mock buildMilerLookup, so the id

tests/lib/findingReport.test.js

@@ -0,0 +1,157 @@
+/**
+ * Finding persistence.
+ *
+ * The fingerprint is the part worth testing hardest. It decides whether a
+ * finding seen on two consecutive polls is the SAME finding — and if it is
+ * wrong, the "how long has this been open" measurement, which is the single
+ * most useful thing the table records, is destroyed.
+ */
+jest.mock('@/api/doormile', () => ({
+  reportFindings: jest.fn().mockResolvedValue({}),
+  reportFindingActed: jest.fn().mockResolvedValue({})
+}));
+
+const api = require('@/api/doormile');
+const { fingerprintOf, reportScan, reportActed, __resetForTests } =
+  require('@/lib/assistant/agent/findingReport');
+
+const finding = (skillId, bookingids, extra = {}) => ({
+  skillId,
+  severity: 'warning',
+  title: 'Something needs attention',
+  proposal: { tool: 'notifyRider', scope: bookingids.map((bookingid) => ({ bookingid })) },
+  ...extra
+});
+
+beforeEach(() => {
+  jest.clearAllMocks();
+  __resetForTests();
+  // The module reads tenantid from localStorage.
+  localStorage.setItem('tenantid', '7');
+});
+
+describe('fingerprintOf', () => {
+  it('is stable across scans regardless of scope order', () => {
+    // Scope order is not guaranteed between scans; an unsorted fingerprint
+    // would make the same problem look new every poll.
+    expect(fingerprintOf(finding('SlaGuardian', [3, 1, 2])))
+      .toBe(fingerprintOf(finding('SlaGuardian', [1, 2, 3])));
+  });
+
+  it('ignores severity, so an escalating finding stays the same finding', () => {
+    // An ageing SLA breach climbs warning -> critical while the underlying
+    // problem is unchanged. Including severity would reset firstseenat and
+    // report a six-hour-old problem as brand new.
+    const a = fingerprintOf(finding('SlaGuardian', [1], { severity: 'warning' }));
+    const b = fingerprintOf(finding('SlaGuardian', [1], { severity: 'critical' }));
+    expect(a).toBe(b);
+  });
+
+  it('distinguishes two proposals from ONE skill over the same bookings', () => {
+    // SlaGuardian raises both "notify the riders" and "assign these orders"
+    // for the same parcels. Keyed on skill + scope alone these collide, and
+    // since the fingerprint is the upsert key one would silently overwrite the
+    // other. Found by running the real page, not by these tests.
+    const notify = finding('SlaGuardian', [1, 2, 3]);
+    const assign = { ...finding('SlaGuardian', [1, 2, 3]), proposal: { tool: 'assignMiler', scope: [1, 2, 3].map((bookingid) => ({ bookingid })) } };
+    expect(fingerprintOf(notify)).not.toBe(fingerprintOf(assign));
+  });
+
+  it('distinguishes different skills on the same booking', () => {
+    expect(fingerprintOf(finding('SlaGuardian', [1])))
+      .not.toBe(fingerprintOf(finding('CashExposure', [1])));
+  });
+
+  it('distinguishes different scopes', () => {
+    expect(fingerprintOf(finding('SlaGuardian', [1])))
+      .not.toBe(fingerprintOf(finding('SlaGuardian', [1, 2])));
+  });
+
+  it('survives a missing scope rather than throwing', () => {
+    expect(fingerprintOf({ skillId: 'SlaGuardian' })).toBe('SlaGuardian:none:');
+    expect(fingerprintOf({})).toBe('unknown:none:');
+  });
+});
+
+describe('reportScan', () => {
+  it('sends ids only, never personal data', async () => {
+    await reportScan([finding('SlaGuardian', [11])]);
+    const payload = api.reportFindings.mock.calls[0][0];
+    const serialised = JSON.stringify(payload);
+    expect(payload.findings[0].bookingids).toEqual([11]);
+    // The findings table measures the skills; it is not a second copy of the
+    // bookings.
+    expect(serialised).not.toMatch(/phone|address|recipient|ridername/i);
+  });
+
+  it('reports what cleared since the previous scan', async () => {
+    await reportScan([finding('SlaGuardian', [1]), finding('CashExposure', [2])]);
+    api.reportFindings.mockClear();
+
+    await reportScan([finding('SlaGuardian', [1])]);
+    const payload = api.reportFindings.mock.calls[0][0];
+    expect(payload.cleared).toEqual(['CashExposure:notifyRider:2']);
+  });
+
+  it('skips the round trip when nothing is open and nothing cleared', async () => {
+    await reportScan([]);
+    expect(api.reportFindings).not.toHaveBeenCalled();
+  });
+
+  it('still reports when everything clears at once', async () => {
+    await reportScan([finding('SlaGuardian', [1])]);
+    api.reportFindings.mockClear();
+    await reportScan([]);
+    expect(api.reportFindings).toHaveBeenCalledWith({
+      findings: [], cleared: ['SlaGuardian:notifyRider:1']
+    });
+  });
+
+  it('never throws when the backend refuses', async () => {
+    api.reportFindings.mockRejectedValueOnce(new Error('403'));
+    // A dispatcher looking at a stalled parcel does not care that telemetry is
+    // down, and must not be shown an error about it.
+    await expect(reportScan([finding('SlaGuardian', [1])])).resolves.toBeUndefined();
+  });
+
+  it('retries the clear set after a failure instead of losing it', async () => {
+    await reportScan([finding('SlaGuardian', [1])]);
+    api.reportFindings.mockClear();
+
+    // The scan where it clears fails...
+    api.reportFindings.mockRejectedValueOnce(new Error('503'));
+    await reportScan([]);
+
+    // ...so the next scan must still report it cleared. Advancing `previous`
+    // on a failed post would drop the clear permanently and leave the finding
+    // open forever.
+    await reportScan([]);
+    const last = api.reportFindings.mock.calls.at(-1)[0];
+    expect(last.cleared).toEqual(['SlaGuardian:notifyRider:1']);
+  });
+});
+
+describe('reportActed', () => {
+  it('records a clean success as ok', async () => {
+    await reportActed(finding('SlaGuardian', [1]), { ok: true });
+    expect(api.reportFindingActed).toHaveBeenCalledWith('SlaGuardian:notifyRider:1', 'ok');
+  });
+
+  it('records a partial success as partial, not ok', async () => {
+    // Six riders notified out of eight is not a success. Flattening it would
+    // lose exactly the distinction actions.js preserves.
+    await reportActed(finding('SlaGuardian', [1, 2]), { ok: true, unreachable: ['DM-2 (no device)'] });
+    expect(api.reportFindingActed).toHaveBeenCalledWith('SlaGuardian:notifyRider:1,2', 'partial');
+  });
+
+  it('records a failure', async () => {
+    await reportActed(finding('SlaGuardian', [1]), { ok: false });
+    expect(api.reportFindingActed).toHaveBeenCalledWith('SlaGuardian:notifyRider:1', 'failed');
+  });
+
+  it('never throws when the backend refuses', async () => {
+    api.reportFindingActed.mockRejectedValueOnce(new Error('500'));
+    // The action itself already happened and was reported to the operator.
+    await expect(reportActed(finding('SlaGuardian', [1]), { ok: true })).resolves.toBeUndefined();
+  });
+});