fix: false|default(true) is true, in five templates that relied on false

Not a demo issue — every install, every send.

Twig's `default` filter substitutes when a value is undefined OR EMPTY,
and `false` is empty. So `false|default(true)` is `true`, and every
caller passing `false` to switch something off was ignored.

The one that was reported: sending a message showed "Message sent." and
"Sending… 8 · Undo" in two toasts an inch apart — the app announcing the
send was finished beside its own offer to call it off. The optimistic
response includes the settle template with `notify: false` for exactly
this reason, and that suppression had never once worked.

Four more, all the same mistake:

- compose/_attachments — `live: false` means "these chips are already on
  screen, do not animate them in", so re-rendering the compose window
  replayed the entrance stagger.
- calendar/_event_chip — the time-grid asks for no start time because
  its own axis already shows one. Every chip printed it anyway.
- _partials/_thread_row — the per-account list asks not to be treated as
  a merged one and got the account disambiguation regardless.
- settings/_appearance — the setup wizard hides the export/import block,
  which its own comment calls "not worth showing someone who has been in
  the app for ninety seconds". It showed.

`?? true` is what was meant: the null-coalescing operator defaults on
null or undefined and leaves false alone.

A guard test refuses `|default(true)` anywhere under templates/, with
the five cases written down. It blanks `{# … #}` before scanning rather
than skipping lines that look like comments — the first version reported
the comments explaining this trap, which necessarily quote it.

THE ASSERTION THAT DID NOT ASSERT

The E2E check went in as `expect(locator).toHaveCount(1)` and passed
against the unfixed code. toHaveCount RETRIES: it watched two toasts,
waited the few seconds for the stray one to auto-hide, and reported
success. Counted once, with no retry, it fails 2 against 1 — which is
the only reason this commit's fix is known to be a fix.

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

templates/_partials/_thread_row.html.twig

@@ -45,7 +45,10 @@ the account view passes false.
    mail's own labels rather than from the route, so a row reached from a search
    result or a live update answers the same as one in the folder listing. #}
 {% set placement = mail_placement(item) %}
-{% set unified = unified|default(true) %}
+{# `?? true`, not `|default(true)`: the default filter fires on false as well
+   as on absent, so the per-account list's `unified: false` was ignored and its
+   rows carried the account disambiguation only the merged lists need. #}
+{% set unified = unified ?? true %}
 {% set rowAccount = item.account ?? null %}
 {% set showAccountDot = unified and rowAccount is not null and accounts|length > 1 %}
 

templates/calendar/_event_chip.html.twig

@@ -60,7 +60,10 @@ identity is threaded through a URL that would outlive the render that made it.
 
 {% set occurrence = cluster.primary %}
 {% set event = occurrence.event %}
-{% set showTime = showTime|default(true) and not event.isAllDay %}
+{# `?? true`, not `|default(true)`: the default filter treats false as absent,
+   so the time-grid's `showTime: false` never took and every chip printed a start
+   time the column's own axis was already showing. #}
+{% set showTime = (showTime ?? true) and not event.isAllDay %}
 {% set tall = tall|default(false) %}
 
 {# Whether the chip prints "18:00–19:00" or just "18:00".

templates/compose/_attachments.html.twig

@@ -16,7 +16,11 @@ appears with no sign it arrived is an attachment people upload twice.
 Variables: message, live (default true)
 #}
 {% set parts = message.messageParts|filter(part => not part.isInline) %}
-{% set live  = live|default(true) %}
+{# `?? true`, not `|default(true)`: the default filter fires on any empty
+   value, so `false|default(true)` is true and `live: false` — which the compose
+   window passes to say "these chips are already here, do not animate them in" —
+   was ignored. #}
+{% set live  = live ?? true %}
 
 {% if parts|length > 0 %}
     {# Stagger, so three files dropped at once read as a list arriving rather

templates/compose/_sent.stream.html.twig

@@ -35,7 +35,14 @@ Variables: message, thread (MessageThread|null), ctx, notify (default true)
    own toast — an error one, which is the news that matters there. Two toasts
    stacked, one saying the send failed to stop and one saying it went, is the
    same fact told twice with the second half burying the first. #}
-{% set notify = notify|default(true) %}
+{# `?? true`, NOT `|default(true)`. Twig's default filter fires on any EMPTY
+   value, not merely a missing one, so `false|default(true)` is true — and every
+   caller that passed `notify: false` to suppress this toast got it anyway. Both
+   of them do that, so a send showed "Message sent." next to its own countdown
+   saying "Sending… 8", and a cancel that arrived too late was told off and
+   congratulated at the same time. The null-coalescing operator defaults only on
+   null or undefined, which is what was meant. #}
+{% set notify = notify ?? true %}
 {# Is the sent message part of the conversation this composer was opened in?
    Both halves are needed: the dock carries `thread` too (a reply opened below
    md lands there), and a message can have no thread at all. #}

templates/settings/_appearance.html.twig

@@ -1122,7 +1122,10 @@
                and a download triggered from inside a turbo-frame either blows the
                frame away or does nothing at all. Neither is worth showing someone
                who has been in the app for ninety seconds. #}
-            {% if show_appearance_transfer|default(true) %}
+            {# `?? true`, not `|default(true)`: false is empty to the default filter, so
+               the wizard's `show_appearance_transfer: false` never took and the block
+               above showed there anyway. #}
+            {% if show_appearance_transfer ?? true %}
                 <section class="flex items-center gap-2 pt-4 border-t border-line">
                     <a href="{{ path('app_appearance_export') }}"
                        class="inline-flex items-center gap-2 px-3 h-9 rounded-pane border border-line text-sm text-ink-muted hover:bg-hover transition-colors">

tests/Infrastructure/Templating/BooleanDefaultsAreNotFilteredTest.php

@@ -0,0 +1,109 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Tests\Infrastructure\Templating;
+
+use PHPUnit\Framework\TestCase;
+use RecursiveDirectoryIterator;
+use RecursiveIteratorIterator;
+use SplFileInfo;
+
+/**
+ * No template may default a flag with `|default(true)`.
+ *
+ * THE BUG THIS EXISTS FOR
+ *
+ * Twig's `default` filter substitutes when a value is undefined **or empty**,
+ * and `false` is empty. So `false|default(true)` is `true`, and every caller
+ * that passed `false` to switch something off was ignored.
+ *
+ * It was not a near miss. Five templates used this form, and all five were
+ * being passed `false` by somebody:
+ *
+ *   • compose/_sent.stream — a send showed "Message sent." beside its own
+ *     countdown reading "Sending… 8", and a cancel that arrived too late was
+ *     told off and congratulated in the same breath.
+ *   • compose/_attachments — `live: false` means "these chips are already on
+ *     screen, do not animate them in", so re-rendering the window replayed the
+ *     entrance stagger.
+ *   • calendar/_event_chip — the time-grid asks for no start time because its
+ *     own axis already shows one. Every chip printed it anyway.
+ *   • _partials/_thread_row — the per-account list asks not to be treated as a
+ *     merged one, and got the account disambiguation regardless.
+ *   • settings/_appearance — the setup wizard hides the export/import block,
+ *     which its own comment calls "not worth showing someone who has been in
+ *     the app for ninety seconds". It showed.
+ *
+ * Each is small on its own. Together they are one mistake made five times,
+ * which is exactly what a guard is for.
+ *
+ * `?? true` is the form that was meant: the null-coalescing operator defaults
+ * on null or undefined and leaves `false` alone. `|default(false)` is not
+ * caught here and does not need to be — an empty value defaults to false, which
+ * is what an empty value already meant.
+ */
+final class BooleanDefaultsAreNotFilteredTest extends TestCase
+{
+    public function testNoTemplateDefaultsAFlagWithTheDefaultFilter(): void
+    {
+        $offenders = [];
+
+        foreach ($this->templates() as $file) {
+            $contents = $this->withoutComments((string) file_get_contents($file->getPathname()));
+
+            foreach (explode("\n", $contents) as $number => $line) {
+                if (true === str_contains($line, '|default(true)')) {
+                    $offenders[] = sprintf(
+                        '%s:%d — use `?? true`; `false|default(true)` is true',
+                        str_replace(self::projectDir() . '/', '', $file->getPathname()),
+                        $number + 1,
+                    );
+                }
+            }
+        }
+
+        self::assertSame([], $offenders, implode("\n", $offenders));
+    }
+
+    /**
+     * The template with its `{# … #}` blanked out, line numbering intact.
+     *
+     * Blanked rather than matched around: the first version of this test
+     * skipped lines that "looked like a comment", and the comments explaining
+     * this very trap — which necessarily quote the offending form — were
+     * reported as offenders. A rule about code should be applied to the code.
+     */
+    private function withoutComments(string $template): string
+    {
+        return (string) preg_replace_callback(
+            '/\{#.*?#\}/s',
+            static fn (array $match): string => preg_replace('/[^\n]/', ' ', $match[0]) ?? '',
+            $template,
+        );
+    }
+
+    /** @return list<SplFileInfo> */
+    private function templates(): array
+    {
+        $found = [];
+
+        /** @var iterable<SplFileInfo> $files */
+        $files = new RecursiveIteratorIterator(
+            new RecursiveDirectoryIterator(self::projectDir() . '/templates', RecursiveDirectoryIterator::SKIP_DOTS),
+        );
+
+        foreach ($files as $file) {
+            if (true === $file->isFile() && 'twig' === $file->getExtension()) {
+                $found[] = $file;
+            }
+        }
+
+        return $found;
+    }
+
+    private static function projectDir(): string
+    {
+        return \dirname(__DIR__, 3);
+    }
+}

tests/e2e/compose-send-optimistic.spec.ts

@@ -55,6 +55,30 @@ test.describe("send closes the composer and puts the undo in a toast", () => {
         const toast = page.locator(TOAST);
         await expect(toast).toContainText("Sending");
         await expect(toast.getByRole("button", { name: "Undo" })).toBeVisible();
+
+        // ONE toast, and it is the countdown.
+        //
+        // The optimistic response includes the settle template with
+        // `notify: false`, precisely so that "Message sent." is not said while
+        // the send can still be called off. That suppression silently did
+        // nothing for as long as it existed: the template read the flag with
+        // `|default(true)`, and Twig's default filter fires on false as well as
+        // on absent. So every send put "Message sent." on screen beside its own
+        // countdown reading "Sending… 8" — the app contradicting itself, twice
+        // over, in two toasts an inch apart.
+        //
+        // Asserted as a count rather than as "does not contain Message sent",
+        // because the failure to catch is a SECOND toast of any wording.
+        // Counted ONCE, not with expect().toHaveCount(), and that is the
+        // difference between this catching the bug and not. toHaveCount retries
+        // until it matches, and the stray toast auto-hides a few seconds in —
+        // so the retrying form sat there watching two toasts, waited for one to
+        // expire, and passed. It reported success against the unfixed code.
+        //
+        // Safe as a single sample because both toasts arrive in one Turbo
+        // stream response, and the assertion above has already waited for the
+        // second of the two.
+        expect(await page.locator(`${TOAST} > *`).count(), "a second toast was raised alongside the countdown").toBe(1);
     });
 
     /**