Hacktoberfest 2026: the issues maintainers tagged for October, open and beginner-friendly. Browse Hacktoberfest issues

Follow-ups from the review of #472 (catalog journey and privacy scan)

Open
  • Is something wrong?
#478 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
38/100
Issue type
Feature
Clarity
Mostly clear
Activity status
Active
Tech stack
github, playwright, typescript
Domain
backend-api-design, testing

Research direction

The issue lists several independent follow-ups, including member-scan.ts, member-routes.ts, fakes/github, and TESTING.md; choose one before starting. Read the matching review note and inspect its named entry point or test first. Define done by satisfying that note's specific assertion or behavior, while avoiding the catalog redesign items marked as likely to be reworked.

Written by the indexing model from the issue text.

Description

area: app type: chore

Follow-ups from the review of #472 (the providers and catalog journey, #435). Nothing here blocks that PR; the one finding that did (P2-2, old runs still name the provider in their log) is fixed in #472 itself.

Heads-up: the catalog redesign will rework parts of this

A redesign is coming: the curated model list lives in code, per provider, and admins only restrict (allow or disallow) models instead of adding free-form ones. That reworks:

  • T01 (the "Add model" dialog, "Check model", the free-form Model id, draft/released entries, registerModels),
  • T04 (retire becomes restrict),
  • the sentinel ids (e2eproof/m-…:beta): with ids in code the forbidden set becomes the real ids of the registry, and a test-only model list needs a registration path that does not ship in the production build this lane runs,
  • the draft-lifecycle assertions and adminCatalog(), which assume entries carry a modelId,
  • SHORTEST_PART = 5 in member-scan.ts: with real ids a bare maker below five characters (xai, meta) would not be searched alone (the whole id still is).

What survives as is: the whole privacy scan (member-scan.ts, member-surfaces.ts, member-routes.ts, member-scan-proof.ts, the recorder, the planted leaks, the signed-in checks), the masks, T05, the tamper test (draft/retired/unknown become restricted/unknown), the own-key path, the usage and log checks. So do the items below with that in mind: prefer the ones that live in the scan and the lane, and do not invest in the T01 helpers.

From the round-2 review

  • P2-1 More pull request surfaces in the scan. allComments reads the issue comments only. Also read the check runs (GET /__fake/check-runs?repository=) and the review verdict (GET /__fake/reviews?repository=&number=) of every opened pull request, labelled "a check run of …" and "a review of …". They are built from error codes today (check-text.ts), so they are clean; the scan should catch a future change to describeCheck or verdictBody.
  • P2-3 Say which number the smoke budget measures. TESTING.md §1 says a smoke run over 5 minutes is a defect. The Playwright step of the smoke job was 4.2 min and the CI job "E2E: browser journeys" was 5m05 after #472 (4m23 before). Write down which one is the budget; if it is the job there is no headroom, so also win back time (see the per-installation setup address below).
  • Per-installation setup address in the fake GitHub. fakes/github has one global setup address for the one registered app, so connect, catalog and operator must never run at once (hence catalog depends on connect and operator on catalog; a red catalog skips operator). Make the setup address per installation or per app. Then no dependencies chain is needed between them, connect's 4 non-smoke tests (about 26 s) stop being pulled into every smoke run, and an unrelated red journey no longer skips another. This changes a fake shared by all suites.
  • P3-1 The tamper test checks the status class, not the reason. Assert the error code of the refusals (MODEL_MANAGED_BY_INSTANCE, the entry codes), and add one positive control with the same request envelope (a released entry is accepted).
  • P3-2 The admin canary passes on any needle. The provider label is on /admin/llm in every way of reading it, so a collector that never sees the model-id table still passes. Require a finding labelled "the model …" for each way.
  • P3-3 The route enumeration has a regex blind spot. member-routes.ts looks for export (async )?function GET or const GET; a route.ts that re-exports (export { GET } from …) is skipped silently. Count the route.ts files in the roots and compare.
  • P3-4 A managed workspace loses the removal confirmation. After removing a key stored earlier, router.refresh() drops the card and with it "The key is removed." (the component was designed to survive that, #178 M1). A toast or a status held in the parent would fix it.
  • P3-5 Over-masking and the ADR citation. The provider mask is a case-insensitive substring match, so a managed workspace that reviews a repository that mentions the provider reads [provider] in its own log (same trade-off as [model]). The comments cite "ADR 0005, 2.17" while the ADR is still Proposed in #422: land #422 or cite the issue.
  • Notes for TESTING.md. recordNetwork buffers whole bodies, so the Owner's and the Member's UI in that lane sees no progressive streaming (no text/event-stream in the app, so nothing hangs). It also sends requests from Node, without Sec-Fetch-* headers; isCrossSite falls back to Origin.

Also noted (no action decided)

  • The 7 non-smoke catalog tests had no CI run before the merge of #472: watch the first full-lane run on development. There is no workflow_dispatch on ci.yml.
  • PR-Agent posts its own comments before the app sees a result, so the app cannot mask them; the journey scans the comments of the two error shapes it triggers, which post none. Another PR-Agent error shape that posts a comment is not covered.
Dominant language
TypeScript
Stars
0
Forks
1
Avg merge
1h 20m
Merged PRs (30d)
309

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from ExpidetGames/code-reviewer

All issues in ExpidetGames/code-reviewer

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.