Close remaining known-risks gaps (A2, D4, SPOF test coverage)

app/Actions/Extract/Japan/Handler.php

@@ -7,8 +7,7 @@
 use App\Actions\Extract\ChunkRawPages;
 use App\Actions\Extract\HandlerInterface;
 use App\Actions\Extract\MarkExtractFailed;
-use App\Actions\Extract\SyncPak;
-use App\Actions\Extract\UpdateOrCreatePage;
+use App\Actions\Extract\UpdateOrCreatePageWithPaks;
 use App\Enums\SiteName;
 use App\Models\Page;
 use App\Models\RawPage;
@@ -22,8 +21,7 @@ public function __construct(
         private ChunkRawPages $chunkRawPages,
         private ExtractLastModified $extractLastModified,
         private ExtractContents $extractContents,
-        private UpdateOrCreatePage $updateOrCreatePage,
-        private SyncPak $syncPak,
+        private UpdateOrCreatePageWithPaks $updateOrCreatePageWithPaks,
         private MarkExtractFailed $markExtractFailed,
     ) {}
 
@@ -42,14 +40,13 @@ public function __invoke(LoggerInterface $logger): void
                     // pageがあって更新有り
                     if (! $rawPage->page || $this->needUpdate($rawPage->page, $lastModiefied)) {
                         $contents = ($this->extractContents)($rawPage);
-                        $page = ($this->updateOrCreatePage)(
+                        ($this->updateOrCreatePageWithPaks)(
                             $rawPage,
                             $contents['title'],
                             $contents['text'],
-                            $lastModiefied
+                            $lastModiefied,
+                            $contents['paks']
                         );
-
-                        ($this->syncPak)($page, $contents['paks']);
                     }
 
                     $this->markExtractFailed->clear($rawPage);

app/Actions/Extract/Portal/Handler.php

@@ -7,8 +7,7 @@
 use App\Actions\Extract\ChunkRawPages;
 use App\Actions\Extract\HandlerInterface;
 use App\Actions\Extract\MarkExtractFailed;
-use App\Actions\Extract\SyncPak;
-use App\Actions\Extract\UpdateOrCreatePage;
+use App\Actions\Extract\UpdateOrCreatePageWithPaks;
 use App\Enums\SiteName;
 use App\Models\Page;
 use App\Models\RawPage;
@@ -22,8 +21,7 @@ public function __construct(
         private ChunkRawPages $chunkRawPages,
         private FindArticle $findArticle,
         private ExtractContents $extractContents,
-        private UpdateOrCreatePage $updateOrCreatePage,
-        private SyncPak $syncPak,
+        private UpdateOrCreatePageWithPaks $updateOrCreatePageWithPaks,
         private MarkExtractFailed $markExtractFailed,
     ) {}
 
@@ -44,14 +42,13 @@ public function __invoke(LoggerInterface $logger): void
                     // pageがあって更新有り
                     if (! $rawPage->page || $this->needUpdate($rawPage->page, $lastModiefied)) {
                         $contents = ($this->extractContents)($article);
-                        $page = ($this->updateOrCreatePage)(
+                        ($this->updateOrCreatePageWithPaks)(
                             $rawPage,
                             $contents['title'],
                             $contents['text'],
                             $lastModiefied,
+                            $contents['paks']
                         );
-
-                        ($this->syncPak)($page, $contents['paks']);
                     }
 
                     $this->markExtractFailed->clear($rawPage);

app/Actions/Extract/Twitrans/Handler.php

@@ -7,8 +7,7 @@
 use App\Actions\Extract\ChunkRawPages;
 use App\Actions\Extract\HandlerInterface;
 use App\Actions\Extract\MarkExtractFailed;
-use App\Actions\Extract\SyncPak;
-use App\Actions\Extract\UpdateOrCreatePage;
+use App\Actions\Extract\UpdateOrCreatePageWithPaks;
 use App\Enums\SiteName;
 use App\Models\Page;
 use App\Models\RawPage;
@@ -22,8 +21,7 @@ public function __construct(
         private ChunkRawPages $chunkRawPages,
         private ExtractLastModified $extractLastModified,
         private ExtractContents $extractContents,
-        private UpdateOrCreatePage $updateOrCreatePage,
-        private SyncPak $syncPak,
+        private UpdateOrCreatePageWithPaks $updateOrCreatePageWithPaks,
         private MarkExtractFailed $markExtractFailed,
     ) {}
 
@@ -42,14 +40,13 @@ public function __invoke(LoggerInterface $logger): void
                     // pageがあって更新有り
                     if (! $rawPage->page || $this->needUpdate($rawPage->page, $lastModiefied)) {
                         $contents = ($this->extractContents)($rawPage);
-                        $page = ($this->updateOrCreatePage)(
+                        ($this->updateOrCreatePageWithPaks)(
                             $rawPage,
                             $contents['title'],
                             $contents['text'],
-                            $lastModiefied
+                            $lastModiefied,
+                            $contents['paks']
                         );
-
-                        ($this->syncPak)($page, $contents['paks']);
                     }
 
                     $this->markExtractFailed->clear($rawPage);

app/Actions/Extract/UpdateOrCreatePageWithPaks.php

@@ -0,0 +1,38 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Actions\Extract;
+
+use App\Enums\PakSlug;
+use App\Models\Page;
+use App\Models\RawPage;
+use Carbon\CarbonImmutable;
+use Illuminate\Support\Facades\DB;
+
+/**
+ * Page の更新と Pak の同期を 1 トランザクションで行う。
+ *
+ * 別々に呼ぶと SyncPak が失敗した際に「title/text は新しいが Pak 紐付けは古いまま」の
+ * 半端な Page が検索/Feed に見えてしまう（D4: 抽出の原子性）。
+ */
+final readonly class UpdateOrCreatePageWithPaks
+{
+    public function __construct(
+        private UpdateOrCreatePage $updateOrCreatePage,
+        private SyncPak $syncPak,
+    ) {}
+
+    /**
+     * @param  array<int,PakSlug>  $paks
+     */
+    public function __invoke(RawPage $rawPage, string $title, string $text, CarbonImmutable $lastModified, array $paks): Page
+    {
+        return DB::transaction(function () use ($rawPage, $title, $text, $lastModified, $paks): Page {
+            $page = ($this->updateOrCreatePage)($rawPage, $title, $text, $lastModified);
+            ($this->syncPak)($page, $paks);
+
+            return $page;
+        });
+    }
+}

app/Actions/Scrape/FetchHtml.php

@@ -5,6 +5,7 @@
 namespace App\Actions\Scrape;
 
 use App\Enums\Encoding;
+use Illuminate\Http\Client\RequestException;
 use Illuminate\Support\Facades\Cache;
 use Illuminate\Support\Facades\Http;
 use Symfony\Component\DomCrawler\Crawler;
@@ -25,7 +26,14 @@ public function __invoke(string $url, Encoding $fromEncoding): Crawler
             /** @var string */
             $html = Cache::get($key);
         } else {
-            $result = retry($this->retryTimes, fn () => Http::get($url), $this->sleepMilliseconds);
+            // 非2xx も失敗として扱い、RawPage にエラーページの内容を書き込まないようにする。
+            // 4xx は再試行しても解消しないため、接続エラー/5xx のみリトライ対象にする。
+            $result = retry(
+                $this->retryTimes,
+                fn () => Http::get($url)->throw(),
+                $this->sleepMilliseconds,
+                fn (\Throwable $throwable): bool => ! $throwable instanceof RequestException || $throwable->response->serverError(),
+            );
             $html = $fromEncoding === Encoding::UTF_8
                 ? $result->body()
                 : mb_convert_encoding((string) $result->body(), Encoding::UTF_8->value, $fromEncoding->value);

docs/known-risks.md

@@ -12,38 +12,38 @@
 
 | ID | 保護すべき挙動 / Expected Outcome | 制御 | テスト | Status | 是正条件 | 記録日 |
 |----|----------------------------------|------|--------|--------|----------|--------|
-| A1 | 1 サイト/1URL の失敗で他サイト・他 URL の処理が止まらない | Preventive（ハンドラ毎の try/catch） | 1（`Extract/Japan/HandlerIsolationTest`） | 🟡 SPOF | テストを 2 本以上に（scrape 側 / 他サイトにも展開） | 2026-06-22 |
-| A2 | HTTP 失敗時に RawPage を空/部分 HTML で上書きしない | Preventive（fetch 例外時は upsert に到達しない） | 1（`Scrape/Japan/HandlerFailureTest`） | 🟡 SPOF | 残課題: 非2xx でも body を書き込む経路がある。`->throw()` 等で非2xx も失敗扱いにするか検討 | 2026-06-22 |
-| A3 | 同一 RawPage に extract を再実行しても Page 重複が出ない（冪等） | Preventive（`pages_url_unique` + HasOne） | 1（`Extract/UpdateOrCreatePageTest`） | 🟡 SPOF | テストを 2 本以上に | 2026-06-22 |
+| A1 | 1 サイト/1URL の失敗で他サイト・他 URL の処理が止まらない | Preventive（ハンドラ毎の try/catch） | 2（`Extract/Japan/HandlerIsolationTest` + `Scrape/Japan/HandlerIsolationTest`） | 🟢 OK | — | 2026-06-22 |
+| A2 | HTTP 失敗時に RawPage を空/部分 HTML で上書きしない | Preventive（`FetchHtml` で `Http::get()->throw()`。非2xx も例外として扱い upsert に到達しない。4xx は解消しないため即失敗、5xx/接続エラーのみ `retry()` で再試行） | 5（`FetchHtmlTest::test_throws_on_non_2xx...` + `test_does_not_retry_on_4xx...` + `test_retries_on_5xx...` + `Scrape/Japan/HandlerFailureTest`×2） | 🟢 OK | — | 2026-06-22 |
+| A3 | 同一 RawPage に extract を再実行しても Page 重複が出ない（冪等） | Preventive（`pages_url_unique` + HasOne） | 2（`Extract/UpdateOrCreatePageTest`） | 🟢 OK | — | 2026-06-22 |
 | A4 | 抽出失敗時にスクレイプ済データ（RawPage）を破壊しない | Preventive（`MarkExtractFailed`：削除せず `extract_failed_at` で隔離、成功時にクリア。`extract_failed_at` に index 追加済み） | 4（`MarkExtractFailedTest`×3 + `HandlerIsolationTest`） | 🟢 OK | — | 2026-06-22 |
 | A5 | 同一 URL の重複行を作らない | Preventive（DB 一意制約） | 2（`Models/UrlUniqueConstraintTest`） | 🟢 OK | — | 2026-06-22 |
 
 ## B. Notion 同期
 
 | ID | 保護すべき挙動 / Expected Outcome | 制御 | テスト | Status | 是正条件 | 記録日 |
 |----|----------------------------------|------|--------|--------|----------|--------|
-| B1 | 再実行で Notion ページを重複作成しない | Preventive（URL 突合で create/update 分岐） | 1（Strong） | 🟡 SPOF | テストを 2 本以上に増やす | 2026-06-22 |
-| B2 | 1 件の Notion API エラーでバッチ全体が止まらない | Preventive（delete/add 両方とも項目単位 try/catch + 継続 + 失敗件数を error ログ。`addNewNotionPages` は `keyNotionPagesByUrl()` で URL 解析失敗も個別隔離。URL が無い Notion ページは削除対象から除外） | 1（`SyncActionTest::test_continues_syncing_when_one_item_fails`） | 🟡 SPOF | テストを 2 本以上に（delete 側の失敗継続・null URL ガードの直接テストを追加） | 2026-06-22 |
+| B1 | 再実行で Notion ページを重複作成しない | Preventive（URL 突合で create/update 分岐） | 2（`test_sync_action_creates_updates_and_deletes_pages` + `test_does_not_create_duplicate_when_page_already_exists_in_notion`） | 🟢 OK | — | 2026-06-22 |
+| B2 | 1 件の Notion API エラーでバッチ全体が止まらない | Preventive（delete/add 両方とも項目単位 try/catch + 継続 + 失敗件数を error ログ。`addNewNotionPages` は `keyNotionPagesByUrl()` で URL 解析失敗も個別隔離。URL が無い Notion ページは削除対象から除外） | 3（`test_continues_syncing_when_one_item_fails` + `test_continues_deleting_when_one_delete_fails` + `test_does_not_delete_notion_page_without_url`） | 🟢 OK | — | 2026-06-22 |
 | B3 | 未変更項目を毎回 re-PUSH せず API クォータを浪費しない | None（`synced_at`/状態フラグ無し） | 0 | 🔴 Missing | **記録のみ**。API 呼び出し回数が問題化したら `synced_at` を導入 | 2026-06-22 |
 | B4 | Notion シークレットを例外/ログ/Discord に流出させない | Preventive（`SecretScrubber` で送出前に伏字化。C3 と一体） | 7（C3 のテストと共有） | 🟢 OK | — | 2026-06-22 |
 
 ## C. シークレット / 資格情報の取扱い
 
 | ID | 保護すべき挙動 / Expected Outcome | 制御 | テスト | Status | 是正条件 | 記録日 |
 |----|----------------------------------|------|--------|--------|----------|--------|
-| C1 | デバッグページが config/スタックトレースを露出しない | Preventive（`.env.example`/`.ci`/`.deploy` すべて `APP_DEBUG=false`、コード既定も false） | 1（C2 の `ApiErrorResponseTest` で間接担保） | 🟡 SPOF | Web 側デバッグページ向けの直接テストは未追加 | 2026-06-22 |
-| C2 | API 例外が生のトレースを返さない | Preventive（`APP_DEBUG=false` で汎用 JSON エラー） | 1（`Http/ApiErrorResponseTest`） | 🟡 SPOF | テストを 2 本以上に | 2026-06-22 |
+| C1 | デバッグページが config/スタックトレースを露出しない | Preventive（`.env.example`/`.ci`/`.deploy` すべて `APP_DEBUG=false`、コード既定も false） | 2（`Http/ApiErrorResponseTest` + `Http/WebErrorResponseTest`） | 🟢 OK | — | 2026-06-22 |
+| C2 | API 例外が生のトレースを返さない | Preventive（`APP_DEBUG=false` で汎用 JSON エラー） | 2（`Http/ApiErrorResponseTest`×2） | 🟢 OK | — | 2026-06-22 |
 | C3 | 例外レポート（log/Discord）に機密値を混入させない | Preventive（`SecretScrubber`（伏字化対象は5文字以上の値のみ。"root"等の短い開発用パスワードでの誤爆を回避）+ `RedactSecretsProcessor`/tap + `ConvertDiscord`（親クラスの `addMessageStacktrace` を no-op 化し、未伏字化の生トレースでの上書きも防止） で送出前に伏字化） | 7（`SecretScrubberTest`×4 + `RedactSecretsProcessorTest` + `ConvertDiscordTest`×2） | 🟢 OK | — | 2026-06-22 |
 | C4 | 実シークレットを git に混入させない | Preventive（`.gitignore` で `.env` 除外、committed な `.env.*` は空値） | N/A | 🟢 OK | — | 2026-06-22 |
 
 ## D. 公開検索 / Feed / API
 
 | ID | 保護すべき挙動 / Expected Outcome | 制御 | テスト | Status | 是正条件 | 記録日 |
 |----|----------------------------------|------|--------|--------|----------|--------|
-| D1 | raw_pages（生 HTML）を公開経路に露出しない | Preventive（検索/Feed で rawPage を eager-load しない、Page に html 列無し） | 1（`PageResourceTest::test_search_does_not_eager_load_raw_page_relation`） | 🟡 SPOF | Feed 経路の assert も追加 | 2026-06-22 |
+| D1 | raw_pages（生 HTML）を公開経路に露出しない | Preventive（検索/Feed で rawPage を eager-load しない、Page に html 列無し） | 2（`PageResourceTest::test_search_does_not_eager_load_raw_page_relation` + `SearchPage/FeedActionTest`） | 🟢 OK | — | 2026-06-22 |
 | D2 | 非公開コンテンツを検索/Feed に出さない | N/A（Page に公開状態の概念が無い） | — | — | 対象外（将来 status 列を足す場合は再評価） | 2026-06-22 |
-| D3 | API/Feed 応答に内部フィールドを含めない | Preventive（PageResource 許可リスト） | 1（`PageResourceTest::test_resource_exposes_only_whitelisted_fields`） | 🟡 SPOF | テストを 2 本以上に | 2026-06-22 |
-| D4 | 半分書かれた Page 行を検索に見せない（原子性） | None（`UpdateOrCreatePage` に transaction 無し） | 0 | 🟡 Structural Weakness | （低優先・データ品質）将来 transaction か status 列で是正 | 2026-06-22 |
+| D3 | API/Feed 応答に内部フィールドを含めない | Preventive（PageResource 許可リスト + `Page::toFeedItem()`） | 2（`PageResourceTest::test_resource_exposes_only_whitelisted_fields` + `test_feed_item_does_not_expose_internal_fields_or_raw_html`） | 🟢 OK | — | 2026-06-22 |
+| D4 | 半分書かれた Page 行を検索に見せない（原子性） | Preventive（`UpdateOrCreatePageWithPaks` が Page 更新 + Pak 同期を `DB::transaction()` で原子化。3 Handler すべてこれを使用） | 2（`Extract/UpdateOrCreatePageWithPaksTest`） | 🟢 OK | — | 2026-06-22 |
 
 <!--
 運用メモ:

tests/Feature/Actions/Extract/Japan/HandlerIsolationTest.php

@@ -11,6 +11,7 @@
 use App\Actions\Extract\MarkExtractFailed;
 use App\Actions\Extract\SyncPak;
 use App\Actions\Extract\UpdateOrCreatePage;
+use App\Actions\Extract\UpdateOrCreatePageWithPaks;
 use App\Enums\SiteName;
 use App\Models\RawPage;
 use Psr\Log\NullLogger;
@@ -43,8 +44,7 @@ public function test_failure_on_one_raw_page_does_not_stop_the_others(): void
             new ChunkRawPages,
             new ExtractLastModified,
             new ExtractContents,
-            new UpdateOrCreatePage,
-            new SyncPak,
+            new UpdateOrCreatePageWithPaks(new UpdateOrCreatePage, new SyncPak),
             new MarkExtractFailed,
         );
 

tests/Feature/Actions/Extract/UpdateOrCreatePageTest.php

@@ -28,4 +28,18 @@ public function test_running_twice_keeps_a_single_page(): void
         $this->assertSame($page->id, $second->id);
         $this->assertSame('タイトル更新', $second->fresh()?->title);
     }
+
+    public function test_running_three_times_from_separate_instances_keeps_a_single_page(): void
+    {
+        $rawPage = RawPage::factory()->create();
+
+        // 同一インスタンスの再利用に依存していないことを確認するため、毎回新しいインスタンスで実行する
+        // （別プロセス/別ジョブ実行からの再実行を模す）。
+        (new UpdateOrCreatePage)($rawPage, '1回目', '本文1', CarbonImmutable::now());
+        (new UpdateOrCreatePage)($rawPage->fresh(), '2回目', '本文2', CarbonImmutable::now());
+        $third = (new UpdateOrCreatePage)($rawPage->fresh(), '3回目', '本文3', CarbonImmutable::now());
+
+        $this->assertSame(1, Page::query()->where('raw_page_id', $rawPage->id)->count());
+        $this->assertSame('3回目', $third->fresh()?->title);
+    }
 }

tests/Feature/Actions/Extract/UpdateOrCreatePageWithPaksTest.php

@@ -0,0 +1,50 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tests\Feature\Actions\Extract;
+
+use App\Actions\Extract\SyncPak;
+use App\Actions\Extract\UpdateOrCreatePage;
+use App\Actions\Extract\UpdateOrCreatePageWithPaks;
+use App\Models\RawPage;
+use Carbon\CarbonImmutable;
+use Illuminate\Support\Facades\DB;
+use Tests\Feature\TestCase;
+
+/**
+ * D4: Page の更新と Pak 同期は 1 トランザクションで行われ、片方だけ反映された
+ * 半端な状態（title/text は新しいが Pak 紐付けは古いまま）が検索に見えないこと。
+ */
+final class UpdateOrCreatePageWithPaksTest extends TestCase
+{
+    public function test_wraps_page_update_and_pak_sync_in_a_single_transaction(): void
+    {
+        $rawPage = RawPage::factory()->create();
+
+        // SyncPak は final のため Mockery では型ヒント越しに差し替えできない。
+        // 代わりに DB::transaction が実際に呼ばれていること（両操作が原子的にまとめられていること）
+        // を直接検証する。partialMock を使い、transaction 以外の実 DB 操作（UpdateOrCreatePage/SyncPak
+        // 内部のクエリ）はそのまま実行されるようにする（shouldReceive だと DB 全体が差し替わり、
+        // それらのクエリが解決できなくなって壊れる）。
+        DB::partialMock()
+            ->shouldReceive('transaction')
+            ->once()
+            ->andReturnUsing(fn (\Closure $callback) => $callback());
+
+        $updateOrCreatePageWithPaks = new UpdateOrCreatePageWithPaks(new UpdateOrCreatePage, new SyncPak);
+        $page = $updateOrCreatePageWithPaks($rawPage, '新タイトル', '新本文', CarbonImmutable::now(), []);
+
+        $this->assertSame('新タイトル', $page->title);
+    }
+
+    public function test_commits_page_and_paks_together_on_success(): void
+    {
+        $rawPage = RawPage::factory()->create();
+
+        $updateOrCreatePageWithPaks = new UpdateOrCreatePageWithPaks(new UpdateOrCreatePage, new SyncPak);
+        $page = $updateOrCreatePageWithPaks($rawPage, '新タイトル', '新本文', CarbonImmutable::now(), []);
+
+        $this->assertSame('新タイトル', $page->fresh()?->title);
+    }
+}