~ knotless Search/
↑↓ move↵ open⇧↵ all results

chadtmiller.com/knotless

git that lives in your Atmosphere account

knotless.dev
1 collaborator

knotless 0.13.0 Latest 2 packages, 1 image

1
10

getTemplate reads the template from the mirror, as getFile does

Merged
Merged chadtmiller.com asked to merge into main from chadtmiller.com/knotless:serve/template-from-mirror

What changes

dev.knotless.getTemplate reads .knotless/<kind>.md from serve's mirror of the repository, as getFile does, rather than opening the repository from its PDS and syncing a mirror of its own for each request. What it answers doesn't change: the template, {} for a branch that isn't there or a repository with no commits, and 400 for a kind that is neither pull nor issue, now checked before the repository is looked up.

Why

On prod, getTemplate was the slowest read route that's consistently slow. Over the day to 2026-11-01 17:00 UTC it averaged 1,492 ms over 46 calls, with 44 of them over 500 ms, while getFile, which reads the same mirror, averaged 26 ms. The new pull and new issue pages wait on it.

How it was checked

The existing tests cover it: gateway/opening.rs (issue and pull templates, a missing base, a bad kind), the private repository test in gateway.rs, and pages.rs. CI runs them. The after number will come from knotless admin stats once this is deployed.

Shortcomings

An error reading the mirror is answered as no template rather than a failure, as a missing file is.

🤖 Generated with Claude Code

Approved

This switches getTemplate to read .knotless/<kind>.md from the serve mirror with repo.objects()?.blob(...), the way getFile does, and checks kind before the repository lookup. I found nothing that must change.

  • Same answers as before: the revision is repo.head or refs/heads/<base>, as forge::template chose it. A missing branch, a missing file or an empty repository gives {}. A bad kind still gives 400.
  • Private repositories: found(&app, ...) is unchanged, so the access check is the same.
  • Docs: docs/reference/api.md and docs/design/serve.md already describe the method in terms that still hold. No change file is needed, since the change is in serve only.
  • Swallowed errors: .ok() on the blob read also hides a real mirror failure, not only a missing file. You list this as a shortcoming, and getFile reads the same mirror, so it is acceptable. If you want to keep the distinction later, not_found from getFile would let other errors through.
  • Checks: check, test and e2e were still queued or running when I looked. The other four were skipped as not applicable. Wait for them before queueing the merge.
Approved

The new commit changes only the private repository test. getTemplate no longer opens the repository as the member, so the test now reads listWorkflows as the member instead. That path still goes through the kept credential, and the earlier test failure at aa6ca6b should be fixed by it. My earlier approval stands, and I have no new findings on the handler.

  • Checks: test, check and e2e were still queued or running when I looked, and the other four were skipped as not applicable. The queue ejected the pull on the old head's test failure, so queue it again once test passes.
  • Nit: in tests/gateway.rs the line "As is its template, which the new issue page asks for." now sits above the workflows comment, and the template assertion it describes comes after the workflows. Moving it down next to that assertion would fix it.
crates/knotless-serve/tests/gateway.rsline 2738Outdated
let issues = xrpc("listItems", &format!("{}&kind=issue", repo("hidden")));
let issues: Value = serde_json::from_str(&looked(&issues, theirs).3).unwrap();
assert_eq!(issues["items"][0]["title"], "Private trouble");
// As is its template, which the new issue page asks for.
review.knotless.dev

This comment describes the template assertion, but it now sits above the workflows comment. Move it down to just before getTemplate.

Approved

The new commit changes only tests. In gateway.rs it drops the listWorkflows assertion from the private pages test, and the stray comment line from my last round is gone with it. In pages.rs the delegation-token test now reads listWorkflows as the member, which still goes through the kept credential. It then reads getTemplate separately, with a comment saying it comes from the mirror.

The handler is unchanged, so my earlier approval stands.

  • Comment nit: In gateway.rs, "As is its template, which the new issue page asks for." now sits directly above the template assertion, which is where it belongs.
  • Dropped workflows assertion: The listWorkflows check no longer appears in the private pages test. pages.rs still covers that path, so I don't think you need it back.
  • Checks: check passed. test and e2e were still running or queued when I looked, and the other four were skipped as not applicable. The pull is already in the merge queue, which will wait on test.
Merged as 92b3b88