feat: harness CLI workbench — phase 1 core (registry, output lanes, async runner) - #43
Draft
noodlemind wants to merge 13 commits into
Draft
feat: harness CLI workbench — phase 1 core (registry, output lanes, async runner)#43noodlemind wants to merge 13 commits into
noodlemind wants to merge 13 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
…nt, learnings, status)
… budgeted agent lane
…nd agent-lane metering
… and cancel verify
…t, reject unsupported lanes, true up the tool contract
…nce boundaries Close two rounds of external adversarial review: route every machine and human output sink (legacy --json, JSONL, events log incl. metadata, ledger, debug stack, telemetry sync) and the toJSON serialize path through one redacting boundary; honor the -- literal argument boundary in every flag read; settle runner cancellation only after the process group is reaped; map verify timeout to exit 8; stream and redact check output live; fix JSONL backpressure and quadratic UTF-8 clipping.
The serialize-then-mask pass could consume a JSON escape backslash when a secret-shaped value preceded an escaped quote, terminating the string early and emitting unparseable JSON from every machine sink. Mask through a stringify replacer instead: it still runs after toJSON, and the serializer owns escaping.
recall, get, and plan-new reported a harness fault (E_UNEXPECTED, exit 1) when invoked without a required argument, so callers could not distinguish misuse from breakage. Enforce the requirement declaratively in the registry and raise the standard usage error instead; the guidance messages are unchanged.
Declares main's new flags on their registry entries so strict validation accepts them, routes its structural-index flag read through the boundary aware check, and keeps both sides' event types, fields, and docs. Known failure: the one-live-plan contract assertion, because two dated plans are now in flight at once. Policy decision, not a code defect.
The primary consumption platform is Windows, but every check ran on Linux, leaving the win32 branches untested: descendant termination via taskkill, LOCALAPPDATA path resolution, console glyph degradation, and the .cmd shim. Captures console output verbatim as an artifact.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Phase 1 of the Harness CLI Workbench (contract: docs/architecture/harness-cli-workbench.md, plan: docs/plans/2026-07-29-harness-cli-phase1-core.md).
Turns the CLI's dispatch, output, error, and process layers into the kernel every later phase builds on — behavior-compatible for all current consumers.
Workstreams (one commit each as they land)
Verification
Named checks: harness-tests, prompt-contracts, build-assets (host-contracts retired on main by #38; plan updated accordingly). Acceptance: AC1–AC10 in the plan file.
Notes
Final review & disclosures
harness verify --planpasses 14/14 with the evidence artifact.orient,learnings,status, plusverify --output jsonl), with every other command rejecting--outputvia a structuredE_USAGEinstead of silently degrading. Lanes expand per command in Phase 2 asresultOfproducers land. The amendment is recorded inline in the plan's AC3 with the reversal path (expandresultOfto all commands) if you'd rather hold the original bar.writeEventcall sites migrate to the event registry with Phase 4a; 8 triaged minors logged in the SDD ledger.Codex external review returned No-go; I reproduced every Critical, so my earlier "ready" was wrong. Root cause: redaction was wired per-new-output-lane and no component made it universal — the legacy
--json/JSONL serializers and the event metadata path (added after payload redaction) still leak.Verified must-fix (fix wave in progress):
parseFlagsignores the--literal boundary (status --no-events -- --jsonleaks--json).emitJsonand the JSONL serializer don't redact (rawghp_token printed).events.mjssink writes host/actor metadata unredacted (HARNESS_HOST=token=…lands raw inevents.jsonl).reportself-instrumentation; feat: update all agent model assignments to latest Claude models #9 verify live-streams check output (AC8); feat: add code-implementer subagent for cost-efficient model routing #10/Add plan: Global Workspace Sync and Copilot CLI 1.104 compatibility #11 agent-lane fence + metering.Do not merge until the fix wave lands, re-review passes, and AC6/AC8 hold end-to-end.
Redaction hardening — landed (2026-08-06)
The universal-redaction fix wave landed (commit
758d746). Every open finding from both Codex rounds is fixed and internally verified — 919/919 tests, all headline bypasses (toJSON,--boundary, ledger/human leak,__proto__, PEM streaming) re-confirmed by controller probes + per-finding evasion tests, secret-free byte-identity preserved.One honest caveat: the third independent Codex certification could not run — the Codex CLI's OAuth token expired (401) mid-session. Last independent third-party confirmation is Codex round 2 (8/11 fixed); the round-2 residuals + new findings are controller-verified but not Codex-re-certified. Re-run
codex login+ a third pass before merge if independent sign-off is required. Details in the plan's "External Review (Codex)" section.Commits: 6ca1e62 docs · f547452 runner · 58dcb76 registry · 02bfd10 redaction · c28dbf5 lanes · 9dd1f17 events · b5ed24d migration · dac4a70 final-review fixes · 758d746 universal-redaction hardening.
Controller review (2026-08-06) — one Critical found and fixed
With Codex unavailable (expired auth), I reviewed the unverified round-3 surface myself and found a regression introduced by that round's own fix:
redactedJsonmasked by running the text redactor over already-serialized JSON, so a secret-shaped value before an escaped quote consumed the escape backslash and emitted unparseable JSON from every machine sink (--json, JSONL, events, evidence, telemetry). Trigger was the harness's ownE_TARGETmessage shape (no learning "token=…" found).Fixed at value level via a
JSON.stringifyreplacer — still runs aftertoJSON(so the bypass it was added for stays closed), but the serializer owns escaping, so masking can't corrupt structure. Audited every other sink:events/evidence/event-registrywere already value-level;envelope/telemetryfixed transitively. 924/924, JSON-validity property tests added, byte-identity preserved. Commit8ea2a29.Also verified clean: human-path redaction (3ms/1000 ledger lines, help intact), runner group-settlement (no hang), and all six prior headline bypasses.
Final PR review (2026-08-06)
Controller: READY. Whole-PR pass — hygiene clean (no
.harness/evidence/scratch committed, no debug debris, 10 commits one-per-workstream); help renders for all 24 commands; read-only smoke across 12 commands with no crashes; the read-paths-never-create-the-store invariant holds; test quality real (254 tests / 794 assertions across the ten new suites, no vacuous padding); doc/code agreement verified (no stale CATALOG claim; all three named event opt-outs exist inevents.mjs). 924/924,harness verify --plan13/13.One Minor, non-blocking, non-regression:
recall/getwith a missing required positional emitE_UNEXPECTED/exit 1 rather than a usage error/exit 2. Verified pre-existing onmain(handler-owned validation, deliberately unchanged by the migration — compatibility preserved). The registry now makes positional validation cheap; suggest closing it in Phase 2.Codex: could not run. Its OAuth token is still expired (
token_expired401 on probe). Independent certification of the round-2/round-3 fixes remains open — runcodex login, then a final Codex pass, if independent sign-off is wanted before merge.Merged with main (2026-08-06) — one known failure
Main advanced with #42 (harness evolution: blueprint + its Phase 1–4 implementation), touching nine of the same files this branch rewrote. Merged in commit
2987e90— 11 conflict hunks across 8 files, resolved keeping both sides.The part that would have broken silently: this branch's registry does strict unknown-flag rejection, and #42 shipped flags it never declared. A clean textual merge would have left main's new features failing with
unknown flag. Eight flags are now declared on their proper entries —index --structural/--since,knowledge --branch/--ids/--merged/--stale/--yes,consolidate --layer— and verified live through registry dispatch (index --structural,knowledge status,consolidate --layer goldenall exit 0). Git's auto-merge had also dropped #42's new value-flag branches into the rewritten parser in a way that bypassed the--literal boundary, silently re-opening a leak an earlier review closed; that is fixed and re-verified.--refresh/--semanticwere deliberately left undeclared — they pre-date the merge base and have no readers anywhere inlib/+bin/. Confirm they aren't placeholders for unlanded work.Suite: 1182 tests, 1173 pass, 8 skipped, 1 fail. The single failure is
prompt-library retains at most one non-terminal PR plan— two dated plans are now in flight (this PR's2026-07-29, and #42's2026-08-06atstatus: review, phase 3). That is a policy collision, not a code defect: the one-plan convention assumed one PR at a time. Left failing by decision, to be resolved separately. Every code-level test from both sides passes.harness verify --planconsequently reports the two check failures that roll up from that assertion; the criteria remain evidenced by the passing suite.