Zuweisen: Warnungen statt Abbruch, Zuweisung unter der Projekt-Sperre

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

server/services/projectAssignment.js

@@ -1,11 +1,21 @@
 import { config } from '../config.js'
 import { moveRepoToOrg, WARN } from './repoMove.js'
+import { splitRepo } from './giteaClient.js'
 import { listRecentPushes } from './pushHistory.js'
 import { createGitPushWorksheet, isSystemPusher } from './gitWorksheet.js'
 import { withLocks } from './keyedLock.js'
 import { ensureProjectTicket } from './ticketAdmin.js'
 import { getDocument, updateDocument } from './appwriteAdmin.js'
 
+/**
+ * Warnungen dieses Moduls, woertlich (Controller-Ruling R14, Fix-Runde 1) - Anfuehrungszeichen
+ * wie die bestehenden Warnungen in repoMove.js (‚...‘ mit U+201A/U+2018).
+ */
+const WARN_BACKFILL_FAILED = (reason) =>
+  `Pushes konnten nicht nachgetragen werden (${reason}). Bitte später ‚Pushes nachtragen‘ klicken.`
+const WARN_RECONCILE_FAILED = (reason) =>
+  `Zuweisung gespeichert, aber Umzug und Nachtragen sind fehlgeschlagen (${reason}). Bitte später ‚Pushes nachtragen‘ klicken.`
+
 /**
  * Commit-Nachrichten (erste Zeile), die frueher das System mit Kensos Schluessel geschrieben hat
  * (Preview-Config, automatisch erstellte Preview-Seite) - siehe Plan, Abschnitt "Global
@@ -42,65 +52,132 @@ export async function backfillPushes(project, { now = new Date() } = {}) {
   try {
     pushes = await listRecentPushes(project.repoFullName, { now })
   } catch (err) {
-    return { backfilled: 0, warnings: [WARN.giteaDown(err.message)] }
+    return { backfilled: 0, warnings: [WARN.giteaDown(err.message)], ticketId: project.ticketId || '' }
   }
 
   const relevant = pushes.filter((push) => !isSystemPusher(push.pusher) && !isOldSystemCommitPush(push))
-  if (relevant.length === 0) return { backfilled: 0, warnings: [] }
-
-  const ticketId = project.ticketId || (await ensureProjectTicket(project, { repoFullName: project.repoFullName }))
-  if (!ticketId) return { backfilled: 0, warnings: [] }
+  if (relevant.length === 0) return { backfilled: 0, warnings: [], ticketId: project.ticketId || '' }
 
+  // `ensureProjectTicket` und die Arbeitsblatt-Schleife sind hier bewusst in EINEM try/catch
+  // (Controller-Ruling R14): schlaegt eine der beiden Appwrite-Schreiboperationen fehl (z. B.
+  // Ausfall waehrend POST workorders/worksheets), bleibt die Zuweisung trotzdem bestehen - siehe
+  // `assignProject`, das sich darauf verlaesst, dass `backfillPushes` nie wirft.
+  let ticketId = project.ticketId || ''
   let backfilled = 0
-  // Aelteste zuerst (siehe `listRecentPushes`), damit die Arbeitsblaetter in derselben
-  // Reihenfolge entstehen, in der die Pushes tatsaechlich passiert sind.
-  for (const push of relevant) {
-    const result = await createGitPushWorksheet({ ticketId, repoFullName: project.repoFullName, ...push })
-    if (result?.created) backfilled += 1
+  try {
+    ticketId = ticketId || (await ensureProjectTicket(project, { repoFullName: project.repoFullName }))
+    if (!ticketId) return { backfilled: 0, warnings: [], ticketId: '' }
+
+    // Aelteste zuerst (siehe `listRecentPushes`), damit die Arbeitsblaetter in derselben
+    // Reihenfolge entstehen, in der die Pushes tatsaechlich passiert sind.
+    for (const push of relevant) {
+      const result = await createGitPushWorksheet({ ticketId, repoFullName: project.repoFullName, ...push })
+      if (result?.created) backfilled += 1
+    }
+    return { backfilled, warnings: [], ticketId }
+  } catch (err) {
+    return { backfilled, warnings: [WARN_BACKFILL_FAILED(err.message)], ticketId }
   }
-  return { backfilled, warnings: [] }
 }
 
 /**
+ * Kern von `reconcileProject`, OHNE selbst die Sperre zu nehmen - fuer `assignProject`, das
+ * seine eigene Lese-Pruef-Schreib-Folge und diesen Kern unter DERSELBEN Sperre laufen laesst
+ * (Controller-Ruling R15), statt die Sperre zweimal zu nehmen. `reconcileProject` (unten) bleibt
+ * der oeffentliche, selbst sperrende Einstieg (z. B. fuer die Migration, Task 9).
+ *
  * Liest das Projekt, verschiebt sein Repo bei Bedarf nach `GITEA_ORG` und traegt danach dessen
- * Pushes nach - alles unter der Projekt-Sperre, damit ein Doppelklick auf "Zuweisen" (oder ein
- * gleichzeitiger Aufruf ueber die Migration, Task 9) nicht zweimal umzieht oder doppelte
- * Arbeitsblaetter erzeugt (Review Focus 3, Plan). Das Projektdokument wird ERST innerhalb der
- * Sperre gelesen, damit ein zweiter, gleichzeitiger Aufruf nie mit einem inzwischen veralteten
- * Objekt arbeitet.
+ * Pushes nach. Wirft nur noch, wenn schon das ERSTE Lesen des Projekts fehlschlaegt (dann gibt es
+ * nichts Sinnvolles zu verschieben/nachzutragen) - `assignProject` faengt das ab (R14).
  */
-export async function reconcileProject(projectId, { now } = {}) {
-  return withLocks(['project:' + projectId], async () => {
-    let project = await getDocument(config.collections.websiteProjects, projectId)
-    const { moved, warnings: moveWarnings } = await moveRepoToOrg(project)
+async function reconcileProjectLocked(projectId, { now } = {}) {
+  let project = await getDocument(config.collections.websiteProjects, projectId)
+  const { moved, warnings: moveWarnings } = await moveRepoToOrg(project)
 
+  try {
     // Immer neu lesen, nicht nur wenn `moved` gesetzt ist: der Selbstheilungs-Zweig in
     // `moveRepoToOrg` (von Hand verschobenes/umbenanntes Repo) kann das Dokument umbenennen und
     // dabei trotzdem `moved: null` liefern (Controller-Ruling R13).
     project = await getDocument(config.collections.websiteProjects, projectId)
+  } catch {
+    // R14: Schlaegt das Nachlesen fehl, mit dem VOR dem Umzug gelesenen Dokument weitermachen -
+    // ist tatsaechlich etwas verschoben worden, von Hand dessen `repoFullName`/`giteaRepoUrl`/
+    // `giteaRepoName` nachziehen (siehe `updateProjectDocument` in repoMove.js). Keine zusaetzliche
+    // Warnung - `moveWarnings` deckt einen etwaigen Gitea-Ausfall bereits ab.
+    if (moved) {
+      project = {
+        ...project,
+        repoFullName: moved.to,
+        giteaRepoUrl: `${config.gitea.baseUrl}/${moved.to}`,
+        giteaRepoName: splitRepo(moved.to)[1],
+      }
+    }
+  }
 
-    const { backfilled, warnings: backfillWarnings } = await backfillPushes(project, { now })
+  const { backfilled, warnings: backfillWarnings, ticketId } = await backfillPushes(project, { now })
+  // R15: die Antwort soll die (evtl. gerade erst automatisch angelegte) ticketId zeigen, auch
+  // wenn `project` oben vor dem Anlegen gelesen wurde.
+  if (ticketId && project.ticketId !== ticketId) project = { ...project, ticketId }
 
-    return { project, moved, backfilled, warnings: [...moveWarnings, ...backfillWarnings] }
-  })
+  return { project, moved, backfilled, warnings: [...moveWarnings, ...backfillWarnings] }
 }
 
 /**
- * Weist ein Projekt einem Kunden/Ticket zu und stoesst danach Umzug + Nachtragen an. Die
- * Zuweisung selbst wird ZUERST und ausserhalb der Projekt-Sperre gespeichert (reines Appwrite,
- * ohne Gitea) - sie bleibt deshalb bestehen, selbst wenn Gitea gerade nicht erreichbar ist
- * (siehe `reconcileProject`/`moveRepoToOrg`, die deshalb nie wegen Gitea werfen duerfen).
+ * Oeffentlicher, selbst sperrender Einstieg fuer den Kern oben - damit ein Doppelklick auf
+ * "Zuweisen" (oder ein gleichzeitiger Aufruf ueber die Migration, Task 9) nicht zweimal umzieht
+ * oder doppelte Arbeitsblaetter erzeugt (Review Focus 3, Plan).
+ */
+export async function reconcileProject(projectId, { now } = {}) {
+  return withLocks(['project:' + projectId], () => reconcileProjectLocked(projectId, { now }))
+}
+
+/**
+ * Ticket-Id fuer die Zuweisung (Controller-Ruling R15): eine mitgegebene `ticketId` gewinnt immer.
+ * Ist sie leer UND bleibt die Kunden-Id gleich, bleibt die gespeicherte `ticketId` erhalten - ein
+ * Entfernen laeuft ueber "Zuweisung aufheben", nicht ueber diesen Endpunkt (sonst loescht ein
+ * zweiter Klick ohne `ticketId` ein zwischenzeitlich automatisch angelegtes Ticket, Review I3).
+ * Aendert sich die Kunden-Id und wird keine `ticketId` mitgegeben, wird sie geleert.
+ */
+function resolveTicketId(payloadTicketId, customerId, existing) {
+  if (payloadTicketId) return payloadTicketId
+  if (customerId === existing.customerId) return existing.ticketId || ''
+  return ''
+}
+
+/**
+ * Weist ein Projekt einem Kunden/Ticket zu und stoesst danach Umzug + Nachtragen an. Laeuft
+ * komplett unter der Projekt-Sperre (Controller-Ruling R15): Lesen (fuer die 404-Pruefung und die
+ * `ticketId`-Aufloesung), Schreiben der Zuweisung und der Reconcile-Kern teilen sich dieselbe
+ * Sperre, statt sie zweimal zu nehmen - ein zweiter, gleichzeitiger Aufruf sieht daher nie einen
+ * veralteten Stand. Ein unbekanntes Projekt wirft weiterhin (`.status = 404`) BEVOR irgendetwas
+ * geschrieben wird. Schlaegt danach der Reconcile-Kern (Umzug/Nachtragen) mit etwas anderem als
+ * einem 404 fehl, bleibt die Zuweisung trotzdem gespeichert - Ruling R14 verlangt eine Warnung
+ * statt eines Abbruchs, denn Spec 5.2 verlangt genau das.
  */
 export async function assignProject(projectId, { customerId = '', ticketId = '' }, { now } = {}) {
-  // 404 fuer ein unbekanntes Projekt, bevor ueberhaupt geschrieben wird.
-  await getDocument(config.collections.websiteProjects, projectId)
+  return withLocks(['project:' + projectId], async () => {
+    // 404 fuer ein unbekanntes Projekt, bevor ueberhaupt geschrieben wird.
+    const existing = await getDocument(config.collections.websiteProjects, projectId)
 
-  await updateDocument(config.collections.websiteProjects, projectId, {
-    customerId,
-    ticketId,
-    updatedAt: new Date().toISOString(),
+    const nextTicketId = resolveTicketId(ticketId, customerId, existing)
+    const saved = await updateDocument(config.collections.websiteProjects, projectId, {
+      customerId,
+      ticketId: nextTicketId,
+      updatedAt: new Date().toISOString(),
+    })
+
+    try {
+      const result = await reconcileProjectLocked(projectId, { now })
+      return { ok: true, ...result }
+    } catch (err) {
+      if (err.status === 404) throw err
+      return {
+        ok: true,
+        project: saved,
+        moved: null,
+        backfilled: 0,
+        warnings: [WARN_RECONCILE_FAILED(err.message)],
+      }
+    }
   })
-
-  const { project, moved, backfilled, warnings } = await reconcileProject(projectId, { now })
-  return { ok: true, project, moved, backfilled, warnings }
 }

test/projectAssignment.test.js

@@ -43,6 +43,22 @@ function pushEntry({ sha, at, user, msg }) {
   }
 }
 
+/**
+ * Wartet, bis `aw.calls` mindestens `count` Eintraege hat - deterministisch, kein fester Timeout.
+ * `calls.push` passiert server-seitig, SOBALD eine Anfrage beim Fake-Appwrite ankommt - also
+ * strikt BEVOR die Antwort beim Aufrufer eintrifft und dieser (falls ueberhaupt) die naechste
+ * Anfrage lostritt. Damit laesst sich `failNext` gezielt auf einen bestimmten von mehreren gleich
+ * aussehenden Aufrufen ansetzen (z. B. das zweite GET desselben Dokuments), ohne zu raten.
+ */
+function waitForAppwriteCalls(count) {
+  return new Promise((resolve) => {
+    ;(function check() {
+      if (aw.calls.length >= count) return resolve()
+      setImmediate(check)
+    })()
+  })
+}
+
 before(async () => {
   gitea = await startFakeGitea({
     repos: [
@@ -50,9 +66,15 @@ before(async () => {
       { full_name: 'JUSN/leer' },
       { full_name: 'JUSN/nur-kunde' },
       { full_name: 'JUSN/doppelt' },
+      { full_name: 'WEBklar/ticket-fail' },
+      { full_name: 'WEBklar/worksheet-fail' },
+      { full_name: 'JUSN/reread-fail' },
+      { full_name: 'WEBklar/inlock-fail' },
     ],
     feeds: {
       // Newest first, wie der echte Gitea-Feed.
+      'WEBklar/ticket-fail': [pushEntry({ sha: 'tf1', user: 'JUSN', at: '2026-09-27T10:00:00Z', msg: 'Ticket-Fail-Push' })],
+      'WEBklar/worksheet-fail': [pushEntry({ sha: 'wf1', user: 'JUSN', at: '2026-09-27T10:00:00Z', msg: 'Worksheet-Fail-Push' })],
       'JUSN/D-ner-Nation': [
         pushEntry({ sha: 's1', user: 'webklar-system', at: '2026-09-27T14:50:00Z', msg: 'System-Push' }),
         pushEntry({ sha: 'k1', user: 'knso', at: '2026-09-27T14:48:00Z', msg: 'Update preview config from ticket system' }),
@@ -73,6 +95,10 @@ before(async () => {
         { $id: 'p2', repoFullName: 'JUSN/leer', projectName: 'leer', customerId: '', ticketId: '' },
         { $id: 'p3', repoFullName: 'JUSN/nur-kunde', projectName: 'nur-kunde', customerId: '', ticketId: '' },
         { $id: 'p4', repoFullName: 'JUSN/doppelt', projectName: 'doppelt', customerId: '', ticketId: '' },
+        { $id: 'p5', repoFullName: 'WEBklar/ticket-fail', projectName: 'ticket-fail', customerId: '', ticketId: '' },
+        { $id: 'p6', repoFullName: 'WEBklar/worksheet-fail', projectName: 'worksheet-fail', customerId: '', ticketId: '' },
+        { $id: 'p7', repoFullName: 'JUSN/reread-fail', projectName: 'reread-fail', customerId: '', ticketId: '' },
+        { $id: 'p8', repoFullName: 'WEBklar/inlock-fail', projectName: 'inlock-fail', customerId: '', ticketId: '' },
       ],
       workorders: [{ $id: 't1', woid: '100115' }],
       customers: [{ $id: 'c1', name: 'Döner Nation' }],
@@ -123,9 +149,34 @@ test('nur Kunde ohne Pushes: kein Ticket', async () => {
   assert.equal(aw.all('workorders').length, before)
 })
 test('nur Kunde mit Pushes: Webpage-Ticket entsteht', async () => {
-  const r = await assignProject('p3', { customerId: 'c1' }, { now })
-  const t = aw.all('workorders').find((w) => w.$id === aw.get('websiteProjects', 'p3').ticketId)
+  // ticketAdmin loggt die automatische Ticket-Anlage (console.log) - hier abfangen, damit die
+  // Testausgabe sauber bleibt (Ruling, Fix-Runde 1).
+  const originalLog = console.log
+  console.log = () => {}
+  let r
+  try {
+    r = await assignProject('p3', { customerId: 'c1' }, { now })
+  } finally {
+    console.log = originalLog
+  }
+  const stored = aw.get('websiteProjects', 'p3')
+  const t = aw.all('workorders').find((w) => w.$id === stored.ticketId)
   assert.equal(t.type, 'Webpage'); assert.equal(r.backfilled, 1)
+  // I2: die Antwort muss die gerade erst automatisch angelegte ticketId zeigen, nicht den Stand
+  // von vor dem Anlegen.
+  assert.equal(r.project.ticketId, stored.ticketId)
+})
+test('Kunde erneut zuweisen ohne Ticket: kein zweites Ticket, kein doppeltes Arbeitsblatt', async () => {
+  // p3 hat aus dem vorigen Test bereits ein automatisch angelegtes Ticket - ein verzoegerter
+  // zweiter Klick (gleicher Kunde, keine ticketId) darf es nicht durch ein leeres ueberschreiben
+  // (Review I3). "Verzoegert" reicht hier als einfaches Abwarten des ersten Aufrufs.
+  const before = { workorders: aw.all('workorders').length, worksheets: aw.all('worksheets').length }
+  const ticketIdBefore = aw.get('websiteProjects', 'p3').ticketId
+  const r = await assignProject('p3', { customerId: 'c1' }, { now })
+  assert.equal(aw.all('workorders').length, before.workorders)
+  assert.equal(aw.all('worksheets').length, before.worksheets)
+  assert.equal(aw.get('websiteProjects', 'p3').ticketId, ticketIdBefore)
+  assert.equal(r.project.ticketId, ticketIdBefore)
 })
 test('Doppelklick: ein Umzug, keine doppelten Arbeitsblätter', async () => {
   await Promise.all([assignProject('p4', { customerId: 'c1', ticketId: 't1' }, { now }), assignProject('p4', { customerId: 'c1', ticketId: 't1' }, { now })])
@@ -155,3 +206,48 @@ test('Gitea-Ausfall: Zuweisung bleibt', async () => {
 test('unbekanntes Projekt', async () => {
   await assert.rejects(assignProject('nope', { ticketId: 't1' }, { now }), (e) => e.status === 404)
 })
+test('Appwrite-Ausfall beim Ticket-Anlegen: Zuweisung bleibt, Warnung statt Abbruch', async () => {
+  aw.failNext({ method: 'POST', pathIncludes: 'workorders', status: 500 })
+  const r = await assignProject('p5', { customerId: 'c1' }, { now })
+  assert.equal(r.ok, true)
+  assert.equal(aw.get('websiteProjects', 'p5').customerId, 'c1')
+  assert.match(r.warnings[0], /^Pushes konnten nicht nachgetragen werden \(/)
+})
+test('Appwrite-Ausfall beim Arbeitsblatt-Anlegen: Zuweisung bleibt, Warnung statt Abbruch', async () => {
+  aw.failNext({ method: 'POST', pathIncludes: 'worksheets', status: 500 })
+  const r = await assignProject('p6', { customerId: 'c1', ticketId: 't1' }, { now })
+  assert.equal(r.ok, true)
+  assert.equal(aw.get('websiteProjects', 'p6').ticketId, 't1')
+  assert.match(r.warnings[0], /^Pushes konnten nicht nachgetragen werden \(/)
+})
+test('Nachlesen nach Umzug schlaegt fehl: Antwort zeigt trotzdem den neuen Namen', async () => {
+  // p7 zieht hier tatsaechlich um (JUSN -> WEBklar); das GET danach (Controller-Ruling R13/R14)
+  // soll scheitern. Gezielt getimt: warten, bis das GET VOR dem Umzug und das PATCH AUS dem
+  // Umzug (`updateProjectDocument` in repoMove.js) durch sind, dann erst failNext scharf machen -
+  // so trifft es sicher das GET NACH dem Umzug und keines der beiden vorherigen.
+  const target = aw.calls.length + 2
+  const p = reconcileProject('p7', { now })
+  await waitForAppwriteCalls(target)
+  aw.failNext({ method: 'GET', pathIncludes: 'p7', status: 500 })
+  const r = await p
+  assert.deepEqual(r.moved, { from: 'JUSN/reread-fail', to: 'WEBklar/reread-fail' })
+  assert.deepEqual(r.warnings, [])
+  assert.equal(r.project.repoFullName, 'WEBklar/reread-fail')
+  assert.equal(r.project.giteaRepoUrl, `${gitea.baseUrl}/WEBklar/reread-fail`)
+  assert.equal(r.project.giteaRepoName, 'reread-fail')
+})
+test('Appwrite-Fehler beim Nachlesen unter der Sperre in assignProject: Zuweisung bleibt, Warnung statt Abbruch', async () => {
+  // Gezielt getimt wie oben: warten, bis die Existenzpruefung und das Speichern der Zuweisung
+  // (beide in `assignProject`) durch sind, dann erst failNext scharf machen - so trifft es sicher
+  // das GET, das der Reconcile-Kern (`reconcileProjectLocked`) direkt danach selbst macht.
+  const target = aw.calls.length + 2
+  const p = assignProject('p8', { customerId: 'c1' }, { now })
+  await waitForAppwriteCalls(target)
+  aw.failNext({ method: 'GET', pathIncludes: 'p8', status: 500 })
+  const r = await p
+  assert.equal(r.ok, true)
+  assert.equal(r.moved, null)
+  assert.equal(r.backfilled, 0)
+  assert.equal(aw.get('websiteProjects', 'p8').customerId, 'c1')
+  assert.match(r.warnings[0], /^Zuweisung gespeichert, aber Umzug und Nachtragen sind fehlgeschlagen \(/)
+})