Fix memory leaks reported by Coverity Scan by bryancall · Pull Request #13552 · apache/trafficserver

bryancall · 2026-08-14T18:26:00Z

Part 3 of 3 splitting a Coverity Scan cleanup into independently reviewable pieces. Each leaked allocation is unreachable after the leak, so releasing it changes no observable behavior.

Repeatable command line options

regex_revalidate, remap_purge, xdebug, stale_response and the uri_signing issuer id all overwrote a previously duplicated string when an option was given twice. Every one of these fields is null-initialized first (memset, an init helper, or an explicit = nullptr), so the first pass frees nothing and no string literal ever reaches free. stale_response compares against PLUGIN_TAG by pointer identity, which is the idiom its own destructor already uses.

Ownership the callers were not honoring

TSMgmtStringGet hands back a copy the caller owns (ats_strdup internally). maxmind_acl and an API regression test dropped it. Both are safe on the failure path too, since the function does not write *result on failure and both callers pre-initialize to null.
jax_fingerprint leaked its configuration on three plugin initialization failure paths. Note the reserve_user_arg failure path is deliberately not given a delete, because the config is captured by a TSLogFieldRegister lambda above it.
The YAML remap parser duplicated a redirect URL that nothing owned. parse_format_redirect_url copies what it needs out of the buffer, so the local string's storage can be handed over directly. It does write into that buffer transiently (nul-terminating chunks in place and restoring them), so the buffer must be mutable and outlive the call, which it is and does.
traffic_cache_tool never released its URL set or stripe hash table.

The one hunk where new code runs

~Cache() in CacheTool.cc is the only place here that adds executing code rather than deleting or substituting. Cache holds std::list<std::unique_ptr<Span>>, so copy and move are implicitly deleted and the new destructor cannot double free; the URLset entries are newed in exactly one place and deleted nowhere else. The = nullptr initializer on stripes_hash_table is load bearing, since most instances never build the table.

Two follow-ups I did not fold in, to keep this reviewable: build_stripe_hash_table() overwrites stripes_hash_table without freeing a previous table (harmless today, one call per instance, but now that the field is owning it is worth guarding), and regex_revalidate's -l is a fourth repeatable option that still leaks a TSTextLogObject.

Verification

Clean build with no new warnings and the full unit test suite passing (137/137) on Fedora, GCC 16.1.1.

Two of the touched plugins are not built by default, so verifying them needed extra options: uri_signing requires cjose, and jax_fingerprint defaults to off and needs -DENABLE_JAX_FINGERPRINT=ON. Both are compiled in the run above.

Draft while CI runs.

Update

Pushed a follow-up commit that replaces the explicit deletes here with a unique_ptr, since a reviewer pointed out the deletes were doing by hand what ownership should do on its own.

That turned up a leak the explicit-delete version missed. TSPluginInit's user-argument reservation failure path returned without freeing, which is only correct when the log field callback has captured the configuration, and that capture is conditional on a log symbol being configured. With no --log-field, nothing owned it and it leaked. Reserving the index before registering the log field puts every failure exit inside the span the unique_ptr still owns, so no path has to reason about who else holds a reference. Note that reordering is a behavior change, small but real: a failed reservation no longer leaves a registered log field behind.

Each leak is on a path where the leaked allocation is unreachable afterwards, so releasing it changes no observable behavior. - Plugin option parsing: a repeatable command line option overwrote the previously duplicated string. Affects regex_revalidate, remap_purge, xdebug, stale_response and the uri_signing issuer id. Every one of these fields starts out null, so the first pass frees nothing. - TSMgmtStringGet hands back a copy the caller owns. maxmind_acl and an API regression test dropped it. - jax_fingerprint leaked its configuration on three plugin initialization failure paths. - The YAML remap parser duplicated a redirect URL that nothing owned. parse_format_redirect_url copies what it needs, so the local string's storage can be passed directly. - traffic_cache_tool never released its URL set or stripe hash table. Cache is neither copyable nor movable, so the new destructor cannot double free. Verified with a clean build (no new warnings) and the full unit test suite on Fedora, GCC 16.1.1.

Copilot

Pull request overview

This PR is the final part of a Coverity-driven cleanup series, removing a set of confirmed memory leaks across core code, plugins, and the traffic_cache_tool by ensuring caller-owned allocations are freed and by correctly owning/freeing repeatable option values.

Changes:

Add missing cleanup for owned allocations (notably traffic_cache_tool::Cache URL set and stripe hash table).
Fix leaks caused by repeatable command-line options overwriting previously duplicated strings in multiple plugins.
Free caller-owned strings returned by TSMgmtStringGet and clean up plugin config on initialization failure paths.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/traffic_cache_tool/CacheTool.cc	Initializes `stripes_hash_table` and adds `Cache` destructor cleanup for URL set entries and the stripe hash table.
src/proxy/http/remap/RemapYamlConfig.cc	Avoids leaking a duplicated redirect URL buffer by passing mutable `std::string` storage to `parse_format_redirect_url()`.
src/api/InkAPITest.cc	Frees the caller-owned string returned by `TSMgmtStringGet()` in the regression test.
plugins/xdebug/xdebug.cc	Frees previously set header name when `-h/--header` is specified multiple times.
plugins/remap_purge/remap_purge.cc	Frees prior option values when repeatable options overwrite instance configuration strings.
plugins/regex_revalidate/regex_revalidate.cc	Frees prior option values for repeatable options that overwrite stored configuration strings.
plugins/experimental/uri_signing/config.cc	Frees previously assigned issuer id before overwriting to prevent leaks when multiple issuers set an id.
plugins/experimental/stale_response/stale_response.cc	Frees prior log filename when the log filename option is repeated (avoids leaking prior `strdup`).
plugins/experimental/maxmind_acl/mmdb.cc	Frees the caller-owned `TSMgmtStringGet()` result in both success and fallback paths.
plugins/experimental/jax_fingerprint/plugin.cc	Deletes plugin config on additional initialization failure paths to prevent leaks.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Replaces the explicit delete on each initialization failure path with a unique_ptr that releases at the point ownership actually transfers: to the instance handle in TSRemapNewInstance, and to the log field callback and continuation in TSPluginInit. This also closes a leak in TSPluginInit. The user argument reservation failure path returned without freeing the configuration, which is only correct when the log field callback has captured it, and that capture is conditional on a log symbol being configured. Without one, nothing owned the configuration and it leaked. Reserving the index before registering the log field puts every failure exit inside the span where the unique_ptr still owns the object, so no path needs to reason about who else holds it.

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

parse_format_redirect_url() nul terminates each chunk in place before copying it out and restores the byte afterwards. For a url containing no format specifier the scan runs to the end and that write lands on the terminating nul, and std::string does not permit a caller to assign through the reference at index size(). Pass a buffer this function owns instead, released once the parser returns. The chunk list holds its own copies, so nothing outlives the call, and the allocation that previously leaked here stays fixed.

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

jax_fingerprint: TSPluginInit and TSRemapNewInstance were using the same name for different things, config being the unique_ptr in one and the raw pointer in the other. Both now spell the owning handle owned_config and the released raw pointer config. xdebug: the header name field is only ever assigned a TSstrdup result, never a literal, so storing it as char * removes the const_cast at the free and a second one at TSUserArgSet. stale_response: the log filename field defaulted to the static PLUGIN_TAG, which forced both a const_cast to free it and a pointer identity comparison to decide whether freeing was safe. It now defaults to null and the tag is substituted where the name is used, so freeing is unconditional and the comparison is gone.

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/proxy/http/remap/RemapYamlConfig.cc:395

The new comment implies the previous code risked mutating std::string storage, but it was already passing an ats_strdup(url.c_str()) buffer (mutable) into parse_format_redirect_url(). Consider rewording to focus on the actual issue being fixed here (ownership/leak of the duplicated buffer), and the requirement that the parser needs a mutable, caller-owned buffer that can be freed after parsing.

  // parse_format_redirect_url() nul terminates each chunk in place before copying it out, and for a
  // url with no format specifier that write lands on the terminating nul, which std::string does not
  // allow a caller to assign. Give it a buffer we own instead, and release it once it returns; the
  // chunk list holds copies.
  ats_scoped_str redirect_url(ats_strdup(url.c_str()));
  url_mapping->redir_chunk_list = redirect_tag_str::parse_format_redirect_url(redirect_url.get());

plugins/experimental/stale_response/stale_response.cc:1075

This plugin is now managing log_info.filename with strdup/free, while other touched areas use ATS allocators (TSstrdup/TSfree or ats_strdup/ats_free). To reduce the risk of allocator mismatches and improve consistency/portability within ATS code, consider switching this pair to the ATS wrappers and updating the corresponding free site (in the ConfigInfo destructor) to match.

      // The option may be repeated; release the previously duplicated name first.
      free(plugin_config->log_info.filename);
      plugin_config->log_info.filename = strdup(optarg);
      break;

src/traffic_cache_tool/CacheTool.cc:695

Now that Cache owns URLset entries and stripes_hash_table, manual deletion in the destructor increases the chance of future leaks/allocator mismatches if these members evolve (e.g., multiple builds of the table, early returns, or additional owners). Consider encoding ownership directly in the types: store URLs as std::unique_ptr<ts::CacheURL> (or a container of owning smart pointers) and wrap stripes_hash_table in a smart pointer with an ats_free deleter (or use a standard container like std::vector<unsigned short> if sizing allows). This would remove the need for an explicit destructor for these fields.

Cache::~Cache()
{
  // The URL set and the stripe hash table are owned solely by this instance.
  for (auto *url : URLset) {
    delete url;
  }
  ats_free(stripes_hash_table);
}

The redirect URL comment described a hazard the final code never has: it explained why passing std::string storage was wrong, which is not visible in this change at all. It now states what is true of the code as written, that the parser needs a mutable buffer, keeps no pointer into it, and that the duplicate previously leaked. stale_response was managing the log filename with strdup and free while every other allocation in the file uses the ATS wrappers. Switched to TSstrdup and TSfree so the pair matches its neighbours and cannot be mixed up with the wrong deallocator later. build_stripe_hash_table() replaced the stripe hash table without releasing the previous one. Harmless while each Cache builds it once, but the destructor now owns that pointer and the function is called from two places, so it releases any table already installed.

bryancall · 2026-08-18T12:13:37Z

Copilot posted three suppressed notes in its last review rather than inline threads, so there is nothing to reply to in place. Recording what I did with them here.

Took two of them in e860bb7:

The redirect URL comment was describing a hazard the final code never has. It explained why passing std::string storage would be wrong, which is not visible in this diff at all, because the net change is from an unowned ats_strdup to a scoped one. Reworded to say what is true of the code as written: the parser needs a mutable buffer, it keeps no pointer into that buffer, and the duplicate previously leaked.
stale_response was managing the log filename with strdup/free while every other allocation in that file uses the ATS wrappers. Those three sites were the only bare ones, so they are now TSstrdup/TSfree.

Partly took the third. The suggestion was to encode ownership in the types for Cache, holding URLset as owning smart pointers and wrapping stripes_hash_table with an ats_free deleter. I agree that is the better design, but it changes the type of a public member that two unrelated code paths iterate, which is more than a leak fix should carry, so I would rather do it separately.

What I did take is the concrete risk behind it: build_stripe_hash_table() replaced the table without releasing the previous one, and it is called from two places. That is harmless while each Cache builds the table once, but the destructor added here makes that pointer owning, so the function now frees any table already installed.

Verified on Fedora with GCC 16.1.1: clean build, no new warnings, 140/140 unit tests, with xdebug, stale_response, jax_fingerprint and uri_signing all compiled.

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (5)

plugins/experimental/stale_response/stale_response.h:52

Changing filename from a non-null default (PLUGIN_TAG) to a nullable owning pointer forces callers to remember to add fallbacks everywhere and makes the field’s semantics less clear ("effective log filename" vs "optional override"). A more maintainable approach is to store this as a value type (e.g., std::string filename_override) or an ATS RAII wrapper (e.g., ats_scoped_str), and compute the effective filename when needed (empty => PLUGIN_TAG). That keeps the invariant simple and avoids having to manage raw ownership in the destructor.

  char           *filename               = nullptr;

plugins/experimental/stale_response/stale_response.h:68

Changing filename from a non-null default (PLUGIN_TAG) to a nullable owning pointer forces callers to remember to add fallbacks everywhere and makes the field’s semantics less clear ("effective log filename" vs "optional override"). A more maintainable approach is to store this as a value type (e.g., std::string filename_override) or an ATS RAII wrapper (e.g., ats_scoped_str), and compute the effective filename when needed (empty => PLUGIN_TAG). That keeps the invariant simple and avoids having to manage raw ownership in the destructor.

    TSfree(this->log_info.filename);

plugins/experimental/uri_signing/config.cc:288

This continues to use manual malloc/strcpy management in code that otherwise appears to rely on ATS allocation helpers elsewhere. Consider replacing this with a single duplication helper (e.g., TSstrdup/TSfree or ats_strdup/ats_free, consistent with the rest of the plugin) to reduce allocator-mismatch risk and simplify the code (fewer lines, less room for mistakes).

          /* An earlier issuer may have set an id; free it so it is not leaked. Last issuer wins. */
          free(cfg->id);
          cfg->id = static_cast<char *>(malloc(strlen(id) + 1));
          strcpy(cfg->id, id);

src/traffic_cache_tool/CacheTool.cc:211

The new destructor makes ownership explicit, but it also introduces manual lifetime management for two members. To make this harder to regress in future edits (e.g., early returns, new mutation sites, additional owning fields), consider switching these members to RAII: store URL entries as smart pointers (or a dedicated owning container) and wrap stripes_hash_table in a smart pointer with an ats_free deleter. That removes the need for a custom destructor and makes ownership self-documenting.

  unsigned short                    *stripes_hash_table = nullptr;

src/traffic_cache_tool/CacheTool.cc:695

The new destructor makes ownership explicit, but it also introduces manual lifetime management for two members. To make this harder to regress in future edits (e.g., early returns, new mutation sites, additional owning fields), consider switching these members to RAII: store URL entries as smart pointers (or a dedicated owning container) and wrap stripes_hash_table in a smart pointer with an ats_free deleter. That removes the need for a custom destructor and makes ownership self-documenting.

Cache::~Cache()
{
  // The URL set and the stripe hash table are owned solely by this instance.
  for (auto *url : URLset) {
    delete url;
  }
  ats_free(stripes_hash_table);
}

A nullable owning char pointer put the burden on every caller to remember the PLUGIN_TAG fallback, and left the destructor responsible for a raw deallocation. Holding it as a std::string states the intent instead: empty means the default tag, and the effective name is computed where it is used. This also makes the defect that started this impossible rather than fixed. Repeating the option previously leaked the earlier duplicate; assigning to a string releases it, so there is no ownership left to get wrong.

bryancall · 2026-08-18T12:36:53Z

Another round of suppressed Copilot notes, so again there are no threads to reply to. Recording the dispositions.

Took the stale_response one in 2b3c27c. The point about a nullable owning pointer pushing the PLUGIN_TAG fallback onto every caller was right, and holding it as a std::string is better than either version I had: empty means the default tag, the effective name is computed where it is used, and the destructor no longer owns anything. It also turns the defect that started this from fixed into impossible, since assigning to a string releases the previous value and there is no ownership left to get wrong.

Declined the uri_signing one. The suggestion was to replace the malloc/strcpy pair for cfg->id with TSstrdup/TSfree for consistency. That file is C style throughout: config_new() allocates with malloc and config_delete() releases with free, so switching only the duplication site would create exactly the allocator mismatch the note warns about. Making the whole file consistent is a reasonable cleanup but it is not this change.

Declined the CacheTool one again, same reasoning as last round: encoding ownership in the member types is the better design, but it changes the type of a public member that two unrelated code paths iterate, and I would rather not carry that in a leak fix. The concrete risk it names, a second build_stripe_hash_table() call leaking the previous table, is already handled in e860bb7.

Verified: Debug build clean with 140/140 unit tests, and a Release build with warnings as errors produces no errors beyond the two that master already produces with this compiler.

Note on the CentOS failure on the previous commit: ci-centos is the only Release configuration in the matrix and was the only platform that failed, with Debian, Fedora, Rocky and Ubuntu all green on the same commit. I could not reproduce it locally, so I am watching whether this push clears it.

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/traffic_cache_tool/CacheTool.cc:695

Manual deletion/freeing in both the destructor and build_stripe_hash_table() works, but it makes ownership easy to regress (e.g., missed frees on future early-return paths, or additional sites that overwrite stripes_hash_table). Consider expressing ownership in the types (e.g., a smart pointer with an ats_free deleter for stripes_hash_table, and an owning container for URLs) so cleanup happens automatically and the overwrite case becomes a simple reset.

Cache::~Cache()
{
  // The URL set and the stripe hash table are owned solely by this instance.
  for (auto *url : URLset) {
    delete url;
  }
  ats_free(stripes_hash_table);
}

src/traffic_cache_tool/CacheTool.cc:1019

Manual deletion/freeing in both the destructor and build_stripe_hash_table() works, but it makes ownership easy to regress (e.g., missed frees on future early-return paths, or additional sites that overwrite stripes_hash_table). Consider expressing ownership in the types (e.g., a smart pointer with an ats_free deleter for stripes_hash_table, and an owning container for URLs) so cleanup happens automatically and the overwrite case becomes a simple reset.

  // The destructor owns this table, so release any table a previous call installed.
  ats_free(stripes_hash_table);
  stripes_hash_table = ttable;

plugins/experimental/uri_signing/config.cc:288

This is safe as written, but allocating with malloc + copying with strcpy is more error-prone than necessary. Prefer a single-call duplication routine (e.g., strdup/project equivalent) to reduce the chance of future length/copy mismatches and to keep allocation/copy logic consistent.

          /* An earlier issuer may have set an id; free it so it is not leaked. Last issuer wins. */
          free(cfg->id);
          cfg->id = static_cast<char *>(malloc(strlen(id) + 1));
          strcpy(cfg->id, id);

src/proxy/http/remap/RemapYamlConfig.cc:392

Use the conventional spelling 'NUL-terminates' instead of 'nul terminates' in this comment.

  // parse_format_redirect_url() nul terminates each chunk in place before copying it out, so it

Holding the stripe hash table in ats_scoped_mem removes both manual frees: the destructor no longer releases it, and installing a new table releases the previous one as part of the assignment rather than relying on a caller to remember. Also replaces a malloc and strcpy pair for the issuer id with strdup, which pairs with the free already in config_delete(), and corrects the spelling of null-terminates in the redirect URL comment to match the rest of the tree.

bryancall · 2026-08-18T13:00:22Z

Third round of suppressed notes. Two of them I had declined before, and on a closer look one of those was worth taking after all.

Took the stripe hash table one in 639cea0. I had been declining this as a type redesign, but there is an in-tree idiom that makes it a two line change: ats_scoped_mem<T> in tscore/ink_memory.h. Its assignment operator clears the existing resource first, so holding the table in one removes both manual frees at once, the destructor release and the free-before-reassign I added in e860bb7. That is a better answer than either version I had, so thanks for pressing on it.

Took the uri_signing one, which changed since last round. Previously it suggested TSstrdup/TSfree, which I declined because that file allocates with malloc and releases with free throughout, so switching only the duplication site would have created the very mismatch the note warned about. This round it suggests a plain single-call duplication instead, and strdup pairs correctly with the free already in config_delete(), so malloc plus strcpy is now just strdup.

Took the spelling note. The tree overwhelmingly writes "null-terminated" (47 occurrences against 2 of the form I used), so the comment now matches.

Still declining the URL set half. URLset is a public member iterated by two code paths that are otherwise untouched here, so converting it to an owning container changes their loops as well. That is a reasonable cleanup on its own and I would rather it be its own change than ride along in a leak fix.

Verified: Debug build clean with 140/140 unit tests, and a Release build with warnings as errors produces no errors beyond the two master already produces with this compiler.

Copilot

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

Suppressed comments (4)

src/traffic_cache_tool/CacheTool.cc:1017

With an owning RAII wrapper like ats_scoped_mem, using an explicit ownership-transfer API (e.g., reset(...)) is clearer than relying on assignment semantics. This reduces the chance of accidental misuse if ats_scoped_mem’s operators change or if a different scoped wrapper type is substituted later.

  // Assigning releases any table a previous call installed.
  stripes_hash_table = ttable;

src/traffic_cache_tool/CacheTool.cc:694

Manual delete over a container of raw owning pointers is fragile (ownership is easy to violate elsewhere). Consider storing ownership directly in the container (e.g., std::unique_ptr<ts::CacheURL> with appropriate hashing/equality) so cleanup is automatic and ownership is enforced by the type system.

Cache::~Cache()
{
  // The URL set is owned solely by this instance; the stripe hash table owns itself.
  for (auto *url : URLset) {
    delete url;
  }
}

plugins/experimental/stale_response/stale_response.cc:1115

TSTextLogObjectCreate returns a status; ignoring it can leave log_info.object in an unusable state and cause later log writes to fail unpredictably. Handle the return value (e.g., log an error and disable logging / clear the object) to make failures deterministic.

    char const *const log_filename =
      plugin_config->log_info.filename_override.empty() ? PLUGIN_TAG : plugin_config->log_info.filename_override.c_str();
    SRDBG(TAG, "[%s] Logging to %s", __FUNCTION__, log_filename);
    TSTextLogObjectCreate(log_filename, TS_LOG_MODE_ADD_TIMESTAMP, &(plugin_config->log_info.object));

plugins/experimental/uri_signing/config.cc:287

strdup is POSIX but not standard C/C++; if this plugin targets non-POSIX platforms/toolchains, this can reduce portability. Consider using the project’s allocator helpers (e.g., ats_strdup/ats_free or TSstrdup/TSfree, as appropriate for this allocation’s lifetime) or storing the id as std::string to avoid platform-API dependencies.

          /* An earlier issuer may have set an id; free it so it is not leaked. Last issuer wins. */
          free(cfg->id);
          cfg->id = strdup(id);

bryancall · 2026-08-18T13:21:05Z

Fourth round of suppressed notes. I am declining all four, and I want to be specific about why rather than just saying no.

reset(...) on the stripe hash table. ats_scoped_mem does not have a reset(). The public API of ats_scoped_resource is clear(), get(), release(), and operator=, so assignment is the ownership transfer API for this type. The suggestion cannot be applied as written.

Owning container for URLset. Fourth time for this one and my answer is unchanged: URLset is a public member iterated by two code paths this change does not otherwise touch, so converting it edits their loops too. Still worth doing, still not in a leak fix.

Unchecked TSTextLogObjectCreate. This one is legitimate and already identified. It was triaged out of this pull request deliberately: adding a failure branch and an error log is a behavior change, and this pull request is scoped to leaks whose fix is not observable. It is sitting in the behavioral batch with the other changes of that kind, and it will come with an error path rather than as a drive-by.

strdup portability. This contradicts the previous round, which asked for exactly this change: it suggested a single-call duplication and I used strdup. On the substance, plain strdup already has 11 call sites across src/ and plugins/, and this file allocates with malloc and releases with free throughout, so strdup is both consistent locally and established in the tree.

More generally, this is the fourth automated review pass on this pull request and the notes have moved from real defects to preferences about the shape of the fixes, including one that contradicts an earlier round and one that targets an API the type does not have. The earlier passes were genuinely valuable and caught a real regression I introduced. This one is not moving the code forward, and every push re-runs the full matrix, so I am going to leave the code here and let human review drive from this point.

State: all checks green on the current head, with the Release build (the only one in the matrix, on CentOS) passing.

bryancall · 2026-08-18T20:50:40Z

[approve ci autest 2]

bryancall requested a lite review from Copilot August 14, 2026 18:26

Copilot started reviewing on behalf of bryancall August 14, 2026 18:26 View session

Copilot AI reviewed Aug 14, 2026

View reviewed changes

bryancall added this to the 11.0.0 milestone Aug 14, 2026

bryancall added Cleanup Coverity Leak labels Aug 14, 2026

bryancall self-assigned this Aug 14, 2026

bryancall requested a lite review from Copilot August 15, 2026 05:40

Copilot started reviewing on behalf of bryancall August 15, 2026 05:40 View session

Copilot AI reviewed Aug 15, 2026

View reviewed changes

Comment thread src/proxy/http/remap/RemapYamlConfig.cc Outdated

bryancall marked this pull request as ready for review August 17, 2026 17:26

Copilot AI review requested due to automatic review settings August 17, 2026 17:26

Copilot started reviewing on behalf of bryancall August 17, 2026 17:29 View session

Copilot AI reviewed Aug 17, 2026

View reviewed changes

bryancall requested a review from JosiahWI August 17, 2026 21:57

JosiahWI requested changes Aug 18, 2026

View reviewed changes

Comment thread plugins/experimental/jax_fingerprint/plugin.cc Outdated

Comment thread plugins/experimental/stale_response/stale_response.cc Outdated

Comment thread plugins/xdebug/xdebug.cc Outdated

Copilot AI review requested due to automatic review settings August 18, 2026 11:51

JosiahWI previously approved these changes Aug 18, 2026

View reviewed changes

Copilot AI reviewed Aug 18, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 18, 2026 12:13

bryancall dismissed JosiahWI’s stale review via e860bb7 August 18, 2026 12:13

Copilot started reviewing on behalf of bryancall August 18, 2026 12:29 View session

Copilot AI reviewed Aug 18, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 18, 2026 12:36

Copilot AI reviewed Aug 18, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings August 18, 2026 12:59

Copilot started reviewing on behalf of bryancall August 18, 2026 13:07 View session

Copilot AI reviewed Aug 18, 2026

View reviewed changes

Copilot started reviewing on behalf of bryancall August 18, 2026 13:30 View session

Copilot started reviewing on behalf of bryancall August 18, 2026 13:47 View session

JosiahWI approved these changes Aug 18, 2026

View reviewed changes

Uh oh!

Conversation

bryancall commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Repeatable command line options

Ownership the callers were not honoring

The one hunk where new code runs

Verification

Update

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

bryancall commented Aug 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

bryancall commented Aug 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

bryancall commented Aug 18, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

bryancall commented Aug 18, 2026

Uh oh!

bryancall commented Aug 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

bryancall commented Aug 14, 2026 •

edited

Loading