Read page metadata from markfluence.yaml's pages: key

README.md

@@ -498,6 +498,23 @@ project-wide default is wrong for every file at once. Credentials are
 deliberately not settings here; see
 [docs/root-model.md](docs/root-model.md#what-it-deliberately-does-not-hold).
 
+It can also hold a **`pages:` block**, giving each file its page metadata so the
+markdown itself stays pristine — no frontmatter at all:
+
+```yaml
+pages:
+  docs/deploy-runbook.md:
+    title: Deploy Runbook
+    page_id: 12346
+```
+
+That is what makes `markfluence update docs/**/*.md` work from CI with no
+per-file inputs, and it is why `update` has no `--page-id`/`--title` flags:
+those would each have to name one file. Both locations are legal and agreement
+is silent, so you can move metadata in a file at a time; a file neither place
+mentions is skipped rather than failed. Details:
+[docs/root-model.md](docs/root-model.md#pages--page-metadata-for-a-pristine-file).
+
 The rest of this section is the precise version of the same idea. Every
 markdown file has a **documentation root**: the directory holding
 `markfluence.yaml`, found by walking up from the file's own directory, or —

cmd/check/check.go

@@ -18,6 +18,7 @@ import (
 	"github.com/mozilla/markfluence/internal/jsonout"
 	"github.com/mozilla/markfluence/internal/labels"
 	"github.com/mozilla/markfluence/internal/linkindex"
+	"github.com/mozilla/markfluence/internal/pagemeta"
 	"github.com/mozilla/markfluence/internal/pageref"
 	"github.com/mozilla/markfluence/internal/pagewidth"
 	"github.com/mozilla/markfluence/internal/project"
@@ -45,10 +46,16 @@ var Cmd = &cobra.Command{
 	Long: "Validate one or more markdown FILEs against the converter and frontmatter\n" +
 		"rules, with no network access and no credentials -- fast, safe, and\n" +
 		"CI/agent-friendly. Reports conversion warnings and broken image/link\n" +
-		"references, and frontmatter sanity (parseable, page_width valid, page_id\n" +
+		"references, and metadata sanity (parseable, page_width valid, page_id\n" +
 		"numeric when present). Each file is processed independently; the command\n" +
 		"exits non-zero if any file is broken or failed outright. Warnings alone do\n" +
 		"not fail.\n\n" +
+		"A file's metadata is checked wherever it lives -- its own frontmatter or a\n" +
+		"'pages:' entry for it in markfluence.yaml -- and an entry is reported only\n" +
+		"when its file is one of the FILEs given, so one bad entry never blocks\n" +
+		"checking the rest of a repository. Two locations naming different pages is\n" +
+		"an error; a file keeping its own keys in a project that uses 'pages:' is a\n" +
+		"warning, since both work.\n\n" +
 		"\"link not resolved: TARGET\" means TARGET is a sibling .md file that exists\n" +
 		"under the documentation root but has no page_id yet -- the normal state of\n" +
 		"a tree that hasn't been published, not a defect. \"same-page anchor not\n" +
@@ -130,23 +137,11 @@ func processFile(filename string, roots *project.Cache, indexes *linkindex.Cache
 		return r.fail(err, jsonout.CodeValidation)
 	}
 
-	if _, err := pagewidth.Declared(mf.Frontmatter); err != nil {
-		return r.fail(err, jsonout.CodeValidation)
-	}
-	// An invalid label is a guaranteed publish defect that needs no network to
-	// see, the same class as an invalid page_width -- and worse in one way: a
-	// name Confluence splits on a space publishes successfully, as the wrong
-	// labels, and then cannot be removed by any spelling of the file (see
-	// docs/confluence/labels.md). Catching it offline is the cheapest place it
-	// can be caught.
-	labelSet, err := labels.Declared(mf.Lists, mf.Frontmatter)
-	if err != nil {
-		return r.fail(err, jsonout.CodeValidation)
-	}
-	if pageID := mf.PageID(); pageID != "" && !pageref.IsDigits(pageID) {
-		return r.fail(errors.New(pageref.NotNumericMessage(pageID)), jsonout.CodeValidation)
-	}
-
+	// The root first, because a file's metadata may live in the project file's
+	// pages: block rather than in the file -- and then everything below
+	// validates the *resolved* metadata, so an entry's values are checked
+	// exactly as a file's own are. That is #139's per-file scoping made real:
+	// an entry is diagnosed when its file is named, and never otherwise.
 	abs, err := filepath.Abs(filename)
 	if err != nil {
 		return r.fail(err, jsonout.CodeIO)
@@ -163,6 +158,34 @@ func processFile(filename string, roots *project.Cache, indexes *linkindex.Cache
 		}
 		return r.fail(project.RootError(err), code)
 	}
+	key, _ := pagemeta.KeyFor(root, abs)
+	meta, err := pagemeta.Resolve(key, mf, root)
+	if err != nil {
+		// The two locations name different pages. Offline-visible, and exactly
+		// the kind of defect check exists to catch before a publish does.
+		return r.fail(err, jsonout.CodeValidation)
+	}
+
+	// A soft disagreement between the two locations, which D6 promises is
+	// reported wherever metadata is resolved -- not only by update.
+	r.warnings = append(r.warnings, meta.Warnings...)
+
+	if _, err := pagewidth.Declared(meta.Fields); err != nil {
+		return r.fail(err, jsonout.CodeValidation)
+	}
+	// An invalid label is a guaranteed publish defect that needs no network to
+	// see, the same class as an invalid page_width -- and worse in one way: a
+	// name Confluence splits on a space publishes successfully, as the wrong
+	// labels, and then cannot be removed by any spelling of the file (see
+	// docs/confluence/labels.md). Catching it offline is the cheapest place it
+	// can be caught.
+	labelSet, err := labels.Declared(meta.Lists, meta.Fields)
+	if err != nil {
+		return r.fail(err, jsonout.CodeValidation)
+	}
+	if pageID := strings.TrimSpace(meta.Fields["page_id"]); pageID != "" && !pageref.IsDigits(pageID) {
+		return r.fail(errors.New(pageref.NotNumericMessage(pageID)), jsonout.CodeValidation)
+	}
 	index, err := indexes.Get(root)
 	if err != nil {
 		return r.fail(fmt.Errorf("building the link index: %w", err), jsonout.CodeIO)
@@ -198,15 +221,32 @@ func processFile(filename string, roots *project.Cache, indexes *linkindex.Cache
 	// reported per file rather than once for the run, which is what keeps every
 	// diagnostic scoped to the files actually named -- a file under a different
 	// project hears nothing about this one.
-	if root.Config.PageWidth != "" && strings.TrimSpace(mf.Frontmatter["page_width"]) == "" {
+	// meta.Fields, not mf.Frontmatter: a width declared in this file's pages:
+	// entry wins over the project default exactly as one in its frontmatter
+	// does, so reading the file alone reported the project's bad value against
+	// a file that publishes perfectly well -- the false positive the paragraph
+	// above says check must not produce.
+	if root.Config.PageWidth != "" && strings.TrimSpace(meta.Fields["page_width"]) == "" {
 		if _, err := pagewidth.Declared(
 			map[string]string{"page_width": root.Config.PageWidth}); err != nil {
 			localBroken = append(localBroken, fmt.Sprintf("%s: %s", root.File, err))
 		}
 	}
-	if title, present := mf.TitleField(); present && title == "" {
+	if title, present := meta.Fields["title"]; present && strings.TrimSpace(title) == "" {
 		localBroken = append(localBroken,
-			"frontmatter has an empty 'title:'; give it a value or remove it")
+			"the title is present but empty; give it a value or remove it")
+	}
+
+	// "No half-and-half" (#139): a file carrying its own markfluence keys in a
+	// project that has chosen the manifest. A *warning*, never an error, and
+	// that is the whole point -- agreement between the two locations is legal,
+	// so this has to be sayable without becoming a wall somebody hits halfway
+	// through a migration. `fix` moving the keys is the remedy.
+	if pagemeta.HasManifest(root) && meta.InFile() {
+		r.warnings = append(r.warnings, fmt.Sprintf(
+			"this file carries markfluence frontmatter in a project that keeps page "+
+				"metadata in %s; both work, but keeping it in one place is clearer",
+			project.Filename))
 	}
 
 	page, err := convert.MdToConfluence(mf, root, index, checkBaseURL, checkSpaceKey, buildinfo.Stamp())
@@ -232,9 +272,12 @@ func processFile(filename string, roots *project.Cache, indexes *linkindex.Cache
 		return r.fail(err, jsonout.CodeConvert)
 	}
 	r.broken = append(localBroken, page.Broken...)
-	// Label warnings lead: they are a property of the frontmatter, so they hold
-	// whatever the converter went on to find in the body.
-	r.warnings = append(labelSet.Warnings, page.Warnings...)
+	// Appended rather than assigned: the half-and-half lint above has already
+	// put a warning here, and assigning discarded it. Label warnings still
+	// lead the converter's -- they are a property of the declared metadata, so
+	// they hold whatever the converter went on to find in the body.
+	r.warnings = append(r.warnings, labelSet.Warnings...)
+	r.warnings = append(r.warnings, page.Warnings...)
 	if showHTML {
 		r.debugHTML = page.HTML
 		r.debugAttachments = page.Attachments

cmd/check/check_test.go

@@ -354,7 +354,7 @@ func TestRunEmptyTitleIsBroken(t *testing.T) {
 	if !ui.IsSilent(err) || ui.ExitCode(err) != 1 {
 		t.Fatalf("run = %v, want a silent exit-1 error", err)
 	}
-	if !strings.Contains(out, "empty 'title:'") {
+	if !strings.Contains(out, "title is present but empty") {
 		t.Errorf("output = %q, want the empty-title message", out)
 	}
 }
@@ -388,7 +388,7 @@ func TestRunEmptyTitleReportedEvenWhenConversionFails(t *testing.T) {
 	if !ui.IsSilent(err) || ui.ExitCode(err) != 1 {
 		t.Fatalf("run = %v, want a silent exit-1 error", err)
 	}
-	if !strings.Contains(out, "empty 'title:'") {
+	if !strings.Contains(out, "title is present but empty") {
 		t.Errorf("output = %q, want the empty-title message alongside the collision", out)
 	}
 }
@@ -629,3 +629,177 @@ func TestRunInvalidProjectPageWidthIsNotReportedForAFileThatOverridesIt(t *testi
 		t.Errorf("output = %q, want no complaint: the file's own width wins", out)
 	}
 }
+
+// --- pages: entries -----------------------------------------------------------
+
+// An entry's values are validated exactly as a file's own are -- and only for
+// a file the invocation names, which is #139's scoping made real.
+func TestRunValidatesAnEntrysValues(t *testing.T) {
+	tests := map[string]struct{ project, want string }{
+		"invalid page_width": {
+			"pages:\n  main.md:\n    page_id: 1\n    page_width: huge\n",
+			"invalid page_width",
+		},
+		"non-numeric page_id": {
+			"pages:\n  main.md:\n    page_id: TODO\n",
+			"not a numeric page id",
+		},
+		"invalid label": {
+			"pages:\n  main.md:\n    page_id: 1\n    labels: [\"has space\"]\n",
+			"label",
+		},
+		"empty title": {
+			"pages:\n  main.md:\n    page_id: 1\n    title: \"\"\n",
+			"title is present but empty",
+		},
+	}
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
+			dir := t.TempDir()
+			write(t, filepath.Join(dir, "markfluence.yaml"), tc.project)
+			write(t, filepath.Join(dir, "main.md"), "# Main\n")
+
+			out, err := captureOutput(t, func() error {
+				return run(testCmd(t, ""), []string{filepath.Join(dir, "main.md")})
+			})
+			if !ui.IsSilent(err) || ui.ExitCode(err) != 1 {
+				t.Fatalf("run = %v, want a silent exit-1 error\n%s", err, out)
+			}
+			if !strings.Contains(out, tc.want) {
+				t.Errorf("output = %q, want it to contain %q", out, tc.want)
+			}
+		})
+	}
+}
+
+// One bad entry must not block checking an unrelated file: the diagnostic is
+// scoped to the file it belongs to, not to the run.
+func TestRunEntryDefectIsScopedToItsOwnFile(t *testing.T) {
+	dir := t.TempDir()
+	write(t, filepath.Join(dir, "markfluence.yaml"),
+		"pages:\n  bad.md:\n    page_id: TODO\n  good.md:\n    page_id: 2\n")
+	write(t, filepath.Join(dir, "bad.md"), "# Bad\n")
+	write(t, filepath.Join(dir, "good.md"), "# Good\n")
+
+	if _, err := captureOutput(t, func() error {
+		return run(testCmd(t, ""), []string{filepath.Join(dir, "good.md")})
+	}); err != nil {
+		t.Fatalf("checking good.md = %v, want success: bad.md's entry is not its business", err)
+	}
+}
+
+// The two locations naming different pages is offline-visible and exactly what
+// check exists to catch before a publish does.
+func TestRunReportsACoordinateDisagreement(t *testing.T) {
+	dir := t.TempDir()
+	write(t, filepath.Join(dir, "markfluence.yaml"), "pages:\n  main.md:\n    page_id: 1\n")
+	write(t, filepath.Join(dir, "main.md"), "---\npage_id: 999\n---\n# Main\n")
+
+	out, err := captureOutput(t, func() error {
+		return run(testCmd(t, ""), []string{filepath.Join(dir, "main.md")})
+	})
+	if !ui.IsSilent(err) || ui.ExitCode(err) != 1 {
+		t.Fatalf("run = %v, want a silent exit-1 error\n%s", err, out)
+	}
+	if !strings.Contains(out, "disagree about where this page is") {
+		t.Errorf("output = %q, want the disagreement message", out)
+	}
+}
+
+// "No half-and-half" is a warning, never an error -- agreement between the two
+// locations is legal, so this has to be sayable without becoming a wall
+// somebody hits halfway through a migration.
+func TestRunWarnsAboutHalfAndHalf(t *testing.T) {
+	dir := t.TempDir()
+	write(t, filepath.Join(dir, "markfluence.yaml"),
+		"pages:\n  other.md:\n    page_id: 2\n")
+	write(t, filepath.Join(dir, "main.md"), "---\ntitle: Main\npage_id: 1\n---\n# Main\n")
+	write(t, filepath.Join(dir, "other.md"), "# Other\n")
+
+	out, err := captureOutput(t, func() error {
+		return run(testCmd(t, ""), []string{filepath.Join(dir, "main.md")})
+	})
+	if err != nil {
+		t.Fatalf("run = %v, want success: half-and-half is a warning\n%s", err, out)
+	}
+	if !strings.Contains(out, "keeps page metadata in markfluence.yaml") {
+		t.Errorf("output = %q, want the half-and-half warning", out)
+	}
+}
+
+// A project with no pages: block has not chosen the manifest, so a file's own
+// frontmatter is simply how it works -- no warning.
+func TestRunNoHalfAndHalfWarningWithoutAManifest(t *testing.T) {
+	dir := t.TempDir()
+	write(t, filepath.Join(dir, "markfluence.yaml"), "space: ENG\n")
+	write(t, filepath.Join(dir, "main.md"), "---\ntitle: Main\npage_id: 1\n---\n# Main\n")
+
+	out, err := captureOutput(t, func() error {
+		return run(testCmd(t, ""), []string{filepath.Join(dir, "main.md")})
+	})
+	if err != nil {
+		t.Fatalf("run = %v, want success\n%s", err, out)
+	}
+	if strings.Contains(out, "keeps page metadata") {
+		t.Errorf("output = %q, want no half-and-half warning", out)
+	}
+}
+
+// A pristine file in a manifest project is the shape #139 exists for, and
+// check must have nothing to say about it.
+func TestRunPristineManifestFileIsClean(t *testing.T) {
+	dir := t.TempDir()
+	write(t, filepath.Join(dir, "markfluence.yaml"),
+		"pages:\n  main.md:\n    title: Main\n    page_id: 1\n")
+	write(t, filepath.Join(dir, "main.md"), "# Main\n")
+
+	out, err := captureOutput(t, func() error {
+		return run(testCmd(t, ""), []string{filepath.Join(dir, "main.md")})
+	})
+	if err != nil {
+		t.Fatalf("run = %v, want success\n%s", err, out)
+	}
+	if !strings.Contains(out, "clean") {
+		t.Errorf("output = %q, want it reported clean", out)
+	}
+}
+
+// A width declared in this file's entry wins over the project default exactly
+// as one in its frontmatter does, so the project's bad value must not fail it.
+// Reading mf.Frontmatter instead of the resolved metadata produced the false
+// positive check's own rule forbids: update publishes this file fine.
+func TestRunInvalidProjectWidthNotReportedWhenAnEntryOverridesIt(t *testing.T) {
+	dir := t.TempDir()
+	write(t, filepath.Join(dir, "markfluence.yaml"),
+		"page_width: huge\npages:\n  main.md:\n    page_id: 1\n    page_width: wide\n")
+	write(t, filepath.Join(dir, "main.md"), "# Main\n")
+
+	out, err := captureOutput(t, func() error {
+		return run(testCmd(t, ""), []string{filepath.Join(dir, "main.md")})
+	})
+	if err != nil {
+		t.Fatalf("run = %v, want success: the entry declares its own width\n%s", err, out)
+	}
+	if strings.Contains(out, "invalid page_width") {
+		t.Errorf("output = %q, want no complaint", out)
+	}
+}
+
+// D6 promises the soft-disagreement warning wherever metadata is resolved, not
+// only from update.
+func TestRunReportsASoftDisagreement(t *testing.T) {
+	dir := t.TempDir()
+	write(t, filepath.Join(dir, "markfluence.yaml"),
+		"pages:\n  main.md:\n    title: Manifest\n    page_id: 1\n")
+	write(t, filepath.Join(dir, "main.md"), "---\ntitle: File\n---\n# Main\n")
+
+	out, err := captureOutput(t, func() error {
+		return run(testCmd(t, ""), []string{filepath.Join(dir, "main.md")})
+	})
+	if err != nil {
+		t.Fatalf("run = %v, want success: a soft disagreement is a warning\n%s", err, out)
+	}
+	if !strings.Contains(out, "overrides") {
+		t.Errorf("output = %q, want the title-override warning", out)
+	}
+}