Skip to content

skills: corpus simplification sweep - #24

Merged
gontzess merged 3 commits into
mainfrom
steve.gontzes/no-ticket/skills-corpus-simplification
Oct 8, 2026
Merged

gontzess merged 3 commits into
mainfrom
steve.gontzes/no-ticket/skills-corpus-simplification

Conversation

@gontzess

@gontzess gontzess commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Follow-up to #23 (CXF-353): a cleanup pass over all 10 skills. It addresses the six corpus findings from the #23 review plus the leftover "host wakeup" phrasing.

Changes

  1. Scorer and gate wording removed from skill text. Skills now just state the rule. Text that explained why, in terms of the eval scorer, is gone, e.g. "fails the S11 gate", "the scorer's S10 gate checks the fixture string", "the S8 gate reads the config set via the API", "string check; the scorer's S1 gate does not enforce it", "in the transcript". Those notes now live in a new ## Eval wiring section in each affected SOURCES.md. Two section titles change:
    • design-access-model: "Eval-alignment contract" is folded into its Output Contract.
    • write-connector-source: "Eval-alignment contract" is renamed "Contract rules".
  2. Self-referential exit criteria removed. The "The body contains the literal ..." bullets are gone from SKILL.md. evals/runner/skills_bundle.test.ts still enforces those strings, and SOURCES.md records that it does.
  3. diagnose-authoring-failure no longer contradicts the notification-first wait. Its draft-test section said "poll with backoff". It now says to wait for the draft test as build-and-test describes, then read the evidence row.
  4. "Funnel run" wording replaced with "before or after the OWNER activates".
    • deploy-and-activate: the "Post-activation reference" section is now "After the OWNER activates". Verification can report activation, but force_sync remains prohibited throughout the authoring session, including a notification-resumed session; the human/operator runs production sync after the OWNER activates. The list-revision readback boundary remains explicit.
    • author-in-app-connector uses the same session-wide force_sync rule in its human-boundary and anti-pattern sections. A notification resumes reporting of the already-ACTIVE revision and epoch, never force-syncing. Update-and-rollback and verify-connector-output retain their after-activation framing.
  5. The S2/S3 step-boundary note moved from author-in-app-connector into its SOURCES.md.
  6. Polling wording unified. "Direct API callers without host wakeup" is now "Otherwise (direct API callers)" in build-and-test (build and draft-test waits) and author-in-app-connector (S5, S10).
  7. Scorer disposition removed from consumer text. The S11b/S11c are skipped_human_boundary clauses moved from the two SKILL.md files into their SOURCES.md Eval wiring sections, and skipped_human_boundary was dropped from their locked-literal entries.
  8. Strict force-sync rule unified. All four orchestrator/deploy sites now prohibit force-sync in the authoring session, even after a notification resumes it; the human/operator performs production sync after the OWNER activates. SKILL_LITERALS and both Eval wiring sections were updated in the same commit.

Also removed:

  • build-and-test and author-in-app-connector: the "(the eval fixture records the string "PASS")" asides.
  • deploy-and-activate: the duplicated "Do not force-sync before activation" anti-pattern.
  • verify-connector-output: "fixture counts" now reads "seed-list counts".

Versions

Touched skills receive patch bumps; the bundle receives MINOR bumps (0.9.0 → 0.10.0 for the skipped-disposition cleanup, then 0.10.0 → 0.11.0 for strict sync unification). Source-openapi-spec is unchanged.

Skill Version
author-in-app-connector 0.3.5
build-and-test 0.3.1
deploy-and-activate 0.3.4
design-access-model 0.1.1
diagnose-authoring-failure 0.1.1
read-authoring-contract 0.2.1
update-and-rollback 0.2.2
verify-connector-output 0.1.1
write-connector-source 0.2.1
bundle 0.11.0 (MINOR)

Scenario and test version pins are updated to match.

The locked-literal test now requires the session-wide force-sync prohibition in both author-in-app-connector and deploy-and-activate; it no longer requires the scorer-only skipped_human_boundary marker. For diagnose-authoring-failure, it now checks for the build-and-test wait reference instead of the two old poll phrases.

Verification

The c1 vendored prompts need a re-vendor after this merges. The proposer workflow handles that.

Drop scorer/gate vocabulary and self-referential literal pins from consumer text (eval wiring moves to SOURCES.md), frame post-activation work as before/after the OWNER activates, point diagnose-authoring-failure at build-and-test for the draft-test wait, move the step-boundary note to SOURCES.md, and unify direct-caller polling wording. Bundle 0.9.0.

Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor
Superseded — see the current review report for commit bcc1350347f7

General PR Review: skills: corpus simplification sweep

Blocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0
Criteria: Criteria status: none loaded - .claude/skills/ci-review.md was not found at trusted base ddd3bc937c02.
Review mode: full
View review run

Review Summary

This PR changes wording only, across 9 SKILL.md files. It removes scorer and gate explanations and the "body contains the literal ..." exit criteria, and moves that material into new ## Eval wiring sections in each skill's SOURCES.md. It replaces "funnel run" with "before/after the OWNER activates" and "host wakeup" with "Otherwise (direct API callers)". It also patch-bumps the skill versions and bumps the bundle to 0.9.0.

I scanned the full diff for security and correctness. Every version pin matches across bundle.json, the frontmatter, the scenarios and the tests. Every updated locked literal in evals/runner/skills_bundle.test.ts is present in the new skill text. The never-redeem, never-force_sync and never-list_revision_summaries negations are still asserted. diagnose-authoring-failure now defers to build-and-test's bounded wait, so it still has a poll bound. No repo-local criteria loaded, so I applied only the base criteria.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

  • New (medium confidence): The force_sync / list_revision_summaries ban is now scoped "before the OWNER activates" (skills/author-in-app-connector/SKILL.md:127, skills/deploy-and-activate/SKILL.md:83-84, :48, :69). The old scope was "the funnel run". But both skills also say a notification that resumes the session means the revision is ACTIVE (author-in-app-connector/SKILL.md:111, deploy-and-activate/SKILL.md:51). So a notification-resumed authoring session could read force_sync as allowed, while the S11 gate still fails any force_sync in the transcript (evals/runner/stages.ts:335-337). author-in-app-connector/SKILL.md:109 still says "never call force_sync", so the skill now states two different rules.
Prompt for AI agents
Verify each finding against the current code and only fix it if needed.

## Suggestions

In `skills/author-in-app-connector/SKILL.md`:
- Around line 127: "Do not call `force_sync` before the OWNER activates" conflicts with line 109 ("never call `force_sync`") and with the line-111 notification-resume case, where the revision is already ACTIVE. The eval S11 gate (evals/runner/stages.ts:335-337) fails any force_sync anywhere in the transcript. Reword it so the authoring session never calls force_sync, e.g. "Do not call `force_sync` in the authoring session; the human/operator runs it after the OWNER activates."

In `skills/deploy-and-activate/SKILL.md`:
- Around lines 48, 69, 83-84: Make clear that "a later session" excludes a session resumed by an activation notification, and that the session that minted the approval token never calls `c1_connector_service_force_sync` or `c1_connector_authoring_list_revision_summaries`. Keep the locked literals "Do not call `c1_connector_service_force_sync` before the OWNER activates" and "Do not call `c1_connector_authoring_list_revision_summaries` before the OWNER activates" (or update evals/runner/skills_bundle.test.ts to match).

Comment thread skills/author-in-app-connector/SKILL.md Outdated
Comment thread skills/deploy-and-activate/SKILL.md Outdated
@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor
Superseded — see the current review report for commit c6f9142e0aba

General PR Review: skills: corpus simplification sweep

Blocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0
Criteria: Criteria status: none loaded - .claude/skills/ci-review.md was not found at trusted base ddd3bc937c02.
Review mode: full
View review run

Review Summary

This PR changes wording only, across 9 SKILL.md files. It removes scorer and gate explanations and the "body contains the literal ..." exit criteria, and moves that material into new ## Eval wiring sections in each skill's SOURCES.md. It replaces "funnel run" with "before/after the OWNER activates" and "host wakeup" with "Otherwise (direct API callers)". It also patch-bumps the skill versions and bumps the bundle to 0.9.0.

I scanned the full diff for security and correctness. Every version pin matches across bundle.json, the frontmatter, the scenarios and the tests. Every updated locked literal in evals/runner/skills_bundle.test.ts is present in the new skill text. The never-redeem, never-force_sync and never-list_revision_summaries negations are still asserted. diagnose-authoring-failure now defers to build-and-test's bounded wait, so it still has a poll bound. No repo-local criteria loaded, so I applied only the base criteria.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

  • New (medium confidence): The force_sync / list_revision_summaries ban is now scoped "before the OWNER activates" (skills/author-in-app-connector/SKILL.md:127, skills/deploy-and-activate/SKILL.md:83-84, :48, :69). The old scope was "the funnel run". But both skills also say a notification that resumes the session means the revision is ACTIVE (author-in-app-connector/SKILL.md:111, deploy-and-activate/SKILL.md:51). So a notification-resumed authoring session could read force_sync as allowed, while the S11 gate still fails any force_sync in the transcript (evals/runner/stages.ts:335-337). author-in-app-connector/SKILL.md:109 still says "never call force_sync", so the skill now states two different rules.
Prompt for AI agents
Verify each finding against the current code and only fix it if needed.

## Suggestions

In `skills/author-in-app-connector/SKILL.md`:
- Around line 127: "Do not call `force_sync` before the OWNER activates" conflicts with line 109 ("never call `force_sync`") and with the line-111 notification-resume case, where the revision is already ACTIVE. The eval S11 gate (evals/runner/stages.ts:335-337) fails any force_sync anywhere in the transcript. Reword it so the authoring session never calls force_sync, e.g. "Do not call `force_sync` in the authoring session; the human/operator runs it after the OWNER activates."

In `skills/deploy-and-activate/SKILL.md`:
- Around lines 48, 69, 83-84: Make clear that "a later session" excludes a session resumed by an activation notification, and that the session that minted the approval token never calls `c1_connector_service_force_sync` or `c1_connector_authoring_list_revision_summaries`. Keep the locked literals "Do not call `c1_connector_service_force_sync` before the OWNER activates" and "Do not call `c1_connector_authoring_list_revision_summaries` before the OWNER activates" (or update evals/runner/skills_bundle.test.ts to match).

Reviewed commit: bcc1350347f7

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No blocking issues found — see the full review report

Remove the S11b/S11c skipped_human_boundary clauses from deploy-and-activate and author-in-app-connector; record the wiring in SOURCES.md. Bundle 0.10.0.

Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor
Superseded — see the current review report for commit c6f9142e0aba

General PR Review: skills: corpus simplification sweep

Blocking Issues: 0 | Suggestions: 2 | Threads Resolved: 0
Criteria: Criteria status: none loaded - .claude/skills/ci-review.md was not found at trusted base ddd3bc937c02.
Review mode: incremental since bcc1350
View review run

Review Summary

The new commit (c6f9142) removes the skipped_human_boundary / "S11b/S11c are skipped" wording from author-in-app-connector/SKILL.md and deploy-and-activate/SKILL.md. It moves that note into each skill's SOURCES.md "Eval wiring" list and drops the literal from SKILL_LITERALS in evals/runner/skills_bundle.test.ts. It also bumps author-in-app-connector to 0.3.4, deploy-and-activate to 0.3.3, and the bundle to 0.10.0. Every scenario and test pin is updated to match; a grep found no leftover 0.9.0 pins. The scorer still emits the skipped_human_boundary record rows (evals/runner/record.ts:82, stages.ts:353), so dropping the word from the skill text doesn't change scoring. I scanned the full PR diff (25 files) for security and correctness. These are markdown and test-pin changes only, with no executable or dependency changes. No repo-local criteria loaded, so I applied only the base criteria.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

  • Prior — still present (medium confidence): the force_sync rule still reads two different ways. skills/author-in-app-connector/SKILL.md:109-110 says "never call force_sync", but :127 and skills/deploy-and-activate/SKILL.md:68,82-83 scope the ban to "before the OWNER activates". Both skills also say a notification that resumes the session means the revision is already ACTIVE (author-in-app-connector/SKILL.md:111, deploy-and-activate/SKILL.md:51-53). Meanwhile the S11 gate fails any force_sync anywhere in the transcript (evals/runner/stages.ts:335-337). The existing open thread already covers this, so I didn't post a new inline comment.
  • New (high confidence, docs only): the PR description no longer matches the code. Its Versions table lists bundle 0.9.0, author-in-app-connector 0.3.3 and deploy-and-activate 0.3.2, and it says "All patch bumps". The code is now at bundle 0.10.0 (a minor bump), author-in-app-connector 0.3.4 and deploy-and-activate 0.3.3 (evals/skills-bundle/bundle.json:2,4,8). The description also doesn't mention that skipped_human_boundary was removed from the locked literals. Update the table and the "Changes" list so the re-vendor proposer and reviewers see the right versions.
Prompt for AI agents
Verify each finding against the current code and only fix it if needed.

## Suggestions

In `skills/author-in-app-connector/SKILL.md`:
- Around line 127 (and 109-111): The anti-pattern "Do not call `force_sync` before the OWNER activates" conflicts with line 109-110 ("never call `force_sync`"). It is also ambiguous for a session resumed by an activation notification, where the revision is already ACTIVE. The eval S11 gate (evals/runner/stages.ts:335-337) fails any force_sync in the transcript. Reword it so the authoring session never calls force_sync, e.g. "Do not call `force_sync` in the authoring session; the human/operator runs it after the OWNER activates."

In `skills/deploy-and-activate/SKILL.md`:
- Around lines 47-49, 68, 82-83: Make clear that "a later session" does not include a session resumed by an activation notification, and that the session that minted the approval token never calls `c1_connector_service_force_sync`. If you change the locked literals, update evals/runner/skills_bundle.test.ts to match.

In the PR description:
- Update the Versions table to bundle 0.10.0, author-in-app-connector 0.3.4 and deploy-and-activate 0.3.3. Drop the "All patch bumps" claim, since the bundle is now a minor bump. Mention that `skipped_human_boundary` moved from SKILL.md to SOURCES.md and was removed from the skills_bundle.test.ts locked literals.

@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor
Superseded — see the current review report for commit a9a88695a0c4

General PR Review: skills: corpus simplification sweep

Blocking Issues: 0 | Suggestions: 2 | Threads Resolved: 0
Criteria: Criteria status: none loaded - .claude/skills/ci-review.md was not found at trusted base ddd3bc937c02.
Review mode: incremental since bcc1350
View review run

Review Summary

The new commit (c6f9142) removes the skipped_human_boundary / "S11b/S11c are skipped" wording from author-in-app-connector/SKILL.md and deploy-and-activate/SKILL.md. It moves that note into each skill's SOURCES.md "Eval wiring" list and drops the literal from SKILL_LITERALS in evals/runner/skills_bundle.test.ts. It also bumps author-in-app-connector to 0.3.4, deploy-and-activate to 0.3.3, and the bundle to 0.10.0. Every scenario and test pin is updated to match; a grep found no leftover 0.9.0 pins. The scorer still emits the skipped_human_boundary record rows (evals/runner/record.ts:82, stages.ts:353), so dropping the word from the skill text doesn't change scoring. I scanned the full PR diff (25 files) for security and correctness. These are markdown and test-pin changes only, with no executable or dependency changes. No repo-local criteria loaded, so I applied only the base criteria.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

  • Prior — still present (medium confidence): the force_sync rule still reads two different ways. skills/author-in-app-connector/SKILL.md:109-110 says "never call force_sync", but :127 and skills/deploy-and-activate/SKILL.md:68,82-83 scope the ban to "before the OWNER activates". Both skills also say a notification that resumes the session means the revision is already ACTIVE (author-in-app-connector/SKILL.md:111, deploy-and-activate/SKILL.md:51-53). Meanwhile the S11 gate fails any force_sync anywhere in the transcript (evals/runner/stages.ts:335-337). The existing open thread already covers this, so I didn't post a new inline comment.
  • New (high confidence, docs only): the PR description no longer matches the code. Its Versions table lists bundle 0.9.0, author-in-app-connector 0.3.3 and deploy-and-activate 0.3.2, and it says "All patch bumps". The code is now at bundle 0.10.0 (a minor bump), author-in-app-connector 0.3.4 and deploy-and-activate 0.3.3 (evals/skills-bundle/bundle.json:2,4,8). The description also doesn't mention that skipped_human_boundary was removed from the locked literals. Update the table and the "Changes" list so the re-vendor proposer and reviewers see the right versions.
Prompt for AI agents
Verify each finding against the current code and only fix it if needed.

## Suggestions

In `skills/author-in-app-connector/SKILL.md`:
- Around line 127 (and 109-111): The anti-pattern "Do not call `force_sync` before the OWNER activates" conflicts with line 109-110 ("never call `force_sync`"). It is also ambiguous for a session resumed by an activation notification, where the revision is already ACTIVE. The eval S11 gate (evals/runner/stages.ts:335-337) fails any force_sync in the transcript. Reword it so the authoring session never calls force_sync, e.g. "Do not call `force_sync` in the authoring session; the human/operator runs it after the OWNER activates."

In `skills/deploy-and-activate/SKILL.md`:
- Around lines 47-49, 68, 82-83: Make clear that "a later session" does not include a session resumed by an activation notification, and that the session that minted the approval token never calls `c1_connector_service_force_sync`. If you change the locked literals, update evals/runner/skills_bundle.test.ts to match.

In the PR description:
- Update the Versions table to bundle 0.10.0, author-in-app-connector 0.3.4 and deploy-and-activate 0.3.3. Drop the "All patch bumps" claim, since the bundle is now a minor bump. Mention that `skipped_human_boundary` moved from SKILL.md to SOURCES.md and was removed from the skills_bundle.test.ts locked literals.

Reviewed commit: c6f9142e0aba

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No blocking issues found — see the full review report

Unify orchestrator and deploy guidance, including notification-resumed sessions; delegate production sync to the human/operator and align literals and eval-wiring notes. Bundle 0.11.0.

Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor
Superseded — see the current review report for commit a9a88695a0c4

General PR Review: skills: corpus simplification sweep

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: none loaded - .claude/skills/ci-review.md was not found at trusted base ddd3bc937c02.
Review mode: incremental since c6f9142
View review run

Review Summary

The new commits make the force_sync rule the same everywhere. author-in-app-connector and deploy-and-activate no longer say "before the OWNER activates". They now say "in the authoring session", and they state that a session resumed by a notification also must not force-sync, because the human/operator runs production sync. This matches evals/runner/stages.ts:186-190,336, where isRedemptionCall flags force_sync anywhere in the transcript. The new phrases are added to SKILL_LITERALS in skills_bundle.test.ts. Both SOURCES.md Eval wiring sections describe the rule, and the bundle and both touched skills get version bumps (bundle 0.11.0, author-in-app-connector 0.3.5, deploy-and-activate 0.3.4), with every scenario and test pin updated. I scanned the full PR diff (25 files: skill markdown, eval tests, scenario pins, bundle.json) for security and correctness and found no new issues. The incremental artifact was complete, with no dropped paths and no truncation. No repo-local criteria loaded, so only the base criteria were applied. The remaining force_sync steps in update-and-rollback and verify-connector-output keep their after-activation framing, as the PR description says.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

Resolved prior findings

  • force_sync rule read two different ways — fixed. skills/author-in-app-connector/SKILL.md:110-114,128 and skills/deploy-and-activate/SKILL.md:47-53,65-69,83 now ban force_sync for the whole authoring session, including a session resumed by a notification. The locked literals at evals/runner/skills_bundle.test.ts:80,94 match the new wording.
  • PR description didn't match the code — fixed. The Versions table now lists bundle 0.11.0 (MINOR), author-in-app-connector 0.3.5 and deploy-and-activate 0.3.4. That matches evals/skills-bundle/bundle.json. The description also mentions that skipped_human_boundary was removed from the locked literals.

@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

General PR Review: skills: corpus simplification sweep

Blocking Issues: 0 | Suggestions: 0 | Threads Resolved: 0
Criteria: Criteria status: none loaded - .claude/skills/ci-review.md was not found at trusted base ddd3bc937c02.
Review mode: incremental since c6f9142
View review run

Review Summary

The new commits make the force_sync rule the same everywhere. author-in-app-connector and deploy-and-activate no longer say "before the OWNER activates". They now say "in the authoring session", and they state that a session resumed by a notification also must not force-sync, because the human/operator runs production sync. This matches evals/runner/stages.ts:186-190,336, where isRedemptionCall flags force_sync anywhere in the transcript. The new phrases are added to SKILL_LITERALS in skills_bundle.test.ts. Both SOURCES.md Eval wiring sections describe the rule, and the bundle and both touched skills get version bumps (bundle 0.11.0, author-in-app-connector 0.3.5, deploy-and-activate 0.3.4), with every scenario and test pin updated. I scanned the full PR diff (25 files: skill markdown, eval tests, scenario pins, bundle.json) for security and correctness and found no new issues. The incremental artifact was complete, with no dropped paths and no truncation. No repo-local criteria loaded, so only the base criteria were applied. The remaining force_sync steps in update-and-rollback and verify-connector-output keep their after-activation framing, as the PR description says.

Security Issues

None found.

Correctness Issues

None found.

Suggestions

None.

Resolved prior findings

  • force_sync rule read two different ways — fixed. skills/author-in-app-connector/SKILL.md:110-114,128 and skills/deploy-and-activate/SKILL.md:47-53,65-69,83 now ban force_sync for the whole authoring session, including a session resumed by a notification. The locked literals at evals/runner/skills_bundle.test.ts:80,94 match the new wording.
  • PR description didn't match the code — fixed. The Versions table now lists bundle 0.11.0 (MINOR), author-in-app-connector 0.3.5 and deploy-and-activate 0.3.4. That matches evals/skills-bundle/bundle.json. The description also mentions that skipped_human_boundary was removed from the locked literals.

Reviewed commit: a9a88695a0c4

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No blocking issues found — see the full review report

@gontzess
gontzess marked this pull request as ready for review October 8, 2026 14:30
@gontzess
gontzess merged commit 8953c5e into main Oct 8, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant