feat: a nook runs code when it has a run command, not when it says so

cmd/nook/main.go

@@ -342,11 +342,12 @@ func initNook(args []string, asJSON bool) error {
 	if _, err := os.Stat(manifest.FileName); err == nil {
 		return fmt.Errorf("%s already exists here", manifest.FileName)
 	}
-	m := manifest.Manifest{Name: name, Type: "static"}
+	m := manifest.Manifest{Name: name}
 	if err := m.Validate(); err != nil {
 		return err
 	}
-	if err := os.WriteFile(manifest.FileName, []byte(fmt.Sprintf("{ \"name\": %q, \"type\": \"static\" }\n", name)), 0o644); err != nil {
+	// Just the name. Adding a "run" command is what makes a nook run code on a server.
+	if err := os.WriteFile(manifest.FileName, []byte(fmt.Sprintf("{ \"name\": %q }\n", name)), 0o644); err != nil {
 		return err
 	}
 	wrote := []string{manifest.FileName}

examples/expenses/nook.json

@@ -1 +1 @@
-{ "name": "expenses", "type": "static" }
+{ "name": "expenses" }

examples/hello/nook.json

@@ -1 +1 @@
-{ "name": "hello", "type": "static" }
+{ "name": "hello" }

examples/invoice/nook.json

@@ -1 +1 @@
-{"name":"invoice","type":"static"}
+{ "name": "invoice" }

examples/pool/nook.json

@@ -1 +1 @@
-{"name":"pool","type":"static"}
+{ "name": "pool" }

examples/todo/nook.json

@@ -1 +1 @@
-{ "name": "todo", "type": "static" }
+{ "name": "todo" }

manifest/manifest.go

@@ -25,7 +25,6 @@ var (
 	secretRe = regexp.MustCompile(`^[A-Z][A-Z0-9_]{0,63}$`)
 	hostRe   = regexp.MustCompile(`^[a-z0-9]([a-z0-9-]*[a-z0-9])?(\.[a-z0-9]([a-z0-9-]*[a-z0-9])?)+$`)
 	reserved = map[string]bool{"api": true, "www": true, "app": true, "admin": true, "static": true, "mail": true, "dashboard": true, "nook": true}
-	types    = map[string]bool{"static": true, "service": true}
 	sources  = map[string]bool{"editors": true, "viewers": true}
 )
 
@@ -37,12 +36,16 @@ type CronEntry struct {
 
 type Manifest struct {
 	Name  string `json:"name"`
-	Type  string `json:"type"`
 	Entry string `json:"entry,omitempty"`
 
-	// Service nooks only. See DESIGN.md §10b.
-	// Source is who may read the run code: "editors" (owner and editors, the default) or
-	// "viewers" (anyone who can open the nook, as a static nook's files always are).
+	// Type is no longer declared: what a nook is follows from whether it has a run command. It
+	// is still read from files written before that, and never written back out, so nothing on
+	// disk breaks and nothing new learns a field it should not.
+	Type string `json:"-"`
+
+	// Only for a nook that runs code. See DESIGN.md §10b.
+	// Source is who may read that code: "editors" (owner and editors, the default) or
+	// "viewers" (anyone who can open the nook, as a page's files always are).
 	Source  string      `json:"source,omitempty"`
 	Run     string      `json:"run,omitempty"`
 	Port    int         `json:"port,omitempty"`
@@ -51,21 +54,26 @@ type Manifest struct {
 	Cron    []CronEntry `json:"cron,omitempty"`
 }
 
-func (m *Manifest) IsService() bool { return m.Type == "service" }
+// IsService is the whole of the distinction: a nook with a run command runs it on a server.
+func (m *Manifest) IsService() bool { return m.Run != "" }
 
 func Parse(b []byte) (*Manifest, error) {
 	var m Manifest
 	if err := json.Unmarshal(b, &m); err != nil {
 		return nil, fmt.Errorf("nook.json: %w", err)
 	}
-	if m.Type == "" {
-		m.Type = "static"
+	// "type" is read from older files and ignored: a nook with a run command runs it, so there
+	// is nothing to declare and nothing to get wrong.
+	var legacy struct {
+		Type string `json:"type"`
 	}
-	if m.Type == "static" && m.Entry == "" {
+	json.Unmarshal(b, &legacy)
+	m.Type = legacy.Type
+	if !m.IsService() && m.Entry == "" {
 		m.Entry = "index.html"
 	}
-	// Written out explicitly, so a pull or an export of a service nook states who can read it.
-	if m.Type == "service" && m.Source == "" {
+	// Written out explicitly, so a pull or an export states who may read the code.
+	if m.IsService() && m.Source == "" {
 		m.Source = "editors"
 	}
 	if err := m.Validate(); err != nil {
@@ -81,9 +89,6 @@ func (m *Manifest) Validate() error {
 	if reserved[m.Name] {
 		return fmt.Errorf("nook.json: name %q is reserved", m.Name)
 	}
-	if !types[m.Type] {
-		return fmt.Errorf("nook.json: type %q is not a nook type (static or service)", m.Type)
-	}
 	if m.IsService() {
 		return m.validateService()
 	}
@@ -93,17 +98,17 @@ func (m *Manifest) Validate() error {
 	for _, f := range []struct {
 		name string
 		set  bool
-	}{{"source", m.Source != ""}, {"run", m.Run != ""}, {"port", m.Port != 0}, {"secrets", len(m.Secrets) > 0}, {"egress", len(m.Egress) > 0}, {"cron", len(m.Cron) > 0}} {
+	}{{"source", m.Source != ""}, {"port", m.Port != 0}, {"secrets", len(m.Secrets) > 0}, {"egress", len(m.Egress) > 0}, {"cron", len(m.Cron) > 0}} {
 		if f.set {
-			return fmt.Errorf(`nook.json: %s only applies to a service nook; set "type": "service" or remove it`, f.name)
+			return fmt.Errorf(`nook.json: %s only means something for a nook that runs code; add a "run" command or remove it`, f.name)
 		}
 	}
 	return nil
 }
 
 func (m *Manifest) validateService() error {
 	if m.Entry != "" {
-		return fmt.Errorf(`nook.json: entry is for static nooks; a service nook serves its own routes from run`)
+		return fmt.Errorf(`nook.json: entry names a file to serve; a nook with a run command serves its own routes`)
 	}
 	if m.Source != "" && !sources[m.Source] {
 		return fmt.Errorf(`nook.json: source %q must be "editors" (the default) or "viewers"`, m.Source)
@@ -192,9 +197,9 @@ func command(field, s string) error {
 	return nil
 }
 
-// SourceVisibleToViewers says whether anyone who can open the nook may read its code. A static
-// nook's files reach every viewer's browser anyway, so they always are; a service nook's do not,
-// and stay with the owner and editors unless the manifest opts in.
+// SourceVisibleToViewers says whether anyone who can open the nook may read its code. A page's
+// files reach every viewer's browser anyway, so they always are; code that runs on a server does
+// not, and stays with the owner and editors unless the manifest opts in.
 func (m *Manifest) SourceVisibleToViewers() bool {
 	return !m.IsService() || m.Source == "viewers"
 }

manifest/manifest_test.go

@@ -1,22 +1,24 @@
 package manifest
 
-import "testing"
+import (
+	"encoding/json"
+	"strings"
+	"testing"
+)
 
 func TestParseStatic(t *testing.T) {
 	m, err := Parse([]byte(`{"name":"my-tool"}`))
-	if err != nil || m.Type != "static" || m.Entry != "index.html" {
+	if err != nil || m.Entry != "index.html" {
 		t.Fatalf("defaults: %+v %v", m, err)
 	}
 	if m.IsService() {
-		t.Error("a static nook is not a service")
+		t.Error("a nook with no run command does not run one")
 	}
 	for _, bad := range []string{
 		`{"name":"API"}`,
 		`{"name":"a"}`,
 		`{"name":"x","entry":"../etc"}`,
-		`{"name":"ok","type":"function"}`,
-		// Service fields on a static nook are a mistake worth naming, not a silent no-op.
-		`{"name":"ok","run":"python app.py"}`,
+		// A field that only means something alongside "run" is a mistake worth naming.
 		`{"name":"ok","port":8000}`,
 		`{"name":"ok","secrets":["TOKEN"]}`,
 		`{"name":"ok","egress":["example.com"]}`,
@@ -30,7 +32,7 @@ func TestParseStatic(t *testing.T) {
 }
 
 func TestParseService(t *testing.T) {
-	m, err := Parse([]byte(`{ "name": "board", "type": "service", "run": "python app.py", "port": 8000,
+	m, err := Parse([]byte(`{ "name": "board", "run": "python app.py", "port": 8000,
 		"secrets": ["SLACK_TOKEN"], "egress": ["hooks.slack.com", "API.example.com"],
 		"cron": [{ "schedule": "0 9 * * 1-5", "run": "python digest.py" }] }`))
 	if err != nil {
@@ -56,36 +58,36 @@ func TestParseService(t *testing.T) {
 
 func TestServiceValidation(t *testing.T) {
 	for name, bad := range map[string]string{
-		"run missing":      `{"name":"board","type":"service","port":8000}`,
-		"run blank":        `{"name":"board","type":"service","run":"  ","port":8000}`,
-		"run multiline":    `{"name":"board","type":"service","run":"a\nb","port":8000}`,
-		"port missing":     `{"name":"board","type":"service","run":"x"}`,
-		"port privileged":  `{"name":"board","type":"service","run":"x","port":80}`,
-		"port too high":    `{"name":"board","type":"service","run":"x","port":70000}`,
-		"entry set":        `{"name":"board","type":"service","run":"x","port":8000,"entry":"index.html"}`,
-		"secret lowercase": `{"name":"board","type":"service","run":"x","port":8000,"secrets":["token"]}`,
-		"secret reserved":  `{"name":"board","type":"service","run":"x","port":8000,"secrets":["NOOK_TOKEN"]}`,
-		"secret twice":     `{"name":"board","type":"service","run":"x","port":8000,"secrets":["A","A"]}`,
-		"egress url":       `{"name":"board","type":"service","run":"x","port":8000,"egress":["https://a.com"]}`,
-		"egress path":      `{"name":"board","type":"service","run":"x","port":8000,"egress":["a.com/hooks"]}`,
-		"egress port":      `{"name":"board","type":"service","run":"x","port":8000,"egress":["a.com:443"]}`,
-		"egress wildcard":  `{"name":"board","type":"service","run":"x","port":8000,"egress":["*.slack.com"]}`,
-		"egress bare word": `{"name":"board","type":"service","run":"x","port":8000,"egress":["localhost"]}`,
-		"cron schedule":    `{"name":"board","type":"service","run":"x","port":8000,"cron":[{"schedule":"0 9 * *","run":"y"}]}`,
-		"cron run blank":   `{"name":"board","type":"service","run":"x","port":8000,"cron":[{"schedule":"* * * * *","run":""}]}`,
-		"source unknown":   `{"name":"board","type":"service","run":"x","port":8000,"source":"everyone"}`,
-		"source public":    `{"name":"board","type":"service","run":"x","port":8000,"source":"public"}`,
+		"run missing":      `{"name":"board","port":8000}`,
+		"run blank":        `{"name":"board","run":"  ","port":8000}`,
+		"run multiline":    `{"name":"board","run":"a\nb","port":8000}`,
+		"port missing":     `{"name":"board","run":"x"}`,
+		"port privileged":  `{"name":"board","run":"x","port":80}`,
+		"port too high":    `{"name":"board","run":"x","port":70000}`,
+		"entry set":        `{"name":"board","run":"x","port":8000,"entry":"index.html"}`,
+		"secret lowercase": `{"name":"board","run":"x","port":8000,"secrets":["token"]}`,
+		"secret reserved":  `{"name":"board","run":"x","port":8000,"secrets":["NOOK_TOKEN"]}`,
+		"secret twice":     `{"name":"board","run":"x","port":8000,"secrets":["A","A"]}`,
+		"egress url":       `{"name":"board","run":"x","port":8000,"egress":["https://a.com"]}`,
+		"egress path":      `{"name":"board","run":"x","port":8000,"egress":["a.com/hooks"]}`,
+		"egress port":      `{"name":"board","run":"x","port":8000,"egress":["a.com:443"]}`,
+		"egress wildcard":  `{"name":"board","run":"x","port":8000,"egress":["*.slack.com"]}`,
+		"egress bare word": `{"name":"board","run":"x","port":8000,"egress":["localhost"]}`,
+		"cron schedule":    `{"name":"board","run":"x","port":8000,"cron":[{"schedule":"0 9 * *","run":"y"}]}`,
+		"cron run blank":   `{"name":"board","run":"x","port":8000,"cron":[{"schedule":"* * * * *","run":""}]}`,
+		"source unknown":   `{"name":"board","run":"x","port":8000,"source":"everyone"}`,
+		"source public":    `{"name":"board","run":"x","port":8000,"source":"public"}`,
 	} {
 		if _, err := Parse([]byte(bad)); err == nil {
 			t.Errorf("%s: expected an error for %s", name, bad)
 		}
 	}
 	// An IP is easier to enforce than a hostname, so it is allowed.
-	if _, err := Parse([]byte(`{"name":"board","type":"service","run":"x","port":8000,"egress":["10.0.0.1"]}`)); err != nil {
+	if _, err := Parse([]byte(`{"name":"board","run":"x","port":8000,"egress":["10.0.0.1"]}`)); err != nil {
 		t.Errorf("egress may be an IP: %v", err)
 	}
 	// No egress at all is valid and means no outbound network.
-	if m, err := Parse([]byte(`{"name":"board","type":"service","run":"x","port":8000}`)); err != nil || len(m.Egress) != 0 {
+	if m, err := Parse([]byte(`{"name":"board","run":"x","port":8000}`)); err != nil || len(m.Egress) != 0 {
 		t.Errorf("egress is optional: %v", err)
 	}
 }
@@ -99,8 +101,36 @@ func TestSourceVisibility(t *testing.T) {
 	if m.Source != "" {
 		t.Errorf("static nooks carry no source field, got %q", m.Source)
 	}
-	open, err := Parse([]byte(`{"name":"board","type":"service","run":"x","port":8000,"source":"viewers"}`))
+	open, err := Parse([]byte(`{"name":"board","run":"x","port":8000,"source":"viewers"}`))
 	if err != nil || !open.SourceVisibleToViewers() {
 		t.Errorf(`"viewers" opts in: %+v %v`, open, err)
 	}
 }
+
+// Files written before the manifest stopped declaring a type still parse, and what they are
+// follows from whether they have a run command rather than from what they claim to be.
+func TestLegacyTypeIsAcceptedAndIgnored(t *testing.T) {
+	page, err := Parse([]byte(`{"name":"old-page","type":"static"}`))
+	if err != nil || page.IsService() || page.Entry != "index.html" {
+		t.Fatalf("an old static manifest: %+v %v", page, err)
+	}
+	svc, err := Parse([]byte(`{"name":"old-svc","type":"service","run":"python app.py","port":8000}`))
+	if err != nil || !svc.IsService() {
+		t.Fatalf("an old service manifest: %+v %v", svc, err)
+	}
+	// A type that never existed is no longer an error, because the field no longer decides.
+	odd, err := Parse([]byte(`{"name":"odd","type":"whatever"}`))
+	if err != nil || odd.IsService() {
+		t.Fatalf("an unknown type is ignored, not fatal: %+v %v", odd, err)
+	}
+	// And it is never written back out, so a pull or an export does not teach it to anyone.
+	b, err := json.Marshal(svc)
+	if err != nil || strings.Contains(string(b), `"type"`) {
+		t.Errorf("type must not be written back: %s", b)
+	}
+	// A nook that claims to be a service without a run command is simply a page.
+	lying, err := Parse([]byte(`{"name":"lying","type":"service"}`))
+	if err != nil || lying.IsService() {
+		t.Fatalf("claiming to be a service is not being one: %+v %v", lying, err)
+	}
+}

skill/SKILL.md

@@ -28,12 +28,12 @@ non-zero exit.
 ## Files a nook needs
 
 ```
-nook.json      { "name": "expenses", "type": "static" }   name: 2-40 chars, a-z 0-9 -
+nook.json      { "name": "expenses" }                    name: 2-40 chars, a-z 0-9 -
 index.html     the app; include <script src="/_nook/nook.js"></script>
 ```
 
 Anything else in the folder (css, js, images) deploys too. Add a `.nookignore` (one path per
-line) to skip files. Deploys are capped at 10 MB. `type` is `static` for now.
+line) to skip files. Deploys are capped at 10 MB.
 
 ## Rules the page must follow