Improve the run-e2e skill

.agents/skills/run-e2e/references/devenv.md

@@ -0,0 +1,209 @@
+# Running E2E tests inside a dev env
+
+Why the dev-env path needs a bootstrap script at all, and what each of its steps works around.
+Read this when `devenv_e2e.py` exits non-zero, or when something behaves differently inside the
+container than it does on the host.
+
+## What the container gets for free, and what it does not
+
+`dda env dev start` already shares the parts of the host that matter most for E2E:
+
+- The host's `~/.aws` is bind-mounted read-write at `/home/dd/.aws`, so the container resolves the same
+  profile definitions. Note that this shares configuration, not authorization — see below.
+- `AWS_PROFILE`, `AWS_REGION` and `AWS_DEFAULT_REGION` are forwarded. A stray `AWS_PROFILE` on the
+  host therefore follows you into the container.
+- `pulumi` is on `PATH` at `/usr/local/bin/pulumi`, inherited from the builder base image.
+
+What it does not give you:
+
+- **No E2E config.** `~/.test_infra_config.yaml` does not exist in a fresh container, and
+  `dda inv -- new-e2e-tests.run` refuses to start without it.
+- **No Pulumi state directory.** Only `/var/lib/dd` and `/var/cache/dd` are persistent volumes.
+  `$HOME` is not, so `~/.pulumi` — the backend selection, the plugins, and the local stack state —
+  is recreated for each container and lost when it is removed.
+- **No keypair.** The config's `privateKeyPath` and `publicKeyPath` are host-absolute paths that do
+  not resolve inside the container.
+- **No AWS authorization, despite the shared `~/.aws`.** The framework authenticates as a profile
+  whose `credential_process` runs `aws-vault`, and aws-vault keeps its tokens in its own keyring, not
+  in `~/.aws/sso/cache`. That keyring is local to the container and there is no way to pre-authorize it
+  from the host, so authenticating in a new container means completing an SSO flow inside it, however
+  healthy the host's session is. The env's browser proxy opens that flow on your desktop, so it is
+  workable — but it needs a person, which is why the bootstrap warns before the step that triggers it
+  and gives up rather than waiting indefinitely. The bootstrap probes rather than predicts, because
+  the host's session state carries no information about the container's.
+
+## How `devenv_e2e.py up` closes the gaps
+
+**The keypair is mounted read-only at `/.e2e/`, not under `$HOME`.** On first start the image
+entrypoint runs `chown -R dd: /home/dd` under `set -e`. A read-only mount anywhere inside `$HOME`
+makes that `chown` fail, which aborts the entrypoint, so the container never reaches
+`Server listening on :: port 22` and `dda env dev start` times out waiting for readiness. The error
+surfaces as `Container ... is not ready`, with `chown: ... Read-only file system` further up the
+container log. Mounting outside `$HOME` avoids it entirely.
+
+**A copy of the config is mounted read-only next to the keys, then copied again to `$HOME` inside the
+container.** It has to live at `$HOME/.test_infra_config.yaml`: `getConfigFilePath` in
+`/test/e2e-framework/testing/runner/local_profile.go` and `get_full_profile_path` in
+`/tasks/e2e_framework/config.py` both hardcode that location and neither accepts an override. Since
+`$HOME` cannot hold a read-only mount, a copy is the way in — and it has the nicer property that the
+container works on its own copy, so nothing it does can reach the host's file. The script re-copies
+on every `up`, so host edits propagate, and chmods it to `600` because it carries the Pulumi
+passphrase and the Datadog API key.
+
+`dda env dev fs import` would be the obvious tool for this and is deliberately not used. It builds
+its internal `mv` with paths wrapped in literal double quotes, which the nu shell passes through as
+part of the filename, so it fails with `nu::shell::io::not_found` against a path like
+`'"/home/dd/.test_infra_config.yaml"'` whenever the env's shell is nu — which it is by default under
+`env.dev.universal-shell`. A plain `cp` of arguments the script controls sidesteps it. Expect
+`fs export` to have the same problem, since it shares that quoting.
+
+**The key paths are rewritten in that copy, not overridden per run.** The copy the container gets is
+generated on the host with `privateKeyPath` and `publicKeyPath` pointing at `/.e2e/`; everything else,
+including the keypair name and both passphrases, is carried over untouched. That copy lives at
+`<temp>/dda-e2e-<id>/.test_infra_config.yaml` in a `0700` directory with mode `0600`, since it holds
+the same secrets as the original, and `devenv_e2e.py down` deletes it along with the env.
+
+The alternative — exporting `E2E_AWS_PRIVATE_KEY_PATH` and `E2E_AWS_PUBLIC_KEY_PATH` on the test
+command line, which would work because the runner builds
+`parameters.NewCascadingStore(envValueStore, configFileValueStore)` and so prefers environment
+variables — is avoided on purpose. **On a Windows host, MSYS path conversion rewrites absolute
+POSIX-looking arguments when a command goes through Git Bash**, so `/.e2e/key.pem` arrives inside the
+container as `C:/Users/.../git/.e2e/key.pem` and the run fails when it tries to reach the VM. Keeping
+container paths out of the command line removes the exposure. `E2E_STACK_NAME_SUFFIX` is still passed
+that way because it contains no slashes.
+
+If you do need to type a container path into a command yourself on Windows, prefix it with
+`MSYS_NO_PATHCONV=1`. The full list of overridable values is in `references/flags.md`, sourced from
+`/test/e2e-framework/testing/runner/parameters/store_env.go`.
+
+**`E2E_STACK_NAME_SUFFIX` is set to the host username.** `localProfile.NamePrefix()` derives the
+stack prefix from the OS username, which is `dd` for every developer's container. Without a
+per-developer suffix, two people running the same suite would collide on stack names and on the
+resource names derived from them, in a shared cloud account. Pass `--stack-name-suffix S` to `up` and
+it becomes `<hostuser>-S`, keeping that property.
+
+This is a workaround for a gap in the framework rather than the right fix. `localProfile.NamePrefix()`
+already rejects usernames that do not identify a person — it maps `""` and `root` to `nouser` — and
+`dd` is one it does not know about, while `ciProfile` takes its prefix as injected data rather than
+guessing. The proper fix is a name-prefix runner parameter that `dda inv -- e2e.setup` writes once, after
+which this injection and this paragraph both go away. Until then, two consequences are worth knowing:
+the suffix has to be re-supplied on every command, and the `username` tag on provisioned resources is
+derived separately inside the Pulumi program, so it reads `dd` regardless — which is why hunting for
+orphaned resources by owner tag does not work from the dev-env path.
+
+**Pulumi's backend is established with `dda inv -- e2e.setup --no-interactive`.** The binary is already
+present, but the plugins and the local-backend selection live in the ephemeral home. The
+`--no-interactive` flag is what makes this safe to run inside a container: it does the Pulumi work
+and skips `setup_aws_config` entirely. Never run the interactive form here — see
+`references/setup.md` for what it would do to your AWS keypair. `PULUMI_SKIP_UPDATE_CHECK=true` is
+set alongside it because `pulumi_version()` in `/tasks/e2e_framework/setup/pulumi.py` treats "a newer
+Pulumi exists upstream" as not-installed and re-downloads it into the ephemeral home, where the
+image's copy shadows it anyway.
+
+## The bootstrap only handles AWS targets
+
+It copies in and rewrites the paths for `configParams.aws` alone, and its access check probes the AWS
+profile. A target provisioning into Azure or GCP would reach the container with `configParams.azure` or
+`configParams.gcp` still holding host-absolute key paths, which do not resolve there, and its
+credentials unverified. Run those with `--host` until this handles them; the shape of the fix is the
+same three steps applied per configured provider.
+
+Targets that use the framework's local provisioners need none of this — no keypair, no cloud
+credentials — but they still need the config file and the Pulumi backend, so the bootstrap is still the
+way in. Give it `--no-aws-check` for those: the access check costs an interactive SSO acceptance, and
+gating a local, cost-free run behind that is a toll for nothing. The bootstrap cannot work this out for
+itself, because it never sees which target you are about to run.
+
+## Dev env states
+
+The states and their transitions are documented in
+`/docs/public/tutorials/dev/env.md`. What matters here is
+the preconditions: `dda env dev start` accepts only `nonexistent` and `stopped`, `remove` only `error`
+and `stopped`, `stop` only `started`, and `start` refuses new mount options on a `stopped` env because
+a stopped env keeps its saved configuration. The script handles this:
+
+| State | What it does |
+|---|---|
+| `started` | Nothing |
+| `stopped` | Resumes with `dda env dev start` and no options. Never removes it — its Pulumi state may still be needed |
+| `error` | Refuses. The state means the container exited non-zero, which covers both "never started" and "ran tests, then died" — and the second holds the only copy of those stacks' state, which cannot be inspected while it is down. Recreating is the user's call |
+| `nonexistent` | Starts with the mounts |
+
+To change the mounts on an existing env, remove it first — but only once you are sure it has no live
+stacks.
+
+The script starts envs with `--no-pull`, so a container is created from whatever image is already
+local. That keeps a routine `up` from re-downloading a 12 GB image, at the cost of not picking up
+image fixes on its own. To move to a current image, remove the env and pull explicitly:
+
+```bash
+docker pull datadog/agent-dev-env-linux
+```
+
+## `env.dev.clone-repos` has to be off
+
+With that setting enabled, `dda env dev start` fetches datadog-agent from GitHub instead of
+bind-mounting your checkout, so the container tests the default branch and not your changes. There is
+no `--no-clone` flag to override it for a single command, so `devenv_e2e.py` detects it and asks you
+to turn it off:
+
+```bash
+dda config set env.dev.clone-repos false
+```
+
+It only affects envs created afterwards, so remove and recreate the env for it to take effect.
+
+Working around it by pre-mounting the checkout and letting the clone step skip a non-empty directory
+does not work either: `git dd-clone` exits 1 in that case, `dda env dev start` reports failure, and
+because `save_config()` runs only after a successful start, every later `dda env dev run` falls back
+to the default shell and sends `&&` to nu, which fails. The revision check below is the backstop that
+catches an env built this way regardless.
+
+## The checkout has to be a plain clone named `datadog-agent`
+
+`dda env dev start` derives the repo bind mount from the current directory's parent: it mounts
+`<cwd>/../datadog-agent` at `/repos/datadog-agent` and aborts with
+`Local repository not found: datadog-agent` if that does not exist.
+
+Two consequences:
+
+- The checkout directory must be named `datadog-agent`.
+- **A git worktree does not work.** Its `.git` is a file pointing into the main clone's
+  `.git/worktrees/`, which is outside every mount, so git inside the container fails. That is fatal
+  rather than cosmetic, because `new-e2e-tests.run` reads the commit SHA unconditionally to populate
+  `E2E_COMMIT_SHA`, so the run aborts before its own preflight.
+
+`/.agents/skills/follow-pr/create_devenv.sh` sidesteps the naming rule with
+`--repo "$(git rev-parse --show-toplevel)"`, which makes the mount destination
+`/repos/<absolute path>`. That is a POSIX-only trick: a Windows path contains a drive colon, which
+makes the resulting `-v` spec unparseable. Do not copy it here.
+
+From a worktree, either run from the main clone or use `--host`.
+
+`devenv_e2e.py up` verifies this rather than trusting it: it compares `git rev-parse HEAD` on the host
+with the same command in the container and refuses to continue if they differ. That catches every way
+the container can end up holding the wrong code — a clone instead of a mount, or an env created before
+you switched branches in a way that did not propagate — because testing the wrong revision silently is
+worse than failing.
+
+## Windows hosts
+
+Pass `-t linux-container` to every `dda env dev` command, for the reason given in
+`/docs/public/tutorials/dev/env.md`. The script always
+passes it explicitly, since it is a no-op on other platforms.
+
+## Getting files out
+
+Test output is written to `$HOME/e2e-output/<suite>/<timestamp>` with a `latest` symlink, resolved
+from the home directory of whoever ran the test — so in the dev-env case, the container's. Use
+`docker cp`, because `dda env dev fs export` shares the quoting bug described above:
+
+```bash
+docker cp dda-linux-container-e2e-run:/home/dd/e2e-output ./e2e-output
+```
+
+For anything more interactive, get a shell in the env:
+
+```bash
+dda env dev shell -t linux-container --id e2e-run
+```

.agents/skills/run-e2e/references/flags.md

@@ -0,0 +1,68 @@
+# Flags and runner variables
+
+Read this when the request needs more than a target and a test name — a locally built package or
+image, dev mode, config-map overrides, retries — or when a single value from
+`~/.test_infra_config.yaml` needs overriding for one run.
+
+## Test command
+
+`dda inv -- new-e2e-tests.run --help` describes every flag and is the reference. This file only groups
+them by purpose and records the judgement calls the help text cannot make for you.
+
+`--tags`, `--targets`, `--configparams`, `--run` and `--skip` are repeatable; the rest take one value.
+
+- Choosing what runs: `--targets`, `--run`, `--skip`, `--tags`, `--osdescriptors`, `--no-recursive`
+- Choosing what gets tested: `--agent-image`, `--cluster-agent-image`, `--local-package`,
+  `--pipeline-id`, `--flavor`, `--configparams`
+- Infrastructure lifecycle: `--keep-stack`, `--stack-name-suffix`, `--max-retries`, `--timeout`
+- Output: `--verbose`, `--cache`, `--logs-folder`, `--result-json`, `--junit-tar`, `--extra-flags`
+
+### Judgement calls
+
+- `--targets` resolves against `/test/new-e2e/`, so repeating that prefix inside the target is wrong.
+  It repeats as a flag, but prefer one target per run: each provisions its own stack.
+- Anchor `--run` at both ends. `TestFlare` also selects `TestFlareOpts`.
+- On the dev-env path, give `--stack-name-suffix` to `devenv_e2e.py up` rather than to the test. It sets
+  the same variable the bootstrap uses to keep your stacks distinct from other developers', so passing
+  it here replaces that instead of adding to it.
+- `--keep-stack` in a dev env means keeping the env too, because the stack's state lives inside it.
+- `dda build docker` is the supported way to produce and push an image for `--agent-image`; it prints
+  the matching command when it finishes.
+- Never `--profile ci` on a developer machine. It skips the local-config preflight that exists to fail
+  early with a clear message, so the run fails later and less legibly instead.
+
+## Overriding config values for one run
+
+The runner resolves each parameter through
+`parameters.NewCascadingStore(envValueStore, configFileValueStore)`
+(`/test/e2e-framework/testing/runner/local_profile.go`), so an environment variable wins over
+`~/.test_infra_config.yaml`. This is what lets the dev-env path keep the host's config file while
+redirecting the key paths at the container's copies. The full mapping is
+`/test/e2e-framework/testing/runner/parameters/store_env.go`; the ones that come up:
+
+| Variable | Overrides |
+|---|---|
+| `E2E_KEY_PAIR_NAME` | `configParams.aws.keyPairName` |
+| `E2E_AWS_PRIVATE_KEY_PATH` | `configParams.aws.privateKeyPath` |
+| `E2E_AWS_PUBLIC_KEY_PATH` | `configParams.aws.publicKeyPath` |
+| `E2E_AWS_PRIVATE_KEY_PASSWORD` | `configParams.aws.privateKeyPassword` |
+| `E2E_PULUMI_PASSWORD` | `configParams.pulumi.passphrase` |
+| `E2E_API_KEY`, `E2E_APP_KEY` | `configParams.agent.apiKey` / `.appKey` |
+| `E2E_STACK_NAME_SUFFIX` | Same as `--stack-name-suffix` |
+| `E2E_DEV_MODE` | Same as `--keep-stack` |
+| `E2E_EXTRA_RESOURCES_TAGS` | Extra tags on provisioned resources |
+| `E2E_OUTPUT_DIR` | Where test output and diagnostics are written |
+| `E2E_FAKEINTAKE_IMAGE_OVERRIDE` | The fakeintake image, instead of the pinned tag |
+
+Note `DD_API_KEY` is not part of this — the E2E path uses `E2E_API_KEY` and the `configParams.agent`
+values, which are length-checked (32 and 40 characters).
+
+Pass these through `dda env dev run` with an `env VAR=value ...` prefix, for the reason given in the
+skill's run step.
+
+## Passing raw flags to `go test`
+
+`--extra-flags` is appended verbatim after `-args`, which covers suite-specific flags the task does not
+model. Reach for that rather than calling `go test` yourself: the invoke task is what computes the build
+tags, runs the local-config preflight and exports `PULUMI_CONFIG_PASSPHRASE`, and a run that skips those
+can fail for reasons that have nothing to do with the code under test.

.agents/skills/run-e2e/references/setup.md

@@ -0,0 +1,45 @@
+# One-time E2E setup
+
+What `dda inv -- e2e.setup` does, and why it belongs on the host. Read this before offering to run it.
+
+Read `/docs/public/how-to/test/e2e.md` for what `dda inv -- e2e.setup` does, the AWS access it assumes,
+and the Azure and GCP variants — it is the authoritative guide and this file does not restate it. What follows is only what that guide does not
+cover: which machine to run setup on, and what breaks if you get that wrong.
+
+Two facts from it are worth having in mind here. Setup does not arrange AWS authentication itself, so
+an expired session shows up as an aws-vault error partway through; `aws-vault login
+sso-agent-sandbox-account-admin-8h` **on the host** fixes that, and does nothing for a container's own
+authorization. And re-running is idempotent, so suggesting it is cheap even when you are not sure it is
+needed.
+
+## Always run it on the host
+
+Two reasons, and they both bite hard.
+
+**The keypair name is derived from the OS username.** It is `e2e-<account>-<username>`, and inside a
+dev env the user is always `dd`. So an interactive setup in a container creates a *second* AWS
+keypair, `e2e-agent-sandbox-dd`, in the shared sandbox account rather than reusing yours.
+
+**Worse, it only works once.** `_ensure_aws_keypair` in `/tasks/e2e_framework/setup/aws.py` has four
+branches: keypair present both in AWS and on disk is a no-op; local files only get imported to AWS;
+neither means create both; and **AWS-only is a hard failure**, deliberately, so it never silently
+overwrites a keypair others might be using. A container's home directory is not persistent, so the
+second fresh container hits exactly that branch — AWS has `e2e-agent-sandbox-dd`, the new container
+has no key files — and fails with no way forward except deleting the keypair by hand.
+
+This is why the bootstrap copies the host's config and keypair into the container instead, and why the
+only form of the task that is safe to run inside one is the `--no-interactive` one, which does the
+Pulumi work and skips the AWS and config-file work entirely. `references/devenv.md` has the exact
+command and the reason for the environment variable that accompanies it.
+
+## What the runner reads
+
+The config file is looked up at `$HOME/.test_infra_config.yaml` by both the Go runner and the invoke
+preflight, and neither accepts an override — which is why the dev-env path has to place a copy there
+rather than point at one. Individual values inside it can still be overridden per run through
+environment variables; see `references/flags.md`.
+
+One side effect worth knowing: the preflight keeps `~/.aws/config` in sync by appending the SSO
+profile when it is absent, for example after a role rename. `~/.aws` is bind-mounted read-write into a
+dev env, so a run inside a container can write to the host's file. It is idempotent and skips when the
+profile is already there.