feat: make the onboarding skill accurate and the first-light test executable (R-042) - #16
Merged
Merged
Conversation
Skill Judge review (inline + independent subagent, both 108/120) applied as the merged priority list: - description: name the re-entry triggers (doctor skill drift, up port conflict, zero connects) so the skill also activates mid-failure, not only on 'embed offbook' - locator: expand a leading ~ in .installed-from's sourcePath before the existence check — the stamp relativizes the homedir, so a literal check false-negatives and falls through to cloning - first light: keep topics --json (bare topics silently falls back to the bundled demo spec); the acceptance test reads the last clientId on status's clients: line, not a bare nonzero count; the violation demo gets concrete mechanics (copy the example: line, break one required field, resend via --payload) - a consolidated Never list with the whys; guides named by file once Verified: check-docs ok (skill-verb and frontmatter gates), full bun test green.
Design record for the two confirmed skill-branch findings from the 2026-08-12 adversarial review of bffa6c1: the false 'silently falls back' rationale, and the unexecutable last-clientId acceptance test. Resolved: skill-only wording fix; delta-form acceptance check with an offbook-logs remediation branch; the refined criterion also lands in adoption.md §10 and the wiring guide so the derived skill is not its only carrier.
…-12, a+b) Fork a: bare topics falls back behind a printed note, not silently; the --json refusal rationale stays. Fork b: the acceptance test is now a delta (note the clients: count, start the app, watch it increment), step 4 captures a configured clientId as a bonus signal, and the offbook-logs ws-connect lines are the remediation branch when several clients interleave.
…n (intake 2026-08-12, c) adoption.md §10 records why (the clients line carries only the last id) and the wiring guide teaches the adopter-facing version, so the derived skill is no longer the only carrier of the executable check.
…2026-08-12) The walk showed offbook logs prints the whole accumulated log across restarts; run-scoping comes from the boot line. The remediation clause now says so instead of claiming the verb itself is run-scoped.
Names the three landing commits, preserves the live-walk acceptance evidence in-repo (the scratch report is untracked), and records the standing of the deferred review findings so the archived record stays honest.
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 this PR does
This PR changes three documents and adds one record. The changes make the onboarding skill (
skills/offbook-onboard/SKILL.md) accurate. They also make the first-light acceptance test executable. The code does not change.Why
Two review rounds examined the skill. The first round scored the skill. The second round attacked each finding. The rounds found two defects that remained:
offbook topicsshows the demo data "silently". This was not correct. The tool shows a note before the demo data.lastclientId is the app's id. This check was not possible. The status line keeps only the last connect. No step finds the app's id.The commits
bffa6c1— This commit improves the skill from the first review round. The description gets trigger words for failure states. The docs locator tells the agent to expand a~prefix. Step 6 gets a sharper acceptance test. A "Never" list collects the prohibitions and their reasons.21aaca9— This commit adds the design record:docs/archive/intake/2026-08-12-first-light-acceptance-fixes.md.5cdfd07— Step 6 now uses a delta check. The agent reads theclients:count. The agent starts the app. The agent reads the count again. If the count increases by one, the app is connected. Step 4 now records the app's clientId when the app sets one. The word "silently" is removed.5536dfb— This commit puts the same procedure indocs/specs/adoption.md§10 and indocs/guides/wiring-your-service.md. Now the skill is not the only document with the procedure.eafb636— A live walk found one more error.offbook logsshows the full log file, not one run. Only the boot line sets the run limits. The text now says this correctly.eca5a13— This commit adds an addendum to the design record. The addendum names the commits. It keeps the walk evidence. It records the open items.Test evidence
A live walk did each instruction of step 6 against a real mock server. All nine steps gave the expected results:
lastid was the app's id.ws-connectline.validationcommand showed the violation.offbook down, bareoffbook topicsshowed the fallback note.The walk found the
offbook logserror. Commiteafb636corrects it. The addendum in the design record keeps the full walk evidence.Gates
bun scripts/check-docs.ts: exit 0.bun test: 570 pass, 0 fail, exit 0.Open items
These items are known and are not part of this PR: