fix(sync): never import sheet rows into a userless group (Refs #184)

src/bot/commands/reconnect.test.ts

@@ -146,13 +146,17 @@ mock.module('../../services/currency/converter', () => ({
   formatAmount: (x: number, c: string) => `${x} ${c}`,
 }));
 
-const importExpensesFromSheetMock = mock(async (..._args: unknown[]): Promise<number> => 0);
-
+const importExpensesFromSheetMock = mock(
+  async (..._args: unknown[]): Promise<{ inserted: number; skippedNoOwner: number }> => ({
+    inserted: 0,
+    skippedNoOwner: 0,
+  }),
+);
 mock.module('./sync', () => ({
   importExpensesFromSheet: importExpensesFromSheetMock,
 }));
 
-const { handleReconnectCommand } = await import('./reconnect');
+const { handleReconnectCommand, formatFullSyncReport } = await import('./reconnect');
 
 let db: SqliteDb;
 let groups: GroupRepository;
@@ -577,3 +581,28 @@ describe('/reconnect budget import from sheet', () => {
     expect(hasChanged).toBe(false);
   });
 });
+
+describe('/reconnect full sync report', () => {
+  // #184: sheet rows skipped for a userless group must be reported, not silently dropped.
+  test('skipped-no-owner rows are listed in the user-facing report', () => {
+    const report = formatFullSyncReport({
+      snapshotId: null,
+      snapshotExpenses: 0,
+      snapshotBudgets: 0,
+      sheetBackupUrl: null,
+      yearCreated: null,
+      sheetToDbExpenses: 0,
+      sheetRowsSkippedNoOwner: 3,
+      dbToSheetExpenses: 0,
+      sheetToBudgets: 0,
+      budgetTabsCreated: [],
+      budgetRowsWritten: 0,
+      audit: [],
+      recreated: [],
+    });
+    expect(report).toContain('пропущено 3 строки');
+    expect(report).toContain('в группе нет ни одного участника');
+    // The skip IS the outcome: the generic «синхронизированы» must not mask it (#184).
+    expect(report).not.toContain('расходы синхронизированы');
+  });
+});

src/bot/commands/reconnect.ts

@@ -102,6 +102,8 @@ interface FullSyncReport {
   sheetBackupUrl: string | null;
   yearCreated: number | null;
   sheetToDbExpenses: number;
+  /** Sheet rows skipped because the group had no user to own them (#184). */
+  sheetRowsSkippedNoOwner: number;
   dbToSheetExpenses: number;
   sheetToBudgets: number;
   budgetTabsCreated: string[];
@@ -205,6 +207,7 @@ export async function fullSyncAfterReconnect(groupId: number): Promise<void> {
     sheetBackupUrl: null,
     yearCreated: null,
     sheetToDbExpenses: 0,
+    sheetRowsSkippedNoOwner: 0,
     dbToSheetExpenses: 0,
     sheetToBudgets: 0,
     budgetTabsCreated: [],
@@ -278,11 +281,13 @@ export async function fullSyncAfterReconnect(groupId: number): Promise<void> {
     report.yearCreated = await ensureCurrentYearSpreadsheet(freshGroup);
 
     // Step 3: Sheet → DB expenses (add-only, never deletes)
-    report.sheetToDbExpenses = await importExpensesFromSheet(
+    const importResult = await importExpensesFromSheet(
       freshGroup.id,
       conn,
       freshGroup.spreadsheet_id,
     );
+    report.sheetToDbExpenses = importResult.inserted;
+    report.sheetRowsSkippedNoOwner = importResult.skippedNoOwner;
 
     // Step 4: DB → Sheet expenses (push missing)
     report.dbToSheetExpenses = await pushMissingExpensesToSheet(freshGroup);
@@ -674,6 +679,12 @@ function formatExpenseSyncSection(report: FullSyncReport): string[] {
     lines.push(
       `\n📥 Таблица → БД: +${report.sheetToDbExpenses} ${pluralize(report.sheetToDbExpenses, 'расход добавлен', 'расхода добавлено', 'расходов добавлено')}`,
     );
+  } else if (report.sheetRowsSkippedNoOwner > 0) {
+    // The primary #184 case (0 imported, N skipped): the skip IS the outcome — saying
+    // «синхронизированы» would obscure that nothing was imported.
+    lines.push(
+      `⏭️ Таблица → БД: пропущено ${report.sheetRowsSkippedNoOwner} ${pluralize(report.sheetRowsSkippedNoOwner, 'строка', 'строки', 'строк')} — в группе нет ни одного участника; они добавятся после того, как кто-нибудь сделает /start`,
+    );
   } else {
     lines.push('\n📥 Таблица → БД: расходы синхронизированы');
   }
@@ -709,7 +720,7 @@ function formatBudgetSyncSection(report: FullSyncReport): string[] {
   return lines;
 }
 
-function formatFullSyncReport(report: FullSyncReport): string {
+export function formatFullSyncReport(report: FullSyncReport): string {
   const lines: string[] = ['✅ Синхронизация завершена!\n'];
   lines.push(...formatBackupSection(report));
   lines.push(...formatRecreatedSection(report));

src/bot/commands/sync-expenses.test.ts

@@ -37,7 +37,8 @@ const { DatabaseService } = await import('../../database/index');
 const service = new DatabaseService(db);
 mock.module('../../database', () => ({ database: service, _budgetWriter: () => service.budgets }));
 
-const { ensureFreshExpenses, handleSyncCommand, syncExpenses } = await import('./sync');
+const { ensureFreshExpenses, handleSyncCommand, importExpensesFromSheet, syncExpenses } =
+  await import('./sync');
 const { executeTool } = await import('../../services/ai/tool-executor');
 
 afterAll(() => db.close());
@@ -204,3 +205,29 @@ describe('sync reports name the skipped rows', () => {
     expect(result.output?.length).toBeLessThan(500);
   });
 });
+
+describe('importExpensesFromSheet ownership fallback', () => {
+  // users.id is global and the FK does not check group_id: a fallback to user 1 would hand the
+  // imported expense to another group's user whenever one exists (same hazard as /sync).
+  test("a userless group imports nothing, never rows owned by another group's user 1", async () => {
+    db.exec("DELETE FROM sqlite_sequence WHERE name = 'users'");
+    const otherGroupId = connectedGroup();
+    const otherUser = service.users.create({ telegram_id: Date.now(), group_id: otherGroupId });
+    expect(otherUser.id).toBe(1);
+    const groupId = connectedGroup();
+    sheetRows = [sheetRow('imported row')];
+
+    const result = await importExpensesFromSheet(
+      groupId,
+      { refreshToken: 'tok', oauthClient: 'current' } as never,
+      'SHEET',
+    );
+
+    expect(result.inserted).toBe(0);
+    // The caller (the /reconnect report) shows the skipped count to the user (#184).
+    expect(result.skippedNoOwner).toBe(1);
+    expect(service.expenses.findByGroupId(groupId, 100)).toEqual([]);
+    expect(logMock.warn).toHaveBeenCalled();
+    expect(logMock.error).not.toHaveBeenCalled();
+  });
+});

src/bot/commands/sync.ts

@@ -394,24 +394,40 @@ function appendResultSection<T>(
   if (items.length > 10) lines.push(`  ...и ещё ${items.length - 10}`);
 }
 
+export interface SheetImportResult {
+  /** Rows added to the DB. */
+  inserted: number;
+  /** Rows skipped because the group has no user to own them — never a global fallback id (#184). */
+  skippedNoOwner: number;
+}
+
 /**
  * One-directional import: read expenses from an arbitrary spreadsheet and add any
- * that are missing from the DB. Never deletes. Returns the number of inserted rows.
+ * that are missing from the DB. Never deletes. Rows are skipped (and counted) when
+ * the group has no user to own them, so callers can report it (#184).
  * Safe to call repeatedly — deduplicates by date|category|amount|currency key.
  */
 export async function importExpensesFromSheet(
   groupId: number,
   conn: GoogleConn,
   spreadsheetId: string,
-): Promise<number> {
+): Promise<SheetImportResult> {
   const { expenses: sheetExpenses } = await readExpensesFromSheet(conn, spreadsheetId);
-  if (sheetExpenses.length === 0) return 0;
+  if (sheetExpenses.length === 0) return { inserted: 0, skippedNoOwner: 0 };
 
   const dbExpenses = database.expenses.findByGroupId(groupId, 100000);
   const existingKeys = new Set(dbExpenses.map((e) => dbExpenseToKey(e)));
 
   const users = database.users.findByGroupId ? database.users.findByGroupId(groupId) : [];
-  const defaultUserId = users[0]?.id ?? 1;
+  // No fallback id: users.id is global, so any constant would hand rows to another group's user.
+  const defaultUserId = users[0]?.id;
+  if (defaultUserId === undefined) {
+    logger.warn(
+      { groupId, rows: sheetExpenses.length },
+      '[SYNC] Sheet import skipped: the group has no user to own the rows',
+    );
+    return { inserted: 0, skippedNoOwner: sheetExpenses.length };
+  }
 
   let inserted = 0;
   for (const row of sheetExpenses) {
@@ -432,7 +448,7 @@ export async function importExpensesFromSheet(
     existingKeys.add(parsed.key); // guard against within-loop duplicates
     inserted++;
   }
-  return inserted;
+  return { inserted, skippedNoOwner: 0 };
 }
 
 // ── Auto-sync with cooldown ──

src/bot/cron.test.ts

@@ -123,7 +123,11 @@ mock.module('./services/budget-sync', () => ({
 }));
 
 const importExpensesFromSheetMock = mock(
-  async (_token: string, _groupId: number, _spreadsheetId: string): Promise<number> => 0,
+  async (
+    _token: string,
+    _groupId: number,
+    _spreadsheetId: string,
+  ): Promise<{ inserted: number; skippedNoOwner: number }> => ({ inserted: 0, skippedNoOwner: 0 }),
 );
 mock.module('./commands/sync', () => ({
   importExpensesFromSheet: importExpensesFromSheetMock,
@@ -686,7 +690,7 @@ describe('recoverPriorYearExpenses', () => {
   beforeEach(() => {
     mockGroupsRepo.findAll.mockReset().mockReturnValue([]);
     mockGroupSpreadsheets.listAll.mockReset().mockReturnValue([]);
-    importExpensesFromSheetMock.mockReset().mockResolvedValue(0);
+    importExpensesFromSheetMock.mockReset().mockResolvedValue({ inserted: 0, skippedNoOwner: 0 });
     logMock.error.mockReset();
     logMock.info.mockReset();
   });
@@ -728,14 +732,36 @@ describe('recoverPriorYearExpenses', () => {
     mockGroupSpreadsheets.listAll.mockReturnValue([
       { year: new Date().getFullYear() - 1, spreadsheetId: 'sid-prev' },
     ]);
-    importExpensesFromSheetMock.mockResolvedValue(7);
+    importExpensesFromSheetMock.mockResolvedValue({ inserted: 7, skippedNoOwner: 0 });
 
     await recoverPriorYearExpenses();
 
     const infoCalls = logMock.info.mock.calls.map((c) => JSON.stringify(c));
     expect(infoCalls.some((c) => c.includes('Restored 7'))).toBe(true);
   });
 
+  it('warns when a userless group forced the import to skip rows (#184)', async () => {
+    mockGroupsRepo.findAll.mockReturnValue([
+      {
+        id: 1,
+        telegram_group_id: -100,
+        google_refresh_token: 'tok',
+        default_currency: 'EUR',
+        enabled_currencies: ['EUR'],
+        active_topic_id: null,
+      },
+    ]);
+    mockGroupSpreadsheets.listAll.mockReturnValue([
+      { year: new Date().getFullYear() - 1, spreadsheetId: 'sid-prev' },
+    ]);
+    importExpensesFromSheetMock.mockResolvedValue({ inserted: 0, skippedNoOwner: 4 });
+
+    await recoverPriorYearExpenses();
+
+    const warnCalls = logMock.warn.mock.calls.map((c) => JSON.stringify(c));
+    expect(warnCalls.some((c) => c.includes('skipped 4 row(s)'))).toBe(true);
+  });
+
   it('logs and continues on per-sheet import failure', async () => {
     mockGroupsRepo.findAll.mockReturnValue([
       {
@@ -753,7 +779,7 @@ describe('recoverPriorYearExpenses', () => {
     ]);
     importExpensesFromSheetMock
       .mockRejectedValueOnce(new Error('OAuth expired'))
-      .mockResolvedValueOnce(3);
+      .mockResolvedValueOnce({ inserted: 3, skippedNoOwner: 0 });
 
     await recoverPriorYearExpenses();
 

src/bot/cron.ts

@@ -241,11 +241,20 @@ export async function recoverPriorYearExpenses(): Promise<void> {
 
     for (const { year, spreadsheetId } of priorSheets) {
       try {
-        const inserted = await importExpensesFromSheet(group.id, googleConn(group), spreadsheetId);
+        const { inserted, skippedNoOwner } = await importExpensesFromSheet(
+          group.id,
+          googleConn(group),
+          spreadsheetId,
+        );
         if (inserted > 0) {
           logger.info(
             `[RECOVER] Restored ${inserted} expense(s) for group ${group.id}, year ${year}`,
           );
+        } else if (skippedNoOwner > 0) {
+          // No user to report to and none to own the rows: the operator trace (#184).
+          logger.warn(
+            `[RECOVER] group ${group.id}, year ${year}: skipped ${skippedNoOwner} row(s) — no user in the group`,
+          );
         } else {
           logger.info(`[RECOVER] group ${group.id}, year ${year}: nothing missing`);
         }