Skip to content
TYPO3Dev Companion

TYPO3 Core Patch Development

Skill: typo3-core-patch-development

Write a TYPO3 core patch and carry it to review: reproduce the issue on the branch you are fixing, make the change, cover it, write the changelog entry, run the project's checks, and push to Gerrit. Also amending after review and backporting to a release branch.

Markdown source#

markdown
---
name: typo3-core-patch-development
description: 'Write a TYPO3 core patch and carry it to review: reproduce the issue on the branch you are fixing, make the change, cover it, write the changelog entry, run the project''s checks, and push to Gerrit. Also amending after review and backporting to a release branch.'
compatibility: Needs the typo3-dev-companion MCP server, which owns every lookup this workflow routes to and publishes this skill together with the references/base.md it opens on. Install it from github.com/TYPO3/dev-companion and run typo3-dev-companion install in the project. A copy taken out of that repository's skills directory alone has neither the tools nor that base file.
---

# TYPO3 Core Patch Development

Carry one change from an issue to a patch somebody can review. Keep this skill
as routing and working order; the suites, the scripts, the contribution rules
and the Gerrit commands are lookups, and a copy of them here goes stale in
somebody else's checkout with nothing to report it.

## Establish the issue before you believe it

1. Work through [references/base.md](references/base.md), which fixes the order
   every task here starts in.
2. `typo3_rule_lookup` for what this kind of change owes — a bugfix, a feature,
   a deprecation and a removal are four different sets of obligations, and which
   one you are in decides the changelog entry, the commit subject and the target
   branch.
3. `typo3_forge_lookup` with the issue number, and read what comes back as a
   report rather than as a specification. An issue can be stale, half fixed, or
   right about the symptom and wrong about the cause, and the maintainers'
   comments on it can be product judgement rather than an API fact. Three parts
   of that answer are not in the description a session otherwise starts from:
   the **status and target version as they stand today**, which is where a
   closure or a reassignment shows without the report ever being rewritten; the
   **relations**, which are one hop from the change that introduced the
   behaviour being complained about, and reach it when a query on the wording
   does not; and the **notes**, where a maintainer said why. Establish which of
   those you have before writing code: what the reporter saw, what the branch
   does today, and what the project intends the API to be for. Two of that
   reading are acts, and what they produce goes into the assessment before any
   code:
   - **Read the closure reason and the target version for what the conversation
     decided, and write that down rather than what the report is worth.** Closed
     for lack of feedback after a long silence is as consistent with an answer
     the reporter could not use as with the reporter giving up. A target version
     says which branch a fix was still expected on. Say what the closure settles
     and what it leaves open; a closed issue is not a finding that the need is
     absent.
   - **Where a comment names an alternative, write out what the alternative
     drops against what the reported code did.** Name the arguments and the
     behaviour the reported code had and the replacement does not. An
     alternative closes an issue only if it does the same work, and what it
     drops is usually the capability the reporter was reaching for.

4. `typo3_gerrit_lookup` with the same issue number, **before any code is
   written**. Its cheapest outcome is the one that cancels the work and it costs
   one call. An answer of nothing is a result, and a narrow one: the review
   server is read without a credential, so it says that nothing public names the
   issue rather than that nobody has fixed it, and a change pushed unlisted is
   invisible to it.

5. **Verify in the checkout every rule the issue quotes.** A rule about what an
   API may or may not be used for is a claim, the way a path or an identifier
   is. Read the class it names, its docblock and the core's own tests for the
   form under dispute, and say which of the three carries the rule. Enforced in
   code, warned about as fragile and advised in prose are three different
   claims, and two neighbouring APIs regularly make different ones. Carry it at
   the strength its own source puts on it. An assessment that hardens "may
   change in a future version" into "must not" argues the patch away on a rule
   nothing holds.

6. **Reproduce against the branch you are fixing**, not against the version in
   the report. Half of what a stale issue describes is usually gone, and the
   half that remains is the patch.

Whether that reproduction can be a test is a property of what you are changing,
and `typo3_test_run_guide` with the paths you are about to touch is what says
so: it names the suites that can fail on them, and a change to backend markup, a
build step or shipped JavaScript may have none that can hold the bug.

Where there is a layer, write the test first and prove it fails before the fix
and passes after it, in that order. A test written afterwards asserts what the
code now does, which is true of any code. Where there is none, reproduce by hand
and write down the steps and what you saw, because that is what a reviewer
repeats — an unreproducible claim is what sends a patch back regardless of
whether it is right.

A reason for not doing something is dated and the API it rested on is not. Where
the issue carries a decision to defer — it needs an event that does not exist,
there is no API for this yet — check that blocker against what the branch has
today before treating it as standing. An expired objection is written in the
same words as one that still holds, and nothing in the notes separates them.

The argument that carries a bugfix is the same inconsistency inside one version.
"The same input is handled one way here and another way there, on the branch as
it stands" is a defect a reviewer can act on; "this would be better if it also
did that" is a wish, and agreeing with it does not make it a bug. Finding the
place where the system already does the right thing is what turns the second
into the first, and failing to find one is an answer as well — it says the
change is a feature, and step 2 has already priced what that owes.

Establish the blast radius here rather than meeting it while working. How much
of the behaviour the suites already pin down has to change with it is what
decides whether this is a quiet bugfix, a change that has to announce itself, or
a breaking one, and that decision sits upstream of the target branch, the commit
subject and the entry. Discovered incrementally it arrives after the change has
been characterised, and then it is the characterisation that has to be taken
back.

Its other half is who may already be extending what you are about to edit, and
no suite in the checkout answers it: a green run says no core class overrides
the method, never that no extension does. Where the shape you have in mind
touches the declaration of a public or protected member — a parameter, a type, a
visibility, a `final` — settle what that commits the patch to before writing it,
with `typo3_hint_lookup` for the id `public-api-surface`. It decides the target
branch, so a fix owed to a maintained release line has to know it first.

## Where the finding is a vulnerability

**Ask it once the reproduction stands and before any code is written, rather
than when a defect happens to look alarming.** This workflow ends in a push, and
a push is publication: it puts the diff, the test that proves the defect and a
message explaining both in front of everyone who can read the project. Nothing
else in this order asks the question, so the fix would disclose the defect it
fixes.

The stopping point is the verified reproduction. It stands, and nothing after it
here is done: no patch, no test pushed, no entry written, no comment on the
issue.

Where it goes instead is `typo3_rule_lookup` with
`documentId="any/security/reporting-a-vulnerability"`, which is the whole
procedure and also stands as
`typo3://guides/any/security/reporting-a-vulnerability`. The address is read
there and never from here: a contact route is the fact that moves, and this file
is a copy no release of this server corrects.

Ask it again wherever the work turns into one, because the issue rarely says so:
a fix whose real effect is that a restriction now holds is this case under
another name. And where something is already up for review, amending it takes
nothing back — every patch set stays fetchable — so what has been published is
named in the report rather than repaired.

## Make the change

Ask `typo3_hint_lookup` with the concrete paths for the conventions of each
subsystem you touch, before writing rather than after. A convention fetched
afterwards confirms what you already wrote.

Keep the patch one change. What else you noticed is another issue and another
patch; a diff that fixes two things is a diff a reviewer has to accept or reject
as one.

Find out whether the area is moving before you build on it, and fetch and rebase
onto the branch you target before you finalise. A patch written against code
that changed underneath it is not a patch that needs adjusting — the method it
called can be gone, and with it the reason the change looked right.

## Verify with the project's own commands

`typo3_test_run_guide` with the changed paths returns the suites that can fail
on this change, each with its targeted invocation, and `typo3_script_lookup`
returns the scripts around them. Run the narrow ones while iterating and the
broad ones before pushing.

Two things decide whether that verification means anything:

- The runner is the project's. A suite run through the host's own PHP or an
  installed binary rather than through the project's runner is a green nobody
  can reproduce.
- A green that ran over no files is not a green. Where a check reports success,
  confirm it inspected something — the count of files, tests or fixtures it
  names — before treating it as evidence. A check that silently found nothing to
  check is the failure mode that survives review.

## The changelog entry the change owes

`typo3_rule_lookup` says which changes need one, what it is named, and where it
lives. Decide it from the change type rather than from habit: writing an entry
for a change that owes none is as much a review finding as omitting one that
does.

## Commit and push

`typo3_commit_message_guide` with `workflow="core"`, the drafted message and the
change type reports what is still wrong with it before the hook does. State the
workflow: its default is a repository of your own, which demands neither the
Forge issue nor the `Releases:` trailer a patch here owes. What the rules behind
it say — the subject, the trailers, the release targets and the changelog entry
the change type owes — is one page: `typo3_rule_lookup` with
`documentId="core/contribution/commit-messages"`, which also stands as
`typo3://guides/core/contribution/commit-messages`. Reading it once here is
cheaper than learning it from checks one call at a time. Then the Gerrit
workflow: what the push actually is, how a change is amended into a new patch
set rather than a second commit, and what must not be edited between patch sets.
That procedure exists whole as `typo3_rule_lookup` with
`documentId="core/contribution/gerrit-workflow"`, which also stands as
`typo3://guides/core/contribution/gerrit-workflow`. Read it before the first
push rather than a section at a time: a search returns the part your words
matched, and everything below here is a different part of the same page.

Before pushing, establish where you are pushing to. A core checkout's remote is
not necessarily the one it fetches from, and the answer is in the checkout's own
git configuration rather than in what the repository is called.

### Rebase where the branch moved under you

A commit that sat while you verified it is behind `origin/main`, and rebasing it
is part of pushing rather than a thing of its own. Two parts of that are not
obvious and both were worked out from scratch by a session that had no skill
telling it either:

- **Stop a running `runTests.sh` suite first.** The script mounts the working
  tree and reads it as it goes, so rebasing underneath a run invalidates it
  without failing it — and the run then reports about a tree that no longer
  exists. Clear the suite's leftover containers before starting.
- **Confirm the `Change-Id` survived the rebase.** It is what makes the push a
  new patch set on the change you already have. Losing it opens a second change
  instead, which is not undone by pushing again.

Then re-run the checks on the new base. Inspect the commits you rebased over
where any of them touch the same files: the suite passing before the rebase is
evidence about the old base.

Fetching somebody else's patch out of review and putting it on a branch is a
different job and belongs to `typo3-core-patch-checkout`.

Pushing is a step of its own and is taken when it is asked for. Everything above
is local and reversible; the push is neither.

**Ask whether the change goes up visible to everyone or unlisted, every time.**
The two are different refspecs and the difference is not a preference: one
publishes the change to whoever watches the project and notifies reviewers, and
neither the publication nor the notification is quietly undone.
`typo3_rule_lookup` for the Gerrit workflow has both forms. Which one this
change wants is the user's decision and never a default read off what the
session did last.

## Where the patch is finished and the review begins

**When the checks pass and the commit is written, invoke
`typo3-core-patch-review` on the diff, before the patch is pushed or handed
over.** That is a step, not a note about ownership: load the skill by name and
work from it. What crosses over is the diff, the branch it targets, the change
type and what the checks reported — and what comes back is the work list the
paragraph below already says to take it as.

The reason it is written as a step is that the ownership sentence did not fire
as one. A session finished a push-ready patch here — three files, two functional
tests, a commit message — ran the project's checks and reported it without ever
opening the review, twenty turns after invoking this skill out of a triage whose
crossing is written as an act. The act fired and the boundary did not, in one
session on one task.

## Amending after review

A patch that came back is the same change, not a new one. Fetch the patch set
that exists, amend it, and keep the identifier that links it to its review —
`typo3_rule_lookup` for the Gerrit workflow says how each of those is done.
Reviewer comments are addressed in the patch or answered in the review, never
silently dropped: a comment nobody replied to is the reason a change sits
unmerged.

This skill owns writing a core patch and delivering it — the change itself, its
tests, its changelog entry, the checks it has to pass, its commit message and
its push. It does not own judging somebody else's patch, and it does not own
judging its own: where the request is to say what is wrong with a change rather
than to make one, `typo3-core-patch-review` owns that and reads different
surfaces for it. Take its findings as a work list when it hands them over.
Changing an extension, a sitepackage or a site project belongs to the extension
skills, whose conventions are not the core's.

References#

Where every task starts#

markdown
# Where every task starts

## Nothing starts until the server answers

A skill is a file the installer left behind: it loads and reads the same whether
the tools behind it are connected or not, and neither side notices. So the first
call below is also the check.

- No `typo3_` tool in this session, or a first call that errors: stop, say this
  workflow needs the server and it is not there, and name what came back.
- Do not fall back to general TYPO3 knowledge or start reading the checkout.
  That answer carries this workflow's order and confidence and none of its
  evidence, and nothing in it says which of the two it is.
- Continue only when asked to after saying so, and repeat it in the answer and
  in every finding a lookup would have carried.

## The order

This is the order, and it is an order rather than a list because each step
decides what the next one is worth. A convention fetched after the code has been
read confirms a view instead of testing it. A command recommended before the
project's own are known is a guess that sounds like advice. Where a step below
carries a condition for skipping it, that condition is narrow on purpose: a
prescription that gets skipped teaches the next reader to skip the ones that
matter too.

1. **`typo3_project_describe`** — the installation, its TYPO3 and PHP version,
   the extensions that are the project's own, its sites, and the commands this
   repository actually declares. Every later answer is filtered by that version,
   and a check the repository does not declare is a wrong answer however
   sensible it sounds. It ends with the whole procedures this server carries, as
   ids: that list is the only place they are named to a client that renders no
   resource list, and each one is a `typo3_rule_lookup` with that `documentId`
   rather than a search.
2. **`typo3_extension_describe`** for each extension in scope — what it
   registers, and what it ships beside that: its manual, its README, its test
   layers, its XLF files with the source language each one declares. What it
   does *not* ship is answered too, and that is the half no file listing can
   give you.

   Where step 1 reported none — a core checkout is the case, since it names the
   project's own extensions and not TYPO3's — that answer is this step, and
   there is nothing to call. Say so. A step passed over in silence cannot be
   told from one that was dropped.
3. **`typo3_task_guide`** with a short English task, the paths it touches, the
   target version and the change type — the workflow this task belongs to and
   the checks that come with it.

   Run it in every session, this skill's own tasks included. The brief is built
   from the paths as well as the task text, and no skill knows which paths the
   caller is holding: a skill that covers the task is not that brief, and
   skipping the step costs the hints and the core checks those paths match.
   Where the guide's own answer is what named this skill, this is one call for
   an answer already in the session. That is the price of a step there is
   nothing to decide about.
4. **`typo3_hint_lookup`** for each subsystem in scope, with its concrete paths.
   One query per subsystem; a single broad query is not subsystem evidence.

   Where step 3 ran with those paths, its answer says whether this step is still
   owed. A brief that carried everything the lookup matched says so — "these are
   everything typo3_hint_lookup matches for these paths" — and there the call is
   made and asking again returns the same hints. One that stopped short says
   that instead and names the ids it left, and those are what is owed: fetch
   them by id rather than repeating the query. Read the sentence rather than the
   populated `hints` key, which is present either way and does not tell the two
   apart.
5. **`typo3_changelog_lookup` with `type: deprecation`**, at each major the
   package declares, bounded by `tag` and with the query omitted. Those three
   are the changelog's own axes, and the extension's vocabulary is not among
   them: an entry carries a query only when its title carries every word of it
   at once, and the core titled those entries about its own code. Words taken
   from what step 2 reported are therefore matched against titles they were
   never written in, and the sweep comes back empty however right the query
   looks.

   Step 2 picks the tags instead. `ext:core`, `ext:frontend`, `ext:form` and the
   rest name the system extension a change is **in** — one call for each one the
   package requires, renders through or registers into, which is more than its
   manifest lists — and `TCA`, `TypoScript`, `Fluid`, `YAML`, `Backend`,
   `Frontend` name the surface, one for each kind of file it ships. An extension
   key of your own is not among them and matches nothing. Every call also
   returns every tag that version and type carry, so the second call onwards is
   read off the first rather than guessed at.

   Step 2 is what the answers are checked against, which is the other half the
   words were doing. Verify each identifier that comes back in the checkout — a
   deprecation nothing here calls is not a finding — and carry the
   `FullyScanned` / `PartiallyScanned` tag into the answer, because it says
   whether the Extension Scanner can find the remaining call sites or whether
   that reading is yours. Bounded this way the sweep is still writable before a
   file is opened, which is why it is a step of the order: one left to the
   reading reaches only what a finding stumbled into, and the deprecation that
   decides whether the package survives the next major is not usually the one a
   finding walks past.

   A changelog records change events, so a pattern nothing has touched for ten
   majors has no entry at all. An empty sweep is therefore not an answer about
   what still works. "Does this still work in version N" goes to
   `typo3_documentation_lookup` at that version — here, and whenever the reading
   raises it again.

   That is a question for a documented surface — a ViewHelper, a TCA type, a
   TypoScript setting. The manual matches page titles and section paths, never
   the text of a page, so a PHP identifier has no page to be titled after. It
   reaches whatever its own words happen to spell instead. An identifier goes to
   `typo3_changelog_lookup` under its own name, which reaches the entries
   writing it however the change was titled, and then to the class below. Where
   the manual has no page for a surface either, that is a result and not an
   answer. Undocumented is not unsupported.

   A task that produces no change does not reach this step at all. The property
   is what the task produces, and a triage, a reproduction and a review are
   illustrations of it rather than the list it is read off. The sweep asks what
   a package will have to stop calling, and a task that writes nothing is not
   going to call anything. Followed literally in a triage of one issue it costs
   one call per declared major per tag and none of them bears on the report.

   The exemption ends where the workflow produces a change. A review asked to
   make the change is that other workflow, and it starts this order again
   holding the files it is about to write.

   Skip the sweep only where the change touches no TYPO3 API — a code style
   fixer, a CI file, an `.editorconfig`. A deprecation is a statement about API
   the package calls, so a change that calls none has nothing for the sweep to
   land on and it is empty before it is run. That condition is worth stating
   because this is the most expensive step of the order: one call per declared
   major per tag. Which side a change falls on is read off the files it touches
   and never off the task it started as — one PHP file edited along the way puts
   it back among the ordinary ones, and a skip there costs the deprecation no
   finding would have walked into.

   A report names the step it did not reach, under either exemption. A step
   passed over in silence cannot be told from one that was dropped, which is
   what step 2 already asks of itself.

**Then** read the checkout. Not before: listing the files first makes everything
after the listing look optional, and the conventions arrive as a footnote to a
verdict that has already formed.

## When the lookups run out

A behaviour question that survives the lookups above is read out of the
installed source rather than guessed at. What answers it is the class that
implements the behaviour and the one it inherits from. That reading is the step
after the lookups, and what it replaces is changing the code until it works.
What it settles is what this installation does and never what TYPO3 supports. So
a finding says the question could not be settled beyond the version installed,
and an answer built on the reading names the version it holds for.

## Two kinds of lookup, and neither stands in for the other

`typo3_backend_module_lookup`, `typo3_icon_lookup`, `typo3_label_lookup`,
`typo3_fluid_namespace_list` and `typo3_configuration_lookup` report what is
registered, what a path resolves to, what a value really is at runtime. They
establish the facts of this installation and they are never a verdict on it.
`typo3_hint_lookup` and `typo3_documentation_lookup` say whether those facts are
right. A subsystem confirmed by its own runtime lookup can still break every
rule that governs it, so it is not established until both were asked.

## A rule is read in both directions

It says what new code should do, and it says what this checkout is already doing
wrong. A file that has settled into the opposite of a rule is a finding, not a
local style to preserve: consistency with a project's own habit establishes
nothing about whether the habit is right.

## What the code is for is evidence, and the repository states it

A mechanism that costs something is not a defect for costing it. Before
reporting one, find what it is there for — the manual, the README, the
changelog, the setting it is driven by, the versions the package declares it
supports — and say so. Where a purpose is documented, what you have is a
trade-off to name with its cost and its alternative, not a defect; where you
cannot find one, the finding says that it could not be established rather than
that none exists. This is the other direction of the rule above, and skipping it
turns a review into a list of everything the author did on purpose.

## What a finding rests on is part of the finding

Three things carry one: a file that was read, at its path and its line; a
command that was run, with what it printed; a mechanism traced into an installed
package. Say which of the three it is. Leaving it unsaid gives a finding read
out of a CI file the weight of one with a verified line, and the reader has no
way to separate them again.

Where one of the project's own commands would settle it, run it.
`typo3_project_describe` marks each command it lists **check**, **change** or
**unknown**, read off the declared body: a check reports and hands the code back
as it was, so even a task told not to change files runs it, and the linter the
repository already declares is the cheapest evidence in it. A change is not run
under that instruction, and an unknown — a test suite, a shell pipeline, a
console command — is named in the answer as evidence that is available rather
than run unasked. What a check prints is not the finding: the configuration that
makes it fail is still what the finding is about, and the run is what takes that
finding from derived to established.

## What this server does not know

It does not read your working tree. Which files changed, which branch you are
on, and whether a path or an identifier still exists there are yours to
establish — then pass the concrete paths back, because that is what turns a
general convention into an answer about this code.

## Query it in English

The knowledge is written in English and matched lexically, so a query in another
language reaches the loanwords the two happen to share and nothing else.
Translate the subject before calling and the answer back afterwards, whatever
language you are speaking with the user.