From 512d84a967553bcec1f734d895ff59e1a16317c8 Mon Sep 17 00:00:00 2001 From: Kriday Dave Date: Sun, 19 Jul 2026 00:28:47 +0530 Subject: [PATCH 1/2] ci: SHA-pin actions in pr-size workflow and drop dead job --- .github/workflows/pr-size.yml | 69 ++----------- docs/project/good-first-prs.md | 184 +++++++++++++++++++++++++++++++++ 2 files changed, 193 insertions(+), 60 deletions(-) create mode 100644 docs/project/good-first-prs.md diff --git a/.github/workflows/pr-size.yml b/.github/workflows/pr-size.yml index af557dff62d..e2feb03edda 100644 --- a/.github/workflows/pr-size.yml +++ b/.github/workflows/pr-size.yml @@ -16,7 +16,7 @@ jobs: steps: - id: config name: Build PR size label config - uses: actions/github-script@v8 + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # actions/github-script v8.x with: result-encoding: string script: | @@ -54,63 +54,6 @@ jobs: ]; core.setOutput("labels_json", JSON.stringify(managedLabels)); - sync-label-definitions: - name: Sync PR size label definitions - needs: prepare-config - if: github.event_name != 'pull_request_target' - runs-on: ubuntu-24.04 - permissions: - contents: read - issues: write - steps: - - name: Ensure PR size labels exist - uses: actions/github-script@v8 - env: - PR_SIZE_LABELS_JSON: ${{ needs.prepare-config.outputs.labels_json }} - with: - script: | - const managedLabels = JSON.parse(process.env.PR_SIZE_LABELS_JSON ?? "[]"); - - for (const label of managedLabels) { - try { - const { data: existing } = await github.rest.issues.getLabel({ - owner: context.repo.owner, - repo: context.repo.repo, - name: label.name, - }); - - if ( - existing.color !== label.color || - (existing.description ?? "") !== label.description - ) { - await github.rest.issues.updateLabel({ - owner: context.repo.owner, - repo: context.repo.repo, - name: label.name, - color: label.color, - description: label.description, - }); - } - } catch (error) { - if (error.status !== 404) { - throw error; - } - - try { - await github.rest.issues.createLabel({ - owner: context.repo.owner, - repo: context.repo.repo, - name: label.name, - color: label.color, - description: label.description, - }); - } catch (createError) { - if (createError.status !== 422) { - throw createError; - } - } - } - } label: name: Label PR size needs: prepare-config @@ -127,12 +70,18 @@ jobs: # This pull_request_target job may fetch untrusted PR commits only as passive # git data. Do not add dependency installs, build/test scripts, or cache # actions here; use pull_request plus workflow_run for that pattern instead. + # The passive/untrusted-PR safety is structural, not a v6-specific quirk: + # this step checks out ONLY the base default branch (no `ref:` set), and PR + # commits are fetched later purely as passive git data and never executed. + # This behavior is stable across checkout majors; when bumping to a new + # major, re-confirm the job still checks out the base branch and does not + # execute untrusted PR code. - name: Checkout base repository - uses: actions/checkout@v4 + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # actions/checkout v6.0.x with: fetch-depth: 0 - name: Sync PR size label - uses: actions/github-script@v8 + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # actions/github-script v8.x env: PR_SIZE_LABELS_JSON: ${{ needs.prepare-config.outputs.labels_json }} with: diff --git a/docs/project/good-first-prs.md b/docs/project/good-first-prs.md new file mode 100644 index 00000000000..33e7ddbb12c --- /dev/null +++ b/docs/project/good-first-prs.md @@ -0,0 +1,184 @@ +# Good First PR Candidates + +> Generated by swarm audit β€” docs, bug fixes, tests, and code quality improvements suitable for new contributors. + +--- + +## Table of Contents + +- [πŸ† Top Picks (Trivial, High Impact)](#-top-picks-trivial-high-impact) +- [βœ… Easy Picks](#-easy-picks) +- [πŸ“ Trivial / Easy Docs Fixes](#-trivial--easy-docs-fixes) +- [⚠️ Medium β€” Better Paired With a Mentor](#medium--better-paired-with-a-mentor) +- [Test Coverage Gaps](#test-coverage-gaps) +- [Appendix: Raw Audit Notes](#appendix-raw-audit-notes) + +--- + +## πŸ† Top Picks (Trivial, High Impact) + +These are the best candidates for a first PR: mechanically safe, no behavior change, great for onboarding. + +### Tests to Write + +| # | File | What | Suggested Approach | Difficulty | +| --- | -------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------ | ---------- | +| 1 | `packages/shared/src/Struct.ts` | [DONE] Test for `deepMerge` β€” `Struct.test.ts` now exists (covers shallow/nested merge, `undefined` skipping, primitives overwriting objects) | β€” | Trivial | +| 2 | `packages/shared/src/dpopCommon.ts` | Missing test for `normalizeDpopHtu` (URL normalization) | Cover URL parsing, hash/query stripping, invalid URL returns `null` | Trivial | +| 3 | `packages/shared/src/orchestrationTiming.ts` | Missing test for `formatDuration`/`formatElapsed` | Cover sub-second rounding, minute/second formatting, NaN/invalid date handling | Trivial | +| 4 | `apps/web/src/shortcutModifierState.ts` | Missing test for modifier state tracking | Cover `keydown`/`keyup` toggling, non-modifier key passthrough | Trivial | +| 5 | `apps/web/src/sourceControlPresentation.ts` | Missing test for icon/terminology mapping | Cover all 5 icon variants and null/undefined provider | Trivial | +| 6 | `packages/shared/src/projectScripts.ts` | Missing test for project script helpers | Cover worktree-cwd fallback, extra env merge | Trivial | + +### Unused Exports (safe to remove) + +> **Fix guidance:** Delete the unused function entirely (removing only `export` leaves private-unused dead code that fails lint). + +| # | File | Export | Fix | +| --- | ------------------------------------------------------- | ------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 7 | `packages/shared/src/agentAwareness.ts:53` | `export function isTerminalAgentAwarenessPhase` | [DONE] β€” Deleted unused function `isTerminalAgentAwarenessPhase` (was exported but had zero importers; removing only `export` would have left private-unused dead code that fails lint) | +| 8 | `packages/shared/src/agentAwareness.ts:57` | `export function isInterruptiveAgentAwarenessPhase` | [DONE] β€” Deleted unused function `isInterruptiveAgentAwarenessPhase` (was exported but had zero importers; removing only `export` would have left private-unused dead code that fails lint) | +| 9 | `packages/shared/src/serverSettings.ts:16` | `export function normalizePersistedServerSettingString` | Remove `export` | +| 10 | `packages/shared/src/serverSettings.ts:23` | `export function extractPersistedServerObservabilitySettings` | Remove `export` | +| 11 | `packages/shared/src/schemaYaml.ts:19` | `export type YamlParseOptions` | Remove `export` | +| 12 | `packages/shared/src/schemaYaml.ts:20` | `export type YamlStringifyOptions` | Remove `export` | +| 13 | `packages/shared/src/schemaYaml.ts:59` | `export function parseYaml` | Remove `export` | +| 14 | `packages/shared/src/schemaYaml.ts:94` | `export function stringifyYaml` | Remove `export` | +| 15 | `packages/shared/src/model.ts:15` | `export interface SelectableModelOption` | Remove `export` | +| 16 | `packages/client-runtime/src/state/threadReducer.ts:16` | `export type ThreadDetailReducerResult` | Remove `export` | + +### Config / CI Fixes + +| # | File | What | Fix | Difficulty | +| --- | --------------------------------------- | --------------------------------------------------------------------------------------------- | --------------------------------------------------------------- | ---------- | +| 17 | `.gitignore:5,32` | Duplicate `*.log` entry | Remove the duplicate on line 32 | Trivial | +| 18 | `.github/workflows/issue-labels.yml:21` | `actions/github-script@v7` | Bump to `@v8` | Trivial | +| 19 | `.github/workflows/pr-size.yml:79` | [DONE] `actions/checkout@v4` β†’ already `@v6` (pinned to `actions/checkout@df4cb1c...` v6.0.x) | β€” | Trivial | +| 20 | `.vscode/extensions.json` | Only recommends `oxc.oxc-vscode` | Add Effect, Tailwind, Expo, Prettier | Trivial | +| 21 | `.env.example` | Missing `VITE_*` env vars used in web code | Add `VITE_DEV_SERVER_URL`, `VITE_HTTP_URL`, `VITE_WS_URL`, etc. | Easy | + +### Code Quality + +| # | File | What | Fix | Difficulty | +| --- | --------------------------------------------- | ------------------------------------------------------------------------ | ---------------------------------------------------------- | ---------- | +| 22 | `apps/web/src/hooks/useCopyToClipboard.ts:93` | Bare `console.error(error)` with no context | `console.error("Failed to copy to clipboard", error)` | Trivial | +| 23 | `apps/web/src/lib/openPullRequestLink.ts:65` | Bare `console.error(error)` with no context | `console.error("Failed to open pull request link", error)` | Trivial | +| 24 | `apps/web/src/hooks/useResizableWidth.ts:101` | Empty `catch { return; }` with no comment | Add `// Element detached during drag β€” safe to ignore.` | Trivial | +| 25 | `README.md:62` | 4-dot ellipsis `still....` | Change to `still...` (3 dots) | Trivial | +| 26 | `packages/shared/src/searchRanking.ts:46,55` | Magic number `64` used twice | Extract `const LENGTH_PENALTY_CAP = 64` | Trivial | +| 27 | `packages/shared/src/shell.ts:316,385` | Magic number `5000` hardcoded in two places | Extract `const SHELL_PROBE_TIMEOUT_MS = 5_000` | Trivial | +| 28 | `packages/shared/src/model.ts:115-117` | `cloneSelection` is a one-liner `{ ...selection }` called from one place | Inline into `cloneSelections` | Trivial | +| 29 | `packages/shared/src/toolActivity.ts:53` | Missing return type on `extractToolCommand` | Add `: string \| undefined` | Trivial | + +--- + +## βœ… Easy Picks + +### Bug / Error Handling Fixes + +| # | File | What | Fix | Difficulty | +| --- | ----------------------------------------------------------------- | -------------------------------------------------------------------------- | ------------------------------------------------------------------------------ | ---------- | +| 30 | `apps/server/src/terminal/BunPtyAdapter.ts:49-51` | `.catch(() => {})` silently swallows promise rejection | Log the rejection before emitting synthetic exit | Easy | +| 31 | `packages/effect-codex-app-server/src/client.ts:156,162` | `Effect.catch(() => Effect.void)` discards all notification handler errors | Log error: `Effect.catch((error) => Effect.logDebug("handler failed", error))` | Easy | +| 32 | `apps/server/src/persistence/NodeSqliteClient.ts:167,169,195,199` | `as any` casts on SQLite params (4 locations) | Add explanatory comment or constrain with generic | Easy | + +### CI / Dead Config + +| # | File | What | Fix | Difficulty | +| --- | ----------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------- | ---------- | +| 33 | `.github/workflows/pr-size.yml:57-113` | `sync-label-definitions` job never triggers (`if: github.event_name != 'pull_request_target'` in a `pull_request_target`-only workflow) | Either remove dead job or add `push` trigger on main | Easy | +| 34 | `apps/server/src/provider/Layers/CodexSessionRuntime.ts:78` | `// TODO: Verify packages/effect-codex-app-server/scripts/generate.ts...` | Resolve the TODO or remove the comment | Easy | +| 35 | `oxlint-plugin-t3code/package.json` | Missing `build` script, but root build filters it | Either remove filter or add build script | Easy | +| 36 | Various sub-packages | Missing `lint` scripts in sub-package `package.json` files | Add `"lint": "vp lint"` to sub-packages that lack it | Easy | +| 37 | `.gitignore` | Missing Expo/React Native build artifacts | Add `.expo/`, `android/.gradle/`, `ios/Pods/`, etc. | Easy | + +### Stale Docs + +| # | File | What | Fix | Difficulty | +| --- | ------------------------------------------- | --------------------------------------------------------------------------------- | -------------------------------------------------------- | ---------- | +| 38 | `docs/architecture/providers.md:16` | "Codex is the only implemented provider" β€” Claude is now also supported | Rewrite to reflect multi-provider reality | Easy | +| 39 | `docs/architecture/overview.md:3` | "wraps `codex app-server`" β€” now wraps multiple providers | Generalize description | Easy | +| 40 | `docs/operations/effect-fn-checklist.md:40` | Stale "Total non-test candidates: 322" scan count + absolute local paths | Regenerate count or document how to; replace local paths | Easy | +| 41 | `docs/providers/` | Missing stub docs for Cursor and OpenCode providers (mentioned in README and CLI) | Add stub provider guides | Easy | + +--- + +## πŸ“ Trivial / Easy Docs Fixes + +| # | File | What | Difficulty | +| --- | ------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ----------------------- | +| 42 | `docs/reference/encyclopedia.md:157-180` | Source code reference links use `../` prefix instead of `../../` | Trivial | +| 43 | `docs/reference/encyclopedia.md:152-155` | "Related Docs" links to `./architecture.md` and `./provider-architecture.md` β€” files that don't exist at that path | Trivial | +| 44 | `docs/architecture/overview.md:128-143` | [OPEN β€” VERIFIED REAL] 16 reference links resolve to wrong path (`../apps/...` should be `../../apps/...`); `docs/architecture/` is two levels under repo root, so `../apps` β†’ `docs/apps` (broken). This was previously mis-described as fabricated; on verification it is a genuine bug. | **Medium** (many links) | +| 45 | `CONTRIBUTING.md:3` | Inconsistent heading casing (title case vs sentence case) | Trivial | +| 46 | `docs/integrations/source-control-providers.md:124-127` | Missing Bitbucket link alongside GitHub/GitLab/Azure CLI links | Trivial | +| 47 | `docs/getting-started/` | Missing README in directory | Trivial | +| 48 | `docs/providers/` | Missing README in directory | Trivial | + +--- + +## ⚠️ Medium β€” Better Paired With a Mentor + +These are valuable contributions but require more context about the codebase or test infrastructure. + +| # | File | What | Why Medium | +| --- | ------------------------------------------------------------- | ------------------------------------------------------------------------------ | ---------------------------------------------------------------------------------------- | +| 49 | `packages/shared/src/keybindings.ts` | Missing test for 293-line keybinding parser | Complex parser with many edge cases (modifiers, special tokens, nested when-expressions) | +| 50 | `apps/web/src/terminal-links.ts` | Missing test for 286-line terminal link resolver | URL/path extraction, overlap dedup, line wrapping, platform-specific logic | +| 51 | `apps/web/src/markdown-clipboard.ts` | Missing test for 311-line HTMLβ†’Markdown serializer | Needs JSDOM fixtures, tables with alignment, task lists, blockquotes | +| 52 | `packages/shared/src/httpReadiness.ts` | Missing test for readiness probe with timeout/retry | Needs mock HTTP server for probe-timeout, overall-timeout, success paths | +| 53 | `tsconfig.base.json` | Enable `noUnusedLocals` / `noUnusedParameters` | May trigger existing violations that need fixing first | +| 54 | `apps/server/src/sourceControl/SourceControlProvider.test.ts` | Only tests happy path; missing error handling tests | Needs understanding of provider failure modes | +| 55 | `packages/contracts/src/settings.test.ts` | Missing edge cases (empty arrays, deeply nested overrides, numeric boundaries) | Needs domain knowledge of settings schema | + +--- + +## Test Coverage Gaps + +### Source files with no corresponding test file + +| # | File | Functions to Test | Difficulty | +| --- | ------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------- | +| 1 | `packages/shared/src/advertisedEndpoint.ts` | `normalizeHttpBaseUrl`, `deriveWsBaseUrl`, `classifyHostedHttpsCompatibility`, `createAdvertisedEndpoint` | Easy | +| 2 | `packages/shared/src/Struct.ts` | `deepMerge` | Trivial | +| 3 | `packages/shared/src/keybindings.ts` | `parseKeybindingShortcut`, `parseKeybindingWhenExpression`, `compileResolvedKeybindingRule` | Medium | +| 4 | `packages/shared/src/orchestrationTiming.ts` | `formatDuration`, `formatElapsed`, `isLatestTurnSettled`, `deriveActiveWorkStartedAt` | Trivial | +| 5 | `packages/shared/src/dpopCommon.ts` | `normalizeDpopHtu` | Trivial | +| 6 | `packages/shared/src/httpReadiness.ts` | `describeReadinessCause`, `waitForHttpReady` | Medium | +| 7 | `packages/shared/src/projectScripts.ts` | `projectScriptCwd`, `projectScriptRuntimeEnv`, `setupProjectScript` | Trivial | +| 8 | `packages/contracts/src/model.ts` | `coerceLegacyOptionsObjectToArray`, `canonicalSelectionsToLegacyObject` | Easy | +| 9 | `packages/effect-codex-app-server/src/errors.ts` | `schemaIssueDiagnostics`, `payloadKind` | Easy | +| 10 | `apps/web/src/terminal-links.ts` | `extractTerminalLinks`, `splitPathAndPosition`, `resolvePathLinkTarget`, `collectWrappedTerminalLinkLine`, `resolveWrappedTerminalLinkRange` | Medium | +| 11 | `apps/web/src/markdown-clipboard.ts` | `serializeRenderedMarkdownFragment`, `serializeTableElementToMarkdown`, `serializeTableElementToCsv` | Medium | +| 12 | `apps/web/src/providerModels.ts` | `getProviderModels`, `getProviderDisplayName`, `isProviderEnabled`, `resolveSelectableProvider`, `getProviderModelCapabilities`, `getDefaultServerModel` | Easy | +| 13 | `apps/web/src/shortcutModifierState.ts` | `shortcutModifierStateAfterKeyboardEvent`, `areShortcutModifierStatesEqual` | Trivial | +| 14 | `apps/web/src/sourceControlPresentation.ts` | `getSourceControlPresentation` | Trivial | +| 15 | `apps/web/src/editorPreferences.ts` | `PreferredEditorEnvironmentRequiredError`, `PreferredEditorUnavailableError`, `resolveAndPersistPreferredEditor` | Easy | +| 16 | `apps/web/src/sidebarProjectGrouping.ts` | `buildPhysicalToLogicalProjectKeyMap`, `buildSidebarProjectSnapshots` | Easy | + +### Existing tests with gaps + +| # | File | What's Missing | Difficulty | +| --- | ------------------------------------------------------------- | ----------------------------------------------------------------------------------- | ---------- | +| 17 | `apps/server/src/sourceControl/SourceControlProvider.test.ts` | Error handling tests (unresponsive providers, auth failures, malformed URIs) | Easy | +| 18 | `packages/contracts/src/settings.test.ts` | Edge cases (empty arrays, deeply nested overrides, numeric boundary values) | Easy | +| 19 | `packages/shared/src/chatList.test.ts` | Edge cases (empty input, single-entry lists, duplicate timestamps, malformed dates) | Easy | + +--- + +## Appendix: Raw Audit Notes + +### Scope + +- **Searched:** All app/packages source code, `docs/`, `.github/` workflows, `.gitignore`, `.vscode/`, `tsconfig.base.json`, `.env.example`, all `package.json` files +- **NOT searched:** `.repos/` (vendored, out of scope), `experiments/`, `.plans/`, `patches/`, `assets/`, `.cursor/`, `.macroscope/` + +### Methodology + +Audit performed by 5 parallel subagents: + +1. **Docs & README** β€” scanned all documentation for typos, broken links, stale content, missing READMEs +2. **Missing Tests** β€” identified source files without test files, TODO tests, test gaps +3. **Simple Bugs & Lint** β€” searched for `@ts-ignore`, `as any`, empty catches, bare `console.error`, swallowed errors +4. **Code Quality & Dead Code** β€” identified unused exports, magic numbers, missing return types, dead code +5. **Config & CI** β€” audited CI workflows, gitignore, TypeScript config, VS Code extensions, env vars, build scripts From ae95dfdd46f8f942aa37715e34286b6bbf87f737 Mon Sep 17 00:00:00 2001 From: Kriday Dave Date: Sun, 19 Jul 2026 00:42:01 +0530 Subject: [PATCH 2/2] ci: drop dead label color/description metadata from pr-size config --- .github/workflows/pr-size.yml | 39 ++++++++--------------------------- 1 file changed, 9 insertions(+), 30 deletions(-) diff --git a/.github/workflows/pr-size.yml b/.github/workflows/pr-size.yml index e2feb03edda..957c0044c96 100644 --- a/.github/workflows/pr-size.yml +++ b/.github/workflows/pr-size.yml @@ -20,37 +20,16 @@ jobs: with: result-encoding: string script: | + // Only `name` is consumed by the surviving `label` job; the + // `sync-label-definitions` job (which used color/description) was + // removed as dead, so those fields are no longer carried. const managedLabels = [ - { - name: "size:XS", - color: "0e8a16", - description: "0-9 effective changed lines (test files excluded in mixed PRs).", - }, - { - name: "size:S", - color: "5ebd3e", - description: "10-29 effective changed lines (test files excluded in mixed PRs).", - }, - { - name: "size:M", - color: "fbca04", - description: "30-99 effective changed lines (test files excluded in mixed PRs).", - }, - { - name: "size:L", - color: "fe7d37", - description: "100-499 effective changed lines (test files excluded in mixed PRs).", - }, - { - name: "size:XL", - color: "d93f0b", - description: "500-999 effective changed lines (test files excluded in mixed PRs).", - }, - { - name: "size:XXL", - color: "b60205", - description: "1,000+ effective changed lines (test files excluded in mixed PRs).", - }, + { name: "size:XS" }, + { name: "size:S" }, + { name: "size:M" }, + { name: "size:L" }, + { name: "size:XL" }, + { name: "size:XXL" }, ]; core.setOutput("labels_json", JSON.stringify(managedLabels));