Skip to content

Run component tests from the Sidekick - #986

Merged
taras merged 2 commits into
mainfrom
agent/repl-testing-896-codex-20261022
Oct 22, 2026
Merged

taras merged 2 commits into
mainfrom
agent/repl-testing-896-codex-20261022

Conversation

@taras

@taras taras commented Oct 22, 2026

Copy link
Copy Markdown
Owner

Why

A Sidekick can author reusable components, but needs a supported way to run their companion tests and receive the actual results. Refs #896. Loader prerequisite #983 is merged.

What changes

Before: the Sidekick has no REPL.Test request for a test document or directory.

After: <REPL.Test path="components/Greeting.test.md" /> runs one test document; <REPL.Test path="components" /> runs the selected suite. Each document uses an isolated testing root. Structured reports reach the Sidekick even when captured; explicit Output controls person-facing display. Shipped guidance teaches authoring, testing, fixing and reuse.

Review guide

Start with specs/repl-spec.md and packages/cli/src/repl/sidekick/testing.ts, then the R1-R6 assertions in packages/cli/tests/repl-sidekick-testing.test.ts.

What must stay true

  • Root admission remains reserved through joined cleanup; entries and tests cannot overlap.
  • Sidekick Stop/Exit cancels its testing work and joins cleanup. An admitted ordinary entry retains its own lifetime.
  • Retained history is inert; cancellation and fatal failures do not fabricate completed test reports.

How to verify it

Focused combined-head evidence passes: the five affected CLI files report 36 tests/185 steps, and packages/testing/tests/use-testing.test.ts reports one test/11 steps. Frozen R1-R6 cover Markdown author/fail/fix/reuse, same-path TypeScript edits, truthful reports, selection/isolation/display, admission/cancellation and cold recovery. Both original independent reviewers PASS signed feedback 8aa1d19. Exhaustive runtime verification belongs to remote CI.

Scope

REPL.Test requests, response-owned execution, production root admission, detached reports, shipped Sidekick teaching and owning specifications. This adds no dependency or build-layout change relative to the merged loader. Existing CLI test selection remains unchanged.

Risks and limitations

This draft is not merge-ready. Required CI, this changed corpus's runner-measured weights, final-head reviews after any weight commit and original Planner metadata APPLY/independent VERIFY remain outstanding.

Scope confirmation

  • Every changed file supports the purpose described above.
  • Unrelated cleanup and formatting changes are excluded.
  • The description matches the final diff and test results.

taras added 2 commits October 22, 2026 09:23
Preserve the reviewed REPL.Test implementation on corrected A and update
its fixture to the supported explicit test host installer path.
@github-actions

Copy link
Copy Markdown

PR #986: Run component tests from the Sidekick

17 files, +1438 / -10

Scope

🔴 PR has 1448 lines changed. Split into focused PRs.

🟡 1448 lines changed. PRs under 400 receive more thorough review.

Structural

Oxlint structural signals:

  • no-unused-vars ×14: packages/cli/tests/fixtures/repl/issue-883/sidekick-harness.ts, packages/cli/src/repl/sidekick/admission.ts, packages/cli/src/repl/sidekick/controls.ts (+2)
  • no-empty-function ×3: packages/cli/src/repl/sidekick/testing.ts, packages/cli/src/repl-profile.ts, packages/cli/src/cli.ts
  • no-unnecessary-type-assertion ×1: packages/cli/tests/fixtures/repl/issue-883/sidekick-harness.ts
  • no-unnecessary-type-arguments ×1: packages/cli/src/repl/sidekick/owner.ts

Slop

Oxlint slop signals:

  • no-console ×3: packages/cli/src/cli.ts

Static Analysis

Oxlint: 66 diagnostics across 8 files (16 rules)
Density: 0.046 violations/added-line

no-unused-vars (14): packages/cli/tests/fixtures/repl/issue-883/sidekick-harness.ts, packages/cli/src/repl/sidekick/admission.ts, packages/cli/src/repl/sidekick/controls.ts (+2)
no-floating-promises (13): packages/cli/src/repl/program.ts, packages/cli/src/cli.ts
no-shadow (12): packages/cli/src/repl/sidekick/admission.ts, packages/cli/src/repl/sidekick/owner.ts, packages/cli/src/repl/program.ts
unbound-method (6): packages/cli/tests/fixtures/repl/issue-883/sidekick-harness.ts, packages/cli/src/repl/program.ts, packages/cli/src/cli.ts
no-empty-function (3): packages/cli/src/repl/sidekick/testing.ts, packages/cli/src/repl-profile.ts, packages/cli/src/cli.ts
no-console (3): packages/cli/src/cli.ts
no-unsafe-type-assertion (3): packages/cli/tests/fixtures/repl/issue-883/sidekick-harness.ts, packages/cli/src/cli.ts
consistent-return (3): packages/cli/src/repl/program.ts, packages/cli/src/cli.ts
no-useless-default-assignment (2): packages/cli/src/repl/program.ts
no-useless-length-check (1): packages/cli/src/repl/sidekick/testing.ts
no-unmodified-loop-condition (1): packages/cli/src/cli.ts
no-useless-spread (1): packages/cli/src/cli.ts
no-unnecessary-type-conversion (1): packages/cli/src/repl/sidekick/controls.ts
no-misused-spread (1): packages/cli/src/repl/sidekick/controls.ts
no-unnecessary-type-assertion (1): packages/cli/tests/fixtures/repl/issue-883/sidekick-harness.ts
no-unnecessary-type-arguments (1): packages/cli/src/repl/sidekick/owner.ts

Correctness

No extraneous code patterns detected.

export function sidekickRequests(
root: RootReading,
submit: SubmitForReply,
test?: ReplTesting,

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this information come from context?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Context would be reasonable if several consumers needed to select a scoped test service. Here, test is an already-wired callback: the root supplies the admission-checked runner, and the conversation wraps it with servingNow() so it belongs to the active response. Passing it explicitly keeps those owners visible and follows the nearby submit callback.

Context would save the argument, but add binding, lookup and absence handling; the root admission and response-lifetime checks would still be needed. If scoped substitution or more consumers become useful, selecting the runner from Context during trusted assembly could make sense.

Given this single composition site, I agree with your clarification: leaving it unchanged is reasonable and doesn't warrant a refactor here.

@taras
taras marked this pull request as ready for review October 22, 2026 21:27
@taras
taras merged commit 3834f19 into main Oct 22, 2026
45 checks passed
@taras
taras deleted the agent/repl-testing-896-codex-20261022 branch October 22, 2026 21:28
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