Repository navigation
build(deps): bump the github-actions group across 1 directory with 3 updates - #13
dependabot[bot] wants to merge 17 commits into
Conversation
schooltools 0.2.0: headless Go CLI for the CBE / D2L / Brightspace LMS. Cobra commands live in cmd/, shared logic in internal/. Includes README, CONTRIBUTING, SECURITY, CODE_OF_CONDUCT, SUPPORT, KNOWN_ISSUES, D2L_API_RESEARCH, CHANGELOG, AGENTS, and a CI workflow (ubuntu + macos, Go 1.26, -race) plus a release workflow. Module path: github.com/aarav/schooltools
Trigger: push of a v* tag (e.g. v0.2.0). Drops the manual
workflow_dispatch path entirely.
What it does:
- Builds the CLI for 7 targets (linux/darwin/windows × amd64/arm64 +
linux/arm) on Go 1.26 with CGO_ENABLED=0 and stripped binaries.
- Runs go mod verify + go vet before each build.
- Uploads each binary + its per-file sha256 as a workflow artifact.
- After all builds succeed, downloads the artifacts, generates a
combined SHA256SUMS, and publishes a GitHub release with
auto-generated notes.
Concurrency group 'release-${{ github.ref }}' prevents two releases
of the same tag from racing.
The previous assertion compared a time.Time in time.Local against one parsed from RFC3339 (which produces a FixedZone with the same offset but a different pointer). reflect.DeepEqual — and therefore assert.Equal — treats those as unequal even when the instants match, so the test would fail on any machine whose time.Local differs from the runner's environment, or whenever the parsed location's pointer diverged from Local's. Switch to time.Time.Equal, which compares instants ignoring location, and emit a clear diff message so future failures are debuggable. ci(workflows): bump golangci-lint version to 'latest' v1.61.0 was built with Go 1.23, which fails to load any go.mod that targets Go 1.26 (schooltools targets 1.26). Pinning to 'latest' keeps the lint job runnable as new Go versions are adopted. The repo's go.mod still pins Go 1.26 explicitly; the linter just needs to be built with a recent Go toolchain.
golangci-lint-action's 'latest' only resolves to the latest v1.x release (v1.64.8, built with Go 1.24), which fails to load a project that targets Go 1.26: Error: can't load config: the Go language version (go1.24) used to build golangci-lint is lower than the targeted Go version (1.26) The actual upstream latest is golangci-lint v2.13.2 (Aug 2026), built with a recent Go toolchain and compatible with Go 1.26. Pin directly to v2.13.2. There is no .golangci.yml in the repo, so the default config is used and the v1 → v2 schema change does not require any config edits. If v2.13.2 ever falls behind, bumping to the latest v2.x patch is safe — minor versions are config-compatible.
v6 rejects golangci-lint v2 with: invalid version string 'v2.13.2', golangci-lint v2 is not supported by golangci-lint-action v6, you must update to golangci-lint-action v7 v9 is the current major release (June 2026) and supports both golangci-lint v1 and v2.
golangci-lint v2.13.2 (run via golangci-lint-action@v9) produced 32
findings on a Go 1.26 codebase. The CI lint job was failing on
errcheck / staticcheck / unused / ineffassign. This commit makes
`golangci-lint run ./...` return 0 issues without changing
runtime behaviour or removing live API.
errcheck (deferred or returned-error values not checked):
* Replace bare `defer f.Close()` / `defer res.Body.Close()` /
`defer first.Release()` with a small anonymous wrapper so the
error is silently discarded at the right scope — failures at the
end of a function are tolerated by convention; failures earlier
in the function are already returned.
* Suppress `fmt.Fprint*` errors on user-facing output writers
with `_, _ =`. These errors can only occur if stdout/stderr is
closed, which is not something a CLI caller can recover from.
* Suppress `rawConn.Close()` errors in internal/httpclient on
TLS-handshake failure paths (an error is already being
returned).
* Suppress `go p.prog.Run()` error in internal/ui via a
goroutine wrapper.
ineffassign:
* cmd/archive.go: `out := table.Render(...)` followed by
conditional `out = table.Render(...)` produced an assignment
…updates (#10) Bumps the github-actions group with 2 updates in the / directory: [github.com/spf13/cobra](https://github.com/spf13/cobra) and [golang.org/x/net](https://github.com/golang/net). Updates `github.com/spf13/cobra` from 1.9.1 to 1.10.2 - [Release notes](https://github.com/spf13/cobra/releases) - [Commits](spf13/cobra@v1.9.1...v1.10.2) Updates `golang.org/x/net` from 0.58.0 to 0.59.0 - [Commits](golang/net@v0.58.0...v0.59.0) Updates `golang.org/x/term` from 0.45.0 to 0.46.0 - [Commits](golang/term@v0.45.0...v0.46.0) --- updated-dependencies: - dependency-name: github.com/spf13/cobra dependency-version: 1.10.2 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: github-actions - dependency-name: golang.org/x/net dependency-version: 0.59.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: github-actions - dependency-name: golang.org/x/term dependency-version: 0.46.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: github-actions ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/download-artifact](https://github.com/actions/download-artifact) from 4 to 8. - [Release notes](https://github.com/actions/download-artifact/releases) - [Commits](actions/download-artifact@v4...v8) --- updated-dependencies: - dependency-name: actions/download-artifact dependency-version: '8' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/checkout](https://github.com/actions/checkout) from 4 to 7. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v4...v7) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/setup-go](https://github.com/actions/setup-go) from 5 to 7. - [Release notes](https://github.com/actions/setup-go/releases) - [Commits](actions/setup-go@v5...v7) --- updated-dependencies: - dependency-name: actions/setup-go dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 4 to 7. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](actions/upload-artifact@v4...v7) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [softprops/action-gh-release](https://github.com/softprops/action-gh-release) from 2 to 3. - [Release notes](https://github.com/softprops/action-gh-release/releases) - [Changelog](https://github.com/softprops/action-gh-release/blob/master/CHANGELOG.md) - [Commits](softprops/action-gh-release@v2...v3) --- updated-dependencies: - dependency-name: softprops/action-gh-release dependency-version: '3' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
… prune (#12) * feat(archive): redesign archive CLI around user jobs; fix destructive prune Implements the archive UX redesign documented in ARCHIVE_UX_PLAN.md and resolves the P0 prune --dry-run bug, plus several related correctness and UX issues uncovered while implementing it. P0 — destructive commands now print a plan first - 'archive prune' is plan-only by default; --delete applies. The old top-level 'prune --dry-run' (and 'cmd/prune.go' entirely) was documented as a preview but actually called archive.Prune and removed files. New: archive.PrunePlan (read-only) and archive.Prune (destructive, gated on the explicit --delete flag). - 'archive update --dry-run' replaces the old --diff flag. Diff no longer calls SaveIndex or creates directories — it is a pure in-memory preview. The TOC persistence that lived in Diff (KNOWN_ ISSUES #2) is superseded; TOC timestamps now land only after a real update pass. P0 — prune now keeps body blobs and trims dangling index records - collectLiveKeys (the new live-set builder) walks every courses/<id>/ index.json on disk and unions current metadata UUIDs with current body SHAs. The previous collectLiveUUIDs only collected metadata UUIDs and never matched .bin blob names, so every body blob was deleted by any prune run — including the live CurrentBody of every File topic. - trimDanglingRecords rewrites every per-course index in the same pass: versions and bodyVersions entries that point at deleted blobs are dropped. SchemaVersion bumps 3 → 4 so older binaries can detect the new shape. P1 — update converges - Body downloads now run for: (a) new/modified File topics, (b) File topics with no CurrentBody, (c) File topics whose CurrentBody blob is missing on disk. --no-bodies opts out (metadata only). When the queued reason is 'body-missing' (cases b/c) the body is always re-saved even if the new SHA matches the stored one, so a deleted blob is restored. Command surface (per ARCHIVE_UX_PLAN.md §2) - 'archive' (bare) is now status (counts, freshness, missing bodies, lock state, 'try next' hints); empty state suggests 'archive update'. - 'archive update [courseId...]' is the new mutating pass (was bare 'archive' + positional). Scope via positional args or repeatable --course. - 'archive find [query...]' replaces 'archive files': single-load in-memory search over the global index + every course index with precomputed lowercase haystacks, AND-token matching, --ext/--type/ --with-bodies/--missing-bodies filters. - 'archive show <ref>' replaces 'archive topic'. <ref> auto-detects a numeric topicId, 32-hex metadata UUID, or 64-hex body SHA-256; --kind forces metadata|body when ambiguous. - 'archive cat <ref>' merges 'archive read' + 'archive body': bytes to stdout for File topics, pretty JSON for everything else, --meta forces the metadata blob. - 'archive path <ref>' keeps the old auto-detection; new --kind flag is shared with show/cat. - 'archive export [ref...] --out DIR [--flat] [--dry-run]' copies file bodies out as <CourseCode>/<Sanitized Title>.<ext> with collision-safe ' (2)' suffixes. - 'archive prune [--delete] [--wait]' replaces the top-level prune. - 'archive verify [--deep]' checks pointer resolution, dangling version records, missing body blobs, per-course index parse, missing course directories, schema-version compatibility, and orphan blobs. Exits 0/1; JSON envelope. Consistency - Persistent flags on the 'archive' group: --dir, --json, --plain, --quiet/-q, --no-spinner, -v/--verbose, --course (repeatable). The -e short flag is gone. - JSON envelopes everywhere (collections {count, items}; records a single object; summaries flat). --plain TSV on list, find, show, export --dry-run for fzf/cut/xargs pipelines. - Errors print exactly once: rootCmd.SilenceErrors = true and Execute prints a single line. JSON-mode commands emit {'error': …} via a typed errJSONShown sentinel that Execute detects and does not re-print. - systemd service template now invokes 'archive update --no-auto-refresh' instead of the bare mutating 'archive'; the 'no silent reauth' guarantee is preserved via --no-auto-refresh. Optimized local search - 'internal/archive.Store' loads the global index plus every per-course index exactly once and serves O(1) topic / metadata-uuid / body-sha lookups. archive find/show/cat/path/status/verify/export all route through it, so repeated queries never re-read per-course indexes. Topic haystacks (lowercased title + url + type) are precomputed at Open time so per-query work is just token Contains checks. Also included - Prerequisite fixes for KNOWN_ISSUES #1–#20 that were already in the working tree (newscripts --body-format, content --timeout, doctor token expiry, archive list counts, archive read defaults, archive topic reverse lookup, archive files enumeration, archive path --kind, etc.). These are landed here as the dependency layer the new command surface is built on. - Closes KNOWN_ISSUES #4, #5, #6, #17, #18, #20 and supersedes the old 'archive --diff persists TOC' fix for #2 (now: update --dry-run writes nothing; real update persists TOC). - Test coverage: archive_ux_test.go covers prune plan (no writes), body blob preservation, dangling record trimming, shared body safety, ref grammar, store search, verify, export (incl. dry-run), LoadStatus missing-body counts, and the two body-convergence cases (missing field + missing blob). go build ./..., go vet ./..., go test -race ./... all clean. New code is gofmt-clean. Backwards compatibility with the old archive surface is explicitly out of scope (per the plan). * fix(archive): address CodeRabbit review findings on archive-UX redesign Resolves the actionable review comments left on PR #12: - archive lock: truncate PID file in Release() before LOCK_UN and drop the post-flock PID-alive check, since a successful flock already proves exclusive access. The old check spuriously rejected callers whenever the recorded PID happened to still be alive (PID reuse), making --wait fail immediately after the previous holder released. - archive cat --meta: require a topic reference and read the metadata UUID from the topic's Current pointer, not the resolved body SHA. - archive cat body: drop the topic-reference and !archiveCatMeta guards so body blobs load for any ref that resolves to a body kind. - archive find TSV: replace tabwriter with tsvWriteLine that sanitises embedded tabs/newlines (tabwriter with tabwidth=0 ate separators; a positive tabwidth emitted alignment tabs that broke | cut -f1). - archive list TSV: same tabwriter -> tsvWriteLine swap. - archive show TSV: same swap. - archive export TSV: same swap. - archive list help: point users at bare `archive` for freshness and missing-body status; drop the unregistered `archive status` mention. - archive export: return an error when a ref fails to resolve instead of silently dropping it; missing body blobs are recorded as skipped-missing-body in both dry-run and real runs; copyFile now propagates Close() errors so close-time write failures don't get reported as successful copies. - archive export sanitizeFilename: truncate by runes (was byte-truncating UTF-8) and reject dot-only results so CourseCode == '..' cannot escape the --out directory. - archive update --dry-run: run before acquireLock so dry runs neither create nor hold the archive lock; pass DownloadFiles: !archiveUpdateNoBodies so body counts match a real update pass. - archive update: stamp idx.Version = SchemaVersion on every successful SaveIndex so existing v3 stores converge to v4 and verify no longer flags them as schema-version-older indefinitely. - archive prune: same SchemaVersion bump after trimDanglingRecords. - archive find --missing-bodies: skip non-File topics so the count matches LoadStatus. - archive update body convergence: when reason == 'body-missing' and the fetched SHA equals the stored CurrentBody, refresh on-disk without appending a duplicate BodyVersions entry. - Locked status probe (loadStatus.lockHeldAt): read the lock file read-only instead of opening archive.lock and creating its parent directory just to read state. - news list: register shared flags via registerNewsFlags on every subcommand (cobra AddFlagSet on a different command does not rebind package variables, so --course silently dropped before). - news list cross-course: track whether at least one per-course fetch succeeded and return an error if every fetch failed. - news list until: filter results locally with withinRange() because D2L's `until` parameter is not honoured on every endpoint. - news list OrgUnitId / HasAttachment: derive from the requested course ID and from len(n.Attachments) > 0 respectively, since the D2L schema does not include those fields on the news response. - token --refresh --json: emit only the token record as JSON and skip the human-readable line. - token --refresh: treat an empty cookiejar the same as a missing session.json, so the saved jar no longer pretends to be a working session. - doctor: separate SAML-credential validity from Brightspace-token validity; `token --refresh` is only suggested when both the session is valid and a RefreshToken is recorded. - Execute() error printing: also suppress on errors.Is(err, errJSONShown) so JSON-mode commands that emit their own envelope don't get a duplicate 'Error: ... shown in JSON envelope' line on stderr. - content --tree --depth N: prune modules by depth before rendering so the tree stops at N; url.Parse(...).Path is used for the file extension label so query strings and fragments don't leak into [File.<ext>]. - systemd Query: detect pre-redesign ExecStart lines and append [LEGACY - run `schooltools systemd install --force`] to the unit path so the migration warning is visible. - tests: TestRepro_Issue20_BodyAndMetaPathDistinct now actually asserts Resolve("42", "") returns Kind=body and the body SHA, and Resolve("42", "metadata") returns Kind=metadata and the saved metadata UUID. TestRepro_Issue4 replaces t.Skip with t.Fatalf when BodiesFetched == 0, so a missing-body regression fails the test instead of being silently skipped. Lint clean (golangci-lint v2.13.2). go build / go vet / go test -race all clean. * fix(archive): address follow-up CodeRabbit review findings - content --depth: reject negative values with an explicit error instead of silently treating them as 'no limit'. - news list withinRange: compare parsed timestamps (RFC3339Nano, RFC3339) so timestamps with fractional seconds or non-Z offsets filter correctly; fall back to string compare only on unparseable values. - news list flattenBody: HTML-only items no longer leak raw HTML into the default text-body field; the text field is empty and the user can pass --body-format html/both to see the HTML. - token --refresh: probe the saved jar against /d2l/home and the OAuth token endpoint, which is where the session cookies are actually scoped. The previous probe against ua.D2LBase / ua.LoginEndpoint reported a populated jar as empty for some saved sessions. - archive prune: return a contextual error when LoadIndex fails after the apply pass (and after the missing-blobs-dir branch), so a failed read no longer leaves the schema-version bump un-done in silence. The missing-blobs-dir branch now also bumps idx.Version, so the schema converges to v4 on stores that never created a blobs dir. - archive export: uniqueDest now seeds its collision map from disk (os.Stat), and copyFile opens with O_EXCL, so an existing file at the chosen destination is treated as a collision instead of being silently overwritten. - systemd Query: move the LEGACY warning out of Status.UnitPath into a new Status.Warnings []string field. UnitPath is now always a real filesystem path. legacyExecStart parses ExecStart= lines explicitly (skipping comments and unrelated directives) so a mention of 'archive update' in a Description= line cannot suppress the warning. Lint clean (golangci-lint v2.13.2). go build / go vet / go test -race all clean. * fix(archive): cancel TOC HTTP on timeout; paginate manageCourses; fix nil-error wrap - cmd/content: fetchTocWithTimeout now passes a deadline-aware context through a new content.FetchTocWithContext helper, which in turn propagates it via httpclient.FetchOptions.Context so the in-flight HTTP request is cancelled when the timeout fires. The goroutine no longer keeps running after the deadline; the call returns the timeout error directly. httpclient.Fetch uses http.NewRequestWithContext when FetchOptions.Context is set, and falls back to context.Background for the existing callers. - internal/dupdata.CourseOrgIDsCSV: bumped pageSize from 20 to 200 and added bookmark-pagination handling for the manageCourses widget. The function now follows PagingInfo.HasMoreItems chains (capped at 25 pages) so a student with more than 20 enrollments sees every course. The first page already includes the entire bookmark URL, which we extract for the next request. - cmd/archive: the res.Ref.Topic == nil branch in runArchiveShow was using fmt.Errorf("%w (try: ...)", err) with an err that was guaranteed nil by the preceding Resolve success check, producing %!w(<nil>) in the output. The branch now formats res.Kind, res.BlobID and res.BlobPath directly into the error message. - CHANGELOG: removed contradictions between the [Unreleased] section and the rest of the document (mentions of removed commands, KNOWN_ISSUES #4 vs #5/#20, the systemd LEGACY suffix that lived on Status.UnitPath). Lint clean (golangci-lint v2.13.2). go build / go vet / go test -race all clean. * fix(archive): address re-review findings (HTML-only body, date validation, export collisions, bookmark pagination, systemd warnings) - cmd/per_course_rare flattenBody: HTML-only bodies now go through a conservative htmlToText conversion (tag strip, whitespace collapse, common-entity decode) so the default 'text' body-format keeps a readable representation instead of dropping the content entirely. - cmd/per_course_rare runNewsList: new validateNewsBounds step rejects malformed --since / --until up front so withinRange never silently includes out-of-range items behind a successful result. - internal/archive/export uniqueDest: now stat-checks each suffixed candidate against the filesystem until it finds an unused name, so a leftover 'report (2).pdf' from an earlier run no longer collides with copyFile's O_EXCL open and aborts the whole export partway through. - internal/dupdata CourseOrgIDsCSV: the bookmark walker now uses the PagingInfo.Bookmark returned by the manageCourses response instead of trying to read it off the previous URL (the widget doesn't echo it there). An unchanged bookmark is rejected, so a stuck cursor is caught immediately instead of failing at page 25. - cmd/systemd printSystemdStatus: Status.Warnings is now rendered as a separate 'Warnings:' block in human output and as a 'warnings' field in the JSON envelope, so the LEGACY ExecStart warning reaches the user. Lint clean (golangci-lint v2.13.2). go build / go vet / go test -race all clean. * fix(archive): HTML entity decoding; export collision reservations; bookmark validation - cmd/per_course_rare htmlToText: pass the result through html.UnescapeString after the existing manual replacements so standard named and numeric entities (\u0026amp; \u0026lt; \u0026#39; etc.) decode to their characters in the default text body instead of appearing as raw escape sequences. - internal/archive uniqueDest: keep a dedicated reserved set of every destination path it has returned in the current export run, and refuse to hand out a path already in that set. Previously a dry run could pick "report (2).pdf" for a topic, and a later topic titled "Report (2)" could then also pick "report (2).pdf" since os.Stat couldn't see the dry-run reservation. The function also now returns unexpected Stat errors (anything other than NotExist) to the caller so a permission problem can't silently loop the suffix counter. - internal/dupdata fetchManageCoursesPage: when PagingInfo reports HasMoreItems=true with an empty Bookmark, return an error instead of silently treating the response as the last page. A malformed pagination response previously truncated the course list without surfacing the problem. Lint clean (golangci-lint v2.13.2). go build / go vet / go test -race all clean. * fix(archive): treat dangling symlinks as occupied export destinations uniqueDest used os.Stat, which reports a dangling symlink's target as absent, so the suffix search picked that filename and copyFile's O_CREATE|O_EXCL open then aborted the export. Switch the destination check to os.Lstat so dangling symlinks bump the suffix like any other occupied destination. Add a unit test that plants a dangling symlink at the candidate filename and asserts the suffix advances.
LabelsThe following labels could not be found: Please fix the above issues or remove invalid values from |
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ping, credential-leak fix `archive update` aborted the entire pass on the first broken topic, so a single dead 2022 dropbox link blocked every course queued after it. Status trusted the index instead of the filesystem, D2L URLs with unescaped path characters were silently broken, and the login debug dump printed SAML assertions and raw cookie values to stderr (and into journald) on every session auto-refresh. **Resilience** - Topics D2L flags broken are skipped without an API call; topic fetch failures are counted and retried on the next pass. - 401/403 and login-page redirects stop the run with `session expired; run 'schooltools login'` instead of raw status lines, and a login-page response is never archived as topic metadata or as a file body. - A burst of consecutive transport failures aborts the pass instead of one doomed request per topic; the abort persists the accumulated index so earlier blobs keep a reference, and a modified topic whose body fails clears its pointer so the next pass retries it. **Honest body state** - `LoadStatus`, `FileTopics` and `bodyNeeded` stat the blobs and treat only not-exist as missing; `find --with-bodies` / `--missing-bodies` and `HasBody` reflect on-disk reality. - `archive verify` reports a present-but-unstattable blob as `unreadable-*-blob` instead of passing it. **URLs** - New `internal/urlutil` percent-encodes D2L content URLs (spaces, a bare `%`, `#`, `?`) for relative and absolute references, routing both `archive update` body downloads and `download` through it. **Security** - The login HTTP debug dump is gated behind `--verbose`, every verbose URL print redacts SAML parameters, cookie values are redacted entirely, and `fetchPost` no longer dumps on auto-refresh. **systemd** - `systemd install` writes `archive update` in `ExecStart`, warnings travel via `Status.Warnings`, `Uninstall` reports real removal errors, and `list-timers` parsing anchors on the trailing unit columns with `-`/`n/a` placeholders and multi-token `LEFT` handled. CHANGELOG.md documents every user-visible change under `[Unreleased]`.
Cuts [Unreleased] into a dated 0.3.0 section and bumps the CLI version string to match. Contents: the archive-UX redesign (archive update / diff / find / show / cat / export around user jobs, plus the non-destructive prune) and the robustness round that followed — broken-topic tolerance, honest on-disk body state, D2L URL escaping, session-expiry handling, and the verbose-only login debug dump with SAML/cookie redaction.
## Why
`archive update` fetched D2L as fast as responses came back. The pass is
strictly sequential, but there was no floor on the gap between requests and
no retry at all. Measured against a stub on a 60-topic cold pass:
```
avg gap 674µs min gap 308µs implied ~1400 req/s
```
Nothing handled a 429 or a 5xx. The incremental gate already keeps
steady-state passes cheap (a warm pass costs 1 roster call + 1 TOC per
course and zero content traffic), so the exposure was a cold archive or a
bulk re-sync.
## What
**Jitter-first pacing.** Each gap is a random duration in `[N, 2N)` rather
than a fixed `N`. A fixed floor only trades an unthrottled stream for an
evenly-spaced one, which is no harder to fingerprint. Default `N` is 50ms.
```
gap min=51.6ms avg=73.1ms max=102.1ms over 59 gaps
distinct gap values: 59 of 59
```
Every gap differs — no lockstep pattern to detect. That's a ~106x rate
reduction (1400 → 13.2 req/s) while staying liberal. The floor is measured
from the **previous request's start**, so a response that already took
200ms satisfies it on its own and pacing adds no wall-clock.
**Retries for 429 and 5xx only** — up to 3 attempts, exponential backoff
with jitter, honoring `Retry-After` (delta-seconds and HTTP-date), capped
at 60s. `--request-interval` tunes the floor; `0` disables it. Retries are
independent of that flag, so disabling pacing still backs off on failure.
## Design call: transport errors are not retried
Retrying them broke `TestRunAbortsAfterConsecutiveTransportFailures` (15
attempts against the existing bound of 5), and the test was right — retrying
per-topic on a dead network is precisely what
`maxConsecutiveTransportFailures` exists to prevent. So `retryableError`
returns false for transport errors and leaves them to that guard. Not a
regression: they were never retried before either. 404/400 and expired
sessions stay unretryable too.
## Also fixed
- `fetchTocFor` and `fetchCoursesPage` decoded response bodies without
closing them, leaking a connection per course per pass. Retries would
have multiplied that.
- `retry.WithRetry` substituted the entire `Options` struct with defaults
when `Attempts` was zero, silently discarding a caller-supplied
`DelayFor`. Defaults now apply field by field.
- A `DelayFor` hint replaced the backoff outright, so every client handed
the same `Retry-After` would wake at the same instant — recreating the
burst the backoff exists to prevent. The hint is now a floor with jitter
re-applied on top.
- Negative `--request-interval` is now rejected instead of silently
disabling pacing.
## Not addressed here
Surfaced during review, deliberately left for follow-up as they touch the
shared `internal/httpclient` rather than archive:
- `decompressBody` returns a wrapper whose `Close` doesn't propagate to the
original body (gzip/brotli), leaking compressed responses. Pre-existing,
affects all `httpclient` callers.
- Redirect hops issued by `FollowRedirects` are unpaced, so body downloads
redirecting to a CDN run ~2x the configured floor.
- `fetchTopicFor`/`downloadFileBody` classify redirect-construction errors
as transport failures, so five malformed redirects trip the network-error
abort. Also pre-existing.
## Verification
- [x] `go build ./...`
- [x] `go vet ./...`
- [x] `go test -race ./...`
- [x] CI green: build + test on ubuntu and macos, lint
- [x] Convergence unaffected — warm passes still issue zero content requests
- [x] `revive`: zero new findings vs. `main` baseline (255 pre-existing)
- [x] Test strength verified by mutation, not by inspection — stubbing
`DelayFor` to a no-op fails the suite deterministically
No new dependencies; `retry` and `httpclient` were already in-tree.
…updates Bumps the github-actions group with 3 updates in the / directory: [github.com/andybalholm/brotli](https://github.com/andybalholm/brotli), [golang.org/x/net](https://github.com/golang/net) and [golang.org/x/term](https://github.com/golang/term). Updates `github.com/andybalholm/brotli` from 1.2.3 to 1.2.6 - [Commits](andybalholm/brotli@v1.2.3...v1.2.6) Updates `golang.org/x/net` from 0.59.0 to 0.61.0 - [Commits](golang/net@v0.59.0...v0.61.0) Updates `golang.org/x/term` from 0.46.0 to 0.47.0 - [Commits](golang/term@v0.46.0...v0.47.0) --- updated-dependencies: - dependency-name: github.com/andybalholm/brotli dependency-version: 1.2.6 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: github-actions - dependency-name: golang.org/x/net dependency-version: 0.61.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: github-actions - dependency-name: golang.org/x/term dependency-version: 0.47.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: github-actions ... Signed-off-by: dependabot[bot] <[email protected]>
a14612b to
4ac9cce
Compare
|
This pull request was built based on a group rule. Closing it will not ignore any of these versions in future pull requests. To ignore these dependencies, configure ignore rules in dependabot.yml |
Bumps the github-actions group with 3 updates in the / directory: github.com/andybalholm/brotli, golang.org/x/net and golang.org/x/term.
Updates
github.com/andybalholm/brotlifrom 1.2.3 to 1.2.6Commits
451b9ebImprove Accept-Encoding negotiation3ecb1baM4: Prevent spurious matches at distance 0.112bc8bFastEncoder: explicitly reject matches less than 4 bytes3995d4aReader: clear input buffer on reset.078bdb0ZM, Trio: update history even for very short blocks9140f7eflate: make the two copies of match splitting look alikef88900bRemove test of implementation details.9933015flate: split long matches in the statistics and token loops2e61b23flate: split matches longer than 258 byteseede312Fix problems found by go vetUpdates
golang.org/x/netfrom 0.59.0 to 0.61.0Commits
c1d31dfgo.mod: update golang.org/x dependencies8456a20quic: properly check for allowed frame types in a packet18ece0chttp2: prevent crash due to HPACK encoder race2d03f9dhttp2: avoid 2x refund on stream close with buffered datacc7d34fhttp2: avoid excessive work on initial window changes57f4235http2: apply header limits to Trailer headers too01e3d03http2: coalesce dials via wrapped Transport.RoundTrip2824783http2: delete malformed framing-related headersb13c0c1internal/http3: delete HTTP/3 package520c891quic: off-path support in loss/congestion/rttUpdates
golang.org/x/termfrom 0.46.0 to 0.47.0Commits
0851034go.mod: update golang.org/x dependencies