profile: take the account name from the environment
Rake read PRO_TACTS_PROFILE_NAME; the served route read none,
so a phone provisioning itself was stuck with the default one.
The name was never a caller's choice, just config passed along,
so render lost the parameter where it never did belong.

Assisted-by: Claude Opus 5 via Claude Code
change uxsqtxxvtrqvykrtvywpwmmwstxpqqyk
commit 0dd69a7168a5ab361b50fa7740d7dbce76968873
author Alpha Chen <alpha@kejadlen.dev>
date
parent lpkyyrum
diff --git a/Rakefile b/Rakefile
index 5e3a432..5b0ea2e 100644
--- a/Rakefile
+++ b/Rakefile
@@ -82,19 +82,18 @@ file "carddav.mobileconfig" => "lib/pro_tacts/profile.rb" do |task|
   require "pro_tacts/profile"
 
   File.write(task.name, ProTacts::Profile.render(
-    hostname: ENV.fetch("PRO_TACTS_HOSTNAME"),
-    name: ENV.fetch("PRO_TACTS_PROFILE_NAME", "pro-tacts")
+    hostname: ENV.fetch("PRO_TACTS_HOSTNAME")
   ))
 end
 
 namespace :profile do
   desc "Remove installed pro-tacts profiles, then stage a fresh one for approval"
   task install: :remove do
+    require "pro_tacts"
     require "pro_tacts/profile"
 
     File.write("carddav.mobileconfig", ProTacts::Profile.render(
-      hostname: ENV.fetch("PRO_TACTS_HOSTNAME"),
-      name: ENV.fetch("PRO_TACTS_PROFILE_NAME", "pro-tacts")
+      hostname: ENV.fetch("PRO_TACTS_HOSTNAME")
     ))
     sh "open", "carddav.mobileconfig"
     sh "open", "x-apple.systempreferences:com.apple.preferences.configurationprofiles"
diff --git a/lib/pro_tacts/config.rb b/lib/pro_tacts/config.rb
index 8ae3ddd..8e8b07f 100644
--- a/lib/pro_tacts/config.rb
+++ b/lib/pro_tacts/config.rb
@@ -65,6 +65,17 @@ module ProTacts
       @env.fetch("PRO_TACTS_DEBUG_LOG", "log/debug.log")
     end
 
+    # The CardDAV account's display name, PRO_TACTS_PROFILE_NAME — what
+    # a device labels the account it installs, and the one way to tell
+    # two pro-tacts accounts apart on the same phone. nil when unset
+    # rather than a default: the name belongs to the profile, so
+    # ProTacts::Profile::DEFAULT_NAME is the fallback and this reports
+    # only the override.
+    #: () -> String?
+    def profile_name
+      @env.fetch("PRO_TACTS_PROFILE_NAME", nil)
+    end
+
     # The running image's version, baked in as a build arg (see
     # Dockerfile and the release job in ci.yml): the image tag, which is
     # the GitHub release's name too — `YYYYMMDD-HHmm-<short sha>`. One
diff --git a/lib/pro_tacts/profile.rb b/lib/pro_tacts/profile.rb
index caced9c..cef300a 100644
--- a/lib/pro_tacts/profile.rb
+++ b/lib/pro_tacts/profile.rb
@@ -1,6 +1,8 @@
 
 require "nokogiri"
 
+require "pro_tacts"
+
 module ProTacts
   # Renders the configuration profile that provisions the pro-tacts CardDAV
   # account, so the resync loop is one rake command instead of the Internet
@@ -28,23 +30,32 @@ module ProTacts
     IDENTIFIER_PREFIX = "dev.kejadlen.pro-tacts.carddav"
     HEX = "0123456789abcdef"
 
-    # The account name when no caller overrides it. A constant because
-    # the install screen states the name the download is about to carry
-    # and must not restate it as a second literal.
+    # The account name when the environment names none. A constant
+    # because the install screen states the name the download is about
+    # to carry and must not restate it as a second literal.
     DEFAULT_NAME = "pro-tacts"
 
+    # The name every render and the install screen state. A method
+    # rather than a render parameter because there is only one source
+    # for it: a caller passing a name of its own would be labelling an
+    # account no device installs.
+    #: () -> String
+    def self.account_name
+      ProTacts.config.profile_name || DEFAULT_NAME
+    end
+
     # The username and password are a throwaway fictional pair and the server
     # ignores them: identity comes from the Tailscale headers that serve
     # injects (see ProTacts::TailscaleAuth). They stay in the template
     # because the account form expects the fields; dropping them is
     # untested.
-    #: (hostname: String, ?name: String) -> String
-    def self.render(hostname:, name: DEFAULT_NAME)
+    #: (hostname: String) -> String
+    def self.render(hostname:)
       identifier = "#{IDENTIFIER_PREFIX}-#{unique_hex}"
 
       template % {
         hostname: escape(hostname),
-        name: escape(name),
+        name: escape(account_name),
         identifier:,
         account_identifier: "#{identifier}.account",
         top_level_uuid: uuid,
diff --git a/lib/pro_tacts/web.rb b/lib/pro_tacts/web.rb
index 401ca67..6969e4a 100644
--- a/lib/pro_tacts/web.rb
+++ b/lib/pro_tacts/web.rb
@@ -208,7 +208,7 @@ module ProTacts
         r.is do
           r.get do
             response["Content-Type"] = "text/html; charset=utf-8"
-            Admin::DeviceSetup.call(hostname: r.host, name: Profile::DEFAULT_NAME)
+            Admin::DeviceSetup.call(hostname: r.host, name: Profile.account_name)
           end
         end
 
diff --git a/test/pro_tacts/admin/test_device_setup.rb b/test/pro_tacts/admin/test_device_setup.rb
index 75f7dc4..8062093 100644
--- a/test/pro_tacts/admin/test_device_setup.rb
+++ b/test/pro_tacts/admin/test_device_setup.rb
@@ -21,14 +21,41 @@ class AdminDeviceSetupTest < Minitest::Test
     header "Host", "box.example.ts.net"
   end
 
+  # The account name is read from the environment, so a name set in the
+  # shell running the suite would otherwise decide what these assert.
+  def with_config(env = {})
+    original = ProTacts.config
+    ProTacts.config = ProTacts::Config.new(env)
+    yield
+  ensure
+    ProTacts.config = original
+  end
+
   def test_screen_names_the_account_the_download_carries
-    get "/setup"
+    with_config do
+      get "/setup"
 
-    assert_equal 200, last_response.status
-    assert_equal "text/html; charset=utf-8", last_response["Content-Type"]
-    assert_includes last_response.body, "box.example.ts.net"
-    assert_includes last_response.body, ProTacts::Profile::DEFAULT_NAME
-    assert_includes last_response.body, "/setup/carddav.mobileconfig"
+      assert_equal 200, last_response.status
+      assert_equal "text/html; charset=utf-8", last_response["Content-Type"]
+      assert_includes last_response.body, "box.example.ts.net"
+      assert_includes last_response.body, ProTacts::Profile::DEFAULT_NAME
+      assert_includes last_response.body, "/setup/carddav.mobileconfig"
+    end
+  end
+
+  # The screen states what the document is about to carry, so a rename
+  # reaching only one of the two would put a name on the screen that the
+  # installed account does not have.
+  def test_the_configured_name_reaches_the_screen_and_the_document
+    with_config("PRO_TACTS_PROFILE_NAME" => "pro-tacts (dev)") do
+      get "/setup"
+
+      assert_includes last_response.body, %(<dd class="type-body-sm">pro-tacts (dev)</dd>)
+
+      get "/setup/carddav.mobileconfig"
+
+      assert_includes last_response.body, "<string>pro-tacts (dev)</string>"
+    end
   end
 
   # The media type is what makes a device offer to install rather than
diff --git a/test/pro_tacts/test_config.rb b/test/pro_tacts/test_config.rb
index a6bf895..0bb0252 100644
--- a/test/pro_tacts/test_config.rb
+++ b/test/pro_tacts/test_config.rb
@@ -57,4 +57,13 @@ class ConfigTest < Minitest::Test
     assert_equal "/tmp/dav.log", ProTacts::Config.new("PRO_TACTS_DEBUG_LOG" => "/tmp/dav.log").debug_log_path
     assert_equal "stderr", ProTacts::Config.new("PRO_TACTS_DEBUG_LOG" => "stderr").debug_log_path
   end
+
+  # nil rather than a default, so the fallback stays Profile's alone.
+  def test_profile_name_is_nil_when_unset
+    assert_nil ProTacts::Config.new({}).profile_name
+  end
+
+  def test_profile_name_is_passed_through
+    assert_equal "pro-tacts (dev)", ProTacts::Config.new("PRO_TACTS_PROFILE_NAME" => "pro-tacts (dev)").profile_name
+  end
 end
diff --git a/test/pro_tacts/test_profile.rb b/test/pro_tacts/test_profile.rb
index ee9e327..41f7839 100644
--- a/test/pro_tacts/test_profile.rb
+++ b/test/pro_tacts/test_profile.rb
@@ -8,6 +8,17 @@ class ProfileTest < Minitest::Test
     ProTacts::Profile.render(hostname:)
   end
 
+  # The name comes from the environment now rather than an argument, so
+  # the tests that care about it swap the config the same way the
+  # install screen's do.
+  def with_config(env = {})
+    original = ProTacts.config
+    ProTacts.config = ProTacts::Config.new(env)
+    yield
+  ensure
+    ProTacts.config = original
+  end
+
   def test_is_well_formed_xml
     refute_nil Nokogiri::XML(render).root
   end
@@ -28,15 +39,19 @@ class ProfileTest < Minitest::Test
   # (dev)") keeps a dev install distinguishable from production, both in
   # Contacts' account list and in System Settings when both are installed.
   def test_name_defaults_to_pro_tacts
-    assert_includes render, "<string>pro-tacts</string>"
+    with_config do
+      assert_includes render, "<string>pro-tacts</string>"
+    end
   end
 
   def test_name_can_be_overridden
-    xml = ProTacts::Profile.render(hostname: "example.ts.net", name: "pro-tacts (dev)")
+    with_config("PRO_TACTS_PROFILE_NAME" => "pro-tacts (dev)") do
+      xml = render
 
-    assert_match(%r{<key>CardDAVAccountDescription</key>\s*<string>pro-tacts \(dev\)</string>}, xml)
-    assert_includes xml, "<string>pro-tacts (dev) CardDAV</string>"
-    assert_includes xml, "<string>example.ts.net</string>"
+      assert_match(%r{<key>CardDAVAccountDescription</key>\s*<string>pro-tacts \(dev\)</string>}, xml)
+      assert_includes xml, "<string>pro-tacts (dev) CardDAV</string>"
+      assert_includes xml, "<string>example.ts.net</string>"
+    end
   end
 
   def test_enables_ssl