If you want other people to extend your application without forking it, loading their code is the easy part. Read a list of module paths from config, import() each one, keep whatever looks like a plugin. An afternoon’s work. The hard part is the contract: where in your lifecycle does third-party code run, what is guaranteed to be true when it runs, what happens when it throws, and how do you keep the freedom to refactor your own internals once strangers depend on them?
I spent a while on that question for GitProxy, which had a plugin system that got the loading right and the contract wrong. Every plugin ran at position zero of the processing chain, which meant third-party code executed before the repository had been checked against the authorised list, before the pushing user had been identified, and before any of the data a plugin would actually want to inspect existed. The revamp is PR #1683, and most of what I learned doing it is not GitProxy-specific, so this is the general version with the case study attached.
A plugin system is three separate problems
It helps to name the parts, because they fail in different ways.
Discovery and loading is turning a config entry into a JavaScript object. GitProxy uses load-plugin for Node’s resolution rules, so a plugin can be an installed npm package or a file on disk.
Identification is deciding whether the object you just imported is a plugin at all, and what kind.
Placement and invocation is when the plugin runs, what it can see, and what it is allowed to do about it. This is where the design work lives, and for anything security-shaped, it is where the security lives too.
Most write-ups on “add a plugin system to your app” cover the first and stop. The interesting failures are all in the other two.
What “plugins run first” actually cost
Here is the old insertion code, in full:
for (const pluginObj of chainPluginLoader.pushPlugins) {
branchPushChain.splice(0, 0, pluginObj.exec);
tagPushChain.splice(0, 0, pluginObj.exec);
}
for (const pluginObj of chainPluginLoader.pullPlugins) {
pullActionChain.splice(0, 0, pluginObj.exec);
}Four problems, in ascending order of how much they matter.
splice(0, 0, ...) inside a loop reverses the configured order, so the last plugin listed in proxy.config.json runs first. Nobody documented that and nobody wanted it.
It mutates module-level arrays, permanently, on the first call. A one-shot pluginsInserted flag guarded the mutation, and the error branch for a missing loader also set that flag to true, which meant a loader arriving late could never insert its plugins for the rest of the process lifetime. The proxy would then serve pushes with zero plugins loaded and log one line about it.
Position zero is before the interesting data exists. By the time a push plugin ran, the action had refs and commit metadata, but no resolved user, no bare clone, no packfile and no diff. A plugin that wanted to look at file contents simply could not, which is a strange limitation for a tool whose whole job is inspecting what people push.
And position zero is before authorisation. checkRepoInAuthorisedList and resolveUserFromToken both sat after the plugins, so third-party code ran for pushes to repositories GitProxy had explicitly not authorised, without knowing who was pushing. That is a considerably wider blast radius than “plugins run first” makes it sound.
The generalizable point: “run first” is not a position, it is the absence of a decision. Somewhere in your lifecycle there is a first moment at which the data a plugin needs exists and your own invariants hold. Find it and name it.
Positions make a bad public API
The obvious fix is to let plugin authors say where they go. There are three ways to let them, and two are traps.
A numeric index dies immediately on a detail specific to GitProxy but general in shape: the same plugin function gets inserted into two different chains. The branch push chain has fifteen steps, the tag push chain has eight. An author writing index: 11 is thinking “after getDiff”, but in the tag chain index 11 is past the end of the array, and getDiff does not exist there at all. One number cannot mean two things.
An anchor by name, something like runImmediatelyAfter: 'pullRemote', is at least semantic. The cost is not obvious at first: it promotes your internal function names into public API. The moment a plugin in some bank’s private fork anchors itself to pullRemote, you can no longer rename or split that function without breaking code you cannot see. You also need a policy for when the anchor is missing from a chain, and two plugins anchored to the same step puts you straight back into undefined ordering.
Named phases are the third option, and they are what essentially every mature plugin system converged on. You define a small set of names, document what each one guarantees, and map them to positions internally. Rollup and Vite give plugin authors named build hooks (resolveId, load, transform, generateBundle) rather than a slot in a pipeline. Fastify exposes onRequest, preHandler, preSerialization and friends. ESLint rules subscribe to AST node types instead of a place in the traversal. VS Code has activation events. Backstage, which is probably the most heavily extended app most of us have worked with, is built around plugins attaching to declared extension points rather than to positions in a startup sequence. Docusaurus does the same with lifecycle methods.
The reason the convergence happened is that a name plus a documented guarantee is a contract you can keep while still refactoring underneath it. An offset is a contract you break the next time you insert a line.
Phases are positions between steps, not labels on steps
This is the implementation detail that makes the rest pleasant. The chain definition becomes an array of a union type: either a processor function or a phase marker.
export type ChainElement = ProcessorExec | PushPhase | PullPhase;
const branchPushChainElements: ChainElement[] = [
proc.push.resolveUserFromToken,
proc.push.checkEmptyBranch,
proc.push.checkRepoInAuthorisedList,
PushPhase.AFTER_PERMISSIONS,
proc.push.checkMessages,
proc.push.checkAuthorEmails,
proc.push.checkUserPushPermission,
proc.push.pullRemote,
proc.push.writePack,
PushPhase.AFTER_CHECKOUT,
proc.push.checkHiddenCommits,
proc.push.checkIfWaitingAuth,
proc.push.preReceive,
proc.push.getDiff,
PushPhase.AFTER_DIFF,
proc.push.gitleaks,
proc.push.scanDiff,
PushPhase.BEFORE_APPROVAL,
proc.push.blockForAuth,
];The alternative, tagging each processor with the phase it belongs to, sounds equivalent and is not. Several consecutive processors would share a tag, so you would still need array order to break ties, plus a rule about whether plugins land before or after the tagged step. Worse, metadata attached to a function cannot vary by chain, and it needs to: checkMessages sits in a completely different neighbourhood in the tag chain than in the branch chain. Position can vary by chain, because there are two arrays.
Telling the two kinds of element apart needs no brand or wrapper, since phase values are strings and processors are functions:
typeof element === 'function'And because the markers are consumed while building, the executor still iterates a plain array of processors. Nothing downstream knows phases exist.
Name the phase after what is true, not after what has run
My favourite bug from this exercise was a naming one. An early draft had AFTER_PERMISSIONS sitting above checkUserPushPermission, so the name promised that push permissions had been verified at a point where they had not. A plugin author would read that name, believe it, and be wrong.
There are only two fixes: move the marker below the step that makes the name true, or rename the phase to describe what genuinely holds there. In the PR the documented guarantee is written narrowly to match the position, promising the authorised-list check and the pusher’s identity rather than full permission resolution, but I would still rather the name and the guarantee agree without needing the paragraph. It is worth being fussy about this before the strings ship, because a phase name is a promise about data that other people will build on, and those literals become permanent the moment they are published.
Which leads to writing the guarantees down properly. Each of GitProxy’s four push phases now documents what is populated on the action when it fires: AFTER_PERMISSIONS gives you the user and commit metadata, AFTER_CHECKOUT gives you a bare clone with the pushed objects written into it, AFTER_DIFF gives you the diff, and BEFORE_APPROVAL is the last point at which you can reject before approval is requested. That list is the actual API. The plugin documentation carries it, and I wrote up the author’s side of it separately in how to write a GitProxy plugin.
A pleasant consequence: the tag push chain has no diff step, so it simply has no AFTER_DIFF phase, and a content-scanning plugin cannot ask to run there. That is a documented, explainable outcome rather than a special case, and it is the correct one, since scanning a diff that was never computed is nonsense.
The regions you did not name should be closed
Ordering was only half the problem. The other half was keeping plugins out of the pre-authorisation region for good, rather than by accident of current line numbers.
The tempting approach is a flag on the processors: runBeforePlugins: true, pinning the important ones to the top. I think that is the wrong shape, for three reasons. It is opt-in security, so someone adding a processor next year forgets the flag and the consequence is silent: third-party code now runs ahead of their check and nothing anywhere complains. It puts plugin-awareness on functions that have no business knowing plugins exist, spreading one composition decision across a dozen files. And it is not expressive enough to do its own job, because two steps both marked runBeforePlugins still need a defined order relative to each other, which sends you back to array position anyway.
Phase markers give you the inverse, which is the property worth having: everything above the first marker is pre-plugin by construction. There is no slot there to name, so a plugin cannot ask for one. With a flag, a region is plugin-accessible unless someone remembers to close it. With markers, it is closed unless someone deliberately opens it.
Then pin the invariant with a test rather than a type. A few lines that walk every chain definition and assert that checkRepoInAuthorisedList and resolveUserFromToken appear before the first phase marker will fail in CI the moment someone reorders things carelessly. A test named plugins cannot run before repo authorisation also explains its own reasoning, which a boolean named runBeforePlugins can never do.
Build the chain instead of mutating it
With markers in the definition, insertion becomes a pure function and the old bugs evaporate:
const buildChain = (elements: ChainElement[], plugins: ActionPlugin[]): ProcessorExec[] =>
elements.flatMap((element) =>
typeof element === 'function'
? [element]
: plugins.filter((plugin) => plugin.phase === element).map(toPluginExec),
);Configured order is preserved because flatMap walks forwards. The definitions are never touched, so tests that load plugins stop leaking into tests that do not. And pluginsInserted can be deleted outright, taking with it the branch that guaranteed plugins would never load after a late loader. Build all the chains once, memoise the result, and expose a reset for tests. Give that memoised object a named type rather than Record<string, T>, which will not tell you a key is missing; I learned that when thirteen tests failed because getChain returned undefined.
Your constructor guarantees nothing about foreign plugins
This is the lesson I would most want to hand to someone starting a plugin API from scratch, because it is the one I did not see coming.
A plugin author installs your package in their own repo and extends your class. That means their plugin extends their copy of your class, from whatever version they pinned, resolved through their own node_modules. This is not a corner case, it is the normal case, and it has two consequences.
The first is that identification has to be structural. instanceof compares class identity, and two copies of the same package in one process are two different classes, so instanceof PushActionPlugin returns false for a perfectly valid plugin. GitProxy brands its instances with an own property, isGitProxyPushActionPlugin, and the loader duck-types on that. It looks unfashionable next to instanceof and it is the only thing that works across the package boundary.
The second consequence took me an afternoon to see, even with the evidence on screen. I had added phase, displayName and isCollectible to the plugin constructor, my constructor logging never appeared, and the plugin never ran. Here is the debug dump of the loaded instance:
RunOnPullPlugin {
isGitProxyPlugin: true,
isGitProxyPullActionPlugin: true,
exec: [Function: logMessage]
}Three properties. No phase. That object never went through the constructor I had just edited, because it extended a different installed copy of the package. And since the builder filters with plugin.phase === element, comparing undefined against 'AFTER_AUTHORISATION' is false, so the plugin matched no marker and vanished from the chain without a word.
So the defaults have to live at the boundary where foreign objects arrive, not only in the constructor:
plugin.phase ?? DEFAULT_PHASE
plugin.chains ?? ['branch', 'tag']The constructor default is a courtesy to authors building against your current version. The resolver default is the load-bearing code. It is also worth knowing that peerDependencies will not save you here: a peer range is an instruction to the installer, not to Node’s resolver, which at runtime simply takes the first match walking up the directory tree.
One small trap in the same area, because it is easy to write and impossible to spot in review. This line looks like a reasonable default-tolerant filter:
plugins.filter((p) => p.phase === element && p.chains?.includes(chainName))Optional chaining yields undefined when chains is absent, undefined is falsy, and so every plugin that predates the chains option gets filtered out of every chain and silently never runs. Optional chaining in a predicate is an accidental deny.
Decide what failure means, and make silence impossible
A plugin API needs an answer for each way a plugin can fail, and the answers should be loud.
For a plugin that rejects the operation, GitProxy has isCollectible, borrowed from its built-in processors: when true, the failure is recorded and reported alongside every other failure at the end of the chain, so a developer sees all the problems at once. When false, the chain stops immediately. Non-critical checks want the former; anything that later steps depend on wants the latter.
For a missing loader, the old code logged an error and carried on. That is the more dangerous option, not the safer one: a deployment whose mandatory controls live in a plugin would be serving pushes with those controls absent. It now throws.
For placement mistakes, validate at load. An unrecognised phase string, which is what a plugin newer than your runtime looks like, should refuse to start and name the phases you do know. A plugin whose phase does not exist in a chain it targets deserves at least a warning that names the plugin, because a plugin that quietly does nothing is much worse for an operator than one that crashes the boot.
The rule underneath all three: for anything security-shaped, silent non-execution is the worst possible failure mode, so prefer the loud crash to the quiet pass.
Make plugin work visible
GitProxy streams progress back to the pushing developer over git’s sideband channel, and every plugin step used to render as the same anonymous running plugin. Plugins carry a displayName now, and the chain builder attaches it while wrapping:
const toPluginExec = (plugin: ActionPlugin): ProcessorExec =>
Object.assign((req: Request, action: Action) => plugin.exec(req, action), {
displayName: plugin.displayName ?? `${plugin.constructor.name}.exec`,
isCollectible: plugin.isCollectible ?? false,
});That wrapper does more than labelling. It calls plugin.exec(req, action) with the plugin as the receiver, which fixes a real latent bug: the old splice(0, 0, pluginObj.exec) ripped the method off the object and lost this, so any plugin that subclassed in order to hold state would break as soon as its exec touched that state. It also creates a fresh function object rather than stapling properties onto the author’s function, which matters when two plugins share one exec reference and want different collectibility. The constructor.name fallback is a nicety rather than a guarantee, since minifiers mangle it, so the docs push authors to set displayName explicitly. Narrowing phase on the subclass has a trap of its own: an uninitialised field declaration wipes the value super() just assigned.
Treat the string literals as the permanent part
One last thing to decide early, because it is expensive to change later. The values that cross the package boundary are plain strings at runtime, whatever your types say, so the literals are the real contract. GitProxy declares phases as a const object rather than a TypeScript enum for exactly that reason:
export const PushPhase = {
AFTER_PERMISSIONS: 'AFTER_PERMISSIONS',
AFTER_CHECKOUT: 'AFTER_CHECKOUT',
AFTER_DIFF: 'AFTER_DIFF',
BEFORE_APPROVAL: 'BEFORE_APPROVAL',
};
export type PushPhase = (typeof PushPhase)[keyof typeof PushPhase];Once published, those four strings can never be renamed without breaking plugins you cannot see, which is why the naming argument above is worth having before release rather than after. Changing the shape of a plugin API is a breaking change for consumers even when your own tests all pass, so it belongs on a major version under semver, ideally with a default that approximates the old behaviour minus whatever part of it was unsafe.
The takeaway
Loading other people’s code is a solved problem. The design work is choosing a small number of named moments in your lifecycle, writing down precisely what is true at each one, and making sure the regions you did not name are closed rather than open. Name phases after the guarantees they offer rather than the steps that precede them, default to deny by leaving unnamed regions unnamed, and resolve every default at the boundary where foreign objects arrive, because that is exactly where your own constructor stops being able to help you.