Version 0.3.0: resolver/merge/cache refactor, signing design notes

- Split the monolith into package/provider/fetch/merge/cache modules with
  the Resolver deciding install/adopt/update/merge/conflict outcomes
- Three-way merges via git merge-file/diff3 against a content-addressed
  merge-base cache; conflicts go to a .picopackage-merge sibling
- Tests for package, provider, resolver, merge, and cache
- notes.md: update UX and signing design — diff-by-default, SSH signature
  identity pinning (TOFU), key changes as a hard stop, exit-code contract
- Add CLAUDE.md; remove the pre-refactor exe/pppkg monolith and scratch files

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EiyJC73Mz8xZyCTvCEY8qn

lib/picopackage.rb

@@ -1,18 +1,20 @@
 # frozen_string_literal: true
 
 require_relative "picopackage/version"
+require_relative "picopackage/package"
+require_relative "picopackage/cache"
+require_relative "picopackage/merge"
 require_relative "picopackage/fetch"
 require_relative "picopackage/provider"
-require_relative "picopackage/package"
 require_relative "picopackage/scanner"
 require_relative "picopackage/cli"
 
 module Picopackage
   class Error < StandardError; end
 
-  class FileTooLargeError < StandardError; end
+  class FileTooLargeError < Error; end
 
-  class FetchError < StandardError; end
+  class FetchError < Error; end
 
-  class LocalModificationError < StandardError; end
+  class LocalModificationError < Error; end
 end

lib/picopackage/cache.rb

@@ -0,0 +1,60 @@
+# frozen_string_literal: true
+
+require "fileutils"
+
+module Picopackage
+  # Content-addressed store of upstream payloads, kept so that updates have a
+  # merge base to work from.
+  #
+  # A three-way merge needs the common ancestor: the payload exactly as upstream
+  # last sent it. Metadata already records that payload's checksum in
+  # `base_checksum`, so the checksum doubles as the cache key and a package
+  # carries the pointer to its own ancestor.
+  class Cache
+    def self.default_root
+      ENV["PICOPACKAGE_CACHE"] ||
+        File.join(ENV["XDG_CACHE_HOME"] || File.join(Dir.home, ".cache"), "picopackage")
+    end
+
+    attr_reader :root
+
+    def initialize(root: self.class.default_root)
+      @root = root
+    end
+
+    # Returns the checksum the payload was stored under, for recording as a
+    # `base_checksum`.
+    def store(payload)
+      Payload.checksum(payload).tap do |checksum|
+        path = path_for(checksum)
+        next if File.exist?(path)
+
+        FileUtils.mkdir_p(File.dirname(path))
+        # Written via a sibling tempfile and renamed so a concurrent reader
+        # never sees a half-written ancestor.
+        tmp = "#{path}.#{Process.pid}.tmp"
+        File.write(tmp, payload)
+        File.rename(tmp, path)
+      end
+    end
+
+    def fetch(checksum)
+      path = path_for(checksum)
+      File.read(path) if path && File.exist?(path)
+    end
+
+    def include?(checksum) = !fetch(checksum).nil?
+
+    private
+
+    # "sha256:abcdef…" => <root>/sha256/ab/cdef…
+    def path_for(checksum)
+      return nil if checksum.nil? || checksum.empty?
+
+      algorithm, digest = checksum.split(":", 2)
+      return nil if digest.nil? || digest.length < 3 || !digest.match?(/\A[a-f0-9]+\z/i)
+
+      File.join(root, algorithm, digest[0, 2], digest[2..])
+    end
+  end
+end

lib/picopackage/cli.rb

@@ -1,140 +1,146 @@
+# frozen_string_literal: true
+
 require "optparse"
 
 module Picopackage
   class CLI
+    COMMANDS = %w[install update package verify inspect scan].freeze
+
     def self.run(argv = ARGV)
       command = argv.shift
+
       case command
-      when "scan"
-        options = {}
-        OptionParser.new do |opts|
-          opts.banner = "Usage: ppkg scan [options] DIRECTORY"
-          # opts.on('-v', '--verbose', 'Run verbosely') { |v| options[:verbose] = v }
-        end.parse!(argv)
-
-        dir = argv.first || "."
-        Picopackage::Scanner.scan(dir).each { |f| puts f.file_path }
-
-      when "init"
-        OptionParser.new do |opts|
-          opts.banner = "Usage: ppkg init FILE"
-        end.parse!(argv)
-
-        file = argv.first
-        Picopackage::Package.from_file(file).init_metadata
-
-      when "checksum"
-        OptionParser.new do |opts|
-          opts.banner = "Usage: ppkg checksum FILE"
-        end.parse!(argv)
-        file = argv.first
-        puts Picopackage::Package.from_file(file).checksum
-
-      when "verify"
-        OptionParser.new do |opts|
-          opts.banner = "Usage: ppkg sign FILE"
-        end.parse!(argv)
-
-        path = argv.first
-        source = Package.from_file(path)
-
-        if source.metadata["content_checksum"].nil?
-          puts "⚠️  No checksum found in #{path}"
-          puts "Run 'ppkg sign #{path}' to add one"
-          exit 1
-        end
-
-        unless source.verify_payload
-          puts "❌ Checksum verification failed for #{path}"
-          puts "Expected: #{source.metadata["content_checksum"]}"
-          puts "Got:      #{source.checksum}"
-          exit 1
-        end
-
-        puts "✅ #{path} verified successfully"
-
-      when "inspect"
-        OptionParser.new do |opts|
-          opts.banner = "Usage: ppkg inspect FILE|DIRECTORY"
-        end.parse!(argv)
-
-        path = argv.first
-        Picopackage::Package.from_file(path).inspect_metadata
-
-      when "fetch"
+      when "install"
         options = {force: false}
         OptionParser.new do |opts|
-          opts.banner = "Usage: ppkg fetch [options] URI [PATH]"
-          opts.on("-f", "--force", "Force fetch") { |f| options[:force] = f }
+          opts.banner = "Usage: ppkg install [options] URL [DIRECTORY]"
+          opts.on("-f", "--force", "Overwrite local changes") { options[:force] = true }
         end.parse!(argv)
 
         url = argv.shift
-        path = argv.shift || "."  # use '.' if no path provided
+        destination = argv.shift || "."
+        abort "Error: URL is required" if url.nil?
 
-        if url.nil?
-          puts "Error: URI is required"
-          exit 1
-        end
-
-        begin
-          Fetch.fetch(url, path, force: options[:force])
-        rescue LocalModificationError => e
-          puts "Error: #{e.message}"
-        rescue => e
-          puts "Error: #{e.message}"
-          exit 1
-        end
+        report Fetch.fetch(url, destination, force: options[:force])
 
       when "update"
         options = {force: false}
         OptionParser.new do |opts|
           opts.banner = "Usage: ppkg update [options] FILE"
-          opts.on("-f", "--force", "Force update") { |f| options[:force] = f }
+          opts.on("-f", "--force", "Discard local changes") { options[:force] = true }
         end.parse!(argv)
 
-        file = argv.first
-        package = Package.from_file(file)
-        begin
-          Fetch.fetch(package.url, File.dirname(file), force: options[:force])
-        rescue LocalModificationError => e
-          puts "Error: #{e.message}"
-        rescue => e
-          puts "Error: #{e.message}"
+        path = argv.shift
+        abort "Error: FILE is required" if path.nil?
+
+        package = load_package(path)
+        abort "Error: #{path} has no url in its metadata, so there is nothing to update from" if package.url.nil? || package.url.empty?
+
+        # Updates write back to the file the user named, not to whatever
+        # filename upstream would prefer — renaming a package locally is
+        # allowed and shouldn't silently install a second copy.
+        report Fetch.fetch(package.url, File.dirname(path), force: options[:force], filename: File.basename(path))
+
+      when "package", "init"
+        options = {}
+        OptionParser.new do |opts|
+          opts.banner = "Usage: ppkg package [--url URL] FILE"
+          opts.on("--url URL", "Where updates should be fetched from") { |url| options[:url] = url }
+        end.parse!(argv)
+
+        path = argv.shift
+        abort "Error: FILE is required" if path.nil?
+
+        package = load_package(path)
+        was_bare = package.bare?
+        package.init_metadata(filename: File.basename(path), url: options[:url])
+        package.save(path)
+        Cache.new.store(package.payload)
+
+        puts was_bare ? "Packaged #{path}" : "Updated metadata in #{path}"
+        warn "Note: no url recorded — 'ppkg update' won't work until one is set with --url" if package.url.nil?
+
+      when "verify"
+        OptionParser.new { |opts| opts.banner = "Usage: ppkg verify FILE" }.parse!(argv)
+
+        path = argv.shift
+        abort "Error: FILE is required" if path.nil?
+
+        package = load_package(path)
+
+        if package.payload_checksum.nil?
+          puts "⚠️  No checksum found in #{path}"
+          puts "Run 'ppkg package #{path}' to add one"
           exit 1
         end
 
+        unless package.verify_payload
+          puts "❌ #{path} does not match its recorded checksum"
+          puts "Recorded: #{package.payload_checksum}"
+          puts "Actual:   #{package.payload_digest}"
+          exit 1
+        end
+
+        puts "✅ #{path} matches its recorded checksum"
+
+      when "inspect"
+        OptionParser.new { |opts| opts.banner = "Usage: ppkg inspect FILE" }.parse!(argv)
+
+        path = argv.shift
+        abort "Error: FILE is required" if path.nil?
+        load_package(path).inspect_metadata
+
+      when "scan"
+        OptionParser.new { |opts| opts.banner = "Usage: ppkg scan [DIRECTORY]" }.parse!(argv)
+
+        found = Scanner.scan(argv.shift || ".")
+        found.each { |entry| puts [entry.path, entry.package.url].compact.join("\t") }
+        puts "No picopackages found" if found.empty?
+
+      when "--version", "-v", "version"
+        puts Picopackage::VERSION
+
+      when nil, "help", "--help", "-h"
+        usage
+        exit(command.nil? ? 1 : 0)
+
       else
-        puts "Unknown command: #{command}"
-        puts "Available commands: fetch, update, scan, sign, inspect"
+        warn "Unknown command: #{command}"
+        usage
         exit 1
       end
-    rescue OptionParser::InvalidOption => e
-      puts e.message
-      exit 1
-    rescue => e
-      puts "Error: #{e.message}"
-      puts e.backtrace if ENV["DEBUG"]
+    rescue OptionParser::ParseError => e
+      abort e.message
+    rescue FetchError, Fetch::Error, ArgumentError => e
+      warn "Error: #{e.message}"
+      warn e.backtrace if ENV["DEBUG"]
       exit 1
     end
 
-    def self.determine_script_source
-      # Get the full path of the currently executing script
-      current_path = File.expand_path($0)
+    def self.usage
+      puts <<~TEXT
+        Usage: ppkg <command> [options]
 
-      # Check if script is in GEM_PATH
-      gem_paths = Gem.path.map { |p| File.expand_path(p) }
+        Commands:
+          install URL [DIR]   Fetch a picopackage into DIR (default: .)
+          update FILE         Re-fetch FILE from the url in its metadata
+          package FILE        Add a metadata block to a local file
+          verify FILE         Check FILE against its recorded checksum
+          inspect FILE        Print FILE's metadata as JSON
+          scan [DIR]          List picopackages under DIR
 
-      is_gem = gem_paths.any? { |path| current_path.start_with?(path) }
+        Run 'ppkg <command> --help' for command options.
+      TEXT
+    end
 
-      if is_gem
-        # Running from gem installation
-        gem_name = File.basename(File.dirname(File.dirname(current_path)))
-        version = File.basename(File.dirname(current_path))
-        {source: :gem, path: current_path, gem_name: gem_name, version: version}
-      else
-        # Running from local installation
-        {source: :local, path: current_path}
-      end
+    def self.load_package(path)
+      abort "Error: no such file: #{path}" unless File.file?(path)
+      Package.from_file(path) or abort "Error: could not read #{path}"
+    end
+
+    def self.report(result)
+      puts result.message
+      exit 1 if result.conflict?
     end
   end
 end

lib/picopackage/fetch.rb

@@ -1,168 +1,278 @@
+# frozen_string_literal: true
+
 require "net/http"
+require "openssl"
 require "fileutils"
-require "tempfile"
 require "json"
-require "debug"
 
 module Picopackage
   class Fetch
     class Error < StandardError; end
-    class HTTPError < Error; end
-    class FileTooLargeError < Error; end
-    class NotModifiedError < Error; end  # Add this
-    class TooManyRedirectsError < Error; end  # Add this
 
-    MAX_REDIRECTS = 5 # This constant is used but not defined
+    class HTTPError < Error; end
+
+    class FileTooLargeError < Error; end
+
+    class NotModifiedError < Error; end
+
+    class TooManyRedirectsError < Error; end
+
+    MAX_REDIRECTS = 5
+    USER_AGENT = "picopackage/#{Picopackage::VERSION}"
+    SCHEMES = %w[http https].freeze
 
     def initialize(max_size: 1024 * 1024, timeout: 10)
       @max_size = max_size
       @timeout = timeout
     end
 
-    def fetch(uri)
+    def fetch(uri, etag: nil)
+      uri = URI.parse(uri.to_s) unless uri.is_a?(URI)
       case uri.scheme
-      when "http", "https" then fetch_http(uri)
+      when "http", "https" then fetch_http(uri, etag: etag)
       when "file" then fetch_file(uri)
       else
-        raise Error, "Unsupported scheme: #{uri.scheme}"
+        raise Error, "Unsupported scheme: #{uri.scheme.inspect}"
       end
     end
 
-    def self.fetch(url, destination, force: false)
+    # Fetches a URL and reconciles it with whatever is already on disk.
+    # Returns a Resolver::Result; deciding what to do about a conflict is the
+    # caller's business, not ours.
+    def self.fetch(url, destination, force: false, filename: nil, cache: Cache.new)
       raise ArgumentError, "Destination directory does not exist: #{destination}" unless Dir.exist?(destination)
+
       provider = Provider.for(url)
-      package = provider.package
-      file_path = File.join(destination, package.filename)
+      raise FetchError, "No provider could handle #{url}" if provider.nil?
 
-      local_package = File.exist?(file_path) ? FileProvider.new(file_path).package : nil
+      remote = provider.package
+      target = filename || remote.filename
+      raise FetchError, "Could not determine a filename for #{url}" if target.nil?
 
-      resolver = Resolver.new(package, local_package, file_path, force: force).resolve
+      file_path = File.join(destination, File.basename(target))
+      local = Package.from_file(file_path)
 
-      case resolver[:state]
-      when :kept, :updated
-        puts resolver[:message]
-      when :conflict
-        raise LocalModificationError, resolver[:message]
-      end
-      provider.package
+      Resolver.new(remote, local, file_path, force: force, cache: cache).resolve
     end
 
     private
 
-    def fetch_http(uri, etag = nil)
-      Net::HTTP.start(uri.host, uri.port, connection_options(uri)) do |http|
-        request = Net::HTTP::Get.new(uri.request_uri)
+    # Net::HTTP rather than URI.open, deliberately. This method's whole job is to
+    # dereference a URL a user handed us, and URI.open treats a leading "|" as a
+    # command to run. The scheme check in #fetch already blocks that, but a
+    # remote-code-fetching tool should not be one refactor away from shelling
+    # out, and doing it by hand is also what makes redirect and 304 handling
+    # possible.
+    def fetch_http(uri, etag: nil, redirects: MAX_REDIRECTS)
+      raise TooManyRedirectsError, "More than #{MAX_REDIRECTS} redirects from #{uri}" if redirects.negative?
+
+      Net::HTTP.start(uri.host, uri.port, use_ssl: uri.scheme == "https",
+        open_timeout: @timeout, read_timeout: @timeout) do |http|
+        request = Net::HTTP::Get.new(uri)
+        request["User-Agent"] = USER_AGENT
+        request["Accept"] = "*/*"
         request["If-None-Match"] = etag if etag
 
-        response = http.request(request)
-        handle_response(response, uri)
+        http.request(request) do |response|
+          case response
+          when Net::HTTPNotModified
+            raise NotModifiedError, "#{uri} is unchanged"
+          when Net::HTTPRedirection
+            return fetch_http(redirect_target(uri, response), etag: etag, redirects: redirects - 1)
+          when Net::HTTPSuccess
+            return read_capped(response)
+          else
+            raise HTTPError, "#{response.code} #{response.message} for #{uri}"
+          end
+        end
       end
+    rescue Net::OpenTimeout, Net::ReadTimeout
+      raise HTTPError, "Timed out after #{@timeout}s fetching #{uri}"
+    rescue SocketError, SystemCallError, OpenSSL::SSL::SSLError => e
+      raise HTTPError, "Could not fetch #{uri}: #{e.message}"
     end
 
-    def fetch_file(uri)
-      File.read(uri.path)
-    end
-
-    def connection_options(uri)
-      {
-        use_ssl: uri.scheme == "https",
-        read_timeout: @timeout,
-        open_timeout: @timeout
-      }
-    end
-
-    def handle_response(response, uri)
-      case response
-      when Net::HTTPSuccess
-        {
-          body: read_body(response),
-          etag: response["ETag"]
-        }
-      when Net::HTTPNotModified
-        raise NotModifiedError.new("Resource not modified", etag: response["ETag"])
-      when Net::HTTPRedirection
-        handle_redirect(response, uri)
-      else
-        raise HTTPError, "HTTP #{response.code}: #{response.message}"
-      end
-    end
-
-    def handle_redirect(response, uri, redirect_count = 0)
-      raise TooManyRedirectsError if redirect_count >= MAX_REDIRECTS
+    def redirect_target(uri, response)
       location = response["location"]
-      new_uri = URI(location)
-      # Handle both relative paths and full URLs
-      new_uri = uri.merge(location) if new_uri.relative?
-      fetch(new_uri, redirect_count: redirect_count + 1)
+      raise HTTPError, "Redirect from #{uri} with no Location header" if location.nil?
+
+      URI.join(uri, location).tap do |target|
+        raise Error, "Refusing to follow a redirect to #{target.scheme.inspect}" unless SCHEMES.include?(target.scheme)
+      end
     end
 
-    def read_body(response)
+    # Trusts Content-Length only to fail early — the streamed tally is what
+    # actually enforces the cap, since the header can lie or be absent.
+    def read_capped(response)
+      declared = response["content-length"]&.to_i
+      raise FileTooLargeError, "#{declared} bytes exceeds the #{@max_size} byte limit" if declared&.positive? && declared > @max_size
+
       buffer = String.new(capacity: @max_size)
       response.read_body do |chunk|
-        raise FileTooLargeError, "Response would exceed #{@max_size} bytes" if buffer.bytesize + chunk.bytesize > @max_size
+        raise FileTooLargeError, "Response exceeds the #{@max_size} byte limit" if buffer.bytesize + chunk.bytesize > @max_size
+
         buffer << chunk
       end
       buffer
     end
+
+    def fetch_file(uri) = File.read(uri.path)
   end
 
   ##
+  # Decides what happens when an upstream payload meets a local file.
+  #
+  # Two rules drive every branch, and they are worth stating because they are
+  # what make the table in notes.md collapse to something small:
+  #
+  #   1. The checksum decides, never the timestamp. mtime is not evidence about
+  #      content — a file re-uploaded unchanged gets a fresh one — so equal
+  #      checksums end the content question outright, whatever the clocks say.
+  #   2. Merges run on the payload alone. The metadata block is derived, so
+  #      merging it would conflict on every update; it is regenerated after.
+  #
   # States:
-  # - kept: local file was converted to a picopackage and kept
-  # - updated: local file was updated with remote picopackage
-  # - conflict: local and remote files differ - manually resolve or use -f to force
+  # - installed: no local file existed, upstream was written
+  # - adopted:   a bare local file gained metadata; its content already matched
+  # - current:   nothing to do
+  # - updated:   local payload replaced with upstream
+  # - merged:    local edits preserved, upstream changes applied on top
+  # - conflict:  needs a human; nothing was written to the original file
   class Resolver
-    attr_reader :remote, :local, :local_path, :force
-    def initialize(remote_package, local_package, local_path, force: false)
+    STATES = %i[installed adopted current updated merged conflict].freeze
+
+    Result = Struct.new(:state, :message, :path, keyword_init: true) do
+      def conflict? = state == :conflict
+
+      def written? = %i[installed adopted updated merged].include?(state)
+    end
+
+    MERGE_SUFFIX = ".picopackage-merge"
+
+    attr_reader :remote, :local, :local_path, :force, :cache
+
+    def initialize(remote_package, local_package, local_path, force: false, cache: Cache.new)
       @remote = remote_package
       @local = local_package
       @local_path = local_path
       @force = force
-      @same_checksum = @remote.payload_checksum == @local&.payload_checksum
+      @cache = cache
     end
 
-    STATES = %i[kept updated conflict].freeze
-
     def resolve
-      validate_state_hash(
-        if @force
-          @remote.save(local_path)
-          {state: :updated, message: "Force mode: overwrote local file with remote package"}
-        elsif @local.nil?
-          @remote.save(local_path)
-          {state: :kept, message: "Saved Package as new file"}
-        elsif @remote.payload_version != @local.payload_version
-          {state: :conflict, message: "Version conflict. Local: #{@local.payload_version}, Remote: #{@remote.payload_version}"}
-        elsif @remote.payload_timestamp_as_time > @local.payload_timestamp_as_time
-          @remote.save(local_path)
-          {state: :updated, message: "Updated to newer version"}
-        elsif !@same_checksum
-          handle_checksum_mismatch
-        elsif @local.was_bare_file
-          debugger
-          @local.save(local_path)
-          {state: :kept, message: "Packaged existing file as Picopackage"}
+      validate(
+        if local.nil?
+          write(:installed, "Installed #{File.basename(local_path)}")
+        elsif force
+          write(:updated, "Forced: overwrote #{File.basename(local_path)} with upstream")
+        elsif same_payload?
+          reconcile_metadata
+        elsif local.bare?
+          # No metadata, no ancestor, and a different payload: this is an
+          # unrelated file that happens to share a name, not a modified
+          # picopackage. There is no shared history to merge along.
+          conflict("#{File.basename(local_path)} already exists, is not a picopackage, and differs from upstream. " \
+                   "Move it aside or use --force to overwrite it.")
+        elsif !local.diverged?
+          write(:updated, "Updated #{File.basename(local_path)} to upstream")
+        elsif upstream_unchanged?
+          keep_local_changes
         else
-          {state: :kept, message: "Local file is up to date"}
+          merge
         end
       )
     end
 
     private
 
-    def validate_state_hash(hash)
-      raise "Invalid state" unless STATES.include?(hash[:state])
-      raise "Missing message" unless hash[:message].is_a?(String)
-      hash
+    def same_payload? = remote.payload_digest == local.payload_digest
+
+    # Same content on both sides, so the only open question is whose metadata
+    # to keep. A bare local file gets adopted; a package is already fine.
+    def reconcile_metadata
+      cache.store(remote.payload)
+
+      if local.bare?
+        write(:adopted, "Adopted existing #{File.basename(local_path)} as a picopackage")
+      else
+        Result.new(state: :current, message: "#{File.basename(local_path)} is up to date", path: local_path)
+      end
     end
 
-    def handle_checksum_mismatch
-      if @force
-        @remote.save(local_path)  # In force mode, remote wins
-        {state: :updated, message: "Overwrote local file with remote package"}
-      else
-        {state: :conflict, message: "Files differ. Use --force to convert both to packages"}
+    # Reached only when the payloads differ and the local one has diverged, so
+    # the difference is entirely ours: upstream still holds the ancestor we
+    # branched from.
+    def upstream_unchanged? = remote.payload_digest == local.ancestor_checksum
+
+    # Nothing upstream to apply. Merging would be a correct no-op — base and
+    # theirs are the same text, so the result is ours — but it spawns a merge
+    # tool to prove it and reports `:merged` when nothing was merged.
+    #
+    # Storing the payload is not redundant: this is the one moment we are
+    # holding the ancestor itself, so a cache that has been cleared gets
+    # repopulated and the *next* update can still merge.
+    def keep_local_changes
+      cache.store(remote.payload)
+      Result.new(state: :current,
+        message: "#{File.basename(local_path)} is up to date; your local changes are untouched",
+        path: local_path)
+    end
+
+    def merge
+      return conflict("#{File.basename(local_path)} has local changes and no merge tool (git or diff3) is available.") unless Merge.available?
+
+      base = cache.fetch(local.ancestor_checksum)
+      if base.nil?
+        return conflict("#{File.basename(local_path)} has local changes and its original upstream version " \
+                        "isn't in the cache, so there is nothing to merge against. " \
+                        "Diff it against upstream by hand, or use --force to discard your changes.")
       end
+
+      upstream_digest = remote.payload_digest
+      cache.store(remote.payload)
+      result = Merge.three_way(base: base, ours: local.payload, theirs: remote.payload)
+
+      case result.status
+      when :clean
+        remote.payload = result.content
+        remote.base_checksum = upstream_digest
+        remote.save(local_path)
+        Result.new(state: :merged, message: "Merged upstream changes into #{File.basename(local_path)}, keeping your edits", path: local_path)
+      when :conflict
+        write_conflict_file(result)
+      else
+        conflict("Merge tool failed on #{File.basename(local_path)}.")
+      end
+    end
+
+    # The original is never touched. A tool with no index and no `reset` has no
+    # business writing conflict markers into a file the user may not have
+    # committed, so the markers go to a sibling and the user drives.
+    def write_conflict_file(result)
+      path = local_path + MERGE_SUFFIX
+      File.write(path, result.content)
+      conflict("#{File.basename(local_path)} has local changes that conflict with upstream " \
+               "(#{result.conflicts} #{(result.conflicts == 1) ? "conflict" : "conflicts"}). " \
+               "Resolved-with-markers copy written to #{File.basename(path)}; your file is untouched.")
+    end
+
+    def write(state, message)
+      # Metadata describes *this* file, so the filename follows the local name
+      # even when it differs from upstream's. `url` is what points upstream.
+      remote.filename = File.basename(local_path)
+      remote.base_checksum = remote.payload_digest
+      cache.store(remote.payload)
+      path = remote.save(local_path)
+      Result.new(state: state, message: message, path: path)
+    end
+
+    def conflict(message) = Result.new(state: :conflict, message: message, path: local_path)
+
+    def validate(result)
+      raise "Invalid state: #{result.state.inspect}" unless STATES.include?(result.state)
+      raise "Missing message" unless result.message.is_a?(String)
+
+      result
     end
   end
 end

lib/picopackage/merge.rb

@@ -0,0 +1,94 @@
+# frozen_string_literal: true
+
+require "tempfile"
+
+module Picopackage
+  # Three-way merge of payloads, delegated to `git merge-file` or POSIX `diff3`.
+  #
+  # Reimplementing diff3 in Ruby would be a few hundred lines of the most
+  # bug-prone code in the project, to produce output people already know how to
+  # read. Both external tools take their arguments in the same order — ours,
+  # base, theirs — and both report a clean merge with status 0 and conflicts
+  # with a positive status, so the two backends differ only in the command line.
+  module Merge
+    Result = Struct.new(:status, :content, :conflicts, keyword_init: true) do
+      def clean? = status == :clean
+
+      def conflict? = status == :conflict
+
+      def unavailable? = status == :unavailable
+    end
+
+    LABELS = {ours: "local", base: "ancestor", theirs: "upstream"}.freeze
+
+    module_function
+
+    # Returns :git, :diff3, or nil.
+    def backend
+      return @backend if defined?(@backend)
+
+      @backend = if command?("git")
+        :git
+      elsif command?("diff3")
+        :diff3
+      end
+    end
+
+    def available? = !backend.nil?
+
+    def command?(name)
+      ENV.fetch("PATH", "").split(File::PATH_SEPARATOR).any? do |dir|
+        File.executable?(File.join(dir, name)) && !File.directory?(File.join(dir, name))
+      end
+    end
+
+    def three_way(base:, ours:, theirs:, labels: LABELS)
+      return Result.new(status: :unavailable, content: nil, conflicts: 0) unless available?
+
+      with_temp_files(ours, base, theirs) do |ours_path, base_path, theirs_path|
+        merged, status = run(backend, ours_path, base_path, theirs_path, labels)
+
+        # Negative (255 as unsigned) means the tool itself failed rather than
+        # finding conflicts — don't report that as a mergeable conflict.
+        if status.nil? || status > 128
+          Result.new(status: :unavailable, content: nil, conflicts: 0)
+        elsif status.zero?
+          Result.new(status: :clean, content: merged, conflicts: 0)
+        else
+          Result.new(status: :conflict, content: merged, conflicts: status)
+        end
+      end
+    end
+
+    def run(backend, ours_path, base_path, theirs_path, labels)
+      argv = case backend
+      when :git
+        ["git", "merge-file", "-p", "--diff3",
+          "-L", labels[:ours], "-L", labels[:base], "-L", labels[:theirs],
+          ours_path, base_path, theirs_path]
+      when :diff3
+        ["diff3", "-m",
+          "-L", labels[:ours], "-L", labels[:base], "-L", labels[:theirs],
+          ours_path, base_path, theirs_path]
+      end
+
+      output = IO.popen(argv, err: File::NULL, &:read)
+      [output, $?&.exitstatus]
+    rescue SystemCallError
+      [nil, nil]
+    end
+
+    def with_temp_files(ours, base, theirs)
+      files = {ours: ours, base: base, theirs: theirs}.map do |name, content|
+        file = Tempfile.new(["picopackage-#{name}", ".txt"])
+        file.binmode
+        file.write(content)
+        file.flush
+        file
+      end
+      yield(*files.map(&:path))
+    ensure
+      files&.each { |f| f.close! }
+    end
+  end
+end

lib/picopackage/package.rb

@@ -1,33 +1,48 @@
+# frozen_string_literal: true
+
 require "yaml"
+require "json"
 require "digest"
+require "time"
 require "forwardable"
 
 module Picopackage
   METADATA_PATTERN = /^\n*#\s*@PICOPACKAGE_START\n(.*?)^\s*#\s*@PICOPACKAGE_END\s*$/m
 
-  class Metadata < Struct.new(:url, :filename, :payload_version, :payload_timestamp, :payload_checksum, :etag, keyword_init: true)
-    # the #from_file method will create a new instance of Metadata from a file path, rather than read a package's metadata
-    def self.from_file(file_path, content: nil)
-      new(content: File.read(file_path))
-    end
-
-    def self.from_url_response(url, response)
-    end
-
+  class Metadata < Struct.new(:url, :filename, :payload_version, :payload_timestamp,
+    :payload_checksum, :base_checksum, :etag, keyword_init: true)
     def self.from_content(content)
-      return new unless content =~ METADATA_PATTERN
+      return new unless (match = METADATA_PATTERN.match(content))
 
-      yaml_content = $1.each_line.map { |line| line.sub(/^\s*#\s?/, "").rstrip }.join("\n")
+      parsed = YAML.safe_load(uncomment(match[1]))
+      return new unless parsed.is_a?(Hash)
 
-      # Load and transform in one chain
-      @metadata = new(**YAML.safe_load(yaml_content)
-                                    .slice(*Metadata.members.map(&:to_s))
-                                    .transform_keys(&:to_sym))
-    rescue
-      new # Return empty hash on any YAML/transformation errors
+      known, unknown = parsed.partition { |key, _| members.include?(key.to_s.to_sym) }
+      new(**known.to_h.transform_keys(&:to_sym)).tap { |metadata| metadata.extra = unknown.to_h }
+    rescue Psych::Exception, TypeError, ArgumentError
+      new
     end
 
-    def empty? = to_h.values.all?(&:nil?)
+    def self.from_file(path) = from_content(File.read(path))
+
+    def self.uncomment(block)
+      block.each_line.map { |line| line.sub(/^\s*#\s?/, "").rstrip }.join("\n")
+    end
+
+    # Keys the struct doesn't model. The spec has a long tail of optional fields
+    # — licence, test_url, signature, parent_revision — and a tool that dropped
+    # the ones it didn't recognise would quietly destroy them the first time it
+    # rewrote a file. Round-tripping them costs nothing and means old tools
+    # can't corrupt packages written by newer ones.
+    attr_writer :extra
+
+    def extra = @extra ||= {}
+
+    def empty? = to_h.values.all?(&:nil?) && extra.empty?
+
+    # Modelled members in declaration order, then unmodelled keys sorted, so a
+    # rewrite that changes one field produces a one-line diff.
+    def to_yaml_hash = to_h.compact.transform_keys(&:to_s).merge(extra.sort.to_h)
   end
 
   class Payload
@@ -35,94 +50,149 @@ module Picopackage
 
     def self.normalize(payload) = payload.rstrip + "\n\n"
 
-    def self.normalized_from_content(content) = Payload.from_content(content).then { Payload.normalize(_1) }
+    def self.normalized_from_content(content) = normalize(from_content(content))
 
-    def self.from_file(file_path) = normalized_from_content(File.read(file_path))
+    def self.from_file(path) = normalized_from_content(File.read(path))
 
     def self.checksum(payload) = "sha256:#{Digest::SHA256.hexdigest(payload)}"
 
-    def self.checksum_from_content(content) = checksum(from_content(content))
+    # Always hashes the *normalised* payload. Hashing the raw payload here while
+    # verification hashed the normalised one meant freshly written packages
+    # failed their own checksum, so every file looked locally modified.
+    def self.checksum_from_content(content) = checksum(normalized_from_content(content))
   end
 
   class Package
     extend Forwardable
-    attr_reader :content, :payload, :metadata, :was_bare_file
+
+    attr_reader :content, :payload, :metadata
 
     def_delegators :@metadata,
       :url, :url=,
       :filename, :filename=,
       :payload_version, :payload_version=,
       :payload_timestamp, :payload_timestamp=,
-      :payload_checksum, :payload_checksum=
+      :payload_checksum, :payload_checksum=,
+      :base_checksum, :base_checksum=,
+      :etag, :etag=
 
     def self.from_file(file_path)
-      if File.exist?(file_path)
-        new(content: File.read(file_path))
-      end
+      new(content: File.read(file_path)) if File.exist?(file_path)
     end
 
     def initialize(content:)
       @content = content
-      @payload = Payload.normalized_from_content(@content)
-      @metadata = Metadata.from_content(@content)
-
-      if is_bare_file?
-        @was_bare_file = true
-        init_metadata
-      else
-        @was_bare_file = false
-      end
+      @had_metadata_block = METADATA_PATTERN.match?(content)
+      @payload = Payload.normalized_from_content(content)
+      @metadata = Metadata.from_content(content)
     end
 
-    def is_bare_file? = @metadata.empty?
+    # True when the file arrived with no metadata block at all: a hand-copied
+    # file, or an upstream that was never packaged. Deliberately not "its
+    # metadata parsed to nothing" — a block holding only optional keys we don't
+    # model would answer yes to that and get clobbered as though it were bare.
+    def bare? = !@had_metadata_block
 
-    def init_metadata
-      @metadata.url ||= url
-      @metadata.filename ||= filename
-      @metadata.payload_checksum ||= Payload.checksum_from_content(content)
-      @metadata.payload_timestamp ||= payload_timestamp
-    end
+    # What the payload hashes to right now.
+    def payload_digest = Payload.checksum(payload)
 
-    def save(path, filename = nil)
-      path = File.join(path, filename || @metadata.filename) if File.directory?(path)
+    # Local edits break the recorded checksum, and that divergence is the only
+    # modification signal available — mtime says nothing about content.
+    #
+    # Note the asymmetry with #verify_payload when no checksum is recorded: we
+    # have no *evidence of modification*, but we also cannot *verify*. Both
+    # answers are false, and they mean different things.
+    def modified?
+      return false unless recorded_checksum?
 
-      File.write(path, generate_package)
+      payload_digest != payload_checksum
     end
 
     def verify_payload
-      return false if metadata.payload_checksum.nil? || metadata.payload_checksum&.empty?
-      Payload.checksum(payload) == metadata.payload_checksum
+      return false unless recorded_checksum?
+
+      payload_digest == payload_checksum
+    end
+
+    # The upstream version this file was last reconciled with. Packages written
+    # before `base_checksum` existed, and hand-written metadata, have only
+    # `payload_checksum` — which meant exactly this before merges did.
+    def ancestor_checksum = base_checksum || payload_checksum
+
+    # Whether the payload carries changes relative to upstream — from a human
+    # edit *or* from an earlier merge.
+    #
+    # This, not #modified?, is what decides whether upstream may simply
+    # overwrite the file. A merged file is unmodified (we wrote it, so its
+    # recorded checksum matches) while still holding local edits that a
+    # fast-forward would silently discard.
+    def diverged?
+      return false if ancestor_checksum.nil? || ancestor_checksum.empty?
+
+      payload_digest != ancestor_checksum
+    end
+
+    # Replaces the payload, e.g. with the result of a merge, and re-points the
+    # recorded checksum at it so the file reads as unmodified until a human
+    # touches it. `base_checksum` is left alone: it tracks the upstream ancestor,
+    # which is not what we just wrote.
+    def payload=(new_payload)
+      @payload = Payload.normalize(new_payload)
+      @content = @payload
+      @metadata.payload_checksum = Payload.checksum(@payload)
+    end
+
+    # Fills in what's missing. `payload_checksum` is deliberately absent: it is
+    # derived at write time by #generate_package rather than carried over from
+    # an input, so a stale value can never survive into a file we write.
+    def init_metadata(filename: nil, url: nil)
+      @metadata.filename ||= filename
+      @metadata.url ||= url
+      @metadata.payload_timestamp ||= Time.now.utc.iso8601
+      @metadata.base_checksum ||= payload_digest
+      self
     end
 
     def payload_timestamp_as_time
-      @metadata&.payload_timestamp ? Time.parse(@metadata.payload_timestamp) : nil
+      payload_timestamp ? Time.parse(payload_timestamp.to_s) : nil
+    rescue ArgumentError
+      nil
     end
 
-    def modified? = !verify_payload
+    # Returns the path written to.
+    def save(path, filename = nil)
+      path = File.join(path, filename || @metadata.filename) if File.directory?(path)
+      File.write(path, generate_package)
+      path
+    end
 
-    def inspect_metadata = puts JSON.pretty_generate(@metadata.to_h)
-
-    private
+    def inspect_metadata = puts(JSON.pretty_generate(@metadata.to_yaml_hash))
 
     def generate_package
-      @metadata.url = url.to_s
-      metadata_block = generate_metadata
-      if METADATA_PATTERN.match?(content)
-        content.sub(METADATA_PATTERN, "\n#{metadata_block}")
+      @metadata.url = url.to_s unless url.nil?
+      # The single choke point for the rule that a checksum is *derived*, never
+      # adopted or preserved. Every write goes through here, so every file we
+      # produce verifies against itself no matter which path built the package —
+      # including one built from an upstream block whose author edited the file
+      # without re-running the tool.
+      @metadata.payload_checksum = payload_digest
+      block = generate_metadata
+
+      if METADATA_PATTERN.match?(@content)
+        @content.sub(METADATA_PATTERN, "\n#{block}")
       else
-        [content.rstrip, "\n#{metadata_block}"].join("\n")
+        [@content.rstrip, "\n#{block}"].join("\n")
       end
     end
 
-    # This will need a comment style one day, to work with other languages
+    private
+
+    def recorded_checksum? = !(payload_checksum.nil? || payload_checksum.empty?)
+
+    # This will need a comment style one day, to work with other languages.
     def generate_metadata
-      yaml_content = @metadata.to_h.transform_keys(&:to_s).to_yaml.strip
-      [
-        "# @PICOPACKAGE_START",
-        yaml_content.lines.map { |line| "# #{line}" }.join,
-        "# @PICOPACKAGE_END",
-        ""
-      ].join("\n")
+      commented = @metadata.to_yaml_hash.to_yaml.strip.each_line.map { |line| "# #{line.chomp}".rstrip }
+      ["# @PICOPACKAGE_START", *commented, "# @PICOPACKAGE_END", ""].join("\n")
     end
   end
 end

lib/picopackage/provider.rb

@@ -1,3 +1,5 @@
+# frozen_string_literal: true
+
 require "time"
 require "pathname"
 
@@ -6,34 +8,49 @@ module Picopackage
     def self.for(url)
       PROVIDERS.each do |provider|
         case provider.handles_url?(url)
-        when false
+        when false, nil
           next
         when true
           return provider.new(url)
         when :maybe
-          instance = provider.new(url)
-          return instance if instance.handles_body?
+          # A `:maybe` provider proves itself by fetching, and providers fetch in
+          # their constructor — so a speculative provider that guesses wrong
+          # (OpenGistProvider appends ".json" to every URL it sees) raises here.
+          # That has to fall through to the next candidate rather than abort the
+          # whole resolution, or no plain URL would ever reach DefaultProvider.
+          begin
+            instance = provider.new(url)
+            return instance if instance.handles_body?
+          rescue FetchError, Fetch::Error, URI::Error
+            next
+          end
         end
       end
-      nil  # Return nil if no provider found
+      nil
     end
   end
 
-  # Base class for fetching content from a URL
-  # The variable `body` will contain the package_data retrieved from the URL
-  # The variable `package_data` will contain both and payload + metadata - this would be writen to a file.
-  # The variable `payload` will contain the payload extracted from `package_data`
-  # The variable `metadata` will contain the metadata extracted from `package_data`
-
-  # Job of the Provider class is to fetch the body from the URL, and then extract the package_data
-  # and the filename from the body. The Package class will then take the body and split it into payload and metadata
-
+  # Base class for fetching content from a URL.
+  #
+  # A provider's job is to fetch the body, and to pull the package content and a
+  # filename out of it. Splitting content into payload and metadata is the
+  # Package class's job, not a provider's.
   class DefaultProvider
     MAX_SIZE = 1024 * 1024
     TIMEOUT = 10
+
     attr_reader :url, :package
 
-    def self.handles_url?(url) = :maybe
+    # Not `:maybe`. This is the last resort in PROVIDERS, so if it declined on a
+    # failed fetch the loop would fall off the end and report "no provider could
+    # handle this" for what is really a 404. Claiming any URL it understands the
+    # scheme of lets the actual HTTP error reach the user.
+    def self.handles_url?(url)
+      scheme = URI(url.to_s).scheme
+      %w[http https file].include?(scheme)
+    rescue URI::InvalidURIError
+      false
+    end
 
     def initialize(url, fetcher: Fetch.new(max_size: MAX_SIZE, timeout: TIMEOUT))
       @url = transform_url(url)
@@ -42,7 +59,7 @@ module Picopackage
       populate_metadata
     end
 
-    def transform_url(url) = URI(url)
+    def transform_url(url) = URI(url.to_s)
 
     def body
       @body ||= @fetcher.fetch(@url)
@@ -56,78 +73,104 @@ module Picopackage
       raise FetchError, "Failed to parse JSON response"
     end
 
-    def payload_timestamp = Time.now.httpdate
+    # ISO 8601 throughout: sortable, unambiguous, and the same shape from every
+    # provider, which httpdate-here-iso-there was not.
+    def payload_timestamp = Time.now.utc.iso8601
 
     def handles_body?
-      true
-    rescue FileTooLargeError, Net::HTTPError, RuntimeError
+      !body.nil? && !body.empty?
+    rescue FetchError, FileTooLargeError
       false
     end
 
-    # Implement in subclass - this come from the `body`.
-    # Spliting content into payload and metadata is the job of the Package class
+    # Implement in a subclass when the content is wrapped in something (JSON,
+    # HTML). Splitting payload from metadata is the Package class's job.
     def content = body
 
-    # Implement in subclass - this should return the filename extracted from the body - if it exists, but not from the metadata
-    def filename = File.basename @url
+    # Implement in a subclass when the body carries a filename. Not to be
+    # confused with the filename in a package's metadata.
+    def filename
+      candidate = File.basename(@url.path.to_s)
+      candidate.empty? ? nil : candidate
+    end
 
+    # Upstream may or may not carry a metadata block, and the two halves of one
+    # are not equally trustworthy:
+    #
+    # - *Claims* are upstream's to make — the canonical url (which outlives the
+    #   mirror we happened to fetch from), payload_version, licence, signature.
+    #   Those are adopted with `||=`.
+    # - *Records* describe a particular copy on a particular disk, so upstream
+    #   cannot speak for ours. Checksums are recomputed from the payload we
+    #   actually received. Adopting a stale payload_checksum installed a file
+    #   that failed its own verification and read as locally modified before
+    #   anyone had touched it.
+    #
+    # The checksum covers the extracted payload, not the raw body: hashing the
+    # body meant an upstream carrying a metadata block hashed its own metadata,
+    # so it never matched the same payload fetched from anywhere else.
     def populate_metadata
       @package.filename ||= filename
-      @package.url ||= @url
+      @package.url ||= @url.to_s
       @package.payload_timestamp ||= payload_timestamp
-      @package.payload_checksum ||= Payload.checksum(content)
+      @package.payload_checksum = @package.payload_digest
+      @package.base_checksum = @package.payload_checksum
     end
   end
 
   class GithubGistProvider < DefaultProvider
-    def self.handles_url?(url) = url.match?(%r{gist\.github\.com})
+    def self.handles_url?(url) = url.to_s.match?(%r{gist\.github\.com})
 
     def content = json_body["files"].values.first["content"]
 
     def filename = json_body["files"].values.first["filename"]
 
     def transform_url(url)
-      gist_id = url[/gist\.github\.com\/[^\/]+\/([a-f0-9]+)/, 1]
-      "https://api.github.com/gists/#{gist_id}"
+      gist_id = url.to_s[%r{gist\.github\.com/[^/]+/([a-f0-9]+)}, 1]
+      raise FetchError, "Could not extract a gist id from #{url}" if gist_id.nil?
+
+      URI("https://api.github.com/gists/#{gist_id}")
     end
 
     def payload_timestamp
-      Time.parse(json_body["created_at"])
-    rescue ArgumentError
-      nil
+      Time.parse(json_body["updated_at"] || json_body["created_at"]).utc.iso8601
+    rescue ArgumentError, TypeError
+      super
     end
   end
 
   class OpenGistProvider < DefaultProvider
-    def handles_url?(url) = :maybe
+    # Speculative: it appends ".json" to anything and sees what comes back, so it
+    # must not inherit DefaultProvider's unconditional claim.
+    def self.handles_url?(url) = :maybe
 
-    def transform_url(url) = "#{url}.json"
+    def transform_url(url) = URI("#{url}.json")
 
     def content = json_body.dig("files", 0, "content")
 
     def filename = json_body.dig("files", 0, "filename")
 
+    # Only claims the URL if the JSON actually looks like a gist listing.
     def handles_body?
-      content && filename
-    rescue FileTooLargeError, Net::HTTPError, RuntimeError
+      !content.nil? && !filename.nil?
+    rescue FetchError, FileTooLargeError
       false
     end
-    # If we successfully fetch the body, and the body contains content and a filename, then we can handle the body
   end
 
   class FileProvider < DefaultProvider
-    def self.handles_url?(url) = File.exist?(url)
+    def self.handles_url?(url) = !url.to_s.empty? && File.exist?(url.to_s)
 
-    def transform_url(url) = Pathname(url)
+    def transform_url(url) = Pathname(url.to_s)
 
     def content = url.read
 
     def filename = url.basename.to_s
 
     def payload_timestamp
-      url.mtime.httpdate
+      url.mtime.utc.iso8601
     rescue Errno::ENOENT
-      nil
+      super
     end
   end
 

lib/picopackage/scanner.rb

@@ -1,11 +1,31 @@
+# frozen_string_literal: true
+
 module Picopackage
   module Scanner
+    Found = Struct.new(:path, :package)
+
+    # Scanning walks whole trees, so it has to survive binaries, unreadable
+    # files and anything large enough that reading it to look for a comment
+    # would be silly.
+    MAX_SCAN_SIZE = 1024 * 1024
+
     def self.scan(directory, pattern: "**/*")
-      Dir.glob(File.join(directory, pattern)).select do |file|
-        next unless File.file?(file)
-        content = File.read(file)
-        content.match?(Package::METADATA_PATTERN)
-      end.map { |file| Package.new(file) }
+      Dir.glob(File.join(directory, pattern)).sort.filter_map do |path|
+        content = readable_text(path)
+        next unless content&.match?(METADATA_PATTERN)
+
+        Found.new(path, Package.new(content: content))
+      end
+    end
+
+    def self.readable_text(path)
+      return nil unless File.file?(path)
+      return nil if File.size(path) > MAX_SCAN_SIZE
+
+      content = File.read(path, encoding: "UTF-8")
+      content.valid_encoding? ? content : nil
+    rescue SystemCallError, ArgumentError
+      nil
     end
   end
 end

lib/picopackage/version.rb

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
 
 module Picopackage
-  VERSION = "0.2.1"
+  VERSION = "0.3.0"
 end