fix: resolve approval queue and catalog Sonar findings

gh-pages-template/assets/js/app.js

@@ -232,7 +232,8 @@ function boot() {
         url.searchParams.set('kind', item.kind);
         url.searchParams.set('id', String(item.id));
         globalThis.history.pushState(null, '', url);
-        showRecord(item);
+        // Record loading reports failures in the detail panel.
+        void showRecord(item);
       });
       column.append(button);
       list.append(column);

src/approval-queue.js

@@ -61,9 +61,11 @@ async function releaseAndPromote({ github, context, issueNumber = context.issue.
   const params = { ...repoParams(context), issue_number: issueNumber };
   const { data: issue } = await github.rest.issues.get(params);
   const labels = labelNames(issue);
-  const labelsToRemove = issue.state === 'closed' ? [QUEUE_LABEL] : [APPROVE_LABEL, QUEUE_LABEL];
-  for (const label of labelsToRemove) {
-    if (labels.has(label)) await github.rest.issues.removeLabel({ ...params, name: label });
+  if (issue.state !== 'closed' && labels.has(APPROVE_LABEL)) {
+    await github.rest.issues.removeLabel({ ...params, name: APPROVE_LABEL });
+  }
+  if (labels.has(QUEUE_LABEL)) {
+    await github.rest.issues.removeLabel({ ...params, name: QUEUE_LABEL });
   }
   if (await hasActiveApproval({ github, context, issueNumber })) return null;
   const queued = await listOpenIssuesWithLabel({ github, context, label: QUEUE_LABEL });

src/workflow-queue.js

@@ -2,7 +2,7 @@
 
 async function waitForOlderApprovals({ github, context, intervalMs = 30000, timeoutMs = 3600000 }) {
   const started = Date.now();
-  while (true) {
+  async function pollOlderApprovals() {
     const runs = await github.paginate(github.rest.actions.listWorkflowRuns, {
       ...context.repo, workflow_id: 'approve-preset.yml', status: 'in_progress', per_page: 100
     });
@@ -13,7 +13,9 @@ async function waitForOlderApprovals({ github, context, intervalMs = 30000, time
       throw new Error(`Timed out waiting for older approval run ${older[0].id}`);
     }
     await new Promise(resolve => setTimeout(resolve, intervalMs));
+    return pollOlderApprovals();
   }
+  return pollOlderApprovals();
 }
 
 module.exports = { waitForOlderApprovals };

tests/approval-queue.test.js

@@ -67,6 +67,59 @@ test('approval labels serialize requests and promote the oldest queued issue', a
   assert.ok(actions.some(action => action[0] === 'add' && action[1] === 2 && action[2] === 'approve-preset'));
 });
 
+test('releasing an open approval removes both labels in order before promoting the next issue', async () => {
+  const current = issue(1, '2026-01-01T00:00:00Z');
+  const queued = issue(2, '2026-01-02T00:00:00Z');
+  current.labels.push({ name: 'approve-preset' }, { name: 'approve-queue' });
+  queued.labels.push({ name: 'approve-queue' });
+  const { github, actions } = mockGithub([current, queued]);
+  const context = { repo: { owner: 'LizardByte', repo: 'PresetDB' }, issue: { number: 1 } };
+  const removeLabel = github.rest.issues.removeLabel;
+  let finishRemoval;
+  github.rest.issues.removeLabel = async params => {
+    if (params.name === 'approve-preset') {
+      await new Promise(resolve => { finishRemoval = resolve; });
+    }
+    return removeLabel(params);
+  };
+  const releasing = releaseAndPromote({ github, context });
+  await new Promise(resolve => setImmediate(resolve));
+  assert.deepEqual(actions, []);
+  finishRemoval();
+  assert.equal((await releasing).number, 2);
+  assert.deepEqual(actions, [
+    ['remove', 1, 'approve-preset'], ['remove', 1, 'approve-queue'], ['add', 2, 'approve-preset']
+  ]);
+});
+
+test('label removal failures stop release without removing the next label or promoting an issue', async () => {
+  const current = issue(1, '2026-01-01T00:00:00Z');
+  const queued = issue(2, '2026-01-02T00:00:00Z');
+  current.labels.push({ name: 'approve-preset' }, { name: 'approve-queue' });
+  queued.labels.push({ name: 'approve-queue' });
+  const { github, actions } = mockGithub([current, queued]);
+  const failedLabels = [];
+  github.rest.issues.removeLabel = async ({ name }) => {
+    failedLabels.push(name);
+    throw new Error('Label removal failed');
+  };
+  await assert.rejects(releaseAndPromote({ github,
+    context: { repo: { owner: 'LizardByte', repo: 'PresetDB' }, issue: { number: 1 } } }),
+  /Label removal failed/);
+  assert.deepEqual(failedLabels, ['approve-preset']);
+  assert.deepEqual(actions, []);
+});
+
+test('release skips absent labels and leaves another active approval in charge', async () => {
+  const current = issue(1, '2026-01-01T00:00:00Z');
+  const active = issue(2, '2026-01-02T00:00:00Z');
+  active.labels.push({ name: 'approve-preset' });
+  const { github, actions } = mockGithub([current, active]);
+  assert.equal(await releaseAndPromote({ github,
+    context: { repo: { owner: 'LizardByte', repo: 'PresetDB' }, issue: { number: 1 } } }), null);
+  assert.deepEqual(actions, []);
+});
+
 test('bot approval commands require an approver or repository admin', async () => {
   const current = issue(5, '2026-01-01T00:00:00Z');
   const { github } = mockGithub([current]);
@@ -98,10 +151,89 @@ test('approval stops when an issue changes after it was queued', async () => {
 
 test('workflow wait exits when no older approval is running', async () => {
   const { github } = mockGithub([]);
+  github.paginate = async () => [
+    { id: 5, status: 'in_progress', display_title: 'approve-preset current' },
+    { id: 6, status: 'in_progress', display_title: 'approve-preset newer' },
+    { id: 1, status: 'completed', display_title: 'approve-preset completed' },
+    { id: 2, status: 'in_progress', display_title: 'another workflow' },
+    { id: 3, status: 'in_progress' }
+  ];
   await assert.doesNotReject(waitForOlderApprovals({ github,
     context: { repo: { owner: 'LizardByte', repo: 'PresetDB' }, runId: 5 } }));
 });
 
+test('workflow wait polls at the configured interval until the older approval finishes', async t => {
+  t.mock.timers.enable({ apis: ['setTimeout', 'Date'], now: 1000 });
+  const { github } = mockGithub([]);
+  const calls = [];
+  github.paginate = async (endpoint, params) => {
+    calls.push({ endpoint, params });
+    return calls.length < 3 ? [{ id: 1, status: 'in_progress', display_title: 'approve-preset older' }] : [];
+  };
+  let finished = false;
+  const waiting = waitForOlderApprovals({ github, intervalMs: 50, timeoutMs: 200,
+    context: { repo: { owner: 'LizardByte', repo: 'PresetDB' }, runId: 5 } }).then(() => {
+    finished = true;
+  });
+  await new Promise(resolve => setImmediate(resolve));
+  assert.equal(calls.length, 1);
+  assert.equal(finished, false);
+  assert.equal(calls[0].endpoint, github.rest.actions.listWorkflowRuns);
+  assert.deepEqual(calls[0].params, {
+    owner: 'LizardByte', repo: 'PresetDB', workflow_id: 'approve-preset.yml',
+    status: 'in_progress', per_page: 100
+  });
+  t.mock.timers.tick(49);
+  await new Promise(resolve => setImmediate(resolve));
+  assert.equal(calls.length, 1);
+  t.mock.timers.tick(1);
+  await new Promise(resolve => setImmediate(resolve));
+  assert.equal(calls.length, 2);
+  assert.equal(finished, false);
+  t.mock.timers.tick(50);
+  await waiting;
+  assert.equal(calls.length, 3);
+  assert.equal(finished, true);
+});
+
+test('workflow wait keeps the original timeout across polling attempts', async t => {
+  t.mock.timers.enable({ apis: ['setTimeout', 'Date'], now: 1000 });
+  const { github } = mockGithub([]);
+  let calls = 0;
+  github.paginate = async () => {
+    calls++;
+    return [{ id: 1, status: 'in_progress', display_title: 'approve-preset older' }];
+  };
+  const timedOut = assert.rejects(waitForOlderApprovals({ github, intervalMs: 50, timeoutMs: 100,
+    context: { repo: { owner: 'LizardByte', repo: 'PresetDB' }, runId: 5 } }),
+  /Timed out waiting for older approval run 1/);
+  await new Promise(resolve => setImmediate(resolve));
+  t.mock.timers.tick(50);
+  await new Promise(resolve => setImmediate(resolve));
+  assert.equal(calls, 2);
+  t.mock.timers.tick(50);
+  await timedOut;
+  assert.equal(calls, 3);
+});
+
+test('workflow wait propagates a polling failure without scheduling another attempt', async t => {
+  t.mock.timers.enable({ apis: ['setTimeout', 'Date'], now: 1000 });
+  const { github } = mockGithub([]);
+  let calls = 0;
+  github.paginate = async () => {
+    calls++;
+    if (calls === 2) throw new Error('Workflow lookup failed');
+    return [{ id: 1, status: 'in_progress', display_title: 'approve-preset older' }];
+  };
+  const failed = assert.rejects(waitForOlderApprovals({ github, intervalMs: 50,
+    context: { repo: { owner: 'LizardByte', repo: 'PresetDB' }, runId: 5 } }), /Workflow lookup failed/);
+  await new Promise(resolve => setImmediate(resolve));
+  t.mock.timers.tick(50);
+  await failed;
+  t.mock.timers.tick(50);
+  assert.equal(calls, 2);
+});
+
 test('published statistics count approvals and escape contributor names', () => {
   const records = [{ presets: [{ history: [
     { issue: 1, action: 'add', author_id: 1, author_login: 'alice<&', approved_at: '2026-01-01T00:00:00Z' },

tests/site-browser.test.js

@@ -182,6 +182,19 @@ test('selecting a card keeps its direct link and superseded detail responses can
   assert.equal(controls['preset-detail'].children[0].textContent, 'Second game');
 });
 
+test('selecting a card reports a rejected record fetch in the detail panel', async t => {
+  const { controls, worker } = fixture(t);
+  globalThis.fetch = async () => { throw new Error('Record request failed'); };
+  boot();
+  worker().handlers.message({ data: { id: worker().messages[0].id, result: result() } });
+  controls['preset-list'].children[0].children[0].handlers.click();
+  await new Promise(resolve => setImmediate(resolve));
+  assert.equal(new URL(location.href).searchParams.get('id'), '1');
+  assert.equal(controls['preset-detail'].hidden, false);
+  assert.equal(controls['preset-detail'].children[0].textContent,
+    'Could not load presets: Record request failed');
+});
+
 test('record fetch failures and worker construction failures show usable errors', async t => {
   const { controls } = fixture(t, '?kind=game&id=42');
   globalThis.Worker = class { constructor() { throw new Error('blocked'); } };