Skip to content

refactor: consolidate duplicated text/error helpers + remove committed __pycache__#264

Merged
danielgwilson merged 2 commits into
mainfrom
claude/code-quality-cleanup-bpf9nl
Jul 8, 2026
Merged

refactor: consolidate duplicated text/error helpers + remove committed __pycache__#264
danielgwilson merged 2 commits into
mainfrom
claude/code-quality-cleanup-bpf9nl

Conversation

@danielgwilson

Copy link
Copy Markdown
Owner

Summary

Code-quality cleanup found while ramping on the recent cua work. Two commits, both behavior-preserving:

1. Remove slop + finish an incomplete dedup

  • Untrack homun/fixtures/shared-world-app/__pycache__/store.cpython-312.pyc (committed Python bytecode, unreferenced) and add __pycache__/+*.pyc to .gitignore so it cannot recur.
  • Finish fix(cua): recover CommandExitError at browser-launch + shared-world seat sites #260's dedup: e2b-desktop-executor.ts still carried a byte-identical private tailOf while already importing from ./command-failure.js — consolidated to the shared import.

2. Consolidate genuine duplicate helpers

  • toErrorMessage — extract the error instanceof Error ? error.message : String(error) coercion from 5 byte-identical private compactError copies into command-failure.ts. The redaction-aware compactError variants in oss-meta-lab/lab-preflight are left local; the helper is non-redacting by contract and every caller still wraps it in redactText(scrub(...)).
  • digestText(text, length = 12) — parameterize the sha256 helper and route the 4 hand-rolled createHash(...).slice(0, 16) sites through it (cua-actor-lab promptDigest/commandDigestOf/digestUrl, lab-preflight digest).
  • redaction.ts tailText + redactedTail — host the two byte-identical tailText copies (run.ts, codex-app-server.ts) and the two near-identical redact-then-ellipsis tails (e2b-terminal-lab, cua-actor-lab). Also fixes a copy-pasted-wrong doc comment ("sha256-12 ..." on a tail function).
  • oss-lab.repoSlug — export the repo slug helper and drop oss-meta-lab's identical repoSlugToken (oss-meta already imports from oss-lab).
  • scripted-browser-actor — extract the duplicated best-effort blocked-step screenshot block into captureBlockedStepScreenshot.
  • observer.ts — document why readObserverData's two best-effort reads swallow all errors (a live poller reading a partially-written run.json must 404-and-retry, never 500). Behavior unchanged.

No behavior change intended: the redact-then-coerce order at every call site is preserved, and digest widths / semantics are identical.

Public Boundary

  • This change does not include PII, PHI, secrets, keys, tokens, raw private transcripts, private screenshots, or private customer/patient data.
  • Examples, fixtures, personas, screenshots, and artifact snippets are synthetic or redacted.
  • Product-specific private context stays outside this repository or in synthetic adapter fixtures only.

Proof

  • pnpm typecheck — green.
  • pnpm test809 passed, 10 skipped (full suite run with a Chromium binary available so the browser-dependent lab tests execute).
  • pnpm build — green.
  • pnpm public-surface:scanok: 419 candidate text files checked, 1 binary asset verified.

Remaining Gaps

  • Pre-existing, environment-only: the scripted-browser-lab "live provisioned clone" test fails when no Chrome/Chromium is discoverable on PATH (HOMUN_SCRIPTED_LAB_BROWSER_MISSING); it passes with HOMUN_BROWSER_COMMAND set. Not touched by this PR — a fully-mocked test still requiring a real browser binary is a latent design smell left for a separate change.

🤖 Generated with Claude Code


Generated by Claude Code

@danielgwilson

Copy link
Copy Markdown
Owner Author

Heads-up from the local-tree session: four PRs merged to main today (#265 local-tree keystone, #267 receipts, #268 phase events, #269 CLI envelopes), and this branch now shows CONFLICTING. Likely collision points with your two commits: src/cua-actor-lab.ts grew a shared provisioning pipeline + phase-event emission (#265/#268), src/program.ts had the dead plannedCommands scaffold removed and gained a command-boundary catch-all (#269), and the error-helper landscape shifted accordingly. Also relevant: the pre-merge audit catalogued the compactError/shellQuote duplication this PR targets (7 and 4 copies respectively) - if you rebase, src/source-archive.ts (new in #265) exports normalizeExtraExcludeEntry and its own error-tail helper that may be worth folding into the consolidation. No changes made to this branch.

danielgwilson and others added 2 commits July 8, 2026 21:41
- Untrack homun/fixtures/shared-world-app/__pycache__/store.cpython-312.pyc
  (Python bytecode cache, unreferenced) and add __pycache__/ + *.pyc to
  .gitignore so it cannot recur.
- Finish the #260 dedup: e2b-desktop-executor still carried a byte-identical
  private copy of tailOf while already importing from ./command-failure.js;
  import the shared tailOf and drop the local copy.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0175FyoDn6pE67rxXRGMjyc2
Remove genuine copy-paste duplication surfaced while ramping on the recent
cua work. All changes are behavior-preserving (809 tests + typecheck + build
+ public-surface scan green); the redact-then-coerce order at every call site
is unchanged.

- toErrorMessage: extract the `error instanceof Error ? error.message :
  String(error)` coercion (5 byte-identical private `compactError` copies:
  concurrent-shared-world-lab, shared-world-lab, scripted-browser-lab,
  e2b-terminal-lab, cua-actor-lab) into command-failure.ts. The redaction-aware
  compactError variants in oss-meta-lab/lab-preflight are deliberately left
  local. Non-redacting by contract; every caller still wraps it in redactText.
- digestText(text, length = 12): parameterize the sha256 helper and route the
  four hand-rolled `createHash(...).slice(0, 16)` sites (cua-actor-lab
  promptDigest/commandDigestOf/digestUrl, lab-preflight digest) through it.
- redaction.ts tailText + redactedTail: host the two byte-identical `tailText`
  copies (run.ts, codex-app-server.ts) and the two near-identical redact-then-
  ellipsis tails (e2b-terminal-lab, cua-actor-lab). Fixes a copy-pasted-wrong
  doc comment on e2b-terminal-lab's tailOf ("sha256-12 ...").
- oss-lab.repoSlug: export the repo slug helper and drop oss-meta-lab's
  identical repoSlugToken (oss-meta already imports from oss-lab).
- scripted-browser-actor: extract the duplicated best-effort blocked-step
  screenshot block into captureBlockedStepScreenshot.
- observer.ts: document why readObserverData's two best-effort reads swallow
  all errors (live-poll partial writes must 404-and-retry, never 500).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0175FyoDn6pE67rxXRGMjyc2
@danielgwilson danielgwilson force-pushed the claude/code-quality-cleanup-bpf9nl branch from 8a07e39 to c3d4380 Compare July 8, 2026 21:44

Copy link
Copy Markdown
Owner Author

Rebased onto current main (through #269). Conflicts were all in import lines — resolved by unioning: kept readFile/rm and the new source-archive import in cua-actor-lab.ts, dropped now-unused createHash (the sha256-16 sites route through digestText(x, 16) now), and merged redactText + tailText in run.ts. One real collision beyond imports: #265's local-tree pipeline added two compactError(error) call sites in cua-actor-lab.ts (packing + subject-upload); since this PR removes that file's local compactError, I pointed both at the shared toErrorMessage — behavior-identical, both stay wrapped in redactText/tailOf.

Full verification green on the rebased tree: typecheck, 879 tests, build, and public-surface:scan (427 files).

On the source-archive.ts tarErrorTail suggestion — leaving it out of this PR on purpose. It's redaction-shaped and specific to tar output; folding it in mid-rebase would widen scope past "consolidate the exact duplicates" and add regression surface. Happy to do it as a focused follow-up.


Generated by Claude Code

@danielgwilson danielgwilson merged commit eea1d9a into main Jul 8, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant