Recognize readable document links, and stop leaking library handles

Three things the canonical-URL switch broke or exposed.

**Cross-document links stopped reading as documents.** Reference only
knew `/plans/<uuid>`, so every link copied out of the address bar after
the switch — which is now every link — landed in the References footnote
as a generic "link" with no target_plan. It recognizes the readable form
too, resolving the path to an id through the same segment walk the
router uses, aliases included: a link written before a rename still
names the document it was always about.

**The truncation lists never included libraries.** Two specs run without
transactional fixtures and clean up by hand; their lists predate
libraries and folders, so those rows accumulated across runs. A library
handle is globally unique, so a leaked row keeps "alice" reserved — and
the list is now named once instead of copy-pasted twice.

**`/l` could omit your own library.** Libraries are materialized on first
touch, but the index read the table directly, which is exactly the path
that skips the invariant. A viewer who hadn't yet loaded a page linking
their library got a list without it. It had looked fine only because a
stale row from an earlier run happened to be sitting in the test DB.

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

Author: HamptonMakes

engine/app/controllers/coplan/libraries_controller.rb

@@ -15,6 +15,12 @@ def mine
     # The top of the tree. `/l` is a real page because every prefix of a
     # browsable URL is one.
     def index
+      # Your own library first, so the list can't omit it. Libraries are
+      # materialized on first touch (User#library), and reading the table
+      # directly is exactly the path that skips that — a user who'd never
+      # loaded a page that links their library got a list without it.
+      current_user.library
+
       @libraries = Library.includes(:owner).order(:handle).to_a
       @plan_counts = Plan.visible_to(current_user).active
         .joins(:placement)

engine/app/models/coplan/reference.rb

@@ -15,13 +15,19 @@ class Reference < ApplicationRecord
     scope :extracted, -> { where(source: "extracted") }
     scope :explicit, -> { where(source: "explicit") }
 
+    # A CoPlan document link in either form. `/l/<handle>/…` is what
+    # people copy out of the address bar now; `/plans/<uuid>` is the old
+    # form that still shows up in anything written before the switch.
+    PLAN_ID_PATH = %r{/plans/([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})}
+    READABLE_PLAN_PATH = %r{/l/([^/?#]+)/([^?#]+)}
+
     def self.classify_url(url)
       case url
       when %r{\Ahttps?://github\.com/[^/]+/[^/]+/pull/\d+}
         "pull_request"
       when %r{\Ahttps?://github\.com/[^/]+/[^/]+/?(\z|#|\?|/tree/|/blob/|/commit/)}
         "repository"
-      when %r{/plans/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}}
+      when PLAN_ID_PATH, READABLE_PLAN_PATH
         "plan"
       when %r{\Ahttps?://docs\.google\.com/}, %r{\Ahttps?://drive\.google\.com/}
         "document"
@@ -34,10 +40,35 @@ def self.classify_url(url)
       end
     end
 
+    # The id of the document a link points at, so the References section
+    # can say which plan it is rather than just showing a URL.
+    #
+    # A readable path has to be resolved, since the id isn't in it. That
+    # walk goes through the alias table too, so a link written before a
+    # rename still finds the document it was always about — the same way
+    # following the link would.
     def self.extract_target_plan_id(url)
       return nil if url.blank?
-      match = url.match(%r{/plans/([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})})
-      match&.[](1)
+
+      if (match = url.match(PLAN_ID_PATH))
+        return match[1]
+      end
+
+      match = url.match(READABLE_PLAN_PATH)
+      return nil if match.nil?
+
+      resolve_readable(match[1], match[2])
+    end
+
+    def self.resolve_readable(handle, slug_path)
+      result = Urls::Resolve.call(handle: handle, slug_path: slug_path)
+      return result.plan&.id if result.redirect_to_path.blank?
+
+      # A stale path the aliases recognized: walk the current one.
+      current_handle, _, rest = result.redirect_to_path.partition("/")
+      return nil if rest.blank?
+
+      Urls::Resolve.call(handle: current_handle, slug_path: rest).plan&.id
     end
 
     def self.ransackable_attributes(auth_object = nil)

engine/app/services/coplan/references/extract_from_content.rb

@@ -19,21 +19,23 @@ def call
         # Remove extracted references for URLs no longer in content
         @plan.references.extracted.where.not(url: found_urls.keys).delete_all
 
-        # Batch-check plan existence for plan-type references
-        candidate_plan_ids = found_urls.keys
-          .select { |url| Reference.classify_url(url) == "plan" }
-          .filter_map { |url| Reference.extract_target_plan_id(url) }
-          .reject { |id| id == @plan.id }
-        existing_plan_ids = candidate_plan_ids.any? ? Plan.where(id: candidate_plan_ids).pluck(:id).to_set : Set.new
+        # Classify once and resolve once. A readable `/l/…` link costs a
+        # segment walk to turn into an id, so doing it per-URL rather than
+        # per-mention keeps a body full of cross-links cheap.
+        types = found_urls.keys.index_with { |url| Reference.classify_url(url) }
+        candidates = {}
+        types.each do |url, type|
+          next unless type == "plan"
+          id = Reference.extract_target_plan_id(url)
+          candidates[url] = id if id.present? && id != @plan.id
+        end
+        existing_plan_ids = candidates.any? ? Plan.where(id: candidates.values).pluck(:id).to_set : Set.new
 
         # Create or update references for found URLs
         found_urls.each do |url, meta|
-          ref_type = Reference.classify_url(url)
-          target_plan_id = nil
-          if ref_type == "plan"
-            candidate_id = Reference.extract_target_plan_id(url)
-            target_plan_id = candidate_id if candidate_id && existing_plan_ids.include?(candidate_id)
-          end
+          ref_type = types[url]
+          candidate_id = candidates[url]
+          target_plan_id = candidate_id if candidate_id && existing_plan_ids.include?(candidate_id)
 
           ref = @plan.references.find_or_initialize_by(url: url)
           # Don't overwrite explicit references

spec/models/coplan/reference_spec.rb

@@ -60,6 +60,18 @@
       expect(described_class.classify_url("https://coplan.example.com/plans/019d54a7-ea13-72d5-bc54-fc44cb9b939a")).to eq("plan")
     end
 
+    # The readable form is what anyone copies out of the address bar now,
+    # so it has to read as a plan link and not a generic one.
+    it "classifies readable CoPlan document URLs" do
+      expect(described_class.classify_url("https://coplan.example.com/l/hampton/cart-roadmap")).to eq("plan")
+      expect(described_class.classify_url("https://coplan.example.com/l/hampton/liveorder/q3/cart-roadmap")).to eq("plan")
+    end
+
+    # A bare library or a workspace path isn't a document.
+    it "does not classify a library root as a plan" do
+      expect(described_class.classify_url("https://coplan.example.com/l/hampton")).to eq("link")
+    end
+
     it "classifies Google Docs URLs" do
       expect(described_class.classify_url("https://docs.google.com/document/d/abc123")).to eq("document")
       expect(described_class.classify_url("https://drive.google.com/file/d/abc123")).to eq("document")
@@ -88,6 +100,36 @@
     it "returns nil for non-plan URLs" do
       expect(described_class.extract_target_plan_id("https://example.com")).to be_nil
     end
+
+    context "readable document URLs" do
+      let(:author) { create(:coplan_user, username: "hampton") }
+      let(:folder) { create(:folder, name: "LiveOrder", created_by_user: author) }
+      let!(:plan) do
+        create(:plan, :published, created_by_user: author, title: "Cart Roadmap").tap do |p|
+          CoPlan::Plans::Place.call(plan: p, folder: folder, actor: author)
+        end
+      end
+
+      it "resolves the path to the document it names" do
+        expect(described_class.extract_target_plan_id("https://coplan.example.com/l/hampton/liveorder/cart-roadmap"))
+          .to eq(plan.id)
+      end
+
+      # An inline link written before a rename still points at the same
+      # document, so it should still name it — the alias walk is the same
+      # one that makes following the link work.
+      it "follows a rename through the alias table" do
+        plan.reload.update!(title: "Basket Roadmap")
+        expect(plan.reload.url_path).to eq("hampton/liveorder/basket-roadmap")
+
+        expect(described_class.extract_target_plan_id("https://coplan.example.com/l/hampton/liveorder/cart-roadmap"))
+          .to eq(plan.id)
+      end
+
+      it "returns nil for a path that names nothing" do
+        expect(described_class.extract_target_plan_id("https://coplan.example.com/l/hampton/nope/gone")).to be_nil
+      end
+    end
   end
 
   describe "scopes" do

spec/models/plan_spec.rb

@@ -195,8 +195,7 @@
     self.use_transactional_tests = false
 
     after do
-      truncate_tables(*%w[coplan_plan_tags coplan_tags coplan_plan_versions coplan_plans
-                          coplan_plan_types coplan_search_queries coplan_users])
+      truncate_plan_tables
     end
 
     let!(:author) { create(:coplan_user, name: "Tessa Engineer") }

spec/requests/libraries_spec.rb

@@ -15,6 +15,17 @@
       expect(response).to have_http_status(:ok)
       expect(response.body).to include("/l/alice", "/l/bob")
     end
+
+    # Libraries are materialized on first touch, so a viewer who has never
+    # loaded a page that links theirs has no row yet. The list still has to
+    # include it — "libraries you can browse" without your own is nonsense.
+    it "includes your own library even when nothing has materialized it" do
+      expect(CoPlan::Library.where(owner_id: alice.id)).not_to exist
+
+      get browse_root_path
+
+      expect(response.body).to include("/l/alice")
+    end
   end
 
   describe "GET /library" do

spec/requests/search_spec.rb

@@ -8,8 +8,7 @@
   self.use_transactional_tests = false
 
   after do
-    truncate_tables(*%w[coplan_plan_tags coplan_tags coplan_plan_versions coplan_plans
-                        coplan_plan_types coplan_search_queries coplan_users])
+    truncate_plan_tables
   end
 
   let!(:alice) { create(:coplan_user, name: "Alice Searcher") }

spec/support/truncation_helpers.rb

@@ -2,6 +2,32 @@
 # transactional fixtures (InnoDB FULLTEXT writes are invisible to
 # MATCH … AGAINST inside the same transaction) and so must clean up manually.
 module TruncationHelpers
+  # Everything a plan-and-author fixture touches, children before parents
+  # (only the SQLite branch below cares about the order).
+  #
+  # Libraries and folders belong here even though no such example reads
+  # them: a library handle is globally unique, so a leaked row keeps
+  # "alice" reserved and the *next* spec's alice quietly gets a different
+  # URL. Truncating users without them leaves exactly that orphan.
+  PLAN_TABLES = %w[
+    coplan_plan_tags
+    coplan_tags
+    coplan_plan_placements
+    coplan_plan_versions
+    coplan_plans
+    coplan_folders
+    coplan_library_events
+    coplan_libraries
+    coplan_url_aliases
+    coplan_plan_types
+    coplan_search_queries
+    coplan_users
+  ].freeze
+
+  def truncate_plan_tables
+    truncate_tables(*PLAN_TABLES)
+  end
+
   def truncate_tables(*tables)
     conn = ActiveRecord::Base.connection
     case conn.adapter_name