Skip to content

Reshape the owned code that the comment sweep flagged MUST KILL #222

Open

Description

@ericlitman

Context

#218 swept the comments out of the code this repository owns (#219 states the rule in AGENTS.md; the sweeps are #221 for plugins/pstack/ and #220 for the repository tooling). pstack's comment-sicko flagged code whose deleted comment was hiding a surprise in our own code (MUST KILL). The sweep PRs fixed the trivial flags in place: the resolver test's snapshot() empty catch, freshRoot's EEXIST rule (now a test), and RELEASED_PAIRS' append-only rule (now a test). This issue lists the reshapes they left: one line per flag with the symbol, its path, and the suggested reshape. The suggested reshapes are suggestions; read the code before choosing one.

Acceptance criteria

  • Each item below is reshaped in a PR, or closed in this issue with the reason it stays as it is.
  • A reshape does not restore a deleted comment, except one on the AGENTS.md keep-list.
  • Each constraint under "Constraints whose comments were deleted" is encoded as a type, test or lint, or recorded here as accepted without enforcement.
  • Each PR passes verify, Unfret and live-gate.

Plugin code (plugins/pstack/skills/poteto-mode/scripts/)

  • captureStream cancel(), runner/run.ts: the empty catch {} around reader.cancel() swallows every rejection, though the "already released" case comes from our own finally { reader.releaseLock() }; record that the read loop finished, skip cancel() then, and drop the catch.
  • SharedGitDiscovery and discoverSharedGitDir, runner/run.ts: split { kind: "found", sharedGitDir: null } into none and deny variants, and name the glob and control-character regex for the Claude Code denyWrite limit it serves.
  • ProviderAdapter.parse, runner/registry/contract.ts and runner/run.ts: after a nonzero exit run.ts keeps only provider-failure, so agy.ts and dsh.ts return malformed on purpose to get stderr reclassified as unavailable-model; put the exit outcome into the parse contract, or let adapters return unavailable-model directly.
  • ProviderAdapter.proof, runner/registry/contract.ts: make it { report: "required"; matches } | { report: "absent" }, then delete the unreachable matches functions in codex.ts, agy.ts, amp.ts, cursor.ts, dsh.ts and opencode.ts.
  • launch.stdinEnvelope, runner/registry/contract.ts: it means something only when prompt is "stdin"; make prompt a union { kind: "stdin"; envelope? } | { kind: "file" }.
  • offered in resolveLane, runner/registry/index.ts: extract offeredEfforts(match, adapter) with one branch per ModelMatch kind instead of the nested ternary.
  • FamilyCommon.upstream, runner/registry/family.ts: rename to say it holds Cursor's selector, for example cursorSelector.
  • unregistered in adapterSuite, runner/providers/adapter-suite.ts: rename to unregisteredEffort.
  • threadIndex, resolve-codex-transcript.ts: rename the predicate to isThreadsIndex.
  • readPrefix, resolve-codex-transcript.ts: MAX_BYTES + 1 is a hidden oversize sentinel that openingMessage depends on; return { bytes, oversized } or name the probe.
  • "Return the marker.\n".repeat(100_000), runner/run.test.ts: name the size whose job is to exceed pipe capacity.
  • fakeDevin, fakeOpencode, fakeDsh, fakeAmp, fakeAgy, fakeCursor, runner/run.test.ts: they splice JS source into generated scripts that depend on names the template declares (args, at, prompt, emit, exportPath, config); build them from one shared stand-in builder with an explicit context.

Constraints whose comments were deleted and that nothing enforces

Repository tooling (.claude/skills/verify-open-pstack/, tests/, .github/)

  • SetupSnapshot, .claude/skills/verify-open-pstack/scripts/isolation.ts: make it a discriminated union of link entries and file entries.
  • SessionRecord.skills, scripts/types.ts: Claude records names and Codex records names or paths (harness.ts parseSession); normalize Codex paths there, or split into skills and skillPaths.
  • classify, scripts/core.ts: move the reflect-resolver regex and the poteto-agent preload special cases into features/registry.json.
  • legacy_model_pins, tests/skill-collision-repro.sh: move the three-stage grep and sed pipeline into runner/registry.test.ts, or name each stage.
  • browser(), features/recipes.ts: the fallback returns a Chrome path it knows is missing; throw browser-missing in prepare instead.
  • verifyProjectDoctor, features/recipes.ts: replace the empty catch {} with parseJson and realpath(...).catch(() => '').
  • record.commands.some(c => c.includes('pstack-runner')), features/recipes.ts (3 runner recipes): a command-text check that the receipt already proves; delete it or match the installed launcher path.
  • rm(join(f.configHome, 'AGENTS.md'), { force: true }), features/recipes.ts (6 copies): extract removeCodexMirror(f).
  • omitted, features/recipes.ts (2 copies): extract withoutOneAliasRole(roles).
  • NEW_FAMILY_LANES and CATALOG_LANES, features/recipes.ts: labeled tuples [sheet: string, rewritten: string] and [sheet: string, dispatched: string].
  • .agents/result.json literal, features/recipes.ts: name it for the Codex workspace-write block, for example WORKSPACE_WRITE_BLOCKED_RESULT.
  • Renames in the verifier: Catalog.kept → currentByPredecessor, sh → shellQuote, bounded → assertBoundedSetting, mirrored → assertCodexMirror, writer → linkedWorktreeWriter, SETTING → WRITER_SANDBOX_PREFIX, DEFAULT_ROLES → DEFAULT_RUNNER_ROLE (all features/recipes.ts); matches → matchesPathOrSubtree (scripts/core.ts); cleanupFailed → publicationForbidden (scripts/verify.ts); wide → wideReason (scripts/types.ts, scripts/verify.ts).
  • Setup-last sort comparator, scripts/harness.ts: name it setupLast.
  • tests/skill-collision-repro.sh: put the literal @~/.claude/pstack-models.md in a legacy_import variable, and name check()'s parameters (local label=$1 marker=$2 output=$3).
  • scripts/catalog.test.ts: split the long tests whose negative sub-cases are labeled by comments into named test.each rows.
  • Planted cmd.exe, .github/workflows/windows-hooks.yml: put the path in a $plantedCmd variable.
  • tests/setup-config-home-repro.sh: not swept. features/registry.json maps no owner for it, so any change to it fails live-gate classification closed. Registering it as nonRuntime would let a change publish live-gate success unexercised (Unfret on chore(tooling): delete comments from the verifier, tests and repository config (#218) #220). Map its ownership first, then delete its one comment.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions