An agent skill for gx, and the round-trip fix found while writing it

gx-cli/src/main/scala/org/jpablo/graphexplorer/gx/Cli.scala

@@ -56,6 +56,8 @@ object Cli:
       |  gx session <command> [--params J]      act on the LIVE view (needs a desktop)
       |  gx open <ref>                          show it in the desktop
       |
+      |  gx skill [<version>] [--latest]        where the agent skill lives
+      |
       |  M = detached | pull | push | sync      (default: pull)
       |  gx run --list                          the commands `run` accepts
       |
@@ -98,6 +100,7 @@ object Cli:
       case "run"     => runCommand(args, env)
       case "session" => sessionCommand(args, env)
       case "open"   => open(args, env)
+      case "skill"  => skill(args, env)
       case other =>
         env.err(s"gx: unknown command '$other'\n")
         env.err(Usage)
@@ -812,6 +815,74 @@ object Cli:
       case None =>
         checkPolicy(env.cwd.resolve(ref), env).left.map(identity)
 
+  // -------------------------------------------------------------- skill
+
+  /** Print where the agent skill lives, and the sentence to hand an agent.
+    *
+    * Deliberately NOT an installer. The skill is a prompt that a coding agent
+    * will load and act on, so writing it into someone's agent directory is a
+    * decision rather than a side effect of asking where it is — and every
+    * harness keeps skills somewhere different anyway. Printing a location plus
+    * the instruction works for all of them and leaves the human in the loop.
+    *
+    * Pinned to this binary by default. The skill names commands, param keys and
+    * exit codes, all of which are API that moves between releases, so an agent
+    * reading the branch tip while driving an older `gx` would be reading about
+    * commands it does not have.
+    */
+  private def skill(args: Args, env: CliEnv): Int =
+    SkillLocation.resolve(
+      requested = args.positionalAt(0),
+      latest = args.has("latest"),
+      running = buildinfo.BuildInfo.version
+    ) match
+      case Left(why) =>
+        env.err(s"gx: $why")
+        ExitCode.Usage
+
+      case Right(found) =>
+        if args.json then
+          env.out(
+            ujson.Obj(
+              "skill"   -> SkillLocation.Name,
+              "version" -> found.version,
+              "ref"     -> found.ref,
+              "pinned"  -> found.pinned,
+              "page"    -> found.page,
+              "raw"     -> found.raw
+            ).render(indent = 2)
+          )
+        else
+          env.out(s"gx ${found.version} — agent skill '${SkillLocation.Name}'")
+          env.out("")
+          env.out(s"  browse:  ${found.page}")
+          env.out(s"  fetch:   ${found.raw}")
+          env.out("")
+          env.out("Tell your coding agent:")
+          env.out("")
+          // Second person, addressed to the agent rather than about it, so the
+          // block can be pasted straight into a prompt. The URL gets a line of
+          // its own: it is the one part that must survive being copied out of a
+          // wrapped terminal intact.
+          env.out("  Read and analyze the skill at")
+          env.out(s"    ${found.raw}")
+          env.out(s"  together with ${SkillLocation.SupportingFiles.mkString(" and ")} beside it, which it links.")
+          env.out("  Check it against the `gx` on this machine, then install all three as a")
+          env.out(s"  skill named `${SkillLocation.Name}` wherever this harness keeps skills — for Claude")
+          env.out(s"  Code that is ${SkillLocation.Name}/ under .claude/skills/ in the project, or under")
+          env.out("  your home directory to have it everywhere. Keep the frontmatter intact.")
+          env.out("")
+          if found.pinned then
+            env.out(s"Pinned to ${found.ref}, the gx you are running.")
+            env.out("`gx skill --latest` prints the branch tip instead.")
+          else
+            env.out(s"This gx (${found.version}) is not a released version, so this is the tip of")
+            env.out(s"`${SkillLocation.DefaultBranch}` and may describe commands it does not have.")
+            SkillLocation.baseRelease(found.version) match
+              case Some(base) => env.out(s"Pin it to a release instead:  gx skill $base")
+              case None       => env.out("Pin it to a release instead:  gx skill <version>")
+        ExitCode.Ok
+
   // ---------------------------------------------------------- resolution
 
   private def withTarget(args: Args, env: CliEnv)(f: Target => Int): Int =

gx-cli/src/main/scala/org/jpablo/graphexplorer/gx/SkillLocation.scala

@@ -0,0 +1,93 @@
+package org.jpablo.graphexplorer.gx
+
+/** Where the agent skill that teaches a coding agent to drive `gx` lives.
+  *
+  * `gx` deliberately does not *install* it. A skill is a prompt that will be
+  * loaded into someone's agent and acted on, and a CLI writing one into
+  * `~/.claude/skills` on their behalf is the kind of thing that should be a
+  * decision rather than a side effect of running a command. Printing a location
+  * plus the sentence to hand the agent keeps the human in the loop and works
+  * for every harness, including the ones that keep skills somewhere else
+  * entirely.
+  *
+  * The location is PINNED to the running binary. The skill documents command
+  * names, param shapes and exit codes, all of which are API that moves between
+  * releases — an agent reading the tip of the branch while driving a
+  * six-months-old `gx` would be reading about commands it does not have.
+  */
+object SkillLocation:
+
+  val Repo    = "https://github.com/jpablo/graph-explorer"
+  val RawHost = "https://raw.githubusercontent.com/jpablo/graph-explorer"
+
+  /** Where the skill lives in the REPO, which is not where it gets installed.
+    *
+    * A vendor-neutral `skills/<name>/` rather than `.claude/skills/`: the Agent
+    * Skills format is read by more than one harness, and each keeps installed
+    * skills somewhere different — so the directory a skill is *published* from
+    * should not be spelled after any one of them. `.claude/skills/gx` is one
+    * possible destination, named in the instruction rather than here.
+    */
+  val Directory = "skills/gx"
+  val File      = s"$Directory/SKILL.md"
+
+  /** The spec requires `name` to match the directory it lives in. */
+  val Name = "gx"
+
+  /** The supporting files beside SKILL.md, which an installer has to take too.
+    *
+    * SKILL.md links them and stops short of repeating them, so fetching it
+    * alone yields a skill whose reference sections are dangling links.
+    */
+  val SupportingFiles = Vector("commands.md", "library.md")
+
+  val DefaultBranch = "viewer"
+
+  /** A released version, with or without the `v` the tags carry. */
+  private val Release = raw"v?(\d+\.\d+\.\d+)".r
+
+  /** The release a dev build was cut from, if its version says.
+    *
+    * dynver stamps `0.9.3+3-468f2c52`, whose leading `0.9.3` is a real tag and
+    * therefore the most useful thing to suggest pinning to — better than naming
+    * a version in the help text, which would rot at the next release.
+    */
+  def baseRelease(version: String): Option[String] =
+    Release.findPrefixMatchOf(version).map(_.group(1))
+
+  /** What a resolution came out as.
+    *
+    * `pinned` is not decoration: an unpinned answer is the branch tip, which
+    * may describe commands the running binary does not have, and the caller has
+    * to be able to say so.
+    */
+  final case class Resolved(version: String, ref: String, pinned: Boolean):
+    def page: String = s"$Repo/tree/$ref/$Directory"
+    def raw:  String = s"$RawHost/$ref/$File"
+
+  /** @param requested
+    *   a version named on the command line, if any
+    * @param latest
+    *   `--latest`: the branch tip, whatever this binary is
+    * @param running
+    *   this binary's own version (`BuildInfo.version`)
+    */
+  def resolve(requested: Option[String], latest: Boolean, running: String): Either[String, Resolved] =
+    (requested, latest) match
+      case (Some(v), true) =>
+        Left(s"--latest and an explicit version ('$v') ask for different things; pick one")
+
+      case (Some(Release(v)), false) => Right(Resolved(v, s"v$v", pinned = true))
+
+      case (Some(other), false) =>
+        Left(s"'$other' is not a version; expected something like 0.9.4")
+
+      case (None, true) => Right(Resolved(running, DefaultBranch, pinned = false))
+
+      case (None, false) =>
+        running match
+          // dynver stamps a dev build `0.9.3+13-2b8d0a46+20260730-2334`, and
+          // there is no tag by that name to point at. The tip is the honest
+          // answer, and the caller says out loud that it is not pinned.
+          case Release(v) if v == running => Right(Resolved(v, s"v$v", pinned = true))
+          case _                          => Right(Resolved(running, DefaultBranch, pinned = false))

gx-cli/src/test/scala/org/jpablo/graphexplorer/gx/CliSpec.scala

@@ -5,7 +5,9 @@ import org.jpablo.graphexplorer.gxcore.fs.{AccessPolicy, Audit, Documents}
 import org.jpablo.graphexplorer.gxcore.rpc.ChannelError
 import org.jpablo.graphexplorer.gxcore.store.LibraryStore
 
-import java.nio.file.{Files, Path}
+import java.nio.file.{Files, Path, Paths}
+
+import scala.jdk.CollectionConverters.*
 
 /** V-09: every command except `open` works with no desktop running.
   *
@@ -776,3 +778,128 @@ class CliSpec extends FunSuite:
     l("ls", "--json")
     assertEquals(ujson.read(l.stdout).arr.size, 1)
   }
+
+  // --------------------------------------------------------------- skill
+
+  /** `gx skill` points an agent at the skill; it must never install one.
+    *
+    * A skill is a prompt someone's agent will load and act on, so writing it
+    * into their harness is a decision rather than a side effect of asking where
+    * it is. These assert the printing, and — via the resolver — that the answer
+    * is pinned to the binary rather than to whatever the branch says today.
+    */
+
+  tmp.test("skill prints a location and an instruction, and writes nothing") { dir =>
+    val r = Run(dir)
+    assertEquals(r("skill"), ExitCode.Ok, r.stderr)
+    assert(r.stdout.contains(SkillLocation.File), r.stdout)
+    assert(r.stdout.contains("Tell your coding agent"), r.stdout)
+    // The whole point of the command: it is a pointer, not an installer.
+    assert(!Files.exists(dir.resolve(SkillLocation.File)), "skill must not install anything")
+  }
+
+  tmp.test("skill --json is machine-readable and says whether it is pinned") { dir =>
+    val r = Run(dir)
+    assertEquals(r("skill", "--json"), ExitCode.Ok, r.stderr)
+    val json = ujson.read(r.stdout)
+    assertEquals(json("skill").str, SkillLocation.Name)
+    assert(json("raw").str.endsWith(SkillLocation.File), json("raw").str)
+    json("pinned").bool // present, and a boolean
+  }
+
+  tmp.test("skill takes an explicit version and pins to that tag") { dir =>
+    val r = Run(dir)
+    assertEquals(r("skill", "0.9.4", "--json"), ExitCode.Ok, r.stderr)
+    val json = ujson.read(r.stdout)
+    assertEquals(json("ref").str, "v0.9.4")
+    assertEquals(json("pinned").bool, true)
+  }
+
+  tmp.test("skill refuses something that is not a version") { dir =>
+    val r = Run(dir)
+    assertEquals(r("skill", "yesterday"), ExitCode.Usage)
+    assert(r.stderr.contains("not a version"), r.stderr)
+  }
+
+  tmp.test("skill --latest is the branch tip, and not pinned") { dir =>
+    val r = Run(dir)
+    assertEquals(r("skill", "--latest", "--json"), ExitCode.Ok, r.stderr)
+    val json = ujson.read(r.stdout)
+    assertEquals(json("ref").str, SkillLocation.DefaultBranch)
+    assertEquals(json("pinned").bool, false)
+  }
+
+  tmp.test("skill will not guess between --latest and a named version") { dir =>
+    val r = Run(dir)
+    assertEquals(r("skill", "0.9.4", "--latest"), ExitCode.Usage)
+    assert(r.stderr.contains("pick one"), r.stderr)
+  }
+
+  // The resolver itself, away from the printing: a dev build has no tag to
+  // point at, and pretending otherwise would send an agent to a 404.
+  test("a released version resolves to its tag; a dev build falls back to the tip") {
+    val release = SkillLocation.resolve(None, latest = false, running = "0.9.4")
+    assertEquals(release.map(_.ref), Right("v0.9.4"))
+    assertEquals(release.map(_.pinned), Right(true))
+
+    val dev = SkillLocation.resolve(None, latest = false, running = "0.9.3+13-2b8d0a46+20260730-2334")
+    assertEquals(dev.map(_.ref), Right(SkillLocation.DefaultBranch))
+    assertEquals(dev.map(_.pinned), Right(false))
+    // The tip is not pinned, but the version still names a real tag to suggest.
+    assertEquals(SkillLocation.baseRelease("0.9.3+13-2b8d0a46+20260730-2334"), Some("0.9.3"))
+  }
+
+  test("a version given with the tag's own 'v' is accepted") {
+    val found = SkillLocation.resolve(Some("v1.2.3"), latest = false, running = "0.9.4")
+    assertEquals(found.map(_.ref), Right("v1.2.3"))
+  }
+
+  /** The path `gx skill` advertises has to be the path the skill is actually
+    * at, or every URL the command prints is a 404 — and nothing else in the
+    * build would notice, because the skill is an asset no Scala code imports.
+    */
+  test("the advertised location exists in this repository") {
+    val root = repoRoot()
+    val skill = root.resolve(SkillLocation.File)
+    assert(Files.exists(skill), s"gx skill points at ${SkillLocation.File}, which does not exist")
+    for name <- SkillLocation.SupportingFiles do
+      val f = root.resolve(SkillLocation.Directory).resolve(name)
+      assert(Files.exists(f), s"SKILL.md links $name, which does not exist")
+  }
+
+  /** Only the six fields the Agent Skills spec allows.
+    *
+    * Not a style preference: packaging or uploading a skill with any other key
+    * fails with a hard error rather than ignoring it, so a Claude Code-only
+    * field here would make the skill unusable everywhere else.
+    */
+  test("the skill's frontmatter is portable") {
+    val allowed = Set("allowed-tools", "compatibility", "description", "license", "metadata", "name")
+    val lines   = Files.readAllLines(repoRoot().resolve(SkillLocation.File)).asScala.toVector
+    assertEquals(lines.headOption, Some("---"), "SKILL.md must open with YAML frontmatter")
+
+    val body = lines.drop(1)
+    val end  = body.indexOf("---")
+    assert(end > 0, "the frontmatter is not closed")
+
+    val keys = body.take(end).filterNot(_.startsWith(" ")).filter(_.contains(":")).map(_.takeWhile(_ != ':'))
+    val bad  = keys.filterNot(allowed.contains)
+    assertEquals(bad, Vector.empty[String], s"non-portable frontmatter key(s): ${bad.mkString(", ")}")
+
+    // The spec ties the name to the directory; a mismatch simply fails to load.
+    assert(keys.contains("name"), "SKILL.md needs a name")
+    assert(
+      body.take(end).contains(s"name: ${SkillLocation.Name}"),
+      s"the skill's name must match its directory (${SkillLocation.Name})"
+    )
+  }
+
+  /** sbt runs tests from wherever it was launched, so walk up rather than
+    * assuming the module directory.
+    */
+  private def repoRoot(): Path =
+    Iterator
+      .iterate(Paths.get(sys.props.getOrElse("user.dir", ".")).toAbsolutePath)(_.getParent)
+      .takeWhile(_ != null)
+      .find(p => Files.exists(p.resolve("build.sbt")))
+      .getOrElse(fail("could not find the repository root"))

gx-core/jvm/src/main/scala/org/jpablo/graphexplorer/gxcore/command/DiagramText.scala

@@ -3,7 +3,7 @@ package org.jpablo.graphexplorer.gxcore.command
 import org.jpablo.graphexplorer.graphviz.Graphviz as ScalaGraphviz
 import org.jpablo.graphexplorer.viewer.backends.DiagramFormat
 import org.jpablo.graphexplorer.viewer.backends.graphviz.vizjs.simplegraph.{SimpleGraph, toViewerGraph}
-import org.jpablo.graphexplorer.viewer.graph.{ViewerGraph, viewerGraphElementsToText}
+import org.jpablo.graphexplorer.viewer.graph.ViewerGraph
 import upickle.default.read
 
 import scala.util.control.NonFatal
@@ -69,6 +69,19 @@ object DiagramText:
     * `omitInternal` because a round trip must not leak the layout's own
     * bookkeeping (`_gvid` and friends) into the user's file — those are
     * artifacts of how the graph was read, not of what it says.
+    *
+    * Goes through `viewerGraphToText` rather than calling the printer directly,
+    * which is not a stylistic preference — the printer is the LAST step of
+    * three, and this used to skip the other two:
+    *
+    *   - `combineStyleAttributes` folds the synthetic sub-attributes
+    *     (`fillstyle` and friends) back into a real `style="filled"`. Without
+    *     it `gx` wrote `fillstyle="true"` into the user's file, and then could
+    *     not read that file back: sub-attributes are not DOT, so the reader
+    *     rejected its own output. Two commands were enough to break a diagram.
+    *   - `graph.id` and `graph.tpe` carry the graph's NAME and whether it is
+    *     directed. Defaulting them silently rewrote `graph MyNet { a -- b }`
+    *     into `digraph "G" { "a" -> "b" }`, which is a different diagram.
     */
   def render(graph: ViewerGraph): String =
-    viewerGraphElementsToText(graph.elements, omitInternal = true)
+    ViewerGraph.viewerGraphToText(graph, omitInternal = true)