Add agent skill for creating web framework integration packages#672
Add agent skill for creating web framework integration packages#6722chanhaeng wants to merge 18 commits intofedify-dev:mainfrom
Conversation
Replace @david/dax process spawning with node:child_process.spawn() using detached: true to create a new process group. On cleanup, use process.kill(-child.pid, 'SIGKILL') to terminate the entire process group, ensuring all descendant processes (tsx watch, dotenvx, etc.) are killed. @david/dax's CommandChild.kill() only sends a signal through its internal KillSignal abstraction, which cannot propagate to the full OS process tree. This is a known limitation (dsherret/dax#351). Closes: fedify-dev#667 Co-Authored-By: GitHub Copilot <noreply@github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds an agent skill and full scaffolding to create Fedify web-framework integration packages: a step-by-step SKILL, example app (architecture, design, code, assets), package templates, an init framework descriptor, contributor docs, and test harness updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant Framework as Framework Middleware
participant Federation
participant Store
participant Remote as Remote Server
Client->>Framework: HTTP request (ActivityPub/federation media-type)
Framework->>Federation: createContext(request) / federation.fetch()
Federation->>Store: read/write actors & posts
Federation->>Remote: outbound ActivityPub fetch/send
Remote-->>Federation: ActivityPub response
Federation-->>Framework: Web Response (or converted framework-native response)
Framework-->>Client: HTTP response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/init/src/test/server.ts (1)
9-9:⚠️ Potential issue | 🟡 MinorStale comment: timeout value is 10 seconds, not 30.
The comment states "30 seconds" but
STARTUP_TIMEOUTis set to10000milliseconds (10 seconds).📝 Proposed fix
-export const STARTUP_TIMEOUT = 10000; // 30 seconds +export const STARTUP_TIMEOUT = 10000; // 10 seconds🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/init/src/test/server.ts` at line 9, The inline comment for the STARTUP_TIMEOUT constant is incorrect: it reads "30 seconds" while STARTUP_TIMEOUT = 10000 (10 seconds). Either update the comment to the correct duration ("10 seconds" or "10000 ms") or change the constant to 30000 if the intended timeout is 30 seconds; edit the export const STARTUP_TIMEOUT declaration accordingly (refer to STARTUP_TIMEOUT) so the numeric value and its comment remain consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/skills/create-integration-package/example/DESIGN.md:
- Around line 9-10: Replace all occurrences of the malformed dash sequence "—-"
in DESIGN.md with a consistent em dash "—" (or replace with a spaced en dash " -
" if that's the project's style); specifically search the documented ranges
(around lines 9-10, 23-24, 250-253, 275-283) and the whole file for "—-" and
perform a simple replace so prose uses a single consistent dash glyph across the
document.
- Line 30: The DESIGN.md file mixes ATX-style headings (e.g., "Surface &
background" prefixed with "###") while markdownlint MD003 expects setext style
like in SKILL.md; update each listed heading (including "Surface & background"
and the other headings referenced at lines 38, 45, 53, 65, 72, 87, 97, 103, 110,
118, 125, 132, 139, 146, 151, 157, 167, 173, 179, 184, 215, 230, 255, 271, 285,
291) to the setext format (use "===" for top-level and "---" for second-level as
used in SKILL.md) so the document consistently uses setext headings and passes
MD003.
In @.agents/skills/create-integration-package/example/public/style.css:
- Around line 190-193: The .form-textarea CSS currently sets resize: none which
prevents any resizing and conflicts with the design spec; update the
.form-textarea rule to allow only vertical resizing by replacing resize: none
with resize: vertical so the textarea remains full-width with border-radius but
can be resized up/down per the spec.
In @.agents/skills/create-integration-package/example/src/federation.ts:
- Around line 1-17: The example references keyPairsStore, relationStore, and
postStore but never imports them, causing type-check failures; import the shared
stores used by createFederation by adding the appropriate named imports for
keyPairsStore, relationStore, and postStore (the same symbols) from the module
that exports your shared stores so that the later references to keyPairsStore,
relationStore, and postStore resolve when calling
createFederation/generateCryptoKeyPair and constructing the federation setup.
- Around line 93-103: The Undo handler currently deletes relationStore for any
Undo(Follow) from the actor regardless of which object was followed; change the
logic so you only delete when the Follow's object matches the local actor you
track: after confirming activity instanceof Follow and both activity.id and
undo.actorId are non-null, check that activity.id.href equals the local actor id
(or that relationStore.get(undo.actorId.href) === activity.id.href) before
calling relationStore.delete(undo.actorId.href); reference the Undo handler,
Follow, activity.id, undo.actorId and relationStore.get/delete when implementing
this guard.
In @.agents/skills/create-integration-package/example/src/logging.ts:
- Around line 11-15: The logger config's category property currently uses the
spaced string "default example"; change it to a hierarchical array (e.g.,
["default", "example"] or ["fedify", "examples"]) in the config object in
logging.ts so filtering works consistently; update the category value where it's
declared (the category property in the logger config) and ensure any code that
reads category (e.g., logtape setup/initialization) accepts an array form or is
adapted to join the array into a single category string if necessary.
In @.agents/skills/create-integration-package/example/src/store.ts:
- Around line 29-34: The delete method currently filters `#timeline` using
reference equality (i !== id) which fails when a new URL instance representing
the same address is passed; change the comparison to value equality by comparing
URL strings (e.g., use i.toString() or i.href) against id.toString()/id.href so
the filter removes entries with the same URL value; update the filter in the
delete method to use that string comparison and keep the existing
`#map.delete`(id.toString()) key usage consistent.
In @.agents/skills/create-integration-package/init/framework.ts:
- Around line 11-12: The object frameworkDescription uses the wrong property
key; WebFrameworkDescription expects label not name, so replace the name
property with label (keeping the same value "프레임워크" or the official framework
name) in the frameworkDescription constant so the shape matches the
WebFrameworkDescription interface and TypeScript compiles.
In @.agents/skills/create-integration-package/package/deno.jsonc:
- Around line 5-21: Add an "imports" section to the deno.jsonc template so Deno
can resolve the package and runtime aliases (e.g., map the module specifier
"@fedify/fedify" and any framework/runtime names used by this scaffold to their
canonical URLs or local entry points); update the existing tasks/check/test
blocks to remain unchanged, but ensure the new "imports" key sits alongside
"exports", "exclude", and "publish". Also mirror these dependency mappings in
package.json for Node/Bun compatibility so both runtimes have consistent
dependency names and sources.
In @.agents/skills/create-integration-package/package/package.jsonc:
- Around line 49-53: The peerDependencies block currently hardcodes a NestJS
peer ("@nestjs/common") which is inappropriate for the generic scaffold; update
the package.jsonc by removing the "@nestjs/common" entry and keep only the
Fedify peer ("@fedify/fedify") plus a generic placeholder comment for
framework-specific peers (leave the "// Add other relevant peer dependencies for
the framework" line or similar). Ensure you modify the peerDependencies object
(not other sections) so only "@fedify/fedify": "workspace:^" and a neutral
placeholder remain, and remove the NestJS-specific constraint.
- Around line 29-41: The package manifest references ESM files that aren't
produced by the build: update package.jsonc so "module" points to
"./dist/mod.js" (or add "type": "module" and change build to emit .mjs), and
make "exports.import" target the actual emitted ESM file (./dist/mod.js if you
keep current tsdown.config.ts output) so Node resolves the correct module type;
adjust either the tsdown.config.ts build output to emit .mjs and add "type":
"module" or change the fields "module" and "exports.import" to ./dist/mod.js to
match the existing build artifacts.
In @.agents/skills/create-integration-package/package/README.md:
- Around line 65-71: The import for federation is using a named import but
federation.ts exports the default; change the import to use the default import
(matching the earlier snippet) so fedifyHandler receives the default export —
update the line that references "import { federation } from
\"./federation.ts\";" to import the default "federation" and then pass that
default to fedifyHandler (symbols: federation, fedifyHandler, fedifyMiddleware).
- Around line 15-18: The README badge and package links use hard-coded
`@fedify/nuxt` values ([JSR badge], [JSR], [npm badge], [npm]); update those
four link targets to use the scaffold's package name placeholder (e.g., replace
`@fedify/nuxt` with the template variable used for the scaffold like
`{{packageName}}` or the actual scaffold name `@fedify/프레임워크`) so generated
READMEs point to the correct JSR and npm URLs; ensure the same placeholder is
used consistently for all four link references.
In @.agents/skills/create-integration-package/package/src/mod.ts:
- Around line 10-14: The parameter signature in fedifyMiddleware is invalid
because it combines an optional marker with a default value; update the
signature for fedifyMiddleware<TContextData, FrameworkContext> so that
contextDataFactory is either optional (remove the default) or has a default
value (remove the '?')—e.g., accept contextDataFactory:
ContextDataFactory<TContextData, FrameworkContext> = (() => void 0 as
TContextData) or contextDataFactory?: ContextDataFactory<TContextData,
FrameworkContext> and handle undefined inside the function; also add or import
the missing FrameworkMiddlewareHandler type (define it to match your target
framework's middleware handler shape) so the return type of fedifyMiddleware is
correctly typed.
In @.agents/skills/create-integration-package/SKILL.md:
- Line 65: The markdown mixes ATX (e.g., "### Request flow") and setext heading
styles causing MD003 violations; update all listed subheadings (start with the
"Request flow" heading and the other noted headings at lines 73, 80, 94, 105,
107, 116, 120, 124, 129, 134, 139, 170, 177, 189, 206, 285) to a consistent
setext style (use underlines with --- or === as appropriate for the level) so
they match the project's lint expectation, or alternatively change the lint rule
to accept ATX — but prefer normalizing the headings in SKILL.md to setext to
resolve the warnings.
- Around line 322-324: Replace the incorrect command "mise check" with the
repository-standard "mise run check" in the SKILL.md step that currently reads
"mise run fmt && mise check"; update the line so it reads "mise run fmt && mise
run check" (search for the exact string "mise run fmt && mise check" to locate
the text), then run the docs linter/preview to confirm formatting remains valid.
- Around line 136-137: Update the testing section in SKILL.md to remove the
contradiction by requiring unit tests for new integration logic and also
retaining guidance to use mise test:init for ad-hoc/manual runs; specifically,
replace the sentence that says "Unit tests aren't mandatory..." with a clear
requirement to "Write unit tests for all new functionality (at minimum for
integration logic) and use mise test:init for manual/local testing," and update
the later paragraph that references tests to align with this requirement so both
sections consistently mandate unit tests where feasible while still recommending
mise test:init.
In `@AGENTS.md`:
- Around line 161-164: AGENTS.md currently only points to
*.agents/skills/create-integration-package/SKILL.md* and must remain
self-describing: add a short summary in AGENTS.md for the new skill that states
the skill name, its purpose, expected inputs and outputs (types/formats), basic
usage pattern or invocation example, and then append a "See full workflow" link
to the SKILL.md; update the section that references the skill (the existing
pointer text) to include these brief contract details so AGENTS.md continues to
be the single source of truth for agent interfaces.
---
Outside diff comments:
In `@packages/init/src/test/server.ts`:
- Line 9: The inline comment for the STARTUP_TIMEOUT constant is incorrect: it
reads "30 seconds" while STARTUP_TIMEOUT = 10000 (10 seconds). Either update the
comment to the correct duration ("10 seconds" or "10000 ms") or change the
constant to 30000 if the intended timeout is 30 seconds; edit the export const
STARTUP_TIMEOUT declaration accordingly (refer to STARTUP_TIMEOUT) so the
numeric value and its comment remain consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 342242d1-3539-43fc-b05c-9877f913a60b
⛔ Files ignored due to path filters (2)
.agents/skills/create-integration-package/example/public/demo-profile.pngis excluded by!**/*.png.agents/skills/create-integration-package/example/public/fedify-logo.svgis excluded by!**/*.svg
📒 Files selected for processing (22)
.agents/skills/create-integration-package/SKILL.md.agents/skills/create-integration-package/example/ARCHITECTURE.md.agents/skills/create-integration-package/example/DESIGN.md.agents/skills/create-integration-package/example/README.md.agents/skills/create-integration-package/example/deno.jsonc.agents/skills/create-integration-package/example/package.jsonc.agents/skills/create-integration-package/example/public/style.css.agents/skills/create-integration-package/example/public/theme.js.agents/skills/create-integration-package/example/src/federation.ts.agents/skills/create-integration-package/example/src/logging.ts.agents/skills/create-integration-package/example/src/main.ts.agents/skills/create-integration-package/example/src/store.ts.agents/skills/create-integration-package/init/framework.ts.agents/skills/create-integration-package/package/README.md.agents/skills/create-integration-package/package/deno.jsonc.agents/skills/create-integration-package/package/package.jsonc.agents/skills/create-integration-package/package/src/mod.ts.agents/skills/create-integration-package/package/tsdown.config.tsAGENTS.mdCONTRIBUTING.mdexamples/test-examples/mod.tspackages/init/src/test/server.ts
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive guide and a set of templates for creating web framework integration packages, located in .agents/skills/create-integration-package/. It also updates AGENTS.md and CONTRIBUTING.md to reference this new resource. Additionally, the server test runner in packages/init/src/test/server.ts was refactored to use Node.js spawn with process groups, ensuring more reliable termination of child processes. Review feedback correctly identified missing imports in the federation template and suggested refinements to the documentation's phrasing and testing guidelines.
…ge of @fedify/fixture
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
.agents/skills/create-integration-package/SKILL.md (1)
136-140:⚠️ Potential issue | 🟠 MajorMake unit tests mandatory, not optional, in the skill guidance.
Line 136 and Line 160 still frame tests as optional, which conflicts with the repository testing expectation for new functionality.
Proposed fix
-You can test the integration using `mise test:init`, which will be explained -later, but write unit tests as well if possible. Import the `test` function +You can test the integration using `mise test:init`, which will be explained +later, and you should write unit tests for all new integration functionality. +Import the `test` function ... -7. Write tests if possible +7. Write unit tests for new integration logicBased on learnings, "Write unit tests for all new functionality following the pattern of existing tests. Import
testfromfedify/fixturefor runtime-agnostic tests."Also applies to: 160-160
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/create-integration-package/SKILL.md around lines 136 - 140, Update the guidance so unit tests are mandatory rather than optional by changing the phrasing that currently reads "You can test..." and "but write unit tests as well if possible" to a required instruction like "Write unit tests for all new functionality using the runtime‑agnostic test helper." Specifically, update the sections that reference importing the `test` function from `@fedify/fixture` to instruct: "Import `test` from `@fedify/fixture` and create `*.test.ts` files (e.g., `src/mod.test.ts`) for all new features," and remove any language that frames tests as optional to ensure consistency with repository expectations..agents/skills/create-integration-package/package/README.md (1)
65-71:⚠️ Potential issue | 🟠 MajorFix the usage snippet imports to match the shown exports and template API.
The example currently mixes import styles that don’t line up:
federation.tsis shown as default-exported (Line 60), but imported as named on Line 67, and the package import on Line 66 does not match the currentsrc/mod.tsnamed export.Proposed fix
-import fedifyHandler from "@fedify/프레임워크"; -import { federation } from "./federation.ts"; +import { fedifyMiddleware } from "@fedify/프레임워크"; +import federation from "./federation.ts"; -const fedifyMiddleware = fedifyHandler(federation); +const middleware = fedifyMiddleware(federation); -app.use(fedifyMiddleware); +app.use(middleware);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/skills/create-integration-package/package/README.md around lines 65 - 71, The snippet mixes default vs named imports: ensure you import the handler and federation to match their actual exports—use the named export from the package and the default export for your local federation file. Change the package import to a named import (e.g., import { fedifyHandler } from "@fedify/프레임워크";) and import the default export from your federation module (e.g., import federation from "./federation";), then call const fedifyMiddleware = fedifyHandler(federation); and app.use(fedifyMiddleware); so the symbols fedifyHandler and federation match the template API.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/skills/create-integration-package/example/src/store.ts:
- Around line 13-18: The append(posts: Note[]) method allows duplicates when two
items in the same posts array share an id because the filter only checks
existing this.#map; change append to also track ids seen within the current
batch (e.g., create a local Set seenBatchIds) and use it alongside this.#map
when deciding to accept a post, adding the id to seenBatchIds as you accept it
so neither the map nor the timeline receive the same id twice in one append
call; update references to this.#map and this.#timeline in append accordingly.
In @.agents/skills/create-integration-package/package/src/mod.ts:
- Around line 15-17: The function fedifyMiddleware currently declares a
FrameworkMiddlewareHandler return type but has no implementation; update
fedifyMiddleware to return a valid FrameworkMiddlewareHandler (or immediately
throw) so the signature is honored—either return a middleware function that
accepts the expected parameters and throws a clear "not implemented" error, or
directly throw a descriptive Error from fedifyMiddleware; reference the
fedifyMiddleware declaration and ensure the returned value conforms to
FrameworkMiddlewareHandler.
---
Duplicate comments:
In @.agents/skills/create-integration-package/package/README.md:
- Around line 65-71: The snippet mixes default vs named imports: ensure you
import the handler and federation to match their actual exports—use the named
export from the package and the default export for your local federation file.
Change the package import to a named import (e.g., import { fedifyHandler } from
"@fedify/프레임워크";) and import the default export from your federation module
(e.g., import federation from "./federation";), then call const fedifyMiddleware
= fedifyHandler(federation); and app.use(fedifyMiddleware); so the symbols
fedifyHandler and federation match the template API.
In @.agents/skills/create-integration-package/SKILL.md:
- Around line 136-140: Update the guidance so unit tests are mandatory rather
than optional by changing the phrasing that currently reads "You can test..."
and "but write unit tests as well if possible" to a required instruction like
"Write unit tests for all new functionality using the runtime‑agnostic test
helper." Specifically, update the sections that reference importing the `test`
function from `@fedify/fixture` to instruct: "Import `test` from
`@fedify/fixture` and create `*.test.ts` files (e.g., `src/mod.test.ts`) for all
new features," and remove any language that frames tests as optional to ensure
consistency with repository expectations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d359fb8d-3adc-48e2-8b7d-96cebf01732d
📒 Files selected for processing (7)
.agents/skills/create-integration-package/SKILL.md.agents/skills/create-integration-package/example/src/store.ts.agents/skills/create-integration-package/init/framework.ts.agents/skills/create-integration-package/package/README.md.agents/skills/create-integration-package/package/deno.jsonc.agents/skills/create-integration-package/package/package.jsonc.agents/skills/create-integration-package/package/src/mod.ts
- Fix em dash + hyphen (—-) to em dash (—) in DESIGN.md - Fix logger category from space-separated string to array in logging.ts - Fix named import to default import in README.md usage example - Fix "may not finish forever" to "may never finish" in SKILL.md
DESIGN.md specifies "Resize is vertical only" but the CSS had resize: none, preventing any resizing.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/skills/create-integration-package/package/README.md:
- Around line 66-71: The README shows importing a default export named
fedifyHandler but the template's package/src/mod.ts exposes a named export
fedifyMiddleware, causing import errors; fix by either changing README to import
the named export (use { fedifyMiddleware } or import { fedifyMiddleware as
fedifyHandler } and adjust usage) or modify package/src/mod.ts to add a default
export (export default fedifyMiddleware) so fedifyHandler can be imported as
shown; update the README or add the default export in mod.ts consistently and
ensure references to fedifyHandler/fedifyMiddleware match across both files.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a92ee1a7-b6eb-439a-aec1-716e97014533
📒 Files selected for processing (5)
.agents/skills/create-integration-package/SKILL.md.agents/skills/create-integration-package/example/DESIGN.md.agents/skills/create-integration-package/example/public/style.css.agents/skills/create-integration-package/example/src/logging.ts.agents/skills/create-integration-package/package/README.md
Closes #653
Summary
Add a
create-integration-packageagent skill that guides AI coding agents (and human contributors) through the full workflow of adding a new web framework integration to Fedify.Changes
New skill:
.agents/skills/create-integration-package/SKILL.md — Step-by-step instructions covering:
Federation.fetchcompatibility)@fedify/initmise test:initpackage/ — Template files (
src/mod.ts,deno.jsonc,package.jsonc,README.md,tsdown.config.ts) that agents copy and adapt when scaffolding a new integration package.init/framework.ts — Template for the
WebFrameworkDescriptionobject added to@fedify/init.example/ — Reference example app with architecture and design docs (
ARCHITECTURE.md,DESIGN.md), source files (federation.ts,logging.ts,main.ts,store.ts), and static assets. Agents use these as the starting point for the example app.Documentation updates
Minor improvement
SERVER_EXAMPLES,SCRIPT_EXAMPLES,MULTI_HANDLE_EXAMPLES, andSKIPPED_EXAMPLES.Follow-up
This skill will be used to resolve #139 and #149 as follow-up PRs, which will also serve as proof that the skill is effective in practice.