feat: per-assignment submission triggers — modes and milestone tags (#477)

cli/gh-student/accept.go

@@ -57,18 +57,57 @@ const (
 	defaultConfigRepoBranch     = "main"
 )
 
+// shimBranchTriggerLine is the exact `on.push.branches` line of the embedded
+// shim (before placeholder substitution). Tag submission mode removes it, so
+// the shim triggers only on submit/* tag pushes. Pinned by the accept shim
+// tests so an embed edit can't silently break the line surgery.
+const shimBranchTriggerLine = "    branches: [\"" + shimBranchPlaceholder + "\"]\n"
+
+// shimTagsTriggerLine is the exact `on.push.tags` line of the embedded shim.
+// Assignments with submission_tags replace it with the union of the
+// teacher's milestone patterns and submit/* (contract.ShimTagsList); the
+// default keeps it verbatim. Pinned by the accept shim tests like
+// shimBranchTriggerLine, and single-occurrence-guarded for the same reason.
+const shimTagsTriggerLine = "    tags: [\"submit/*\"]\n"
+
 // renderEmbeddedShim returns the embedded shim with the org, submission-branch,
 // and config-branch placeholders substituted. The shim never changes after
 // accept — runtime customization, runner edits, and teacher overrides all flow
 // through the runner workflow + assignments.json on the teacher's side.
-func renderEmbeddedShim(org, branch, configBranch string) string {
+//
+// submissionMode contract.SubmissionModeTag drops the branch-push trigger line
+// so only submission-tag pushes grade (`gh student submit` creates the tag; a
+// hand-pushed submit/* tag works too). Every other value — including "" and an
+// explicit "every-push" — takes the identical code path as before the field
+// existed, keeping the default shim byte-identical.
+//
+// submissionTags (teacher-named milestone patterns, e.g. phase1) widen the
+// tags trigger to their union with the always-on submit/* namespace; empty
+// keeps the tags line verbatim (again byte-identical). Orthogonal to
+// submissionMode: an every-push assignment can also name milestone tags.
+func renderEmbeddedShim(org, branch, configBranch, submissionMode string, submissionTags []string) string {
 	if branch == "" {
 		branch = defaultConfigRepoBranch
 	}
 	if configBranch == "" {
 		configBranch = defaultConfigRepoBranch
 	}
-	out := strings.ReplaceAll(embeddedShimContent, shimOrgPlaceholder, org)
+	shim := embeddedShimContent
+	if submissionMode == contract.SubmissionModeTag {
+		// Exact-line surgery, not a template branch: the embed stays one
+		// lintable file and every-push output can't drift. If the embed's
+		// trigger line ever changes shape, the tests pin this constant and
+		// the fallback below keeps accept emitting a valid (every-push) shim
+		// rather than garbage.
+		shim = strings.Replace(shim, shimBranchTriggerLine, "", 1)
+	}
+	if len(submissionTags) > 0 {
+		// Same exact-line surgery for the tags trigger: milestone patterns
+		// union submit/*, so the canonical namespace always fires.
+		shim = strings.Replace(shim, shimTagsTriggerLine,
+			"    tags: ["+contract.ShimTagsList(submissionTags)+"]\n", 1)
+	}
+	out := strings.ReplaceAll(shim, shimOrgPlaceholder, org)
 	out = strings.ReplaceAll(out, shimBranchPlaceholder, branch)
 	out = strings.ReplaceAll(out, shimConfigBranchPlaceholder, configBranch)
 	return out
@@ -458,7 +497,7 @@ func acceptAssignment(cmd *cobra.Command, client githubapi.Client, u *ui.UI, out
 			}
 			configBranch = commitBranch
 		}
-		shim = renderEmbeddedShim(org, commitBranch, configBranch)
+		shim = renderEmbeddedShim(org, commitBranch, configBranch, entry.SubmissionMode, entry.SubmissionTags)
 	}
 
 	repoName := reponame.Name(classroom, assignment, username)

cli/gh-student/accept_shim_test.go

@@ -6,14 +6,16 @@ import (
 	"net/http/httptest"
 	"strings"
 	"testing"
+
+	"github.com/foundation50/classroom50-cli-shared/contract"
 )
 
 func TestRenderEmbeddedShim(t *testing.T) {
 	// The embedded shim is the universal one-body-fits-all that gh student
 	// accept drops into every student repo. {{ORG}}, the submission branch, and
 	// the config-repo branch are the per-repo substitutions; everything else is
 	// fixed.
-	got := renderEmbeddedShim("cs50-fall-2026", "main", "main")
+	got := renderEmbeddedShim("cs50-fall-2026", "main", "main", "", nil)
 
 	// Trigger contract: branch pushes auto-grade; manual submit/* tag pushes
 	// still work (the runner detects which fired and creates or reuses the tag).
@@ -74,7 +76,7 @@ func TestRenderEmbeddedShim_OrgSubstitution(t *testing.T) {
 	// matching anything else.
 	for _, org := range []string{"cs50-fall-2026", "foundation50", "very-long-org-name-2026"} {
 		t.Run(org, func(t *testing.T) {
-			got := renderEmbeddedShim(org, "main", "main")
+			got := renderEmbeddedShim(org, "main", "main", "", nil)
 			wantUses := `uses: "` + org + `/classroom50/.github/workflows/autograde-runner.yaml@main"`
 			if !strings.Contains(got, wantUses) {
 				t.Errorf("expected %q in shim, got:\n%s", wantUses, got)
@@ -90,7 +92,7 @@ func TestRenderEmbeddedShim_BranchSubstitution(t *testing.T) {
 	// A master-default assignment repo must trigger on `master`; a config repo
 	// that stayed on `master` (rename didn't land) must be referenced via
 	// `@master` so the reusable-workflow ref resolves.
-	got := renderEmbeddedShim("cs50", "master", "master")
+	got := renderEmbeddedShim("cs50", "master", "master", "", nil)
 	if !strings.Contains(got, `branches: ["master"]`) {
 		t.Errorf("expected branches: [\"master\"], got:\n%s", got)
 	}
@@ -100,7 +102,7 @@ func TestRenderEmbeddedShim_BranchSubstitution(t *testing.T) {
 	}
 
 	// Empty branch/configBranch default to main.
-	def := renderEmbeddedShim("cs50", "", "")
+	def := renderEmbeddedShim("cs50", "", "", "", nil)
 	if !strings.Contains(def, `branches: ["main"]`) {
 		t.Errorf("empty branch should default to main, got:\n%s", def)
 	}
@@ -109,6 +111,69 @@ func TestRenderEmbeddedShim_BranchSubstitution(t *testing.T) {
 	}
 }
 
+func TestRenderEmbeddedShim_TagMode(t *testing.T) {
+	got := renderEmbeddedShim("cs50-fall-2026", "main", "main", contract.SubmissionModeTag, nil)
+
+	// Tag mode keeps ONLY the submit/* tag trigger: a plain `git push` must
+	// not grade. The branch trigger line is removed whole — no leftover key.
+	if strings.Contains(got, "branches:") {
+		t.Errorf("tag-mode shim must not contain a branches: trigger:\n%s", got)
+	}
+	if !strings.Contains(got, `tags: ["submit/*"]`) {
+		t.Errorf("tag-mode shim missing the submit/* tag trigger:\n%s", got)
+	}
+
+	// Everything else is unchanged: uses: line, permissions, no placeholders.
+	wantUses := `uses: "cs50-fall-2026/classroom50/.github/workflows/autograde-runner.yaml@main"`
+	if !strings.Contains(got, wantUses) {
+		t.Errorf("tag-mode shim missing %q\nfull:\n%s", wantUses, got)
+	}
+	for _, ph := range []string{"{{ORG}}", "{{BRANCH}}", "{{CONFIG_BRANCH}}"} {
+		if strings.Contains(got, ph) {
+			t.Errorf("tag-mode shim still contains unsubstituted %s:\n%s", ph, got)
+		}
+	}
+	for _, perm := range []string{"contents: write", "statuses: write"} {
+		if !strings.Contains(got, perm) {
+			t.Errorf("tag-mode shim missing required permission %q\nfull:\n%s", perm, got)
+		}
+	}
+
+	// The removal is exactly one line: tag-mode output equals every-push
+	// output minus the substituted branch trigger line.
+	everyPush := renderEmbeddedShim("cs50-fall-2026", "main", "main", "", nil)
+	wantTag := strings.Replace(everyPush, "    branches: [\"main\"]\n", "", 1)
+	if got != wantTag {
+		t.Errorf("tag-mode shim is not every-push minus the branch line:\ngot:\n%s\nwant:\n%s", got, wantTag)
+	}
+}
+
+// TestRenderEmbeddedShim_EveryPushByteIdentical pins that every non-tag mode
+// value — absent, the explicit wire default, and junk (validated upstream) —
+// renders the identical bytes, so introducing submission_mode changed nothing
+// for existing assignments.
+func TestRenderEmbeddedShim_EveryPushByteIdentical(t *testing.T) {
+	base := renderEmbeddedShim("cs50", "main", "main", "", nil)
+	for _, mode := range []string{contract.SubmissionModeEveryPush, "unvalidated-junk"} {
+		if got := renderEmbeddedShim("cs50", "main", "main", mode, nil); got != base {
+			t.Errorf("mode %q rendered different bytes than the default", mode)
+		}
+	}
+}
+
+// TestShimBranchTriggerLine_MatchesEmbed guards the line-surgery constant
+// against embed drift: if autograde-shim.yaml's trigger line changes shape,
+// tag mode would silently stop removing it (falling back to an every-push
+// shim). Fail here instead.
+func TestShimBranchTriggerLine_MatchesEmbed(t *testing.T) {
+	if !strings.Contains(embeddedShimContent, shimBranchTriggerLine) {
+		t.Fatalf("embed/autograde-shim.yaml no longer contains the exact branch trigger line %q — update shimBranchTriggerLine in lockstep", shimBranchTriggerLine)
+	}
+	if strings.Count(embeddedShimContent, shimBranchTriggerLine) != 1 {
+		t.Fatalf("branch trigger line appears more than once in the embed; single-occurrence surgery would remove the wrong one")
+	}
+}
+
 func TestResolveConfigRepoBranch(t *testing.T) {
 	newServer := func(t *testing.T, handler http.HandlerFunc) *httptest.Server {
 		server := httptest.NewServer(handler)
@@ -152,3 +217,60 @@ func TestResolveConfigRepoBranch(t *testing.T) {
 		}
 	})
 }
+
+func TestRenderEmbeddedShim_SubmissionTags(t *testing.T) {
+	// Milestone patterns widen the tags trigger to their union with the
+	// always-on submit/* namespace — plain `git tag phase1 && git push origin
+	// phase1` grades, and `gh student submit` keeps working unchanged.
+	got := renderEmbeddedShim("cs50", "main", "main", "", []string{"phase1", "v*"})
+	wantTags := `tags: ["phase1", "v*", "submit/*"]`
+	if !strings.Contains(got, wantTags) {
+		t.Errorf("shim missing widened tags trigger %q\nfull:\n%s", wantTags, got)
+	}
+	// The branch trigger stays (every-push assignment with milestone tags).
+	if !strings.Contains(got, `branches: ["main"]`) {
+		t.Errorf("milestone tags must not drop the branch trigger:\n%s", got)
+	}
+
+	// Orthogonal to tag mode: both together drop the branch line AND widen
+	// the tags line.
+	both := renderEmbeddedShim("cs50", "main", "main", contract.SubmissionModeTag, []string{"phase1"})
+	if strings.Contains(both, "branches:") {
+		t.Errorf("tag mode + milestone tags must drop the branch trigger:\n%s", both)
+	}
+	if !strings.Contains(both, `tags: ["phase1", "submit/*"]`) {
+		t.Errorf("tag mode + milestone tags missing the widened trigger:\n%s", both)
+	}
+
+	// The widening is exactly one line: tags-mode output equals the default
+	// output with only the tags line swapped.
+	base := renderEmbeddedShim("cs50", "main", "main", "", nil)
+	want := strings.Replace(base, "    tags: [\"submit/*\"]\n",
+		"    tags: [\"phase1\", \"v*\", \"submit/*\"]\n", 1)
+	if got != want {
+		t.Errorf("submission-tags shim is not the default minus one line swap:\ngot:\n%s\nwant:\n%s", got, want)
+	}
+}
+
+// TestRenderEmbeddedShim_NoTagsByteIdentical pins that an empty/nil pattern
+// list renders the identical bytes — introducing submission_tags changed
+// nothing for existing assignments.
+func TestRenderEmbeddedShim_NoTagsByteIdentical(t *testing.T) {
+	base := renderEmbeddedShim("cs50", "main", "main", "", nil)
+	if got := renderEmbeddedShim("cs50", "main", "main", "", []string{}); got != base {
+		t.Error("empty submission_tags rendered different bytes than nil")
+	}
+}
+
+// TestShimTagsTriggerLine_MatchesEmbed guards the tags line-surgery constant
+// against embed drift, exactly like TestShimBranchTriggerLine_MatchesEmbed:
+// if the embed's tags line changes shape, milestone patterns would silently
+// stop being rendered (falling back to submit/*-only). Fail here instead.
+func TestShimTagsTriggerLine_MatchesEmbed(t *testing.T) {
+	if !strings.Contains(embeddedShimContent, shimTagsTriggerLine) {
+		t.Fatalf("embed/autograde-shim.yaml no longer contains the exact tags trigger line %q — update shimTagsTriggerLine in lockstep", shimTagsTriggerLine)
+	}
+	if strings.Count(embeddedShimContent, shimTagsTriggerLine) != 1 {
+		t.Fatalf("tags trigger line appears more than once in the embed; single-occurrence surgery would replace the wrong one")
+	}
+}

cli/gh-student/internal/assignments/assignments.go

@@ -75,6 +75,28 @@ type Entry struct {
 	// own create default template-less), an explicit true/false forces the
 	// feature on/off. See accept.go's repo-feature PATCH.
 	RepoFeatures *RepoFeatures `json:"repo_features,omitempty"`
+
+	// SubmissionMode picks when the autograder fires: absent/"every-push"
+	// (the wire default) keeps the shim's every-push trigger; "tag" makes the
+	// shim trigger ONLY on submit/* tag pushes, which `gh student submit`
+	// creates after the branch push. Consumed at accept time (shim rendering)
+	// and at submit time (tag push).
+	SubmissionMode string `json:"submission_mode,omitempty"`
+
+	// SubmissionTags is the teacher-named milestone tag patterns (e.g.
+	// phase1, v*) that ALSO trigger grading, alongside the always-on submit/*
+	// namespace. Consumed at accept time: the shim's tags trigger renders as
+	// the union of these patterns and submit/*. Empty/absent means no
+	// milestone tags (the default shim, byte-identical to before the field
+	// existed).
+	SubmissionTags []string `json:"submission_tags,omitempty"`
+}
+
+// IsTagSubmissionMode reports whether the entry grades only on submit/* tag
+// pushes. Centralized so accept (shim rendering) and submit (tag push) can't
+// drift on the absent-means-every-push default.
+func (e Entry) IsTagSubmissionMode() bool {
+	return e.SubmissionMode == contract.SubmissionModeTag
 }
 
 // RepoFeatures is the tri-state Issues/Wiki/Projects/Pull-requests override; a nil pointer

cli/gh-student/internal/assignments/assignments_test.go

@@ -330,6 +330,55 @@ func TestEntryDecodesFeedbackPR(t *testing.T) {
 	}
 }
 
+func TestEntryDecodesSubmissionMode(t *testing.T) {
+	// submission_mode decodes when present and defaults to every-push
+	// semantics when absent — accept (shim rendering) and submit (tag push)
+	// both branch on IsTagSubmissionMode, so the wire contract matters.
+	var file assignmentsFile
+	if err := json.Unmarshal([]byte(`{
+  "schema": "classroom50/assignments/v1",
+  "assignments": [
+    {"slug": "tagged", "name": "Tagged", "mode": "individual", "autograder": "default", "submission_mode": "tag"},
+    {"slug": "explicit", "name": "Explicit", "mode": "individual", "autograder": "default", "submission_mode": "every-push"},
+    {"slug": "hello", "name": "Hello", "mode": "individual", "autograder": "default"}
+  ]
+}`), &file); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	if !file.Assignments[0].IsTagSubmissionMode() {
+		t.Error("tagged.IsTagSubmissionMode() = false, want true")
+	}
+	if file.Assignments[1].IsTagSubmissionMode() {
+		t.Error("explicit(every-push).IsTagSubmissionMode() = true, want false")
+	}
+	if file.Assignments[2].IsTagSubmissionMode() {
+		t.Error("hello(absent).IsTagSubmissionMode() = true, want false")
+	}
+}
+
+func TestEntryDecodesSubmissionTags(t *testing.T) {
+	// submission_tags decodes when present and reads empty when absent —
+	// accept renders the shim's tags trigger from it, so the wire contract
+	// matters.
+	var file assignmentsFile
+	if err := json.Unmarshal([]byte(`{
+  "schema": "classroom50/assignments/v1",
+  "assignments": [
+    {"slug": "proj", "name": "Project", "mode": "individual", "autograder": "default", "submission_tags": ["phase1", "v*"]},
+    {"slug": "hello", "name": "Hello", "mode": "individual", "autograder": "default"}
+  ]
+}`), &file); err != nil {
+		t.Fatalf("unmarshal: %v", err)
+	}
+	got := file.Assignments[0].SubmissionTags
+	if len(got) != 2 || got[0] != "phase1" || got[1] != "v*" {
+		t.Errorf("proj.SubmissionTags = %v, want [phase1 v*]", got)
+	}
+	if len(file.Assignments[1].SubmissionTags) != 0 {
+		t.Errorf("hello(absent).SubmissionTags = %v, want empty", file.Assignments[1].SubmissionTags)
+	}
+}
+
 func TestEntryDecodesLocked(t *testing.T) {
 	// locked decodes when present and defaults to false when absent — the
 	// accept flow refuses a locked assignment, so the wire contract matters.

cli/gh-student/internal/submitcmd/copysubmittable_test.go

@@ -140,10 +140,11 @@ func TestCopySubmittableFiles_ControlFilesKeptEvenUnderStarIgnore(t *testing.T)
 	}
 }
 
-func TestFetchAllowedFiles_FetchFailureReturnsNil(t *testing.T) {
+func TestFetchSubmitEntry_FetchFailureReturnsNil(t *testing.T) {
 	// Best-effort guarantee: a manifest fetch failure must never block
-	// submission — fetchAllowedFiles returns nil (submit all) since the
-	// runner enforces the allowlist authoritatively.
+	// submission — fetchSubmitEntry returns (nil, err); the caller submits
+	// all files (the runner enforces allowed_files authoritatively) and
+	// warns about the unknown submission mode post-push.
 	orig := fetchEntryFn
 	t.Cleanup(func() { fetchEntryFn = orig })
 	fetchEntryFn = func(ctx context.Context, org, classroom, secret, assignment string) (assignments.Entry, error) {
@@ -152,24 +153,37 @@ func TestFetchAllowedFiles_FetchFailureReturnsNil(t *testing.T) {
 
 	u := ui.NewForced(os.Stderr, false)
 	cfg := &classroomcfg.Config{Classroom: "cs-principles", Assignment: "hello"}
-	got := fetchAllowedFiles(context.Background(), "o", cfg, u, false)
-	if got != nil {
-		t.Errorf("fetchAllowedFiles on fetch failure = %#v, want nil (submit all)", got)
+	entry, err := fetchSubmitEntry(context.Background(), "o", cfg, u, false)
+	if entry != nil {
+		t.Errorf("fetchSubmitEntry on fetch failure = %#v, want nil (submit all)", entry)
+	}
+	if err == nil {
+		t.Error("fetchSubmitEntry on fetch failure must surface the error for the post-push warning")
 	}
 }
 
-func TestFetchAllowedFiles_SuccessReturnsPatterns(t *testing.T) {
+func TestFetchSubmitEntry_SuccessReturnsEntry(t *testing.T) {
 	orig := fetchEntryFn
 	t.Cleanup(func() { fetchEntryFn = orig })
 	fetchEntryFn = func(ctx context.Context, org, classroom, secret, assignment string) (assignments.Entry, error) {
-		return assignments.Entry{AllowedFiles: []string{"*", "!hello.py"}}, nil
+		return assignments.Entry{
+			AllowedFiles:   []string{"*", "!hello.py"},
+			SubmissionMode: "tag",
+		}, nil
 	}
 
 	u := ui.NewForced(os.Stderr, false)
 	cfg := &classroomcfg.Config{Classroom: "cs-principles", Assignment: "hello"}
-	got := fetchAllowedFiles(context.Background(), "o", cfg, u, false)
+	entry, err := fetchSubmitEntry(context.Background(), "o", cfg, u, false)
+	if err != nil || entry == nil {
+		t.Fatalf("fetchSubmitEntry = (%#v, %v), want entry", entry, err)
+	}
+	got := entry.AllowedFiles
 	if len(got) != 2 || got[0] != "*" || got[1] != "!hello.py" {
-		t.Errorf("fetchAllowedFiles = %#v, want [* !hello.py]", got)
+		t.Errorf("AllowedFiles = %#v, want [* !hello.py]", got)
+	}
+	if !entry.IsTagSubmissionMode() {
+		t.Error("IsTagSubmissionMode() = false, want true")
 	}
 }