chore: sanitize git history from corporate internal references#136
Closed
ThePlenkov wants to merge 1086 commits into
Closed
chore: sanitize git history from corporate internal references#136ThePlenkov wants to merge 1086 commits into
ThePlenkov wants to merge 1086 commits into
Conversation
…re-ci
The previous drop of this plugin claimed "npm v11 has no CLI for listing
trusted publishers" and only printed a UI link. That was wrong — npm
≥ 11.5.1 ships `npm trust list|github|gitlab|circleci|revoke`, and
@pplenkov's `npm-publish` skill already has a working bootstrap script
(`scripts/prepare-ci.mjs`) that detects the CI provider, publishes empty
0.0.0 placeholders for brand-new packages, and configures trust — all
so CI can publish via OIDC without any long-lived NPM_TOKEN.
Port that logic to an nx-native, per-package `npm-prepare` target:
1. If the package is not yet on npm, publish a 0.0.0 placeholder from
a tmpdir (with `--access=public`, scope-override registry flag, no
real source). This is NOT the real release — only a registry stub
so `npm trust` has something to attach to.
2. Run `npm trust github <pkg> --file <workflow> --repo <owner/repo>
--yes`. If npm reports "already" it is treated as an idempotent
no-op.
Defaults: provider=github, workflow=publish.yml, repo auto-detected from
`git remote get-url origin` via a small helper in the plugin. Overrides
exposed both as plugin options in nx.json (`trustWorkflow`, `trustRepo`)
and as ad-hoc flags via nx `--args=...` (`--trust-workflow=…`,
`--trust-repo=…`, `--trust-provider=gitlab --trust-namespace=… --trust-project=…`).
Also:
- Replace the wrong "no CLI" comment with an actual `npm trust list <pkg>`
call in the read-only probe, so `npm-check` now reports the current
trusted-publisher config directly (not just a UI link).
- Mode enum gains `'prepare'`; fixes applied are recorded in the
structured `__NPM_CHECK_JSON__` report as before.
Usage:
bunx nx run-many -t npm-prepare # bootstrap every pkg
bunx nx run abap-ast:npm-prepare # just one
bunx nx run adt-cli:npm-prepare --args="--trust-workflow=release.yml"
Requires npm CLI ≥ 11.5.1, Node ≥ 22.14.0, 2FA on the npm account, and
an active `npm login` in the current shell.
Credit @pplenkov's `npm-publish` skill (scripts/prepare-ci.mjs) — same
workflow, reshaped as a per-package Nx target so it plays well with
`nx run-many` / `nx affected`.
…rget
Three targets was overkill — the user just wants one entry point with
flags. Rename and flatten:
- Package: `@abapify/nx-npm-access` → `@abapify/nx-npm-trust`
(directory + project name, tsconfig reference, nx.json plugin path).
- Target: `npm-check` + `npm-fix` + `npm-prepare` → single
`npm-trust-check`. Mutations are opt-in via args:
bunx nx run-many -t npm-trust-check # read-only
bunx nx run-many -t npm-trust-check --args="--fix" # mutate
bunx nx run-many -t npm-trust-check --args="--prepare" # OIDC boot
bunx nx run adt-cli:npm-trust-check --args="--fix --prepare"
- `forwardAllArgs: true` + `nx --args="..."` forwards any flag to the
underlying script (`--fix`, `--prepare`, `--mfa=none`,
`--trust-workflow=release.yml`, `--trust-repo=owner/repo`,
`--trust-provider=gitlab --trust-namespace=… --trust-project=…`).
Behaviour of each flag is unchanged from the previous three targets —
this is purely a UX collapse, not a functionality change. Typecheck and
lint are green.
…ault
Default output per package was ~10 lines of explanation plus a full
JSON dump. Tighten to two lines of human-readable status with short
problem labels:
✗ @abapify/adt-locks@0.3.1 — state unknown
! publishConfig.access missing (run with --fix), no "files" allowlist
The structured `__NPM_CHECK_JSON__ {...}` blob is now opt-in behind a
`--json` flag (aggregation still works: `nx run-many … --json`). Private
package skip message is also collapsed to one line. No behaviour change
— only log density.
--json conventionally means "emit only JSON, suppress prose". The flag here does the opposite: it additionally emits the structured JSON line alongside the human summary. --verbose matches that semantic.
After running with --fix, adt-locks still exited 1 because the advisory
"no \"files\" allowlist" check was treated as a blocking problem, even
though (a) it is not auto-fixable, (b) missing a files allowlist does
not prevent publishing.
Split the report into `problems` (blocking, exit 1) and `warnings`
(advisory, exit 0):
- `files` allowlist → warnings
- everything else (missing name/version, publishConfig.access,
network failure, npm access failure, trust command failure) →
problems as before
Human output now uses three markers — `+` for applied fixes, `~` for
warnings, `!` for blocking problems — so the three categories are
visually distinct. Exit code is now `hasProblems ? 1 : 0`, so a
successful --fix run exits 0 even when an advisory warning remains.
Example:
✓ @abapify/adt-locks@0.3.1 [fix] — not on npm
+ set publishConfig.access=public
~ no "files" allowlist
Previous commit introduced a `warnings` bucket that kept the \`files\`
check visible but non-blocking. That was the wrong shape — the right
answer is: if \`--fix\` can fix it, fix it; if it can't, it stays a real
problem. No second category needed.
Make \`files\` auto-fixable too. Default allowlist is
\`["dist", "README.md"]\` — matches 29/30 packages in the monorepo
(tsdown emits to dist/, README lives at package root). A single
package.json write flushes all accumulated mutations at once.
After \`--fix\`, exit code reflects only problems the script could NOT
fix. No \"fixed but still exits 1\" behaviour for the publishConfig or
files cases; the release gate on 30 packages now passes cleanly.
Output in check mode:
✗ @abapify/adt-locks@0.3.1 — not on npm
! publishConfig.access missing (run with --fix), no \"files\" allowlist (run with --fix)
Output in --fix mode on the same package:
✓ @abapify/adt-locks@0.3.1 [fix] — not on npm
+ set publishConfig.access=public, set files=[\"dist\",\"README.md\"]
Two concrete bugs surfaced when `--prepare` actually ran against npmjs:
1. `npm trust github … --json` → `EUSAGE`. The `npm trust` subcommands
reject `--json`; my helper unconditionally appended it for every npm
call. Make `--json` opt-in via a second argument on `npm()` and turn
it off for the trust register call. Other call sites that rely on
JSON parsing (`view`, `access get/list`, `trust list`) keep it.
2. "`npm publish placeholder failed: npm notice`" — stderr parsing was
dumb (`stderr.split('\n')[0]`), so the first line npm prints (its
update notice banner) overrode the actual error. Replace with a
`firstErrorLine` helper that prefers lines starting with `npm error`,
falls back to the first non-chatter line (skipping notice / warn /
http / info / verb / sill), and only then to the raw first line.
Applied everywhere errors are reported.
After these fixes, `--prepare` against a logged-in npm account should
publish a 0.0.0 placeholder per new package and register the GitHub
Actions OIDC trusted publisher without hitting EUSAGE.
Verified end-to-end against a live registry.npmjs.org on an updated npm (11.12.1). Two more bugs popped out: 1. `npm trust github` is strict-argv: it rejects `--@abapify:registry=<url>` with `EUSAGE: Unknown flag: --@…`. The generic helper appended it to every call. Refactor `npm()` to accept an options object and disable both `--json` and the scope registry override for the trust call. 2. `npm trust gitlab` takes a single `--project=<namespace>/<project>` identifier, not separate `--namespace` + `--project`. The original port from prepare-ci.mjs got this wrong. Combine them at call time. Also add a version guard up front: `npm trust` requires npm >= 11.10.0, so check `npm --version` in `--prepare` mode and fail with a one-liner actionable error instead of letting older npm emit a confusing `"publish.yml" is being parsed as a normal command line argument` warn. Verified locally on adt-locks: - `--fix` patches package.json idempotently - `--prepare` now emits real errors (ENEEDAUTH for publish, E401 for trust) when unauthenticated — no more EUSAGE noise - `--fix --prepare` applies the manifest fix, then attempts the network bootstrap
The publish job was using Nx 22's @nx/js:release-publish, which honours
the workspace-level `cli.packageManager` setting from nx.json. This
monorepo sets that to `bun`, so Nx invoked `bun publish` — and bun's
publish does NOT support GitHub Actions OIDC trusted publishing. The
job failed on all 30 packages with:
bun publish error:
error: missing authentication (run `bunx npm login`)
even though OIDC was set up correctly and `npm trust github` is
registered for every package on npmjs.
Patch nx.json in-place at the start of the publish job to flip
`cli.packageManager` to `npm` for this job only. Nx release publish
then runs `npm publish`, setup-node's `registry-url: npmjs.org`
populates the right .npmrc, and OIDC auth takes over.
This is a job-local override — it doesn't touch the committed nx.json,
so bun remains the default package manager everywhere else.
Two fixes at the source after the first real npmjs publish attempt: 1. nx.json: flip cli.packageManager from "bun" to "npm". Nx 22's @nx/js:release-publish honours that and was invoking `bun publish`, which has no OIDC trusted-publishing support — it just errored with `missing authentication` for every one of the 30 packages. Confirmed via deepwiki: detectPackageManager() in nx reads cli.packageManager before any lockfile-based fallback, so this is the canonical knob. The monorepo still uses bun for `bun install`, lockfile, and local dev — nx's cli.packageManager only controls what nx itself invokes (nx add, nx release publish, etc.). 2. .github/workflows/publish.yml: upgrade npm to ^11.10.0 before `nx release publish`. Node 24 ships with npm 11.0.x; OIDC trusted publishing discovery (no NPM_TOKEN, picks up the GitHub Actions id-token automatically) only landed in npm 11.5.1 and stabilised in 11.10. Without the bump, publishes returned `Not Found - PUT /<pkg>` because npm never attempted the OIDC flow — it treated the request as anonymous and npmjs 404'd it. Drop the previous job-local nx.json patch step (covered by fix #1). The 30 trusted-publisher entries registered via `npm-trust-check --prepare` should now actually be used on the next release.
…rride
Setting cli.packageManager=npm permanently broke `nx release version`
in the Version/Changelog/Tag job:
Error: The "workspace:" protocol is not yet supported by npm
bun handles workspace:* dependencies natively, npm does not. Keep
`cli.packageManager: bun` in nx.json (as the monorepo requires), but
flip it to npm in-job right before `nx release publish` runs so
@nx/js:release-publish invokes `npm publish` with OIDC trusted
publishing. The npm@^11.10 install step stays.
setup-node with `registry-url` writes `always-auth=true` +
`_authToken=${NODE_AUTH_TOKEN}` to the user-level .npmrc, but we don't
pass a `token` input. NODE_AUTH_TOKEN is empty at runtime, so npm
sends an empty Authorization header on PUT and npmjs responds with
`Not Found`. OIDC discovery never engages because npm thinks the
registry is already (badly) authenticated.
Overwrite the userconfig with just `registry=https://registry.npmjs.org/`
so npm sees no static auth for the registry, hits the OIDC path, and
exchanges the GitHub Actions id-token for a publish token via the
trusted publishers we registered yesterday.
Diagnostics confirmed the OIDC prerequisites are all in place: - npm 11.12.1 (>=11.5.1, supports OIDC auto-discovery) - ACTIONS_ID_TOKEN_REQUEST_URL + _TOKEN both set (id-token: write) - userconfig overridden to just `registry=https://registry.npmjs.org/` (no _authToken for npmjs) - trusted publishers registered per package (verified via npm trust list) Yet npm still bails with 'requires you to be logged in'. Per deepwiki, npm's OIDC auto-discovery checks for _any_ _authToken in the effective config — including for unrelated registries — and if found, assumes the user is already authenticated and skips OIDC. The repo-level `.npmrc` carries `//npm.pkg.github.com/:_authToken=${NODE_AUTH_TOKEN}` for the GHP scope (used by CI consumers of the packages). Even though it's for a different registry, it appears to short-circuit the OIDC path. Nuke the repo `.npmrc` at the start of the publish job (it's only needed for install-side concerns, not publish). Leaves the committed file alone.
npm publish with provenance (sigstore-signed attestation) checks that
the package.json's repository.url matches the GitHub repo the CI is
running in. Without this field, publishes against npmjs return:
422 Unprocessable Entity — Error verifying sigstore provenance bundle:
package.json: "repository.url" is "", expected to match
"https://github.com/abapify/adt-cli" from provenance
Add the standard `repository` (git+https, with `directory` pointing to
the package subpath), `homepage`, and `bugs` fields to every
publishable package.json in the monorepo. One-shot jq patch — shape is
identical across packages, only `directory` varies.
Also fixes `@abapify/asjson-parser` which previously pointed at the old
`abapify/js` repo.
release.yml calls publish.yml via workflow_call with only `contents: read` permission. Without `id-token: write` at the caller job level, the OIDC env vars (ACTIONS_ID_TOKEN_REQUEST_URL/TOKEN) are not available inside the reusable workflow — so npm publish falls through to 'requires you to be logged in', even though publish.yml itself declares id-token: write. Direct workflow_dispatch of publish.yml worked (no caller job to filter permissions); release.yml -> publish.yml did not.
…:* is resolved
nx release publish via npm (required for OIDC trusted publishing) aborted
on 22 of 30 packages with:
The "workspace:" protocol is not yet supported by npm
(npm/rfcs#765). Please ensure you have a
valid setup according to your package manager before attempting to
release packages.
By default nx release keeps workspace-local dependency protocols
verbatim during the version phase, expecting the package manager to
substitute them at publish time. bun and pnpm do this; npm does not —
so a `workspace:*` dep in a published package.json is a hard failure.
Flip `release.version.preserveLocalDependencyProtocols` to false so nx
rewrites `workspace:*` to the resolved semver (e.g. `^0.3.4`) during
`nx release version`. After this, the published package.json files
match what npm expects and the publish job can process all 30 packages.
Foundation packages (zero workspace deps) already published at 0.3.4;
the remaining 22 will follow on the next release.
Root cause of 30x 'requires you to be logged in' on npm publish: when release.yml invokes publish.yml via `uses: ./.github/workflows/ publish.yml` (a reusable workflow call), GitHub's OIDC token claims reflect the CALLER: - workflow_ref: release.yml - job_workflow_ref: publish.yml npm's trusted publisher matcher rejects this mix — even though job_workflow_ref does match our registered trust entries, npmjs also cross-checks the token against the caller chain, and release.yml is not a registered trust target. A directly-dispatched publish.yml in contrast has workflow_ref=publish.yml and matches perfectly (proven by the first 7 foundation packages which did publish under OIDC when publish.yml was triggered via workflow_dispatch). Wire publish.yml to also fire on `push: tags: v*`. The release job in release.yml already does `git push --tags` at the end of its Version/ Changelog/Tag step; that tag push now automatically triggers publish with workflow_ref=publish.yml, which matches the 30 OIDC trust entries we registered via `npm trust github --file publish.yml`. Keep the `workflow_call` entry on publish.yml so it remains callable for ad-hoc use; just don't chain it from release.yml anymore. Comment in release.yml explains why the publish job is gone, so future editors don't re-introduce the broken wiring.
… push won't trigger)
…st helper in server tests Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
…ret assertion Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
…s to reduce cyclomatic complexity Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
… to reduce cyclomatic complexity Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
…reduce complexity Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
…reateRequestHandler Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
… cyclomatic complexity Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
… helpers Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
… with scalar/identity helpers Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
…ationClaims cyclomatic complexity Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
…into helpers Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
…lpers Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
… complex conditionals Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
… overrides Co-Authored-By: Petr Plenkov <petr.plenkov@gmail.com>
feat(adt-server): add bounded ADT server sidecar with REST API
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
✅ Deploy Preview for adt-cli ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
ThePlenkov
changed the title
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review Summary
This PR executes a comprehensive git history rewrite to sanitize internal references for open-source release. The approach and execution appear sound.
Key Changes Verified
- ✅ Internal system references (
ADT→ADT,TRL→TRL) properly replaced - ✅ Corporate references (
abapify.org→abapify.org) sanitized - ✅ Author email mapping applied (
petr.plenkov@gmail.com→petr.plenkov@gmail.com) - ✅ Ticket references (
-*) removed from commit messages and CHANGELOG - ✅ Security guard added in
.husky/pre-committo prevent future leaks
Verification Completed
The PR description indicates thorough verification:
- Test rewrite run completed
git log -Gsearches confirmed no remaining sensitive references- Backup created at
/home/ubuntu/repos/adt-cli-backup.git
Configuration Files Review
Sampled key files including package.json, README.md, .env.example, adt.config.ts, and .github/workflows/ci.yml - all show proper sanitization with no sensitive data or internal references remaining.
Recommendation
This history rewrite is ready to be force-pushed to main. The sanitization is comprehensive and properly executed for open-source release.
Note: As indicated in the PR description, this should be force-pushed rather than merged since it rewrites commit history.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| // This will fail at runtime (no real server), but MUST compile | ||
| try { | ||
| const sysInfo = | ||
| await client.core.http.systeminformation.getSystemInformation(); |
| v == null || | ||
| v === '' || | ||
| (Array.isArray(v) && v.length === 0) || | ||
| (typeof v === 'object' && v !== null && Object.keys(v).length === 0); |
| ); | ||
| } | ||
|
|
||
| if (!current && !loading) { |
|
|
||
| // These property accesses should be type-safe | ||
| const _systemId: string | undefined = sysInfo.systemID; | ||
| const client: string | undefined = sysInfo.client; |
| // This will fail at runtime (no real server), but MUST compile | ||
| try { | ||
| const sysInfo = | ||
| await client.core.http.systeminformation.getSystemInformation(); |
| rootSchema: SchemaLike, | ||
| prefix: string | undefined, | ||
| ): void { | ||
| if (value === undefined || value === null) return; |
| prefix: string | undefined, | ||
| elementForm: string | undefined, | ||
| ): void { | ||
| if (value === undefined || value === null) return; |
| const capitalize = (s: string) => s.charAt(0).toUpperCase() + s.slice(1); | ||
|
|
||
| // Main index file | ||
| const mainIndex = [ |
| console.log('User deleted'); | ||
|
|
||
| // Using composedClient (same API, different structure) | ||
| const composedUsers = await composedClient.users.list(); |
|
|
||
| // Using composedClient (same API, different structure) | ||
| const composedUsers = await composedClient.users.list(); | ||
| const composedPosts = await composedClient.posts.listByUser('123'); |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| BestPractice | 17 high |
| ErrorProne | 4 medium 16 high |
| Security | 2 critical 61 high |
🟢 Metrics 19250 complexity · 2018 duplication
Metric Results Complexity 19250 Duplication 2018
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
|
ThePlenkov
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR contains a full
git filter-reporewrite of the repository history to remove corporate-internal references that are not relevant to the open-source project. It is opened for review only; once approved it will be force-pushed tomain(a normal merge is not possible because the rewritten history has new commit hashes).What was sanitized
ADT→ADT(ABAP Release Manager was a corporate internal system; the broker concept in the repo is now referred to as ADT)adt-api→adt-api,ADT_BROKER_BASE_URL→ADT_BROKER_BASE_URL,ZCL_ADT_REVIEW→ZCL_ADT_REVIEW,x-adt-server-token→x-adt-server-tokenTRL/trl-*→TRL/trl-*(corporate internal SAP SID; replaced with a neutral demo SID)systemSid: 'TRL'→systemSid: 'TRL',trl-adt→trl-adt,trl-rise→trl-rise, transport IDsTRLK900...→TRLK900...-*ticket references removed from commit messages andCHANGELOG.mdabapify.org/abapifyreplaced with neutralabapify.org/abapifypetr.plenkov@gmail.comauthor email mapped topetr.plenkov@gmail.com.husky/pre-commitupdated: removedabapify.orgfrom the bun.lock guard pattern, keptjfrogdetectionVerification done
/home/ubuntu/repos/adt-cli-rewrite-testgit log -Gconfirms noabapify.org, ``,TRL, or `ADT` references with word boundaries remain in the rewritten historypetr.plenkov@gmail.com/home/ubuntu/repos/adt-cli-backup.gitNext step
After review, force-push
clean-historytomainand close this PR.Summary by cubic
Establishes
.agents/as the single source of truth for multi‑agent work and adds shared rules, skills, and workflows for ADT and OpenSpec, with thin IDE stubs, docs-sync tooling, and a CI monitor subagent. This standardizes agent behavior across runtimes and adds developer utilities for docs and CI.New Features
.agents/tree for rules, skills, workflows, and ADT command guides (commands/adt/*).check-structure.ts,generate-stubs.tswith Nx targets (docs-check,docs-stubs)./monitor-ci(MCP tools + decision/state scripts)..cursor/*and.claude/*stubs delegate to.agents/; hooks and settings stay local..dockerignore,.env.example, Dependabot, CodeScene, GitHub hooks.Migration
.agents/*; keep.cursor/*and.claude/*thin and redirecting.bunx nx docs-check adt-cli-docsandbunx nx docs-stubs adt-cli-docs.tmp/for scratch, extensionless imports with bundler resolution./monitor-cifor CI tracking; autonomous commits allowed only within$monitor-ci..agents/commands/adt/*workflows (schema, contract, command, page, ADK).Written for commit f901358. Summary will update on new commits.