diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 1f1d01a..129165d 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -170,6 +170,17 @@ jobs: echo "ok: ${tarball}" done + # A pre-release tag (v0.3.0-pre.1) exists to be pinned: it is never marked latest + # and never reaches Homebrew. + - name: Classify the tag + id: tag + run: | + if [[ "${GITHUB_REF_NAME%%+*}" == *-* ]]; then + echo "prerelease=true" >> "${GITHUB_OUTPUT}" + else + echo "prerelease=false" >> "${GITHUB_OUTPUT}" + fi + - name: Create GitHub release env: GH_TOKEN: ${{ github.token }} @@ -177,9 +188,11 @@ jobs: # Attach the checksum manifests alongside the tarballs so consumers can # verify downloads independently. gh release create "${GITHUB_REF_NAME}" dist/*.tar.gz dist/*.sha256 \ - --title "${GITHUB_REF_NAME}" --generate-notes + --title "${GITHUB_REF_NAME}" --generate-notes \ + --prerelease=${{ steps.tag.outputs.prerelease }} - name: Render and push the Homebrew formula + if: steps.tag.outputs.prerelease == 'false' env: TAP_TOKEN: ${{ secrets.HOMEBREW_TAP_TOKEN }} run: scripts/render-formula.sh "${GITHUB_REF_NAME}" diff --git a/README.md b/README.md index beac3b8..bf445de 100644 --- a/README.md +++ b/README.md @@ -20,7 +20,7 @@ A command-line tool for [Concord Consortium](https://concord.org) researchers to - `get history `: the interactive state history series (full series by default), as JSONL. - `get attachments `: S3 file attachments referenced by answers/history (open-response audio, offloaded CODAP/SageModeler state). - **See what's in a dataset**: `cc-data dataset list` and `dataset show` render instant summaries from the manifest (downloads by type, answer/history/attachment counts, coverage, size, age). `--json` emits the same summary for Claude and scripts, and every `get` ends with a machine-readable result line so tooling can confirm what happened. -- **Query everything with SQL** (`cc-data query`, `cc-data repl`): an ephemeral in-memory DuckDB exposes unified `reports`, `answers`, and `history` views across everything in the dataset, plus `run_membership` (which runs' fetches covered which records), `attachment_files` (per-attachment metadata and local paths), `attachment_states` (queryable offloaded CODAP/SageModeler state), `student_id_mapping` and `student_metadata` (the Portal student reports as deduplicated join dimensions, one row per learner), and a `downloads` provenance dimension table. Audio attachment content itself isn't SQL-queryable; `attachment_files` gives you its local path instead. +- **Query everything with SQL** (`cc-data query`, `cc-data repl`): an ephemeral in-memory DuckDB exposes unified `reports`, `answers`, and `history` views across everything in the dataset, plus `run_membership` (which runs' fetches covered which records), `run_answers` (every answer with the `run_id` of each run whose answers fetch holds it), `attachment_files` (per-attachment metadata and local paths), `attachment_states` (queryable offloaded CODAP/SageModeler state), `student_id_mapping` and `student_metadata` (the Portal student reports as deduplicated join dimensions, one row per learner), and a `downloads` provenance dimension table. Audio attachment content itself isn't SQL-queryable; `attachment_files` gives you its local path instead. - **Teach Claude about itself**: on install, `cc-data` writes a Claude Code skill (and a one-line `~/.claude/CLAUDE.md` pointer) so Claude can download and query data for you, and ships `cc-data mcp`, an MCP stdio server for Claude Desktop using the same stored credential. ## Datasets @@ -68,6 +68,10 @@ cc-data get history --dataset / cc-data get attachments --dataset / [--answer ] [--url] [--inline] cc-data query --dataset / "SELECT ..." [--format table|csv|json|jsonl] cc-data repl --dataset / +cc-data package init [dir] [--name ] # manifest.json skeleton + run.py stub for a dashboard package +cc-data package run [dir] --dataset / --scope # run it locally under the runner's rules +cc-data package build [dir] [--out ] # reproducible zip, checked against the catalog's rules +cc-data package publish [--portal ] [--origin projects/] [--official] [--json] cc-data mcp # MCP stdio server for Claude Desktop ``` @@ -94,7 +98,7 @@ The CLI is written in Go. DuckDB is embedded via [`duckdb-go`](https://github.co go build ./... ``` -Releases are built with goreleaser on native GitHub Actions runners per platform, with a Homebrew formula for install (`brew install concord-consortium/tap/cc-data`). Note that `brew uninstall` removes only the binary; run `cc-data uninstall` first to remove the Claude skill, the `~/.claude/CLAUDE.md` pointer, and (optionally) your stored credentials. Your datasets are never removed automatically — `cc-data uninstall` prints where they remain. +Releases are built with goreleaser on native GitHub Actions runners per platform, with a Homebrew formula for install (`brew install concord-consortium/tap/cc-data`). A tag with a pre-release suffix, such as `v0.3.0-pre.1`, publishes a GitHub pre-release with the same archives and no formula, for pinning by the runner image and package workflows. Note that `brew uninstall` removes only the binary; run `cc-data uninstall` first to remove the Claude skill, the `~/.claude/CLAUDE.md` pointer, and (optionally) your stored credentials. Your datasets are never removed automatically — `cc-data uninstall` prints where they remain. The macOS signing/notarization and Homebrew-tap credentials the release needs live in this repo's GitHub Actions secrets. Maintainers can re-create those secrets from `cc-data-release-secrets.zip` in the **Developer Admin** 1Password vault, which bundles the credentials and a script that sets them. It exists so the release secrets can be restored, rotated, or handed to a new maintainer without redoing the Apple and GitHub setup from scratch (the tap push token in particular is a fine-grained GitHub PAT that must be rotated at least yearly). diff --git a/cmd/package.go b/cmd/package.go new file mode 100644 index 0000000..55a1e05 --- /dev/null +++ b/cmd/package.go @@ -0,0 +1,374 @@ +package cmd + +import ( + "context" + "errors" + "fmt" + "os" + "os/exec" + "os/signal" + "path/filepath" + "strings" + "time" + + "github.com/concord-consortium/cc-data-cli/internal/api" + "github.com/concord-consortium/cc-data-cli/internal/dataset" + "github.com/concord-consortium/cc-data-cli/internal/output" + "github.com/concord-consortium/cc-data-cli/internal/packages" + "github.com/spf13/cobra" +) + +const runDifferences = `What run cannot reproduce from the VM: the package runs as you, with no network +sandbox, and with HOME, PATH, the locale and any proxy settings passed through so its own +cc-data calls find your login and reach the server. A .py entrypoint runs with python3.11 +when it is on PATH, as on the VM, else python3. A clue_prepull package is refused, since +cc-data cannot fetch CLUE data yet.` + +const packageLong = `Develop, test and publish a Researcher Dashboard package. + +A package is a directory holding manifest.json and an entrypoint. init writes both; run +executes the package against your own dataset under the runner's rules; build zips it; +publish registers the zip in the catalog with your cc-data token. + +` + runDifferences + +func newPackageCmd() *cobra.Command { + cmd := &cobra.Command{ + Use: "package", + Short: "Develop, test and publish a Researcher Dashboard package", + Long: packageLong, + } + cmd.AddCommand(newPackageInitCmd(), newPackageRunCmd(), newPackageBuildCmd(), newPackagePublishCmd()) + return cmd +} + +// packageDir is the optional [dir] argument, defaulting to the current directory. +func packageDir(args []string) string { + if len(args) == 1 { + return args[0] + } + return "." +} + +// loadPackage collects a package directory and validates its manifest against what it holds. +func loadPackage(dir string) (packages.Manifest, packages.Files, error) { + files, err := packages.Collect(dir) + if err != nil { + return packages.Manifest{}, files, &output.CLIError{ExitCode: output.ExitUsage, Code: "INVALID_PACKAGE", Message: err.Error()} + } + data, err := os.ReadFile(filepath.Join(dir, "manifest.json")) + if err != nil { + return packages.Manifest{}, files, &output.CLIError{ExitCode: output.ExitUsage, Code: "INVALID_MANIFEST", Message: fmt.Sprintf("no manifest.json in %s", dir)} + } + m, err := packages.ReadManifest(data, files.Set()) + if err != nil { + return m, files, &output.CLIError{ExitCode: output.ExitUsage, Code: "INVALID_MANIFEST", Message: err.Error()} + } + if err := files.CheckEntrypointMode(m.Entrypoint); err != nil { + return m, files, &output.CLIError{ExitCode: output.ExitUsage, Code: "INVALID_PACKAGE", Message: err.Error()} + } + return m, files, nil +} + +func newPackageInitCmd() *cobra.Command { + var name string + cmd := &cobra.Command{ + Use: "init [dir]", + Short: "Write a manifest.json skeleton and a run.py stub", + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + dir := packageDir(args) + written, err := packages.Init(dir, name) + if err != nil { + return output.Usagef("%v", err) + } + return output.ResultLine(map[string]any{"dir": dir, "written": written}) + }, + } + cmd.Flags().StringVar(&name, "name", "", "the package name (default: the directory's name, when it fits ^[a-z0-9][a-z0-9-]{0,62}$)") + return cmd +} + +// loadedPackage is a package directory and scope file read and checked, before any credential +// is needed. +type loadedPackage struct { + dir string + manifest packages.Manifest + files packages.Files + scope packages.LocalScope +} + +type packageRunFlags struct { + datasetRef, scopePath string + environ func() []string +} + +func (f packageRunFlags) load(dir string) (loadedPackage, error) { + m, files, err := loadPackage(dir) + if err != nil { + return loadedPackage{}, err + } + raw, err := os.ReadFile(f.scopePath) + if err != nil { + return loadedPackage{}, &output.CLIError{ExitCode: output.ExitUsage, Code: "INVALID_SCOPE", Message: fmt.Sprintf("reading the scope file: %v", err)} + } + scope, err := packages.ParseLocalScope(raw) + if err != nil { + return loadedPackage{}, &output.CLIError{ExitCode: output.ExitUsage, Code: "INVALID_SCOPE", Message: err.Error()} + } + return loadedPackage{dir: dir, manifest: m, files: files, scope: scope}, nil +} + +// run validates, runs and reports a loaded package; it takes the client so it can be exercised +// against a test server without a stored credential. +func (f packageRunFlags) run(ctx context.Context, client *api.Client, pkg loadedPackage, ref dataset.Ref, dataRoot string) error { + // The package runs from its staged copy, where a relative root would name another folder. + dataRoot, err := filepath.Abs(dataRoot) + if err != nil { + return output.Internalf("%v", err) + } + archive, err := packages.Zip(pkg.files) + if err != nil { + return output.Internalf("%v", err) + } + // A version that is already published, or a portal that cannot store packages yet, + // still runs; only a publish is refused. + if _, err := validate(ctx, client, archive, "", false); err != nil { + return err + } + exe, _ := os.Executable() + start := time.Now() + res, err := packages.Run(ctx, packages.RunOptions{ + Dir: pkg.dir, Manifest: pkg.manifest, Files: pkg.files, Scope: pkg.scope, Applies: appliesFor(client, pkg.scope), + Dataset: ref.String(), Name: ref.Name, Portal: ref.Portal.Host(), DataRoot: dataRoot, + ReportSrv: client.BaseURL, CCData: exe, Stderr: output.Stderr(), Environ: f.environ, LookPath: exec.LookPath, + }) + if err != nil { + return packageRunError(err) + } + return output.ResultLine(map[string]any{ + "display": res.DisplayPath, "display_bytes": res.DisplayBytes, "summary": res.Summary, + "counts": res.Counts, "elapsed_seconds": int(time.Since(start).Seconds()), + }) +} + +func newPackageRunCmd() *cobra.Command { + f := packageRunFlags{environ: os.Environ} + cmd := &cobra.Command{ + Use: "run [dir] --dataset --scope ", + Short: "Run a package locally under the dashboard runner's rules", + Long: "Run a package against your own dataset under the runner's rules: the same scope.json,\n" + + "environment variables, output files, display.md cap and applicability check.\n\n" + + "--scope names a file holding the scope's kind, id, classes and assignments.\n" + + "Results are written under /.cc-data-run/, which build never ships.\n\n" + runDifferences, + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + if f.datasetRef == "" || f.scopePath == "" { + return output.Usagef("--dataset and --scope are required") + } + pkg, err := f.load(packageDir(args)) + if err != nil { + return err + } + cfg, dataRoot, err := loadRuntime() + if err != nil { + return err + } + ref, err := resolveRef(cfg, f.datasetRef) + if err != nil { + return err + } + client, err := api.ForPortal(ref.Portal) + if err != nil { + return err + } + // The terminal's Ctrl-C cannot reach the package's own process group, so it cancels the run. + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt) + defer stop() + return f.run(ctx, client, pkg, ref, dataRoot) + }, + } + cmd.Flags().StringVar(&f.datasetRef, "dataset", "", "the dataset the package pulls into and reads: /") + cmd.Flags().StringVar(&f.scopePath, "scope", "", "a JSON file holding the scope's kind, id, classes and assignments") + return cmd +} + +// derivationTimeout covers report-server's wait of up to 270 seconds on report-service's +// deriver; the client's 60-second default would cut it off. +const derivationTimeout = 5 * time.Minute + +// validate asks report-server whether a publish of the archive would be accepted. A server +// without the route gets a warning and a nil answer: the archive is unchecked, not refused. +func validate(ctx context.Context, client *api.Client, archive []byte, origin string, official bool) (*api.ValidatedPackage, error) { + ans, err := client.ValidatePackage(ctx, archive, origin, official) + switch { + case api.RouteMissing(err): + output.Warnf("report-server has no validate route yet, so this package is not checked against the catalog's rules until publish") + return nil, nil + case err != nil: + return nil, api.AsCLIError(err) + } + return ans, nil +} + +// appliesFor is package run's applicability check: report-server's one matcher, against the +// scope's assignment URLs and the interactive URLs report-service's deriver finds in them. +func appliesFor(client *api.Client, scope packages.LocalScope) func(context.Context, packages.URLs) (packages.Verdict, error) { + return func(ctx context.Context, urls packages.URLs) (packages.Verdict, error) { + long := *client + long.RequestTimeout = derivationTimeout + ans, err := long.PackageApplies(ctx, api.AppliesRequest{ + URLs: map[string][]string{"all": urls.All, "any": urls.Any, "none": urls.None}, + AssignmentURLs: scope.AssignmentURLs(), + }) + switch { + case api.RouteMissing(err): + return packages.Verdict{Unconfirmed: true}, nil + case err != nil: + return packages.Verdict{}, api.AsCLIError(err) + } + for _, u := range ans.Unread { + output.Warnf("the class profile could not read %s (%s), so its interactives are not matched", u.URL, u.Reason) + } + if ans.Truncated { + output.Warnf("the class profile was truncated, so not every interactive URL was matched") + } + return packages.Verdict{Applies: ans.Applies, Reason: ans.Reason}, nil + } +} + +func packageRunError(err error) error { + var refused *packages.Refused + var failed *packages.Failed + var out *packages.OutputRefused + var cliErr *output.CLIError + switch { + case errors.As(err, &cliErr): + return cliErr + case errors.As(err, &refused): + return &output.CLIError{ExitCode: output.ExitInternal, Code: "PACKAGE_REFUSED", Message: err.Error()} + case errors.As(err, &failed): + return &output.CLIError{ExitCode: output.ExitInternal, Code: "PACKAGE_FAILED", Message: err.Error()} + case errors.As(err, &out): + return &output.CLIError{ExitCode: output.ExitInternal, Code: "PACKAGE_OUTPUT_REFUSED", Message: err.Error()} + } + return output.Internalf("%v", err) +} + +type packageBuildFlags struct { + out, portal, origin string + official bool +} + +// run builds, validates and writes; it takes the client so it can be exercised against a test +// server without a stored credential. +func (f packageBuildFlags) run(ctx context.Context, client *api.Client, dir string) error { + m, files, err := loadPackage(dir) + if err != nil { + return err + } + out := f.out + if out == "" { + // The manifest is unchecked until validate answers, and unchecked for good on a server + // without the route, so its name and version must not be able to steer the path. + name := fmt.Sprintf("%s-%s.zip", m.Name, m.Version) + if strings.ContainsAny(name, `/\`) { + return &output.CLIError{ExitCode: output.ExitUsage, Code: "INVALID_MANIFEST", Message: fmt.Sprintf("the manifest's name and version make %q, which is not a file name; fix them or pass --out", name)} + } + out = filepath.Join(dir, packages.BuildDirName, name) + } else if err := packages.CheckBuildOutput(dir, out); err != nil { + return output.Usagef("%v", err) + } + archive, err := packages.Zip(files) + if err != nil { + return output.Internalf("%v", err) + } + ans, err := validate(ctx, client, archive, f.origin, f.official) + if err != nil { + return err + } + if ans != nil && ans.AlreadyPublished { + output.Warnf("%s %s is already published; publish will refuse it until the version changes", ans.Identity, ans.Version) + } + if ans != nil && ans.PublishingUnavailable != nil { + output.Warnf("this package cannot be published yet: %s", *ans.PublishingUnavailable) + } + if err := packages.WriteBuild(out, archive); err != nil { + return output.Internalf("%v", err) + } + return output.ResultLine(map[string]any{ + "path": out, "checksum": packages.Checksum(archive), "bytes": len(archive), "files": len(files.Paths), + }) +} + +func newPackageBuildCmd() *cobra.Command { + var f packageBuildFlags + cmd := &cobra.Command{ + Use: "build [dir]", + Short: "Zip a package reproducibly, check it against the catalog's rules and print its checksum", + Args: cobra.MaximumNArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + client, err := reportsClient(f.portal) + if err != nil { + return err + } + return f.run(context.Background(), client, packageDir(args)) + }, + } + cmd.Flags().StringVar(&f.out, "out", "", "where to write the zip (default: /.cc-data-build/-.zip)") + cmd.Flags().StringVar(&f.portal, "portal", "", "portal whose catalog rules to check against: an environment alias or a hostname") + cmd.Flags().StringVar(&f.origin, "origin", "", "projects/, when the package will be published as a project's") + cmd.Flags().BoolVar(&f.official, "official", false, "check it as an official publish will be checked (publisher role only)") + return cmd +} + +type packagePublishFlags struct { + portal, origin string + official, asJSON bool +} + +// run publishes the archive; it takes the client so it can be exercised against a test server +// without a stored credential. +func (f packagePublishFlags) run(ctx context.Context, client *api.Client, archive []byte) error { + got, err := client.PublishPackage(ctx, archive, f.origin, f.official) + if err != nil { + return api.AsWriteCLIError(err, "Publishing again is safe: ALREADY_EXISTS then means this attempt landed") + } + if want := packages.Checksum(archive); got.Checksum != want { + return output.Internalf("report-server recorded checksum %s, but the archive sent is %s", got.Checksum, want) + } + if f.asJSON { + return output.ResultLine(got) + } + fmt.Fprintf(output.Stdout(), "identity %s\nversion %s\nvisibility %s\nofficial %t\ncurrent version %s\nchecksum %s\n", + got.Identity, got.Version, got.Visibility, got.Official, got.CurrentVersion, got.Checksum) + return nil +} + +func newPackagePublishCmd() *cobra.Command { + var f packagePublishFlags + cmd := &cobra.Command{ + Use: "publish [--portal ] [--origin projects/] [--official] [--json]", + Short: "Publish a built package to the catalog with your cc-data token", + Long: "POST the zip to report-server's catalog, which creates the package private on its first\n" + + "publish and records the version. --origin projects/ publishes a project package;\n" + + "--official needs the publisher role. The request is never retried.", + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + archive, err := os.ReadFile(args[0]) + if err != nil { + return output.Usagef("reading %s: %v", args[0], err) + } + client, err := reportsClient(f.portal) + if err != nil { + return err + } + return f.run(context.Background(), client, archive) + }, + } + cmd.Flags().StringVar(&f.portal, "portal", "", "portal whose catalog to publish to: an environment alias or a hostname") + cmd.Flags().StringVar(&f.origin, "origin", "", "projects/ to publish a project's package") + cmd.Flags().BoolVar(&f.official, "official", false, "mark the package official (publisher role only)") + cmd.Flags().BoolVar(&f.asJSON, "json", false, "emit the catalog's answer as JSON") + return cmd +} diff --git a/cmd/package_test.go b/cmd/package_test.go new file mode 100644 index 0000000..e07f1d8 --- /dev/null +++ b/cmd/package_test.go @@ -0,0 +1,666 @@ +package cmd + +import ( + "archive/zip" + "bytes" + "context" + "encoding/json" + "errors" + "io" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "runtime" + "strings" + "sync" + "sync/atomic" + "testing" + + "github.com/concord-consortium/cc-data-cli/internal/api" + "github.com/concord-consortium/cc-data-cli/internal/config" + "github.com/concord-consortium/cc-data-cli/internal/dataset" + "github.com/concord-consortium/cc-data-cli/internal/output" + "github.com/concord-consortium/cc-data-cli/internal/packages" +) + +const testManifest = `{"name": "wildfire-responses", "title": "Wildfire responses", "version": "0.1.0", + "urls": {"all": [], "any": [], "none": []}, "clue_prepull": false, + "entrypoint": "run.py", "expected_duration_seconds": 300}` + +type seenRequest struct { + method, path, contentType, auth string + query map[string][]string + contentLength int64 + body []byte +} + +// packageServer answers the routes it is given and 404s every other path, as a report-server +// without the validate and applies routes does. +type packageServer struct { + *httptest.Server + mu sync.Mutex + seen []seenRequest +} + +func newPackageServer(t *testing.T, routes map[string]func(http.ResponseWriter, []byte)) *packageServer { + t.Helper() + s := &packageServer{} + s.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + s.mu.Lock() + s.seen = append(s.seen, seenRequest{r.Method, r.URL.Path, r.Header.Get("Content-Type"), r.Header.Get("Authorization"), r.URL.Query(), r.ContentLength, body}) + s.mu.Unlock() + if handle, ok := routes[r.URL.Path]; ok { + handle(w, body) + return + } + w.WriteHeader(http.StatusNotFound) + w.Write([]byte(`{"error":"NOT_FOUND","message":"Not found."}`)) + })) + t.Cleanup(s.Close) + return s +} + +func (s *packageServer) requests(path string) []seenRequest { + s.mu.Lock() + defer s.mu.Unlock() + var out []seenRequest + for _, r := range s.seen { + if r.path == path { + out = append(out, r) + } + } + return out +} + +func answer(status int, body string) func(http.ResponseWriter, []byte) { + return func(w http.ResponseWriter, _ []byte) { + w.WriteHeader(status) + w.Write([]byte(body)) + } +} + +const validated = `{"identity": "users/7/p", "version": "0.1.0", "checksum": "sha256:x", "visibility": "private", "already_published": false, "publishing_unavailable": null}` + +func validateAnswering(body string) map[string]func(http.ResponseWriter, []byte) { + return map[string]func(http.ResponseWriter, []byte){"/api/v1/packages/validate": answer(http.StatusOK, body)} +} + +func writePackage(t *testing.T) string { + t.Helper() + dir := t.TempDir() + for name, content := range map[string]string{"manifest.json": testManifest, "run.py": "print(1)\n"} { + if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + return dir +} + +func capture(t *testing.T) (*bytes.Buffer, *bytes.Buffer) { + t.Helper() + var out, errb bytes.Buffer + restore := output.SetStreams(&out, &errb) + t.Cleanup(restore) + return &out, &errb +} + +func defaultZip(dir string) string { + return filepath.Join(dir, packages.BuildDirName, "wildfire-responses-0.1.0.zip") +} + +func asCLIError(t *testing.T, err error) *output.CLIError { + t.Helper() + var cliErr *output.CLIError + if !errors.As(err, &cliErr) { + t.Fatalf("err = %v (%T), want a *output.CLIError", err, err) + } + return cliErr +} + +func TestPackageInitRefusesASecondInit(t *testing.T) { + dir := filepath.Join(t.TempDir(), "my-package") + written, err := packages.Init(dir, "") + if err != nil || strings.Join(written, ",") != "manifest.json,run.py" { + t.Fatalf("first init: %v, %v", written, err) + } + manifest, err := os.ReadFile(filepath.Join(dir, "manifest.json")) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(manifest), `"name": "my-package"`) { + t.Errorf("manifest = %s", manifest) + } + if err := os.WriteFile(filepath.Join(dir, "run.py"), []byte("mine"), 0o644); err != nil { + t.Fatal(err) + } + if _, err := packages.Init(dir, ""); err == nil { + t.Error("a second init succeeded") + } + if got, _ := os.ReadFile(filepath.Join(dir, "run.py")); string(got) != "mine" { + t.Errorf("the second init overwrote run.py: %q", got) + } + notADir := filepath.Join(t.TempDir(), "a-file") + if err := os.WriteFile(notADir, nil, 0o644); err != nil { + t.Fatal(err) + } + if _, err := packages.Init(notADir, ""); err == nil || strings.Contains(err.Error(), "already exists") { + t.Errorf("init into a regular file: err = %v, want the real error", err) + } + if _, err := packages.Init(filepath.Join(t.TempDir(), "Not_A_Name"), ""); err == nil { + t.Error("init accepted a directory name outside the grammar without --name") + } +} + +func TestPackageBuildValidatesThenWrites(t *testing.T) { + dir := writePackage(t) + srv := newPackageServer(t, validateAnswering(validated)) + out, _ := capture(t) + if err := (packageBuildFlags{}).run(context.Background(), api.New(srv.URL, "tok"), dir); err != nil { + t.Fatal(err) + } + reqs := srv.requests("/api/v1/packages/validate") + if len(reqs) != 1 { + t.Fatalf("%d validate requests, want 1", len(reqs)) + } + written, err := os.ReadFile(defaultZip(dir)) + if err != nil { + t.Fatal(err) + } + if !bytes.Equal(reqs[0].body, written) || reqs[0].contentType != "application/zip" { + t.Errorf("validate got %d bytes as %q; the zip written is %d bytes", len(reqs[0].body), reqs[0].contentType, len(written)) + } + if gi, err := os.ReadFile(filepath.Join(dir, packages.BuildDirName, ".gitignore")); err != nil || string(gi) != "*\n" { + t.Errorf(".cc-data-build/.gitignore = %q, %v", gi, err) + } + r, err := zip.NewReader(bytes.NewReader(written), int64(len(written))) + if err != nil || len(r.File) != 2 { + t.Fatalf("zip: %v, %v", r, err) + } + var line map[string]any + if err := json.Unmarshal(out.Bytes(), &line); err != nil { + t.Fatal(err) + } + if line["checksum"] != packages.Checksum(written) { + t.Errorf("printed checksum %v, archive %s", line["checksum"], packages.Checksum(written)) + } + + t.Chdir(dir) + out.Reset() + if err := (packageBuildFlags{}).run(context.Background(), api.New(srv.URL, "tok"), "."); err != nil { + t.Fatal(err) + } + var again map[string]any + if err := json.Unmarshal(out.Bytes(), &again); err != nil { + t.Fatal(err) + } + if again["checksum"] != line["checksum"] { + t.Errorf("a second build from inside the package gave %v, want %v", again["checksum"], line["checksum"]) + } +} + +func TestPackageBuildWritesNothingTheServerRefuses(t *testing.T) { + dir := writePackage(t) + srv := newPackageServer(t, map[string]func(http.ResponseWriter, []byte){ + "/api/v1/packages/validate": answer(http.StatusUnprocessableEntity, `{"error":"UNPROCESSABLE","message":"version must be MAJOR.MINOR.PATCH"}`), + }) + capture(t) + err := (packageBuildFlags{}).run(context.Background(), api.New(srv.URL, "tok"), dir) + if cliErr := asCLIError(t, err); cliErr.Code != "UNPROCESSABLE" || cliErr.ExitCode != output.ExitContract { + t.Errorf("err = %+v", cliErr) + } + if _, err := os.Stat(defaultZip(dir)); err == nil { + t.Error("a zip was written for a refused archive") + } +} + +// buildWarnings builds against a validate route answering body and returns stderr. +func buildWarnings(t *testing.T, body string) string { + t.Helper() + dir := writePackage(t) + srv := newPackageServer(t, validateAnswering(body)) + _, errb := capture(t) + if err := (packageBuildFlags{}).run(context.Background(), api.New(srv.URL, "tok"), dir); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(defaultZip(dir)); err != nil { + t.Errorf("no zip written: %v", err) + } + return errb.String() +} + +func TestPackageBuildWarnsOnAPublishedVersionAndStillWrites(t *testing.T) { + got := buildWarnings(t, strings.Replace(validated, `"already_published": false`, `"already_published": true`, 1)) + if !strings.Contains(got, "users/7/p 0.1.0 is already published; publish will refuse it until the version changes") { + t.Errorf("stderr = %q", got) + } +} + +func TestPackageBuildIsQuietOnANewVersion(t *testing.T) { + if got := buildWarnings(t, validated); strings.Contains(got, "already published") { + t.Errorf("stderr = %q", got) + } +} + +func TestPackageBuildWarnsWhenThePortalCannotPublishAndStillWrites(t *testing.T) { + got := buildWarnings(t, strings.Replace(validated, `"publishing_unavailable": null`, `"publishing_unavailable": "publishing is not configured for x"`, 1)) + if !strings.Contains(got, "this package cannot be published yet: publishing is not configured for x") { + t.Errorf("stderr = %q", got) + } +} + +func TestPackageBuildIsQuietWhenPublishingIsAvailable(t *testing.T) { + if got := buildWarnings(t, validated); strings.Contains(got, "cannot be published yet") { + t.Errorf("stderr = %q", got) + } +} + +func TestPackageBuildValidatesAsThePublishWillBe(t *testing.T) { + dir := writePackage(t) + srv := newPackageServer(t, validateAnswering(validated)) + capture(t) + client := api.New(srv.URL, "tok") + if err := (packageBuildFlags{origin: "projects/20", official: true}).run(context.Background(), client, dir); err != nil { + t.Fatal(err) + } + if err := (packageBuildFlags{}).run(context.Background(), client, dir); err != nil { + t.Fatal(err) + } + reqs := srv.requests("/api/v1/packages/validate") + if len(reqs) != 2 { + t.Fatalf("%d validate requests, want 2", len(reqs)) + } + if q := reqs[0].query; q["origin"][0] != "projects/20" || q["official"][0] != "true" { + t.Errorf("flagged build's query = %v", q) + } + if q := reqs[1].query; len(q) != 0 { + t.Errorf("plain build's query = %v, want none", q) + } +} + +func TestValidateReturnsAPublishedVersionAsAnAnswerNotAnError(t *testing.T) { + srv := newPackageServer(t, validateAnswering(strings.Replace(validated, `"already_published": false`, `"already_published": true`, 1))) + capture(t) + ans, err := validate(context.Background(), api.New(srv.URL, "tok"), []byte("zip"), "", false) + if err != nil || ans == nil || !ans.AlreadyPublished { + t.Errorf("validate = %+v, %v", ans, err) + } +} + +func TestValidateReturnsAnUnavailablePortalAsAnAnswerNotAnError(t *testing.T) { + srv := newPackageServer(t, validateAnswering(strings.Replace(validated, `"publishing_unavailable": null`, `"publishing_unavailable": "publishing is not configured for x"`, 1))) + capture(t) + ans, err := validate(context.Background(), api.New(srv.URL, "tok"), []byte("zip"), "", false) + if err != nil || ans == nil || ans.PublishingUnavailable == nil || *ans.PublishingUnavailable != "publishing is not configured for x" { + t.Errorf("validate = %+v, %v", ans, err) + } +} + +func TestPackageBuildOnAServerWithoutTheRouteStillBuilds(t *testing.T) { + dir := writePackage(t) + srv := newPackageServer(t, nil) + _, errb := capture(t) + if err := (packageBuildFlags{}).run(context.Background(), api.New(srv.URL, "tok"), dir); err != nil { + t.Fatal(err) + } + if !strings.Contains(errb.String(), "no validate route yet") { + t.Errorf("stderr = %q", errb.String()) + } + if _, err := os.Stat(defaultZip(dir)); err != nil { + t.Errorf("no zip written: %v", err) + } +} + +func TestPackageBuildRefusesAnOutputItWouldCollect(t *testing.T) { + dir := writePackage(t) + srv := newPackageServer(t, validateAnswering(validated)) + capture(t) + err := (packageBuildFlags{out: filepath.Join(dir, "p.zip")}).run(context.Background(), api.New(srv.URL, "tok"), dir) + if cliErr := asCLIError(t, err); cliErr.ExitCode != output.ExitUsage { + t.Errorf("err = %+v", cliErr) + } + for _, out := range []string{dir, filepath.Join(dir, packages.RunDirName, "pkg", "p.zip"), filepath.Join(dir, strings.ToUpper(packages.RunDirName), "p.zip")} { + err := (packageBuildFlags{out: out}).run(context.Background(), api.New(srv.URL, "tok"), dir) + if cliErr := asCLIError(t, err); cliErr.ExitCode != output.ExitUsage { + t.Errorf("--out %s: err = %+v, want a usage error", out, cliErr) + } + } + if err := (packageBuildFlags{out: filepath.Join(dir, ".builds", "p.zip")}).run(context.Background(), api.New(srv.URL, "tok"), dir); err != nil { + t.Errorf("an --out under a dot directory was refused: %v", err) + } +} + +func TestPackageBuildRefusesANameThatIsAPath(t *testing.T) { + root := t.TempDir() + dir := filepath.Join(root, "a", "pkg") + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + manifest := strings.Replace(testManifest, `"name": "wildfire-responses"`, `"name": "../../escaped"`, 1) + for name, content := range map[string]string{"manifest.json": manifest, "run.py": "print(1)\n"} { + if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } + srv := newPackageServer(t, nil) + capture(t) + err := (packageBuildFlags{}).run(context.Background(), api.New(srv.URL, "tok"), dir) + if cliErr := asCLIError(t, err); cliErr.Code != "INVALID_MANIFEST" { + t.Errorf("err = %+v", cliErr) + } + var zips []string + filepath.WalkDir(root, func(p string, d os.DirEntry, err error) error { + if err == nil && strings.HasSuffix(p, ".zip") { + zips = append(zips, p) + } + return nil + }) + if len(zips) != 0 { + t.Errorf("zips written: %v", zips) + } +} + +const testScopeJSON = `{"kind": "class", "id": "h", + "classes": [{"class_hash": "0123456789abcdef0123456789abcdef0123456789abcdef", "class_id": 6}], + "assignments": [{"offering_id": 9, "runnable_id": 1234, "name": "Wildfire", "url": "https://activity-player.concord.org/?sequence=830"}]}` + +func testScope(t *testing.T) packages.LocalScope { + t.Helper() + scope, err := packages.ParseLocalScope([]byte(testScopeJSON)) + if err != nil { + t.Fatal(err) + } + return scope +} + +// loadRunPackage writes a package of the given manifest and files and a scope file, and loads +// them as package run does before it looks up a credential. +func loadRunPackage(t *testing.T, manifest string, files map[string]string, environ func() []string) (packageRunFlags, loadedPackage) { + t.Helper() + dir := t.TempDir() + files["manifest.json"] = manifest + for name, content := range files { + if err := os.WriteFile(filepath.Join(dir, name), []byte(content), 0o755); err != nil { + t.Fatal(err) + } + } + scopePath := filepath.Join(t.TempDir(), "scope.json") + if err := os.WriteFile(scopePath, []byte(testScopeJSON), 0o644); err != nil { + t.Fatal(err) + } + f := packageRunFlags{scopePath: scopePath, environ: environ} + pkg, err := f.load(dir) + if err != nil { + t.Fatal(err) + } + return f, pkg +} + +func TestPackageRunAsksTheServersMatcher(t *testing.T) { + srv := newPackageServer(t, map[string]func(http.ResponseWriter, []byte){ + "/api/v1/packages/applies": answer(http.StatusOK, `{"applies": false, "reason": "no URL in this class matches the required pattern *//wildfire.concord.org/*", "interactive_urls": [], "unread": [], "truncated": false}`), + }) + capture(t) + verdict, err := appliesFor(api.New(srv.URL, "tok"), testScope(t))(context.Background(), packages.URLs{All: []string{"*//wildfire.concord.org/*"}}) + if err != nil { + t.Fatal(err) + } + if verdict.Applies || verdict.Unconfirmed || !strings.Contains(verdict.Reason, "required pattern") { + t.Errorf("verdict = %+v", verdict) + } + var sent struct { + URLs map[string][]string `json:"urls"` + AssignmentURLs []string `json:"assignment_urls"` + } + if err := json.Unmarshal(srv.requests("/api/v1/packages/applies")[0].body, &sent); err != nil { + t.Fatal(err) + } + if strings.Join(sent.URLs["all"], ",") != "*//wildfire.concord.org/*" || strings.Join(sent.AssignmentURLs, ",") != "https://activity-player.concord.org/?sequence=830" { + t.Errorf("sent %+v", sent) + } +} + +func TestPackageRunOnAServerWithoutTheRouteIsUnconfirmed(t *testing.T) { + srv := newPackageServer(t, nil) + capture(t) + verdict, err := appliesFor(api.New(srv.URL, "tok"), testScope(t))(context.Background(), packages.URLs{Any: []string{"*x*"}}) + if err != nil || !verdict.Unconfirmed { + t.Errorf("verdict = %+v, %v", verdict, err) + } +} + +func TestPackageRunAlwaysSendsURLs(t *testing.T) { + srv := newPackageServer(t, map[string]func(http.ResponseWriter, []byte){ + "/api/v1/packages/applies": answer(http.StatusOK, `{"applies": true, "reason": null}`), + }) + capture(t) + if _, err := appliesFor(api.New(srv.URL, "tok"), testScope(t))(context.Background(), packages.URLs{None: []string{"*x*"}}); err != nil { + t.Fatal(err) + } + var sent map[string]json.RawMessage + if err := json.Unmarshal(srv.requests("/api/v1/packages/applies")[0].body, &sent); err != nil { + t.Fatal(err) + } + if urls, ok := sent["urls"]; !ok || string(urls) == "null" { + t.Errorf("applies body = %s", srv.requests("/api/v1/packages/applies")[0].body) + } +} + +func TestPackageRunKeepsTheServersCode(t *testing.T) { + for _, tc := range []struct { + apiErr *api.APIError + exit int + }{ + {&api.APIError{Status: 503, Code: "SERVICE_UNAVAILABLE", Message: "deriver down"}, output.ExitContract}, + {&api.APIError{Status: 401, Code: "NOT_AUTHENTICATED"}, output.ExitNotAuth}, + } { + got := asCLIError(t, packageRunError(api.AsCLIError(tc.apiErr))) + if got.Code != tc.apiErr.Code || got.ExitCode != tc.exit { + t.Errorf("%s: got %s exit %d", tc.apiErr.Code, got.Code, got.ExitCode) + } + } +} + +func publishAnswering(checksum string) map[string]func(http.ResponseWriter, []byte) { + return map[string]func(http.ResponseWriter, []byte){"/api/v1/packages": func(w http.ResponseWriter, body []byte) { + if checksum == "" { + checksum = packages.Checksum(body) + } + w.WriteHeader(http.StatusCreated) + json.NewEncoder(w).Encode(map[string]any{"catalog_id": 3, "identity": "projects/20/p", "version": "0.1.0", + "checksum": checksum, "visibility": "private", "official": false, "current_version": "0.1.0"}) + }} +} + +func TestPackagePublishSendsTheZipAsTheBody(t *testing.T) { + srv := newPackageServer(t, publishAnswering("")) + out, _ := capture(t) + archive := []byte("PK zip bytes") + if err := (packagePublishFlags{origin: "projects/20"}).run(context.Background(), api.New(srv.URL, "tok"), archive); err != nil { + t.Fatal(err) + } + reqs := srv.requests("/api/v1/packages") + if len(reqs) != 1 { + t.Fatalf("%d publish requests, want 1", len(reqs)) + } + r := reqs[0] + if r.method != http.MethodPost || r.contentType != "application/zip" || r.contentLength != int64(len(archive)) || !bytes.Equal(r.body, archive) || r.auth != "Bearer tok" { + t.Errorf("request = %s %q length %d auth %q", r.method, r.contentType, r.contentLength, r.auth) + } + if r.query["origin"][0] != "projects/20" || r.query["official"] != nil { + t.Errorf("query = %v", r.query) + } + if !strings.Contains(out.String(), "identity projects/20/p") { + t.Errorf("stdout = %q", out.String()) + } +} + +func TestPackagePublishRefusesAChecksumItDidNotSend(t *testing.T) { + srv := newPackageServer(t, publishAnswering("sha256:other")) + capture(t) + err := (packagePublishFlags{}).run(context.Background(), api.New(srv.URL, "tok"), []byte("zip")) + if err == nil || !strings.Contains(err.Error(), "sha256:other") { + t.Errorf("err = %v", err) + } +} + +func TestPackagePublishPassesTheServersCodeThrough(t *testing.T) { + srv := newPackageServer(t, map[string]func(http.ResponseWriter, []byte){ + "/api/v1/packages": answer(http.StatusConflict, `{"error":"ALREADY_EXISTS","message":"version 0.1.0 exists"}`), + }) + capture(t) + err := (packagePublishFlags{}).run(context.Background(), api.New(srv.URL, "tok"), []byte("zip")) + if cliErr := asCLIError(t, err); cliErr.Code != "ALREADY_EXISTS" || cliErr.ExitCode != output.ExitContract { + t.Errorf("err = %+v", cliErr) + } + if n := len(srv.requests("/api/v1/packages")); n != 1 { + t.Errorf("%d publish requests, want 1", n) + } +} + +func TestPackageRunWarnsWhatTheDeriverCouldNotRead(t *testing.T) { + srv := newPackageServer(t, map[string]func(http.ResponseWriter, []byte){ + "/api/v1/packages/applies": answer(http.StatusOK, `{"applies": true, "reason": null, "unread": [{"url": "https://a/1", "reason": "timeout"}], "truncated": true}`), + }) + _, errb := capture(t) + if _, err := appliesFor(api.New(srv.URL, "tok"), testScope(t))(context.Background(), packages.URLs{Any: []string{"*x*"}}); err != nil { + t.Fatal(err) + } + for _, want := range []string{"could not read https://a/1 (timeout)", "profile was truncated"} { + if !strings.Contains(errb.String(), want) { + t.Errorf("stderr = %q, want %q", errb.String(), want) + } + } +} + +func TestPackageRunRefusesAnUnreadableScopeFile(t *testing.T) { + dir := writePackage(t) + capture(t) + cmd := newPackageRunCmd() + cmd.SetArgs([]string{dir, "--dataset", "learn.concord.org/d", "--scope", filepath.Join(t.TempDir(), "missing.json")}) + cmd.SetOut(io.Discard) + cmd.SetErr(io.Discard) + if cliErr := asCLIError(t, cmd.Execute()); cliErr.Code != "INVALID_SCOPE" || cliErr.ExitCode != output.ExitUsage { + t.Errorf("err = %+v", cliErr) + } +} + +func TestPackagePublishMapsAuthAndUnansweredFailures(t *testing.T) { + srv := newPackageServer(t, map[string]func(http.ResponseWriter, []byte){ + "/api/v1/packages": answer(http.StatusUnauthorized, `{"error":"NOT_AUTHENTICATED","message":"bad token"}`), + }) + capture(t) + err := (packagePublishFlags{}).run(context.Background(), api.New(srv.URL, "tok"), []byte("zip")) + if cliErr := asCLIError(t, err); cliErr.ExitCode != output.ExitNotAuth { + t.Errorf("NOT_AUTHENTICATED: %+v", cliErr) + } + + var attempts atomic.Int32 + dropped := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + attempts.Add(1) + conn, _, _ := w.(http.Hijacker).Hijack() + conn.Close() + })) + t.Cleanup(dropped.Close) + err = (packagePublishFlags{}).run(context.Background(), api.New(dropped.URL, "tok"), []byte("zip")) + if cliErr := asCLIError(t, err); !strings.Contains(cliErr.Action, "Publishing again is safe") { + t.Errorf("unanswered: %+v", cliErr) + } + if n := attempts.Load(); n != 1 { + t.Errorf("an unanswered publish was sent %d times, want 1", n) + } +} + +func TestPackageRunValidatesThenRunsWithTheRunnersNames(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("the entrypoint is a shell script") + } + manifest := strings.Replace(testManifest, `"entrypoint": "run.py"`, `"entrypoint": "run.sh"`, 1) + script := "#!/bin/sh\nenv > \"$RD_OUTPUT_DIR/../env.txt\"\ncp \"$RD_SCOPE_FILE\" \"$RD_OUTPUT_DIR/../scope.json\"\necho ok > \"$RD_OUTPUT_DIR/display.md\"\n" + f, pkg := loadRunPackage(t, manifest, map[string]string{"run.sh": script}, func() []string { return []string{"PATH=" + os.Getenv("PATH")} }) + dir := pkg.dir + var ranBeforeValidate atomic.Bool + srv := newPackageServer(t, map[string]func(http.ResponseWriter, []byte){"/api/v1/packages/validate": func(w http.ResponseWriter, body []byte) { + if _, err := os.Stat(filepath.Join(dir, packages.RunDirName)); err == nil { + ranBeforeValidate.Store(true) + } + answer(http.StatusOK, validated)(w, body) + }}) + out, _ := capture(t) + ref := dataset.Ref{Portal: config.MustPortal("learn.concord.org"), Name: "wildfire"} + t.Chdir(t.TempDir()) + cwd, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + if err := f.run(context.Background(), api.New(srv.URL, "tok"), pkg, ref, "data-root"); err != nil { + t.Fatal(err) + } + if ranBeforeValidate.Load() { + t.Error("the run tree existed before validate answered") + } + + reqs := srv.requests("/api/v1/packages/validate") + if len(reqs) != 1 || reqs[0].contentType != "application/zip" || len(reqs[0].body) == 0 { + t.Fatalf("validate requests = %d", len(reqs)) + } + run := filepath.Join(dir, packages.RunDirName) + env, err := os.ReadFile(filepath.Join(run, "env.txt")) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{"RD_DATASET=learn.concord.org/wildfire\n", "CC_DATA_PORTAL=learn.concord.org\n", "CC_DATA_ROOT=" + filepath.Join(cwd, "data-root") + "\n", "RD_REPORT_SERVER_URL=" + srv.URL + "\n"} { + if !strings.Contains(string(env), want) { + t.Errorf("environment lacks %q", strings.TrimSpace(want)) + } + } + var scope map[string]any + raw, err := os.ReadFile(filepath.Join(run, "scope.json")) + if err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(raw, &scope); err != nil || scope["dataset"] != "wildfire" { + t.Errorf("scope.json dataset = %v, %v", scope["dataset"], err) + } + var line map[string]any + if err := json.Unmarshal(out.Bytes(), &line); err != nil { + t.Fatalf("stdout %q: %v", out.String(), err) + } + for _, key := range []string{"display", "display_bytes", "summary", "counts", "elapsed_seconds"} { + if _, ok := line[key]; !ok { + t.Errorf("result line lacks %s: %v", key, line) + } + } +} + +func TestPackageRunPassesAnAppliesFailureThrough(t *testing.T) { + manifest := strings.Replace(testManifest, `"all": []`, `"all": ["*//wildfire.concord.org/*"]`, 1) + f, pkg := loadRunPackage(t, manifest, map[string]string{"run.py": "print(1)\n"}, func() []string { return nil }) + srv := newPackageServer(t, map[string]func(http.ResponseWriter, []byte){ + "/api/v1/packages/validate": answer(http.StatusOK, validated), + "/api/v1/packages/applies": answer(http.StatusServiceUnavailable, `{"error":"SERVICE_UNAVAILABLE","message":"the deriver is unavailable"}`), + }) + capture(t) + ref := dataset.Ref{Portal: config.MustPortal("learn.concord.org"), Name: "wildfire"} + err := f.run(context.Background(), api.New(srv.URL, "tok"), pkg, ref, "/data/root") + if cliErr := asCLIError(t, err); cliErr.Code != "SERVICE_UNAVAILABLE" || cliErr.ExitCode != output.ExitContract { + t.Errorf("err = %+v", cliErr) + } +} + +func TestPackageRunErrorsAreTheRunnersCodesAtExitOne(t *testing.T) { + for _, tc := range []struct { + err error + code string + }{ + {&packages.Refused{Reason: "does not apply"}, "PACKAGE_REFUSED"}, + {&packages.Failed{Reason: "exited 3"}, "PACKAGE_FAILED"}, + {&packages.OutputRefused{Reason: "no display.md"}, "PACKAGE_OUTPUT_REFUSED"}, + } { + got := asCLIError(t, packageRunError(tc.err)) + if got.Code != tc.code || got.ExitCode != output.ExitInternal || got.Message != tc.err.Error() { + t.Errorf("%T: got %s exit %d %q", tc.err, got.Code, got.ExitCode, got.Message) + } + } +} diff --git a/cmd/root.go b/cmd/root.go index 077176d..9353c1a 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -49,6 +49,7 @@ func newRootCmd() *cobra.Command { root.AddCommand(newReportsCmd()) root.AddCommand(newGetCmd()) root.AddCommand(newQueryCmd()) + root.AddCommand(newPackageCmd()) root.AddCommand(newReplCmd()) root.AddCommand(newInitCmd()) root.AddCommand(newUninstallCmd()) diff --git a/docs/researcher-guide.md b/docs/researcher-guide.md index 3c0d112..f4deef8 100644 --- a/docs/researcher-guide.md +++ b/docs/researcher-guide.md @@ -346,6 +346,7 @@ When you (or Claude) query a dataset, the data is exposed as a set of SQL | `attachment_content` | The text/JSON content of every saved CODAP/SageModeler snapshot, queryable and diffable. | | `student_id_mapping` | One row per learner from Student ID Mapping runs, with the ids that join them to their answers and history. | | `student_metadata` | One row per learner from Student Metadata runs: name, username, class, school, teachers, permission forms. | +| `run_answers` | Every answer with the `run_id` of each run that holds it: `SELECT count(*) FROM run_answers WHERE run_id = 584` counts one run's answers. Join it to `student_id_mapping` on `remote_endpoint = run_remote_endpoint` to count learners by `user_id`. | | `run_membership`, `downloads` | Provenance: which run's fetch covered which records, and what each download was, including whether its run hid names. | ### Speeding up a large dataset @@ -584,7 +585,101 @@ write the cross-dataset query for you. --- -## 7. A few things to keep in mind +## 7. Developing and publishing a package + +A Researcher Dashboard **package** is a small program the dashboard runs for a +class on a short-lived virtual machine (the "runner"), which pulls the class's +data with `cc-data` and writes a short report. `cc-data package` lets you write +one on your own machine, run it the way the runner will, and publish it to the +dashboard's catalog with the cc-data login you already have. + +### The loop + +``` +cc-data package init my-package # writes manifest.json and run.py +# edit run.py and manifest.json +cc-data package run my-package --dataset learn.concord.org/my-package-dev --scope ~/scopes/class-6.json +cc-data package build my-package # writes my-package/.cc-data-build/-.zip +cc-data package publish my-package/.cc-data-build/my-package-0.1.0.zip --portal prod +``` + +The `run.py` that `init` writes is a complete package: it finds or creates a +Student ID Mapping run for the class, pulls its answers, counts answers and +learners through `cc-data query`, and writes `display.md`, `summary.txt` and +`counts.json`. Change the counting and the display, and keep the rest. + +### The scope file + +`package run` needs to know which class to run for. On the VM the dashboard +supplies that; on your machine a scope file does, holding exactly these four keys: + +```json +{ + "kind": "class", + "id": "", + "classes": [{"class_hash": "", "class_id": 6}], + "assignments": [{"offering_id": 9, "runnable_id": 1234, "name": "Moth 1.2", "url": "https://..."}] +} +``` + +Keep it **outside the package directory**, or under a name starting with a dot +such as `.scope.json`, so `build` never ships a class hash in the zip. + +### What `run` reproduces, and what it cannot + +`run` stages exactly the files `build` would ship into +`/.cc-data-run/pkg/` and runs the package from there, writes the runner's +`scope.json`, sets the runner's environment variables (`RD_DATASET`, +`RD_SCOPE_FILE`, `RD_OUTPUT_DIR` and the rest), applies the same time limit, and +reads the result as the runner does, including the 65,536-byte cap on +`display.md`. A package that reads a file `build` leaves out, such as something +under `local-data/`, fails locally as it would on the VM. + +What it cannot reproduce: + +- **No sandbox.** The package runs as you, with your network. The VM runs it as + its own user behind an egress proxy. +- **A few of your variables pass through.** `HOME`, `PATH`, your locale, the + proxy settings and a handful of others reach the package, so its own `cc-data` + calls find your login. Nothing else from your environment does. A private folder + holding only a link to the `cc-data` running `package run` goes first on the + package's `PATH`, so the package calls that same version rather than an older + one installed elsewhere, and nothing else moves ahead of your `PATH`. +- **Python.** A `.py` entrypoint runs with `python3.11` when it is on your + `PATH`, as on the VM, and otherwise with `python3` after a note saying so. +- **CLUE.** A package with `clue_prepull: true` is refused locally, since + `cc-data` cannot fetch CLUE data yet. + +### Checked against report-server + +`build` and `run` send the zip to report-server, which checks it against the +catalog's rules without publishing it, and `run` asks report-server whether the +package's `urls` patterns apply to the scope's assignments. A refusal stops the +command with report-server's own message. A report-server too old to have these +checks answers with a warning, and the command carries on: the package is then +unchecked until you publish it. + +`build` also warns, and still writes the zip, when that version is already +published (bump `version` in `manifest.json`) or when the portal cannot store +packages yet. + +### Publishing + +`publish` sends the zip to the catalog with your cc-data token. A first publish +creates the package private to you; `--origin projects/` publishes it as a +project's, and `--official` needs the publisher role. A portal whose report-server +has no package storage configured answers `UNPROCESSABLE` "publishing is not +configured for " even though the zip itself passed every check. + +### Files the commands leave in your package + +`.cc-data-run/` (the last run's staged copy, inputs, outputs and the package's +data directory) and `.cc-data-build/` (built zips) each carry a `.gitignore` +of `*`, so git never commits them. **`.cc-data-run/` holds student data** +(`display.md` is drawn from it), so it is readable only by you, as datasets are. +`build` never ships either folder, nor any other path starting with a dot. + +## 8. A few things to keep in mind - **One dataset belongs to one portal.** Run IDs are specific to a portal (and to the report server behind it); the same run number on a different portal is a @@ -607,7 +702,7 @@ write the cross-dataset query for you. --- -## 8. Troubleshooting +## 9. Troubleshooting - **`NOT_AUTHENTICATED`, or a prompt to "run `cc-data login`".** Your token is missing or expired. Run `cc-data login` (add `--portal` if you're not on @@ -627,7 +722,7 @@ write the cross-dataset query for you. --- -## 9. Sensitive data and cleanup +## 10. Sensitive data and cleanup Downloading turns a transient export into a durable local corpus that can contain student names and other PII. Keep only what you need, and purge datasets when you diff --git a/internal/api/client.go b/internal/api/client.go index b48db69..e61c75e 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -95,6 +95,11 @@ const maxBodyBytes = 64 << 20 // do runs a request under the retry policy and returns the 2xx body, or a typed // *APIError (contract) / *TransientError (budget exhausted). func (c *Client) do(ctx context.Context, method, path string, query url.Values, body []byte) ([]byte, error) { + return c.send(ctx, method, path, query, body, "application/json") +} + +// send is do with the body's media type named, for the routes that take raw bytes. +func (c *Client) send(ctx context.Context, method, path string, query url.Values, body []byte, contentType string) ([]byte, error) { u := c.BaseURL + path if len(query) > 0 { u += "?" + query.Encode() @@ -110,7 +115,7 @@ func (c *Client) do(ctx context.Context, method, path string, query url.Values, return nil, err } - data, status, err := c.attempt(ctx, method, u, body) + data, status, err := c.attempt(ctx, method, u, body, contentType) if err != nil { last = err // A non-idempotent request (POST) may have reached the server, so @@ -139,7 +144,7 @@ func (c *Client) do(ctx context.Context, method, path string, query url.Values, // attempt performs one HTTP request under a per-attempt deadline and returns the // body and status; a transport error (including deadline) is returned as err. -func (c *Client) attempt(ctx context.Context, method, u string, body []byte) ([]byte, int, error) { +func (c *Client) attempt(ctx context.Context, method, u string, body []byte, contentType string) ([]byte, int, error) { reqCtx := ctx var cancel context.CancelFunc if c.RequestTimeout > 0 { @@ -160,7 +165,7 @@ func (c *Client) attempt(ctx context.Context, method, u string, body []byte) ([] } req.Header.Set("Accept", "application/json") if body != nil { - req.Header.Set("Content-Type", "application/json") + req.Header.Set("Content-Type", contentType) } resp, err := c.HTTP.Do(req) diff --git a/internal/api/packages.go b/internal/api/packages.go new file mode 100644 index 0000000..ad5591f --- /dev/null +++ b/internal/api/packages.go @@ -0,0 +1,108 @@ +package api + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/url" +) + +// PublishedPackage is report-server's answer to a publish. +type PublishedPackage struct { + CatalogID int64 `json:"catalog_id"` + Identity string `json:"identity"` + Version string `json:"version"` + Checksum string `json:"checksum"` + Visibility string `json:"visibility"` + Official bool `json:"official"` + CurrentVersion string `json:"current_version"` +} + +// PublishPackage posts a package archive as the raw request body. origin is "" for the +// caller's own users/, or projects/; official asks the server to mark it official, +// which only a publisher may. Like every POST it is never retried. +func (c *Client) PublishPackage(ctx context.Context, archive []byte, origin string, official bool) (*PublishedPackage, error) { + data, err := c.send(ctx, http.MethodPost, "/api/v1/packages", packageQuery(origin, official), archive, "application/zip") + if err != nil { + return nil, err + } + var out PublishedPackage + if err := json.Unmarshal(data, &out); err != nil { + return nil, err + } + return &out, nil +} + +// ValidatedPackage is what a publish of the archive would record. The last two are not +// refusals: each says only a publish would be refused now, with publish's own message for the +// second. +type ValidatedPackage struct { + Identity string `json:"identity"` + Version string `json:"version"` + Checksum string `json:"checksum"` + Visibility string `json:"visibility"` + AlreadyPublished bool `json:"already_published"` + PublishingUnavailable *string `json:"publishing_unavailable"` +} + +// ValidatePackage asks report-server to apply every publish check to the archive and stop +// before storing it, with the same origin and official a publish would send. A refusal is the +// same coded error a publish would answer. +func (c *Client) ValidatePackage(ctx context.Context, archive []byte, origin string, official bool) (*ValidatedPackage, error) { + data, err := c.send(ctx, http.MethodPost, "/api/v1/packages/validate", packageQuery(origin, official), archive, "application/zip") + if err != nil { + return nil, err + } + var out ValidatedPackage + if err := json.Unmarshal(data, &out); err != nil { + return nil, err + } + return &out, nil +} + +// AppliesRequest names the patterns and the scope's assignment URLs, which report-service's +// deriver follows into their activities, so the URLs matched are the runner's. +type AppliesRequest struct { + URLs map[string][]string `json:"urls"` + AssignmentURLs []string `json:"assignment_urls,omitempty"` +} + +// AppliesAnswer is report-server's verdict, with what the deriver read and could not. +type AppliesAnswer struct { + Applies bool `json:"applies"` + Reason string `json:"reason"` + Unread []struct { + URL string `json:"url"` + Reason string `json:"reason"` + } `json:"unread"` + Truncated bool `json:"truncated"` +} + +// PackageApplies asks report-server's one matcher whether the patterns match the scope. +func (c *Client) PackageApplies(ctx context.Context, req AppliesRequest) (*AppliesAnswer, error) { + var out AppliesAnswer + if err := c.postJSON(ctx, "/api/v1/packages/applies", req, &out); err != nil { + return nil, err + } + return &out, nil +} + +func packageQuery(origin string, official bool) url.Values { + q := url.Values{} + if origin != "" { + q.Set("origin", origin) + } + if official { + q.Set("official", "true") + } + return q +} + +// RouteMissing reports a 404 from the validate or applies route: a report-server older than +// those routes. Their contract is never to answer 404 themselves, refusing with 403 or 422 as +// publish does, so a 404 cannot be a refusal. +func RouteMissing(err error) bool { + var apiErr *APIError + return errors.As(err, &apiErr) && apiErr.Status == http.StatusNotFound +} diff --git a/internal/duck/run_views_test.go b/internal/duck/run_views_test.go new file mode 100644 index 0000000..58ba696 --- /dev/null +++ b/internal/duck/run_views_test.go @@ -0,0 +1,116 @@ +package duck + +import ( + "context" + "encoding/json" + "fmt" + "os" + "strings" + "testing" + "time" + + "github.com/concord-consortium/cc-data-cli/internal/store" +) + +// The documented learner count, scoped by the answers' run as the guidance gives it. +const learnersOfRun = "SELECT count(DISTINCT m.user_id) FROM student_id_mapping m JOIN run_answers a ON a.remote_endpoint = m.run_remote_endpoint WHERE a.run_id = %d" + +// Each wrong way to count gives a different number here: counting endpoints instead of users +// (user 1 answered in two assignments), joining the bare-endpoint learner (who would borrow +// another bare answer), counting an answer once instead of once per run (e3/q1 is in runs 700 +// and 900), joining membership untyped (run 700's history holds e1a/q1 too), leaving the +// answers unscoped (run 800 is another class), and scoping the mapping by run (run 900, a later +// mapping run of the same class, holds user 3's row in the deduplicated view). +func TestRunAnswersAndTheDocumentedLearnerCount(t *testing.T) { + d := newDS(t, "ds") + buildStore(t, d, 700, [][]byte{ + answerRec("s", "https://p/d/e1a", "q1", "a"), answerRec("s", "https://p/d/e1b", "q1", "b"), + answerRec("s", "https://p/d/e3", "q1", "c"), answerRec("s", "https://p/d/", "q1", "bare"), + }) + hist, _ := json.Marshal(map[string]any{"source_key": "s", "remote_endpoint": "https://p/d/e1a", "question_id": "q1", "history_id": "h1"}) + seg := store.OpenSegment(d.Dir, store.TypeHistory, 700) + if err := seg.AppendPage([][]byte{hist}, time.Unix(700, 0).UTC(), 700); err != nil { + t.Fatal(err) + } + seg.WriteCursor(&store.Cursor{Items: 1}) + if _, err := d.MergeCompact(store.TypeHistory, 700, seg); err != nil { + t.Fatal(err) + } + buildStore(t, d, 800, [][]byte{answerRec("s", "https://p/d/e9", "q1", "z")}) + buildStore(t, d, 900, [][]byte{answerRec("s", "https://p/d/e3", "q1", "c")}) + row := func(learner, user, class, offering int, endpoint string) string { + return fmt.Sprintf("%d,%d,%d,s%d,%d,%d,https://act/1,%s", learner, user, user, user, class, offering, endpoint) + } + addDimensionCSV(t, d, dimFixture{run: 700, slug: "student-id-mapping", fetchedAt: at(1), filter: `{}`, csv: mappingCSV( + row(101, 1, 1, 70, "https://p/d/e1a"), row(102, 1, 1, 71, "https://p/d/e1b"), + row(103, 3, 1, 70, "https://p/d/e3"), row(104, 4, 1, 70, "https://p/d/"))}) + addDimensionCSV(t, d, dimFixture{run: 800, slug: "student-id-mapping", fetchedAt: at(2), filter: `{}`, csv: mappingCSV( + row(109, 9, 2, 90, "https://p/d/e9"))}) + addDimensionCSV(t, d, dimFixture{run: 900, slug: "student-id-mapping", fetchedAt: at(3), filter: `{}`, csv: mappingCSV( + row(103, 3, 1, 70, "https://p/d/e3"))}) + e := openEngine(t, []DatasetSpec{{DS: d}}, nil) + + if n := queryInt(t, e, "SELECT count(*) FROM run_answers WHERE run_id = 700"); n != 4 { + t.Errorf("run 700 answers = %d, want 4", n) + } + if n := queryInt(t, e, "SELECT count(*) FROM run_answers WHERE run_id = 900"); n != 1 { + t.Errorf("run 900 answers = %d, want 1", n) + } + if n := queryInt(t, e, fmt.Sprintf(learnersOfRun, 700)); n != 2 { + t.Errorf("run 700 learners = %d, want 2 (users 1 and 3; user 4's endpoint is bare)", n) + } + if n := queryInt(t, e, fmt.Sprintf(learnersOfRun, 800)); n != 1 { + t.Errorf("run 800 learners = %d, want 1", n) + } +} + +func TestRunAnswersIsEmptyNotMissingOnAFreshDataset(t *testing.T) { + e := openEngine(t, []DatasetSpec{{DS: newDS(t, "ds")}}, nil) + if n := queryInt(t, e, "SELECT count(*) FROM run_answers"); n != 0 { + t.Errorf("run_answers has %d rows", n) + } + if n := queryInt(t, e, fmt.Sprintf(learnersOfRun, 1)); n != 0 { + t.Errorf("the learner count binds to %d on a fresh dataset, want 0", n) + } +} + +func TestRunAnswersRunIDIsBigint(t *testing.T) { + d := newDS(t, "ds") + buildStore(t, d, 700, [][]byte{answerRec("s", "https://p/d/e1", "q1", "a")}) + for name, e := range map[string]*Engine{ + "with membership": openEngine(t, []DatasetSpec{{DS: d}}, nil), + "fresh": openEngine(t, []DatasetSpec{{DS: newDS(t, "fresh")}}, nil), + } { + rows, err := e.Query(context.Background(), "SELECT data_type FROM information_schema.columns WHERE table_name = 'run_answers' AND column_name = 'run_id'") + if err != nil { + t.Fatalf("%s: %v", name, err) + } + var typ string + if rows.Next() { + rows.Scan(&typ) + } + rows.Close() + if typ != "BIGINT" { + t.Errorf("%s: run_id is %s, want BIGINT", name, typ) + } + } +} + +// The guidance and the init stub each carry the learner count as text; both must stay the +// query the test above checks. +func TestTheDocumentedLearnerCountIsTheTestedOne(t *testing.T) { + flat := func(path string) string { + raw, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + return strings.Join(strings.Fields(string(raw)), " ") + } + if want := "`" + fmt.Sprintf(learnersOfRun, 584) + "`"; !strings.Contains(flat("../guidance/src/core.md"), want) { + t.Errorf("core.md no longer documents %q", want) + } + stub := strings.ReplaceAll(flat("../packages/template/run.py"), " AS n FROM", " FROM") + if want := strings.ReplaceAll(learnersOfRun, "%d", "{run}") + `"""`; !strings.Contains(stub, want) { + t.Errorf("the init stub no longer runs %q", want) + } +} diff --git a/internal/duck/views.go b/internal/duck/views.go index 51db436..e3d262b 100644 --- a/internal/duck/views.go +++ b/internal/duck/views.go @@ -71,6 +71,7 @@ func (vs viewSet) statements() []viewStmt { stmts = append(stmts, vs.storeView(store.TypeAnswers)) stmts = append(stmts, vs.storeView(store.TypeHistory)) stmts = append(stmts, vs.runMembershipView()) + stmts = append(stmts, vs.runAnswersView()) stmts = append(stmts, vs.downloadsView()) stmts = append(stmts, vs.attachmentFilesView()) stmts = append(stmts, vs.attachmentStatesView()) @@ -360,6 +361,19 @@ func (vs viewSet) runMembershipView() viewStmt { return viewStmt{name: name, primary: primary, fallback: fallback, files: files} } +// runAnswersView is answers with the run_id of each answers membership. Always bound, unlike +// answers_; it must follow both views it reads and declares no files, so materialize reads +// it through theirs. +func (vs viewSet) runAnswersView() viewStmt { + name := vs.prefix + `"run_answers"` + answers := vs.prefix + sqlIdent(store.TypeAnswers) + membership := vs.prefix + `"run_membership"` + primary := fmt.Sprintf("CREATE VIEW %s AS SELECT CAST(m.run_id AS BIGINT) AS run_id, s.* FROM %s s JOIN %s m USING (source_key, remote_endpoint, question_id) WHERE m.type = %s", + name, answers, membership, sqlStr(store.TypeAnswers)) + fallback := fmt.Sprintf("CREATE VIEW %s AS SELECT CAST(NULL AS BIGINT) AS run_id, s.* FROM %s s WHERE false", name, answers) + return viewStmt{name: name, primary: primary, fallback: fallback} +} + // downloadsView is a VALUES dimension table from the manifest. // // hide_names separates the two meanings of a name column: where a run hid names, student_name diff --git a/internal/guidance/src/core.md b/internal/guidance/src/core.md index a77a57c..50e7008 100644 --- a/internal/guidance/src/core.md +++ b/internal/guidance/src/core.md @@ -122,6 +122,27 @@ like `wildfire_2026.answers`): **type-qualified**: `answers a JOIN run_membership m USING (source_key, remote_endpoint, question_id) WHERE m.run_id = 584 AND m.type = 'answers'`. History joins add `history_id` to the USING list. +- `run_answers` — every `answers` row with `run_id`, once for each run whose + answers fetch holds it (a Student Answers or a Student ID Mapping run), so one + run's answers are `SELECT count(*) FROM + run_answers WHERE run_id = 584`. It is the type-qualified membership join above, + always present, so a run with no answers counts zero rather than failing to bind + as `answers_` does. Its `run_id` is membership; the store's own `_run_id` is + only the run that last fetched the record, so filtering `answers` by `_run_id` + undercounts a run whose answers a later run re-fetched. +- A run's learners with at least one answer come from its Student ID Mapping + report: `SELECT count(DISTINCT m.user_id) FROM student_id_mapping m JOIN + run_answers a ON a.remote_endpoint = m.run_remote_endpoint WHERE a.run_id = + 584`, where 584 is a Student ID Mapping run whose answers and report were + fetched. Scope it by the answers' `run_id` only: `student_id_mapping` keeps each + learner's row from whichever run was fetched last, so `m.run_id = 584` drops a + learner another mapping run of the class also holds. Count by `user_id`, never + by endpoint, since a student has one endpoint per assignment. A Student ID + Mapping run is computed live, so re-reading it with `--refresh` picks up + learners who joined since; a Student Answers run is fixed when its query ran, + and a whole-class one fails above three or four assignments. +- A run's log freshness is `SELECT count(*) AS logs, max(event_time) AS + log_freshness_at FROM logs WHERE run_id = `. - Reports-to-stores join: `reports.res__remote_endpoint = answers.remote_endpoint`, with `res___*` pairing to `answers.question_id`. diff --git a/internal/guidance/src/skill_header.md b/internal/guidance/src/skill_header.md index 0f974d4..cf7024d 100644 --- a/internal/guidance/src/skill_header.md +++ b/internal/guidance/src/skill_header.md @@ -51,6 +51,19 @@ guessing flags. These take `--portal` when no default portal is configured. +## Packages + +Researcher Dashboard packages are developed and published with `cc-data package`: + +- `cc-data package init [dir]` writes a `manifest.json` skeleton and a `run.py` stub. +- `cc-data package run [dir] --dataset --scope ` runs it locally the + way the dashboard's runner will, with the scope file holding the class's + `kind`, `id`, `classes` and `assignments`. +- `cc-data package build [dir]` zips it reproducibly and checks it against the + catalog's rules. +- `cc-data package publish ` registers it in the catalog with the cc-data + token. + ## Fetching data - `cc-data get report --dataset ` — the report CSV. A Portal report diff --git a/internal/packages/archive.go b/internal/packages/archive.go new file mode 100644 index 0000000..df285ee --- /dev/null +++ b/internal/packages/archive.go @@ -0,0 +1,149 @@ +package packages + +import ( + "archive/zip" + "bytes" + "crypto/sha256" + "encoding/hex" + "fmt" + "io" + "io/fs" + "os" + "path" + "path/filepath" + "runtime" + "sort" + "strings" + "time" +) + +// buildTime is every entry's modification time, so the same tree always zips to the same +// bytes and the same checksum. +var buildTime = time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC) + +// Files is a package's contents: slash-separated paths relative to its directory, sorted, +// each with the mode it ships with. +type Files struct { + Dir string + Paths []string + Modes map[string]os.FileMode +} + +// shipMode is the one mode a file keeps through build and unzip: 0755 when any execute bit is +// set, else 0644. unzip restores it on the VM, where a non-.py entrypoint is executed directly. +func shipMode(m fs.FileMode) os.FileMode { + if m.Perm()&0o111 != 0 { + return 0o755 + } + return 0o644 +} + +// Set is the paths as a lookup, the shape ReadManifest checks the entrypoint against. +func (f Files) Set() map[string]bool { + set := make(map[string]bool, len(f.Paths)) + for _, p := range f.Paths { + set[p] = true + } + return set +} + +// excluded is what a build leaves out: any dot path (.git, .cc-data-run, .cc-data-build), +// __pycache__, *.pyc and local-data. +func excluded(rel string, isDir bool) bool { + base := path.Base(rel) + if strings.HasPrefix(base, ".") || base == "__pycache__" || (isDir && base == "local-data") { + return true + } + return !isDir && strings.HasSuffix(base, ".pyc") +} + +// Collect walks dir for the files a package is made of. A link or other non-regular file that +// build would ship is refused rather than skipped, since the runner refuses an archive holding +// one; under an excluded name it is skipped like any other file. +func Collect(dir string) (Files, error) { + // The package directory itself may be a link; only what is inside it may not. + dir, err := filepath.EvalSymlinks(dir) + if err != nil { + return Files{}, err + } + files := Files{Dir: dir, Modes: map[string]os.FileMode{}} + err = filepath.WalkDir(dir, func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if p == dir { + return nil + } + rel, err := filepath.Rel(dir, p) + if err != nil { + return err + } + rel = filepath.ToSlash(rel) + if excluded(rel, d.IsDir()) { + if d.IsDir() { + return filepath.SkipDir + } + return nil + } + switch { + case d.Type()&fs.ModeSymlink != 0: + return fmt.Errorf("%s is a symbolic link; a package may hold only regular files", rel) + case d.IsDir(): + return nil + case !d.Type().IsRegular(): + return fmt.Errorf("%s is not a regular file", rel) + } + info, err := d.Info() + if err != nil { + return err + } + files.Paths = append(files.Paths, rel) + files.Modes[rel] = shipMode(info.Mode()) + return nil + }) + sort.Strings(files.Paths) + return files, err +} + +// Zip writes a reproducible archive: sorted entries, Deflate, a fixed time and each file's ship +// mode. The size limits are report-server's. +func Zip(files Files) ([]byte, error) { + var buf bytes.Buffer + w := zip.NewWriter(&buf) + for _, rel := range files.Paths { + h := &zip.FileHeader{Name: rel, Method: zip.Deflate, Modified: buildTime} + h.SetMode(files.Modes[rel]) + dst, err := w.CreateHeader(h) + if err != nil { + return nil, err + } + src, err := os.Open(filepath.Join(files.Dir, filepath.FromSlash(rel))) + if err != nil { + return nil, err + } + _, err = io.Copy(dst, src) + src.Close() + if err != nil { + return nil, err + } + } + if err := w.Close(); err != nil { + return nil, err + } + return buf.Bytes(), nil +} + +// CheckEntrypointMode refuses a non-.py entrypoint that would not be executable once unzipped, +// since the runner executes it directly. Windows records no execute bit to check. +func (f Files) CheckEntrypointMode(entrypoint string) error { + if runtime.GOOS == "windows" || strings.HasSuffix(entrypoint, ".py") || f.Modes[entrypoint]&0o111 != 0 { + return nil + } + return fmt.Errorf("the entrypoint %s is not executable; the runner executes a non-.py entrypoint directly (chmod +x it)", entrypoint) +} + +// Checksum is the catalog's spelling of an archive's digest. +func Checksum(b []byte) string { + sum := sha256.Sum256(b) + return "sha256:" + hex.EncodeToString(sum[:]) +} diff --git a/internal/packages/build.go b/internal/packages/build.go new file mode 100644 index 0000000..20d2121 --- /dev/null +++ b/internal/packages/build.go @@ -0,0 +1,113 @@ +package packages + +import ( + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/concord-consortium/cc-data-cli/internal/fsutil" +) + +// BuildDirName is where build writes by default: inside the package, and skipped by +// Collect like every dot path, so one build's zip never reaches the next. +const BuildDirName = ".cc-data-build" + +// CheckBuildOutput refuses an explicit build output that is a directory, that package run would +// empty, or that a later build would collect. +func CheckBuildOutput(dir, out string) error { + if info, err := os.Stat(out); err == nil && info.IsDir() { + return fmt.Errorf("--out %s is a directory; name the zip file to write", out) + } + absDir, err := resolvedPath(dir) + if err != nil { + return err + } + absOut, err := resolvedPath(out) + if err != nil { + return err + } + rel, err := filepath.Rel(absDir, absOut) + if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return nil + } + // Compared without case, since macOS and Windows file systems ignore it by default. + if strings.EqualFold(strings.SplitN(filepath.ToSlash(rel), "/", 2)[0], RunDirName) { + return fmt.Errorf("--out %s is inside %s, which package run empties", out, RunDirName) + } + if excluded(filepath.ToSlash(rel), false) || excludedAncestor(filepath.ToSlash(rel)) { + return nil + } + return fmt.Errorf("--out %s is inside the package, where the next build would include it", out) +} + +// resolvedPath is p made absolute with every link in its existing part resolved, so the same +// place reached through a link and by its real path compares equal. +func resolvedPath(p string) (string, error) { + abs, err := filepath.Abs(p) + if err != nil { + return "", err + } + rest := "" + for cur := abs; ; { + if real, err := filepath.EvalSymlinks(cur); err == nil { + return filepath.Join(real, rest), nil + } + parent := filepath.Dir(cur) + if parent == cur { + return abs, nil + } + rest = filepath.Join(filepath.Base(cur), rest) + cur = parent + } +} + +func excludedAncestor(rel string) bool { + parts := strings.Split(rel, "/") + for i := 1; i < len(parts); i++ { + if excluded(strings.Join(parts[:i], "/"), true) { + return true + } + } + return false +} + +// WriteBuild writes the archive, creating its directory, and ignores the default build +// directory for git. +func WriteBuild(out string, archive []byte) error { + dir := filepath.Dir(out) + if filepath.Base(dir) != BuildDirName { + if err := os.MkdirAll(dir, 0o755); err != nil { + return err + } + } else if err := os.MkdirAll(filepath.Dir(dir), 0o755); err != nil { + return err + } else if err := ensureRealDir(dir); err != nil { + return err + } else if err := ignoreAll(dir); err != nil { + return err + } + return replaceFile(out, archive) +} + +// replaceFile writes data beside path and renames it into place, so a link already at path is +// replaced rather than followed. +func replaceFile(path string, data []byte) error { + tmp, err := os.CreateTemp(filepath.Dir(path), ".build-*") + if err != nil { + return err + } + defer os.Remove(tmp.Name()) + if _, err := tmp.Write(data); err != nil { + tmp.Close() + return err + } + if err := tmp.Chmod(0o644); err != nil { + tmp.Close() + return err + } + if err := tmp.Close(); err != nil { + return err + } + return fsutil.RenameAtomic(tmp.Name(), path) +} diff --git a/internal/packages/files_test.go b/internal/packages/files_test.go new file mode 100644 index 0000000..0a58177 --- /dev/null +++ b/internal/packages/files_test.go @@ -0,0 +1,264 @@ +package packages + +import ( + "archive/zip" + "bytes" + "errors" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + "time" +) + +const goodManifest = `{ + "name": "wildfire-responses", + "title": "Wildfire responses", + "version": "0.1.0", + "urls": {"all": [], "any": [], "none": []}, + "clue_prepull": false, + "entrypoint": "run.py", + "expected_duration_seconds": 300 +}` + +// writeTree writes each path's content under dir, creating parents; a path ending in "/" is an +// empty directory. +func writeTree(t *testing.T, dir string, files map[string]string) { + t.Helper() + for rel, content := range files { + p := filepath.Join(dir, filepath.FromSlash(rel)) + if strings.HasSuffix(rel, "/") { + if err := os.MkdirAll(p, 0o755); err != nil { + t.Fatal(err) + } + continue + } + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(content), 0o644); err != nil { + t.Fatal(err) + } + } +} + +func skipOnWindows(t *testing.T, why string) { + t.Helper() + if runtime.GOOS == "windows" { + t.Skip(why) + } +} + +func TestReadManifestNeedsWhatTheRunActsOn(t *testing.T) { + files := map[string]bool{"run.py": true} + if _, err := ReadManifest([]byte(goodManifest), files); err != nil { + t.Fatalf("good manifest refused: %v", err) + } + if _, err := ReadManifest([]byte(goodManifest), map[string]bool{"other.py": true}); err == nil { + t.Error("a missing entrypoint was accepted") + } + for _, duration := range []string{`"300"`, `1.5`, `0`} { + m := strings.Replace(goodManifest, `"expected_duration_seconds": 300`, `"expected_duration_seconds": `+duration, 1) + if _, err := ReadManifest([]byte(m), files); err == nil { + t.Errorf("duration %s was accepted", duration) + } + } +} + +func TestCollectLeavesOutWhatBuildMustNotShip(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{ + "manifest.json": goodManifest, + "run.py": "print(1)", + "lib/q.sql": "SELECT 1", + "lib/y.pyc": "x", + ".git/HEAD": "ref", + ".cc-data-run/out/display.md": "x", + ".cc-data-build/p-0.1.0.zip": "x", + "__pycache__/cache.txt": "x", + "local-data/answers.csv": "x", + ".DS_Store": "x", + }) + files, err := Collect(dir) + if err != nil { + t.Fatal(err) + } + if got := strings.Join(files.Paths, ","); got != "lib/q.sql,manifest.json,run.py" { + t.Errorf("collected %s", got) + } +} + +func TestCollectRefusesALink(t *testing.T) { + skipOnWindows(t, "links need privileges on Windows") + dir := t.TempDir() + writeTree(t, dir, map[string]string{"manifest.json": goodManifest, "run.py": "print(1)"}) + if err := os.Symlink(filepath.Join(dir, "run.py"), filepath.Join(dir, "alias.py")); err != nil { + t.Fatal(err) + } + if _, err := Collect(dir); err == nil || !strings.Contains(err.Error(), "symbolic link") { + t.Errorf("err = %v, want a symbolic-link refusal", err) + } +} + +func TestZipIsReproducible(t *testing.T) { + dir := t.TempDir() + writeTree(t, dir, map[string]string{"manifest.json": goodManifest, "run.py": "print(1)"}) + build := func() string { + files, err := Collect(dir) + if err != nil { + t.Fatal(err) + } + b, err := Zip(files) + if err != nil { + t.Fatal(err) + } + return Checksum(b) + } + first := build() + later := time.Now().Add(time.Hour) + if err := os.Chtimes(filepath.Join(dir, "run.py"), later, later); err != nil { + t.Fatal(err) + } + if second := build(); second != first { + t.Errorf("checksum changed with an mtime: %s then %s", first, second) + } +} + +func TestModesSurviveTheZip(t *testing.T) { + skipOnWindows(t, "Windows records no execute bit") + dir := t.TempDir() + writeTree(t, dir, map[string]string{"manifest.json": goodManifest, "run.py": "print(1)", "run.sh": "#!/bin/sh\n"}) + if err := os.Chmod(filepath.Join(dir, "run.py"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Chmod(filepath.Join(dir, "manifest.json"), 0o600); err != nil { + t.Fatal(err) + } + files, err := Collect(dir) + if err != nil { + t.Fatal(err) + } + b, err := Zip(files) + if err != nil { + t.Fatal(err) + } + r, err := zip.NewReader(bytes.NewReader(b), int64(len(b))) + if err != nil { + t.Fatal(err) + } + modes := map[string]os.FileMode{} + for _, f := range r.File { + modes[f.Name] = f.Mode().Perm() + } + if modes["run.py"] != 0o755 || modes["manifest.json"] != 0o644 { + t.Errorf("zipped modes = %v, want run.py 0755 and manifest.json 0644", modes) + } + if err := files.CheckEntrypointMode("run.sh"); err == nil { + t.Error("a 0644 run.sh entrypoint passed") + } + if err := os.Chmod(filepath.Join(dir, "run.py"), 0o644); err != nil { + t.Fatal(err) + } + files, err = Collect(dir) + if err != nil { + t.Fatal(err) + } + if err := files.CheckEntrypointMode("run.py"); err != nil { + t.Errorf("a 0644 run.py was refused: %v", err) + } +} + +func TestDisplayCapEdges(t *testing.T) { + if DisplayLimitBytes != 65536 { + t.Fatalf("DisplayLimitBytes = %d, want 65536", DisplayLimitBytes) + } + out := t.TempDir() + write := func(n int) { + if err := os.WriteFile(filepath.Join(out, "display.md"), bytes.Repeat([]byte("x"), n), 0o644); err != nil { + t.Fatal(err) + } + } + write(DisplayLimitBytes) + if res, err := ReadResult(out); err != nil || res.DisplayBytes != DisplayLimitBytes { + t.Errorf("at the cap: %+v, %v", res, err) + } + write(DisplayLimitBytes + 1) + var refused *OutputRefused + if _, err := ReadResult(out); !errors.As(err, &refused) { + t.Errorf("one byte over the cap: err = %v, want OutputRefused", err) + } +} + +func TestResultReadsOnlyTheRunnersKeys(t *testing.T) { + out := t.TempDir() + writeTree(t, out, map[string]string{ + "display.md": "# Class\n", + "summary.txt": " 3 answers from 2 learners \r\nsecond line\n", + "counts.json": `{"answers": 3, "logs": 7, "log_freshness_at": "2026-10-08T00:00:00Z", "learners": 2}`, + }) + res, err := ReadResult(out) + if err != nil { + t.Fatal(err) + } + if res.Summary == nil || *res.Summary != "3 answers from 2 learners" { + t.Errorf("summary = %v", res.Summary) + } + if len(res.Counts) != 3 || res.Counts["answers"] != float64(3) || res.Counts["learners"] != nil { + t.Errorf("counts = %v", res.Counts) + } +} + +func TestResultRefusesALinkedDisplay(t *testing.T) { + skipOnWindows(t, "links need privileges on Windows") + out := t.TempDir() + target := filepath.Join(t.TempDir(), "elsewhere.md") + if err := os.WriteFile(target, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, filepath.Join(out, "display.md")); err != nil { + t.Fatal(err) + } + var refused *OutputRefused + if _, err := ReadResult(out); !errors.As(err, &refused) { + t.Errorf("err = %v, want OutputRefused", err) + } +} + +func TestCollectFollowsALinkedPackageDirectory(t *testing.T) { + skipOnWindows(t, "links need privileges on Windows") + real := t.TempDir() + writeTree(t, real, map[string]string{"manifest.json": goodManifest, "run.py": "print(1)"}) + link := filepath.Join(t.TempDir(), "pkg") + if err := os.Symlink(real, link); err != nil { + t.Fatal(err) + } + files, err := Collect(link) + if err != nil { + t.Fatal(err) + } + if got := strings.Join(files.Paths, ","); got != "manifest.json,run.py" { + t.Errorf("collected %q through a linked package directory", got) + } +} + +func TestCheckBuildOutputSeesThroughLinks(t *testing.T) { + skipOnWindows(t, "links need privileges on Windows") + real := t.TempDir() + link := filepath.Join(t.TempDir(), "pkg") + if err := os.Symlink(real, link); err != nil { + t.Fatal(err) + } + for _, tc := range []struct{ dir, out string }{ + {link, filepath.Join(real, RunDirName, "x.zip")}, + {real, filepath.Join(link, RunDirName, "x.zip")}, + {link, filepath.Join(real, "p.zip")}, + } { + if err := CheckBuildOutput(tc.dir, tc.out); err == nil { + t.Errorf("--out %s for package %s was accepted", tc.out, tc.dir) + } + } + if err := CheckBuildOutput(link, filepath.Join(t.TempDir(), "p.zip")); err != nil { + t.Errorf("an --out outside the package was refused: %v", err) + } +} diff --git a/internal/packages/init.go b/internal/packages/init.go new file mode 100644 index 0000000..1d2730a --- /dev/null +++ b/internal/packages/init.go @@ -0,0 +1,59 @@ +package packages + +import ( + _ "embed" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" +) + +//go:embed template/run.py +var runStub []byte + +// Init writes manifest.json and run.py into dir, creating it if needed. It reads nothing in +// dir except to refuse when either file already exists, in which case it writes neither. +func Init(dir, name string) ([]string, error) { + if name == "" { + abs, err := filepath.Abs(dir) + if err != nil { + return nil, err + } + name = filepath.Base(abs) + if !ValidName(name) { + return nil, fmt.Errorf("the directory name %q is not a package name; pass --name (^[a-z0-9][a-z0-9-]{0,62}$)", name) + } + } else if !ValidName(name) { + return nil, fmt.Errorf("--name %q must match ^[a-z0-9][a-z0-9-]{0,62}$", name) + } + for _, f := range []string{"manifest.json", "run.py"} { + _, err := os.Lstat(filepath.Join(dir, f)) + switch { + case err == nil: + return nil, fmt.Errorf("%s already exists in %s; init writes only into a package that has neither file", f, dir) + case !errors.Is(err, os.ErrNotExist): + return nil, err + } + } + manifest := Manifest{ + Name: name, Title: name, Version: "0.1.0", + Description: "One line saying what this package shows.", + URLs: URLs{All: []string{}, Any: []string{}, None: []string{}}, + Entrypoint: "run.py", ExpectedDurationSeconds: 300, + } + b, err := json.MarshalIndent(manifest, "", " ") + if err != nil { + return nil, err + } + if err := os.MkdirAll(dir, 0o755); err != nil { + return nil, err + } + if err := os.WriteFile(filepath.Join(dir, "manifest.json"), append(b, '\n'), 0o644); err != nil { + return nil, err + } + if err := os.WriteFile(filepath.Join(dir, "run.py"), runStub, 0o755); err != nil { + return nil, err + } + return []string{"manifest.json", "run.py"}, nil +} diff --git a/internal/packages/manifest.go b/internal/packages/manifest.go new file mode 100644 index 0000000..6309f2d --- /dev/null +++ b/internal/packages/manifest.go @@ -0,0 +1,77 @@ +// Package packages is the local side of a Researcher Dashboard package: reading the manifest +// fields the runner acts on, collecting and zipping the package, staging and running it the +// way the runner does, and reading the result files. The package rules themselves, and +// applicability, are report-server's, which cc-data asks rather than copies. +package packages + +import ( + "encoding/json" + "fmt" + "regexp" + "strings" + "unicode" +) + +var nameRe = regexp.MustCompile(`^[a-z0-9][a-z0-9-]{0,62}$`) + +// ValidName reports whether name fits the catalog's name grammar, which init needs before +// report-server has seen anything. +func ValidName(name string) bool { return nameRe.MatchString(name) } + +// URLs is a manifest's applicability patterns, passed to report-server as written. +type URLs struct { + All []string `json:"all"` + Any []string `json:"any"` + None []string `json:"none"` +} + +// DeclaresPatterns reports whether any sense holds a pattern; a package that declares none +// applies to every scope. +func (u URLs) DeclaresPatterns() bool { + return len(u.All)+len(u.Any)+len(u.None) > 0 +} + +// Manifest is the part of manifest.json the runner acts on. Everything else, and every limit, +// is report-server's to check (cmd's validate call). +type Manifest struct { + Name string `json:"name"` + Title string `json:"title"` + Version string `json:"version"` + Description string `json:"description,omitempty"` + URLs URLs `json:"urls"` + CluePrepull bool `json:"clue_prepull"` + Entrypoint string `json:"entrypoint"` + ExpectedDurationSeconds int `json:"expected_duration_seconds"` +} + +// ReadManifest reads the fields a run acts on, refusing an entrypoint outside files and a +// duration that is not a positive integer; every other rule is report-server's. +func ReadManifest(data []byte, files map[string]bool) (Manifest, error) { + var m Manifest + if err := json.Unmarshal(data, &m); err != nil { + return m, fmt.Errorf("manifest.json: %v", err) + } + if !files[m.Entrypoint] || unsafePath(m.Entrypoint) { + return m, fmt.Errorf("manifest.json: entrypoint %q is not a file in the package", m.Entrypoint) + } + if m.ExpectedDurationSeconds < 1 { + return m, fmt.Errorf("manifest.json: expected_duration_seconds must be a positive integer") + } + return m, nil +} + +// unsafePath is report-server's Archive.unsafe_path?: absolute, a drive letter, or a ".." segment. +func unsafePath(name string) bool { + if strings.HasPrefix(name, "/") || strings.HasPrefix(name, `\`) { + return true + } + if len(name) >= 2 && name[1] == ':' && unicode.IsLetter(rune(name[0])) { + return true + } + for _, seg := range strings.FieldsFunc(name, func(r rune) bool { return r == '/' || r == '\\' }) { + if seg == ".." { + return true + } + } + return false +} diff --git a/internal/packages/proc_unix.go b/internal/packages/proc_unix.go new file mode 100644 index 0000000..0375c19 --- /dev/null +++ b/internal/packages/proc_unix.go @@ -0,0 +1,38 @@ +//go:build !windows + +package packages + +import ( + "os" + "os/exec" + "path/filepath" + "syscall" +) + +// killGroup runs the package in its own process group and kills the whole group when the run +// is canceled or times out, so a child the entrypoint started does not outlive it. +func killGroup(cmd *exec.Cmd) { + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + cmd.Cancel = func() error { return syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) } +} + +// reapGroup kills whatever is left of the package's process group once the entrypoint has +// exited. The group outlives its leader while any member does, so the kill still finds it. +func reapGroup(cmd *exec.Cmd) { + if cmd.Process != nil { + syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) + } +} + +// exposeCCData links bin/cc-data to the running binary, whatever its name, and returns bin to put +// first on the package's PATH. Unlike the binary's own folder, which may be a shared bin, it puts +// nothing else ahead of the user's PATH. +func exposeCCData(bin, exe string, _ func(string) (string, error)) (string, string, error) { + if exe == "" { + return "", "", nil + } + if err := os.Symlink(exe, filepath.Join(bin, "cc-data")); err != nil { + return "", "", err + } + return bin, "", nil +} diff --git a/internal/packages/proc_windows.go b/internal/packages/proc_windows.go new file mode 100644 index 0000000..a6ff887 --- /dev/null +++ b/internal/packages/proc_windows.go @@ -0,0 +1,30 @@ +//go:build windows + +package packages + +import ( + "fmt" + "os/exec" + "path/filepath" + "strings" +) + +// On Windows, which has no release, exec.CommandContext kills only the entrypoint, and nothing +// is killed after it exits. +func killGroup(cmd *exec.Cmd) {} + +func reapGroup(cmd *exec.Cmd) {} + +// exposeCCData puts the running binary's own folder first on the package's PATH when it is named +// cc-data, since creating a link needs a privilege on Windows. Under any other name the package's +// cc-data calls reach whatever is on PATH, and the warning names it. +func exposeCCData(_, exe string, lookPath func(string) (string, error)) (string, string, error) { + if exe != "" && strings.EqualFold(strings.TrimSuffix(filepath.Base(exe), ".exe"), "cc-data") { + return filepath.Dir(exe), "", nil + } + found, err := lookPath("cc-data") + if err != nil { + return "", "this binary is not named cc-data and no cc-data is on PATH, so the package's own cc-data calls will fail", nil + } + return "", fmt.Sprintf("this binary is not named cc-data, so the package's own cc-data calls reach %s", found), nil +} diff --git a/internal/packages/proc_windows_test.go b/internal/packages/proc_windows_test.go new file mode 100644 index 0000000..2172234 --- /dev/null +++ b/internal/packages/proc_windows_test.go @@ -0,0 +1,25 @@ +//go:build windows + +package packages + +import ( + "os/exec" + "path/filepath" + "strings" + "testing" +) + +func TestExposeCCDataOnWindowsUsesTheBinarysFolderByName(t *testing.T) { + other := func(string) (string, error) { return `C:\tools\cc-data.exe`, nil } + none := func(string) (string, error) { return "", exec.ErrNotFound } + build := filepath.Join(`C:\`, "build") + if dir, warning, err := exposeCCData("", filepath.Join(build, "cc-data.exe"), other); err != nil || dir != build || warning != "" { + t.Errorf("cc-data.exe: %q, %q, %v", dir, warning, err) + } + if dir, warning, _ := exposeCCData("", filepath.Join(build, "main.exe"), other); dir != "" || !strings.Contains(warning, `C:\tools\cc-data.exe`) { + t.Errorf("go run: %q, %q", dir, warning) + } + if _, warning, _ := exposeCCData("", filepath.Join(build, "main.exe"), none); !strings.Contains(warning, "will fail") { + t.Errorf("no cc-data on PATH: %q", warning) + } +} diff --git a/internal/packages/result.go b/internal/packages/result.go new file mode 100644 index 0000000..decd6a8 --- /dev/null +++ b/internal/packages/result.go @@ -0,0 +1,114 @@ +package packages + +import ( + "encoding/json" + "errors" + "fmt" + "io" + "io/fs" + "os" + "path/filepath" + "strings" +) + +// DisplayLimitBytes is the runner's cap on display.md in UTF-8 bytes, decided only in section 10 +// of the Researcher Dashboard design (final-design.md, kept outside this repository). +const DisplayLimitBytes = 64 * 1024 + +// countKeys are the counts.json keys the runner reads; anything else a package writes there +// is ignored, as it is on the VM. +var countKeys = []string{"answers", "logs", "log_freshness_at"} + +// Result is what the runner keeps of a run: the display, the summary line and the counts. +type Result struct { + DisplayPath string `json:"display"` + DisplayBytes int64 `json:"display_bytes"` + Summary *string `json:"summary"` + Counts map[string]any `json:"counts"` +} + +// OutputRefused is a result the runner would refuse: a missing, linked or oversized +// display.md, a linked summary.txt, or a linked output directory. +type OutputRefused struct{ Reason string } + +func (e *OutputRefused) Error() string { return e.Reason } + +// ReadResult reads the output directory as the runner does, following no link: the +// directory must be itself, and each file must be a regular file. +func ReadResult(outDir string) (Result, error) { + var res Result + if err := RealDir(outDir); err != nil { + return res, &OutputRefused{Reason: "the package replaced its output directory with a link"} + } + _, size, err := readOwnFile(outDir, "display.md", DisplayLimitBytes) + switch { + case errors.Is(err, fs.ErrNotExist): + return res, &OutputRefused{Reason: "the package wrote no display.md"} + case err != nil: + return res, err + case size > DisplayLimitBytes: + return res, &OutputRefused{Reason: fmt.Sprintf("the package's display.md is %d bytes, over the %d-byte limit", size, DisplayLimitBytes)} + } + res.DisplayPath = filepath.Join(outDir, "display.md") + res.DisplayBytes = size + // The runner reads summary.txt and counts.json under the display cap too, truncating them. + if text, _, err := readOwnFile(outDir, "summary.txt", DisplayLimitBytes); err == nil { + line := strings.TrimSpace(strings.SplitN(strings.ReplaceAll(text, "\r\n", "\n"), "\n", 2)[0]) + if line != "" { + res.Summary = &line + } + } else if !errors.Is(err, fs.ErrNotExist) { + return res, err + } + res.Counts = map[string]any{} + if text, _, err := readOwnFile(outDir, "counts.json", DisplayLimitBytes); err == nil { + var parsed map[string]any + if json.Unmarshal([]byte(text), &parsed) == nil { + for _, k := range countKeys { + if v, ok := parsed[k]; ok { + res.Counts[k] = v + } + } + } + } + return res, nil +} + +// readOwnFile returns up to limit+1 bytes of a regular, unlinked file and its full size. The +// Lstat-then-SameFile pair stands in for O_NOFOLLOW, which Windows lacks. +func readOwnFile(dir, name string, limit int64) (string, int64, error) { + p := filepath.Join(dir, name) + before, err := os.Lstat(p) + if err != nil { + return "", 0, err + } + if before.Mode()&fs.ModeSymlink != 0 { + return "", 0, &OutputRefused{Reason: fmt.Sprintf("the package's %s is a link, not a file", name)} + } + if !before.Mode().IsRegular() { + return "", 0, &OutputRefused{Reason: fmt.Sprintf("the package's %s is not a file", name)} + } + f, err := os.Open(p) + if err != nil { + return "", 0, err + } + defer f.Close() + after, err := f.Stat() + if err != nil || !os.SameFile(before, after) { + return "", 0, &OutputRefused{Reason: fmt.Sprintf("the package's %s changed while it was read", name)} + } + b, err := io.ReadAll(io.LimitReader(f, limit+1)) + return string(b), after.Size(), err +} + +// RealDir reports an error unless p is a directory reached without following a link. +func RealDir(p string) error { + info, err := os.Lstat(p) + if err != nil { + return err + } + if info.Mode()&fs.ModeSymlink != 0 || !info.IsDir() { + return fmt.Errorf("%s is a link or not a directory", p) + } + return nil +} diff --git a/internal/packages/run.go b/internal/packages/run.go new file mode 100644 index 0000000..9d04a20 --- /dev/null +++ b/internal/packages/run.go @@ -0,0 +1,304 @@ +package packages + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "os" + "os/exec" + "path/filepath" + "slices" + "sort" + "strings" + "time" + + "github.com/concord-consortium/cc-data-cli/internal/fsutil" +) + +// TimeoutMargin is what the runner adds to expected_duration_seconds before it kills a run. +const TimeoutMargin = 10 * time.Minute + +// RunDirName is the working tree package run keeps inside the package directory. It starts +// with "." so Collect, and so build, never ships it. +const RunDirName = ".cc-data-run" + +// passthrough is what a laptop adds so the package's own cc-data calls find the stored +// credential, reach the server through any proxy, and run at all. These names match in any +// case, so the lower-case proxy spellings pass; the LC_ prefix matches as written. +var passthrough = []string{ + "HOME", "PATH", "USER", "LOGNAME", "LANG", "TZ", "TMPDIR", + "DBUS_SESSION_BUS_ADDRESS", "XDG_RUNTIME_DIR", + "HTTPS_PROXY", "HTTP_PROXY", "NO_PROXY", + // Windows has no release, but CI builds and tests there. + "SYSTEMROOT", "USERPROFILE", "APPDATA", "LOCALAPPDATA", "PATHEXT", "COMSPEC", +} + +// Verdict is report-server's answer to whether a package's patterns match a scope. Unconfirmed +// means the report-server answering has no applicability route yet, so nothing was checked. +type Verdict struct { + Applies bool + Reason string + Unconfirmed bool +} + +// Refused is a package the runner would not start: it does not apply, or it needs the CLUE +// pre-pull, which cc-data cannot do. +type Refused struct{ Reason string } + +func (e *Refused) Error() string { return e.Reason } + +// Failed is a package that started and did not finish cleanly. +type Failed struct{ Reason string } + +func (e *Failed) Error() string { return e.Reason } + +// RunOptions is everything package run decides before the entrypoint starts. +type RunOptions struct { + Dir string // the package directory + Manifest Manifest // validated against Files + Files Files // what build would ship + Scope LocalScope // the author's scope file + Applies func(context.Context, URLs) (Verdict, error) // report-server's applicability answer + Dataset string // the full ref, / + Name string // the ref's bare name + Portal string // the ref's portal host + DataRoot string // CC_DATA_ROOT, absolute + ReportSrv string // the stored credential's server, or "" + CCData string // the running cc-data, which the package's own cc-data calls reach + Stderr io.Writer // where the package's own output goes + Environ func() []string + LookPath func(string) (string, error) + Timeout time.Duration // zero means expected_duration_seconds + TimeoutMargin +} + +// Run prepares the runner's layout, runs the entrypoint and reads its result. +func Run(ctx context.Context, o RunOptions) (Result, error) { + if o.Manifest.CluePrepull { + return Result{}, &Refused{Reason: "the package sets clue_prepull, and cc-data cannot fetch CLUE data yet, so it runs only on the VM"} + } + if o.Manifest.URLs.DeclaresPatterns() { + v, err := o.Applies(ctx, o.Manifest.URLs) + switch { + case err != nil: + return Result{}, err + case v.Unconfirmed: + fmt.Fprintln(o.Stderr, "warning: report-server cannot check applicability yet; the package runs, and the VM's runner is the check") + case !v.Applies: + return Result{}, &Refused{Reason: "the package does not apply to this class: " + v.Reason} + } + } + run := filepath.Join(o.Dir, RunDirName) + paths, err := prepare(run, o.Files) + if err != nil { + return Result{}, err + } + binDir, warning, err := exposeCCData(paths.bin, o.CCData, o.LookPath) + if err != nil { + return Result{}, err + } + if warning != "" { + fmt.Fprintln(o.Stderr, "warning: "+warning) + } + scope := ScopeFile{LocalScope: o.Scope, ClueSource: "firebase", Dataset: o.Name, OutputDir: paths.out} + b, err := json.MarshalIndent(scope, "", " ") + if err != nil { + return Result{}, err + } + scopePath := filepath.Join(paths.in, "scope.json") + if err := fsutil.WriteFileAtomic0600(scopePath, b); err != nil { + return Result{}, err + } + + env := map[string]string{ + "CC_DATA_ROOT": o.DataRoot, + "CC_DATA_PORTAL": o.Portal, + "CC_DATA_LOCAL": paths.data, + "RD_DATASET": o.Dataset, + "RD_SCOPE_FILE": scopePath, + "RD_DATA_DIR": paths.data, + "RD_OUTPUT_DIR": paths.out, + } + if o.ReportSrv != "" { + env["RD_REPORT_SERVER_URL"] = o.ReportSrv + } + command, args, note, err := entrypointCommand(paths.pkg, o.Manifest.Entrypoint, o.LookPath) + if err != nil { + return Result{}, err + } + if note != "" { + fmt.Fprintln(o.Stderr, note) + } + + timeout := o.Timeout + if timeout == 0 { + timeout = time.Duration(o.Manifest.ExpectedDurationSeconds)*time.Second + TimeoutMargin + } + runCtx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + cmd := exec.CommandContext(runCtx, command, args...) + cmd.Dir = paths.pkg + cmd.Env = environment(o.Environ(), env, binDir) + cmd.Stdout = o.Stderr + cmd.Stderr = o.Stderr + killGroup(cmd) + err = cmd.Run() + // On Unix, whatever is left in the package's process group dies before its output is read. + reapGroup(cmd) + if err != nil { + switch { + case errors.Is(runCtx.Err(), context.DeadlineExceeded): + return Result{}, &Failed{Reason: fmt.Sprintf("the package ran past its %ds bound", int(timeout.Seconds()))} + case ctx.Err() != nil: + return Result{}, &Failed{Reason: "the package was interrupted"} + } + return Result{}, &Failed{Reason: fmt.Sprintf("the package exited unsuccessfully: %v", err)} + } + return ReadResult(paths.out) +} + +type runPaths struct{ pkg, in, out, bin, data string } + +// prepare empties and rebuilds the run tree, keeping only data/ from one run to the next. Every +// directory it removes or creates under the package must be a real directory, so a link left +// there can never aim the removal outside. +func prepare(run string, files Files) (runPaths, error) { + p := runPaths{ + pkg: filepath.Join(run, "pkg"), + in: filepath.Join(run, "in"), + out: filepath.Join(run, "out"), + bin: filepath.Join(run, "bin"), + data: filepath.Join(run, "data"), + } + if err := ensureRealDir(run); err != nil { + return p, err + } + if err := ignoreAll(run); err != nil { + return p, err + } + for _, d := range []string{p.pkg, p.in, p.out, p.bin} { + if _, err := os.Lstat(d); err == nil { + if err := RealDir(d); err != nil { + return p, fmt.Errorf("refusing to empty %s: it is a link or not a directory", d) + } + if err := os.RemoveAll(d); err != nil { + return p, err + } + } + if err := os.Mkdir(d, 0o700); err != nil { + return p, err + } + } + if err := ensureRealDir(p.data); err != nil { + return p, err + } + abs, err := filepath.Abs(run) + if err != nil { + return p, err + } + p = runPaths{filepath.Join(abs, "pkg"), filepath.Join(abs, "in"), filepath.Join(abs, "out"), filepath.Join(abs, "bin"), filepath.Join(abs, "data")} + for _, rel := range files.Paths { + if err := copyFile(filepath.Join(files.Dir, filepath.FromSlash(rel)), filepath.Join(p.pkg, filepath.FromSlash(rel)), files.Modes[rel]); err != nil { + return p, err + } + } + return p, nil +} + +// ignoreAll keeps git out of a directory this tool makes inside a package, whose files can +// hold student data. +func ignoreAll(dir string) error { + p := filepath.Join(dir, ".gitignore") + if info, err := os.Lstat(p); err == nil && !info.Mode().IsRegular() { + return fmt.Errorf("refusing to write %s: it is a link or not a file", p) + } + return fsutil.WriteFileAtomic0600(p, []byte("*\n")) +} + +// ensureRealDir creates d, or checks that it is a real directory, and keeps it private: what +// this tool writes inside a package can hold student data. +func ensureRealDir(d string) error { + if _, err := os.Lstat(d); errors.Is(err, os.ErrNotExist) { + return os.Mkdir(d, 0o700) + } + if err := RealDir(d); err != nil { + return fmt.Errorf("refusing to use %s: it is a link or not a directory", d) + } + return os.Chmod(d, 0o700) +} + +// copyFile stages one file with the mode build gives it, so the staged package is the one +// unzip produces on the VM. +func copyFile(src, dst string, mode os.FileMode) error { + if err := os.MkdirAll(filepath.Dir(dst), 0o700); err != nil { + return err + } + in, err := os.Open(src) + if err != nil { + return err + } + defer in.Close() + out, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_EXCL, mode) + if err != nil { + return err + } + if _, err := io.Copy(out, in); err != nil { + out.Close() + return err + } + return out.Close() +} + +// entrypointCommand is the runner's choice of interpreter: python3.11 for a .py entrypoint, +// otherwise the entrypoint itself. A laptop without python3.11 falls back to python3, saying so. +func entrypointCommand(pkgDir, entrypoint string, lookPath func(string) (string, error)) (string, []string, string, error) { + path := filepath.Join(pkgDir, filepath.FromSlash(entrypoint)) + if !strings.HasSuffix(entrypoint, ".py") { + return path, nil, "", nil + } + if p, err := lookPath("python3.11"); err == nil { + return p, []string{path}, "", nil + } + if p, err := lookPath("python3"); err == nil { + return p, []string{path}, "note: python3.11, the VM's interpreter, is not on PATH; running with python3", nil + } + return "", nil, "", fmt.Errorf("neither python3.11 nor python3 is on PATH") +} + +// environment is the passthrough taken from the caller's environment (LC_* included), then +// the runner's variables over it; nothing else is inherited. +func environment(from []string, set map[string]string, binDir string) []string { + keep := map[string]string{} + for _, kv := range from { + k, v, ok := strings.Cut(kv, "=") + if !ok { + continue + } + if strings.HasPrefix(k, "LC_") || slices.Contains(passthrough, strings.ToUpper(k)) { + keep[k] = v + } + } + for k, v := range set { + keep[k] = v + } + if binDir != "" { + pathKey := "PATH" + for k := range keep { + if strings.EqualFold(k, "PATH") { + pathKey = k + } + } + if keep[pathKey] == "" { + keep[pathKey] = binDir + } else { + keep[pathKey] = binDir + string(os.PathListSeparator) + keep[pathKey] + } + } + out := make([]string, 0, len(keep)) + for k, v := range keep { + out = append(out, k+"="+v) + } + sort.Strings(out) + return out +} diff --git a/internal/packages/run_test.go b/internal/packages/run_test.go new file mode 100644 index 0000000..5da878f --- /dev/null +++ b/internal/packages/run_test.go @@ -0,0 +1,389 @@ +package packages + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" +) + +const scopeJSON = `{ + "kind": "class", + "id": "0123456789abcdef0123456789abcdef0123456789abcdef", + "classes": [{"class_hash": "0123456789abcdef0123456789abcdef0123456789abcdef", "class_id": 6}], + "assignments": [{"offering_id": 9, "runnable_id": 1234, "name": "Moth 1.2", "url": "https://activity-player.concord.org/?activity=1"}] +}` + +// shellPackage writes a package whose entrypoint is run.sh with the given body. +func shellPackage(t *testing.T, body string, files map[string]string) (string, Files) { + t.Helper() + dir := t.TempDir() + tree := map[string]string{"manifest.json": goodManifest, "run.sh": "#!/bin/sh\n" + body} + for k, v := range files { + tree[k] = v + } + writeTree(t, dir, tree) + if err := os.Chmod(filepath.Join(dir, "run.sh"), 0o755); err != nil { + t.Fatal(err) + } + collected, err := Collect(dir) + if err != nil { + t.Fatal(err) + } + return dir, collected +} + +// runOpts is a run of the shell package against a fixed ref, with the given environment and an +// Applies that must not be called unless the test sets one. +func runOpts(t *testing.T, dir string, files Files, environ []string, stderr *bytes.Buffer) RunOptions { + t.Helper() + scope, err := ParseLocalScope([]byte(scopeJSON)) + if err != nil { + t.Fatal(err) + } + return RunOptions{ + Dir: dir, + Manifest: Manifest{Entrypoint: "run.sh", ExpectedDurationSeconds: 60}, + Files: files, + Scope: scope, + Applies: func(context.Context, URLs) (Verdict, error) { + t.Error("Applies called for a package that declares no patterns") + return Verdict{Applies: true}, nil + }, + Dataset: "learn.concord.org/wildfire", + Name: "wildfire", + Portal: "learn.concord.org", + DataRoot: "/data/root", + ReportSrv: "https://report.example", + Stderr: stderr, + Environ: func() []string { return environ }, + LookPath: exec.LookPath, + } +} + +func readEnv(t *testing.T, path string) map[string]string { + t.Helper() + raw, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + env := map[string]string{} + for _, line := range strings.Split(string(raw), "\n") { + if k, v, ok := strings.Cut(line, "="); ok { + env[k] = v + } + } + return env +} + +func TestLocalScopeRefusesTheRunnersKeys(t *testing.T) { + withDataset := strings.Replace(scopeJSON, `"kind": "class",`, `"kind": "class", "dataset": "pkg-1",`, 1) + if _, err := ParseLocalScope([]byte(withDataset)); err == nil { + t.Error("a scope file setting dataset was accepted") + } + if _, err := ParseLocalScope([]byte(scopeJSON + ` {"dataset": "other"}`)); err == nil { + t.Error("a scope file with a second JSON value after it was accepted") + } + if _, err := ParseLocalScope([]byte(scopeJSON + "\n")); err != nil { + t.Errorf("a good scope file was refused: %v", err) + } +} + +func TestRunGivesThePackageTheRunnersLayout(t *testing.T) { + skipOnWindows(t, "the entrypoint is a shell script") + dir, files := shellPackage(t, `env > "$RD_OUTPUT_DIR/../env.txt" +cp "$RD_SCOPE_FILE" "$RD_OUTPUT_DIR/../scope.json" +ls -A > "$RD_OUTPUT_DIR/../ls.txt" +ls -l run.sh > "$RD_OUTPUT_DIR/../mode.txt" +echo ok > "$RD_OUTPUT_DIR/display.md" +`, map[string]string{"local-data/x.csv": "secret"}) + var stderr bytes.Buffer + environ := []string{"PATH=" + os.Getenv("PATH"), "HOME=/home/r", "LC_ALL=C", "https_proxy=http://proxy:3128", "AWS_SECRET_ACCESS_KEY=leak"} + o := runOpts(t, dir, files, environ, &stderr) + o.CCData = "/opt/shared/bin/cc-data" + res, err := Run(context.Background(), o) + if err != nil { + t.Fatalf("%v\n%s", err, stderr.String()) + } + run := filepath.Join(dir, RunDirName) + if res.DisplayPath != filepath.Join(run, "out", "display.md") { + t.Errorf("display path %s", res.DisplayPath) + } + + env := readEnv(t, filepath.Join(run, "env.txt")) + for k, want := range map[string]string{ + "RD_DATASET": "learn.concord.org/wildfire", + "CC_DATA_PORTAL": "learn.concord.org", + "CC_DATA_ROOT": "/data/root", + "CC_DATA_LOCAL": filepath.Join(run, "data"), + "RD_DATA_DIR": filepath.Join(run, "data"), + "RD_SCOPE_FILE": filepath.Join(run, "in", "scope.json"), + "RD_OUTPUT_DIR": filepath.Join(run, "out"), + "RD_REPORT_SERVER_URL": "https://report.example", + "LC_ALL": "C", + "HOME": "/home/r", + "PATH": filepath.Join(run, "bin") + string(os.PathListSeparator) + os.Getenv("PATH"), + "https_proxy": "http://proxy:3128", + } { + if env[k] != want { + t.Errorf("%s = %q, want %q", k, env[k], want) + } + } + if target, err := os.Readlink(filepath.Join(run, "bin", "cc-data")); err != nil || target != "/opt/shared/bin/cc-data" { + t.Errorf("bin/cc-data links to %q (%v), want the running binary", target, err) + } + if entries, _ := os.ReadDir(filepath.Join(run, "bin")); len(entries) != 1 { + t.Errorf("bin holds %v, want only the cc-data link", entries) + } + if _, ok := env["AWS_SECRET_ACCESS_KEY"]; ok { + t.Error("a variable outside the passthrough reached the package") + } + + var scope map[string]any + raw, err := os.ReadFile(filepath.Join(run, "scope.json")) + if err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(raw, &scope); err != nil { + t.Fatal(err) + } + if scope["dataset"] != "wildfire" || scope["clue_source"] != "firebase" || scope["output_dir"] != filepath.Join(run, "out") { + t.Errorf("scope.json = %v", scope) + } + + ls, err := os.ReadFile(filepath.Join(run, "ls.txt")) + if err != nil { + t.Fatal(err) + } + if got := strings.Fields(string(ls)); strings.Join(got, ",") != "manifest.json,run.sh" { + t.Errorf("working directory holds %v, want only what build ships", got) + } + mode, err := os.ReadFile(filepath.Join(run, "mode.txt")) + if err != nil { + t.Fatal(err) + } + if !strings.HasPrefix(string(mode), "-rwxr-xr-x") { + t.Errorf("staged run.sh mode: %s", mode) + } + if gi, err := os.ReadFile(filepath.Join(run, ".gitignore")); err != nil || string(gi) != "*\n" { + t.Errorf(".gitignore = %q, %v", gi, err) + } +} + +func TestRunRefusesBeforeStarting(t *testing.T) { + skipOnWindows(t, "the entrypoint is a shell script") + dir, files := shellPackage(t, `touch "$RD_OUTPUT_DIR/../started"`+"\n", nil) + var stderr bytes.Buffer + o := runOpts(t, dir, files, nil, &stderr) + o.Manifest.URLs = URLs{All: []string{"*//wildfire.concord.org/*"}} + o.Applies = func(context.Context, URLs) (Verdict, error) { + return Verdict{Reason: "no URL in this class matches the required pattern *//wildfire.concord.org/*"}, nil + } + _, err := Run(context.Background(), o) + var refused *Refused + if !errors.As(err, &refused) || err.Error() != "the package does not apply to this class: no URL in this class matches the required pattern *//wildfire.concord.org/*" { + t.Errorf("err = %v", err) + } + if _, err := os.Stat(filepath.Join(dir, RunDirName, "started")); err == nil { + t.Error("the entrypoint started for a package that does not apply") + } + + o = runOpts(t, dir, files, nil, &stderr) + o.Manifest.CluePrepull = true + if _, err := Run(context.Background(), o); !errors.As(err, &refused) { + t.Errorf("clue_prepull: err = %v, want Refused", err) + } + if _, err := os.Stat(filepath.Join(dir, RunDirName, "started")); err == nil { + t.Error("the entrypoint started for a clue_prepull package") + } +} + +func TestRunWarnsAndRunsWhenApplicabilityIsUnconfirmed(t *testing.T) { + skipOnWindows(t, "the entrypoint is a shell script") + dir, files := shellPackage(t, `echo ok > "$RD_OUTPUT_DIR/display.md"`+"\n", nil) + var stderr bytes.Buffer + o := runOpts(t, dir, files, []string{"PATH=" + os.Getenv("PATH")}, &stderr) + o.Manifest.URLs = URLs{Any: []string{"*question-interactives/*"}} + o.Applies = func(context.Context, URLs) (Verdict, error) { return Verdict{Unconfirmed: true}, nil } + if _, err := Run(context.Background(), o); err != nil { + t.Fatal(err) + } + if !strings.Contains(stderr.String(), "cannot check applicability yet") { + t.Errorf("stderr = %q, want the unconfirmed warning", stderr.String()) + } +} + +func TestRunKillsAtTheBound(t *testing.T) { + skipOnWindows(t, "the entrypoint is a shell script") + dir, files := shellPackage(t, "sleep 30 &\nwait\n", nil) + var stderr bytes.Buffer + o := runOpts(t, dir, files, []string{"PATH=" + os.Getenv("PATH")}, &stderr) + o.Timeout = 300 * time.Millisecond + start := time.Now() + _, err := Run(context.Background(), o) + var failed *Failed + if !errors.As(err, &failed) || !strings.Contains(err.Error(), "past its") || time.Since(start) > 5*time.Second { + t.Errorf("err = %v after %v", err, time.Since(start)) + } +} + +func TestRunKillsTheGroupWhenInterrupted(t *testing.T) { + skipOnWindows(t, "the entrypoint is a shell script") + dir, files := shellPackage(t, "sleep 30 &\nwait\n", nil) + var stderr bytes.Buffer + ctx, cancel := context.WithCancel(context.Background()) + time.AfterFunc(300*time.Millisecond, cancel) + start := time.Now() + _, err := Run(ctx, runOpts(t, dir, files, []string{"PATH=" + os.Getenv("PATH")}, &stderr)) + var failed *Failed + if !errors.As(err, &failed) || !strings.Contains(err.Error(), "interrupted") || time.Since(start) > 5*time.Second { + t.Errorf("err = %v after %v", err, time.Since(start)) + } +} + +func TestRunRefusesALinkedRunDirectory(t *testing.T) { + skipOnWindows(t, "links need privileges on Windows") + dir, files := shellPackage(t, `echo ok > "$RD_OUTPUT_DIR/display.md"`+"\n", nil) + elsewhere := t.TempDir() + keep := filepath.Join(elsewhere, "out", "keep.txt") + writeTree(t, elsewhere, map[string]string{"out/keep.txt": "keep"}) + if err := os.Symlink(elsewhere, filepath.Join(dir, RunDirName)); err != nil { + t.Fatal(err) + } + var stderr bytes.Buffer + if _, err := Run(context.Background(), runOpts(t, dir, files, nil, &stderr)); err == nil { + t.Error("a linked run directory was used") + } + if _, err := os.Stat(keep); err != nil { + t.Errorf("a file behind the link was removed: %v", err) + } +} + +func TestRunRefusesALinkedGitignore(t *testing.T) { + skipOnWindows(t, "links need privileges on Windows") + dir, files := shellPackage(t, `echo ok > "$RD_OUTPUT_DIR/display.md"`+"\n", nil) + target := filepath.Join(t.TempDir(), "precious") + writeTree(t, filepath.Dir(target), map[string]string{"precious": "keep"}) + writeTree(t, dir, map[string]string{RunDirName + "/": ""}) + if err := os.Symlink(target, filepath.Join(dir, RunDirName, ".gitignore")); err != nil { + t.Fatal(err) + } + var stderr bytes.Buffer + if _, err := Run(context.Background(), runOpts(t, dir, files, nil, &stderr)); err == nil { + t.Error("a linked .gitignore was written through") + } + if got, _ := os.ReadFile(target); string(got) != "keep" { + t.Errorf("the link's target became %q", got) + } +} + +func TestWriteBuildRefusesALinkedBuildDirectory(t *testing.T) { + skipOnWindows(t, "links need privileges on Windows") + dir := t.TempDir() + elsewhere := t.TempDir() + if err := os.Symlink(elsewhere, filepath.Join(dir, BuildDirName)); err != nil { + t.Fatal(err) + } + if err := WriteBuild(filepath.Join(dir, BuildDirName, "p-0.1.0.zip"), []byte("zip")); err == nil { + t.Error("a linked build directory was written into") + } + if entries, _ := os.ReadDir(elsewhere); len(entries) != 0 { + t.Errorf("files written behind the link: %v", entries) + } +} + +func TestEntrypointFallsBackToPython3WithANote(t *testing.T) { + look := func(have ...string) func(string) (string, error) { + return func(name string) (string, error) { + for _, h := range have { + if h == name { + return "/usr/bin/" + name, nil + } + } + return "", exec.ErrNotFound + } + } + cmd, _, note, err := entrypointCommand("/pkg", "run.py", look("python3.11", "python3")) + if err != nil || cmd != "/usr/bin/python3.11" || note != "" { + t.Errorf("with python3.11: %s, %q, %v", cmd, note, err) + } + cmd, _, note, err = entrypointCommand("/pkg", "run.py", look("python3")) + if err != nil || cmd != "/usr/bin/python3" || !strings.Contains(note, "python3.11") { + t.Errorf("without python3.11: %s, %q, %v", cmd, note, err) + } + if _, _, _, err := entrypointCommand("/pkg", "run.py", look()); err == nil { + t.Error("no interpreter was not an error") + } +} + +func TestWriteBuildReplacesALinkedZipRatherThanFollowingIt(t *testing.T) { + skipOnWindows(t, "links need privileges on Windows") + dir := t.TempDir() + target := filepath.Join(t.TempDir(), "precious") + writeTree(t, filepath.Dir(target), map[string]string{"precious": "keep"}) + writeTree(t, dir, map[string]string{BuildDirName + "/": ""}) + out := filepath.Join(dir, BuildDirName, "p-0.1.0.zip") + if err := os.Symlink(target, out); err != nil { + t.Fatal(err) + } + if err := WriteBuild(out, []byte("zip")); err != nil { + t.Fatal(err) + } + if got, _ := os.ReadFile(target); string(got) != "keep" { + t.Errorf("the link's target became %q", got) + } + if info, err := os.Lstat(out); err != nil || !info.Mode().IsRegular() { + t.Errorf("the zip is not a regular file: %v, %v", info, err) + } + if entries, _ := os.ReadDir(filepath.Join(dir, BuildDirName)); len(entries) != 2 { + t.Errorf("build directory holds %v, want the zip and .gitignore only", entries) + } +} + +func TestRunKeepsItsTreePrivate(t *testing.T) { + skipOnWindows(t, "Windows has no POSIX mode bits") + dir, files := shellPackage(t, `echo ok > "$RD_OUTPUT_DIR/display.md"`+"\n", nil) + run := filepath.Join(dir, RunDirName) + if err := os.Mkdir(run, 0o755); err != nil { + t.Fatal(err) + } + var stderr bytes.Buffer + if _, err := Run(context.Background(), runOpts(t, dir, files, []string{"PATH=" + os.Getenv("PATH")}, &stderr)); err != nil { + t.Fatal(err) + } + for rel, want := range map[string]os.FileMode{ + ".": 0o700, "pkg": 0o700, "in": 0o700, "out": 0o700, "bin": 0o700, "data": 0o700, + "in/scope.json": 0o600, ".gitignore": 0o600, + } { + info, err := os.Stat(filepath.Join(run, rel)) + if err != nil { + t.Fatal(err) + } + if got := info.Mode().Perm(); got != want { + t.Errorf("%s is %o, want %o", rel, got, want) + } + } +} + +func TestRunReportsAFailedPackageAndAMissingDisplay(t *testing.T) { + skipOnWindows(t, "the entrypoint is a shell script") + var stderr bytes.Buffer + dir, files := shellPackage(t, "exit 3\n", nil) + _, err := Run(context.Background(), runOpts(t, dir, files, []string{"PATH=" + os.Getenv("PATH")}, &stderr)) + var failed *Failed + if !errors.As(err, &failed) || !strings.Contains(err.Error(), "exit status 3") { + t.Errorf("a non-zero exit: err = %v, want Failed", err) + } + dir, files = shellPackage(t, "true\n", nil) + _, err = Run(context.Background(), runOpts(t, dir, files, []string{"PATH=" + os.Getenv("PATH")}, &stderr)) + var refused *OutputRefused + if !errors.As(err, &refused) || !strings.Contains(err.Error(), "no display.md") { + t.Errorf("no display.md: err = %v, want OutputRefused", err) + } +} diff --git a/internal/packages/run_unix_test.go b/internal/packages/run_unix_test.go new file mode 100644 index 0000000..efa9047 --- /dev/null +++ b/internal/packages/run_unix_test.go @@ -0,0 +1,56 @@ +//go:build !windows + +package packages + +import ( + "bytes" + "context" + "errors" + "os" + "path/filepath" + "strconv" + "strings" + "syscall" + "testing" + "time" +) + +// alive reports whether pid is a process that has not exited; a zombie awaiting its reaper +// counts as exited. +func alive(pid int) bool { + if err := syscall.Kill(pid, 0); errors.Is(err, syscall.ESRCH) { + return false + } + stat, err := os.ReadFile(filepath.Join("/proc", strconv.Itoa(pid), "stat")) + if err != nil { + return true + } + fields := strings.Fields(string(stat[bytes.LastIndexByte(stat, ')')+1:])) + return len(fields) == 0 || fields[0] != "Z" +} + +func TestRunLeavesNothingBehind(t *testing.T) { + dir, files := shellPackage(t, `sleep 30 >/dev/null 2>&1 & +echo $! > "$RD_OUTPUT_DIR/../bg.pid" +echo ok > "$RD_OUTPUT_DIR/display.md" +`, nil) + var stderr bytes.Buffer + if _, err := Run(context.Background(), runOpts(t, dir, files, []string{"PATH=" + os.Getenv("PATH")}, &stderr)); err != nil { + t.Fatal(err) + } + raw, err := os.ReadFile(filepath.Join(dir, RunDirName, "bg.pid")) + if err != nil { + t.Fatal(err) + } + pid, err := strconv.Atoi(strings.TrimSpace(string(raw))) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { syscall.Kill(pid, syscall.SIGKILL) }) + for deadline := time.Now().Add(5 * time.Second); alive(pid); { + if time.Now().After(deadline) { + t.Fatalf("the package's background process %d outlived the run", pid) + } + time.Sleep(20 * time.Millisecond) + } +} diff --git a/internal/packages/scope.go b/internal/packages/scope.go new file mode 100644 index 0000000..3d78c59 --- /dev/null +++ b/internal/packages/scope.go @@ -0,0 +1,102 @@ +package packages + +import ( + "bytes" + "encoding/json" + "fmt" + "io" + "regexp" +) + +var classHashRe = regexp.MustCompile(`^[0-9a-f]{48}$`) + +// Class and Assignment are scope.json's entries. +type Class struct { + ClassHash string `json:"class_hash"` + ClassID int64 `json:"class_id"` +} + +type Assignment struct { + OfferingID int64 `json:"offering_id"` + RunnableID int64 `json:"runnable_id"` + Name *string `json:"name"` + URL string `json:"url"` +} + +// LocalScope is the four keys rigse supplies on the VM, which an author writes for a laptop. +type LocalScope struct { + Kind string `json:"kind"` + ID string `json:"id"` + Classes []Class `json:"classes"` + Assignments []Assignment `json:"assignments"` +} + +// ScopeFile is scope.json as the runner writes it: ClueSource is its constant "firebase", and +// Dataset the bare name, as it writes pkg-, while RD_DATASET carries the full ref. +type ScopeFile struct { + LocalScope + ClueSource string `json:"clue_source"` + Dataset string `json:"dataset"` + OutputDir string `json:"output_dir"` +} + +// MarshalJSON keeps the runner's key order, so a diff between a local and a VM scope.json is +// only the values. +func (s ScopeFile) MarshalJSON() ([]byte, error) { + type ordered struct { + Kind string `json:"kind"` + ID string `json:"id"` + Classes []Class `json:"classes"` + Assignments []Assignment `json:"assignments"` + ClueSource string `json:"clue_source"` + Dataset string `json:"dataset"` + OutputDir string `json:"output_dir"` + } + return json.Marshal(ordered{s.Kind, s.ID, s.Classes, s.Assignments, s.ClueSource, s.Dataset, s.OutputDir}) +} + +// ParseLocalScope reads an author's scope file, refusing any key the four do not name (the +// other three are the runner's to set) and any shape the runner would never send. +func ParseLocalScope(data []byte) (LocalScope, error) { + var s LocalScope + dec := json.NewDecoder(bytes.NewReader(data)) + dec.DisallowUnknownFields() + if err := dec.Decode(&s); err != nil { + return s, fmt.Errorf("scope file: %v (it holds exactly kind, id, classes and assignments)", err) + } + if err := dec.Decode(&struct{}{}); err != io.EOF { + return s, fmt.Errorf("scope file: it must hold one JSON object and nothing after it") + } + switch { + case s.Kind == "": + return s, fmt.Errorf("scope file: kind is required") + case s.ID == "": + return s, fmt.Errorf("scope file: id is required") + case len(s.Classes) == 0: + return s, fmt.Errorf("scope file: classes must name at least one class") + case s.Assignments == nil: + return s, fmt.Errorf("scope file: assignments is required (an empty list is allowed)") + } + for i, c := range s.Classes { + if !classHashRe.MatchString(c.ClassHash) || c.ClassID < 1 { + return s, fmt.Errorf("scope file: classes[%d] needs a 48-hex class_hash and a positive class_id", i) + } + } + for i, a := range s.Assignments { + if a.OfferingID < 1 || a.RunnableID < 1 { + return s, fmt.Errorf("scope file: assignments[%d] needs a positive offering_id and runnable_id", i) + } + } + return s, nil +} + +// AssignmentURLs is the scope's half of the URLs a package's patterns are matched against. +func (s LocalScope) AssignmentURLs() []string { + var urls []string + for _, a := range s.Assignments { + if a.URL != "" { + urls = append(urls, a.URL) + } + } + return urls +} diff --git a/internal/packages/template/run.py b/internal/packages/template/run.py new file mode 100644 index 0000000..41455f0 --- /dev/null +++ b/internal/packages/template/run.py @@ -0,0 +1,87 @@ +#!/usr/bin/env python3 +"""A Researcher Dashboard package: counts a class's answers and the learners who wrote them. + +It runs the same way under `cc-data package run` on a laptop and on the dashboard's VM. It +reads the scope from RD_SCOPE_FILE, pulls its own data into RD_DATASET with cc-data, counts +through cc-data's views, and writes display.md, summary.txt and counts.json to the scope's +output_dir. Stdlib only: the VM installs no Python packages. + +It reads the class through a Student ID Mapping run, which the portal computes on every +download, so re-reading it picks up learners who joined since, and which has no limit on how +many assignments a class has. It reuses the researcher's own run for the class when there is +one and otherwise creates one, on the server the dataset's portal names. +""" +import json +import os +import subprocess +import sys + + +def cc_data(*args): + result = subprocess.run(["cc-data", *args], capture_output=True, text=True) + if result.returncode != 0: + detail = " ".join(p for p in (result.stderr.strip(), result.stdout.strip()) if p) + raise RuntimeError(f"cc-data {' '.join(args[:2])} exited {result.returncode}: {detail or '(no output)'}") + return result.stdout + + +def query(dataset, sql): + return json.loads(cc_data("query", "--dataset", dataset, sql, "--format", "json")) + + +def mapping_run(portal, class_id): + """The newest Student ID Mapping run filtered to exactly this class, else a new one.""" + runs = json.loads(cc_data("reports", "list", "--portal", portal, "--json"))["runs"] + mine = [r["run_id"] for r in runs + if r.get("slug") == "student-id-mapping" + and (r.get("report_filter") or {}).get("filters") == ["class"] + and (r.get("report_filter") or {}).get("class") == [class_id]] + if mine: + return max(int(run) for run in mine) + created = json.loads(cc_data( + "reports", "create", "--portal", portal, "--report-slug", "student-id-mapping", + "--report-filter", json.dumps({"class": [class_id]}), "--json")) + return int(created["run"]["run_id"]) + + +def main(): + with open(os.environ["RD_SCOPE_FILE"], encoding="utf-8") as handle: + scope = json.load(handle) + # RD_DATASET is the full / ref; scope["dataset"] is the bare name. + dataset = os.environ["RD_DATASET"] + class_id = int(scope["classes"][0]["class_id"]) + + try: + cc_data("dataset", "create", dataset) + except RuntimeError as err: + if "already exists" not in str(err).lower(): + raise + run = mapping_run(os.environ["CC_DATA_PORTAL"], class_id) + cc_data("get", "report", str(run), "--dataset", dataset, "--refresh") + cc_data("get", "answers", str(run), "--dataset", dataset) + + rows = query(dataset, f"SELECT count(*) AS n FROM report_{run}")[0]["n"] + if rows == 0: + raise RuntimeError("no report-service access to this class, or the class has no data") + answers = query(dataset, f"SELECT count(*) AS n FROM run_answers WHERE run_id = {run}")[0]["n"] + # By user_id, never by endpoint: a student has one endpoint per assignment. + learners = query(dataset, f""" + SELECT count(DISTINCT m.user_id) AS n FROM student_id_mapping m + JOIN run_answers a ON a.remote_endpoint = m.run_remote_endpoint + WHERE a.run_id = {run}""")[0]["n"] + + out = scope["output_dir"] + with open(os.path.join(out, "display.md"), "w", encoding="utf-8") as handle: + handle.write(f"# Class {class_id}\n\n- {answers} answers\n- {learners} learners with at least one answer\n") + with open(os.path.join(out, "summary.txt"), "w", encoding="utf-8") as handle: + handle.write(f"{answers} answers from {learners} learners\n") + with open(os.path.join(out, "counts.json"), "w", encoding="utf-8") as handle: + json.dump({"answers": answers}, handle) + + +if __name__ == "__main__": + try: + main() + except Exception as err: + print(str(err), file=sys.stderr) + sys.exit(1) diff --git a/release_test.go b/release_test.go index 29a117b..a951881 100644 --- a/release_test.go +++ b/release_test.go @@ -2,7 +2,10 @@ package main import ( "os" + "os/exec" + "path/filepath" "regexp" + "runtime" "strings" "testing" ) @@ -121,3 +124,42 @@ func readFile(t *testing.T, path string) string { } return string(data) } + +// A pre-release tag exists to be pinned, so it must not become GitHub's latest release or +// reach researchers through brew upgrade. Both steps read the one classification. +func TestReleaseWorkflowKeepsPreReleasesBack(t *testing.T) { + rel := readFile(t, ".github/workflows/release.yml") + for _, want := range []string{ + "--prerelease=${{ steps.tag.outputs.prerelease }}", + "if: steps.tag.outputs.prerelease == 'false'", + } { + if !strings.Contains(rel, want) { + t.Fatalf("release workflow must hold a pre-release tag back from latest and from Homebrew; missing %q", want) + } + } +} + +// The tag classification is shell, so it is run here against the tags that matter: a +// pre-release, a release, and build metadata whose hyphen is not a pre-release. +func TestReleaseWorkflowClassifiesTags(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("the step is bash") + } + rel := readFile(t, ".github/workflows/release.yml") + _, after, ok := strings.Cut(rel, "id: tag\n run: |\n") + if !ok { + t.Fatal("release workflow has no tag-classification step") + } + script, _, _ := strings.Cut(after, "\n\n") + for tag, want := range map[string]string{"v0.3.0-pre.1": "true", "v0.3.0": "false", "v1.2.3+ci-build": "false"} { + out := filepath.Join(t.TempDir(), "output") + cmd := exec.Command("bash", "-c", script) + cmd.Env = append(os.Environ(), "GITHUB_REF_NAME="+tag, "GITHUB_OUTPUT="+out) + if b, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("%s: %v\n%s", tag, err, b) + } + if got := strings.TrimSpace(readFile(t, out)); got != "prerelease="+want { + t.Errorf("%s: %s, want prerelease=%s", tag, got, want) + } + } +} diff --git a/specs/REPORT-146-package-commands-and-dataset-views.md b/specs/REPORT-146-package-commands-and-dataset-views.md new file mode 100644 index 0000000..70665bc --- /dev/null +++ b/specs/REPORT-146-package-commands-and-dataset-views.md @@ -0,0 +1,498 @@ +# cc-data-cli: the package commands and the dataset views that replace `_lib` + +**Jira**: https://concord-consortium.atlassian.net/browse/REPORT-146 + +**Status**: **Closed** + +## Overview + +Researchers get four `cc-data package` commands (`init`, `run`, `build`, `publish`) for writing a Researcher Dashboard package on a laptop, testing it the way the dashboard's runner will run it, and publishing it to the package catalog with the cc-data token they already have. A new dataset view, `run_answers`, and documented joins on views cc-data already has replace the counting SQL the spike copied into every package. + +## Requirements + +### Dataset views + +- **R1. `run_answers`.** A static view with every row of `answers`, plus `run_id`: one row for each run whose answers fetch holds that answer in membership (`run_membership.type = 'answers'`), whether a Student Answers or a Student ID Mapping run. Its columns are `run_id` (BIGINT) followed by the `answers` columns. Like every static view, it registers on every dataset: with no answers store or no membership it is empty with the same columns, never a bind error. +- **R2. Dropped (Doug, 2026-10-08): no `learner_endpoints` view.** It would have unioned each Student Answers CSV's `res__remote_endpoint` columns, so it is empty for a Student ID Mapping run and rests on the route REPORT-147 measured as failing on real classes. Nothing uses it, so nothing builds it. +- **R3. The documented counts.** For a run whose answers were fetched: + - answers: `SELECT count(*) FROM run_answers WHERE run_id = `; + - learners with at least one answer, for a Student ID Mapping run: `SELECT count(DISTINCT m.user_id) FROM student_id_mapping m JOIN run_answers a ON a.remote_endpoint = m.run_remote_endpoint WHERE a.run_id = `, by `user_id` and never by endpoint, since a student has one endpoint per assignment. It is scoped by the answers' run only: `student_id_mapping` keeps each learner's row from whichever mapping run was fetched last, so filtering it by `run_id` drops a learner another mapping run of the class also holds. + + The tests run both on a synthetic dataset built so that each wrong answer differs from the right one. It has: + - a learner with answers in two assignments, so counting endpoints gives 2 where counting users gives 1; + - a learner with a bare `.../` endpoint, who must not join; + - an answer whose membership puts it in two runs, so it counts once per run, the second run being a later mapping run of the same class that holds that learner's row in `student_id_mapping`; + - a history membership for one of the answers, which an untyped join would count; + - a second Student ID Mapping run for another class, which must not leak into the first run's counts. + +- **R4. Freshness needs no new view.** `logs.run_id` and `logs.event_time` already exist. The guidance documents the per-run form `SELECT count(*) AS logs, max(event_time) AS log_freshness_at FROM logs WHERE run_id = `, the same names `counts.json` carries. +- **R5. `run_answers` is documented where the drift guard checks,** in `core.md`'s Views section and the researcher guide's table. The guidance gives R3's learner count through `student_id_mapping`, says why it counts by `user_id`, and says why a Student ID Mapping run is the route for answers and learners (computed live, no assignment limit). The `run_answers` entry also says that its `run_id` is membership (every run that holds the answer), while the store's own `_run_id` is only the run that last fetched it. Filtering `answers` by `_run_id` undercounts a run whose answers a later run re-fetched. + +### Package contract checks + +- **R6. The package rules are report-server's, and cc-data asks for them.** + - **Where the check runs.** `build` and `run` send the zip they built to `POST /api/v1/packages/validate` before writing or running anything. A refusal keeps report-server's code and message and exits 5, and nothing is written or run. + - **What cc-data keeps.** Besides `init`'s name check (R9), which runs before report-server has seen anything and is one of the copies `final-design.md` section 17 records as deliberate, cc-data checks only what it must know to act: the manifest parses, the entrypoint is a file in the package, `expected_duration_seconds` is a positive integer (it sets `run`'s time bound), and `urls` and `clue_prepull` are read as written. It keeps no copy of report-server's limits. + - **Before the route exists.** If report-server answers 404 (the route is not deployed yet), `build` and `run` print a warning that the package is unchecked until publish, and continue. +- **R7. Applicability is report-server's, and cc-data asks for it.** cc-data has no glob matcher and no copy of the pattern fixture. `run` asks `POST /api/v1/packages/applies` (R12). +- **R8. The display cap is 65,536 bytes.** The cap is counted in UTF-8 bytes of `display.md`. One Go constant holds it, and its comment cites `final-design.md` section 10 as the only place the number is decided. A test pins both edges: 65,536 bytes passes and 65,537 is refused. This is one of the runner behaviors `run` reproduces (R11 to R14), which have no server to ask, since the runner lives on a VM. + +### `cc-data package init` + +- **R9.** `cc-data package init [dir]` (default `.`) creates `dir` if needed and writes `manifest.json` and `run.py`. It reads nothing in the directory except to check that neither file already exists; it refuses, changing nothing, if either does. + - The manifest has `name` (from `--name`, else the directory's base name when that matches the name grammar; otherwise `--name` is required), `title` (the name), `version` `0.1.0`, a placeholder one-line `description`, `urls` with empty `all`, `any` and `none`, `clue_prepull` `false`, `entrypoint` `run.py` and `expected_duration_seconds` `300`. report-server accepts it. + - The stub is stdlib-only Python and is the smallest complete package. It reads `RD_SCOPE_FILE` and `RD_DATASET`. + - **Runs.** It takes the researcher's newest Student ID Mapping run filtered to exactly the scope's class (from `cc-data reports list --json`: slug `student-id-mapping`, `report_filter.filters` equal to `["class"]` and `report_filter.class` equal to `[]`), as REPORT-147's R7 does. Otherwise it creates one with `cc-data reports create --report-slug student-id-mapping --report-filter '{"class":[]}'`. + - **Pulls and counts.** It re-reads the run with `get report --refresh` (computed live, so learners who joined since appear) and pulls `get answers` on it, then counts with the R3 queries through `cc-data query`, never by locating dataset files. + - **Guard.** It keeps the empty-scope guard that section 13 places in package code: no rows in the run's own `report_` is an error, which, unlike `student_id_mapping`, holds every learner of that run. + - **Output.** It writes `display.md`, `summary.txt` and `counts.json` to the scope's `output_dir`. + - **Why the stub creates runs.** On the VM its dataset starts empty, so a stub that only read an existing run would report nothing there. The VM path is what a package exists for. + +### `cc-data package run` + +- **R10. Inputs.** `cc-data package run [dir] --dataset --scope `. + - `--dataset` names the researcher's own dataset, resolved like every other dataset ref. `package run` neither requires that it exists nor creates it, since a package creates and fills its own dataset on the VM too. + - `--scope` names a local scope file. It holds the four keys rigse supplies on the VM: `kind`, `id`, `classes` (`[{class_hash, class_id}]`) and `assignments` (`[{offering_id, runnable_id, name, url}]`). The shape is checked; any other key is refused. + - `package init` does not write a scope file, so the file comes from the author. +- **R11. The package sees the runner's layout.** Before the entrypoint runs: + - **A fresh run directory** under `dir/.cc-data-run/`, with `in/` and `out/`, both emptied. `.cc-data-run` and everything `package run` empties or creates under it must be a real directory, never a link. A link is refused before anything is removed, so a stray link can never point the emptying at a directory outside the package, and a `.gitignore` that is a link is refused rather than written through. Like every other folder cc-data writes student data into, `.cc-data-run` and the folders `package run` creates in it are 0700, tightened if an earlier run left them wider, and the files it writes there (`scope.json`, `.gitignore`) are 0600. + - **A `.gitignore` containing `*`** written into `dir/.cc-data-run/`. A package directory is usually a git checkout (cc-data-studies ignores only `local-data/`, `__pycache__/` and `*.pyc`), and `out/display.md` holds counts or text drawn from student data. + - **`in/scope.json`** in the runner's exact shape: the scope file's four keys; `clue_source` `"firebase"`; `dataset` as the ref's bare name, as the runner writes `pkg-`; and `output_dir` as the absolute `out/` path. + - **The runner's environment:** + - `RD_DATASET`: the full ref. + - `RD_SCOPE_FILE`. + - `RD_OUTPUT_DIR`. + - `RD_DATA_DIR` and `CC_DATA_LOCAL`: `dir/.cc-data-run/data/`. + - `CC_DATA_ROOT`: the data root `package run` itself resolved, made absolute, since the package runs from its staged copy, where a relative root would name another folder. + - `CC_DATA_PORTAL`: the ref's portal. + - `RD_REPORT_SERVER_URL`: the portal's stored credential's server, when one is stored. + - **A short passthrough list a laptop needs.** It covers `HOME`, `PATH`, `USER`, `LOGNAME`, `LANG`, `LC_*`, `TZ`, `TMPDIR`, `DBUS_SESSION_BUS_ADDRESS`, `XDG_RUNTIME_DIR` and the proxy variables `HTTPS_PROXY`, `HTTP_PROXY` and `NO_PROXY` in either case (the runner sets the first two when it has a proxy, and `NO_PROXY` keeps a laptop's exemptions with them), plus Windows's `SYSTEMROOT`, `USERPROFILE`, `APPDATA`, `LOCALAPPDATA`, `PATHEXT` and `COMSPEC`. There is no Windows release, but CI builds and tests on `windows-2022`, and these variables are absent on Linux and macOS. These are the variables the package's own `cc-data` calls need to find the researcher's stored credential (the keychain or the credentials file), to reach the server, and to run at all. Nothing else is inherited. `.cc-data-run/bin/` holds only a link named `cc-data` to the running binary, whatever its name (`go run` included), and goes first on the package's `PATH`. The package's own calls therefore reach the same cc-data rather than an older install, and nothing else in the binary's folder moves ahead of the user's `PATH`. On Windows, which has no release and needs a privilege to create links, the binary's own folder goes first when it is named `cc-data`, and otherwise `run` warns which `cc-data` the package will reach. + - **No storage variables.** `RD_BUCKET` and `RD_STORAGE_PREFIX` are never set. +- **R12. Applicability before the entrypoint.** + - **The question asked.** When the manifest declares any pattern, `package run` sends the patterns and the scope's assignment URLs to `POST /api/v1/packages/applies`. report-server derives the interactive URLs inside those assignments, so the URLs matched are the ones the runner matches. + - **A refusal.** If the package does not apply, `run` refuses with report-server's reason, prefixed as the runner prefixes it ("the package does not apply to this class: "), and does not start the entrypoint. + - **Partial profiles.** Each URL the deriver could not read, and a truncated profile, is reported on stderr. + - **The call's timeout** is 5 minutes, since report-server waits up to 270 seconds for a deriver that may spend 240; the client's 60-second default would cut it off. + - **Before the route exists (404).** `run` warns that applicability cannot be checked yet and that the VM's runner is the check, and runs the package. +- **R13. Execution.** + - **The interpreter.** A `.py` entrypoint runs with `python3.11` when it is on `PATH`, else with `python3`, after a one-line note that the VM runs 3.11. Any other entrypoint is executed directly. + - **The working directory** is a staged copy of exactly the files R17's `build` would include, at `dir/.cc-data-run/pkg/`, rebuilt on every run. The runner's working directory is the unpacked archive (`steps.js` `runPackage`, `cwd: manifest.dir`). A package that reads a file `build` leaves out, such as something under `local-data/`, therefore fails locally just as it would on the VM. The same collection refuses a link, so `run` and `build` cannot disagree about what the package is. + - **Output streams.** The package's stdout and stderr stream to `package run`'s stderr. + - **The time bound.** A run longer than `expected_duration_seconds` plus 10 minutes is killed and reported as having run past its bound, as the runner does. + - **Nothing left behind.** When the entrypoint exits, anything still running in its process group is killed before the output is read, as the runner kills every process of the package's uid (RD-4 pass 2's R28a). A background process the package started therefore neither outlives the run nor writes into a later one. A process that leaves the group (`setsid`, a daemon) escapes this locally, as it escapes the time bound. + - **An interrupt.** Ctrl-C (or `SIGINT`) during the run kills the package and everything it started, and is reported as interrupted. The package runs in its own process group so the bound can kill its children, which also puts it out of reach of the terminal's own Ctrl-C. + - **No sandbox.** No network namespace, no other uid and no egress proxy: those are the VM's, and the story forbids needing researcher-dashboard infrastructure locally. +- **R14. The result, read as the runner reads it.** After the entrypoint exits 0, the output is read without following links: + - `display.md` is required, and is refused over R8's cap with its size and the limit (never truncated). + - `summary.txt` is optional, and its first line, trimmed, is the summary. A `summary.txt` that is a link or not a file refuses the result, as the runner's reader does. + - `counts.json`, when present and valid, contributes `answers`, `logs` and `log_freshness_at` only. One that is a link, not a file or not JSON is ignored, as on the VM. + + On success, stdout carries one JSON line: the display path and size in bytes, the summary, the counts and the elapsed seconds. +- **R15. A `clue_prepull: true` package is refused locally.** The message says CLUE data cannot be fetched by cc-data yet (section 13), so such a package runs only on the VM. +- **R16. Failures follow the CLI's exit-code contract.** + - A manifest cc-data cannot act on, an invalid scope file, or a package tree `build` cannot collect (a link, a non-executable entrypoint), is a usage error (exit 2, codes `INVALID_MANIFEST`, `INVALID_SCOPE`, `INVALID_PACKAGE`). + - A refusal or failure from report-server's validate or applies route keeps the server's code and exit class: 5 for a contract error, 3 for `NOT_AUTHENTICATED`, as every server call does. + - A package the scope does not satisfy, or a `clue_prepull` package, is `PACKAGE_REFUSED`. + - A non-zero exit, a timeout or an interrupt is `PACKAGE_FAILED`. + - A missing, linked or oversized `display.md`, or a linked `summary.txt`, is `PACKAGE_OUTPUT_REFUSED`. + + The last three exit 1 with the single JSON error envelope on stdout. + +### `cc-data package build` + +- **R17. Contents.** + - **What goes in:** `cc-data package build [dir] [--out ]` zips the regular files under `dir`. + - **What is left out:** any path with a segment beginning with `.` (which covers `.cc-data-run/`, `.git/` and `.DS_Store`), `__pycache__/`, `*.pyc` and `local-data/`. + - **What is refused:** a symbolic link, or any file that is neither regular nor a directory. Nothing outside `dir` is ever included, so no shared library can be vendored by the tool. + - **Before writing:** `manifest.json` must be at the root, it must pass R6, and the entrypoint must be one of the files collected. + - **Modes:** each file ships 0755 when any execute bit is set on disk, else 0644. `unzip` on the VM restores these, and the runner executes a non-`.py` entrypoint directly, so such an entrypoint must be executable or `run` and `build` refuse it. `run`'s staged copy uses the same modes. (Windows records no execute bit, so the check is skipped there.) +- **R18. Reproducible.** The same tree builds to the same bytes: entries in sorted order, a fixed modification time (which Go's writer also records as an extended-timestamp extra field), and nothing that varies between builds. The same package therefore always has the same checksum. +- **R19. `build` refuses what `publish` would refuse,** by asking report-server (R6). It takes `--portal` (default: the configured portal), `--origin projects/` and `--official` so the check is made as the publish will be. A refused archive is not written. When validate answers `already_published`, `build` still writes the zip and warns " is already published; publish will refuse it until the version changes". When `publishing_unavailable` names a reason, `build` warns "this package cannot be published yet: " and still writes the zip. `run` ignores both, since running a package needs neither a new version nor a bucket. +- **R20. Output.** + - The default output path is `dir/.cc-data-build/-.zip`. It sits inside an excluded dot directory, which must be a real directory as R11's are, with the same `.gitignore` as R11's, so building from inside the package directory never feeds one build's zip into the next. A name or version holding a path separator is `INVALID_MANIFEST` rather than a path, since it is unchecked until validate answers, and for good on a server without the route. + - An explicit `--out` is refused when it is a directory (the package folder included), when it is inside `.cc-data-run` (in any letter case, since macOS and Windows file systems ignore case by default), which `package run` empties, or when it is inside `dir` where R17 would collect it. + - stdout carries one JSON line: the path, `sha256:`, the size and the file count. + +### `cc-data package publish` + +- **R21. The request.** + - **The command:** `cc-data package publish [--portal ] [--origin projects/] [--official] [--json]`. + - **No local check:** publish sends the zip as it is. report-server's publish applies every rule, including the runner's stricter ones once REPORT-167 lands, so a hand-made zip is held to the same rules as a built one. + - **The upload:** `POST /api/v1/packages` on the portal's report server, with the portal's stored cc-data token. The body is the zip bytes, sent with `Content-Type: application/zip` and `Content-Length`. `origin` and `official=true` are sent only when given. + - **Retries:** the request is never retried. + - **Never S3:** it never talks to S3. +- **R22. Success is checked, not assumed.** On 201, the response's `checksum` must equal the local `sha256:` of the zip; a mismatch is an error. Output is the response: as JSON with `--json`, otherwise one line each for identity, version, visibility, official, current version and checksum. +- **R23. Errors pass through.** A server error keeps its code and message: `ALREADY_EXISTS`, `FORBIDDEN`, `UNPROCESSABLE`, `BAD_REQUEST` and `SERVICE_UNAVAILABLE` exit 5, and `NOT_AUTHENTICATED` exits 3. A failure where the server never answered carries an action: publishing again is safe, and `ALREADY_EXISTS` then means the first attempt landed. + +### Documentation + +- **R24.** The following are updated: the README's command sketch, a new researcher-guide section on developing and publishing a package, and the skill header's command list. `cc-data package --help` explains the local/VM differences R11 and R13 leave (the passthrough variables, the interpreter, no sandbox). + +### Release + +- **R25. A pre-release tag is safe to cut** (Doug, 2026-10-08, stream channel #21 and #22). After this story merges, `main` is tagged `v0.3.0-pre.1`, and RD-4 pass 2's runner image and REPORT-147's publish workflow pin that exact tag, rather than waiting for 0.3.0, whose fix version also holds the CLUE export chain. + - **The release workflow tells the two kinds of tag apart.** A tag with a semver pre-release suffix (a `-` after the version) still builds, checks and uploads every archive and its `.sha256`, and creates a GitHub release, marked as a pre-release so GitHub never shows it as the latest. + - **Homebrew never sees it.** The formula step is skipped for a pre-release, so `brew upgrade` never hands it to researchers. + - **One classification.** Both steps read one decision made once in the workflow, and a test pins both uses. + - The change ships in this story's PR, so the first tag after merge is already safe. + +### Done when + +- A package made with `init`, edited and run with `package run` against a real dataset, produces `display.md`, `summary.txt` and `counts.json` under the runner's layout. +- `display.md` over 65,536 bytes fails `package run` at the same size the runner refuses. +- With REPORT-167's routes deployed: `build` and `run` refuse what report-server refuses, and `run` refuses a package report-server's matcher says does not apply. Before they are deployed, both warn and proceed. +- `cc-data package publish` creates a private package and a version row on staging (whose `PackageBuckets` was set 2026-10-08, stream channel #34). +- No package imports a shared library from its archive: `build` ships only the package's own files. +- After merge, `main` is tagged `v0.3.0-pre.1`, and its GitHub release is marked as a pre-release, carries `cc-data_0.3.0-pre.1_linux_amd64.tar.gz` and its `.sha256` (which REPORT-147's workflow downloads), and pushed no Homebrew formula. +- Moved to REPORT-147 (Doug, 2026-10-08): a package built on a laptop runs on the VM and produces the same output. + +## Technical Notes + +### Dependencies + +**REPORT-167** (report-service: one home for the package rules and applicability, created 2026-10-08) owns the routes cc-data calls. REPORT-146 codes against the contract below, which that story's spec may refine. Until those routes are deployed, `build` and `run` warn and proceed (R6, R12), so REPORT-146 is not blocked on it. + +- **`POST /api/v1/packages/validate`** (cc-data token). The body is the raw zip, sent exactly as for publish, with `origin` and `official` as optional query parameters, as publish takes them. It runs every check `POST /api/v1/packages` runs, including permission and origin, and stops before writing any row or object. It answers 200 with `{identity, version, checksum, visibility, already_published}` (what a publish would record now), or the same coded error a publish would give for any other refusal. **A version that is already published is reported, not refused:** `already_published` is true and the answer is still 200, while only the publish itself answers `ALREADY_EXISTS` (REPORT-167's R12, refined after REPORT-146's first spec commit). **A portal that cannot store packages yet is reported, not refused:** `publishing_unavailable` is `null` or publish's own message (today only "publishing is not configured for ", before the portal has a bucket in `PackageBuckets`), the answer is still 200, and every other check still runs; only the publish answers that 422. Without this, `package run` would refuse every package on staging until its bucket is set (REPORT-167, `1ff2fc0`). The answer is thus `{identity, version, checksum, visibility, already_published, publishing_unavailable}`. + - It is a separate route, not a `dry_run` parameter on publish. Today's publish ignores parameters it does not know, so a `dry_run=true` sent to a server without the feature would publish for real. A missing route answers 404. +- **`POST /api/v1/packages/applies`** (cc-data token, or the researcher token the runner holds). The body is JSON: `{urls: {all, any, none}, assignment_urls: [...], scope_urls: [...]}`. + - **Assignment URLs** are followed by report-service's existing deriver (`deriveProfile` in `functions/src/researcher-dashboard/derive-profile.ts`). report-server reaches it through a new `derive_urls` route on the `api` function, the way it already calls `bulk_read`. + - **Scope URLs** are matched as given; that is the runner's case, since it already holds the class profile. + - **`urls` is required**: a missing or `null` `urls` is 400 `BAD_REQUEST`, while a `null` group inside it counts as empty. cc-data always sends it. + - **The answer** is `{applies, reason, interactive_urls, unread: [{url, reason}], truncated}`. The `reason` uses the runner's wording. + - **The matcher** behind it is the only glob matcher in the system. The app gets the same answer through the package-list call it already makes. +- **One contract fixture** of identity, version and package-key cases lives in report-service, asserted by report-server, the function, the runner and the app. cc-data's only copy of that grammar is `init`'s name check (R9), kept on purpose. +- **Neither route answers 404 on a report-server that has it** (REPORT-167). So a 404 still means only that the route is not deployed. A deriver failure answers 400 or 503, and report-server waits up to 270 seconds for the deriver, inside cc-data's 5-minute timeout. +- **report-server adopts the runner's stricter rules,** so that anything it accepts the runner will run: `expected_duration_seconds` at most 7,200 (today 28,800), and no symbolic-link entries in an archive. + +### Notes + +- **Views live in `internal/duck/views.go`.** Add one builder to `viewSet.statements()`, after `runMembershipView` (`run_answers` reads `answers` and `run_membership`, so it must register after both). Use `vs.prefix` for every referenced view so multi-dataset sessions resolve to the right schema, as `perStoreView` does. +- **What the tests can see.** `StaticViewNames()` and the guard pick up the view automatically. `cmd/query.go`'s help and `internal/mcpserver/tools.go`'s `query` description list it with no edit. +- **Verified against real staging data.** On `learn.portal.staging.concord.org/staging-smoketest`, run 164: `_lib`'s answers SQL gives 5, and `run_answers` gives 5. REPORT-147 measured the Student ID Mapping learner join against the Student Answers union on production for both Wildfire classes (runs 2390 to 2393): the two count the same today. +- **The answers-type CSV also carries the class's shape**: `class_id`, and per assignment `res__name`, `res__offering_id` and `res__resource_url`. `runnable_id` and `class_hash` are not in it. A local scope therefore cannot be derived from a pulled report alone, which is why R10 takes the scope from a file. +- **report-server already calls report-service's `api` function** through `ReportServer.ReportService` (`server/lib/report_server/report_service.ex`), with the shared `REPORT_SERVICE_TOKEN`, for `bulk_read`, `fetch_attachment_meta`, `answer`, `plugin_states` and `resource`. REPORT-167's `derive_urls` route follows that same path, so `deriveProfile` stays the only copy of the walk and no new credential is created. +- **The publish request needs a new client method.** `internal/api/client.go`'s `do` always sends `Content-Type: application/json` with a body, so publishing needs a sibling that sends raw bytes with `application/zip`. `net/http` sets `Content-Length` for a `*bytes.Reader` body. `isIdempotent` already makes POST non-retried. `AsWriteCLIError` is the existing pattern for a write that must not invite a blind retry. +- **The Go `archive/zip` writer sets data-descriptor flag `0x8` and zero local-header sizes.** report-server's `Archive` reads offsets and sizes from the central directory for this reason (REPORT-142). The runner unpacks with `unzip`, which handles it. A reproducible build sets each header's `Modified` to a fixed time and uses `Deflate`. +- **cc-data releases.** The Jira fix version stays `cc-data-cli 0.3.0`, but the view and the package commands reach a VM through the pre-release tag `v0.3.0-pre.1` (R25). The runner image pins `CC_DATA_REF=v0.2.0` (`runner/Dockerfile`) and builds cc-data from the git tag, so RD-4 pass 2 raises that pin to the tag (channel #15 and #21). REPORT-147's publish workflow downloads the tag's linux/amd64 archive instead, which is why the pre-release must carry its assets. The R11 environment adds nothing the runner does not already set. +- **What staging answers before `PackageBuckets` is set.** report-server validates the archive and manifest before it looks up the bucket (`Packages.publish`). Until the parameter is set for a portal server, a publish that passes every other check answers 422 `UNPROCESSABLE` "publishing is not configured for ". That answer is itself evidence that `publish`'s request was well formed. +- **Checked with throwaway code (stage 4), then deleted.** + - **The pattern fixture, for REPORT-167.** RD-4 pass 2's `fixtures/url-patterns.json` holds 27 match cases and 12 applicability cases (sha256 `35a994a1...c420`); the stream docs say 26. A Go port of the runner's matcher passed all of them, and making `?` a wildcard failed the `a?c` case, so the fixture catches that mutation. The port was then dropped with the decision to match only in report-server, where this fixture belongs. + - **The archive.** A Go `archive/zip` build with sorted entries, `Deflate` and a fixed `Modified` gives identical bytes after a source file's mtime changes. `unzip` restores each entry's mode, so a 0644 shell entrypoint is "Permission denied" after unpacking (R17's mode rule). `unzip -p` and Python's `zipfile` read it, and a symbolic link in the tree is refused during the walk. report-server's own tests already cover reading a Go-written zip (`server/test/support/fixtures/packages/class-counts-go-1.0.6.zip`). + - **The view.** `run_answers` was added to a scratch copy of `views.go` and run through the real engine: 5 answers for run 164 on `staging-smoketest`, as `_lib` gives; empty with its columns on a dataset with no answers (`log-test`); resolved per schema in a two-dataset session; and, after `dataset materialize`, a join over the materialized `answers` and `run_membership` with unchanged counts. The full test suite passed apart from the two documentation guards, which named exactly the new view. (A `learner_endpoints` view was also prototyped and verified, then dropped with R2.) + - **The stub.** It was run through `package run`'s engine against a synthetic dataset with a Student ID Mapping run and its answers, with a wrapper `cc-data` faking only the server calls. Out of a `reports list` holding a run filtered by class and school, a Student Answers run and another class's run, it picked the class-only Student ID Mapping run, and wrote 3 answers and 2 learners (a learner in two assignments counted once, a learner with no answers not at all). With no matching run it created one with the class filter, and with no rows it stopped at the empty-scope guard's message. +- **Out-of-band references.** The stream docs (`plan.md`, `fy26-sprint-26.md`, `speccing.md`, `final-design.md`) are global oob files under `streams/researcher-dashboard/`. RD-4 pass 2's throwaway build is branch oob `researcher-dashboard/RD-4-pass-2-pull-loop/stage-verification/`. + +## Out of Scope + +- REPORT-167's routes and rule changes (Dependencies), and the dashboard app's and runner's use of them (RD-3, RD-4). +- MCP tools for the package commands; an agent can run the CLI. +- Changing a package's visibility, `official`, `archived` or `current_version` after publish (report-server's `POST /api/v1/packages/:kind/:owner_id/:name/:state`), and listing the catalog. +- Creating report runs or pulling data on a package's behalf, locally or on the VM. +- The VM's sandbox (network namespace, package uid, egress proxy) and the CLUE pre-pull. +- Bumping the runner image's cc-data pin; deleting `_lib` or the spike packages; a dashboard control that hands an author a ready-made scope file (RD-3). +- Recording the cc-data version in a package's result, which is the runner's image version (section 13). + +## Not Yet Implemented + +- Tagging `main` as `v0.3.0-pre.1` after this story merges, and checking that its GitHub release is marked as a pre-release with the linux/amd64 archive and its `.sha256` and no Homebrew formula (R25, Done when). It is a release step after the merge, not code on the branch. +- Publishing a private package and a version row on staging with `cc-data package publish` (Done when). It runs against staging once the branch is released. +- `build` and `run` refusing what report-server refuses, and `run` refusing a package that does not apply, as observed against a live server (Done when): both wait on REPORT-167's routes being deployed, and until then both warn and proceed. +- A package built on a laptop producing the same output on the VM: moved to REPORT-147 (Doug, 2026-10-08). + +## Decisions + +### Which URLs does `package run` match a package's patterns against? +**Context**: The runner matches against the class profile's `assignment_urls` ∪ `interactive_urls`, while `scope.json` carries only assignment URLs, so a pattern on an interactive (the Wildfire case) would be refused locally and accepted on the VM. The profile is derived by report-service's function from public authoring JSON. +**Options considered**: +- A) A Go port of the deriver's walk in cc-data. +- B) An author-supplied `profile_urls` list in the scope file. +- C) Assignment URLs only. +- D) Ask report-server, which reaches report-service's existing deriver. + +**Decision**: D, taken further (Doug, 2026-10-08): report-server holds the only matcher, and cc-data, the dashboard app and the runner all ask it (Dependencies, `POST /api/v1/packages/applies`). The deriver keeps its one copy, and there is no glob matcher anywhere else. The app and the runner already call report-service at startup, so the call adds no new dependency for either. Before the route is deployed, `run` warns and proceeds (R12). + +--- + +### Where does the shared pattern fixture live? +**Decision**: The question dissolves under the decision above (Doug, 2026-10-08). With one matcher in report-server, `url-patterns.json` is an ordinary test file beside it in report-service, and nothing vendors a copy. cc-data has none. + +--- + +### How is "a package built on a laptop runs in the VM and produces the same output" verified? +**Decision**: REPORT-147 owns it (Doug, 2026-10-08). Its done-when already requires the Wildfire package to run "identically under `cc-data package run` on a laptop and on a VM", and it is the first story that can put a package on a VM. REPORT-146 closes on the criteria in "Done when" below. + +--- + +### Doing applicability and the rules in report-server rather than in cc-data +**Options considered**: +- A) cc-data keeps Go copies of the manifest rules, the archive limits and the matcher, guarded by fixtures. +- B) cc-data asks report-server's validate and applies routes, keeping only what it needs to stage and start a package. + +**Decision**: B (Doug, 2026-10-08, to remove DRY violations without multiplying stories). The copies already disagreed: report-server allows 28,800 seconds where the runner refuses over 7,200. REPORT-167 makes report-server adopt the runner's stricter rules, so its answer is the runner's. + +--- + +### Keep `learner_endpoints`, and which route the `init` stub teaches +**Context**: REPORT-147 (`bca55aa`) counts learners through a Student ID Mapping run with `run_answers` and `student_id_mapping`, after measuring on production that whole-class Student Answers runs fail above three or four assignments and that a reused run's learner list goes stale. That left `learner_endpoints` with no consumer, and the stub teaching the failing route. +**Options considered**: +- A) Drop `learner_endpoints`, document the Student ID Mapping learner count, and switch the stub to Student ID Mapping. +- B) Keep the view for researchers who pull Student Answers runs, and keep the stub. + +**Decision**: A (Doug, 2026-10-08). Nothing uses the view, the route it rests on fails on real classes, and the existing `student_id_mapping` view already makes the count without double-counting. The stub is the template authors copy, so it teaches the route that works. + +--- + +### Where a local scope comes from +**Options considered**: +- A) An author-supplied file (`--scope`) carrying the four keys rigse supplies. +- B) Synthesized from a pulled Student Answers report (`class_id`, and `res__offering_id`, `_name` and `_resource_url` per assignment). + +**Decision**: A. The report has no `class_hash` or `runnable_id`, and the scope contract has no room for nulls in either. Any synthesized scope would differ from the VM's in exactly the fields a package might key on. A dashboard control that hands out the real scope is the eventual source (RD-3, out of scope). + +--- + +### A `clue_prepull` package under `package run` +**Options considered**: +- A) Refuse it locally (R15). +- B) Run it with `clue_source: "firebase"` and an empty corpus. + +**Decision**: A. B tells the package something false, and the package would produce numbers the VM would not. Section 13 already says a CLUE package is the one package that cannot be developed locally until REPORT-109 to 111. + +--- + +### The passthrough environment in R11 +**Context**: The runner replaces the environment entirely. Locally, a package's own `cc-data` calls must find the researcher's credential: the OS keychain via `go-keyring`, or `~/.config/cc-data/credentials.json`. +**Options considered**: +- A) The R11 allowlist. +- B) Inherit the whole environment and set the runner's variables over it. + +**Decision**: A, verified on this machine (Linux, keyring backend). `env -i` with the R11 list, then `cc-data reports list --portal learn.concord.org --json`, reads the keyring token and lists runs. So does `env -i HOME PATH USER` alone: go-keyring reaches the session bus without `DBUS_SESSION_BUS_ADDRESS` here, so the D-Bus and XDG variables are kept only for setups that need them. On macOS go-keyring calls `/usr/bin/security`, which needs nothing beyond `HOME` and a `PATH` reaching `/usr/bin`. A cc-data query under the same `env -i` reads a dataset. B would let a package pass locally while depending on a variable the VM never sets. + +--- + +### `scope.json`'s `dataset` is a bare name +**Context**: `final-design.md` section 10 shows `"dataset": ""`. RD-4 pass 2's `scopeFile` writes the bare `pkg-`, while `RD_DATASET` carries the full ref. A package that passes `scope["dataset"]` to `cc-data query --dataset` resolves it under the configured default portal, which may not be the dataset's. +**Options considered**: +- A) Mirror the runner (bare name), have the `init` stub read `RD_DATASET`, and record the discrepancy for RD-4 and the design to settle. +- B) Write the full ref locally, and ask RD-4 to change `scopeFile` to match section 10. + +**Decision**: A. This story's contract is "exactly as the runner", and B changes a contract RD-4 owns from a story that consumes it. RD-4 pass 2 has since settled it as the bare name, and `final-design.md` section 10 says so (stream channel #28). The `init` stub reads `RD_DATASET`, the one name that is a full ref on both sides. + +--- + +### Emptying the run directory could follow a link out of the package +**Context**: Raised in self-review (Security Engineer). + +**Decision**: R11 empties `dir/.cc-data-run/in` and `out` on every run. If `.cc-data-run` were a symbolic link to another directory, the emptying would delete under that target. Fixed in R11: every directory `package run` empties or creates must be a real directory, and a link is refused before anything is removed. + +--- + +### `publish` would upload a hand-made zip the runner then refuses +**Context**: Raised in self-review (Security Engineer). + +**Decision**: report-server checks entry paths but not entry types, and the runner refuses a symbolic link after unpacking (RD-4's `checkTree`). So a zip made outside `build` could publish and then fail on every VM. Resolved by REPORT-167: report-server refuses link entries at publish, so the one check covers every zip. + +--- + +### Run output written inside a git checkout +**Context**: Raised in self-review (Education Researcher). + +**Decision**: `out/display.md` is derived from student data and lands inside the package directory. cc-data-studies' `.gitignore` ignores only `local-data/`, `__pycache__/` and `*.pyc` (read 2026-10-07), so `git add -A` would stage it. Fixed in R11 and R20: each working directory gets a `.gitignore` of `*`. + +--- + +### `_run_id` reads like run scoping and is not +**Context**: Raised in self-review (Education Researcher). + +**Decision**: The answers store stamps `_run_id` with the run that fetched each stored version (`internal/store/segment.go` `AppendPage`), and a later run's re-fetch overwrites it. Next to `run_answers.run_id`, a researcher could filter `answers` by `_run_id` and silently undercount. Fixed in R5's documentation requirement. + +--- + +### The default build output would be swept into the next build +**Context**: Raised in self-review (Package Author). + +**Decision**: With the output in the current directory and `build` run from inside the package, as authors do, R20 as written either refused its own default or zipped the last build into the next. Fixed in R20: the default is `dir/.cc-data-build/`, which R17 already excludes. + +--- + +### An `init` stub that only reads existing runs reports nothing on the VM +**Context**: Raised in self-review (Package Author). + +**Decision**: On the VM a package's dataset (`pkg-`) starts empty, and section 13 makes run creation package code. A stub that assumed a pulled run would pass locally and count zero on its first VM run. Fixed in R9: the stub reuses or creates its own run, as `_lib/ccdata.py` and RD-4's smoke package do (a Student ID Mapping run since 2026-10-08). + +--- + +### R3 could pass with a fixture that cannot tell the queries apart +**Context**: Raised in self-review (QA Engineer). + +**Decision**: With one assignment per learner, counting endpoints and counting users give the same number, so a broken learner view would pass. Fixed in R3: the synthetic dataset includes a two-assignment learner, a bare endpoint, an answer in two runs and a second class's run. + +--- + +### Staging's answer before `PackageBuckets` exists was unstated +**Context**: Raised in self-review (Operator). + +**Decision**: `Store.bucket_for` returns 422 "publishing is not configured for " after every archive and manifest check has passed. Recorded in Technical Notes, so that answer reads as a well-formed request rather than a failure of this story. + +--- + +### How a raw zip body reaches the server +**Options considered**: +- A) Generalize `Client.do` into `send` with a content type. +- B) Build a separate `http.Request` for the zip routes. + +**Decision**: A. Publish and validate then share the bearer, the cross-origin redirect refusal, the no-retry rule for POST and the error-envelope decoding with every other call. B would re-implement each of them. + +--- + +### The exit class for a refused or failed package +**Options considered**: +- A) Exit 1 with codes `PACKAGE_REFUSED`, `PACKAGE_FAILED` and `PACKAGE_OUTPUT_REFUSED`. +- B) A new exit code 7. + +**Decision**: A. The root help defines exit 1 as "internal/other error", and the JSON envelope's code already distinguishes the cases. A new exit class is a contract change for every script that reads cc-data's exit codes. + +--- + +### Following no link without `O_NOFOLLOW` +**Options considered**: +- A) `Lstat`, then open, then `os.SameFile`. +- B) `syscall.O_NOFOLLOW`. + +**Decision**: A. CI tests on `windows-2022`, where `syscall.O_NOFOLLOW` does not exist. On a laptop the race A leaves open would need the author's own package to swap a file mid-read, and the runner, which runs as root, is the boundary that matters. + +--- + +### What a 404 from the new routes means +**Options considered**: +- A) Any 404 from validate or applies means the route is not deployed: warn and proceed. +- B) Require a specific error code before treating it as "not deployed". + +**Decision**: A. report-server's router has no route either path could match today (`/packages/validate` and `/packages/applies` are one segment, and the only other `POST /packages/...` route takes four). So a 404 can only mean the route is missing, and B would need a code that an older server, by definition, cannot send. + +--- + +### The process-group kill on macOS +**Context**: `TestRunKillsAtTheBound` ran on Linux only. + +**Decision**: `Setpgid` and `kill(-pgid, SIGKILL)` are POSIX and behave the same on Darwin, and `GOOS=darwin go build ./internal/packages/` compiles. CI's `macos-15` and `macos-15-intel` jobs run the test, so a Darwin difference would fail there. + +--- + +### A shell entrypoint would run locally and be refused on the VM +**Context**: Raised in self-review (Senior Engineer). + +**Decision**: The zip originally gave every entry mode 0644, and the runner executes a non-`.py` entrypoint directly. A Go zip with a 0644 `plain.sh` and a 0755 `exec.sh`, unpacked with `unzip`, gives "Permission denied" for the first and runs the second, so `unzip` restores the zip's modes. Locally, the staged copy kept the source's 0755, so `package run` passed. Fixed: +- `Files` records a ship mode per file (0755 if any execute bit is set, else 0644). +- `Zip` and the staging copy both use it. +- `CheckEntrypointMode` refuses a non-executable non-`.py` entrypoint in `run` and `build`. + +R17 states the rule. + +--- + +### A scope file inside the package ships with it +**Context**: Raised in self-review (Operator). + +**Decision**: An author naming the file `scope.json` in the package directory would publish a class hash in every archive. The documentation step says to keep it outside the package or under a dot name, which `Collect` already excludes. + +--- + +### The applicability call would time out before the deriver finishes +**Context**: Raised in self-review (Operator). + +**Decision**: The deriver may spend up to 240 seconds on a scope's activities, and the client's per-attempt timeout for JSON calls is 60 seconds. `appliesFor` copies the client with a 5-minute `RequestTimeout`, which also keeps the shared client's default for every other call. + +--- + +### An applies failure lost its code and exit class +**Context**: Raised in self-review (Senior Engineer). + +**Decision**: `Run` returns `appliesFor`'s `*output.CLIError` unchanged, and `packageRunError` matched only the three package error types, so it fell through to `Internalf`. A throwaway test fed it applies' 503 `SERVICE_UNAVAILABLE` and `NOT_AUTHENTICATED`, and both came back exit 1 `INTERNAL`, against R16. `packageRunError` now returns a `CLIError` as it is, and `TestPackageRunKeepsTheServersCode` pins it. + +--- + +### Ctrl-C left the package running +**Context**: Raised in self-review (Package Author). + +**Decision**: `Setpgid` takes the package out of the terminal's foreground group, and cc-data had no interrupt handling outside `dataset materialize`. Sending `SIGINT` to cc-data's group, as a terminal does, ended cc-data while the package's `sleep 30` kept running. `package run` now cancels on `os.Interrupt` (the `materialize` idiom), which kills the group through the existing `cmd.Cancel`, and `Run` reports "the package was interrupted". Rerun with the fix, the package was gone a second after the signal. + +--- + +### `build`'s default path came from unchecked manifest fields +**Context**: Raised in self-review (Security Engineer). + +**Decision**: On a server without the validate route, a manifest named `../../escaped` built to `escaped-1.0.0.zip` two directories above the package, outside `.cc-data-build` and its `.gitignore`. `build` now refuses a name and version that make a path. It is a path check, not a copy of report-server's name grammar. + +--- + +### The proxy variables were not passed through +**Context**: Raised in self-review (Operator). + +**Decision**: cc-data's client uses Go's default transport, which reads `HTTPS_PROXY`, and the runner sets the proxy variables when it has a proxy (`package-env.js`). Behind a proxy, `package run`'s own requests would succeed while the package's `cc-data` calls failed. They are now on the passthrough list. + +--- + +### "No extra fields" in the zip was untrue +**Context**: Raised in self-review (Senior Engineer). + +**Decision**: `archive/zip` writes `Modified` as an extended-timestamp extra field (`5554...` on every entry of a scratch build). It is as fixed as the time, so R18 holds; the comment and R18 now say so. + +--- + +### `publish` kept a local copy of the origin rule +**Context**: Raised in self-review (One Source of Truth). + +**Decision**: `publish` refused an `--origin` without the `projects/` prefix, `build` did not, and report-server answers the same case with 422 "origin must be projects/; a user origin is always your own". The local check is deleted, so both commands give report-server's answer. + +--- + +### Assumptions the second self-review checked and kept +**Context**: Raised in self-review (second round), each checked against the code or the live sources. + +**Decision**: Found sound: a second `get answers` on a fetched run re-fetches and merges new answers, so the stub stays current; `reports list --json`'s `report_filter` carries `filters` and `class` as the stub reads them; `query --format json` answers an array of objects; report-server's 270-second wait sits between the deriver's 240 and cc-data's 300; an old report-server answers both new routes with a JSON 404 even without a token; and no publish check refuses an ordinary user's own origin, so `run`'s validate call cannot lock out a researcher who may use the API. + +--- + +### The documented learner count filtered the deduplicated mapping by run +**Context**: Raised in Copilot's review of PR #18. `student_id_mapping` keeps one row per learner across every mapping run, latest fetch winning (`TestPerRunDuplicateCheckIsNotConfusedByLaterRuns` already showed that filtering it by an older `run_id` loses learners). The count's `AND m.run_id = ` therefore dropped a learner whom a later-fetched mapping run of the same class also held, and the `init` stub's empty-scope guard, which counted `student_id_mapping` rows for the run, could fail on a class with data. +**Options considered**: +- A) A new per-run mapping view, deduplicated within each run only. +- B) Count over `report_`, the raw per-run CSV, repeating the bare-endpoint rule in the query. +- C) Drop the mapping-side filter: the answers are already scoped by `a.run_id`, a run's answers fetch holds only its own learners' answers, and an endpoint maps to one learner whichever mapping run's row won. + +**Decision**: C for the count, and the guard counts `report_`, which the stub has just pulled. No new view, and the bare-endpoint rule stays in `student_id_mapping` alone. The test gains a later mapping run of the same class, so scoping the mapping by run fails it. + +--- + +### Copilot's other findings on PR #18 +**Context**: Raised in Copilot's review of PR #18. + +**Decision**: The default build zip is written to a temporary file and renamed into place, so a link already at that path is replaced rather than followed. The scope file must hold one JSON object and nothing after it. `summary.txt` and `counts.json` stay read under the display cap, since the runner reads them that way too, and the code now says so. + +--- + +### The PR review's findings (Ethan McElroy, PR #18) +**Context**: Raised in Ethan McElroy's review of PR #18, which requested changes for one blocker. + +**Decision**: All were fixed, each with a test that fails when the fix is reverted. +- **Blocker, a private run folder.** `.cc-data-run` and everything `package run` creates in it are 0700, tightened if an earlier run left them wider, and `scope.json` and `.gitignore` are 0600, as every other student-data folder in cc-data is. +- **A relative data root** is made absolute before it reaches the package. +- **`--out`** refuses a directory, including the package folder, and any path inside `.cc-data-run`. +- **A 404 from validate or applies** still means the route is not deployed. REPORT-167's contract already says neither route ever answers 404, and `RouteMissing`'s comment now states that agreement. Matching the catch-all's body would not help, since a real `not_found` sends the same body. +- **A package folder that is itself a link** is resolved before it is walked. Links inside it are still refused. +- **The package's own `cc-data`** is the running binary, reached through a link in `.cc-data-run/bin/`, the only folder put ahead of the user's `PATH`. On Windows the binary's own folder is used when it is named `cc-data`. +- **`init`** reports the real error when it cannot check the folder, and **`build`** renames its zip through `fsutil.RenameAtomic`, which retries on Windows. +- **Tests added** for the three package error codes, a package that fails or writes no `display.md`, the request count when publish's connection drops, an applies 503 through `run`, validate running before the run, `HOME`, and the run folder's modes. The leftover-process test now checks that the process is dead rather than racing a timed write. A new test keeps the learner query identical in the test, the guidance and the stub. The release workflow's tag classification is now run against sample tags, and it ignores build metadata after `+`. +- **Comments and help text** corrected where they overstated behavior. `Files.Bytes`, which nothing read, is removed, and the stub ignores only an "already exists" error from `dataset create`. + +--- + +### The approving review's notes (Ethan McElroy, PR #18) +**Context**: Ethan McElroy approved PR #18 with four minor notes left to the author's discretion. + +**Decision**: All four were taken. +- **The `PATH` side effect.** Putting the binary's own folder first also put every other tool in a shared `bin` ahead of the user's `PATH`. The package now gets `.cc-data-run/bin/`, holding only a link to the running binary, so nothing else moves. This also drops the "not named cc-data" warning, since a link can carry the name, except on Windows. +- **`--out` through links.** The package folder and `--out` are both resolved through links before they are compared, so the same place reached two ways compares equal. +- **A blank line** in `release_test.go` and **a flag shared with a test server's handler** without synchronization, now an `atomic.Bool`, are fixed. +