fix(skills): restore ask-user-authority skill lost in squash-import - #42
Merged
Freudator86 merged 3 commits intoAug 1, 2026
Merged
Conversation
AGENTS.md section 7 requires the ask-user decision procedure and section 9 requires an evidence-first escalation, but the skill that owns both was never carried into this curated fork, and no AGENTS.md entry pointed at it. Restore the upstream skill verbatim and re-add its section 13 trigger line so the procedure is discoverable at the moment an ask-user finding is decided.
…per-skill trigger assertions
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
2 participants
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.
Intent
Restore the ask-user-authority agent-only skill from upstream (kunchenguid/firstmate) into this curated fork, where it was lost during a squash-import rather than removed deliberately.
The captain decided on 2026-07-28 to restore the missing upstream guidance file rather than write a one-sentence pointer into AGENTS.md instead. AGENTS.md already requires the procedure this skill governs - section 7 on ask-user findings returning as needs-decision, and section 9's evidence-first escalation bar - but nothing pointed at the skill that owns it, and the skill file itself was absent.
Deliberate decisions in this change:
The branch is rebased onto current main (6fa6926) and up to date with it.
The ci step is skipped on the captain's explicit instruction: in this fleet that step loops indefinitely on a non-repo working directory and has never produced a verdict. CI will be verified externally with gh-axi pr checks and reported as externally verified.
This is a documentation, agent-instruction, and test-guard change only. No shell scripts under bin/, no runtime behavior.
What Changed
.agents/skills/ask-user-authority/SKILL.mdverbatim from upstream blobec09871, so the ask-user decision procedure thatAGENTS.mdsections 7 and 9 already require has an owning skill again — it was dropped by a squash-import, not removed deliberately.AGENTS.mdsection 13 trigger line forask-user-authority, placed afterdiagnostic-reasoningand worded like its neighbours; upstream's second inline pointer in the section 7 Validate block was deliberately not taken, since section 13 is this fork's declared trigger location.tests/fm-instruction-owners.test.shto guard the skill against the same loss: the per-skill trigger assertions collapse into a four-fieldownerstable now coveringask-user-authority, and a newtest_ask_user_owner_covers_authority_procedurepins the ownership sentence, both body headings, the 8 authority steps, the 5 escalation elements, and 14 anchor phrases. Both failure modes were proven by removal — deleting the trigger line and deleting the SKILL.md each fail with their own named assertion.Risk Assessment
✅ Low: The follow-up is a test-only change that adds a structural guard and deduplicates existing assertions with no assertion lost, every pinned phrase and count verified to match the restored SKILL.md, and no runtime, shell, or instruction behavior touched.
Testing
Ran the changed guard test (11/11 green) and then proved it can actually fail: both removal modes the intent claims reproduce their exact error messages, and four separate content-degradation modes prove the final commit's body-pinning is non-vacuous rather than decorative. Confirmed the loop refactor did not weaken the three pre-existing skill owners by re-breaking each one individually. Verified the "restored VERBATIM from ec09871" claim by fetching the upstream blob read-only and diffing byte-for-byte — identical sha256, size, and git blob hash. Captured the agent-facing before/after surface showing that at the base commit the section 7 requirement had no owner and no file, and now resolves to a real agent-only skill. Ran the adjacent skill-consuming and fork-drift suites for regressions; all green. No visual artifact applies — this is an agent-instruction, documentation, and test-guard change with no rendered user surface, so the end-user experience is the markdown an agent loads, captured as text. Everything passes and the worktree is clean; the one issue is a factual premise in the intent rather than a test failure: upstream modified this file after ec09871, which the captain specifically asked to have confirmed before landing under the fleet freeze.
Evidence: Guard proven by removal — both failure modes the intent claims, with exact error messages and restored-green confirmation
=== FAILURE MODE 1: delete the AGENTS.md section 13 trigger line === 505:-ask-user-authority- load before deciding any ask-user finding, regardless of the project'syoloposture. --- test output --- not ok - ask-user-authority must have exactly one AGENTS.md trigger entry, found 0 exit=1 --- restored; test output --- ok - ask-user-authority owns the authority procedure and the evidence-first escalation contract exit=0 === FAILURE MODE 2: delete .agents/skills/ask-user-authority/SKILL.md === --- test output --- not ok - ask-user-authority skill is missing exit=1 --- restored; test output --- ok - ask-user-authority owns the authority procedure and the evidence-first escalation contract exit=0Evidence: Verbatim restore confirmed byte-for-byte against upstream ec09871 (read-only gh api fetch)
=== upstream kunchenguid/firstmate @ ec09871 (fetched read-only via gh api) === 651f57fc1fe492ea1b58b25e0add19688ffebef23272170cce844866c49089be upstream-ec09871-SKILL.md 3697 upstream-ec09871-SKILL.md === restored file in this fork @ 33f2e8e === 651f57fc1fe492ea1b58b25e0add19688ffebef23272170cce844866c49089be .agents/skills/ask-user-authority/SKILL.md 3697 .agents/skills/ask-user-authority/SKILL.md === git blob hashes === upstream: d4b63d525bf39edc4e0729e44784d52379e28b7d restored: d4b63d525bf39edc4e0729e44784d52379e28b7d === byte-for-byte diff === IDENTICAL - zero byte difference. VERBATIM RESTORE CONFIRMED.Evidence: Upstream drift after ec09871 — contradicts the intent's "never modified since" premise
=== upstream commits touching this path === daf6dce 2026-07-30T01:06:41Z fix: scope validation corrections by accepted behavior (#1281) ec09871 2026-07-24T00:25:47Z fix: enforce contract boundaries for ask-user findings (#945) === blob shas === restored in this fork (== ec09871): d4b63d525bf39edc4e0729e44784d52379e28b7d upstream HEAD : 38761e6d98a8e975480c6a43e41c596800102350 === diff: restored (ec09871, LEFT) vs upstream HEAD after daf6dce (RIGHT) === -3. Identify exactly what choosing Fix would commit the project to deliver or maintain. +3. Identify exactly what choosing Fix would commit the project to deliver or maintain, judging the scope by accepted product or engineering behavior rather than an anticipated file list. + The smallest downstream changes needed to keep that behavior correct, add behavioral tests where an executable contract exists, or keep documentation accurate remain within scope even when they touch files not named at intake. + Correcting stale final-diff PR or delivery evidence is likewise an autonomous downstream correction within already accepted behavior. === effect on the guard's pinned structural counts, if upstream HEAD were taken === authority steps restored: 8 authority steps upstream: 8 escalation elems restored: 5 escalation elems upstream: 5Evidence: Body-pinning proven non-vacuous — gutting content while keeping file and trigger still fails
=== DEGRADE A: drop authority step 7 (causal-theme escalation) === not ok - ask-user-authority must keep all 8 numbered authority steps, found 7 exit=1 === DEGRADE B: drop escalation element 3 (smallest alternative) === not ok - ask-user-authority must keep all 5 numbered escalation elements, found 4 exit=1 === DEGRADE C: soften the yolo-independence wording in the description === not ok - ask-user-authority skill metadata lost its precise load trigger exit=1 === DEGRADE D: replace body with a stub that keeps only the headings === not ok - ask-user-authority must keep all 8 numbered authority steps, found 0 exit=1 === RESTORED: full suite green again === (11 ok, exit=0) === worktree clean? === (empty == clean)Evidence: Agent-facing surface, base 6fa6926 vs target 33f2e8e — dangling requirement now has a resolvable owner
=== BEFORE: base commit 6fa6926 ==== --- AGENTS.md section 7 REQUIRES the ask-user procedure: --- 314:An ask-user finding returns asneeds-decision; firstmate decides only when the configured authority permits, otherwise escalates to the captain. --- ...but section 13 names NO owner for it: --- 0 occurrences of ask-user-authority in AGENTS.md --- ...and the skill file itself is absent: --- fatal: path '.agents/skills/ask-user-authority/SKILL.md' exists on disk, but not in '6fa6926' === AFTER: target commit 33f2e8e ==== --- section 13 now names the owner (line 505): --- -diagnostic-reasoning- load before scoping a reported bug and before acting on a diagnostic report. -ask-user-authority- load before deciding any ask-user finding, regardless of the project'syoloposture. -harness-adapters- load before spawning or recovering a crewmate or secondmate, ... --- the named owner resolves to a real, loadable skill: --- -rw-rw-r-- 1 captain captain 3697 .agents/skills/ask-user-authority/SKILL.md --- frontmatter proves it is agent-only (not captain-invocable): --- user-invocable: false metadata: internal: trueEvidence: Refactor did not weaken the three pre-existing owners — each still fails independently
not ok - diagnostic-reasoning must have exactly one AGENTS.md trigger entry, found 0 not ok - diagnostic-reasoning skill metadata lost its precise load trigger not ok - project-management must have exactly one AGENTS.md trigger entry, found 0 not ok - project-management skill metadata lost its precise load trigger not ok - secrets-handling must have exactly one AGENTS.md trigger entry, found 0 not ok - secrets-handling skill metadata lost its precise load trigger === all restored === ok (exit=0), worktree cleanEvidence: Intent-constraint sweep — one AGENTS.md line, section 13 only, no bin/ or docs/ changes, rebased on main
=== exactly ONE AGENTS.md line added === 1 0 AGENTS.md === section 7 Validate-block pointer deliberately NOT taken === 505:-ask-user-authority- load before deciding any ask-user finding, regardless of the project'syoloposture. -> single occurrence, in section 13 only. === no bin/ shell scripts, no runtime behavior === .agents/skills/ask-user-authority/SKILL.md | 52 +++++++++++++++++ AGENTS.md | 1 + tests/fm-instruction-owners.test.sh | 89 ++++++++++++++++++++++++------ === docs/fork-patches.md deliberately untouched === (no output == docs/ untouched) === branch rebased onto current main 6fa6926 === 6fa6926 IS an ancestor of 33f2e8eEvidence: Changed guard test green at HEAD 33f2e8e
ok - new internal skills have one precise AGENTS.md trigger each ok - diagnostic-reasoning owns the approved evidence procedure ok - project-management owns registry, delivery posture, consent, initialization, and removal safety ok - secrets-handling owns the dangerous-command doctrine, contained stow-and-clear scope, and escalation triggers ok - ask-user-authority owns the authority procedure and the evidence-first escalation contract ok - generic effort fallback applies only below captain and standing configuration ok - firstmate-coding-guidelines owns compatibility review and deterministic enforcement ok - secondmate registry guidance keeps concise routes and points to the charter ok - state, startup, and ordinary recovery have focused owners and triggers ok - compressed AGENTS.md records the approved one-owner map ok - compressed AGENTS.md retains authority, supervision, AFK, and X safety EXIT=0Evidence: Upstream ec09871 blob, fetched read-only for byte comparison
Evidence: Upstream HEAD blob (post-daf6dce), for the drift comparison
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
tests/fm-instruction-owners.test.sh:46- The new guard pins only the skill's existence, frontmatter, and description sentence — not its decision procedure. All three sibling skills in this file have a dedicated body-content test (test_diagnostic_owner_covers_causal_procedure, test_project_management_owner_covers_guarded_operations, test_secrets_owner_covers_exposure_response) that pins the substantive phrases they own. ask-user-authority has none, so the entire 8-step authority procedure and the 5-element escalation contract in SKILL.md (lines 15-46) could be emptied down to the frontmatter block and the whole suite would still pass. A squash-import can silently revert a file's contents as easily as it can delete the file, and only the deletion mode fails visibly today. Suggested follow-up: a small test pinning a few load-bearing phrases, e.g. 'single owner of the decision procedure for ask-user findings', 'never as authority to broaden the task', and 'The implementation worker never decides or answers its own ask-user finding.'tests/fm-instruction-owners.test.sh:34- Lines 34-49 now repeat the same two-assertion pair (skill description trigger + AGENTS.md trigger line) four times, once per skill, immediately after a loop at lines 24-33 that already iterates the same four skills. The trigger sentences could be carried as two extra fields in the loop's pair data so adding the fifth internal skill is a one-line data change rather than another copy-pasted block. Non-functional test-only dedup; the change itself correctly follows the existing pattern.🔧 Fix: pin ask-user-authority body structure, collapse per-skill trigger assertions
2 infos still open:
tests/fm-instruction-owners.test.sh:39- The newownerstable is a flat array read with a stride of 4, but nothing asserts the array length is actually a multiple of 4. A future entry added with 3 or 5 fields shifts every later record: fields get read from the wrong slot (a path where a name is expected), and the final record indexes past the end, which under this file'sset -uaborts with a raw bash 'owners[N]: unbound variable' instead of anot ok -line fromfail. One line after the array literal keeps the misalignment failing visibly and in the suite's own vocabulary:[ $((${#owners[@]} % 4)) -eq 0 ] || fail 'owners table must carry four fields per skill'..agents/skills/ask-user-authority/SKILL.md:46- Noting the residual scope, not requesting a change: the guard now pins the ownership sentence, both procedure headings, the 8/5 numbered-item counts and 14 anchor terms, but the third body section, '## Classification examples' (SKILL.md:46-52), has no assertion. Deleting those five calibration bullets leaves the suite green. This does not contradict the captain's stated bar - emptying the body wholesale still fails, because the headings and counts go with it - and the pinned scope was exactly what he specified, so this is informational only..agents/skills/ask-user-authority/SKILL.md:22- The intent states the restored file is unchanged upstream ("upstream has never modified the file since"), and records that the captain required this be checked explicitly before landing under the fleet freeze. Verification contradicts it: upstream commit daf6dce (2026-07-30, PR fix: scope validation corrections by accepted behavior kunchenguid/firstmate#1281 "fix: scope validation corrections by accepted behavior") amended authority step 3, expanding it with guidance that scope is judged by accepted product/engineering behavior rather than an anticipated file list, plus two new lines on downstream changes and stale final-diff evidence. Upstream HEAD is blob 38761e6d98a8e975480c6a43e41c596800102350; the restored file is d4b63d525bf39edc4e0729e44784d52379e28b7d. The restore itself is still a faithful repair of the lost file, and choosing ec09871 over HEAD is arguably more consistent with "not absorbing new upstream material" under the freeze, so the landing decision looks sound. But the premise given to the captain does not hold, and there is now an unmade decision about whether the fork should carry the newer upstream text. The new guard will not surface this: both versions have 8 authority steps and 5 escalation elements, so its pinned structural counts stay green either way. Evidence: /tmp/no-mistakes-evidence/01KYWT0014QEM096PQQ97Z4G96/09-upstream-drift-after-ec09871.txtbash tests/fm-instruction-owners.test.sh— the changed guard, 11/11 ok at HEAD 33f2e8eGuard failure mode 1: deleted the AGENTS.md section 13 trigger line →not ok - ask-user-authority must have exactly one AGENTS.md trigger entry, found 0, exit 1; restored greenGuard failure mode 2: deleted.agents/skills/ask-user-authority/SKILL.md→not ok - ask-user-authority skill is missing, exit 1; restored greenBody-pinning non-vacuity: dropped authority step 7 →found 7; dropped escalation element 3 →found 4; softened the yolo wording in the description →lost its precise load trigger; headings-only stub →found 0; all restored greenRefactor regression check: for each of diagnostic-reasoning, project-management, secrets-handling, removed its AGENTS.md trigger line and separately corrupted its SKILL.md description → each still fails with its own named assertiongh api repos/kunchenguid/firstmate/contents/.agents/skills/ask-user-authority/SKILL.md?ref=ec09871→ decoded and diffed against the restored file: identical sha256 651f57fc…, identical 3697 bytes, identical git blob d4b63d5, zero-line diffgh api repos/kunchenguid/firstmate/commits?path=.agents/skills/ask-user-authority/SKILL.md→ upstream history shows daf6dce (2026-07-30) after ec09871 (2026-07-24); diffed restored vs upstream HEADAdjacent-suite regression run:bash tests/fm-brief.test.sh,fm-bootstrap.test.sh,fm-stow-contract.test.sh,fm-private-material-ignore.test.sh,fm-decision-inventory.test.sh— all exit 0Fork/drift suites that track.agents/skills/:bash tests/fm-self-drift.test.sh(9 ok),fm-fork-sync-check.test.sh(11 ok),fm-ensure-agents-md.test.sh(9 ok),fm-nm-test-contract.test.sh(4 ok) — all exit 0Intent-constraint sweep:git diff --numstat 6fa6926..33f2e8e -- AGENTS.md(1 added, 0 removed),grep -n 'ask-user-authority' AGENTS.md(single hit, line 505, section 13 only),git diff --name-only 6fa6926..33f2e8e(3 files, no bin/, no docs/),git merge-base --is-ancestor 6fa6926 33f2e8eAgent-facing before/after capture: base 6fa6926 has the section 7 requirement but zero AGENTS.md occurrences of the owner and no SKILL.md on disk; target 33f2e8e resolves the section 13 trigger to a real agent-only skillgit status --porcelainafter every mutation round and at the end — worktree clean, HEAD unchanged at 33f2e8edocs/fork-patches.md:5- docs/fork-patches.md gets no row for these three commits. The ledger already declares itself incomplete (four rows against pull requests through docs(skills): re-verify codex harness-adapters facts against codex-cli 0.145.0 #28) and carries an explicitly deferred open decision on whether to backfill rows or narrow the per-commit contract, since the fleet-repository work is meant to retire the ledger. Adding one row here would settle that open decision unilaterally and is out of scope; leaving the ledger untouched keeps it consistent with its own stated posture. Follow-up: settle the backfill-vs-narrow decision once, then apply it to all missing rows at once rather than per-change.✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.