Plan integration: PlanApplication seam, Bugs A/B closed (Phases 0–1) [in progress] - #20
Merged
Merged
Conversation
…x-first Three failing tests, written from the spec invariants rather than from the code, so the fix has a definition of done before it exists: - POST /api/plans with status=active on an unready plan currently returns 201 and persists an active plan whose own readiness says ready=false with three blockers. Spec #7 invariant: activation is readiness-gated on EVERY entry path; only the PATCH status path was gated. - PATCH /api/plans/{id} with a whole-document markdown replacement whose frontmatter says active and has no milestones currently returns 200. Same invariant, third door. - evaluate_and_record() discards record_checkpoint()'s boolean. The function swallows its own failures and returns False, so a failed DB write produced an evaluation with warnings=[] — complete recording claimed after a partial write (issue #9 acceptance criterion). Two companion tests pass today and pin the non-regression side (a ready plan still activates; a successful DB write adds no warning).
…, arbitration, openspec change Why: issues #7–#15 (Study Plan integration) were specified on 2026-09-04 and nothing landed; two of the bugs the parent issue names as must-fix-first are now RED-tested at 3a4f6b01. The owner asked for the outstanding work to be planned by a council of models (GPT Astra, Grok 4.6, one best-for-purpose seat) and executed TDD with a per-task definition of done. What lands: - scripts/council/run_council.py — fans one brief out to N gateway models in parallel, one receipt per seat + manifest (brief sha256, latency, tokens). scripts/council/system-seat.md — the no-tools seat contract, added after Grok's first run announced "I'll inspect the repo" and looped for 20k tokens (the invalid receipt is kept, renamed, as the instrument record). - council/brief-plan-2026-09-15.md and the three seat receipts. - council/arbitration-plan-round1-2026-09-15.md — decisions D-1..D-17 with the rejected alternatives named per seat, plus the facts the seats flagged as unknown, now verified in the tree (CLI status is already gated; energy_floor exists; previous_notes renders a "Resuming" section so it is the wrong carrier for a planning brief). - openspec/changes/plan-application-seam/{proposal,design,tasks}.md — the work order: phases, owners, files, RED test names, command-checkable DoD, and the council review gates. - .gitignore: un-ignore docs/architecture/plan-integration/ (same shape as the session-memory block; delivered HTML stays ignored). - .pre-commit-config.yaml: detect-secrets excludes council manifest.json — they hold sha256 digests of committed public text, the same false positive the UAT registry exclusion already documents.
`index.record_checkpoint` reports failure two ways: it swallows its own errors and returns False (no database, INSERT failed), and it can still raise from an import or connection fault. `evaluate_and_record` only handled the raise, so a False return produced an evaluation with warnings=[] — complete recording claimed after a partial write. The RED test committed at 3a4f6b01 (`test_failed_checkpoint_db_write_is_reported_ as_a_warning`) pinned exactly this. Both paths now append the existing "checkpoint not saved to the database" string, so callers reading an empty warning list can trust the checkpoint is durably recorded. The index's swallow-and-return-False stays as its best-effort policy (council D-1); the fix lives in the one caller that was ignoring the answer. Decision: D-1 (arbitration-plan-round1-2026-09-15).
Twenty-two failing tests for the seam that closes Bug A, written against design §1 and decisions D-2/D-3/D-4 rather than against any adapter. On this tree the module does not exist, so the file fails at import (ModuleNotFoundError: studyloop.planning.application). The load-bearing invariant is that activation is readiness-gated on EVERY entry path. Four doors are pinned — create-with-status, lifecycle transition, whole-document replacement, and document import — and one test asserts that all four refuse with the identical ReadinessView payload and write nothing first. The fourth door (ImportDocument) is not in the design's intent list: POST /api/plans has a raw-markdown import branch that would otherwise stay an ungated path into "active" or have to keep a route-local readiness gate, which D-2 forbids. It is a create, so it ships in Phase 1 with the other create door. Also pinned: browse ordering equals store.list_plans (active first, then ascending updated, then id) so the Web list and CLI table do not reorder when they migrate; PlanSummary/ReadinessView serialise byte-for-byte to StudyPlan.summary() and authoring.readiness() (D-3), which is what keeps the REST bodies unchanged; views are frozen, tuple-only, and to_json_dict() hands every caller a fresh container. The file carries a RED-commit-only pyright directive: the workspace pre-commit hook type-checks tests, and unresolved imports would otherwise make a test-before-code commit impossible without skipping hooks. T1.2 removes the directive when the modules exist. Ticks T0.1 in the tasks file with its sha (c16ffa35).
…oor (T1.2)
Four new modules under studyloop/planning, per design §1 and D-3:
errors.py PlanError + PlanNotFound, InvalidPlanId, PlanConflict,
InvalidField, PlanNotReady(readiness), InvalidMilestone.
Exceptions with no CLI/HTTP/MCP vocabulary; adapters map
them once. Names are the arbitration's (no `Error`
suffix — the suffixed forms already exist in store.py with
stdlib bases and are what the store raises *to* the seam).
views.py Frozen, tuple-only read models; to_json_dict() builds a
fresh container per call. PlanSummary and ReadinessView
serialise to StudyPlan.summary() and authoring.readiness()
key for key, so REST bodies do not change.
intents.py CreatePlan, ImportDocument, ReplaceDocument,
TransitionLifecycle — the four ways a document can become
active. `overwrite` stays on CreatePlan/ImportDocument for
the Web/CLI request shapes (D-4); MCP will not expose it.
application.py browse / inspect / prepare_planning / apply. `apply` runs
the readiness check whenever the RESULTING document would
be active and raises PlanNotReady before any write.
Why a seam rather than a shared helper: the two Bug A doors exist because
the gate lived on one Web route. A helper called from each route is the
same duplication with a function name (D-2 rejected exactly that). Here
the adapters never hold a StudyPlan to mutate, so a door cannot forget
the gate — it has no way to write except through `apply`.
Deviations from design.md, each deliberate:
- ImportDocument is an eighth intent. POST /api/plans has a raw-markdown
import branch that is also a create-and-activate door; without an intent
it would stay ungated or keep a route-local gate.
- ReadinessView carries plan_id; MilestoneView carries notes; PlanDetail
carries learning_records, resources and the document's checkpoints, with
`history` (the DB log) behind include_history. All so the existing GET
body serialises unchanged (D-3 outranks the sketch).
- No `plans_dir` constructor argument: directory resolution stays with
store.plans_dir() (env var / settings), as every fixture already relies
on. Threading a base path through eleven store functions for a parameter
no adapter passes would widen the diff for no caller.
Removes the RED-only pyright directive from test_plan_application.py.
23 tests green; pyright 0 errors on studyloop/planning.
…ate deleted (T1.3)
Closes Bug A. POST /api/plans (both the interview and the raw-markdown
branch), PATCH markdown, PATCH status, GET list/detail/markdown/history
and GET interview now go through the seam. The readiness check that lived
only on the PATCH-status branch is gone from this file — `rg 'readiness\('`
finds nothing — because every door into "active" is gated once, inside
`apply` (D-2: delete the route gate, do not add a third copy).
The two RED tests from 3a4f6b01 go green: an unready create-with-status
and an unready active document replacement both return 422 with the same
body the PATCH-status refusal always had ({"message", "plan_id", "ready",
"blockers", "nudges"}) and persist nothing. Every pre-existing assertion
in test_web_plans.py is unchanged (git diff against 3a4f6b01 is empty)
and passes: 27/27.
Domain errors map to status codes in one function (design §2):
PlanNotFound 404, InvalidPlanId/InvalidField 400, PlanConflict 409,
PlanNotReady 422, InvalidMilestone 404.
PATCH ordering: existence (404) → validate every field edit (400) →
lifecycle transition (400/422) → field edits → save. Previously the
readiness 422 was checked before the title/energy/milestone 400s; now a
body that is both unready-active and carries a bad field gets the 400.
Both refuse without writing. The alternative — transition first — would
persist the status change before a 400 on a sibling field, which the
single save_plan never did.
Still on direct imports until Phase 2 (AssessPlan, RevisePlan,
SetMilestone, DeletePlan): GET/POST evaluate, PATCH field/milestone
edits, the milestone toggle and DELETE.
…cation (T1.4) `plan list` browses, `plan show` inspects (with the raw document only when --markdown asks for it), and `plan status` applies a TransitionLifecycle intent. The CLI's own readiness check before `status … active` is gone: the refusal now comes from the seam's single gate, so the message a learner sees in the terminal — "Cannot activate 'x' — the plan is incomplete." followed by the blockers and nudges — is produced from the same ReadinessView the Web API turns into its 422 body. That is what the parity test in T1.5 asserts. `_print_readiness` consumes a ReadinessView; `_refuse_activation` is the one place the "Cannot activate" copy lives (it was duplicated between `new --activate` and `status`). `plan new` still drafts and creates directly — it moves onto CreatePlan in Phase 2 — but builds its ReadinessView from the draft so the output path is already the shared one. Exit codes and output are unchanged: test_cli_plan.py is byte-identical to 3a4f6b01 and passes 22/22. pyright 0 errors on cli/_plan.py.
…identically (T1.5) Two tests that meet the invariant from the outside, the way a learner or an agent does, rather than through the seam's own API: - test_activation_refusal_is_identical_via_cli_and_web: one unready draft; `studyloop plan status … active` exits 1 and its bullet list is exactly the Web PATCH 422 body's blockers followed by its nudges, in order; the document on disk is byte-identical afterwards, `plan show --json` still says draft and reports the same readiness the Web refused with, and the active listing is empty. This one already held on 3a4f6b01 — both surfaces gated the transition door — so it pins parity rather than reproducing a bug. - test_every_web_door_into_active_refuses_with_the_same_body: create-with- status, status transition, whole-document replacement and raw-markdown import all return 422 with equal bodies (plan_id aside for the import, which names its own). RED on the 3a4f6b01 adapters (create returned 201); green on the seam. RED evidence was taken by checking out the pre-seam web/routes/plans.py and cli/_plan.py into the working tree against the current planning package (1 failed, 1 passed), then restoring HEAD (2 passed).
…s + public doc (T1.6) Delta specs for the plan-application-seam change, in the repo's ADDED/Requirement/Scenario shape, one per capability the seam touches: - web-ui: the routes hold no readiness check; every door into "active" (create-with-status, document replacement, status transition, raw import) returns the same 422 body and writes nothing; the seam→HTTP error mapping is stated once; REST bodies are unchanged. - cli-surface: `plan status … active` applies TransitionLifecycle and its bullet list is the Web body's blockers then nudges; list/show read through the seam with their --json shapes unchanged. - active-learning-decisions: the seam itself — the single gate before any canonical write, frozen views that serialise to the existing key sets, domain exceptions with no adapter vocabulary — plus the Bug B rule that a False from record_checkpoint is reported exactly like a raise. Each scenario corresponds to a test that exists on this branch (test_plan_application.py, test_web_plans.py, test_cli_plan.py, test_plan_surface_parity.py, test_planning_evaluation.py). `openspec validate plan-application-seam` passes. docs/study-plans.md gains an "Activation" section saying what the gate requires, that it runs on every route into active on every surface, and that a refusal writes nothing. The "What a plan does not do yet" list is untouched: nothing in Phase 1 changes what a plan does, only how safely it becomes active. Ticks T1.1–T1.5 in the tasks file with their shas.
Recorded from command output on 2ca6bdb0: `just lint` clean (ruff check + format --check over 1011 files), `just typecheck` 0 errors workspace-wide, `pytest packages/studyloop/tests -k "plan or planning"` 346 passed exit 0, and the full studyloop suite 4576 passed / 4 skipped / 0 failed exit 0. Phase 1 stops here per the work order; Council review 1 gates Phase 2.
…urrent release) The release-consistency check wants every openspec change with commits since the last tag either archived or carrying an explicit deferred reason. Phases 0-1 have landed; Phases 2-6 remain. Archive when #15 closes.
…g-document gate)
Council code review 1 (GPT Astra REJECT, qwen3-coder's own red finding
agrees) found that the Phase 1 route composes a seam transition with a
second, unguarded save, so two doors into active-but-unready survived:
- F1 PATCH {"status":"active","milestones":[]} on a ready draft -> 200,
stored active with 0 milestones, ready=false. Its mirror is also
wrong today: adding the missing milestones in the same request is
refused (422) because readiness is judged on the pre-edit document.
- F1b PATCH {"milestones":[]} on an already-active plan -> 200, leaving an
active plan that cannot be evaluated. This one predates the branch.
- F4 POST with a duplicate id AND an unready active body -> 422; the
delta spec's "Duplicate id without overwrite" scenario says 409
unconditionally (identity before readiness).
Reproduced by hand against ac121874 before writing these; all four fail
on this tree. The fix is RevisePlan brought forward from Phase 2: one
load, all edits applied to a candidate, readiness judged on the result,
one save — never a route-side mutation after apply().
…n the resulting document
Council review 1 (seat openai.gpt-6-astra, finding F1, 🔴) rejected Phase 1
because `PATCH /api/plans/{id}` composed a seam transition with a second,
unguarded route-side save: `{"status": "active", "milestones": []}` was
gated against the OLD milestones, saved as active, and then had its
milestones stripped — an active-but-unready plan, the very state issue #7
exists to prevent. The mirror case (an unready draft supplied with its
missing milestones in the same request) was refused before those milestones
were considered, and (F1b) a field-only `{"milestones": []}` against an
already-active plan bypassed the seam entirely. Two saves also meant a
failed second write left the status change committed.
Bring `RevisePlan` forward from Phase 2 with design §1's explicit fields
(title, topics, target_date, energy_floor, review_cadence_days, notes,
milestones, learning_record) plus `status`, so a compound PATCH is ONE
intent. `_revise` loads once, validates every field before applying any
(404 before 400; a bad field beside a good status change writes nothing),
applies them to the candidate, runs `_assert_can_be_active` whenever the
RESULTING document is active — whether `status` makes it so or the plan
already is — and saves once. `plan_id` and `created` are preserved and
`updated` is bumped by the store's single save. `TransitionLifecycle` is
now the one-field case of a revision, so there is exactly one gate path.
The route only translates the body into the intent: `_field_updates` and
the route-side `save_plan` are gone. Field validation moved into the seam
as `InvalidField` with the same messages the route used ("title cannot be
empty", "milestones must be a list", "energy_floor must be an integer"), so
the existing 400 assertions in test_web_plans.py are unchanged. The
milestone toggle is expressed as a full-list `RevisePlan` until Phase 2's
`SetMilestone`, which removes the last route-side write:
`rg 'readiness\(|save_plan' web/routes/plans.py` → 0 hits.
`learning_record` mirrors `store.record_learning`'s rules (empty title and
H1-H3 body lines refused; identical title+body is an idempotent no-op) on
the in-memory candidate so the revision stays one save; the store writer
remains for the CLI/MCP paths Phase 2 migrates.
RED→GREEN: the four parity tests committed at 8d11ee40 for F1/F1-mirror/F1b
now pass; new seam tests in test_plan_application.py pin one save per
compound revision (monkeypatched `store.save_plan` call count == 1),
id/created preservation, the active-but-would-be-unready refusal, and that
invalid fields raise before any write.
…ment is judged Council review 1, finding F4 (🟡): `_persist_new` ran the readiness gate before the store's duplicate-id check, so `POST /api/plans` with an id that already exists AND `status: active` on an unready body answered 422 rather than the 409 the delta spec's "Duplicate id without overwrite" scenario promises unconditionally. Both outcomes wrote nothing, but the API's precedence was asserted in one place and contradicted in another. Order identity, conflict, readiness, write: validate the id (a traversal id is an `InvalidPlanId`, never a readiness refusal), probe the plans directory for the id and raise `PlanConflict` unless `overwrite` was set, then gate, then create. The store's own `PlanExistsError` is still translated so the race between the probe and the write stays a conflict. RED→GREEN: parity test `test_duplicate_id_is_a_conflict_even_when_the_new_document_is_unready_active` (committed RED at 8d11ee40) passes; seam tests `test_duplicate_unready_active_create_reports_conflict` (parametrised over create-with-status, import by frontmatter id, import by explicit id) and `test_malformed_explicit_id_is_refused_before_readiness` pin the precedence for every create door.
…maps every refusal Council review 1, finding F3 (🟡, seat openai.gpt-6-astra) and the qwen3-coder seat's "CLI error mapping incompleteness": two gaps in the promise that a domain error is translated exactly once per adapter. Seam: `inspect` read the raw document with `store.load_plan_text` *after* `_load` had translated the parse, so a plan deleted between the two calls escaped as the store's `LookupError` past every adapter's `except PlanError`. `_load_text` now applies the same translation as `_load`. CLI: `plan list` called `browse` with no `PlanError` handler, and `_fail_for` knew only `PlanNotFound` — `PlanConflict`, `InvalidField`, `InvalidPlanId` and `InvalidMilestone` fell through to the bare exception text, and `PlanNotReady` was handled at one call site rather than in the mapping. `_fail_for` now gives each its own one-line message (design §2) and owns the `PlanNotReady` → `_refuse_activation` case, so `plan status` needs one `except`; `plan list` routes its refusal through it. Exit codes and the existing "Cannot activate" / "No study plan with id" lines are unchanged — test_cli_plan.py is untouched and green. RED→GREEN: `test_inspect_markdown_translates_store_not_found_after_initial_load` (seam), `test_plan_list_domain_refusal_exits_without_traceback` and `test_cli_maps_each_seam_refusal_to_a_specific_message` (parametrised over the five refusals) in test_plan_surface_parity.py.
… by one factory Council review 1, finding F2 (🟡): the module docstring claimed views are "deep-frozen on construction", but only `PlanningBrief.build()` froze the evidence seed. The generated constructor accepted a mutable mapping as-is, so `seed["notes"].append(...)` after construction changed a frozen view, and `_freeze` returned unsupported leaves (a model, a bytearray, an arbitrary object) unchanged — mutable objects a "frozen" view could not vouch for. Freeze and defensively copy `evidence_seed` in `__post_init__` via `object.__setattr__` (the sanctioned way for a frozen dataclass to normalise its own fields), and normalise `interview`/`existing_plans` to tuples there too. `_freeze` recurses through nested mappings and sequences, copying as it goes, and raises `TypeError` for any leaf that is not a JSON scalar (str, int, float, bool, None) — the only leaf types the seed readers produce and the only ones `json.dumps` on the CLI path ever accepted. `build()` stays as a convenience factory over the plain dicts the authoring module returns. RED→GREEN in test_plan_application.py: `test_planning_brief_direct_constructor_defensively_freezes_seed`, `test_planning_brief_nested_seed_mutation_cannot_change_view`, `test_planning_brief_json_calls_do_not_share_nested_containers`, `test_planning_brief_rejects_unsupported_mutable_seed_leaf` (object, bytearray, model; and a non-mapping seed).
… the file on every write Council review 1, finding F5 (🟡): `ImportDocument` let an explicit id override the frontmatter (accepted deviation 1) but never implemented its documented final fallback. `parse_plan` resolves a missing frontmatter id to the bare title slug, so a second import of an untitled-by-id document was a `PlanConflict` where the pre-seam route (and `CreatePlan`) allocated `unique_plan_id`'s `-2`, `-3`… The successful import paths — explicit-id override actually persisting under the override, `created` preservation, a ready `active` import, a ready `active` replacement — had no coverage. `_import` now settles identity before the readiness gate, in order: explicit `plan_id`, else the frontmatter id, else a unique title slug. The parser is handed a sentinel fallback that cannot pass `validate_plan_id`, so "no frontmatter id" is distinguishable from a real one and can never be filed. `_load` pins the returned model to the *storage* id. The parser lets a document's own frontmatter `id` win over the filename, so a hand-edited plan under `target.md` whose frontmatter said `id: other` was re-saved by replace, revise and transition as `other.md` — a second file, with `target.md` left untouched. Every write path loads through `_load`, so "the id is the file" now holds for all of them; `_replace` keeps preserving `created`. RED→GREEN in test_plan_application.py: `test_import_without_id_allocates_unique_title_slug` and `test_replace_keeps_requested_storage_identity_when_frontmatter_disagrees` (parametrised over replace / revise / transition; done-criterion: one updated target document, no second file). Pinned as passing: `test_import_explicit_id_overrides_frontmatter_without_creating_old_id`, `test_import_preserves_document_created`, `test_ready_active_import_succeeds`, `test_ready_active_replacement_succeeds`.
…int database Council review 1, finding F6 (🟡): the Bug B fix (T0.1, c16ffa35) shipped without the regression tests that pin it, and the seam tests isolated the plans directory but not visibly the checkpoint database — `test_inspect_carries_markdown_and_history_only_on_request` asserted an empty history for "demo" against whatever the suite's shared database held. New `tests/test_plan_recording_failures.py` runs every test against its own `STUDYLOOP_DB` and plans directory and asserts both the returned evaluation and each sink's outcome independently: a `record_checkpoint` that returns `False` or raises adds exactly the database warning and still appends the checkpoint to the document; a successful record adds no database warning and leaves exactly one row; a failed document write (a raising `save_plan`) adds only the document warning and does not discard the row the database already holds; `append_to_plan=False` skips the document sink silently; and the evaluation is returned even when both sinks fail (D-1/D-3: no `PartialRecording`). Mutation check: reverting the boolean handling the way the original bug did fails three of the six. test_plan_application.py gains the same per-test database fixture and `test_inspect_history_is_newest_first_and_honours_the_limit`, which seeds three rows through `index.record_checkpoint` and reads them back through `inspect(include_history=True, history_limit=…)`: newest first, the limit honoured, the six-key row shape serialised, another plan's log empty. The three protected legacy test files are untouched.
…thing (Web) The seam test test_revise_invalid_field_raises_before_any_write already covers RevisePlan(status="active", title="") → InvalidField with no save; this pins the same fact end to end through PATCH so the delta spec's new scenario "A bad field beside a status change writes nothing" has a test a reviewer can run: 400 with the legacy message, status still draft, document bytes unchanged.
…Activation paragraph, F1–F6 ticked
GPT Astra's §3 spec/doc review found the delta spec did not exactly match
the code and the public paragraph over-promised.
web-ui delta: the requirement now names in-place revision (including a body
that combines a status change with field edits) among the doors it gates,
says the gate judges the document "as it would be saved" whether the request
makes the plan active or it already is, requires no route-side store write
(`rg 'readiness\(|save_plan'` → 0), and expresses the refusal as the full
response `{"detail": {...}}` with `plan_id` kept — the old route included it,
so design §2's shorter sketch is corrected rather than used to drop a legacy
key. New scenarios: compound PATCH refused as one resulting document;
compound PATCH that supplies what was missing activates in one write;
field-only edit cannot make an active plan unready; ready raw-Markdown import
succeeds; conflict is judged before readiness on every create door; a bad
field beside a status change writes nothing. Each has a test in
tests/test_plan_surface_parity.py or tests/test_plan_application.py.
cli-surface delta: a requirement for the complete `_fail_for` mapping (six
domain errors, one line each, exit 1, no traceback; `plan list` routed
through it), with the two scenarios the parity tests pin.
docs/study-plans.md: the "Activation" paragraph is replaced by the bounded
wording from the review — application-mediated Web writes, CLI activation
commands, refused activation writes nothing, several plans may be active.
The old text claimed "a plan never appears active while it cannot be
tracked", which the destructive PATCH bypass had made false and which
externally edited Markdown cannot guarantee. The "What a plan does not do
yet" list is untouched, as T1.6 requires.
tasks.md: ticks the review-1 corrections (F1–F6, one commit each, shas
listed) under the review gate; T2.1/T2.2 shrink because `RevisePlan` — and
the Web field/milestone PATCH on it — shipped with the corrections, and note
the Phase 2 follow-up of folding `store.record_learning`'s validation into
the seam's copy once MCP `record_plan_learning` migrates.
…, six seats, arbitration Why: the owner mandated that implementation, test results and documentation are reviewed by a council including GPT Astra and Grok 4.6 plus a best-for-purpose seat. This records both reviews and what was done about each finding, so the gate decision is auditable. Code review 1 (Phase 0 + 1 seam): GPT Astra REJECT on a real fourth door — a compound PATCH composed a seam transition with a second unguarded save, reproduced by hand (200/active/0 milestones/ready=false) before acceptance. Six findings F1–F6 fixed at 705ba58b..f827f69c; verified 422/200/422/409 on the probe, 4629 tests green. Grok's first run exhausted 16k tokens on hidden reasoning (manifest.run1.json kept); the 40k re-run is the seat weighed. qwen3-coder was the code seat. §5 receipt review: unanimous that adopt:false is the correct reading of the frozen rule, and that the historical +0.142 was mostly the crash fix main already has. judge() now fails closed (crash reject, registered pair, finite CI) with the frozen verdict re-derived byte-identical; ADR-0011 wording separates decision from execution. deepseek-r1 was the stats seat. Gate: Phase 0+1 accepted as the Phase 2 base; §5 complete as measured. The detect-secrets exclude now also covers the kept manifest.run1.json.
10 tasks
…l-history sweep clean Why: a Bedrock bearer token and a GitHub token are temporarily present on this machine (outside the repo) while the harness checks for #21 run. The owner's rule is that neither may reach any output or any file in the repo. detect-secrets is entropy-based; trufflehog adds provider-specific detectors (AWS/Bedrock, GitHub, OpenAI, Anthropic, ...) as an independent layer. What: scripts/security/trufflehog_redacted.py wraps `trufflehog git file://. --since-commit=HEAD --results=verified,unknown,unverified --json` and prints detector / commit / file:line only -- never the raw match -- so a caught secret cannot leak through the hook's own output. Registered as a local system hook (always_run, pass_filenames: false). Measured before trusting it: a planted random ghp_ token in the index is caught (exit 1, reported as "Staged _planted.txt:1", value never printed); a clean tree exits 0. `--results=verified,unknown` alone silently DROPPED the planted key (verification fails on a fake), hence `unverified` is included: a revoked or fake key still leaks the shape of a real one. Full-history sweep (`--history`): 19 unverified hits, all inspected, all false positives -- 32-hex substrings of sha256 digests in receipts and minified vendor JS (Box/Phrase), the scrubber's own connection-string fixtures (MongoDB/Postgres), and an already-allowlisted `user:pass@` test URI. No AWS, GitHub, Anthropic or LiteLLM credential in ~2,100 commits.
…g from command output The two 'Execution pending' lines were written before the actions ran, per council review (decision separated from execution). Both actions have now run; this records their output. The remote branch survives a ruleset that blocks deletion on every branch — noted rather than worked around.
…1 execution record
…lan, assess, guidance T2.1. Two new contract files, written against the seam and not against any adapter, so the same invariants hold from the Web API, the CLI and the MCP tools once those migrate in T2.2: * tests/test_plan_application_mutations.py — SetMilestone is idempotent and one write; an unknown or negative index is InvalidMilestone with nothing written; the resulting-document gate still applies to an active plan. DeletePlan needs confirmed=True (InvalidField otherwise), removes the canonical document and the index row, keeps the durable checkpoint log, and returns a frozen DeleteResult — a PlanDetail cannot describe a plan that no longer exists (review-1 GPT hazard). assess() wraps the Phase-0 evaluate_and_record / evaluate_plan and reports the two sinks on a frozen AssessmentResult: preview writes neither; record=True reports both saved; a False/raised DB write and a failed document write are reported independently, with the evaluation always returned (D-1, D-3). browse over a directory with a malformed document shows exactly what the store shows. The learning-record rule is the store's single copy reached through the seam; PlanDetail.learning_record_matching lets adapters report "created". reindex() is the one index write an adapter may still reach. * tests/test_plan_guidance.py — get_active_guidance: one ActivePlanGuidance per active plan, ordered by plan id, non-active skipped; match_keys are casefolded, punctuation-stripped topics + every milestone concept (equality, never substring); target_urgency buckets overdue/soon(≤7)/ later/undated against an injectable `today`; completion_action only when every milestone is done; malformed documents become warnings, never exceptions; views frozen and JSON-fresh. Seen failing on a486230: both modules fail at collection with ImportError (AssessPlan / DeletePlan / SetMilestone / AssessmentResult / DeleteResult / ActiveGuidance / normalise_match_key do not exist yet). Line-level pyright suppressions mark the planned symbols per the review-1 process convention; the GREEN commit removes every one of them.
…ce() on the seam T2.2 (seam half). GREEN for 9285260: 45 new tests pass, plan-filtered suite 581 passed, pyright 0. Every RED-only pyright suppression is removed. Intents: SetMilestone(plan_id, index, done) is set-not-toggle so a retry is safe; an index the plan does not have — past the end or negative — is InvalidMilestone with nothing written, and the resulting-document gate still applies to an active plan. DeletePlan(plan_id, confirmed=False) refuses with InvalidField unless confirmed: deletion is the one irreversible write, so the caller says so in the intent. AssessPlan is deliberately not in PlanIntent — it goes to assess(), because an assessment returns an evaluation plus a report on two sinks, not the plan as it now is. Views: DeleteResult, because a PlanDetail cannot describe a plan that no longer exists (review-1 GPT hazard). PlanEvaluationView freezes the evaluation field-for-field and serialises to exactly PlanEvaluation.to_dict() so the REST/CLI shapes do not move when the adapters delegate; database rows are frozen leniently (isoformat()/str() for a non-JSON leaf) because the checkpoint log has always been written with default=str. AssessmentResult reports db_write / document_write ∈ not_requested|saved|failed independently and keeps the evaluation's warning strings; recording_complete is "no requested sink failed" (vacuously true for a preview). No PartialRecording exception, no second checkpoint writer: assess() calls the Phase-0 evaluate_and_record / evaluate_plan and reads the two Bug-B warning strings back into the sink fields. ActivePlanGuidance / ActiveGuidance give the `now` ranker one entry per active plan, ordered by plan id, with the next unchecked milestone, normalise_match_key() over topics + every milestone concept (equality, never substring), target urgency overdue/soon(≤7 days)/ later/undated, energy floor, a completion action when every milestone is done, and warnings for what was worked around. `today` is injectable for frozen-clock callers; the unparseable-document case is a collection warning. Store: append_learning_record(plan, ...) is factored out of record_learning as the single copy of the learning-record rule (validation + idempotent numbering); record_learning wraps it and saves only when a record was created, so the byte-level no-op holds. The seam's duplicate of that rule is deleted and _revise now calls the store's function on the candidate. PlanDetail.learning_record_matching(spec) lets an adapter report `created` without carrying its own copy of the identity rule. Also: PlanApplication.reindex() so `plan reindex` can drop its index import (D-6), and apply() gains @Overloads so DeletePlan → DeleteResult and every other intent → PlanDetail are precise for adapters. The Web route's _apply helper narrows its parameter to PlanDetailIntent — the one adapter line this commit touches, so the workspace type-check stays green between the seam landing and the route migration that follows. Finding recorded for the owner, out of scope here: the Markdown parser's concepts regex stops at the first ')' so a concept literally named "RANK()" does not round-trip; the tests use a punctuation-bearing concept without parentheses instead.
…nes, delete confirmed
Pins what changes once evaluate / toggle / DELETE delegate to the seam
(T2.2, Web half). test_web_plans.py stays frozen at its pre-seam assertions
(D-3); this file carries the new contract:
* POST /plans/{id}/evaluate returns db_write / document_write and an honest
`recorded` — Bug B (issue #7) was a bare `true` over a failed write; a
failed database write still returns 201 with the evaluation, because the
evaluation succeeded and the client is entitled to it;
* the checkbox toggle is an idempotent SetMilestone behind the route, and an
out-of-range or negative index is the seam's InvalidMilestone → 404 with
the document untouched;
* DELETE applies a confirmed DeletePlan (the verb is the confirmation this
route has always had), keeps the durable checkpoint log, drops the index
row, and maps a malformed id to the seam's 400.
Seen failing on fed155c (route still on direct store/evaluation calls):
6 failed, 4 passed — the four passes are pre-existing behaviour (preview
writes nothing; out-of-range 404; unknown-phase codes) that the migration
must preserve.
T2.2 (Web half). GREEN for ccfe1d1: test_web_plans_seam.py 10 passed; test_web_plans.py and test_plan_surface_parity.py unchanged and green (50 in total); `git diff 3a4f6b01 -- tests/test_web_plans.py` is empty. web/routes/plans.py now imports nothing from planning.store, .index, .authoring or .evaluation (D-6) and holds no rule of its own: * GET/POST /plans/{id}/evaluate → PlanApplication.assess(AssessPlan). The route-local phase check is gone: an unknown phase is the seam's InvalidField (400), judged after the plan is found (404 first, like every write). POST reports `recorded` as recording_complete plus db_write / document_write, so a client can no longer read a bare `true` over a failed write (Bug B, issue #7). Response body keys are additive; status stays 201. * POST /plans/{id}/milestones/{i}/toggle → SetMilestone(done=not current). The interim full-list RevisePlan trick from review-1 F1 is replaced; the index check is the seam's InvalidMilestone → 404, so a negative index is refused the same way as one past the end. * DELETE /plans/{id} → DeletePlan(confirmed=True): the HTTP verb is the confirmation this route contract has always had, so the existing 200/404 behaviour is preserved while the seam owns the write and the retained checkpoint history. _load_or_404 and the store error imports are deleted.
… through the seam
Pins the CLI half of T2.2. test_cli_plan.py stays frozen at its pre-seam
assertions (exit codes and --json shapes are the agent contract, D-3); this
file carries what changes when the six remaining commands — and the two other
CLI readers of plans, `exercise from-milestone` and `brain publish` — go
through PlanApplication:
* `plan new` is one CreatePlan(status="active"|"draft") and the --activate
refusal is the seam's PlanNotReady with nothing written — council review 1
(Grok) found the command still drafted, gated and wrote itself, a second
policy site D-2 forbids; the --json shape keeps plan/readiness/path;
* `plan interview` is prepare_planning, still emitting {"questions","seed"};
* `plan evaluate` is assess(); --record prints "Checkpoint recorded." only
when every sink saved and otherwise names the failed sink, exit 0;
* `plan milestone` is an idempotent SetMilestone (set twice stays set; no
flag toggles) and a negative index is refused like one past the end;
* `plan record` is RevisePlan(learning_record=...) and still reports
`created` honestly on a retry; an empty title is the seam's Invalid value;
* `plan reindex` calls PlanApplication.reindex();
* `exercise from-milestone` reads through inspect(); `_selected_plan_ids`
in _brain browses through browse().
Seen failing on da0026f: 14 failed, 2 passed (the JSON-shape test and the
exception-text test pass on the old code by coincidence; the apply-spy tests
are the discriminating ones).
…em 3 design settled, next steps Written when the session stopped on model availability. Carries the verification commands, the eight commits with the receipt that backs each, the Kiro tools probe as the one evidence-based departure from the parent handover's wording, the settled item-3 design decisions (husks() read, PlanSummary.ready as an 18th key with StudyPlan.summary() moving with it, the brief threaded through ctx.invoke's extra kwargs, brief_intro on the persona builder), the item-4 code map, and the session's hard-won lessons. Ticks T2.1/T2.2 in the change's tasks with their receipts.
…lls import _load_dotenv_once() walks from cwd up six parents and calls is_file() on each candidate .env unguarded. Run studyloop from anywhere under a directory the process may not traverse and every entry point dies before main() with PermissionError -- the documented contract is "silent no-op". How it was found: with cwd under ~/.kiro/crew/… (KiroCrew's private tree, which is NOT a StudyLoop harness -- only kiro-cli is), the walk reached ~/.kiro/crew/.env and raised EPERM. The four TestDotenvCannotSetTheTestHatch reds recorded on 2026-09-11 as "sandbox-only, pass in CI" were this: pytest's tmp_path sits under that tree in the sandbox and under /tmp in CI. They pass here now with no test change. The walk itself (cwd-relative, loads whichever .env it meets first, including another tool's) is a separate question for the owner; this commit only stops the crash. Two tests inject EPERM at the stat boundary in a fresh interpreter and prove the skip and the keep-walking behaviour; both fail without the fix. (cherry picked from commit daf46c8)
…es hold The handover pinned the grant spelling (tools = visibility, allowedTools in @server/tool = trust, mcp_server_tool inert) to kiro-cli 2.21.4 and asked for the two /tools probes to be re-run after any upgrade. The CLI moved to 2.22.0 overnight; both probes reproduced the 2.21.4 output exactly, so item 1's grants and the mentor fix stand. Records the hazard hit on the way: an empty MCP section with a "did not load" banner was the studyloop server dying at import (the dotenv EPERM crash, now cherry-picked as bfe0695), not a rule change. The next re-runner should read kiro-chat.log for "Error loading server" before concluding anything about tools/allowedTools.
Deviation 12 stays: an active-but-unready document (a "husk") refuses every write until it is paused or repaired. Until now nothing told the learner one existed before they hit the refusal. These tests pin the discovery and the guided repair the owner asked for, against the settled design §3: - doctor: `check_study_plans` under the existing `config` category — one warn row per husk naming id, title, blockers and an honest provenance hint (pre-gate `created` → "predates the readiness gate"; otherwise the seam "cannot tell how it got that way", never "hand edit"); fix_hint names both `plan repair <id>` and `plan status <id> paused`; fix_auto False. All-ready → one pass row; no plans → info. Registered, not just defined. - seam: `PlanApplication.husks()` — read-only, active ∧ ¬ready, browse order, drafts and paused-incomplete plans excluded. `PlanSummary.ready` as the 18th key on both `to_json_dict()` and `StudyPlan.summary()`, so the D-3 legacy-dict pin keeps holding. - CLI: `plan list` marks a husk `active !`, `--husks` filters, `--json` rows carry `ready`; `plan repair <id>` launches the architect through the one launch chain with `brief` (first section `### Repair: what this plan is missing` = exactly readiness.blockers, then the plan as it stands) and `brief_intro` naming a PLAN REPAIR session — and writes nothing. Ready plan → "Nothing to repair"; unknown id → the seam's not-found. The husk refusal names both exits. - web: `GET /api/plans` rows carry `ready` (18 keys). - persona: `build_canonical_persona(brief_intro=)` — None keeps today's planning sentence byte-for-byte (the Web door's persona_hash must not move); a given intro replaces it and keeps the data-not-instructions framing; an intro without a brief renders nothing. 17 failed / 120 passed across the five files; every failure is the intended missing symbol, unknown command or missing key (verified with --tb=line). One test was corrected mid-RED: its Rich-row parser matched nothing (rows start with a box-drawing bar), so it failed on the parser rather than the marker — it now reads the Status cell by id. Pyright suppressions sit only on lines naming a not-yet-existing symbol, per the handover; remove them in GREEN.
An active plan that fails the readiness gate (a "husk") refuses every write until it is paused or repaired, and until now nothing told the learner one existed before they tripped over the refusal. Deviation 12 stands; this adds the discovery and the guided way out, all through the one seam. Read: PlanApplication.husks() — active and not ready, storage-pinned identity (loaded through _load, like get_active_guidance) so the repair hint always names the file that produced it; browse order. PlanSummary gains `ready` as its 18th key and StudyPlan.summary() gains it too, so the D-3 legacy-dict pin holds with the contract grown by one key on both sides (plan list --json and GET /api/plans). Surfaces: doctor's check_study_plans() under the existing `config` category (one warn row per husk with id, title, exact blockers, provenance, both exits; pass when every active plan is ready; info when none is active); `plan list` marks a husk `!` after its status and gains --husks; the Web sidebar marks it from the row's own `ready`; the husk refusal names `plan repair <id>` beside `plan status <id> paused`. Repair: `plan repair <id>` is the architect launch through the one chain — ctx.invoke(study, …) with `brief`/`brief_intro` as plain keywords on study() (not click options) threaded to build_canonical_persona. The brief's first section lists exactly readiness.blockers; the intro says PLAN REPAIR and the default intro is byte-for-byte unchanged so the Web door's persona_hash does not move (pinned). The command writes nothing. Provenance is one sentence shared by doctor and the brief: "predates the readiness gate" only when `created` parses before READINESS_GATE_DATE; otherwise the seam admits it cannot tell. It lives in views.py (a sentence about a verdict), while the gate date stays in authoring.py (policy) — the architecture guard's covering test flagged the authoring placement, and this is the structural answer rather than an allowlist entry. Persona: a "Repairing a Plan" section that is honest about the gap — no tool writes the mission, so a mission blocker is fixed by the learner's edit while milestones go through update_study_plan, and a partial write on an active plan is refused — with the three projections re-projected and the manifest regenerated (`updated` restored on the 20 entries whose hash did not move). Secrets baseline refreshed with the hook's exclude regex: 72 results files before and after; only the two manifest hashes changed. Verification: 17 RED tests green; full suite 31 failed / 7211 passed / 14 errors, and every one of those 45 ids also fails on an untouched 6f05be5 control worktree (comm on the sorted id sets: item3 − control = ∅, control − item3 = exactly the 17 REDs); just lint, just typecheck, mkdocs --strict and openspec validate clean.
…s; file item 3b (unbuilt) Design §3 gains the two decisions taken with the owner present at GREEN: `plan repair` on a non-active plan exits 0 with a pointer to the architect (a draft is unready by nature, not a husk), and `husk_provenance` lives in views.py because it is a sentence about a verdict while the gate date is policy in authoring.py — the placement the architecture guard's covering test asked for. §3b files the gap the owner's question surfaced — `readiness()` has three blocker classes but `RevisePlan` has no mission fields, and the only mission writer is the Web PATCH route — as a follow-on with its own RED/GREEN rather than a widening of item 3, so item 3's finish stayed countable. Marked unbuilt in both files; T3b.0 is an owner decision on whether the CLI gains a matching `plan revise`.
… 3b)
readiness() has three blocker classes but RevisePlan carried no mission
fields, so the architect `plan repair` launches could only dictate the fix
for the commonest husk (no mission). These tests pin the writer that closes
that, on the doors the owner chose (MCP and Web; no CLI `plan revise`,
T3b.0):
- application: why/success/constraints/out_of_scope on RevisePlan, applied
to the one candidate and saved once; None leaves as is and a list replaces
the whole list; a mission repair on a draft flips readiness without
activating; on an active husk a partial mission write is PlanNotReady with
the one remaining blocker and nothing written, and both fields in one call
are saved once and the plan stops being a husk; a bare string for a list
field is InvalidField before any write (built in the test body, not a
parametrize, so a missing field fails one test rather than the file's
collection).
- MCP: update_study_plan passes the four fields to RevisePlan, and omitted
ones arrive as None, never as "" or [].
- Web: PATCH /api/plans/{id} carries them to the seam — the partial write on
a husk is the seam's 422 with the remaining blocker, the whole write lands
and the list row flips to ready; a string where a list belongs is 400.
10 failed / 195 passed; each failure is the intended missing field,
unexpected keyword, or — on the Web — the body keys being silently dropped
today (a partial PATCH still reports both blockers; a bad `success` is 200).
…em 3b)
RevisePlan gains why / success / constraints / out_of_scope, applied to the
one candidate's Mission after every field is validated and before the single
readiness gate, saved once — the same contract as topics and milestones:
None leaves as is, a list replaces the whole list stripped of blanks, a bare
string where a list belongs is InvalidField before any write. The
duplicate-record short-circuit now also sees mission updates, so a mission
edit beside a repeated learning record is never dropped.
update_study_plan (MCP) and PATCH /api/plans/{id} (Web) expose the four
fields on the same intent; the CLI deliberately does not (owner decision
T3b.0, 2026-09-17: MCP and Web only). With this every blocker readiness()
can name — mission why, success criteria, milestones — is repairable with
the tool the architect already holds, so `plan repair` on the commonest husk
(no mission) is a repair rather than dictation.
Contract changes, each recorded where it is pinned: the update_study_plan
schema grows by four properties (test_schemas_carry_the_design_signatures);
docs/agent-install.md's review-5 pin flipped with the schema it was grounded
in — the doc now lists the mission among what MCP revises and the row names
every property (renamed test_agent_install_doc_promises_exactly_what_
update_study_plan_revises). Persona: the repair table's mission row is a tool
write, the two Revise rows drop "not the mission", the gate paragraph gains
the one-call alternative and keeps the pause path (pinned); projections
re-projected, manifest regenerated with `updated` restored on the 20 unmoved
entries, secrets baseline refreshed whole-repo (72 → 72 files, two manifest
hashes). docs/study-plans.md loses "mission changes only by editing its
Markdown" and the husk bullet's dictation limit.
Verification: 10 RED → green; twelve-file targeted run 478 passed; full
suite 31 failed / 7219 passed / 14 errors, and against a clean 35d890e
control worktree run in parallel: item3b − control = ∅, control − item3b =
exactly the 10 REDs, and the 45 shared ids are byte-identical to the
item-3 environmental set; just lint, just typecheck, mkdocs --strict and
openspec validate clean.
… §3b recorded as built
T3b.0 records the owner's decision verbatim ("Do 3b first, MCP and Web
only"): no CLI `plan revise`. §3b moves from proposal to record — what was
built, the None/whole-list/bare-string rules, the two contract pins that
flipped with the design and where each is now asserted.
…g, not a failure `exporter_schema` reported `fail` whenever ~/.local/bin/session-export was absent, unconditionally. That verdict is right when a session database exists — hooks are (or were) capturing history and now every run fails silently, the incident the check was born of on 2026-09-12 — and wrong when there is no database at all: nothing has ever been captured, so nothing is being lost. That second case is every fresh install, and it is exactly what the release `install-smoke` job builds (a wheel in a fresh venv, empty HOME): the job has been red on every run since the check landed, including PR #20's runs at 1f97352 and 44899c5, with only "unexpected doctor status: fail" to go on. Reproduced locally by running the installed wheel's `doctor --json` under `env -i HOME=<empty>` and a venv-only PATH: 1 fail row, harness/exporter_schema, "pinned exporter … is missing; every export hook fails". With this change the no-database branch returns `warn` + `studyloop install tools` (fix_auto=True), the same shape as the sibling "session-export: not found on PATH" row, and the database-present branch keeps its `fail`. scripts/smoke-installed-cli.sh now passes end to end in both the runner-like environment and the local one. Doctor family + install contracts: 193 passed; the one failure (test_doctor_second_brain::test_rows_vault_missing_warns) is in the recorded sandbox-environmental set on every control run and predates this change.
…re any fixture The kiro lane promised a named skip when STUDYLOOP_ACC_HARNESS did not name kiro, via a class-level autouse fixture calling require_harness(). That fixture is function-scoped, and pytest builds higher scopes first, so the lane's session-scoped Playwright `browser` (behind _acp_auth_context) launched before the skip ever ran. On the CI `test` job — no browsers installed; only the e2e jobs run `playwright install` — that is a fixture error where a skip was promised, and test_acceptance_selection::test_kiro_lane_named_skips_when_harness_not_selected failed on exactly that (PR #20 runs 35214968238 and 35216220593). Locally it passed only because browsers happen to be installed. A marker is evaluated before any fixture of any scope, so the lane module now carries `not_selected_marker("kiro")` in its pytestmark; the class fixture stays as the second line and its docstring no longer claims an ordering it cannot give. Validation outranks selection: the marker skips only when the selection is valid and excludes the harness, so an unknown value still reaches _acceptance_gate and fails loudly (TestUnknownValuesFailLoudly drives two tests in this very module — the first cut of the marker masked them, and that test caught it). Proved by stashing under PLAYWRIGHT_BROWSERS_PATH pointed at an empty dir: the selection test fails before the change and passes after; the whole selection file passes with and without browsers; with kiro selected the mechanics tests still run (2 passed); with an unknown value the gate fails loudly (2 errors naming the value).
…-failure test test_planner_patch_restored_after_tool_error called monkeypatch.undo() to lift its `_run` explosion, which reverts every patch on the fixture — including the autouse STUDYLOOP_CONFIG that gives this module its `memory.default_scope: unclassified`. The follow-up McpArm then read whichever scope the machine had: fine on a developer box with a configured scope, `scope_unconfigured` under a fresh HOME — which is CI, and also why the test sat in the local full-suite "environmental" set on every control run while passing in isolation. The explosion now lives in its own monkeypatch.context(), so only the transport patch is lifted and the hermetic config survives. Reproduced and proved with HOME pointed at an empty directory: 1 failed / 6 passed before, 62/62 after; 62/62 in the normal environment too.
…ging the agent startPlanning() navigated to the Study Session console, then startSession() returned before any fetch with "Select an agent to continue." whenever `this.agent` was still unset — and it is only set once init()'s /api/session/options has resolved. On a cold server a "Plan with architect" click can beat that fetch: the learner lands on the console with no session and a refusal for a choice they were never offered. In CI this was the first e2e test of test_web_plan_architect_journey.py failing on both PR #20 runs (F......... — a navigated page and no POST, then a 20 s timeout) while the nine warm-server tests passed; locally the cold start answered in 60 ms, so it never showed. init() now records the options settlement as _optionsReady (resolved in a finally, never rejecting), and a planning start with no agent awaits it before the check. No agent after the options resolve is still the same named refusal. A focus start is untouched: its Start button is disabled until an agent exists. Proved in a real browser by holding /api/session/options (no continue), clicking, and asserting no POST and no refusal, then releasing and requiring exactly one 201: fails on the old code with "Could not start the architect: Select an agent to continue.", passes with the fix. That scenario is now a permanent journey test; the JS unit suite gains the same race and the no-agent-after-options case (135/135). Journey module 11 passed on ten consecutive runs; one earlier run of the module had a single failure whose identity was not captured before it passed again — recorded, not explained.
main's last green e2e (run 34160855304) ran 515 tests in 14m23s. This branch runs 567: 21m42s on run 35216220593, and run 35217712505 was cancelled by the 25-minute ceiling at 25m16s with the test matrix green — a timeout, not a verdict. Forty minutes is ~1.6x the measured run: still a real guard against a hung browser, no longer decided by runner speed.
The `.brain-active` class arrives with refreshBrain()'s async
/api/second-brain/launch-target response; until then settings-panel.js
deliberately renders every card muted ("including every card while state is
still loading"). The test read count() the moment the section was visible,
so on a slow runner it measured the loading state and asserted 0 == 1 (PR #20
run 35220795456; the same test passed on run 35216220593 — a race, not a
regression). Wait for the first active card to attach, then assert.
This was referenced Sep 18, 2026
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.
Draft so CI runs on every push, following the #18/#19 convention. Not ready to merge until Phase 6 (#15).
What this branch is
Closes the two bugs issue #7 named as must-fix-first and lands the
PlanApplicationseam (#8) they exist because of. Tracked inopenspec/changes/plan-application-seam/(proposal, design, tasks with a per-task definition of done) and the council record underdocs/architecture/plan-integration/council/.evaluate_and_recorddiscardedrecord_checkpoint's boolean; a failed DB write reportedwarnings=[]. Fixed at the caller (D-1).RevisePlanforward.planning/{errors,views,intents,application}.py; Web and CLI list/inspect/activate/create/replace/revise routed through it; the route file has zeroreadiness()/save_plancalls.Evidence
test_web_plans.py,test_cli_plan.py,test_planning_evaluation.py) are byte-identical to the RED commit.just lint,just typecheckclean.review-1-arbitration-2026-09-15.md.Still to land on this branch
Phase 2 (#9:
SetMilestone,DeletePlan,assess,get_active_guidance, AST architecture guard, remaining CLI migration) → Phase 3 (#10 plan-awarenow∥ #11 six MCP tools ∥ #13a planning purpose) → Phase 4 (#12 ∥ #13b) → Phase 5 (#14 Web architect) → Phase 6 (#15 reconcile; closes #7–#15).Related: #7 #8 #9 #10 #11 #12 #13 #14 #15