Skip to content

awf 0.0.3: cr-received, session-learnings, auto review on the v0.0.2 cr-request - #3

Merged
ihororlovskyi merged 4 commits into
mainfrom
261018-dev-session
Oct 18, 2026
Merged

ihororlovskyi merged 4 commits into
mainfrom
261018-dev-session

Conversation

@ihororlovskyi

@ihororlovskyi ihororlovskyi commented Oct 18, 2026 •

Copy link
Copy Markdown
Member
  • Rename cr-receive to cr-received (0.0.1, first release under the new name); cr-request 0.0.3 hands off to it.
  • Add session-learnings 0.0.1: record reusable session learnings in the right agent instruction file, deduplicated, applied only after approval.
  • Plugin, manifests, marketplace description, and README at 0.0.3.
  • Auto review installs cr-request from the v0.0.2 tag commit (CR_REQUEST_REF).
  • AGENTS.md: when claude-review.yml changes take effect, and the reserved .auto-review/ path.

@yngwald

yngwald commented Oct 18, 2026

Copy link
Copy Markdown
Collaborator

Code review - PR #3: Install cr-request 0.0.2 in the auto review

Target: sentimony/agentic-workflow PR #3. The range runs from merge base 006400f88d768452a9aa07c6736bbbb7bb5d55c3 to head cfa822ed0c574268e79b103434aa6e72143002a8. It is one commit, "Install cr-request from the v0.0.2 tag commit in the auto review", and it changes .github/workflows/claude-review.yml (+3 -4). There is no uncommitted state.
Review mode: independent reviewer, with key claims spot-checked by the requester.
Intent sources: the PR title and description, and AGENTS.md. There is no separate spec.

Verdicts

  • Intent: PASS. CR_REQUEST_REF now points at the v0.0.2 tag commit. The reviewer skill still comes from a released tag and never from the PR head. The stale "bump after release" note is gone, and nothing unrelated changed.
  • System: PASS. CR_REQUEST_REF is used in only one place, the install step at .github/workflows/claude-review.yml:168. That step runs git fetch --depth 1 <repo> "$CR_REQUEST_REF", then checkout FETCH_HEAD, and then copies the skill. A full commit SHA reachable from a tag can be fetched this way, the same as the old pin. At 006400f, cr-request is version 0.0.2 (plugins/awf/skills/cr-request/SKILL.md:6). Its prepared target mode (SKILL.md:32, references/target.md:54) matches the .auto-review/target/ files that the workflow prepares. Nothing else in the workflow mentions 0.0.1 or the old SHA.
  • Craft: PASS. The new comment is accurate. Pinning the full 40-character SHA instead of a tag name means a moved tag cannot change what gets installed.

Findings

None.

Suggestions (optional)

  • Add a # v0.0.2 suffix after the SHA at .github/workflows/claude-review.yml:117, to match the style of the action pins (for example # v5.1.0). This is cosmetic only, because the comment above the SHA already names the tag.

Set aside

  • Release or version bump: none is needed. The change touches only the workflow, not any skill, manifest, CHANGELOG.md or README version. The AGENTS.md version rules apply to plugin and skill releases.
  • Branch naming (YYMMDD-dev-session): this could not be checked from the detached checkout.
  • Fetching the skill from a public URL: this existed before the PR, and pinning an immutable SHA limits the supply-chain risk.
  • README listing cr-receive as 0.0.1: that is the correct current version of that skill, not a stale reference.

Open questions

None.

Review gaps

  • Tests were not run in this job. CI (skill frontmatter, manifests and marketplaces, README versions, hidden Unicode) is reported green on HEAD SHA and was not re-checked here.
  • The tag check was local only. git rev-parse v0.0.2^{} returns 006400f88d768452a9aa07c6736bbbb7bb5d55c3, which is consistent with the PR's git ls-remote claim, but the remote was not queried.
  • This review did not run the workflow end to end with the new pin.

Merge readiness

Ready. The pin is correct, the install step uses it correctly, and nothing else needs updating.

Reviewed with: cr-request skill

cr-received ships at 0.0.1 as its first release under the new name;
cr-request 0.0.3 hands off to it. session-learnings moves in from
skills-aiassist, reworked around placement by scope, deduplication
against the environment, and approval before any edit. awf 0.0.3.
@ihororlovskyi ihororlovskyi changed the title Install cr-request 0.0.2 in the auto review awf 0.0.3: cr-received, session-learnings, auto review on the v0.0.2 cr-request Oct 18, 2026
@yngwald

yngwald commented Oct 18, 2026

Copy link
Copy Markdown
Collaborator

Code review - PR #3: awf 0.0.3: cr-received, session-learnings, auto review on the v0.0.2 cr-request (fresh review pass)

This is a fresh review pass. The same reviewer last reviewed the older head cfa822ed0c574268e79b103434aa6e72143002a8, which held only the workflow pin, and found it ready. This pass reviews the whole range again.

Target: sentimony/agentic-workflow PR #3. The range runs from merge base 006400f88d768452a9aa07c6736bbbb7bb5d55c3 to head 6898cd7e2e9fe64531479ee889bdc96df4eb50ae. It has 3 commits (cfa822e, 706f781, 6898cd7) and touches 15 files, including 4 renames (+169 -23). There is no uncommitted state.

Review mode: independent reviewer. The requester spot-checked the key claims: the search for the old name, the tag commit, CI skill discovery, and the cited lines.

Intent sources: the PR title and description, and AGENTS.md. There is no separate spec.

Risk: low to medium. The change covers skill docs, manifests, the README, and one CI workflow pin.

Verdicts

  • Intent: PASS. All five items in the description are implemented.
    • Rename: cr-receive is now cr-received in the directory, the frontmatter name, and the title. Its version is 0.0.1, and its CHANGELOG notes the rename.
    • cr-request 0.0.3: it hands off to cr-received in both the body and the "Next step" line.
    • session-learnings 0.0.1: added with SKILL.md, CHANGELOG, and an MIT LICENSE for Ihor Orlovskyi. It is listed in the Claude Code manifest skills.
    • Version 0.0.3: both plugin manifests, the marketplace description, and the README are at 0.0.3.
    • Pin: CR_REQUEST_REF points to the v0.0.2 tag commit.
    • AGENTS.md: the rule about when review workflow changes take effect and the reserved .auto-review/ path is added.
  • System: PASS.
    • Old name: no live references to cr-receive remain. The only matches are deliberate history lines, at plugins/awf/skills/cr-request/CHANGELOG.md:5,9 and plugins/awf/skills/cr-received/CHANGELOG.md:5.
    • Marketplaces: neither marketplace carries a version or ref. The changed Claude Code marketplace description matches both plugin manifests word for word.
    • CI: .github/workflows/ci.yml:26 finds skills with */skills/*/SKILL.md, so the new skill is checked with no CI change: name versus directory, version, license, CHANGELOG, manifest list, and README rows. argument-hint is checked only when present (ci.yml:57), so session-learnings does not need one.
    • Pin: git rev-parse v0.0.2^{commit} returns 006400f88d768452a9aa07c6736bbbb7bb5d55c3. The job copies only cr-request from that commit. The older handoff text in that copy (cr-receive) is never used by the review-only job.
  • Craft: FINDINGS. Two minor wording and documentation items, listed below. Otherwise:
    • The edits are small and consistent with the AGENTS.md rules.
    • session-learnings is well structured, with an approval gate before any edit, commits only on request, a "When not to use" section, and an explicit trust boundary.

Findings

F1 [Minor] [confidence: high] - README.md:36-42 (Install section)

  • Issue: the rename has no migration note for people who already installed the skill.
  • Impact:
    • Anyone who installed with npx skills add ... -s cr-request cr-receive keeps an orphaned cr-receive next to cr-received.
    • Claude Code and Codex plugin users lose /awf:cr-receive on update without warning.
  • Evidence: the README never mentions the old name. The rename is recorded only in the two skill CHANGELOGs.
  • Direction: add one line to the README, for example: "0.0.3 renames cr-receive to cr-received; remove the old skill or reinstall."

F2 [Minor] [confidence: low] - plugins/awf/skills/session-learnings/SKILL.md:34-35

  • Issue: "keep content from a repository whose instructions restrict it inside that repository" sits in the Filter step. There, "Keep a candidate only when..." (line 27) uses "keep" to mean "retain as a learning", so the sentence can be misread.
  • Impact: an agent could read it as "keep restricted content as a candidate" rather than "confine it to that repository". It might then copy restricted content into a global or other-repository file.
  • Evidence: "keep" has two meanings in the same section. The trailing "describe such a case generically" reduces, but does not remove, the ambiguity.
  • Direction: reword, for example: "Do not carry content out of a repository whose instructions restrict it; record it only there, or describe it generically elsewhere."

Suggestions (optional)

  • After merge, tag v0.0.3 and bump the awf ref in sentimony/skills .claude-plugin/marketplace.json, as AGENTS.md requires.
  • Later, consider moving CR_REQUEST_REF to the v0.0.3 tag commit, so the copy of the skill that the auto review installs names the current handoff skill. This has no effect on the job today.
  • In section 3 of session-learnings, consider naming each target file outside the current repository explicitly in the approval list, since global and cross-repository edits reach further.

Set aside

  • CHANGELOG lines that still say cr-receive: they are intentional history.
  • No argument-hint in session-learnings: the skill takes no arguments, and CI treats the key as optional.
  • cr-received restarting at 0.0.1: this is stated as intentional in the PR description and in its CHANGELOG.
  • Earlier cosmetic suggestion to add # v0.0.2 after the pin: the comment above CR_REQUEST_REF already names the tag.
  • "whatever the conversation language" in the session-learnings description: this describes when the skill triggers, not repository text, so the English-only rule does not apply.
  • Branch naming (YYMMDD-dev-session): this cannot be checked from the detached checkout.

Open questions

  • Does sentimony/skills, or any other consumer, refer to the cr-receive name or path in a way that a ref bump alone will not fix?

Review gaps

  • Tests were not run in this job. CI (skill frontmatter, manifests and marketplaces, README versions, hidden Unicode) is reported green on HEAD SHA and was not re-run here.
  • The tag check was local only. The remote was not queried.
  • The auto review workflow was not run end to end with the new pin, and the workflow_run change takes effect only after merge.
  • sentimony/skills was not inspected.
  • The unchanged cr-received/references/records.md was searched for the old name but not re-read.

Merge readiness

Ready with fixes. There are two minor, non-blocking items: a migration note in the README for the rename, and one ambiguous sentence in session-learnings.

Reviewed with: cr-request skill

@ihororlovskyi
ihororlovskyi merged commit 132a003 into main Oct 18, 2026
1 check passed
@ihororlovskyi
ihororlovskyi deleted the 261018-dev-session branch October 18, 2026 16:27
@yngwald

yngwald commented Oct 18, 2026

Copy link
Copy Markdown
Collaborator

Code review - PR #3: awf 0.0.3: cr-received, session-learnings, auto review on the v0.0.2 cr-request (fresh review pass)

This is a fresh review pass. The same reviewer last reviewed the older head 6898cd7e2e9fe64531479ee889bdc96df4eb50ae and rated it "ready with fixes" because of two Minor items: F1, no README migration note for the rename, and F2, an ambiguous "keep" sentence in session-learnings. This pass reviews the whole range again.

Target: sentimony/agentic-workflow PR #3.

  • Range: from merge base 006400f88d768452a9aa07c6736bbbb7bb5d55c3 to head 149837d4d97d1a40f6666846ca872a82f2518750.
  • Commits: 4 (cfa822e, 706f781, 6898cd7, 149837d).
  • Files: 15, including 4 renames (+173 -23).
  • Uncommitted state: none.
  • Source: the prepared files in .auto-review/target/.

Review mode: independent reviewer. The requester then checked the main claims: the tag commit, the search for the old name, and the cited lines.

Intent sources: the PR title and description, and AGENTS.md. There is no separate spec.

Risk: low to medium. The change covers skill docs, manifests, the README, and one CI workflow pin.

Verdicts

  • Intent: PASS. Every item in the description is implemented.
    • Rename: cr-receive is now cr-received in the directory, the frontmatter name, and the title.
    • cr-request 0.0.3: it hands off to cr-received in both the body and the "Next step" line.
    • session-learnings 0.0.1: added with SKILL.md, CHANGELOG, and an MIT LICENSE for Ihor Orlovskyi, and listed in the Claude Code manifest.
    • Version 0.0.3: both manifests, the marketplace description, and the README are at 0.0.3.
    • Pin: CR_REQUEST_REF points at the v0.0.2 tag commit.
    • AGENTS.md: the new rule is added.
    • Earlier F1 is resolved: README.md:24 now has the migration note.
    • Earlier F2 is resolved: plugins/awf/skills/session-learnings/SKILL.md:34-35 now says "Never carry content out of a repository whose instructions restrict it: record it only there, or describe it generically elsewhere".
  • System: PASS.
    • Pin: git rev-parse v0.0.2^{commit} returns 006400f88d768452a9aa07c6736bbbb7bb5d55c3, which matches CR_REQUEST_REF at .github/workflows/claude-review.yml:117.
    • Old name: whole-word matches for cr-receive appear only in deliberate history and notes: README.md:24, plugins/awf/skills/cr-received/CHANGELOG.md:5, and plugins/awf/skills/cr-request/CHANGELOG.md:5,9.
    • Skill list: the Claude Code manifest matches the skill directories. Codex reads ./skills/.
    • Versions: each top CHANGELOG entry matches its metadata.version.
    • README: it shows the repository, plugin, and skill versions.
  • Craft: FINDINGS. One Minor formatting item, below. session-learnings is otherwise well built:
    • It edits nothing before approval and applies only the approved items.
    • It flags every target outside the current repository before asking.
    • It commits only on request and stages only its own lines.
    • It treats the transcript and tool output as untrusted evidence.
    • It asks the user to settle a conflicting rule instead of adding a second one.

Findings

F1 [Minor] [confidence: high] - plugins/awf/skills/session-learnings/SKILL.md:75

  • Issue: line 75 stops early at "edit reaches further. Edit nothing", and the sentence continues on line 76. This looks like a leftover from an edit.
  • Impact: cosmetic only, and the rendered Markdown is unaffected. The source breaks the file's own roughly 90-column wrap, though the skill tells agents to match each file's line width.
  • Evidence: lines 73-77 of the HEAD file. Line 75 is about 35 characters long, while the lines around it are about 85-90.
  • Direction: rewrap lines 73-77.

Suggestions (optional)

  • plugins/awf/skills/session-learnings/SKILL.md:35 is 91 characters, one over the width used elsewhere. This is trivial.
  • After merge, tag v0.0.3 and bump the awf ref in sentimony/skills .claude-plugin/marketplace.json, as AGENTS.md requires.

Set aside

  • CHANGELOG lines and the README note that still say cr-receive: these are intentional history and migration text.
  • cr-received restarting at 0.0.1: the PR states this is intentional, and the CHANGELOG line and README note explain it.
  • The pinned v0.0.2 cr-request still naming cr-receive in its handoff text: the review-only job never uses the handoff.
  • Moving CR_REQUEST_REF to v0.0.3: that tag does not exist yet. The pin has to be a released tag.
  • The overlap between "project instructions" (trusted) and "repository files" (untrusted) in the session-learnings Security Model: this follows the usual convention and does not weaken the approval gate.
  • Branch naming (YYMMDD-dev-session): this cannot be checked from the detached checkout.

Open questions

  • Should the README note tell npx skills users exactly how to remove an installed cr-receive directory? Reinstalling with npx skills add adds cr-received but probably leaves the old directory in place. "remove the old skill or reinstall" may not cover that case.

Review gaps

  • Tests were not run in this job. CI (skill frontmatter, manifests and marketplaces, README versions, hidden Unicode) is reported green on HEAD SHA and was not re-run here.
  • The tag check was local only. The remote was not queried.
  • The auto review workflow was not run end to end with the new pin. Its changes take effect only after merge.
  • sentimony/skills was not inspected.
  • These parts were not re-read in full:
    • the unchanged body of cr-received/SKILL.md (99% similar);
    • cr-received/references/records.md (100% similar);
    • most of ci.yml.

Merge readiness

Ready. Both earlier findings are resolved. The only remaining item is a Minor line wrap that does not block.

Reviewed with: cr-request skill

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.

2 participants