refactor(entry-actions): dedup scope guard + guard publish cache-flush

Address code-review findings on the trip publish/unpublish toggle:

- Extract EntryScopeGuard::resolveChildOf() so resolveActiveDailyChild and
  resolveTripChild share one find() + parent-route-assert body instead of two
  copies that could drift (P1 maintainability).
- Wrap setTripPublished's post-save cache invalidation in try/catch. save() has
  already persisted the published flag to disk, so a flush failure now logs a
  loud reconciliation warning (and still returns success + the audit line)
  rather than bubbling to a bare 500 that reads as "nothing happened"
  (P2 reliability / adversarial).

Behavior-preserving; PHP lint clean; trip-publish Playwright suite 8/8.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Mpdu3Dt1iVoozHwAMyjrbn

plugins/cache-on-save/classes/EntryScopeGuard.php

@@ -97,19 +97,17 @@ class EntryScopeGuard
     }
 
     /**
-     * Resolve a folder segment to the page that is a DIRECT child of the active
+     * Resolve a safe segment to the page that is a DIRECT child of $parentRoute,
-     * trip's dailies container, or null when the segment is unsafe, no active trip
+     * or null when the segment is unsafe, the page does not exist, or its parent
-     * is set, the page does not exist, or its parent is not the active dailies.
+     * is not exactly $parentRoute. Resolving via $pages->find() + a parent-route
+     * assertion (never raw path concatenation) is what closes the traversal hole;
+     * both public resolvers below share this one body so they cannot drift.
      */
-    public static function resolveActiveDailyChild(Grav $grav, string $segment): ?PageInterface
+    private static function resolveChildOf(Grav $grav, string $parentRoute, string $segment): ?PageInterface
     {
         if (!self::isSafeSegment($segment)) {
             return null;
         }
-        $dailies = self::dailiesRoute($grav);
-        if ($dailies === null) {
-            return null;
-        }
         $pages = $grav['pages'];
         // In the API request context the page tree is lazily disabled; enable it
         // so find() can resolve (mirrors the api plugin's own resolvePageByRoute).
@@ -117,17 +115,31 @@ class EntryScopeGuard
         if (method_exists($pages, 'enablePages')) {
             $pages->enablePages();
         }
-        $page = $pages->find($dailies . '/' . $segment);
+        $page = $pages->find($parentRoute . '/' . $segment);
         if ($page === null) {
             return null;
         }
         $parent = $page->parent();
-        if ($parent === null || $parent->route() !== $dailies) {
+        if ($parent === null || $parent->route() !== $parentRoute) {
             return null;
         }
         return $page;
     }
 
+    /**
+     * Resolve a folder segment to the page that is a DIRECT child of the active
+     * trip's dailies container, or null when the segment is unsafe, no active trip
+     * is set, the page does not exist, or its parent is not the active dailies.
+     */
+    public static function resolveActiveDailyChild(Grav $grav, string $segment): ?PageInterface
+    {
+        $dailies = self::dailiesRoute($grav);
+        if ($dailies === null) {
+            return null;
+        }
+        return self::resolveChildOf($grav, $dailies, $segment);
+    }
+
     /**
      * Resolve a slug to the trip page that is a DIRECT child of /trips, or null
      * when the segment is unsafe, the page does not exist, or its parent is not
@@ -140,23 +152,6 @@ class EntryScopeGuard
      */
     public static function resolveTripChild(Grav $grav, string $slug): ?PageInterface
     {
-        if (!self::isSafeSegment($slug)) {
+        return self::resolveChildOf($grav, '/trips', $slug);
-            return null;
-        }
-        $pages = $grav['pages'];
-        // In the API request context the page tree is lazily disabled; enable it
-        // so find() can resolve (mirrors resolveActiveDailyChild). Idempotent.
-        if (method_exists($pages, 'enablePages')) {
-            $pages->enablePages();
-        }
-        $page = $pages->find('/trips/' . $slug);
-        if ($page === null) {
-            return null;
-        }
-        $parent = $page->parent();
-        if ($parent === null || $parent->route() !== '/trips') {
-            return null;
-        }
-        return $page;
     }
 }

plugins/entry-actions/classes/EntryActionsApiController.php

@@ -209,12 +209,26 @@ class EntryActionsApiController extends AbstractApiController
         // store (deleteAll → APCu flushAll) AND apcu_clear_cache() directly to be
         // certain, clear the compiled files, and reset the in-memory tree so the
         // next request rebuilds from disk and re-reads the published flag.
-        $this->grav['cache']->deleteAll();
+        // save() above is already persisted to disk. If any invalidation call
-        if (function_exists('apcu_clear_cache')) {
+        // throws, do NOT let it bubble to a plain 500 (which reads to the owner as
-            apcu_clear_cache();
+        // "nothing happened") and skip the audit line: the on-disk flag DID change.
+        // Log a loud reconciliation warning instead so an operator knows to clear
+        // cache manually, then still report success.
+        try {
+            $this->grav['cache']->deleteAll();
+            if (function_exists('apcu_clear_cache')) {
+                apcu_clear_cache();
+            }
+            $this->grav['pages']->reset();
+            $this->grav['cache']->clearCache('standard');
+        } catch (\Throwable $e) {
+            $this->grav['log']->error(sprintf(
+                'entry-actions: trip "%s" published=%s SAVED to disk but cache invalidation failed (%s) — clear cache manually',
+                $slug,
+                $published ? 'true' : 'false',
+                $e->getMessage()
+            ));
         }
-        $this->grav['pages']->reset();
-        $this->grav['cache']->clearCache('standard');
         // Audit trail: publish state is owner-only and changes site-wide
         // visibility — record who flipped which trip to what.
         $this->grav['log']->info(sprintf('entry-actions: owner "%s" set trip "%s" published=%s', $user->username, $slug, $published ? 'true' : 'false'));